Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ethereum/sync hive failure: reth does not set FCU blocks in RPC after successful pipeline run #8579

Closed
Rjected opened this issue Jun 3, 2024 · 2 comments · Fixed by #12006
Labels
A-consensus Related to the consensus engine A-rpc Related to the RPC implementation C-bug An unexpected or incorrect behavior C-test A change that impacts how or what we test M-prevent-stale Prevents old inactive issues/PRs from being closed due to inactivity

Comments

@Rjected
Copy link
Member

Rjected commented Jun 3, 2024

After applying this fix in hive:
ethereum/hive#1112

We still fail the ethereum/sync test because we do not set the latest block after a successful pipeline run.

For example, the test sends a newPayload, then FCU which triggers sync using the pipeline:

2024-06-03T22:16:33.074258Z TRACE rpc::engine: Serving engine_newPayloadV2
2024-06-03T22:16:33.074924Z TRACE rpc::engine: Serving engine_forkchoiceUpdatedV2
2024-06-03T22:16:33.075108Z  INFO reth_node_events::node: Live sync in progress, downloading blocks remaining_blocks=1 target_block_hash=0x3303c6e3577579843c427f795d7485abdddc55694b5a37af0b32b48259ebe103
2024-06-03T22:16:33.075141Z  INFO reth_node_events::node: Received forkchoice updated message when syncing head_block_hash=0x07f7d3c5744ffddc5441cf31074c91a934afbf6ddb7e8234a081ee1f21140455 safe_block_hash=0x07f7d3c5744ffddc5441cf31074c91a934afbf6ddb7e8234a081ee1f21140455 finalized_block_hash=0x07f7d3c5744ffddc5441cf31074c91a934afbf6ddb7e8234a081ee1f21140455

It then checks if the node has finished syncing by calling eth_getHeaderByNumber and the latest tag:

2024-06-03T22:16:48.100023Z  INFO sync::stages::index_history: Writing indices progress=99.71%
2024-06-03T22:16:48.100112Z  INFO reth_node_events::node: Finished stage pipeline_stages=11/12 stage=IndexAccountHistory checkpoint=2000 target=2000
2024-06-03T22:16:48.101517Z  INFO reth_node_events::node: Preparing stage pipeline_stages=12/12 stage=Finish checkpoint=0 target=2000
2024-06-03T22:16:48.101532Z  INFO reth_node_events::node: Executing stage pipeline_stages=12/12 stage=Finish checkpoint=0 target=2000
2024-06-03T22:16:48.101589Z  INFO reth_node_events::node: Finished stage pipeline_stages=12/12 stage=Finish checkpoint=2000 target=2000
2024-06-03T22:16:48.147681Z TRACE rpc::eth: Serving eth_getBlockByNumber number=Latest full=false
2024-06-03T22:16:49.153164Z TRACE rpc::eth: Serving eth_getBlockByNumber number=Latest full=false
2024-06-03T22:16:50.159387Z TRACE rpc::eth: Serving eth_getBlockByNumber number=Latest full=false
2024-06-03T22:16:51.166554Z TRACE rpc::eth: Serving eth_getBlockByNumber number=Latest full=false

We return zero because we still have unset ChainInfo after the pipeline run completes.

@Rjected Rjected added C-bug An unexpected or incorrect behavior A-rpc Related to the RPC implementation A-consensus Related to the consensus engine C-test A change that impacts how or what we test labels Jun 3, 2024
@Rjected
Copy link
Member Author

Rjected commented Jun 3, 2024

thinking this could be solved by fcu buffering, for example, while the fcu does kick off a pipeline run, we do "skip" the head / safe parts of the FCU in this case. So we should buffer every unique forkchoice state which we respond SYNCING to. This would then be solved because the FCU would be buffered, applied later, and the latest block would be properly set.

Copy link
Contributor

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

@github-actions github-actions bot added the S-stale This issue/PR is stale and will close with no further activity label Jun 25, 2024
@emhane emhane added M-prevent-stale Prevents old inactive issues/PRs from being closed due to inactivity and removed S-stale This issue/PR is stale and will close with no further activity labels Jun 25, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in Reth Tracker Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Related to the consensus engine A-rpc Related to the RPC implementation C-bug An unexpected or incorrect behavior C-test A change that impacts how or what we test M-prevent-stale Prevents old inactive issues/PRs from being closed due to inactivity
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants