feat: new attestations calculation algorithm #270
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
Implement a prototype of the new algorithm for attestation calculation to support the new attestation data format introduced in the EIP-7549 that will be added to the next Pectra hardfork.
As attestations for slots in all epochs except the current epoch and the previous epoch are already handled during previous app cycles, we now exclude these attestations from processing. So, the logic of attestation processing is kept the same as it was before. Also, update the `@lodestar/types` lib to support Pectra hardfork types.
Add backward compatibility of the attestation calculation algorithm with pre-Pectra forks. So, the app should work normally on the chain before Pectra and after Pectra without any changes in app code or in configs. Now information about fork epochs is fetched from the CL node instead of setting it in the env variables or hard-coding these epochs for known chains.
|
UPD Add backward compatibility of the attestation calculation algorithm with pre-Pectra forks. So, the app should work normally on the chain before Pectra and after Pectra without any changes in app code or in configs. Now information about fork epochs is fetched from the CL node instead of setting it in the env variables or hard-coding these epochs for known chains. |
vgorkavenko
reviewed
Feb 6, 2025
Comment on lines
93
to
94
| dencun: spec.DENEB_FORK_EPOCH != null ? parseInt(spec.DENEB_FORK_EPOCH, 10) : Number.MAX_SAFE_INTEGER, | ||
| pectra: spec.ELECTRA_FORK_EPOCH != null ? parseInt(spec.ELECTRA_FORK_EPOCH, 10) : Number.MAX_SAFE_INTEGER, |
There was a problem hiding this comment.
would be better to have deneb and electra to be consistent with response here
There was a problem hiding this comment.
Yep, I agree. I've renamed forks in all application symbols.
|
|
||
| const forkEpochs = await this.clClient.getForkEpochs(); | ||
| const isDencunFork = epoch >= forkEpochs.dencun; | ||
| const isPectraFork = epoch >= forkEpochs.pectra; |
There was a problem hiding this comment.
I think it should be smth like this:
const attestedSlotInEpoch = attestation.includedInBlock // slotsPerEpoch;
const isPectraBlock = attestedSlotInEpoch >= forkEpochs.pectra;
Also isDencunFork should be re-reviewed. Here might be a bug
There was a problem hiding this comment.
You're correct, that's a good finding, thank you. I've fixed this in the code.
As the `AttestationService` fetches attestation for the epoch that is processed on the current EVM cycle and for the epoch that was processed on the previous cycle, an epoch in the `this.processedEpoch` does not correctly reflect the epoch of the attestation that is currently processed. So, the logic of how fork is determined worked incorrectly for some attestations. Now this bug is fixed and the fork is now determined using the slot of each particular attestation.
Rename "dencun" to "deneb" and "pectra" to "electra" in all symbols.
Move common code in the attestation calculation logic to a separate `processAttestationCommittee` method.
Round down an epoch number in the attestation processing algorithm.
vgorkavenko
approved these changes
Feb 10, 2025
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.
Implement a prototype of the new algorithm for attestation calculation to support the new attestation data format introduced in the EIP-7549 that will be added to the next Pectra hardfork.
Some useful links that illustrate new data format and calculation rules
Open questions:
What to do if the
committeein theprocessAttestationmethod isnull? In the previous version of this algorithm, it just skipped that attestation and went ahead. But in the new version of the algorithm, if a committee doesn't exist in thecommitteesdata structure if the algorithm skips this committee, it will mean that all attestations aggregated by this committee will be skipped.