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

Optimistic sync changes for HTTP API #2945

Closed
paulhauner opened this issue Jan 21, 2022 · 1 comment
Closed

Optimistic sync changes for HTTP API #2945

paulhauner opened this issue Jan 21, 2022 · 1 comment
Labels
bellatrix Required to support the Bellatrix Upgrade

Comments

@paulhauner
Copy link
Member

Description

This issue is a WIP, please don't start work on it yet. I'm still ironing out the details 🙏

As a part of implementing optimistic sync (presently defined in this spec PR), we must ensure that the BN HTTP API doesn't present information about optimistic blocks as canonical. We also need to ensure that we don't create blocks/attestations/sync messages for our head when it's optimistic.

There are a few things we need to consider to make our API optimistic-safe. I'll try to list them all here.

Details

The {block_id} and {state_id} tags

When we get a request using a {block_id} or {state_id}, we must consider optimistic sync. I will enumerate all the types and how they should behave.

  • head: return the latest_verified_ancestor.
  • genesis: always return the genesis block (it's never optimistic).
  • finalized: if the finalized checkpoint is optimistic, return a 503 syncing.
  • justified: if the justified checkpoint is optimistic, return a 503 syncing.
  • <slot>: return a 503 syncing if the block is optimistic.
  • <root>: return a 503 syncing if the block is optimistic.

Note, it might be onerous to determine if some ancient <slot> or <root> blocks are optimistic or not. I'd say that if their prior to the finalized checkpoint (i.e., no longer in fork choice) then just use the state of the finalized checkpoint. I.e., if the finalized checkpoint is optimistic, assume all prior block are as well.

The */beacon/pool/* endpoints

For the GET requests, one could argue that we should filter out all results that reference an optimistic block, but I don't think that's necessary.

For the POST requests, only the */pool/attestations route is relative to optimistic sync. I think we should still publish attestations to optimistic heads. If a validator has signed the message, it must have authority from some other BN that the block's payload is valid.

So, no changes for the pool endpoints.

GET /eth/v1/beacon/headers

If no query parameters are provided, use the latest_verified_ancestor. If parameters are provided, return a 503 syncing if the matching block is optimistic.

POST /eth/v1/beacon/blocks

No need to apply restrictions here, either. If a validator POSTs a block here and it references an optimistic parent and can be imported optimistically, I think we should import it and return a 200 OK as usual.

If a VC has managed to produce a block, we assume it must have done it via a node which as verified the parent payload (it's impossible to produce a payload on an unknown parent payload, anyway).

/eth1/v2/debug/beacon/heads

TODO: I'm not sure about this one.

/eth/v1/events

TODO: I'm not sure if we should emit head, reorg or block events for optimistic blocks.

The */validator/* endpoints

The following endpoints require no changes:

  • GET /eth/v1/validator/duties/*
  • POST /eth/v1/validator/duties/*
  • POST /eth/v1/validator/aggregate_and_proofs
  • POST /eth/v1/validator/beacon_committee_subscriptions
  • POST /eth/v1/validator/sync_committee_contributions
  • POST /eth/v1/validator/contribution_and_proofs

/eth/*/validator/blocks/{slot}

This endpoint should never allow building a block atop an optimistic parent. It's impossible anyway, and EL can't build a payload upon an unknown parent.

We don't actually need to do anything here, apart from ensure that a block can't be optimistically produced. This should be natural since the engine_getPayloadV1 call will fail and we won't have an ExecutionPayload to put in the block.

GET /eth/*/validator/blocks/attestation_data

This endpoint will need to be modified to ensure that we never return at attestation to an optimistic head. Doing so might cause the chain to finalize invalid data 🙀

I think we should go about this by producing an attestation, then checking to see if the attestation.data.beacon_block_root is optimistic. If so, return an error.

GET /eth/*/validator/blocks/aggregate_attestation

I think we should filter this response like the attestation_data call. If it turns out that the beacon_block_root is optimistic, return an error.

/eth/*/validator/blocks/sync_committee_contribution

Just like attestation_data, filter out any responses where the beacon_block_root is optimistic.

@paulhauner paulhauner added the bellatrix Required to support the Bellatrix Upgrade label Jan 21, 2022
@paulhauner
Copy link
Member Author

Deprecated in favor or #2946

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bellatrix Required to support the Bellatrix Upgrade
Projects
None yet
Development

No branches or pull requests

1 participant