Reject create block and attestation requests when chain head is optimistic #4706
Conversation
…istic It is not safe for validators to produce blocks on top of or attest to blocks that have only been optimistically synced. So if the referenced block is not fully validated, return a node syncing response rather than creating the block/attestation. Node syncing is used because the execution engine is still syncing and unable to verify the payload. Since blocks and attestations use the chain head which is normally not optimistic block production and attestation creation can continue while there are some optimistically sync'd blocks, as long as we still have a fully validated block close enough to use as our chain head and build on. If we've optimistically finalized, we will cease block and attestation production.
…ad block and has fun corner cases around genesis
| LOG.warn( | ||
| "Unable to produce block at slot {} because parent has optimistically validated payload", | ||
| slot); |
There was a problem hiding this comment.
are we expecting this to be rare? will warning be annoying?
There was a problem hiding this comment.
Yes that should be rare and I think it's worth knowing if it happens.
| @@ -427,6 +436,7 @@ public void onExecutionPayloadResult( | |||
| protoArray.markNodeValid(blockRoot); | |||
| break; | |||
| case INVALID: | |||
| LOG.warn("Payload for block root {} was invalid", blockRoot); | |||
There was a problem hiding this comment.
mean to leave this in? invalid isn't super uncommon i thought...
There was a problem hiding this comment.
Invalid signatures and stuff like that aren't uncommon, but invalid payload are. Geth prints a very big, scary multiline warning if it gets a block that is invalid when executed (as opposed to invalid PoW). I'm a little unsure but I think its worth leaving this in at least until we get some experience on merge testnets to see how annoying it is as I expect it will be very useful during interop testing.
|
Excellent questions to ask btw. |
PR Description
It is not safe for validators to produce blocks on top of or attest to blocks that have only been optimistically synced. So if the referenced block is not fully validated, return a node syncing response rather than creating the block/attestation. Node syncing is used because the execution engine is still syncing and unable to verify the payload.
Since blocks and attestations use the chain head which is normally not optimistic block production and attestation creation can continue while there are some optimistically sync'd blocks, as long as we still have a fully validated block close enough to use as our chain head and build on. If we've optimistically finalized, we will cease block and attestation production.
We don't prevent creating aggregates because the validator client will only create aggregates for attestations they produced, so if they managed to produce the attestation it must have been validated (though potentially by a different beacon node).
Fixed Issue(s)
#4650 - need to handle sync committee messages separately as they don't have a create call.
Documentation
documentationlabel to this PR if updates are required.Changelog