Skip to content

Commit 89a8343

Browse files
committed
refactor: review changes
1 parent c5bacca commit 89a8343

File tree

2 files changed

+28
-14
lines changed

2 files changed

+28
-14
lines changed

src/contracts/libraries/Merkle.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ library Merkle {
5353
bytes32 leaf,
5454
uint256 index
5555
) internal pure returns (bytes32) {
56-
require(proof.length != 0 && proof.length % 32 == 0, InvalidProofLength());
56+
require(proof.length % 32 == 0, InvalidProofLength());
5757
bytes32 computedHash = leaf;
5858
for (uint256 i = 32; i <= proof.length; i += 32) {
5959
if (index % 2 == 0) {

src/test/unit/libraries/Merkle.t.sol

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,23 @@ import "src/contracts/libraries/Merkle.sol";
66
import "src/test/utils/Murky.sol";
77

88
abstract contract MerkleBaseTest is Test, MurkyBase {
9+
uint internal constant MIN_LEAVES = 2; // Minimum number of leaves.
10+
uint internal constant MAX_LEAVES = 65; // Maximum number of leaves.
11+
912
bytes32[] leaves; // The contents of the merkle tree (unsorted).
1013
bytes32 root; // The root of the merkle tree.
1114
bytes[] proofs; // The proofs for each leaf in the tree.
1215

13-
function setUp() public {
14-
leaves = _genLeaves(vm.randomBool() ? 9 : 10);
16+
/// @dev Takes in `seed` which should be provided as a fuzz input.
17+
/// Ensures that RNG is deterministic, and tests are easily reproducible.
18+
modifier rng(uint seed) {
19+
_rng(seed);
20+
_;
21+
}
22+
23+
function _rng(uint seed) internal {
24+
vm.setSeed(seed);
25+
leaves = _genLeaves(vm.randomBool() ? MIN_LEAVES : MAX_LEAVES);
1526
proofs = _genProofs(leaves);
1627
root = _genRoot(leaves);
1728
}
@@ -21,24 +32,25 @@ abstract contract MerkleBaseTest is Test, MurkyBase {
2132
/// -----------------------------------------------------------------------
2233

2334
/// @notice Verifies that (Murky's) proofs are compatible with our implementation.
24-
function testFuzz_verifyInclusion_ValidProof(uint) public {
35+
function testFuzz_verifyInclusion_ValidProof(uint seed) public rng(seed) {
2536
_checkAllProofs(true);
2637
}
2738

2839
/// @notice Verifies that an empty proof(s) is invalid.
29-
function testFuzz_verifyInclusion_EmptyProofs(uint) public {
40+
function testFuzz_verifyInclusion_EmptyProofs(uint seed) public rng(seed) {
41+
if (!usingSha()) vm.skip(true); // TODO: Breaking change, add in future.
3042
proofs = new bytes[](proofs.length);
3143
_checkAllProofs(false);
3244
}
3345

3446
/// @notice Verifies that using wrong root fails verification
35-
function testFuzz_verifyInclusion_WrongRoot(uint) public {
47+
function testFuzz_verifyInclusion_WrongRoot(uint seed) public rng(seed) {
3648
root = bytes32(vm.randomUint());
3749
_checkAllProofs(false);
3850
}
3951

4052
/// @notice Verifies valid proofs cannot be used to prove invalid leaves.
41-
function testFuzz_verifyInclusion_WrongProofs(uint) public {
53+
function testFuzz_verifyInclusion_WrongProofs(uint seed) public rng(seed) {
4254
bytes memory proof0 = proofs[0];
4355
bytes memory proof1 = proofs[1];
4456
(proofs[0], proofs[1]) = (proof1, proof0);
@@ -47,26 +59,28 @@ abstract contract MerkleBaseTest is Test, MurkyBase {
4759
}
4860

4961
/// @notice Verifies that a valid proof with excess data appended is invalid.
50-
function testFuzz_verifyInclusion_ExcessProofLength(uint) public {
62+
function testFuzz_verifyInclusion_ExcessProofLength(uint seed) public rng(seed) {
5163
unchecked {
5264
proofs[0] = abi.encodePacked(proofs[0], vm.randomBytes(vm.randomUint(1, 10) * vm.randomUint(31, 32)));
5365
}
5466
_checkSingleProof(false, 0);
5567
}
5668

5769
/// @notice Verifies that a valid proof with a truncated length is invalid.
58-
function testFuzz_verifyInclusion_TruncatedProofLength(uint) public {
70+
function testFuzz_verifyInclusion_TruncatedProofLength(uint seed) public rng(seed) {
5971
bytes memory proof = proofs[0];
72+
console.log("proof length %d", proof.length); // 32
6073
/// @solidity memory-safe-assembly
6174
assembly {
6275
mstore(proof, sub(mload(proof), 32))
6376
}
77+
console.log("proof length %d", proof.length); // 0
6478
proofs[0] = proof;
65-
_checkSingleProof(false, 0);
79+
_checkSingleProof(false, 0); // Should revert, but doesn't...
6680
}
6781

6882
/// @notice Verifies that a valid proof with a manipulated word is invalid.
69-
function testFuzz_verifyInclusion_ManipulatedProof(uint) public {
83+
function testFuzz_verifyInclusion_ManipulatedProof(uint seed) public rng(seed) {
7084
bytes memory proof = proofs[0];
7185
/// @solidity memory-safe-assembly
7286
assembly {
@@ -79,14 +93,14 @@ abstract contract MerkleBaseTest is Test, MurkyBase {
7993
}
8094

8195
/// @notice Verifies that an out-of-bounds index reverts.
82-
function testFuzz_verifyInclusion_IndexOutOfBounds(uint) public {
96+
function testFuzz_verifyInclusion_IndexOutOfBounds(uint seed) public rng(seed) {
8397
uint index = vm.randomUint(leaves.length, type(uint).max);
8498
vm.expectRevert(stdError.indexOOBError);
8599
_checkSingleProof(false, index);
86100
}
87101

88102
/// @notice Verifies that an internal node cannot be used as a proof.
89-
function testFuzz_verifyInclusion_InternalNodeAsProof(uint) public {
103+
function testFuzz_verifyInclusion_InternalNodeAsProof(uint seed) public rng(seed) {
90104
// Generate a tree with at least 4 leaves to ensure internal nodes exist
91105
leaves = _genLeaves(vm.randomUint(4, 8));
92106
proofs = _genProofs(leaves);
@@ -97,7 +111,7 @@ abstract contract MerkleBaseTest is Test, MurkyBase {
97111
}
98112

99113
/// @notice Verifies behavior with duplicate leaves in the tree
100-
function testFuzz_verifyInclusion_DuplicateLeaves(uint) public {
114+
function testFuzz_verifyInclusion_DuplicateLeaves(uint seed) public rng(seed) {
101115
leaves = _genLeaves(vm.randomUint(4, 8));
102116
leaves[0] = leaves[vm.randomUint(1, leaves.length - 1)];
103117
proofs = _genProofs(leaves);

0 commit comments

Comments
 (0)