skip eth1data voting after electra #14835
Conversation
james-prysm
requested review from
kasey,
nalepae,
saolyn,
nisdas,
terencechain and
prestonvanloon
and removed request for
nalepae and
saolyn
beacon-chain/core/helpers/legacy.go
Outdated
| } | ||
|
|
||
| // Should not happen, but if the canonicalEth1Data is nil | ||
| // default to legacy |
There was a problem hiding this comment.
This should be impossible, I don't think this is the right approach
| // post eth1 deposits, the Eth 1 data will then be frozen | ||
| if !helpers.IsLegacyDepositProcessPeriod(beaconState, vs.HeadFetcher.HeadETH1Data()) { | ||
| return vs.HeadFetcher.HeadETH1Data(), nil | ||
| } | ||
|
|
There was a problem hiding this comment.
First, I'm not quite sure that eth1_data will be frozen after the transition period, as it's up to each client team how to handle Eth1 polling after Electra.
Second, if we decided to freeze eth1_data, isn't it fine to simply fetch from beaconState, like beaconState.Eth1Data()?
There was a problem hiding this comment.
thats's a good point should just do it from beacon state. there is an update to the spec here
ethereum/consensus-specs#4106
There was a problem hiding this comment.
DepositRequestHaveStarted seems much better to read, and more aligned with the latest spec. I will use this for my work also!
There was a problem hiding this comment.
i should rename to Requests oops
beacon-chain/core/helpers/legacy.go
Outdated
| } | ||
|
|
||
| //canonicalEth1Data should never be nil | ||
| eth1DepositIndexLimit := math.Min(canonicalEth1Data.DepositCount, requestsStartIndex) |
There was a problem hiding this comment.
Co-authored-by: Jun Song <[email protected]>
What type of PR is this?
Feature
What does this PR do? Why is it needed?
https://eips.ethereum.org/EIPS/eip-6110
mentions the following
Eth 1 voting should stop post electra and the eth1 transition period. This PR adds a check that has already been added in deposit packing by @syjn99 in #14697
Which issues(s) does this PR fix?
Fixes #
Other notes for review
Acknowledgements