status-im
/
nimbus-eth2
mirror of https://github.com/status-im/nimbus-eth2.git
2
0
Fork 0
Code Issues Projects Releases Wiki Activity

Fix a block proposal issue caused by incorrect merkle proofs 

The key change here is that `addChunksAndGenMerkleProofs` is called
with all pending deposits instead of just the deposits included in
the block. The later was effectively producing merkle proofs against
a different root.
This commit is contained in:
Zahary Karadjov 2020-12-02 20:15:36 +02:00 committed by zah
parent b4defc9b4a
commit 027d2547cd
2 changed files with 17 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
beacon_chain/eth1_monitor.nim
View File

@ -455,7 +455,7 @@ proc advanceMerkleizer(eth1Chain: Eth1Chain,
return merkleizer.getChunkCount == depositIndex
proc getDepositsRange(eth1Chain: Eth1Chain, first, last: uint64): seq[Deposit] =
proc getDepositsRange(eth1Chain: Eth1Chain, first, last: uint64): seq[DepositData] =
# TODO It's possible to make this faster by performing binary search that
# will locate the blocks holding the `first` and `last` indices.
# TODO There is an assumption here that the requested range will be present
@ -472,7 +472,7 @@ proc getDepositsRange(eth1Chain: Eth1Chain, first, last: uint64): seq[Deposit] =
for i in 0 ..< blk.deposits.lenu64:
let globalIdx = firstDepositIdxInBlk + i
if globalIdx >= first and globalIdx < last:
result.add Deposit(data: blk.deposits[i])
result.add blk.deposits[i]
proc lowerBound(chain: Eth1Chain, depositCount: uint64): Eth1Block =
# TODO: This can be replaced with a proper binary search in the
@ -565,21 +565,22 @@ proc getBlockProposalData*(m: Eth1Monitor,
if pendingDepositsCount > 0:
if hasLatestDeposits:
let totalDepositsInNewBlock = min(MAX_DEPOSITS, pendingDepositsCount)
var deposits = m.eth1Chain.getDepositsRange(
let
totalDepositsInNewBlock = min(MAX_DEPOSITS, pendingDepositsCount)
deposits = m.eth1Chain.getDepositsRange(
state.eth1_deposit_index,
state.eth1_deposit_index + totalDepositsInNewBlock)
let depositRoots = mapIt(deposits, hash_tree_root(it.data))
state.eth1_deposit_index + pendingDepositsCount)
depositRoots = mapIt(deposits, hash_tree_root(it))
var scratchMerkleizer = copy m.eth2FinalizedDepositsMerkleizer
if m.eth1Chain.advanceMerkleizer(scratchMerkleizer, state.eth1_deposit_index):
let proofs = scratchMerkleizer.addChunksAndGenMerkleProofs(depositRoots)
for i in 0 ..< deposits.lenu64:
deposits[i].proof[0..31] = proofs.getProof(i.int)
deposits[i].proof[32].data[0..7] =
toBytesLE uint64(state.eth1_deposit_index + i + 1)
swap(result.deposits, deposits)
for i in 0 ..< totalDepositsInNewBlock:
var proof: array[33, Eth2Digest]
proof[0..31] = proofs.getProof(i.int)
proof[32] = default(Eth2Digest)
proof[32].data[0..7] = toBytesLE uint64(result.vote.deposit_count)
result.deposits.add Deposit(data: deposits[i], proof: proof)
else:
error "The Eth1 chain is in inconsistent state" # This should not really happen
result.hasMissingDeposits = true

4
beacon_chain/merkle_minimal.nim
View File

@ -34,7 +34,9 @@ func attachMerkleProofs*(deposits: var openArray[Deposit]) =
template getProof*(proofs: seq[Eth2Digest], idxParam: int): openArray[Eth2Digest] =
let
idx = idxParam
startIdx = idx * DEPOSIT_CONTRACT_TREE_DEPTH
## TODO: It's surprising that we have to do +1 here.
## It seems that `depositContractLimit` is set too high.
startIdx = idx * (DEPOSIT_CONTRACT_TREE_DEPTH + 1)
endIdx = startIdx + DEPOSIT_CONTRACT_TREE_DEPTH - 1
proofs.toOpenArray(startIdx, endIdx)