feat: support electra devnet-1 #6892
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
ensi321
commented
Jun 18, 2024
| if (i === indices.length) { | ||
| // Spec does not have this condition to end infinite loop. Spec test won't pass if we | ||
| // only loop the validator indices once. Electra spec test requires MAX_ITERATION_OVER_VALIDATORS >= 6 | ||
| if (i === indices.length * MAX_ITERATION_OVER_VALIDATORS) { |
There was a problem hiding this comment.
In the original spec, this loop is infinite where as Lodestar has a early return statement to return -1 after looping through the validator set once.
Lodestar will return -1 for proposer index when the validator set is small. This phenomenon is surfaced during spec testing. Tests will fail unless we loop the validator set at least 6 times.
As a workaround, I have introduced MAX_ITERATION_OVER_VALIDATORS to relax the early return condition, but would also consider to remove it entirely to align with the spec here
If someone can point out the original intention of this early return statement, please shed some light cc @wemeetagain
There was a problem hiding this comment.
I think it's not correct to do early return after an iteration, the same validator index could have different randByte value after each iteration
this could be read as an improvement we made compared to spec, but it's important to be spec compliant first
suggest removing this condition completely
g11tech
reviewed
Jun 21, 2024
g11tech
reviewed
Jun 21, 2024
g11tech
reviewed
Jun 21, 2024
g11tech
reviewed
Jun 21, 2024
twoeths
reviewed
Jun 22, 2024
g11tech
pushed a commit
that referenced
this pull request
Jun 25, 2024
* Update spec test version * Use new max effective balance * Relax loop breaking condition for `computeProposerIndex` * fix remaining * check-types & lint * Skip invalid test * Fix rebase + lint * Remove early return statement in `computeProposers` * Address comment * Address comment
g11tech
pushed a commit
that referenced
this pull request
Jun 25, 2024
* Update spec test version * Use new max effective balance * Relax loop breaking condition for `computeProposerIndex` * fix remaining * check-types & lint * Skip invalid test * Fix rebase + lint * Remove early return statement in `computeProposers` * Address comment * Address comment
g11tech
pushed a commit
that referenced
this pull request
Jul 1, 2024
* Update spec test version * Use new max effective balance * Relax loop breaking condition for `computeProposerIndex` * fix remaining * check-types & lint * Skip invalid test * Fix rebase + lint * Remove early return statement in `computeProposers` * Address comment * Address comment
g11tech
pushed a commit
that referenced
this pull request
Jul 30, 2024
* Update spec test version * Use new max effective balance * Relax loop breaking condition for `computeProposerIndex` * fix remaining * check-types & lint * Skip invalid test * Fix rebase + lint * Remove early return statement in `computeProposers` * Address comment * Address comment
g11tech
pushed a commit
that referenced
this pull request
Jul 31, 2024
* Update spec test version * Use new max effective balance * Relax loop breaking condition for `computeProposerIndex` * fix remaining * check-types & lint * Skip invalid test * Fix rebase + lint * Remove early return statement in `computeProposers` * Address comment * Address comment
g11tech
pushed a commit
that referenced
this pull request
Aug 2, 2024
* Update spec test version * Use new max effective balance * Relax loop breaking condition for `computeProposerIndex` * fix remaining * check-types & lint * Skip invalid test * Fix rebase + lint * Remove early return statement in `computeProposers` * Address comment * Address comment
g11tech
pushed a commit
that referenced
this pull request
Aug 9, 2024
* Update spec test version * Use new max effective balance * Relax loop breaking condition for `computeProposerIndex` * fix remaining * check-types & lint * Skip invalid test * Fix rebase + lint * Remove early return statement in `computeProposers` * Address comment * Address comment
g11tech
pushed a commit
that referenced
this pull request
Aug 9, 2024
* Update spec test version * Use new max effective balance * Relax loop breaking condition for `computeProposerIndex` * fix remaining * check-types & lint * Skip invalid test * Fix rebase + lint * Remove early return statement in `computeProposers` * Address comment * Address comment
g11tech
pushed a commit
that referenced
this pull request
Aug 23, 2024
* Update spec test version * Use new max effective balance * Relax loop breaking condition for `computeProposerIndex` * fix remaining * check-types & lint * Skip invalid test * Fix rebase + lint * Remove early return statement in `computeProposers` * Address comment * Address comment
g11tech
pushed a commit
that referenced
this pull request
Aug 27, 2024
* Update spec test version * Use new max effective balance * Relax loop breaking condition for `computeProposerIndex` * fix remaining * check-types & lint * Skip invalid test * Fix rebase + lint * Remove early return statement in `computeProposers` * Address comment * Address comment
philknows
pushed a commit
that referenced
this pull request
Sep 3, 2024
* Update spec test version * Use new max effective balance * Relax loop breaking condition for `computeProposerIndex` * fix remaining * check-types & lint * Skip invalid test * Fix rebase + lint * Remove early return statement in `computeProposers` * Address comment * Address comment
|
🎉 This PR is included in v1.22.0 🎉 |
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.
Bump spec test version to
v1.5.0-alpha.3.Fix everything to make it pass:
computeProposersis now fork aware to useMAX_EFFECTIVE_BALANCE_ELECTRAto determine post-electraepoch_processing.test.tsandoperations.test.tsto correctly run associated test casesincorrect_not_enough_consolidation_churn_availabletest as it passes in an invalid beacon state. This test will also be rewritten in the next release. See Updatetest_incorrect_not_enough_consolidation_churn_availableand add assertions to test cases ethereum/consensus-specs#3814Misc:
getForkSeqFromEpochto better convert epoch toForkSeq