Invalid latestValidHash caused Lodestar to halt on invalid block

[siladu]

See https://discord.com/channels/593655374469660673/593655641445367808/1200195786672185438

From @g11tech

Ok from logs I understand the issue: lodestar tries to download all the forks, and some peers which accepted the 19056922, hash: 0x5970f4b49ecfba must be serving the same, which lodestar would send to besu for the validation, and as expected besu would reply with invalid, but here is the problem:

Jan 21 13:14:17 nuc13 besu[59010]: 2024-01-21 13:14:17.083-05:00 | vert.x-worker-thread-0 | WARN | AbstractEngineNewPayload | Invalid new payload: number: 19056922, hash: 0x5970f4b49ecfbad6b02a1cc8fad8a0e47382576b1b28eeb4ec2a8c1649fa6c90, parentHash: 0x932123bf49f6ffce68aac29820bda6028d3bf7aebbebd5fdc758dac9d1c81c46, latestValidHash: 0x0000000000000000000000000000000000000000000000000000000000000000, status: INVALID, validationError: Block already present in bad block manager.

besu is not supposed to send: latestValidHash: 0x0000000000000000000000000000000000000000000000000000000000000000, it can send it null, this response (0x0000000000000000000000000000000000000000000000000000000000000000) if only reserved if the valid block in besu's forkchoice is PREMERGE, which obviously is incorrect at this point of time.

This causes us to invalidate the all post merge forkchoice, although we can add check for the same and prevent this issue on our side, but this is still essentially besu bug

here are the specs: https://github.com/ethereum/execution-apis/blob/main/src/engine/paris.md#response

wemeetagain — 28/01/2024 02:22
ok, and for besu, they really should send a real LVH if feasible, if they don't want to do that, send null instead of 0x0.
g11tech — 28/01/2024 02:24
yes, correct
wemeetagain — 28/01/2024 02:24
I mean, sending a real LVH is definitely possible, afaik other ELs sent a real LVH in this scenario, so i'm assuming its just an oversight.
g11tech — 28/01/2024 02:25
yes, these are unfinalized blocks and its this invalid block's parent would still be in their non pruned tree

[siladu]

Verify corresponding Hive tests are failing, this could be a regression.

[g11tech]

we(lodestar) should now be able to deal with it with this PR: ChainSafe/lodestar#6361, but yea could be nice to have this addressed on besu too ❤️