lodestar
lodestar copied to clipboard
feat: implement EIP-6110
Motivation
To implement EIP-6110 and to modify the existed codebase to accomodate EIP-6110 including:
- Introduce unfinalized
pubkey2Index
inEpochCache
that is fork specific and will be cloned on state.clone() - To modify existed
pubkey2Index
andindex2Pubkey
inEpochCache
such that they only contain validators that are confirmed active, in the activation queue, or initialized but pending entering activation queue.
Description
Implementation
PubkeyCache
- Introduce class
UnfinalizedPubkeyIndexMap
which is an alias of immutable.Map - Existed
pubkey2Index
andindex2Pubkey
are renamed tofinalizedPubkey2index
andfinalizedIndex2pubkey
. - Add new field
unfinalizedPubkey2Index: UnfinalizedPubkeyIndexMap
in theEpochCache
and will be included during state cloning - Populating
finalizedPubkey2index
andfinalizedIndex2pubkey
happens duringafterProcessEpoch()
. At the same time,UnfinalizedPubkeyIndexMap
is pruned for every pubkeys that are inserted into finalized caches. - Introduce interfaces to hide finalized and unfinalized caches from callers. Callers to the cache should not interact with finalized and unfinalized cache directly
- Fork guard to ensure new pubkey cache logic does not get activated before
eip6110
- Add immutabe-js to the dependencies
EIP-6110
New
- Boilerplate code to add new fork
eip6110
- Update execution engine to use new api
-
engine_newPayloadV6110
-engine_getPayloadV6110
- Add
upgradeStateToEIP6110
- Add/update related containers
- Introduce new constants
-
MAX_DEPOSIT_RECEIPTS_PER_PAYLOAD
-
UNSET_DEPOSIT_RECEIPTS_START_INDEX
-
- Add
processDepositReceipt()
intoprocessOperation()
Modify
- Update current logic for eth1 deposit by introducing
getEth1DepositCount()
- Extract
applyDeposit()
logic out ofprocessDeposit()
Testing
- Add 6110 spec test
- Add 6110 sim test
- Add
besu
andbesudocker
to EL interop - Unit test for
getEth1DepositCount()
- Unit test for state cloning using new pubkey cache
- Perf test on moving pubkeys from unfinalized to finalized cache on new finalized checkpoint
- Memory test on unfinalized pubkey cache
Closes #5366
Open Point of Discussion There exists an identical PR opened in my personal repo, and it has some old discussions/concerns. I will migrate all points of discussions that are still open to here and will close the old PR imminently. Old PR for reference only: https://github.com/naviechan/lodestar/pull/1
My debate for the future of the PR is to merge to unstable or not. Lodestar has historically been eager to merge future fork code into unstable, ensuring that it has absolutely no impact on current forks. This PR implements a feature that still a bit far away but that I'm confident that we have to develop at some point. The arguments are:
- Merge to unstable: ensures the PR / branch never goes stale, with spec tests we will be always ready for when eventual testing for the PR happen. If the feature has some changes we can do those on unstable
- Not merge to unstable: 0% chance of disruption to current forks, but the PR / branch is very likely to go stale. If we have some deep structural change between now and a few months it could require to re-write some big parts
I would prefer to merge to unstable, being very confident that we will not have to pay any price in terms of stability of performance to current forks.
If we can safely merge to unstable
with some good stability guarantees, that's what I would prefer. I think EIP-6110 is mainly in an implementation + testing phase now, so hopefully no large changes. Main concern is if we're doing large changes to Lodestar in the medium-term which is likely.
To make this PR easier to review, I have reverted some of the trivial changes to reduce the diffs. Let me know if there is anything else we could do to make this easier🙂
This includes:
- Rename epoch cache's
pubkey2index
andindex2pubkey
tofinalizedPubkey2Index
andfinalizedIndex2Pubkey
- Restrict callers (esp those from beacon api) from getting direct access to
finalizedPubkey2Index
andfinalizedIndex2Pubkey
but to usegetPubkey()
,getValidatorIndex()
. - Add unfinished unit test for 6110 light client header
Will create a follow-up PR to re-introduce these changes after we sort out all controversial changes in this PR cc. @dapplion @g11tech @tuyennhv
Posting the perf test result on updateUnfinalizedPubkeys
ie. removing newly finalized pubkeys from unfinalizedPubkey2index
and add them to pubkey2Index
+ index2Pubkey
upon new finalized checkpoint. Open to discuss whether the benchmark is acceptable or further optimization should be made. cc. @dapplion @tuyennhv
Background
- Test is written in
updateUnfinalizedPubkeys.test.ts
which emulates the cache behaviour inchain. onForkChoiceFinalized()
-
updateUnfinalizedPubkeys
should be performed on average ~1 time per epoch. Whenever a new finalized checkpoint is set inForkChoiceStore
post 6110. - In post 6110, theoretically newly finalized pubkeys per epoch could reach maximum of 1271 * 32 = 40672. This happens when an epoch full of 30M gas blocks, each deposit in the blocks creates a new validator, and at least 40664 of them fail to reach activation balance. Practically speaking, 1000 pubkeys is already on the higher end depending on the amount of network activities.
Result
Benchmark date from Mon Nov 21 2023 - Intel Core i7-9750H @ 2.60Ghz ✔ updateUnfinalizedPubkeys - updating 10 pubkeys 1496.612 ops/s 668.1760 us/op - 276 runs 3.39 s ✔ updateUnfinalizedPubkeys - updating 100 pubkeys 174.9926 ops/s 5.714528 ms/op - 142 runs 2.19 s ✔ updateUnfinalizedPubkeys - updating 1000 pubkeys 10.17848 ops/s 98.24650 ms/op - 28 runs 3.75 s
Benchmark date from Fri Nov 24 2023 - Apple M2 ✔ updateUnfinalizedPubkeys - updating 10 pubkeys 2878.294 ops/s 347.4280 us/op - 389 runs 2.35 s ✔ updateUnfinalizedPubkeys - updating 100 pubkeys 296.2824 ops/s 3.375158 ms/op - 250 runs 1.79 s ✔ updateUnfinalizedPubkeys - updating 1000 pubkeys 20.07801 ops/s 49.80572 ms/op - 13 runs 1.18 s ✔ updateUnfinalizedPubkeys - updating 40672 pubkeys 0.4405013 ops/s 2.270141 s/op - 13 runs 32.1 s
Also want to share the amount of memory used by unfinalizedPubkeyCache
aka immutable.js Map when holding [1000, 10000, 100000] pubkeys which I believe is ~80% less than the vanilla map and it scales linearly:
UnfinalizedPubkey2Index 1000 keys - 274956.5 bytes / instance UnfinalizedPubkey2Index 10000 keys - 2591129.3 bytes / instance UnfinalizedPubkey2Index 100000 keys - 27261443.4 bytes / instance
Instead you can add a new cache into the state tracking historical validator length counts. On every epoch transition push the current count of validators. Once a state finalizes, retrieve count at index state.epoch - finalized_checkpoint.epoch. Then you don't need to retrieve the finalized state for this purpose.
@dapplion Can you please confirm if #006c26b suits what you are asking before I polish up and test the code?
I believe we only need to maintain historicalValidatorLengths
from the latest finalized cp epoch to the current epoch.
Also headState
is not a reliable indicator to stop the eth1Data polling so finalizedState
is still needed. But I think it shouldn’t be a big deal to stop the polling late if we fail to retrieve finalizedState
couple times in a row.
@dapplion Can you please confirm if #006c26b suits what you are asking before I polish up and test the code?
Looks good! continue
Codecov Report
Merging #6042 (2f10313) into electra-fork (9014c8c) will increase coverage by
0.05%
. The diff coverage is65.84%
.
Additional details and impacted files
@@ Coverage Diff @@
## electra-fork #6042 +/- ##
================================================
+ Coverage 61.88% 61.94% +0.05%
================================================
Files 556 559 +3
Lines 58108 58740 +632
Branches 1830 1844 +14
================================================
+ Hits 35962 36386 +424
- Misses 22109 22317 +208
Partials 37 37