From e44b51e95551e18dc3b8abf1affd69dd8536a86f Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Thu, 11 May 2023 10:45:55 +0200 Subject: [PATCH] fix `attachMerkleProofs` to support multiple deposits (#4932) `attachMerkleProofs` is used by `mockUpdateStateForNewDeposit` to create a single deposit. The function doesn't work correctly when trying with with multiple deposits, though. Fix this to enable more complex tests, and also return the `deposit_root` for forming matching `Eth1Data`. --- beacon_chain/eth1/merkle_minimal.nim | 25 ++++++++++++++----------- tests/mocking/mock_deposits.nim | 5 ++--- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/beacon_chain/eth1/merkle_minimal.nim b/beacon_chain/eth1/merkle_minimal.nim index a58e0a34f..722a3830a 100644 --- a/beacon_chain/eth1/merkle_minimal.nim +++ b/beacon_chain/eth1/merkle_minimal.nim @@ -19,17 +19,8 @@ import ../spec/[eth2_merkleization, digest], ../spec/datatypes/base -func attachMerkleProofs*(deposits: var openArray[Deposit]) = - let depositsRoots = mapIt(deposits, hash_tree_root(it.data)) - - var incrementalMerkleProofs = createMerkleizer(DEPOSIT_CONTRACT_LIMIT) - - for i in 0 ..< depositsRoots.len: - incrementalMerkleProofs.addChunkAndGenMerkleProof(depositsRoots[i], deposits[i].proof) - deposits[i].proof[32] = default(Eth2Digest) - deposits[i].proof[32].data[0..7] = toBytesLE uint64(i + 1) - -template getProof*(proofs: seq[Eth2Digest], idxParam: int): openArray[Eth2Digest] = +template getProof*( + proofs: seq[Eth2Digest], idxParam: int): openArray[Eth2Digest] = let idx = idxParam ## TODO: It's surprising that we have to do +1 here. @@ -38,3 +29,15 @@ template getProof*(proofs: seq[Eth2Digest], idxParam: int): openArray[Eth2Digest endIdx = startIdx + DEPOSIT_CONTRACT_TREE_DEPTH - 1 proofs.toOpenArray(startIdx, endIdx) +func attachMerkleProofs*(deposits: var openArray[Deposit]): Eth2Digest = + let depositsRoots = mapIt(deposits, hash_tree_root(it.data)) + + var merkleizer = createMerkleizer(DEPOSIT_CONTRACT_LIMIT) + let proofs = merkleizer.addChunksAndGenMerkleProofs(depositsRoots) + for i in 0 ..< depositsRoots.len: + deposits[i].proof[0 ..< DEPOSIT_CONTRACT_TREE_DEPTH] = getProof(proofs, i) + deposits[i].proof[DEPOSIT_CONTRACT_TREE_DEPTH] = default(Eth2Digest) + deposits[i].proof[DEPOSIT_CONTRACT_TREE_DEPTH].data[0..7] = + toBytesLE deposits.lenu64 + + mixInLength(merkleizer.getFinalHash(), deposits.len) diff --git a/tests/mocking/mock_deposits.nim b/tests/mocking/mock_deposits.nim index 4a7c26b8f..b78dc069a 100644 --- a/tests/mocking/mock_deposits.nim +++ b/tests/mocking/mock_deposits.nim @@ -126,12 +126,11 @@ proc mockUpdateStateForNewDeposit*( ) var result_seq = @[result] - attachMerkleProofs(result_seq) + let deposit_root = attachMerkleProofs(result_seq) result.proof = result_seq[0].proof # TODO: this logic from the consensus-specs test suite seems strange # but confirmed by running it state.eth1_deposit_index = 0 - state.eth1_data.deposit_root = - hash_tree_root(List[DepositData, 2'i64^DEPOSIT_CONTRACT_TREE_DEPTH](@[result.data])) + state.eth1_data.deposit_root = deposit_root state.eth1_data.deposit_count = 1