Skip to content
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

*: electra versioned attestations #3473

Merged
merged 8 commits into from
Jan 22, 2025

Conversation

KaloyanTanev
Copy link
Collaborator

First PR with Electra updates from the go-eth2-client's electra branch. It includes:

  • Refactoring the Attestation object to VersionedAttestation. Similarly to the proposals and blocks, attestations are versioned now, as there are some changes in their structure for Electra. Most functions take as arguments versioned instead of regular attestations now. The same logic for unfolding a versioned attestations is applied as it is for the blocks.
  • Adding a new BlockAttestationsV2 endpoint, as per the new Beacon API for Electra. Note that this endpoint is one of the few that is not supported by go-eth2-client, so we include it in our wrapper and implement it ourselves.
  • Add electra as a version in most switch/case statements for already existing versioned structures (blocks, signed blocks, etc.). The list is not exhaustive though and I haven't checked yet in full if electra is included everywhere. More on that in later PRs, I have focused only on making the current test cases pass.
  • Changes of golden files was required, as the fuzzer that we use for random generation of a structure is generating a different random now, as some structures are different (i.e.: the VersionedBlock structure now also has electra in it). This results in different results for the test, hence the different golden files for testing.
  • Skipping Gnosis fixes and tests. As we are using our own forked version of go-eth2-client for Gnosis, it seemed unfeasible to me to update our fork every time go-eth2-client push new commit to their electra branch and we adapt the new changes. That said, I've commented out the gnosis tests and the go.mod replacement.

category: feature
ticket: none

@KaloyanTanev KaloyanTanev force-pushed the kalo/pectra-versioned-attestations branch from 83bea7b to 3a4bbcb Compare January 20, 2025 09:28
@github-actions github-actions bot added the electra PR targeting electra upgrade label Jan 20, 2025
@KaloyanTanev KaloyanTanev force-pushed the kalo/pectra-versioned-attestations branch from 3a4bbcb to a23ddb6 Compare January 20, 2025 09:28
@KaloyanTanev KaloyanTanev marked this pull request as draft January 20, 2025 09:40
Copy link

codecov bot commented Jan 20, 2025

Codecov Report

Attention: Patch coverage is 36.17607% with 464 lines in your changes missing coverage. Please review.

Please upload report for BASE (electra@8a7a42e). Learn more about missing BASE report.

Files with missing lines Patch % Lines
core/signeddata.go 28.97% 139 Missing and 13 partials ⚠️
core/tracker/inclusion.go 19.33% 108 Missing and 13 partials ⚠️
app/eth2wrap/httpwrap.go 22.01% 77 Missing and 8 partials ⚠️
core/ssz.go 64.42% 39 Missing and 14 partials ⚠️
core/unsigneddata.go 26.92% 15 Missing and 4 partials ⚠️
core/validatorapi/validatorapi.go 44.00% 10 Missing and 4 partials ⚠️
app/eth2wrap/eth2wrap_gen.go 0.00% 5 Missing ⚠️
eth2util/types.go 0.00% 4 Missing ⚠️
app/eth2wrap/lazy.go 50.00% 2 Missing and 1 partial ⚠️
app/eth2wrap/multi.go 86.95% 3 Missing ⚠️
... and 2 more
Additional details and impacted files
@@            Coverage Diff             @@
##             electra    #3473   +/-   ##
==========================================
  Coverage           ?   56.45%           
==========================================
  Files              ?      218           
  Lines              ?    33088           
  Branches           ?        0           
==========================================
  Hits               ?    18680           
  Misses             ?    12465           
  Partials           ?     1943           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@KaloyanTanev KaloyanTanev marked this pull request as ready for review January 21, 2025 09:35
@KaloyanTanev
Copy link
Collaborator Author

The integration tests are failing because of yet unimplemented v2 endpoints, however, this PR grew beyond what is normal amount of work to be done in one PR, so I'd put those new v2 endpoints in a separate one...

@KaloyanTanev KaloyanTanev requested review from gsora, pinebit and DiogoSantoss and removed request for gsora January 21, 2025 09:36
Copy link
Collaborator

@pinebit pinebit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quickly skimmed, did not find any red flags. Implementation looks very accurate.

Copy link
Collaborator

@pinebit pinebit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quickly skimmed, did not find any red flags. Implementation looks very accurate.

@KaloyanTanev KaloyanTanev added the merge when ready Indicates bulldozer bot may merge when all checks pass label Jan 22, 2025
@KaloyanTanev KaloyanTanev merged commit eab13c1 into electra Jan 22, 2025
10 of 11 checks passed
@KaloyanTanev KaloyanTanev deleted the kalo/pectra-versioned-attestations branch January 22, 2025 13:01
KaloyanTanev added a commit that referenced this pull request Jan 28, 2025
* Comment out gnosis fix

* SubmitAttestation to versioned attestation

* Rework attestations, add BlockAttestationsV2 endpoint, update signed blocks with electra version

* go mod tidy

* Add more random attestation util functions

* Fix duty attester test signatures

* Fix raw attestation json tag

* Fix error messages
KaloyanTanev added a commit that referenced this pull request Jan 31, 2025
* Comment out gnosis fix

* SubmitAttestation to versioned attestation

* Rework attestations, add BlockAttestationsV2 endpoint, update signed blocks with electra version

* go mod tidy

* Add more random attestation util functions

* Fix duty attester test signatures

* Fix raw attestation json tag

* Fix error messages
KaloyanTanev added a commit that referenced this pull request Feb 5, 2025
* Comment out gnosis fix

* SubmitAttestation to versioned attestation

* Rework attestations, add BlockAttestationsV2 endpoint, update signed blocks with electra version

* go mod tidy

* Add more random attestation util functions

* Fix duty attester test signatures

* Fix raw attestation json tag

* Fix error messages
KaloyanTanev added a commit that referenced this pull request Feb 7, 2025
* Comment out gnosis fix

* SubmitAttestation to versioned attestation

* Rework attestations, add BlockAttestationsV2 endpoint, update signed blocks with electra version

* go mod tidy

* Add more random attestation util functions

* Fix duty attester test signatures

* Fix raw attestation json tag

* Fix error messages
KaloyanTanev added a commit that referenced this pull request Feb 10, 2025
* Comment out gnosis fix

* SubmitAttestation to versioned attestation

* Rework attestations, add BlockAttestationsV2 endpoint, update signed blocks with electra version

* go mod tidy

* Add more random attestation util functions

* Fix duty attester test signatures

* Fix raw attestation json tag

* Fix error messages
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
electra PR targeting electra upgrade merge when ready Indicates bulldozer bot may merge when all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants