diff --git a/packages/incremental-merkle-tree.sol/contracts/IncrementalQuinTree.sol b/packages/incremental-merkle-tree.sol/contracts/IncrementalQuinTree.sol index 5131d9f..96118c8 100644 --- a/packages/incremental-merkle-tree.sol/contracts/IncrementalQuinTree.sol +++ b/packages/incremental-merkle-tree.sol/contracts/IncrementalQuinTree.sol @@ -116,9 +116,10 @@ library IncrementalQuinTree { uint256 depth = self.depth; uint256 hash = newLeaf; + uint256 updateIndex; for (uint8 i = 0; i < depth; ) { uint256[5] memory nodes; - + updateIndex += proofPathIndices[i] * 5 ** i; for (uint8 j = 0; j < 5; ) { if (j < proofPathIndices[i]) { nodes[j] = proofSiblings[i][j]; @@ -142,6 +143,7 @@ library IncrementalQuinTree { ++i; } } + require(updateIndex < self.numberOfLeaves, "IncrementalQuinTree: leaf index out of range"); self.root = hash; } diff --git a/packages/incremental-merkle-tree.sol/test/IncrementalQuinTreeTest.ts b/packages/incremental-merkle-tree.sol/test/IncrementalQuinTreeTest.ts index a35b645..36bb3db 100644 --- a/packages/incremental-merkle-tree.sol/test/IncrementalQuinTreeTest.ts +++ b/packages/incremental-merkle-tree.sol/test/IncrementalQuinTreeTest.ts @@ -185,6 +185,48 @@ describe("IncrementalQuinTreeTest", () => { await expect(transaction).to.emit(contract, "LeafRemoved").withArgs(treeId, BigInt(2), root) }) + it("Should not update a leaf that hasn't been inserted yet", async () => { + // deploy a new, empty tree + const treeId = ethers.utils.formatBytes32String("brokenTree") + contract.createTree(treeId, depth) + const tree = createTree(depth, 0, 5) + + // insert 4 leaves into the tree + for (let i = 0; i < 4; i += 1) { + tree.insert(BigInt(i + 1)) + await contract.insertLeaf(treeId, BigInt(i + 1)) + } + + // we're going to try to update leaf 7, despite there only being 4 leaves in the tree + const leaf = BigInt(42069) + + // note that we can insert zeros into the js library tree and the root won't change! + // that's because we use the zeros optimization to calculate the roots efficiently. + // technically speaking, there isn't an "empty" tree, there is only a tree that is + // entirely full of the zero value at every index. therefore inserting the zero value + // at any point into an incremental merkle tree doesn't change it's root, because + // that is already the data the root was calculated from previously. in principle, + // we can update any leaf that hasn't been inserted yet using this method + const rootBeforeZeros = tree.root + tree.insert(0) + tree.insert(0) + tree.insert(0) + // the root doesn't change because the tree started full with 0s! + expect(tree.root).to.be.equal(rootBeforeZeros) + + // now we can make a merkle proof of zero being included at the uninitialized index + const { pathIndices, siblings } = tree.createProof(6) + + const transaction = contract.updateLeaf( + treeId, + BigInt(0), + leaf, + siblings, + pathIndices + ) + await expect(transaction).to.be.revertedWith("IncrementalQuinTree: leaf index out of range") + }) + it("Should not remove a leaf that does not exist", async () => { const treeId = ethers.utils.formatBytes32String("hello") const tree = createTree(depth, 3, 5)