Switch the rest of the spec to MAX_EFFECTIVE_BALANCE_ELECTRA#3783
Switch the rest of the spec to MAX_EFFECTIVE_BALANCE_ELECTRA#3783hwwhww merged 6 commits intoethereum:devfrom
Conversation
| if not is_post_electra(spec): | ||
| return misc_balances(spec) |
There was a problem hiding this comment.
is it possible that, instead of adding new misc_balances_electra, we modify misc_balances by adding new if is_post_electra(spec) logic?
| if not is_post_electra(spec): | ||
| return default_balances(spec) |
There was a problem hiding this comment.
ditto. Is it possible that instead of adding new default_balances_electra, we modify default_balances by adding new if is_post_electra(spec) logic?
There was a problem hiding this comment.
I was thinking about this approach but then decided to keep default balances as is because it makes a test run with 32 ETH balance validators which is also important for Electra. Ideally, we should run tests with MIN_ACTIVATION_BALANCE, MAX_EFFECTIVE_BALANCE_ELECTRA and something in between. It is probably better to handle this modification separately.
There was a problem hiding this comment.
It's probably more just the naming problem; when I read default_balances_electra, I thought it is just "the default balances post-electra," but it is actually compatible with pre-electra. But I understand what you wanted to present.
There was a problem hiding this comment.
what about calling it max_balances instead? To me it explains pretty clearly what it does
Co-authored-by: Hsiao-Wei Wang <[email protected]>
| if not is_post_electra(spec): | ||
| return default_balances(spec) |
There was a problem hiding this comment.
It's probably more just the naming problem; when I read default_balances_electra, I thought it is just "the default balances post-electra," but it is actually compatible with pre-electra. But I understand what you wanted to present.
Co-authored-by: Hsiao-Wei Wang <[email protected]>
hwwhww
force-pushed
the
electra-maxeb-preset
branch
from
ba90964 to
718aadf
Compare
| exit_epoch=spec.FAR_FUTURE_EPOCH, | ||
| withdrawable_epoch=spec.FAR_FUTURE_EPOCH, | ||
| effective_balance=min(balance - balance % spec.EFFECTIVE_BALANCE_INCREMENT, spec.MAX_EFFECTIVE_BALANCE) | ||
| effective_balance=min(balance - balance % spec.EFFECTIVE_BALANCE_INCREMENT, max_effective_balace) |
There was a problem hiding this comment.
| effective_balance=min(balance - balance % spec.EFFECTIVE_BALANCE_INCREMENT, max_effective_balace) | |
| effective_balance=min(balance - balance % spec.EFFECTIVE_BALANCE_INCREMENT, max_effective_balance) |
| else: | ||
| # insecurely use pubkey as withdrawal key as well | ||
| withdrawal_credentials = spec.BLS_WITHDRAWAL_PREFIX + spec.hash(withdrawal_pubkey)[1:] | ||
| max_effective_balace = spec.MAX_EFFECTIVE_BALANCE |
There was a problem hiding this comment.
| max_effective_balace = spec.MAX_EFFECTIVE_BALANCE | |
| max_effective_balance = spec.MAX_EFFECTIVE_BALANCE |
| else: | ||
| # insecurely use pubkey as withdrawal key as well | ||
| withdrawal_credentials = spec.BLS_WITHDRAWAL_PREFIX + spec.hash(withdrawal_pubkey)[1:] | ||
| max_effective_balace = spec.MAX_EFFECTIVE_BALANCE_ELECTRA |
There was a problem hiding this comment.
| max_effective_balace = spec.MAX_EFFECTIVE_BALANCE_ELECTRA | |
| max_effective_balance = spec.MAX_EFFECTIVE_BALANCE_ELECTRA |
There was a problem hiding this comment.
ooops. will fix it after the release.
Thanks for the review!
ralexstokes
left a comment
ralexstokes
left a comment
There was a problem hiding this comment.
lgtm! +1 to hww's comments, but it looks like something was sorted
5 participants
This PR makes the rest of the places of the spec to use the new
MAX_EFFECTIVE_BALANCE_ELECTRApreset instead of the old one, namely:initialize_beacon_state_from_eth1get_next_sync_committee_indicesOne exception is
compute_weak_subjectivity_periodfunction which implements the strategy outlined in the WS research paper thus doesn’t need to be updated.This change entails the following updates into the spec tests:
MAX_EFFECTIVE_BALANCE_ELECTRAin thebuild_mock_validatorfor post-Electra forksMAX_EFFECTIVE_BALANCE_ELECTRAintest_initialize_beacon_state_some_small_balanceswhen post-ElectraMAX_EFFECTIVE_BALANCE_ELECTRAbalance for sync committee tests post-Electra affected by the change toget_next_sync_committee_indices