Skip to content

Allow fallback to empty payload when engine_getPayload fails#3233

Closed
etan-status wants to merge 1 commit into
ethereum:devfrom
etan-status:vd-emptyblock
Closed

Allow fallback to empty payload when engine_getPayload fails#3233
etan-status wants to merge 1 commit into
ethereum:devfrom
etan-status:vd-emptyblock

Conversation

@etan-status

Copy link
Copy Markdown
Contributor

So far, it was possible for the CL produce an empty execution payload without any transactions, to handle an EL implementation that fails to produce new blocks (engine_getPayload keeps failing due to bug). An example can be found in Nimbus:
https://github.com/status-im/nimbus-eth2/blob/ba7c0bc091161f261148dbfcb27b21cc48f1fdf8/beacon_chain/spec/helpers.nim#L460

With Capella, that is no longer possible, as including the maximum amount of withdrawals is required. While the CL can compute and include those withdrawals, it cannot compute the matching EL state_root.

This means that with Capella, when there is a block production issue in a single popular EL implementation, that this no longer just slows down transaction confirmation times, but also disallows consensus to finalize (if more than 1/3rd of nodes use said buggy EL implementation).

To address this, this PR proposes allowing inclusion of 0 withdrawals at the cost of of forfeiting the entire EL block reward (0 transactions). Note that the ability to create an empty execution payload would have to be retained across future forks as well.

Alternatively, the engine API could be extended with a suggestion that on internal errors during engine_getPayload, a fallback block should be produced that solely includes withdrawals, instead of erroring.

At least, this issue should be consciously marked as irrelevant instead of being just an undocumented side effect of withdrawals.

So far, it was possible for the CL produce an empty execution payload
without any transactions, to handle an EL implementation that fails to
produce new blocks (`engine_getPayload` keeps failing due to bug).
An example can be found in Nimbus:
https://github.com/status-im/nimbus-eth2/blob/ba7c0bc091161f261148dbfcb27b21cc48f1fdf8/beacon_chain/spec/helpers.nim#L460

With Capella, that is no longer possible, as including the maximum
amount of withdrawals is required. While the CL can compute and include
those withdrawals, it cannot compute the matching EL `state_root`.

This means that with Capella, when there is a block production issue
in a single popular EL implementation, that this no longer just slows
down transaction confirmation times, but also disallows consensus to
finalize (if more than 1/3rd of nodes use said buggy EL implementation).

To address this, this PR proposes allowing inclusion of 0 withdrawals at
the cost of of forfeiting the entire EL block reward (0 transactions).
Note that the ability to create an empty execution payload would have to
be retained across future forks as well.

Alternatively, the engine API could be extended with a suggestion that
on internal errors during `engine_getPayload`, a fallback block should
be produced that solely includes withdrawals, instead of erroring.

At least, this issue should be consciously marked as irrelevant instead
of being just an undocumented side effect of withdrawals.
@etan-status etan-status marked this pull request as ready for review January 27, 2023 10:11
@etan-status

Copy link
Copy Markdown
Contributor Author

This topic was discussed during Capella interop, and this PR serves as a public record.

Nimbus is the only client that currently handles engine_getPayload failure (timeout/error) by producing a block with empty ExecutionPayload. So, if someone crafts a malicious transaction and pushes it into a mempool, and a popular EL client will consistently time out on any future requests to engine_getPayload, we already have the issue today that many nodes will skip their block proposals (except the Nimbus nodes).

@potuz suggested that even if ~2/3 of blocks are completely skipped skipped, that the chain can still collect enough attestations into the remaining blocks to finalize. This means that producing blocks with empty ExecutionPayload should not be needed.

@etan-status etan-status deleted the vd-emptyblock branch January 27, 2023 10:17
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.

1 participant