From 113443470cfa3e7480d789e8897f2fcceb637450 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Fri, 3 Jun 2022 04:36:14 -0300 Subject: [PATCH] Reorder arguments for multiProof functions (#3447) --- contracts/mocks/MerkleProofWrapper.sol | 14 ++++---- contracts/utils/cryptography/MerkleProof.sol | 38 ++++++++++---------- test/utils/cryptography/MerkleProof.test.js | 16 ++++----- 3 files changed, 33 insertions(+), 35 deletions(-) diff --git a/contracts/mocks/MerkleProofWrapper.sol b/contracts/mocks/MerkleProofWrapper.sol index 519222613..589db33a6 100644 --- a/contracts/mocks/MerkleProofWrapper.sol +++ b/contracts/mocks/MerkleProofWrapper.sol @@ -30,19 +30,19 @@ contract MerkleProofWrapper { } function multiProofVerify( - bytes32 root, - bytes32[] calldata leafs, bytes32[] calldata proofs, - bool[] calldata proofFlag + bool[] calldata proofFlag, + bytes32 root, + bytes32[] calldata leaves ) public pure returns (bool) { - return MerkleProof.multiProofVerify(root, leafs, proofs, proofFlag); + return MerkleProof.multiProofVerify(proofs, proofFlag, root, leaves); } function processMultiProof( - bytes32[] calldata leafs, bytes32[] calldata proofs, - bool[] calldata proofFlag + bool[] calldata proofFlag, + bytes32[] calldata leaves ) public pure returns (bytes32) { - return MerkleProof.processMultiProof(leafs, proofs, proofFlag); + return MerkleProof.processMultiProof(proofs, proofFlag, leaves); } } diff --git a/contracts/utils/cryptography/MerkleProof.sol b/contracts/utils/cryptography/MerkleProof.sol index 5b225757d..4c35d46aa 100644 --- a/contracts/utils/cryptography/MerkleProof.sol +++ b/contracts/utils/cryptography/MerkleProof.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.0; /** - * @dev These functions deal with verification of Merkle Trees proofs. + * @dev These functions deal with verification of Merkle Tree proofs. * * The proofs can be generated using the JavaScript library * https://github.com/miguelmota/merkletreejs[merkletreejs]. @@ -75,43 +75,41 @@ library MerkleProof { } /** - * @dev Returns true if a `leafs` can be proved to be a part of a Merkle tree - * defined by `root`. For this, `proofs` for each leaf must be provided, containing - * sibling hashes on the branch from the leaf to the root of the tree. Then - * 'proofFlag' designates the nodes needed for the multi proof. + * @dev Returns true if the `leaves` can be proved to be a part of a Merkle tree defined by + * `root`, according to `proof` and `proofFlags` as described in {processMultiProof}. * * _Available since v4.7._ */ function multiProofVerify( + bytes32[] calldata proof, + bool[] calldata proofFlags, bytes32 root, - bytes32[] calldata leaves, - bytes32[] calldata proofs, - bool[] calldata proofFlag + bytes32[] calldata leaves ) internal pure returns (bool) { - return processMultiProof(leaves, proofs, proofFlag) == root; + return processMultiProof(proof, proofFlags, leaves) == root; } /** - * @dev Returns the rebuilt hash obtained by traversing a Merkle tree up - * from `leaf` using the multi proof as `proofFlag`. A multi proof is - * valid if the final hash matches the root of the tree. + * @dev Returns the root of a tree reconstructed from `leaves` and the sibling nodes in `proof`, + * consuming from one or the other at each step according to the instructions given by + * `proofFlags`. * * _Available since v4.7._ */ function processMultiProof( - bytes32[] calldata leaves, - bytes32[] calldata proofs, - bool[] calldata proofFlag + bytes32[] calldata proof, + bool[] calldata proofFlags, + bytes32[] calldata leaves ) internal pure returns (bytes32 merkleRoot) { // This function rebuild the root hash by traversing the tree up from the leaves. The root is rebuilt by // consuming and producing values on a queue. The queue starts with the `leaves` array, then goes onto the // `hashes` array. At the end of the process, the last hash in the `hashes` array should contain the root of // the merkle tree. uint256 leavesLen = leaves.length; - uint256 totalHashes = proofFlag.length; + uint256 totalHashes = proofFlags.length; // Check proof validity. - require(leavesLen + proofs.length - 1 == totalHashes, "MerkleProof: invalid multiproof"); + require(leavesLen + proof.length - 1 == totalHashes, "MerkleProof: invalid multiproof"); // The xxxPos values are "pointers" to the next value to consume in each array. All accesses are done using // `xxx[xxxPos++]`, which return the current value and increment the pointer, thus mimicking a queue's "pop". @@ -123,10 +121,10 @@ library MerkleProof { // - a value from the "main queue". If not all leaves have been consumed, we get the next leaf, otherwise we // get the next hash. // - depending on the flag, either another value for the "main queue" (merging branches) or an element from the - // `proofs` array. + // `proof` array. for (uint256 i = 0; i < totalHashes; i++) { bytes32 a = leafPos < leavesLen ? leaves[leafPos++] : hashes[hashPos++]; - bytes32 b = proofFlag[i] ? leafPos < leavesLen ? leaves[leafPos++] : hashes[hashPos++] : proofs[proofPos++]; + bytes32 b = proofFlags[i] ? leafPos < leavesLen ? leaves[leafPos++] : hashes[hashPos++] : proof[proofPos++]; hashes[i] = _hashPair(a, b); } @@ -135,7 +133,7 @@ library MerkleProof { } else if (leavesLen > 0) { return leaves[0]; } else { - return proofs[0]; + return proof[0]; } } diff --git a/test/utils/cryptography/MerkleProof.test.js b/test/utils/cryptography/MerkleProof.test.js index eeef6f1b5..1bb3de7f1 100644 --- a/test/utils/cryptography/MerkleProof.test.js +++ b/test/utils/cryptography/MerkleProof.test.js @@ -78,7 +78,7 @@ contract('MerkleProof', function (accounts) { const proof = merkleTree.getMultiProof(proofLeaves); const proofFlags = merkleTree.getProofFlags(proofLeaves, proof); - expect(await this.merkleProof.multiProofVerify(root, proofLeaves, proof, proofFlags)).to.equal(true); + expect(await this.merkleProof.multiProofVerify(proof, proofFlags, root, proofLeaves)).to.equal(true); }); it('returns false for an invalid Merkle multi proof', async function () { @@ -91,7 +91,7 @@ contract('MerkleProof', function (accounts) { const badProof = badMerkleTree.getMultiProof(badProofLeaves); const badProofFlags = badMerkleTree.getProofFlags(badProofLeaves, badProof); - expect(await this.merkleProof.multiProofVerify(root, badProofLeaves, badProof, badProofFlags)).to.equal(false); + expect(await this.merkleProof.multiProofVerify(badProof, badProofFlags, root, badProofLeaves)).to.equal(false); }); it('revert with invalid multi proof #1', async function () { @@ -104,10 +104,10 @@ contract('MerkleProof', function (accounts) { await expectRevert( this.merkleProof.multiProofVerify( - root, - [ leaves[0], badLeaf ], // A, E [ leaves[1], fill, merkleTree.layers[1][1] ], [ false, false, false ], + root, + [ leaves[0], badLeaf ], // A, E ), 'MerkleProof: invalid multiproof', ); @@ -123,10 +123,10 @@ contract('MerkleProof', function (accounts) { await expectRevert( this.merkleProof.multiProofVerify( - root, - [ badLeaf, leaves[0] ], // A, E [ leaves[1], fill, merkleTree.layers[1][1] ], [ false, false, false, false ], + root, + [ badLeaf, leaves[0] ], // A, E ), 'reverted with panic code 0x32', ); @@ -141,7 +141,7 @@ contract('MerkleProof', function (accounts) { const proof = merkleTree.getMultiProof(proofLeaves); const proofFlags = merkleTree.getProofFlags(proofLeaves, proof); - expect(await this.merkleProof.multiProofVerify(root, proofLeaves, proof, proofFlags)).to.equal(true); + expect(await this.merkleProof.multiProofVerify(proof, proofFlags, root, proofLeaves)).to.equal(true); }); it('limit case: can prove empty leaves', async function () { @@ -149,7 +149,7 @@ contract('MerkleProof', function (accounts) { const merkleTree = new MerkleTree(leaves, keccak256, { sort: true }); const root = merkleTree.getRoot(); - expect(await this.merkleProof.multiProofVerify(root, [], [ root ], [])).to.equal(true); + expect(await this.merkleProof.multiProofVerify([ root ], [], root, [])).to.equal(true); }); }); });