[Merged by Bors] - Downgrade Geth to v1.10.20 in EE integration tests

[paulhauner]

Issue Addressed

NA

Proposed Changes

The execution integration tests have started failing since Geth updated to v1.10.21. More details here: ethereum/go-ethereum#25427 (comment)

This PR pins our version at v1.10.20.

Additional Info

NA

[paulhauner]

After reconsidering this, I'm not sure this is the correct behaviour from Geth. I've raised an issue here: ethereum/go-ethereum#25427

[paulhauner]

It turns out there was more to this issue than I originally thought. See ethereum/go-ethereum#25427 (comment).

I'm going to try the tests on Geth v1.10.20 now.

[michaelsproul]

Looks good, shall we batch with #3380 and maybe #3370?

[paulhauner]

bors r+

[karalabe]

Hey all, the new behavior is correct (though it should have returned SYNCING), just changed from the previous incorrect one.

Geth by default starts in snap sync mode. In snap sync, the node must finish a sync cycle first before it accepts blocks. The reason is that snap sync goes around all block validation pathways and writes stuff directly to the database, so it cannot allow some process to inject something conflicting out of band. I.e. if Geth is asked to snap sync, it will deny importing a payload on the engine API until snap sync finishes.

You should start geth with --syncmode=full if you want to test feeding Geth blocks from #1 onward via the engine API.

[bors]

Build failed (retrying...):

• execution-engine-integration-ubuntu

[karalabe]

Thinking about it a bit more, I think ACCEPTED is the correct response (just the warning message is wrong).

The reasoning is that if I have the parent block, but not it's state (e.g. the beacon client gives me an old block because it's resyncing), then the expected response is ACCEPTED; because the block seems right and we store it for potentially doing a fork choice update, but we don't act upon the payload.

Another scenario for ACCEPTED is if I'm currently syncing (full sync via the beacon client) and the beacon client gives me a side chain payload (missing parent). In that case I will save the payload and ACCEPT it, but I will not act upon it until the FCU.

I the same vein, getting a payload whilst in snap sync mode should behave kind of similarly: ACCEPT the payload and enable FCU-ing to it, but not really importing it since the chain is not "ready" for it.

[michaelsproul]

will merge this for now just to unblock CI

bors r-

[bors]

Canceled.

[michaelsproul]

bors r+

[bors]

Build failed (retrying...):

• beacon-chain-tests

[bors]

This PR was included in a batch that was canceled, it will be automatically retried

[paulhauner]

bors r-

[bors]

Canceled.

[paulhauner]

bors r+

[bors]

Build failed (retrying...):

• release-tests-ubuntu

[bors]

Pull request successfully merged into unstable.

Build succeeded:

• arbitrary-check
• beacon-chain-tests
• cargo-audit
• cargo-fmt
• cargo-udeps
• cargo-vendor
• check-benchmarks
• check-consensus
• check-msrv
• clippy
• debug-tests-ubuntu
• dockerfile-ubuntu
• doppelganger-protection-test
• ef-tests-ubuntu
• eth1-simulator-ubuntu
• execution-engine-integration-ubuntu
• merge-transition-ubuntu
• no-eth1-simulator-ubuntu
• op-pool-tests
• release-tests-ubuntu
• release-tests-windows
• state-transition-vectors-ubuntu

[paulhauner]

Thinking about it a bit more, I think ACCEPTED is the correct response (just the warning message is wrong).

This also seems reasonable to me. The semantics in the engine API are a little odd, it could be argued that a CL issuing a fcU to block A and then EL responding with a request that declares that block A is not on the canonical chain is incorrect. However, it could also be argued that a CL should expect a no-op when SYNCING is returned to fcU. I think the latter is most realistic, therefore Geth is behaving well and I was incorrect in my original determination.