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.

Sign up for GitHub

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

Jump to bottom

Update attestation APIs to support EIP-7549 #448

Merged
merged 20 commits into from
Jul 4, 2024
Merged

Update attestation APIs to support EIP-7549 #448

merged 20 commits into from
Jul 4, 2024

Conversation

mehdi-aouadi
Copy link
Contributor

Following the Electra containers updates:

  • Update the /block and /pool attestation GET APIs to handle the new Electra attestations
  • Add Electra.Attestation schema
  • Extract the AttestationData schema used by both Attestation and Electra.Attestation
  • Update the attester slashing API
  • Update the IndexedAttestation schema

Partially fixes #445
Related to #447

nflaig
nflaig reviewed May 24, 2024
View reviewed changes
apis/beacon/blocks/attestations.yaml Outdated Show resolved Hide resolved
apis/beacon/blocks/attestations.yaml Outdated Show resolved Hide resolved
apis/beacon/blocks/attestations.yaml Outdated Show resolved Hide resolved
types/attestation.yaml Outdated Show resolved Hide resolved
types/electra/attestation.yaml Show resolved Hide resolved
@nflaig
Copy link
Collaborator

nflaig commented May 27, 2024

What about attestation event, should it be part of this PR?

beacon-APIs/apis/eventstream/index.yaml

Lines 64 to 68 in fb718b6

attestation:
description: The node has received an Attestation (from P2P or API) that passes validation rules of the `beacon_attestation_{subnet_id}` topic
value: |
event: attestation
data: {"aggregation_bits":"0x01", "signature":"0x1b66ac1fb663c9bc59509846d6ec05345bd908eda73e670af888da41af171505cc411d61252fb6cb3fa0017b679f8bb2305b26a285fa2737f175668d0dff91cc1b66ac1fb663c9bc59509846d6ec05345bd908eda73e670af888da41af171505", "data":{"slot":"1", "index":"1", "beacon_block_root":"0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", "source":{"epoch":"1", "root":"0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2"}, "target":{"epoch":"1", "root":"0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2"}}}

@mehdi-aouadi
Copy link
Contributor Author

What about attestation event, should it be part of this PR?

beacon-APIs/apis/eventstream/index.yaml

Lines 64 to 68 in fb718b6

attestation:
description: The node has received an Attestation (from P2P or API) that passes validation rules of the `beacon_attestation_{subnet_id}` topic
value: |
event: attestation
data: {"aggregation_bits":"0x01", "signature":"0x1b66ac1fb663c9bc59509846d6ec05345bd908eda73e670af888da41af171505cc411d61252fb6cb3fa0017b679f8bb2305b26a285fa2737f175668d0dff91cc1b66ac1fb663c9bc59509846d6ec05345bd908eda73e670af888da41af171505", "data":{"slot":"1", "index":"1", "beacon_block_root":"0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", "source":{"epoch":"1", "root":"0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2"}, "target":{"epoch":"1", "root":"0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2"}}}

I considered the event stream events out of the scope of this PR. Might be addressed in a separate PR imo

@mehdi-aouadi mehdi-aouadi requested a review from nflaig May 28, 2024 12:59
nflaig
nflaig reviewed May 28, 2024
View reviewed changes
Copy link
Collaborator

@nflaig nflaig left a comment

Choose a reason for hiding this comment

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

Haven't reviewed everything in detail but general approach looks good to me, let's get some feedback from others, especially in regard to having to bump everything to v2 or not

apis/beacon/blocks/attestations.v2.yaml Outdated Show resolved Hide resolved
apis/beacon/pool/attestations.yaml Outdated Show resolved Hide resolved
apis/beacon/pool/attester_slashings_v2.yaml Outdated Show resolved Hide resolved
types/electra/attestation.yaml Outdated Show resolved Hide resolved
types/electra/attestation.yaml Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
@rolfyone
Copy link
Collaborator

Yep I agree with new version for interfaces given they're a new required field (breaking) and headers.
minor comment about references to /dev versions.

rkapka
rkapka reviewed Jun 5, 2024
View reviewed changes
apis/beacon/pool/attestations.v2.yaml Outdated Show resolved Hide resolved
apis/beacon/pool/attestations.v2.yaml Outdated Show resolved Hide resolved
apis/beacon/pool/attestations.v2.yaml Outdated Show resolved Hide resolved
apis/beacon/pool/attestations.v2.yaml Outdated Show resolved Hide resolved
apis/beacon/pool/attestations.v2.yaml Outdated Show resolved Hide resolved
apis/beacon/pool/attester_slashings.v2.yaml Outdated Show resolved Hide resolved
apis/beacon/pool/attester_slashings.v2.yaml Outdated Show resolved Hide resolved
apis/beacon/pool/attestations.v2.yaml Outdated Show resolved Hide resolved
apis/beacon/pool/attester_slashings.v2.yaml Outdated Show resolved Hide resolved
types/electra/attestation.yaml Show resolved Hide resolved
@nflaig nflaig mentioned this pull request Jun 18, 2024
Open
8 tasks
nflaig
nflaig reviewed Jun 27, 2024
View reviewed changes
Copy link
Collaborator

@nflaig nflaig left a comment

Choose a reason for hiding this comment

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

need to resolve conflicts since we merged #447

types/electra/attestation.yaml Outdated Show resolved Hide resolved
types/electra/attestation.yaml Show resolved Hide resolved
apis/beacon/blocks/attestations.v2.yaml Outdated Show resolved Hide resolved
apis/beacon/pool/attestations.v2.yaml Outdated Show resolved Hide resolved
types/attestation_data.yaml Outdated Show resolved Hide resolved
types/electra/attestation.yaml Show resolved Hide resolved
@mehdi-aouadi mehdi-aouadi requested a review from rolfyone June 27, 2024 17:32
nflaig
nflaig reviewed Jun 27, 2024
View reviewed changes
beacon-node-oapi.yaml Show resolved Hide resolved
types/electra/attestation.yaml Outdated Show resolved Hide resolved
@mehdi-aouadi mehdi-aouadi requested a review from nflaig June 28, 2024 09:00
nflaig
nflaig previously approved these changes Jun 28, 2024
View reviewed changes
@realbigsean realbigsean mentioned this pull request Jul 1, 2024
Merged
7 tasks
This was referenced Jul 3, 2024
Closed
Closed
Closed
@nflaig nflaig merged commit 41cc574 into ethereum:master Jul 4, 2024
3 checks passed
@mehdi-aouadi mehdi-aouadi deleted the eip-7549 branch July 4, 2024 11:24
This was referenced Jul 5, 2024
Closed
Closed
This was referenced Aug 10, 2024
Merged
Merged
Closed
Merged
@pedromiguelmiranda pedromiguelmiranda mentioned this pull request Sep 17, 2024
Merged
This was referenced Dec 12, 2024
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Emmanuel0899 Emmanuel0899 Emmanuel0899 approved these changes

@nflaig nflaig nflaig approved these changes

@dapplion dapplion dapplion approved these changes

@rkapka rkapka rkapka approved these changes

@rolfyone rolfyone Awaiting requested review from rolfyone

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update attestation related APIs to be compatible with EIP-7549
6 participants
@mehdi-aouadi @nflaig @rolfyone @rkapka @dapplion @Emmanuel0899