-
-
Notifications
You must be signed in to change notification settings - Fork 300
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: devnet-4 support #7154
feat: devnet-4 support #7154
Conversation
…7118) Include skipped partial withdrawal to count
* initial commit * process deposits on epoch processing * lint * Fix spec test * Remove commented out code * Update packages/state-transition/src/epoch/processPendingDeposits.ts Co-authored-by: twoeths <[email protected]> * Address comments * lint --------- Co-authored-by: twoeths <[email protected]>
* Pass execution requests in bytes * lint
* initial commit * Update packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts Co-authored-by: Nico Flaig <[email protected]> --------- Co-authored-by: Nico Flaig <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## unstable #7154 +/- ##
============================================
- Coverage 49.21% 49.18% -0.04%
============================================
Files 598 598
Lines 39726 39770 +44
Branches 2092 2097 +5
============================================
+ Hits 19550 19559 +9
- Misses 20136 20170 +34
- Partials 40 41 +1 |
* New compound switching logic * clean up * fix: remove newCompoundingValidators in EpochTransitionCache * Bump spec version * Address comment * address comment * Lint --------- Co-authored-by: Tuyen Nguyen <[email protected]>
* chore: clean up single-used function * Remove Uint8Array * Present state-transition constants in Uint8Array
we should probably forward merge this into unstable instead of squashing it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did a cursory, looks good to me
Performance Report✔️ no performance regression detected Full benchmark results
|
It will be hard since the PR has a mix of direct commits and commits from other PRs. The easiest way is to squash the commits, and just reference the other PRs in the description of this PR. |
@@ -25,12 +31,19 @@ describe("Ensure config is synced", () => { | |||
}); | |||
|
|||
function assertCorrectPreset(localPreset: BeaconPreset, remotePreset: BeaconPreset): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remote preset is missing MAX_BLOBS_PER_BLOCK
since it has moved to config. But Lodestar has not made it configurable and it remains in local config.
Adding ignoredLocalPresetFields
is a temporary workaround to make this test pass such that this PR is safe to be merged into unstable.
See #7172 for details
} | ||
} else { | ||
if (fork < ForkSeq.electra) { | ||
const isNewValidator = cachedIndex === null || !Number.isSafeInteger(cachedIndex) || cachedIndex >= validators.length; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to add back condition cachedIndex >= validators.length
to pass the benchmark check. @twoeths
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Benchmark seems to pass, is this still an issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Benchmark seems to pass, is this still an issue?
It is passing because I added back this condition
We can squash those extra commits in this PR before merging |
yes pls |
🎉 This PR is included in v1.23.0 🎉 |
Includes all necessary consensus spec changes versioned
v1.5.0-alpha.7
andv1.5.0-alpha.8
. Also implements Electra builder specs.partialWithdrawalsCount
fix: include skipped partial withdrawal topartialWithdrawalsCount
#7118