Fix NewPayload Validation during header download

[mininny]

This PR addresses an issue with the current implementation of the engine_newPayload request handling during the initial sync process.

Problem

The engine_newPayload request triggers validation of newly received payload, and during the initial sync process, Erigon downloads the block headers and blocks since more data is required for validation.

Inside this download logic is validating the chain data after it has finished downloading all the headers. When the node is initially downloading the headers, there is no validated blocks in the Erigon yet as it hasn't run the staged sync yet. Because of this, chain validation is skipped and mark the download process as complete.

This in turn marks the execution as synced and currently returns ValidStatus to the engine_newPayload request even though no validation has actually been done.

I think this behavior deviates from the engine API specification and incorrectly returns VALID even though Erigon doesn't have any synced blocks.

Proposed Change

This PR modifies the handleNewPayload function to additionally validate the chain. If this chain validation is failed(or skipped), it will return SYNCING as a result for the engine_newPayload. When the header download and staged sync is complete, only then will newPayload return VALID after actually validating the chain data.

---

This is my first time contributing to Erigon, and I'd appreciate any feedback for this kind of change. I'd also love to know if current logic is an effort for performance improvement or if there's other rationale for it.

[AskAlexSharov]

@Giulio2002 what do you think?

[Giulio2002]

Validation occurs directly in the EngineDownloader class, this PR does not add new behaviour

[mininny]

@Giulio2002 @AskAlexSharov Thanks for looking at this! I agree that the block validation occurs in EngineDownloader so usually this isn't needed.

But when the downloaded block range is too large (ExecutionStatus_TooFarAway), it just skips the block validation and return synced. This is problematic because the newPayload finishes downloading the block headers/bodies and returns VALID to the consensus layer when only the block download has been finished. As far as I understand, the execution client should return VALID when the block is executed and validated (which is done in validateChain)

This came up when I was working on https://github.com/testinprod-io/op-erigon which is a fork of erigon to work with the optimism protocol. Returning VALID incorrectly informs the consensus layer of optimism of finalized block in execution client, so having this merged will make execution layer return SYNCING when the staged sync is yet to run.

Another solution for this is to always return SYNCING after downloading block headers since the next invocation of newPayload will insert the block and return VALID

[Giulio2002]

Actually, you are right. apologies

[mininny]

Hi @AskAlexSharov @Giulio2002 , do you guys have any more comments on this? Would it be okay to merge this please? 😄

Also, i wonder if it would be possible to get this merged into release/v2.60 as well? I'm unsure if this would qualify as a critical bug, but having this released in the next release v2.60.2 would allow us to have this fixed in our fork as well :)

[Giulio2002]

Hey, auto-merge was enabled but your CI failed. let me rerun it

prohibits approved merge