Proper EL hashes in from_syncing_to_invalid test

[sauliusgrigaitis]

As discussed with @hwwhww on Discord there seems to be a problem related to PR #3126. The new test uses the same block_hash for different branches, particularly 0x376f20f22f880c3be5a8c360f8ff86941e6aa5fac3b5de7825d2efee3f70e891, 0x2e0f73c5b1489715b161e0c11af557c8ac758c3b99aed6744bbbbe73687beb38, 0x6c59e0eb4b2de1a922c639c12f6a0528c672f69d388bff4f7dbbad261ab12bee. This should not happen under normal behavior. The previous test version used different block_hashes. We found this by updating Grandine, but other clients may not be affected as they may be ignoring block_hash (at least Prysm according to @potuz).

[hwwhww]

Thanks @sauliusgrigaitis!

The case is: for our test target, we need different block hashes for the different fork chains. we used to hardcode the block hashes with some distinct values. However, the hardcode values were removed in #3126 in the latest release.

Two solutions:

1. Revert the change of this test case of proper EL block hash implementation in tests #3126
2. Manually update block.body.execution_payload.extra_data

I will try (2) tomorrow.

/cc @djrtwo, this is a should-fix of the next spec release.

[djrtwo]

This should not happen under normal behavior

It's not expected but it is valid. Two blocks with different CL contents (e.g. different attestations) could be created for the same slot (thus same EL timestamp), same parent, and same TX/etc EL contents

So this is valid behavior for the underlying EL chain after the merge, and should be tested explicitly. Maybe it shouldn't show up in a bunch of places accidentally but it should be in our test suite

[potuz]

Same hash may happen with two different CL contents but in this case I think the execution validity would be the same, not different as in these tests

[sauliusgrigaitis]

Yes, I meant that under normal conditions same payload should not get it's status changed from VALID to SYNCING.

[sauliusgrigaitis]

BTW, the test now as it is doesn't make much sense for me. The previous version was very clear - it had a chain with #33-#36 blocks with VALID payloads and then it re-orgs to an alternative heavier branch with alternative #34-#35 blocks with SYNCING payloads ending with an INVALID #36 block that invalidated this branch and the chain re-orged back to the lighter VALID branch.

However, now I really don't understand what this new test tries to test. The new test doesn't expect the same re-org as the previous version.

[djrtwo]

Got it, I misunderstood that these had different validity responses

[sauliusgrigaitis]

Looks like the new version solved this issue, thanks!