Beacon api + validator electra

[realbigsean]

Issue Addressed

• implements this block production change https://github.com/ethereum/consensus-specs/blob/dev/specs/electra/validator.md#deposits

Actual endpoint changes

• GET /eth/v2/validator/aggregate_attestation - this now requires committee_index as a query param

Just the types change

We may need to filter for the consensus_version_header in requests, add it to responses, and wrap responses in ForkVersionedData. I don't think any of these are required to a LH BN<>VC connection so I don't think these need to be implemented until they are spec'd.

• GET /eth/v1/beacon/pool/attestations
• POST /eth/v1/beacon/pool/attestations
• GET /eth/v1/beacon/pool/attester_slashings
• POST /eth/v1/beacon/pool/attester_slashings/
• GET /eth/v1/beacon/blocks/{block_id}/attestations
• POST /eth/v1/validator/aggregate_and_proofs

[dapplion]

What if someone queries the pool at the fork boundary? There may be attestations from both forks

[realbigsean]

that's a great point.. looks like we'll have to convert to the new format for all of these attestations

Seems to mean Pawan's idea is the best way to approach it: #5744 (comment)

[dapplion]

Not a reliable way to get the attester slashing fork. One can have an attester slashing electra with an attestation data from deneb. Instead we should implement fork_name_unchecked in slashing

[pawanjay176]

Why aren't we always calculating the fork based on slot_clock.now() here?

[realbigsean]

Good catch

Why aren't we always calculating the fork based on slot_clock.now() here?

I think this makes sense considering return types could be mixed for a given Eth-Consensus-Version as pointed out here #5744 (comment)

[realbigsean]

raised a spec issue as well ethereum/beacon-APIs#458

[dapplion]

should match?

[realbigsean]

V2 is just a struct wrapper around a u64 not an enum

pub const V2: EndpointVersion = EndpointVersion(2);

#[derive(Debug, Clone, Copy, PartialEq)]
pub struct EndpointVersion(pub u64);

[pawanjay176]

Why aren't we always calculating the fork based on slot_clock.now() here?

[pawanjay176]

Can we not have a Vec<Attestation> with both variants?

[realbigsean]

I think with these updates no: 386aacd

This traits for when JSON wraps a data structure with a fork name like this:

{
  "version": "phase0",
  "execution_optimistic": false,
  "finalized": false,
  "data": [
    {
      "aggregation_bits": "0x01",
      "data": {
        "slot": "1",
        "index": "1",
        "beacon_block_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2",
        "source": {
          "epoch": "1",
          "root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2"
        },
        "target": {
          "epoch": "1",
          "root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2"
        }
      },
      "signature": "0x1b66ac1fb663c9bc59509846d6ec05345bd908eda73e670af888da41af171505cc411d61252fb6cb3fa0017b679f8bb2305b26a285fa2737f175668d0dff91cc1b66ac1fb663c9bc59509846d6ec05345bd908eda73e670af888da41af171505"
    }
  ]
}

The fork name's meant to be used to deserialize the struct, so i think a consistent fork through the list is implied. More specifically, this is used for when we are displaying attestations from a specific block, which should be uniform, or from the op pool, which should be uniform with the changes listed

[pawanjay176]

I didn't get why we are doing it this way for attester slashings. Why not just post the json with the content header?

[realbigsean]

i sort of just went with what was there already. I actually think this was just included for testing this PR and should have been reverted #4575

[realbigsean]

updated 71a2ead

[realbigsean]

Thanks for the reviews! Updates have been made

[realbigsean]

In the issue I made on the spec repo, sounds like other teams just drop messages from the op pool that are from the old fork. How do we feel about going that route instead? Might be a little more straightforward for an end user because those message won't actually make it into a block.

[realbigsean]

@mergify queue

[mergify]

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at f290c68