Conversation
|
cc @nalepae re ExecutionProofStatus |
| `MAX_EXECUTION_PROOFS_PER_PAYLOAD` proofs, the `count` field MUST satisfy | ||
| `count * MAX_EXECUTION_PROOFS_PER_PAYLOAD <= compute_max_request_execution_proofs()`. |
There was a problem hiding this comment.
Not a request for change, just an observation: This allows count == 0, which seems consistent with DataColumnSidecarsByRange
| This protocol enables peers to exchange their current execution proof | ||
| verification status. The request and response use the same type. | ||
|
|
There was a problem hiding this comment.
Perhaps we write a short note on why this allows us to not need Metadata?
There was a problem hiding this comment.
Do you mean adding context to explain why we don't include it in the Metadata rpc? Makes sense to me. If we add this, we can also mention that for the mandatory proofs hardfork, we will deprecate this type and integrate the inner data into Metadata.
Will wait for guidance from @jtraglia if this note aligns with spec standards.
There was a problem hiding this comment.
Do you mean adding context to explain why we don't include it in the Metadata rpc?
yep exactly
Will wait for guidance from @jtraglia if this note aligns with spec standards.
sounds good ser
|
The |
Yes, it was intentional. The idea is that What do you think about this? Does it sound reasonable? |
| Requests the MetaData of a peer, using the new `MetaData` definition given above | ||
| that is extended from Altair. Other conditions for the `GetMetaData` protocol | ||
| are unchanged from the Altair p2p networking document. | ||
| This protocol enables peers to exchange their current execution proof |
There was a problem hiding this comment.
In case a node does not have EIP8025 enabled, MUST the node not register this protocol and thus not respond on this request?
Basically, what replaces the current GetMetaData v4 execution_proof_aware field to denote a node is not running optional-proofs.
There was a problem hiding this comment.
Yep exactly -- so if a node doesn't register this protocol, it means they don't support 8025
There was a problem hiding this comment.
The baseline assumption is that a peer does not support this protocol. If the peer signals support via "eproof" key in their ENR then this implies that they do support the protocol. Only the peer initiating the connection has visibility of the other peers' ENR. This creates an information asymmetry: the peer initiating the connection knows the peer supports eproof, but the peer receiving the inbound connection request does not, because it cannot see the peer's ENR. To address this asymmetry, the node initiating the connection MUST invoke the request upon connection if it also supports execution proofs.
There was a problem hiding this comment.
The baseline assumption is that a peer does not support this protocol. If the peer signals support via "eproof" key in their ENR then this implies that they do support the protocol.
Yes, that makes sense. But should we not explicitly add that somewhere in the specs?
To address this asymmetry, the node initiating the connection MUST invoke the request upon connection if it also supports execution proofs.
So should we add something like this or similar:
"The dialing client MUST send a ExecutionProofStatus request upon connection when optional execution proof–aware."
There was a problem hiding this comment.
Good point, yes, I think that's a good suggestion I will add it.
Description
MetadataandGetMetaData. We deprecate these modifications to prevent potential future conflicts with the core protocol.ExecutionProofStatusrpc, which can be used to communicate the latest block and slot that the peer has received and verified sufficient execution proofs for to be considered valid. This is used for peer selection in the sync protocol.ExecutionProofsByRangerpc, which can be used to sync a contiguous range of proofs from a peer. Often useful when initially connecting to a network / filling large gaps.