verifyMultiNamespaced #326
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
v1.8.2
WalkthroughThis update enhances a smart contract library by integrating the "forge-std" submodule, which enriches the development environment. New bit manipulation functions and improved Merkle Tree verification methods have been introduced, alongside expanded testing capabilities. The project now supports both "forge-std" and "openzeppelin-contracts-upgradeable," providing developers with more robust tools. Additionally, documentation clarifications have been made to facilitate effective usage of the new features. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant SW as Software
participant Utils as Utility Functions
participant Merkle as Merkle Tree
participant Test as Testing Framework
Dev->>SW: Integrate forge-std
SW->>Utils: Update remappings
SW->>Merkle: Add Utilities
SW->>Merkle: Verify Multi-Proofs
Merkle->>Test: Retrieve Proof Data
Test-->>Merkle: Send Proof Data
Merkle-->>SW: Verify Proofs
SW-->>Dev: Testing Complete
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Outside diff range, codebase verification and nitpick comments (3)
src/lib/tree/namespace/NamespaceMerkleTree.sol (1)
Line range hint
82-93: Replace the removed_nextSubtreeSizefunction.The function
verifyMultiHashescalls_nextSubtreeSize, which has been removed. Provide a replacement function or logic to handle subtree size calculation.- uint256 subtreeSize = _nextSubtreeSize(leafIndex, proof.beginKey); + uint256 subtreeSize = 1 << (proof.beginKey - leafIndex).bitLength() - 1;src/lib/tree/namespace/test/NamespaceMerkleMultiproof.t.sol (2)
92-95: Clarify the source of the byte arrays.The comments indicate that the byte arrays were generated using Rust code and the JSON test vector is located at a specific path. Ensure that this information is accurate and accessible to maintainers.
97-99: Improve readability of byte arrays.The byte arrays are lengthy and difficult to read. Consider splitting them into smaller chunks or using a more readable format.
bytes memory proofData = hex" 0000000000000000000000000000000000000000000000000000000000000020 0000000000000000000000000000000000000000000000000000000000000008 0000000000000000000000000000000000000000000000000000000000000020 0000000000000000000000000000000000000000000000000000000000000060 0000000000000000000000000000000000000000000000000000000000000002 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000004 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 000000000000000000000 </blockquote></details> </blockquote></details> <details> <summary>Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** <details> <summary>Commits</summary> Files that changed from the base of the PR and between 57c8c01b0f23f3c70ebeeb3387457c9c6ff3be0a and b083ebbeb17484bb7dd6f27daffbf610b2ecde35. </details> <details> <summary>Files ignored due to path filters (2)</summary> * `src/lib/tree/test/blob.dat` is excluded by `!**/*.dat` * `src/lib/tree/test/header.dat` is excluded by `!**/*.dat` </details> <details> <summary>Files selected for processing (11)</summary> * .gitmodules (1 hunks) * lib/forge-std (1 hunks) * remappings.txt (1 hunks) * src/lib/tree/Utils.sol (1 hunks) * src/lib/tree/binary/BinaryMerkleMultiproof.sol (1 hunks) * src/lib/tree/binary/BinaryMerkleTree.sol (2 hunks) * src/lib/tree/binary/test/BinaryMerkleTree.t.sol (2 hunks) * src/lib/tree/namespace/NamespaceMerkleMultiproof.sol (1 hunks) * src/lib/tree/namespace/NamespaceMerkleTree.sol (1 hunks) * src/lib/tree/namespace/test/NamespaceMerkleMultiproof.t.sol (1 hunks) * src/lib/tree/test/proofs.json (1 hunks) </details> <details> <summary>Files skipped from review due to trivial changes (4)</summary> * lib/forge-std * remappings.txt * src/lib/tree/namespace/NamespaceMerkleMultiproof.sol * src/lib/tree/test/proofs.json </details> <details> <summary>Additional comments not posted (5)</summary><blockquote> <details> <summary>src/lib/tree/binary/BinaryMerkleMultiproof.sol (1)</summary><blockquote> `1-12`: **LGTM!** The `BinaryMerkleMultiproof` struct is well-defined and straightforward. </blockquote></details> <details> <summary>.gitmodules (1)</summary><blockquote> `10-12`: **LGTM!** The new submodule entry for `forge-std` is correctly formatted and follows the existing pattern. </blockquote></details> <details> <summary>src/lib/tree/Utils.sol (2)</summary><blockquote> `80-91`: **LGTM!** The `nextSubtreeSize` function is well-implemented and the logic for calculating the subtree size is sound. --- `93-106`: **LGTM!** The `bitsTrailingZeroes` function is well-implemented and the logic for counting trailing zero bits is sound. </blockquote></details> <details> <summary>src/lib/tree/binary/BinaryMerkleTree.sol (1)</summary><blockquote> `186-193`: **LGTM!** The function `_popProofIfNonEmpty` is correctly implemented. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Comment on lines
82
to
93
| function verifyMulti(bytes32 root, BinaryMerkleMultiproof memory proof, bytes[] memory data) | ||
| internal | ||
| pure | ||
| returns (bool) | ||
| { | ||
| bytes32[] memory nodes = new bytes32[](data.length); | ||
| for (uint256 i = 0; i < data.length; i++) { | ||
| nodes[i] = leafDigest(data[i]); | ||
| } | ||
|
|
||
| return verifyMultiHashes(root, proof, nodes); | ||
| } |
There was a problem hiding this comment.
Add error handling for empty data arrays.
The function verifyMulti does not handle the case where the data array is empty, which could lead to unexpected behavior.
+ require(data.length > 0, "Data array cannot be empty");Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function verifyMulti(bytes32 root, BinaryMerkleMultiproof memory proof, bytes[] memory data) | |
| internal | |
| pure | |
| returns (bool) | |
| { | |
| bytes32[] memory nodes = new bytes32[](data.length); | |
| for (uint256 i = 0; i < data.length; i++) { | |
| nodes[i] = leafDigest(data[i]); | |
| } | |
| return verifyMultiHashes(root, proof, nodes); | |
| } | |
| function verifyMulti(bytes32 root, BinaryMerkleMultiproof memory proof, bytes[] memory data) | |
| internal | |
| pure | |
| returns (bool) | |
| { | |
| require(data.length > 0, "Data array cannot be empty"); | |
| bytes32[] memory nodes = new bytes32[](data.length); | |
| for (uint256 i = 0; i < data.length; i++) { | |
| nodes[i] = leafDigest(data[i]); | |
| } | |
| return verifyMultiHashes(root, proof, nodes); | |
| } |
Comment on lines
113
to
139
| function verifyMultiHashes(bytes32 root, BinaryMerkleMultiproof memory proof, bytes32[] memory leafNodes) | ||
| internal | ||
| pure | ||
| returns (bool) | ||
| { | ||
| uint256 leafIndex = 0; | ||
| bytes32[] memory leftSubtrees = new bytes32[](proof.sideNodes.length); | ||
|
|
||
| for (uint256 i = 0; leafIndex != proof.beginKey && i < proof.sideNodes.length; ++i) { | ||
| uint256 subtreeSize = nextSubtreeSize(leafIndex, proof.beginKey); | ||
| leftSubtrees[i] = proof.sideNodes[i]; | ||
| leafIndex += subtreeSize; | ||
| } | ||
|
|
||
| uint256 proofRangeSubtreeEstimate = _getSplitPoint(proof.endKey) * 2; | ||
| if (proofRangeSubtreeEstimate < 1) { | ||
| proofRangeSubtreeEstimate = 1; | ||
| } | ||
|
|
||
| (bytes32 rootHash, uint256 proofHead,,) = | ||
| _computeRootMulti(proof, leafNodes, 0, proofRangeSubtreeEstimate, 0, 0); | ||
| for (uint256 i = proofHead; i < proof.sideNodes.length; ++i) { | ||
| rootHash = nodeDigest(rootHash, proof.sideNodes[i]); | ||
| } | ||
|
|
||
| return (rootHash == root); | ||
| } |
There was a problem hiding this comment.
Add error handling for empty leaf nodes.
The function verifyMultiHashes does not handle the case where the leafNodes array is empty, which could lead to unexpected behavior.
+ require(leafNodes.length > 0, "Leaf nodes array cannot be empty");Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function verifyMultiHashes(bytes32 root, BinaryMerkleMultiproof memory proof, bytes32[] memory leafNodes) | |
| internal | |
| pure | |
| returns (bool) | |
| { | |
| uint256 leafIndex = 0; | |
| bytes32[] memory leftSubtrees = new bytes32[](proof.sideNodes.length); | |
| for (uint256 i = 0; leafIndex != proof.beginKey && i < proof.sideNodes.length; ++i) { | |
| uint256 subtreeSize = nextSubtreeSize(leafIndex, proof.beginKey); | |
| leftSubtrees[i] = proof.sideNodes[i]; | |
| leafIndex += subtreeSize; | |
| } | |
| uint256 proofRangeSubtreeEstimate = _getSplitPoint(proof.endKey) * 2; | |
| if (proofRangeSubtreeEstimate < 1) { | |
| proofRangeSubtreeEstimate = 1; | |
| } | |
| (bytes32 rootHash, uint256 proofHead,,) = | |
| _computeRootMulti(proof, leafNodes, 0, proofRangeSubtreeEstimate, 0, 0); | |
| for (uint256 i = proofHead; i < proof.sideNodes.length; ++i) { | |
| rootHash = nodeDigest(rootHash, proof.sideNodes[i]); | |
| } | |
| return (rootHash == root); | |
| } | |
| function verifyMultiHashes(bytes32 root, BinaryMerkleMultiproof memory proof, bytes32[] memory leafNodes) | |
| internal | |
| pure | |
| returns (bool) | |
| { | |
| require(leafNodes.length > 0, "Leaf nodes array cannot be empty"); | |
| uint256 leafIndex = 0; | |
| bytes32[] memory leftSubtrees = new bytes32[](proof.sideNodes.length); | |
| for (uint256 i = 0; leafIndex != proof.beginKey && i < proof.sideNodes.length; ++i) { | |
| uint256 subtreeSize = nextSubtreeSize(leafIndex, proof.beginKey); | |
| leftSubtrees[i] = proof.sideNodes[i]; | |
| leafIndex += subtreeSize; | |
| } | |
| uint256 proofRangeSubtreeEstimate = _getSplitPoint(proof.endKey) * 2; | |
| if (proofRangeSubtreeEstimate < 1) { | |
| proofRangeSubtreeEstimate = 1; | |
| } | |
| (bytes32 rootHash, uint256 proofHead,,) = | |
| _computeRootMulti(proof, leafNodes, 0, proofRangeSubtreeEstimate, 0, 0); | |
| for (uint256 i = proofHead; i < proof.sideNodes.length; ++i) { | |
| rootHash = nodeDigest(rootHash, proof.sideNodes[i]); | |
| } | |
| return (rootHash == root); | |
| } |
Comment on lines
141
to
184
| function _computeRootMulti( | ||
| BinaryMerkleMultiproof memory proof, | ||
| bytes32[] memory leafNodes, | ||
| uint256 begin, | ||
| uint256 end, | ||
| uint256 headProof, | ||
| uint256 headLeaves | ||
| ) private pure returns (bytes32, uint256, uint256, bool) { | ||
| // reached a leaf | ||
| if (end - begin == 1) { | ||
| // if current range overlaps with proof range, pop and return a leaf | ||
| if (proof.beginKey <= begin && begin < proof.endKey) { | ||
| // Note: second return value is guaranteed to be `false` by | ||
| // construction. | ||
| return _popLeavesIfNonEmpty(leafNodes, headLeaves, leafNodes.length, headProof); | ||
| } | ||
|
|
||
| // if current range does not overlap with proof range, | ||
| // pop and return a proof node (leaf) if present, | ||
| // else return nil because leaf doesn't exist | ||
| return _popProofIfNonEmpty(proof.sideNodes, headProof, end, headLeaves); | ||
| } | ||
|
|
||
| // if current range does not overlap with proof range, | ||
| // pop and return a proof node if present, | ||
| // else return nil because subtree doesn't exist | ||
| if (end <= proof.beginKey || begin >= proof.endKey) { | ||
| return _popProofIfNonEmpty(proof.sideNodes, headProof, end, headLeaves); | ||
| } | ||
|
|
||
| // Recursively get left and right subtree | ||
| uint256 k = _getSplitPoint(end - begin); | ||
| (bytes32 left, uint256 newHeadProofLeft, uint256 newHeadLeavesLeft,) = | ||
| _computeRootMulti(proof, leafNodes, begin, begin + k, headProof, headLeaves); | ||
| (bytes32 right, uint256 newHeadProof, uint256 newHeadLeaves, bool rightIsNil) = | ||
| _computeRootMulti(proof, leafNodes, begin + k, end, newHeadProofLeft, newHeadLeavesLeft); | ||
|
|
||
| // only right leaf/subtree can be non-existent | ||
| if (rightIsNil == true) { | ||
| return (left, newHeadProof, newHeadLeaves, false); | ||
| } | ||
| bytes32 hash = nodeDigest(left, right); | ||
| return (hash, newHeadProof, newHeadLeaves, false); | ||
| } |
There was a problem hiding this comment.
Add error handling for empty proof or leaf nodes.
The function _computeRootMulti does not handle the case where the proof or leafNodes arrays are empty, which could lead to unexpected behavior.
+ require(proof.sideNodes.length > 0, "Proof side nodes array cannot be empty");
+ require(leafNodes.length > 0, "Leaf nodes array cannot be empty");Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function _computeRootMulti( | |
| BinaryMerkleMultiproof memory proof, | |
| bytes32[] memory leafNodes, | |
| uint256 begin, | |
| uint256 end, | |
| uint256 headProof, | |
| uint256 headLeaves | |
| ) private pure returns (bytes32, uint256, uint256, bool) { | |
| // reached a leaf | |
| if (end - begin == 1) { | |
| // if current range overlaps with proof range, pop and return a leaf | |
| if (proof.beginKey <= begin && begin < proof.endKey) { | |
| // Note: second return value is guaranteed to be `false` by | |
| // construction. | |
| return _popLeavesIfNonEmpty(leafNodes, headLeaves, leafNodes.length, headProof); | |
| } | |
| // if current range does not overlap with proof range, | |
| // pop and return a proof node (leaf) if present, | |
| // else return nil because leaf doesn't exist | |
| return _popProofIfNonEmpty(proof.sideNodes, headProof, end, headLeaves); | |
| } | |
| // if current range does not overlap with proof range, | |
| // pop and return a proof node if present, | |
| // else return nil because subtree doesn't exist | |
| if (end <= proof.beginKey || begin >= proof.endKey) { | |
| return _popProofIfNonEmpty(proof.sideNodes, headProof, end, headLeaves); | |
| } | |
| // Recursively get left and right subtree | |
| uint256 k = _getSplitPoint(end - begin); | |
| (bytes32 left, uint256 newHeadProofLeft, uint256 newHeadLeavesLeft,) = | |
| _computeRootMulti(proof, leafNodes, begin, begin + k, headProof, headLeaves); | |
| (bytes32 right, uint256 newHeadProof, uint256 newHeadLeaves, bool rightIsNil) = | |
| _computeRootMulti(proof, leafNodes, begin + k, end, newHeadProofLeft, newHeadLeavesLeft); | |
| // only right leaf/subtree can be non-existent | |
| if (rightIsNil == true) { | |
| return (left, newHeadProof, newHeadLeaves, false); | |
| } | |
| bytes32 hash = nodeDigest(left, right); | |
| return (hash, newHeadProof, newHeadLeaves, false); | |
| } | |
| function _computeRootMulti( | |
| BinaryMerkleMultiproof memory proof, | |
| bytes32[] memory leafNodes, | |
| uint256 begin, | |
| uint256 end, | |
| uint256 headProof, | |
| uint256 headLeaves | |
| ) private pure returns (bytes32, uint256, uint256, bool) { | |
| require(proof.sideNodes.length > 0, "Proof side nodes array cannot be empty"); | |
| require(leafNodes.length > 0, "Leaf nodes array cannot be empty"); | |
| // reached a leaf | |
| if (end - begin == 1) { | |
| // if current range overlaps with proof range, pop and return a leaf | |
| if (proof.beginKey <= begin && begin < proof.endKey) { | |
| // Note: second return value is guaranteed to be `false` by | |
| // construction. | |
| return _popLeavesIfNonEmpty(leafNodes, headLeaves, leafNodes.length, headProof); | |
| } | |
| // if current range does not overlap with proof range, | |
| // pop and return a proof node (leaf) if present, | |
| // else return nil because leaf doesn't exist | |
| return _popProofIfNonEmpty(proof.sideNodes, headProof, end, headLeaves); | |
| } | |
| // if current range does not overlap with proof range, | |
| // pop and return a proof node if present, | |
| // else return nil because subtree doesn't exist | |
| if (end <= proof.beginKey || begin >= proof.endKey) { | |
| return _popProofIfNonEmpty(proof.sideNodes, headProof, end, headLeaves); | |
| } | |
| // Recursively get left and right subtree | |
| uint256 k = _getSplitPoint(end - begin); | |
| (bytes32 left, uint256 newHeadProofLeft, uint256 newHeadLeavesLeft,) = | |
| _computeRootMulti(proof, leafNodes, begin, begin + k, headProof, headLeaves); | |
| (bytes32 right, uint256 newHeadProof, uint256 newHeadLeaves, bool rightIsNil) = | |
| _computeRootMulti(proof, leafNodes, begin + k, end, newHeadProofLeft, newHeadLeavesLeft); | |
| // only right leaf/subtree can be non-existent | |
| if (rightIsNil == true) { | |
| return (left, newHeadProof, newHeadLeaves, false); | |
| } | |
| bytes32 hash = nodeDigest(left, right); | |
| return (hash, newHeadProof, newHeadLeaves, false); | |
| } |
Comment on lines
341
to
350
| function testMultiproof() public { | ||
| bytes memory proofData = | ||
| hex"00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000090000000000000000000000000000000000000000000000000000000000000006ce29bcde696f84e35c5626904542a549b080e92603243b34794242473940706917519bf954f5b30495af5c8cdb9983e6319104badc1ea811ed2c421018a3ad7821ea268d3540deab8f9b2024464618610c9a7083620badcf505bda647cc8e9f82bfc87d990d8344f6efd44fcb09b46b87f9a92230d41329452efee8656c6760a9ad9f3a95af971e89e2a80b255bb56d5aae15de69803b52aa5079b33374b16e16178fc62a2f2ce6bf21909c0a0edea9525486e0ece65bff23499342cca38dd62"; | ||
| BinaryMerkleMultiproof memory multiproof = abi.decode(proofData, (BinaryMerkleMultiproof)); | ||
| bytes32 dataroot = hex"ef8920d86519bd5f8ce3c802b84fc9b9512483e4d4a5c9608b44af4d6639f7d1"; | ||
| bytes memory leafData = | ||
| hex"00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000009000000000000000000000000000000000000000000000000000000000000012000000000000000000000000000000000000000000000000000000000000001a0000000000000000000000000000000000000000000000000000000000000022000000000000000000000000000000000000000000000000000000000000002a0000000000000000000000000000000000000000000000000000000000000032000000000000000000000000000000000000000000000000000000000000003a0000000000000000000000000000000000000000000000000000000000000042000000000000000000000000000000000000000000000000000000000000004a00000000000000000000000000000000000000000000000000000000000000520000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000102030405746e218305fe3dbbef65feceed939fe8dd93c88b06c95473fbe344fb864060f3000000000000000000000000000000000000000000000000000000000000000000000000005a0000000000000000000000000000000000000000000000000102030405000000000000000000000000000000000000000000000000010203040555cd7fb524ae792c9d4bc8946d07209728c533a3e14d4e7c0c95c0b150d0c284000000000000000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000001020304050000000000000000000000000000000000000000000000000102030405505c1e7c897461a152e152f1ff3ecc358fefdf1f69448ab1165b6ca76836933b000000000000000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000001020304050000000000000000000000000000000000000000000000000102030405100a0548893d8eab0322f34f45ac84785cdf50dfab5102a12d958e6031bacebe000000000000000000000000000000000000000000000000000000000000000000000000005a0000000000000000000000000000000000000000000000000102030405000000000000000000000000000000000000000000000000010203040566e5eb1da67430f204a3c5615591f71316695c7ec1f1f713cde7e936d4a43ec1000000000000000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000001020304050000000000000000000000000000000000000000000000000102030405d2a5de6299e28c2fec359a2718599f5ac22c2948a71d26a438295e531b6f4cb5000000000000000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000001020304050000000000000000000000000000000000000000000000000102030405688c5238e50c0a8a556bfabff31bef1fa9cdd812c9fd4dcee5c2a0836f687fbf000000000000000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000001020304050000000000000000000000000000000000000000000000000102030405b55a5b1efc2a22cdbfa21d050bd67147ff2b936c68354eb1a83bcdf14eb57e38000000000000000000000000000000000000000000000000000000000000000000000000005a000000000000000000000000000000000000000000000000010203040500000000000000000000000000000000000000000067480c4a88c4d129947e11c33fa811daa791771e591dd933498d1212d46b8cde9c34c28831b0b532000000000000"; | ||
| bytes[] memory leaves = abi.decode(leafData, (bytes[])); | ||
| assertTrue(BinaryMerkleTree.verifyMulti(dataroot, multiproof, leaves)); | ||
| } |
There was a problem hiding this comment.
Add validation for decoded data.
The function testMultiproof does not validate the decoded multiproof and leaves data, which could lead to unexpected behavior.
+ require(multiproof.sideNodes.length > 0, "Multiproof side nodes array cannot be empty");
+ require(leaves.length > 0, "Leaves array cannot be empty");Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function testMultiproof() public { | |
| bytes memory proofData = | |
| hex"00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000090000000000000000000000000000000000000000000000000000000000000006ce29bcde696f84e35c5626904542a549b080e92603243b34794242473940706917519bf954f5b30495af5c8cdb9983e6319104badc1ea811ed2c421018a3ad7821ea268d3540deab8f9b2024464618610c9a7083620badcf505bda647cc8e9f82bfc87d990d8344f6efd44fcb09b46b87f9a92230d41329452efee8656c6760a9ad9f3a95af971e89e2a80b255bb56d5aae15de69803b52aa5079b33374b16e16178fc62a2f2ce6bf21909c0a0edea9525486e0ece65bff23499342cca38dd62"; | |
| BinaryMerkleMultiproof memory multiproof = abi.decode(proofData, (BinaryMerkleMultiproof)); | |
| bytes32 dataroot = hex"ef8920d86519bd5f8ce3c802b84fc9b9512483e4d4a5c9608b44af4d6639f7d1"; | |
| bytes memory leafData = | |
| hex"00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000009000000000000000000000000000000000000000000000000000000000000012000000000000000000000000000000000000000000000000000000000000001a0000000000000000000000000000000000000000000000000000000000000022000000000000000000000000000000000000000000000000000000000000002a0000000000000000000000000000000000000000000000000000000000000032000000000000000000000000000000000000000000000000000000000000003a0000000000000000000000000000000000000000000000000000000000000042000000000000000000000000000000000000000000000000000000000000004a00000000000000000000000000000000000000000000000000000000000000520000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000102030405746e218305fe3dbbef65feceed939fe8dd93c88b06c95473fbe344fb864060f3000000000000000000000000000000000000000000000000000000000000000000000000005a0000000000000000000000000000000000000000000000000102030405000000000000000000000000000000000000000000000000010203040555cd7fb524ae792c9d4bc8946d07209728c533a3e14d4e7c0c95c0b150d0c284000000000000000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000001020304050000000000000000000000000000000000000000000000000102030405505c1e7c897461a152e152f1ff3ecc358fefdf1f69448ab1165b6ca76836933b000000000000000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000001020304050000000000000000000000000000000000000000000000000102030405100a0548893d8eab0322f34f45ac84785cdf50dfab5102a12d958e6031bacebe000000000000000000000000000000000000000000000000000000000000000000000000005a0000000000000000000000000000000000000000000000000102030405000000000000000000000000000000000000000000000000010203040566e5eb1da67430f204a3c5615591f71316695c7ec1f1f713cde7e936d4a43ec1000000000000000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000001020304050000000000000000000000000000000000000000000000000102030405d2a5de6299e28c2fec359a2718599f5ac22c2948a71d26a438295e531b6f4cb5000000000000000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000001020304050000000000000000000000000000000000000000000000000102030405688c5238e50c0a8a556bfabff31bef1fa9cdd812c9fd4dcee5c2a0836f687fbf000000000000000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000001020304050000000000000000000000000000000000000000000000000102030405b55a5b1efc2a22cdbfa21d050bd67147ff2b936c68354eb1a83bcdf14eb57e38000000000000000000000000000000000000000000000000000000000000000000000000005a000000000000000000000000000000000000000000000000010203040500000000000000000000000000000000000000000067480c4a88c4d129947e11c33fa811daa791771e591dd933498d1212d46b8cde9c34c28831b0b532000000000000"; | |
| bytes[] memory leaves = abi.decode(leafData, (bytes[])); | |
| assertTrue(BinaryMerkleTree.verifyMulti(dataroot, multiproof, leaves)); | |
| } | |
| function testMultiproof() public { | |
| bytes memory proofData = | |
| hex"00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000090000000000000000000000000000000000000000000000000000000000000006ce29bcde696f84e35c5626904542a549b080e92603243b34794242473940706917519bf954f5b30495af5c8cdb9983e6319104badc1ea811ed2c421018a3ad7821ea268d3540deab8f9b2024464618610c9a7083620badcf505bda647cc8e9f82bfc87d990d8344f6efd44fcb09b46b87f9a92230d41329452efee8656c6760a9ad9f3a95af971e89e2a80b255bb56d5aae15de69803b52aa5079b33374b16e16178fc62a2f2ce6bf21909c0a0edea9525486e0ece65bff23499342cca38dd62"; | |
| BinaryMerkleMultiproof memory multiproof = abi.decode(proofData, (BinaryMerkleMultiproof)); | |
| bytes32 dataroot = hex"ef8920d86519bd5f8ce3c802b84fc9b9512483e4d4a5c9608b44af4d6639f7d1"; | |
| bytes memory leafData = | |
| hex"00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000009000000000000000000000000000000000000000000000000000000000000012000000000000000000000000000000000000000000000000000000000000001a0000000000000000000000000000000000000000000000000000000000000022000000000000000000000000000000000000000000000000000000000000002a0000000000000000000000000000000000000000000000000000000000000032000000000000000000000000000000000000000000000000000000000000003a0000000000000000000000000000000000000000000000000000000000000042000000000000000000000000000000000000000000000000000000000000004a00000000000000000000000000000000000000000000000000000000000000520000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000102030405746e218305fe3dbbef65feceed939fe8dd93c88b06c95473fbe344fb864060f3000000000000000000000000000000000000000000000000000000000000000000000000005a0000000000000000000000000000000000000000000000000102030405000000000000000000000000000000000000000000000000010203040555cd7fb524ae792c9d4bc8946d07209728c533a3e14d4e7c0c95c0b150d0c284000000000000000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000001020304050000000000000000000000000000000000000000000000000102030405505c1e7c897461a152e152f1ff3ecc358fefdf1f69448ab1165b6ca76836933b000000000000000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000001020304050000000000000000000000000000000000000000000000000102030405100a0548893d8eab0322f34f45ac84785cdf50dfab5102a12d958e6031bacebe000000000000000000000000000000000000000000000000000000000000000000000000005a0000000000000000000000000000000000000000000000000102030405000000000000000000000000000000000000000000000000010203040566e5eb1da67430f204a3c5615591f71316695c7ec1f1f713cde7e936d4a43ec1000000000000000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000001020304050000000000000000000000000000000000000000000000000102030405d2a5de6299e28c2fec359a2718599f5ac22c2948a71d26a438295e531b6f4cb5000000000000000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000001020304050000000000000000000000000000000000000000000000000102030405688c5238e50c0a8a556bfabff31bef1fa9cdd812c9fd4dcee5c2a0836f687fbf000000000000000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000001020304050000000000000000000000000000000000000000000000000102030405b55a5b1efc2a22cdbfa21d050bd67147ff2b936c68354eb1a83bcdf14eb57e38000000000000000000000000000000000000000000000000000000000000000000000000005a000000000000000000000000000000000000000000000000010203040500000000000000000000000000000000000000000067480c4a88c4d129947e11c33fa811daa791771e591dd933498d1212d46b8cde9c34c28831b0b532000000000000"; | |
| BinaryMerkleMultiproof memory multiproof = abi.decode(proofData, (BinaryMerkleMultiproof)); | |
| require(multiproof.sideNodes.length > 0, "Multiproof side nodes array cannot be empty"); | |
| bytes32 dataroot = hex"ef8920d86519bd5f8ce3c802b84fc9b9512483e4d4a5c9608b44af4d6639f7d1"; | |
| bytes memory leafData = | |
| hex"00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000009000000000000000000000000000000000000000000000000000000000000012000000000000000000000000000000000000000000000000000000000000001a0000000000000000000000000000000000000000000000000000000000000022000000000000000000000000000000000000000000000000000000000000002a0000000000000000000000000000000000000000000000000000000000000032000000000000000000000000000000000000000000000000000000000000003a0000000000000000000000000000000000000000000000000000000000000042000000000000000000000000000000000000000000000000000000000000004a00000000000000000000000000000000000000000000000000000000000000520000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000102030405746e218305fe3dbbef65feceed939fe8dd93c88b06c95473fbe344fb864060f3000000000000000000000000000000000000000000000000000000000000000000000000005a0000000000000000000000000000000000000000000000000102030405000000000000000000000000000000000000000000000000010203040555cd7fb524ae792c9d4bc8946d07209728c533a3e14d4e7c0c95c0b150d0c284000000000000000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000001020304050000000000000000000000000000000000000000000000000102030405505c1e7c897461a152e152f1ff3ecc358fefdf1f69448ab1165b6ca76836933b000000000000000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000001020304050000000000000000000000000000000000000000000000000102030405100a0548893d8eab0322f34f45ac84785cdf50dfab5102a12d958e6031bacebe000000000000000000000000000000000000000000000000000000000000000000000000005a0000000000000000000000000000000000000000000000000102030405000000000000000000000000000000000000000000000000010203040566e5eb1da67430f204a3c5615591f71316695c7ec1f1f713cde7e936d4a43ec1000000000000000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000001020304050000000000000000000000000000000000000000000000000102030405d2a5de6299e28c2fec359a2718599f5ac22c2948a71d26a438295e531b6f4cb5000000000000000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000001020304050000000000000000000000000000000000000000000000000102030405688c5238e50c0a8a556bfabff31bef1fa9cdd812c9fd4dcee5c2a0836f687fbf000000000000000000000000000000000000000000000000000000000000000000000000005a00000000000000000000000000000000000000000000000001020304050000000000000000000000000000000000000000000000000102030405b55a5b1efc2a22cdbfa21d050bd67147ff2b936c68354eb1a83bcdf14eb57e38000000000000000000000000000000000000000000000000000000000000000000000000005a000000000000000000000000000000000000000000000000010203040500000000000000000000000000000000000000000067480c4a88c4d129947e11c33fa811daa791771e591dd933498d1212d46b8cde9c34c28831b0b532000000000000"; | |
| bytes[] memory leaves = abi.decode(leafData, (bytes[])); | |
| require(leaves.length > 0, "Leaves array cannot be empty"); | |
| assertTrue(BinaryMerkleTree.verifyMulti(dataroot, multiproof, leaves)); | |
| } |
rach-id
reviewed
Jul 27, 2024
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
src/lib/tree/binary/BinaryMerkleTree.sol (1)
95-98: Remove commented-out code.The commented-out code is unnecessary and should be removed to improve code readability.
- /* - This helps with gas efficiency so we can avoid unnecessary copying and looping through the NamespaceNodes - when we go to verify the row inclusion multiproof - */
Comment on lines
+99
to
+111
| function verifyMultiNamespaced(bytes32 root, BinaryMerkleMultiproof memory proof, NamespaceNode[] memory data) | ||
| internal | ||
| pure | ||
| returns (bool) | ||
| { | ||
| bytes32[] memory nodes = new bytes32[](data.length); | ||
| for (uint256 i = 0; i < data.length; i++) { | ||
| // the bytes.concat(...) converts a bytes32 into a bytes | ||
| nodes[i] = leafDigest(bytes.concat(data[i].digest)); | ||
| } | ||
|
|
||
| return verifyMultiHashes(root, proof, nodes); | ||
| } |
There was a problem hiding this comment.
Add error handling for empty data arrays.
The function verifyMultiNamespaced does not handle the case where the data array is empty, which could lead to unexpected behavior.
+ require(data.length > 0, "Data array cannot be empty");Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function verifyMultiNamespaced(bytes32 root, BinaryMerkleMultiproof memory proof, NamespaceNode[] memory data) | |
| internal | |
| pure | |
| returns (bool) | |
| { | |
| bytes32[] memory nodes = new bytes32[](data.length); | |
| for (uint256 i = 0; i < data.length; i++) { | |
| // the bytes.concat(...) converts a bytes32 into a bytes | |
| nodes[i] = leafDigest(bytes.concat(data[i].digest)); | |
| } | |
| return verifyMultiHashes(root, proof, nodes); | |
| } | |
| function verifyMultiNamespaced(bytes32 root, BinaryMerkleMultiproof memory proof, NamespaceNode[] memory data) | |
| internal | |
| pure | |
| returns (bool) | |
| { | |
| require(data.length > 0, "Data array cannot be empty"); | |
| bytes32[] memory nodes = new bytes32[](data.length); | |
| for (uint256 i = 0; i < data.length; i++) { | |
| // the bytes.concat(...) converts a bytes32 into a bytes | |
| nodes[i] = leafDigest(bytes.concat(data[i].digest)); | |
| } | |
| return verifyMultiHashes(root, proof, nodes); | |
| } |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New to Solidity so idk if there's a better way, but I wrote this to save us from needing to loop through the
NamespaceNodesjust to copy theirdigestfield into another array.Summary by CodeRabbit
New Features
Utilslibrary, including functions for subtree size calculations and bit manipulation.BinaryMerkleMultiproofstructure for improving Merkle Tree proof verification.BinaryMerkleTreelibrary with multiple functions for verifying multi-proofs, including the newverifyMultiNamespacedfunction.NamespaceMerkleTree.Bug Fixes
NamespaceMerkleMultiproofstructure.Chores
proofs.jsonfile for managing proof-related data in testing.