feat: Provide sequential Merkle tree builder

[jan-ferdinand]

Previously, the only way to construct a Merkle tree was to use CpuParallel. However, in some cases, using parallelism does not make sense or even adversely effects the program.

Add the Merkle tree maker CpuSequential, which constructs the entire tree sequentially.

Please note that at first glance, it might seem as if this PR increases the DEFAULT_PARALLELIZATION_CUTOFF. However, this change is merely to offset the changes around it's only use I introduce in this PR.

I'm not fully convinced that the new struct CpuSequential should go into the crate's prelude. Opinions welcome.

[Sword-Smith]

Looks good. One comment: Maybe also verify that the roots agree in the tests, where you Merkle trees built in parallel and serial? To me, it's not obvious what equality for two Merkle trees mean. If the tests compared the roots, that doubt would be removed from my mind.

I don't have an opinion on the prelude question.

[jan-ferdinand]

To me, it's not obvious what equality for two Merkle trees mean.

MerkleTree derives Eq, so equality is deferred to the underlying nodes: Vec<Digest>. I can also just compare equality of the roots, since this should imply equality of all the rest.¹ I think this is a slightly weaker test, but don't have strong feelings in this regard.

Footnotes

1. Of course, it's possible to craft Merkle trees of different heights with identical roots, but I don't assume the testing framework is adversely motivated. 😃 ↩

[coveralls]

coverage: 97.72% (-0.1%) from 97.843%
when pulling cf6a398 on sequential_merkle_tree_maker_pr
into dc6cab6 on master.

[aszepieniec]

I'm not fully convinced that the new struct CpuSequential should go into the crate's prelude. Opinions welcome.

Why not? I would treat it the same as CpuParallel.

Incidentally, this addition provides an opportunity to get rid of the unnecessary and annoying trait MerkleTreeMaker altogether, along with the dysfunctional and poorly named struct CpuParallel. What matters is the functions. Any thoughts on that matter?

[jan-ferdinand]

I'm happy to extend the scope thusly. Following a rather established naming convention, MerkleTree::new could be the newly introduced sequential Merkle tree building, and MerkleTree::par_new could be what is currently MerkleTree::new::<MerkleTreeMaker> but with the type parameter removed. Objections?

[aszepieniec]

👍

[Sword-Smith]

Can we make it explicit that the sequential constructor is sequential, please? E.g. sequential_new

[jan-ferdinand]

That would be MerkleTree::sequential_new and MerkleTree::par_new, right? This draws attention to any location constructing Merkle trees sequentially, which we assume to be the exception. 👍

[jan-ferdinand]

Do you want to (re-)review the changes?