Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Extend MerkleProof to support unsorted Merkle tree #2815

Closed
wants to merge 4 commits into from
Closed

Extend MerkleProof to support unsorted Merkle tree #2815

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5b0233c
Extend MerkleProof to support unsorted Merkle tree
k06a Aug 7, 2021
ab80341
Introduce verifyAndRecoverIndex
k06a Aug 7, 2021
c9e587e
Fix linter
k06a Aug 7, 2021
d10badc
Fix name typo
k06a Aug 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 49 additions & 0 deletions contracts/utils/cryptography/MerkleProof.sol
View file
Open in desktop
Expand Up @@ -23,14 +23,63 @@ library MerkleProof {
bytes32 root,
bytes32 leaf
) internal pure returns (bool) {
(bool success, ) = verifyAndRecoverIndex(proof, root, leaf);
return success;
}

/**
* @dev Returns true if a `leaf` can be proved to be a part of a Merkle tree
* defined by `root`. For this, a `proof` must be provided, containing
* sibling hashes on the branch from the leaf to the root of the tree. Each
* pair of leaves and each pair of pre-images are assumed to be sorted.
*/
function verifyAndRecoverIndex(
bytes32[] memory proof,
bytes32 root,
bytes32 leaf
) internal pure returns (bool, uint256) {
bytes32 computedHash = leaf;
uint256 index = 0;

for (uint256 i = 0; i < proof.length; i++) {
bytes32 proofElement = proof[i];

if (computedHash <= proofElement) {
// Hash(current computed hash + current element of the proof)
computedHash = keccak256(abi.encodePacked(computedHash, proofElement));
} else {
// Hash(current element of the proof + current computed hash)
computedHash = keccak256(abi.encodePacked(proofElement, computedHash));
index |= 1 << i;
Copy link
Collaborator

@Amxx Amxx Sep 10, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Amxx I see, will think of it.

Copy link
Collaborator

@Amxx Amxx Oct 12, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@k06a Anything new on your end?

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could and should be stored in immutable paired with merkleRoot

}
}

// Check if the computed hash (root) is equal to the provided root
return (computedHash == root, index);
}

/**
* @dev Returns true if a `leaf` can be proved to be a part of a Merkle tree
* defined by `root`. For this, a `proof` must be provided, containing
* sibling hashes on the branch from the leaf to the root of the tree. Each
* pair of leaves and each pair of pre-images are assumed to be sorted.
* Instead of assuming pair of leaves and pair of pre-images are being sorted,
* this method relies on leaf index to recover merkle path directions.
*/
function verify(
bytes32[] memory proof,
bytes32 root,
bytes32 leaf,
uint256 leafIndex
) internal pure returns (bool) {
bytes32 computedHash = leaf;

for (uint256 i = 0; i < proof.length; i++) {
bytes32 proofElement = proof[i];

if ((leafIndex >> i) & 1 == 0) {
// Hash(current computed hash + current element of the proof)
computedHash = keccak256(abi.encodePacked(computedHash, proofElement));
} else {
// Hash(current element of the proof + current computed hash)
computedHash = keccak256(abi.encodePacked(proofElement, computedHash));
Expand Down