Extend MerkleProof to support unsorted Merkle tree

[k06a]

Let's discuss and if we would decide to have this functionality then would add tests and docs.

PR Checklist

• Tests
• Documentation
• Changelog entry

[Amxx]

Hello @k06a

Usually, we would discuss that in an issue before starting a PR. Can you give more details about the use case? Have you faced a situation where sorting the Merkle tree was an issue? While it is not a situation that we might ever face, this technically adds a restriction on the height of the tree.

I'd love to know more about Merkle tree management libraries, which might justify this alternate version.

[k06a]

@Amxx index-based Merkle Trees could be useful for bit invalidation. In case of Sorted Merkle Tree it is also possible to recover index by restoring index bit on each operation.

Also what is interesting is to use 128 bits for hashes to 2x reduce calldata, but solidity implementation costs much more somehow for me. Only using Solidity assembly I was able to have 2x gas cheaper implementation.

[k06a]

@Amxx please check index recovery technique I just submitted. It could be useful for the bit invalidation, what Uniswap had in their Merkle Distributor.

[Amxx]

@Amxx please check index recovery technique I just submitted. It could be useful for the bit invalidation, what Uniswap had in their Merkle Distributor.

Uniswap uses our Merkle proof library, without the additional feature you are proposing. They are also using a bitmap structure, which we also provide

I'm not sure what in uniswap would have benefited from unsorted merkle trees

Edit: I just saw the verifyAndRecoverIndex function. This might indeed be useful !

[k06a]

@Amxx I meant they we needed to include index into nodes, but unique index could be reconstructed during proof validation.my PR now proposes 2 different things: index reconstruction and unsorted merkle proofs validation.

[Amxx]

These are 2 different features, that I think should be discussed independently, and have their own PR. This is why we usually recommend filling issues first.

I think index reconstruction is nice indeed. @frangio, any opinion ?

[Amxx]

Note: this indexing me logic doesn't guarantee uniqueness of indexes because proofs are processed from the leaf up to the root, and not all branches have equal length.

Example:

   Root
   /  \
  X    C
 / \
A   B

Path from B to root is :

• right for B to X → index |= 1 << 0
• left from X to root → no change

in the end, index = 1

Path from C to root:

• right from C to root → index |= 1 << 0

in the end, index = 1

This can be fixed by using index |= 1 << (proof.length -1) or using the solution proposed in #2841

[Amxx]

AFAIK, there is no simple way to retrieve the leaf index with the information we have here.

[k06a]

@Amxx I see, will think of it.

[Amxx]

@k06a Anything new on your end?

I'm considering merging #2841 and closing this ... but I'd love your feedback before I do so.

[frangio]

This PR adds a variant of verify for unsorted trees, so it is not redundant with #2841.

[k06a]

The only solution I see here is to add one more argument: treeHeight

[k06a]

It could and should be stored in immutable paired with merkleRoot

[Amxx]

This PR includes features requests:

• Being able to verify proofs for "unsorted" trees by providing an index that acts as a "path" to properly rebuild the root
• Being able to rebuild a similar index when verifying proofs in "sorted" trees.

We understand the values in both proposals, but before committing to maintaining a codebase, we want to make sure that the index we use has the right mathematical properties, and is supported by tooling. So far, it is not clear is a specific construct is "standard", and we want to investigate further before we commit to merging any code.

We are closing this PR in favor of an issue where the properties of these objects can be discussed.