search

paradigmxyz / reth

Modular, contributor-friendly and blazing-fast implementation of the Ethereum protocol, in Rust
https://reth.rs/
Apache License 2.0
3.97k stars 1.19k forks source link

hive: fcu fields not validated #6217

Open rkrasiuk opened 9 months ago

rkrasiuk commented 9 months ago

Description

The chain formed by forkchoice update fields (finalized -> safe -> head) is not validated by the consensus engine and it erroneously responds with FCU status Syncing

Hive

At the time of writing the test covering this behavior is named Inconsistent Head in ForkchoiceState (Paris) under engine-api test suite.

Test breakdown

The simulation sends following Engine API requests to the client:

FCU head: 0xd462b6793c2895fd61cd63e098874774d3e03556e14ec40fb9861956e39eb8b0 - Valid

New Payload: 0xab4eb0d9f2a392ccf1edf43ffd0c9d6a197d1215a94f7294d032e962812a175b (parent: 0xd462b6793c2895fd61cd63e098874774d3e03556e14ec40fb9861956e39eb8b0) - Valid

New Payload: 0x2e656d340bf43036bf48c6fd966497414ee79d8cf6c3bc86c15c47ed907e51f7 (parent: 0xd462b6793c2895fd61cd63e098874774d3e03556e14ec40fb9861956e39eb8b0) - Valid

FCU head: 0x2e656d340bf43036bf48c6fd966497414ee79d8cf6c3bc86c15c47ed907e51f7 - Valid

FCU head: 0x2e656d340bf43036bf48c6fd966497414ee79d8cf6c3bc86c15c47ed907e51f7 - Valid

New Payload: 0xb307f97d770d75d8f61e996162a063cbe3ed3c6a9b8b2392e237c3364225a585 (parent: 0xab4eb0d9f2a392ccf1edf43ffd0c9d6a197d1215a94f7294d032e962812a175b) - Valid

New Payload: 0x444680403ae5c7e1d3eb67d2e7d77bdf6492bdf0f108cca77f7d9097dfd97652 (parent: 0x2e656d340bf43036bf48c6fd966497414ee79d8cf6c3bc86c15c47ed907e51f7) - Valid

FCU head: 0x444680403ae5c7e1d3eb67d2e7d77bdf6492bdf0f108cca77f7d9097dfd97652 safe: 0x2e656d340bf43036bf48c6fd966497414ee79d8cf6c3bc86c15c47ed907e51f7 - Valid

FCU head: 0x444680403ae5c7e1d3eb67d2e7d77bdf6492bdf0f108cca77f7d9097dfd97652 safe: 0x2e656d340bf43036bf48c6fd966497414ee79d8cf6c3bc86c15c47ed907e51f7 - Valid

New Payload: 0xcd8924ad6d3e6075f7548297a35a46a85e384b3520ed1abdf1c168cae381ac71 (parent: 0xb307f97d770d75d8f61e996162a063cbe3ed3c6a9b8b2392e237c3364225a585) - Valid

New Payload: 0xd6d53c45ebebd35fe4abaf030eeaec52e830a328e0cc0d15b93a9fe687f408de (parent: 0x444680403ae5c7e1d3eb67d2e7d77bdf6492bdf0f108cca77f7d9097dfd97652) - Valid

FCU head: 0xd6d53c45ebebd35fe4abaf030eeaec52e830a328e0cc0d15b93a9fe687f408de safe: 0x444680403ae5c7e1d3eb67d2e7d77bdf6492bdf0f108cca77f7d9097dfd97652 finalized: 0x2e656d340bf43036bf48c6fd966497414ee79d8cf6c3bc86c15c47ed907e51f7 - Valid

FCU head: 0xcd8924ad6d3e6075f7548297a35a46a85e384b3520ed1abdf1c168cae381ac71 safe: 0x444680403ae5c7e1d3eb67d2e7d77bdf6492bdf0f108cca77f7d9097dfd97652 finalized: 0x2e656d340bf43036bf48c6fd966497414ee79d8cf6c3bc86c15c47ed907e51f7 - Syncing (wrong)

After analyzing the chains formed by these blocks, we can see that head 0xcd89..ac71 can never extend chain formed by 0x4446..7652 and 0x2e65..51f7.

Solution

After validating the existence of the head block in the blockchain tree, we should validate that it forms a valid chain by extending safe and finalized blocks.

Rjected commented 9 months ago

To add some more context here, we do perform a similar check here

https://github.com/paradigmxyz/reth/blob/720815a83465a4c79c0066e466ad53d005cc32c1/crates/consensus/beacon/src/engine/mod.rs#L827-L868

This happens after make_canonical and the check is only performed if make_canonical returns a VALID response. Given that cd89 and all its ancestors should exist in the blockchain tree, SYNCING is definitely the wrong response here

github-actions[bot] commented 9 months ago

This issue is stale because it has been open for 21 days with no activity.