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

Update attestation APIs to support EIP-7549 #448

Merged
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

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?

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?

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
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.

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
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
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update attestation related APIs to be compatible with EIP-7549
6 participants