hyperledger / besu

An enterprise-grade Java-based, Apache 2.0 licensed Ethereum client https://wiki.hyperledger.org/display/besu
https://www.hyperledger.org/projects/besu
Apache License 2.0
1.52k stars 840 forks source link

Remove backward sync functionality from engine_newPayload. #5687

Open siladu opened 1 year ago

siladu commented 1 year ago

Only trigger backward sync on engine_forkchoiceUpdated.

Fixes #5411

This is a significant improvement for at least besu-nimbus pairs, off the back off https://github.com/hyperledger/besu/issues/5411#issuecomment-1620817201 and has been tested successfully with Teku and Nimbus, see POC https://github.com/hyperledger/besu/pull/5666

Main thing to work out is if we should return SYNCING, ACCEPTED or something else in this case.


Spec allows it:

https://github.com/ethereum/execution-apis/blob/main/src/engine/paris.md#specification (newPayload)

Client software MAY initiate a sync process if requisite data for payload validation is missing. Sync process is specified in the Sync section.


Geth impl: https://github.com/ethereum/go-ethereum/blob/59f7b289c329b5a56fa6f4e9acee64e504c4cc0d/eth/catalyst/api.go#L476-L485

Note, geth also stash away these newPayloads for later use. That would be a further optimism but is not critical for this issue.

non-fungible-nelson commented 1 year ago

@siladu - can we status this, #5622, and #5411?

Do we think #5699 solves these?

non-fungible-nelson commented 1 year ago

bump for status