[DRAFT] pyspec changes#4
Closed
michaelneuder wants to merge 14 commits into
Closed
Conversation
hwwhww
reviewed
May 31, 2023
hwwhww
left a comment
hwwhww
left a comment
There was a problem hiding this comment.
If you see an error at "assert FINALIZED_ROOT_INDEX == get_generalized_index(BeaconState, 'finalized_checkpoint', 'root')", it's because:
- We have to calculate the
BeaconStateSSZ GeneralizedIndex ahead for light client specs. - Your PR inserted new fields in the middle of
BeaconStateand changed the tree position. That would makeFINALIZED_ROOT_INDEXvalue change in the building stage.
By convention, we only add new fields at the end of an existing Container.
potuz
reviewed
Aug 23, 2023
| # --- MODIFIED --- # | ||
| # Increase balance by deposit amount, up to MIN_ACTIVATION_BALANCE | ||
| index = ValidatorIndex(validator_pubkeys.index(pubkey)) | ||
| increase_balance(state, index, min(amount, MIN_ACTIVATION_BALANCE - state.balances[index])) |
There was a problem hiding this comment.
-
If an active validator has a balance above
MIN_ACTIVATION_BALANCEand deposits, we can have a negative value forMIN_ACTIVATION_BALANCE - state.balances[index], this will overflow the uint64 thatincrease_balancetakes. -
This changes the behavior from the current: "the deposit is useless but it's not lost ETH" to "you just burned your ETH if you deposit twice".
Collaborator
There was a problem hiding this comment.
- Please look at this PR as reference for latest canonical MaxEB diff [DRAFT] Increase
MAX_EFFECTIVE_BALANCEminimal spec change #3
updated beacon-chain.md with changes from the spec diff PR
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Spec changes associated with https://notes.ethereum.org/@mikeneuder/increase-maxeb, built from https://github.com/ethereum/consensus-specs/blob/dev/docs/docs/new-feature.md.
For analysis on the changes see https://notes.ethereum.org/@fradamt/meb-increase-security
Diff view – #3
Changes: