avoid materializing potentially long deposits seq (#3947)

When fetching eth1 data and deposits for a new block proposal, the list
of deposits from previous eth1 data to the next one is fully loaded into
a `seq`. This can potentially be a very long list in active periods.
Changing this to an `iterator` saves memory by ensuring that the entire
list is no longer materialized; only the `DepositData` roots are needed.
This commit is contained in:
Etan Kissling 2022-08-12 15:52:06 +02:00 committed by GitHub
parent 03d6a1a934
commit c360db8194
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 45 additions and 11 deletions

View File

@ -150,10 +150,11 @@ OK: 2/2 Fail: 0/2 Skip: 0/2
OK: 10/12 Fail: 0/12 Skip: 2/12
## Eth1 monitor
```diff
+ Deposits chain OK
+ Rewrite HTTPS Infura URLs OK
+ Roundtrip engine RPC and consensus ExecutionPayload representations OK
```
OK: 2/2 Fail: 0/2 Skip: 0/2
OK: 3/3 Fail: 0/3 Skip: 0/3
## Eth2 specific discovery tests
```diff
+ Invalid attnets field OK
@ -586,4 +587,4 @@ OK: 1/1 Fail: 0/1 Skip: 0/1
OK: 9/9 Fail: 0/9 Skip: 0/9
---TOTAL---
OK: 327/332 Fail: 0/332 Skip: 5/332
OK: 328/333 Fail: 0/333 Skip: 5/333

View File

@ -30,7 +30,7 @@ from std/times import getTime, inSeconds, initTime, `-`
from ../spec/engine_authentication import getSignedIatToken
export
web3Types, deques,
web3Types, deques, base,
beacon_chain_db.DepositContractSnapshot
logScope:
@ -99,7 +99,7 @@ type
## the Eth2 validators according to the follow distance and
## the ETH1_VOTING_PERIOD
blocks: Deque[Eth1Block]
blocks*: Deque[Eth1Block]
## A non-forkable chain of blocks ending at the block with
## ETH1_FOLLOW_DISTANCE offset from the head.
@ -777,24 +777,24 @@ func advanceMerkleizer(chain: Eth1Chain,
return merkleizer.getChunkCount == depositIndex
func getDepositsRange(chain: Eth1Chain, first, last: uint64): seq[DepositData] =
iterator getDepositsRange*(chain: Eth1Chain, first, last: uint64): 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
# in the Eth1Chain. This should hold true at the single call site right
# now, but we need to guard the pre-conditions better.
# in the Eth1Chain. This should hold true at the call sites right now,
# but we need to guard the pre-conditions better.
for blk in chain.blocks:
if blk.depositCount <= first:
continue
let firstDepositIdxInBlk = blk.depositCount - blk.deposits.lenu64
if firstDepositIdxInBlk >= last:
return
break
for i in 0 ..< blk.deposits.lenu64:
let globalIdx = firstDepositIdxInBlk + i
if globalIdx >= first and globalIdx < last:
result.add blk.deposits[i]
yield blk.deposits[i]
func lowerBound(chain: Eth1Chain, depositCount: uint64): Eth1Block =
# TODO: This can be replaced with a proper binary search in the
@ -911,8 +911,13 @@ proc getBlockProposalData*(chain: var Eth1Chain,
let
totalDepositsInNewBlock = min(MAX_DEPOSITS, pendingDepositsCount)
postStateDepositIdx = stateDepositIdx + pendingDepositsCount
deposits = chain.getDepositsRange(stateDepositIdx, postStateDepositIdx)
depositRoots = mapIt(deposits, hash_tree_root(it))
var
deposits = newSeqOfCap[DepositData](totalDepositsInNewBlock)
depositRoots = newSeqOfCap[Eth2Digest](pendingDepositsCount)
for data in chain.getDepositsRange(stateDepositIdx, postStateDepositIdx):
if deposits.lenu64 < totalDepositsInNewBlock:
deposits.add data
depositRoots.add hash_tree_root(data)
var scratchMerkleizer = copy chain.finalizedDepositsMerkleizer
if chain.advanceMerkleizer(scratchMerkleizer, stateDepositIdx):

View File

@ -60,6 +60,34 @@ suite "Eth1 monitor":
gethWsUrl == "ws://localhost:8545"
test "Deposits chain":
var
chain = Eth1Chain()
depositIndex = 0.uint64
for i in 0 ..< (MAX_DEPOSITS + 1) * 3:
var deposits = newSeqOfCap[DepositData](i)
for _ in 0 ..< i mod (MAX_DEPOSITS + 1):
deposits.add DepositData(amount: depositIndex.Gwei)
inc depositIndex
const interval = defaultRuntimeConfig.SECONDS_PER_ETH1_BLOCK
chain.blocks.addLast Eth1Block(
number: i.Eth1BlockNumber,
timestamp: i.Eth1BlockTimestamp * interval,
deposits: deposits,
depositCount: depositIndex)
proc doTest(first, last: uint64) =
var idx = first
for data in chain.getDepositsRange(first, last):
check data.amount == idx.Gwei
inc idx
check idx == last
for i in 0 .. depositIndex:
for j in i .. depositIndex:
doTest(i, j)
test "Roundtrip engine RPC and consensus ExecutionPayload representations":
# Each Eth2Digest field is chosen randomly. Each uint64 field is random,
# with boosted probabilities for 0, 1, and high(uint64). There can be 0,