1 << 32 to (1 << 32) - 1 to allow compiling on 32-bit targets

[KaffinPX]

No description provided.

[jan-ferdinand]

I appreciate the intention. However, the suggested solution does not work with the current implementation of Merkle trees, as the number of leafs and the number of nodes must be a power of 2.
It is definitely possible to achieve what you want. However, it requires bigger, potentially even substantial refactors.

[coveralls]

coverage: 97.851%. remained the same
when pulling 7b9155b on KaffinPX:master
into c67226f on Neptune-Crypto:master.

[jan-ferdinand]

It's interesting to see that no tests fail. That means there are tests missing.

Elaborating a bit, in the current implementation, the number of leafs in a Merkle tree must be a power of two. To simplify the index arithmetic, the number of non-leaf nodes is equal to the number of leafs. As an artifact of this design choice, the node with index 0 is never used. In order to support Merkle trees with $2^{31}$ leafs on 32-bit platforms, the representation of Merkle trees would need to change: the number of non-leaf nodes would be 1 less than the number of leafs. This would complicate the index arithmetic; however, that trade-off might be well worth it.

[aszepieniec]

If the objective is to allow compiling to 32-bit targets, what options are there that do not involve changing indexation?

[jan-ferdinand]

If the objective is to allow compiling to 32-bit targets, what options are there that do not involve changing indexation?

1. Limit Merkle trees to have at most $2^{30}$ leafs, which would imply they have at most $2^{31}$ nodes (given the current design).
2. Use a backing storage that is not std::vec::Vec if the number of nodes exceeds usize::MAX, for example by using two Merkle trees of smaller height internally.

Note that changing the internal indexing scheme does not imply a (big) change to the public API since the backing storage, i.e., the field nodes, is private. It can be changed to a custom type that performs the necessary index translation. Then, from the point of view of the Merkle tree implementation, the new type is used, but the index arithmetic doesn't change. Similarly, the public API doesn't change (much¹).

Footnotes

1. The pub fn nodes(&self) -> &[Digest] could be a problem: it exposes what should be implementation details. ↩

[aszepieniec]

In regards to pub fn nodes(&self) -> &[Digest] ...

• This strikes me as a rust antipattern. I would expect a function that returns an iterator instead.
• I can imagine it is convenient to write, *e.g. *, tree.nodes()[index ^ 1] but that might as well be tree.node(index ^ 1).

[aszepieniec]

I won't shed tears if that method gets refactored into the dustbin.

[KaffinPX]

btw I know 32 bit is not widely used on practice anymore but compiling into WASM32 is still a practical application.

[jan-ferdinand]

Here's a limitation I hadn't been aware of so far: std::vec::Vec can only allocate at most isize::MAX bytes.¹² Since each node in a Merkle tree requires 40 bytes, using a single Vec allows Merkle trees with $2^{\texttt{ilog}_2(\texttt{isize::MAX} / 40)} = 2^{25}$ nodes at most, which translates to a maximum tree height of 24 on 32-bit systems.³

A correct first step is to relax the requirement of constant MAX_NUM_NODES, enabling compilation on 32-bit targets. If we want to guarantee that Merkle trees have the same maximum height across 32-bit and 64-bit targets, we need to shrink the maximum tree height; however, given the above findings, we might want to re-evaluate that goal (@Sword-Smith).

Footnotes

1. documentation, playground ↩

2. I don't know why this is isize, not usize. ↩

3. On 64-bit systems, the maximum tree height is 56. ↩

[jan-ferdinand]

@KaffinPX, what is the target triple you had in mind?

For the record (and to help me remember), on this branch, both cargo clippy and cargo build execute successfully for both the targets wasm32-wasip2 and wasm32-unknown-emscripten. Running cargo test fails for both those targets, albeit for different reasons. The compilation failure happens in some dependency in both cases.

[KaffinPX]

@KaffinPX, what is the target triple you had in mind?

For the record (and to help me remember), on this branch, both cargo clippy and cargo build execute successfully for both the targets wasm32-wasip2 and wasm32-unknown-emscripten. Running cargo test fails for both those targets, albeit for different reasons. The compilation failure happens in some dependency in both cases.

wasm32-unknown-unknown via wasm-bindgen.