Add capella hardfork and types #4568
Merged
Conversation
This file contains 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
Performance Report✔️ no performance regression detected Full benchmark results
|
g11tech
force-pushed
the
g11tech/capella-fork
branch
2 times, most recently
from
c850ac5
to
5161b99
Compare
25 tasks
dapplion
reviewed
Sep 20, 2022
| @@ -36,6 +36,9 @@ export const chainConfig: IChainConfig = { | |||
| // Bellatrix | |||
| BELLATRIX_FORK_VERSION: b("0x02000000"), | |||
| BELLATRIX_FORK_EPOCH: 144896, // Sept 6, 2022, 11:34:47am UTC | |||
| // Capella | |||
| CAPELLA_FORK_VERSION: b("0x03000000"), | |||
| CAPELLA_FORK_EPOCH: Infinity, | |||
| // Sharding | |||
| SHARDING_FORK_VERSION: b("0x03000000"), | |||
There was a problem hiding this comment.
Should not have the same fork_version
There was a problem hiding this comment.
Remove, SHARDING_* fields, no longer in upstream spec
There was a problem hiding this comment.
should i rename them to EIP4844 something, thats what the new spec calls them
dapplion
reviewed
Sep 20, 2022
| }, | ||
| altair: { | ||
| BeaconBlockBody: altair.BeaconBlockBody, | ||
| BeaconBlock: altair.BeaconBlock, | ||
| SignedBeaconBlock: altair.SignedBeaconBlock, | ||
| BeaconState: altair.BeaconState, | ||
| Metadata: altair.Metadata, | ||
| // Not used in altair but added for type consitency | ||
| ExecutionPayload: bellatrix.ExecutionPayload, | ||
| ExecutionPayloadHeader: bellatrix.ExecutionPayloadHeader, |
There was a problem hiding this comment.
to be dropped after rebase right?
There was a problem hiding this comment.
i plan to move them to a separate allTypesExecution as some fork based usage is there
g11tech
force-pushed
the
g11tech/capella-fork
branch
from
ba4dc64
to
90c80fb
Compare
dapplion
reviewed
Sep 20, 2022
| withdrawableEpoch: FAR_FUTURE_EPOCH, | ||
| effectiveBalance, | ||
| slashed: false, | ||
| fullyWithdrawnEpoch: FAR_FUTURE_EPOCH, |
There was a problem hiding this comment.
Note, this property is likely to be removed
There was a problem hiding this comment.
awesome, will simplify processing deposits back to original, will comment it
There was a problem hiding this comment.
@dapplion should i remove this fullyWithdrawnEpoch from here ?
* handle capella state upgrade case * fix more spec tests * lint params * add the new domain for constants check * rebase fixes
g11tech
force-pushed
the
g11tech/capella-fork
branch
from
d557a64
to
d7ff4a0
Compare
wemeetagain
reviewed
Sep 26, 2022
|
|
||
| // Union at the TreeViewDU level | ||
| // - Works well as function argument and as generic type for allForks functions | ||
| // | ||
| // Quasy equivalent to | ||
| // CompositeViewDU<typeof ssz.phase0.BeaconState | typeof ssz.altair.BeaconState | typeof ssz.bellatrix.BeaconState> | ||
| export type BeaconStateAllForks = BeaconStatePhase0 | BeaconStateAltair | BeaconStateBellatrix; | ||
| export type BeaconStateAllForks = BeaconStatePhase0 | BeaconStateAltair | BeaconStateBellatrix | BeaconStateCapella; | ||
| export type BeaconStateExecutions = BeaconStateBellatrix | BeaconStateCapella; |
There was a problem hiding this comment.
Might call this BeaconStatePostMerge (somehow easier than post-bellatrix) or BeaconStateWithExecution or BeaconStateWithEL. 🤷
We're probably going to have more types of beacon states over time with different capabilities, so maybe we can figure out a naming convention that can suit.
Just brainstorming:
Naming after fork:
BeaconStatePostB - beacon states bellatrix and after
BeaconStatePostC - beacon states capella and after
or
postBellatrix.BeaconState, postCapella.BeaconState
Naming by capability:
BeaconStateWithExecution - beacon states bellatrix and after
BeaconStateWithWithdrawals - beacon states capella and after
thoughts?
There was a problem hiding this comment.
i like capability idea since that makes code more intutive 🙂
There was a problem hiding this comment.
Both BeaconStatePostBellatrix or BeaconStateWithExecution sound good
dapplion
approved these changes
Oct 2, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Add capella hardfork and types
Dependent on
base is set master to run the github workflows
Part of