Skip to content

Commit

Permalink
fix: handle the precondition in path length from key when the number …
Browse files Browse the repository at this point in the history
…of leaves is <= 1 (#274)

<!--
Please read and fill out this form before submitting your PR.

Please make sure you have reviewed our contributors guide before
submitting your
first PR.
-->

## Overview

Fix the issue with path length from key when the number of leaves is <=
1. This happens when the key and the number of leaves are consecutive
numbers.

## Checklist

<!-- 
Please complete the checklist to ensure that the PR is ready to be
reviewed.

IMPORTANT:
PRs should be left in Draft until the below checklist is completed.
-->

- [ ] New and updated code has appropriate documentation
- [ ] New and updated code has new and/or updated testing
- [ ] Required CI checks are passing
- [ ] Visual proof for any user facing features like CLI or
documentation updates
- [ ] Linked issues closed with keywords


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Bug Fixes**
- Improved the `pathLengthFromKey` function to handle cases with minimal
leaves efficiently.

- **Tests**
- Added new test cases to ensure the reliability of binary Merkle proofs
with various key and leaves scenarios.
- Enhanced existing tests with additional assertions for better
coverage.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
rach-id authored Jan 5, 2024
1 parent 3e950b9 commit fff73c2
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 2 deletions.
5 changes: 4 additions & 1 deletion src/lib/tree/Utils.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,12 @@ function getStartingBit(uint256 numLeaves) pure returns (uint256 startingBit) {
/// @param key: The key of the leaf
/// @param numLeaves: The total number of leaves in the tree
/// @return pathLength : The length of the path to the leaf
/// @dev A precondition to this function is that `numLeaves > 1`, so that `(pathLength - 1)` does not cause an underflow when pathLength = 0.
// solhint-disable-next-line func-visibility
function pathLengthFromKey(uint256 key, uint256 numLeaves) pure returns (uint256 pathLength) {
if (numLeaves <= 1) {
// if the number of leaves of the tree is 1 or 0, the path always is 0.
return 0;
}
// Get the height of the left subtree. This is equal to the offset of the starting bit of the path
pathLength = Constants.MAX_HEIGHT - getStartingBit(numLeaves);

Expand Down
22 changes: 22 additions & 0 deletions src/lib/tree/binary/test/BinaryMerkleTree.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,28 @@ contract BinaryMerkleProofTest is DSTest {
assertEq(result[1], data[2]);
}

function testSameKeyAndLeavesNumber() external {
bytes32 root = 0xb855b42d6c30f5b087e05266783fbd6e394f7b926013ccaa67700a8b0c5a596f;
bytes32[] memory sideNodes = new bytes32[](0);
uint256 key = 3;
uint256 numLeaves = 3;
BinaryMerkleProof memory proof = BinaryMerkleProof(sideNodes, key, numLeaves);
bytes memory data = bytes(hex"01");
bool isValid = BinaryMerkleTree.verify(root, proof, data);
assert(!isValid);
}

function testConsecutiveKeyAndNumberOfLeaves() external {
bytes32 root = 0xb855b42d6c30f5b087e05266783fbd6e394f7b926013ccaa67700a8b0c5a596f;
bytes32[] memory sideNodes = new bytes32[](0);
uint256 key = 6;
uint256 numLeaves = 7;
BinaryMerkleProof memory proof = BinaryMerkleProof(sideNodes, key, numLeaves);
bytes memory data = bytes(hex"01");
bool isValid = BinaryMerkleTree.verify(root, proof, data);
assert(!isValid);
}

function testInvalidSliceBeginEnd() public {
bytes32[] memory data = new bytes32[](4);
data[0] = "a";
Expand Down
Loading

0 comments on commit fff73c2

Please sign in to comment.