feat: load state from Uint8Array #6057
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
|
This reverts commit a420c54.
dapplion
reviewed
Oct 26, 2023
| const currentShufflingDecisionBlock = getShufflingDecisionBlock(state, currentEpoch); | ||
| const currentShufflingIn = opts?.shufflingGetter?.(currentEpoch, currentShufflingDecisionBlock); | ||
| const nextShufflingDecisionBlock = getShufflingDecisionBlock(state, nextEpoch); | ||
| const nextShufflingIn = opts?.shufflingGetter?.(nextEpoch, nextShufflingDecisionBlock); |
There was a problem hiding this comment.
What does the In suffix mean? Can you comment above this block detailing the rationale and purpose of these lines?
There was a problem hiding this comment.
those are shufflings comeing from ShufflingCache provided by BeaconChain. Will refactor to cachedNextShuffling and add more comments 👍
| seedValidator: CompositeViewDU<typeof ssz.phase0.Validator>, | ||
| newValidatorBytes: Uint8Array | ||
| ): CompositeViewDU<typeof ssz.phase0.Validator> { | ||
| if (isSameValidator(seedValidator, newValidatorBytes)) { |
There was a problem hiding this comment.
Extend this checks for the 3 cases: same pk, same withdrawals_creds, and same pk + same withdrawals_creds
| * ✔ byteArrayEquals 123687377 3.077884 ops/s 324.8985 ms/op - 1 runs 64.5 s | ||
| * ✔ Buffer.compare 123687377 114.7834 ops/s 8.712061 ms/op - 13 runs 12.1 s | ||
| */ | ||
| describe("compare Uint8Array", () => { |
There was a problem hiding this comment.
You should extend this comparision tests to various checks:
- compare equal byte arrays
- compare byte arrays different at all bytes (2 rand arrays)
- compare byte arrays different only on the last byte
| for (let i = 0; i < validatorCount; i++) { | ||
| const validator = validators[i]; | ||
|
|
||
| // Note: Not usable for fork-choice balances since in-active validators are not zero'ed | ||
| effectiveBalanceIncrements[i] = Math.floor(validator.effectiveBalance / EFFECTIVE_BALANCE_INCREMENT); | ||
|
|
||
| if (isActiveValidator(validator, previousEpoch)) { | ||
| if (previousShufflingIn == null && isActiveValidator(validator, previousEpoch)) { |
There was a problem hiding this comment.
Can you detail more the motivation of this change?
dapplion
approved these changes
Oct 30, 2023
|
@naviechan will coordinate with you on #6042 |
|
🎉 This PR is included in v1.12.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.
Motivation
Description
loadValidators()api which use the samevalidatorstree of seed state and return modified validators. This comes from the fact that validators are rarely changedinactivityScoresare rarely changed as monitored on mainnetloadCachedBeaconState()api which callloadState()and update modified validators to the pubkey cachepart of #5968
TODO: coordinate with #6042