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

Fix failing execution tests with latest geth version #6937

Open
jimmygchen opened this issue Feb 7, 2025 · 1 comment
Open

Fix failing execution tests with latest geth version #6937

jimmygchen opened this issue Feb 7, 2025 · 1 comment
Labels
infra-ci test improvement Improve tests v7.0.0 New release c. Q1 2025

Comments

@jimmygchen
Copy link
Member

We made a workaround to use geth v1.10.10 instead of master to get CI to pass for the time being:

#6936

Our tests need to be updated to work with latest geth, due to these breaking changes:

  • removal of personal namespace in v1.14.12: See #30704
  • removal of totalDifficulty field from RPC in v1.14.11. See #30386.
@michaelsproul
Copy link
Member

michaelsproul commented Feb 7, 2025

I think the removal of total difficulty was also causing some of our readiness loggers to error. From interop channel on ETH R&D:

{"msg":"Unable to check genesis execution payload","level":"ERRO","ts":"2025-02-03T17:07:31.659060494Z","service":"slot_notifier","error":"ExecutionLayerGetBlockByNumberFailed(EngineError(Api { error: Json(Error("missing field totalDifficulty", line: 0, column: 0)) }))"}

It's cosmetic, but would be nice to fix (only relevant for devnets)

mergify bot pushed a commit that referenced this issue Mar 5, 2025
This change makes the `total_difficulty` field in `ExecutionBlock` an `Option<Uint256>` since newer clients are no longer including the `totalDifficulty` field.

I think this will fix #6937 but I was actually more focused on the builder registration case described below.

In our [builder-playground](https://github.com/flashbots/builder-playground) we setup a local devnet using lighthouse, reth, and mev-boost-relay.  After upgrading to reth 1.2.0 and lighthouse v7.0.0.beta.0 for Pectra, we noticed that the validator registration process was _sometimes_ failing with:

```
Feb 25 23:35:25.038 ERRO Unable to publish proposer preparation to all beacon nodes, error: Some endpoints failed, num_failed: 1 http://localhost:3500/ => RequestFailed(ServerMessage(ErrorMessage { code: 400, message: "BAD_REQUEST: error updating proposer preparations: ForkchoiceUpdate(EngineError(Api { error: Json(Error(\"missing field `totalDifficulty`\", line: 0, column: 0)) }))", stacktraces: [] })), service: preparation
Feb 25 23:35:25.099 WARN Unable to publish validator registrations to the builder network, error: Some endpoints failed, num_failed: 1 http://localhost:3500/ => RequestFailed(ServerMessage(ErrorMessage { code: 400, message: "BAD_REQUEST: error updating proposer preparations: ForkchoiceUpdate(EngineError(Api { error: Json(Error(\"missing field `totalDifficulty`\", line: 0, column: 0)) }))", stacktraces: [] })), service: preparation
```

What was even more confusing, was that it was sometimes working, which actually led to a wild goose chase thinking it was a networking issue.  However, when tracing through the LH code, I came across this comment:

https://github.com/sigp/lighthouse/blob/70194dfc6a3f4d10c9059610f889ff5a4e863a6a/beacon_node/beacon_chain/src/beacon_chain.rs#L6048-L6049

This explained why it sometimes worked, in our playground we run lighthouse with `--prepare-payload-lookahead 8000` thus there was always a 4-second window where the call wasn't made.

But, if the call was made, then this code would 100% fail with updated reth:

https://github.com/sigp/lighthouse/blob/unstable/beacon_node/execution_layer/src/lib.rs#L1688-L1692

Which would then mapped to a `Error::ForkchoiceUpdate` in `update_execution_engine_forkchoice`.


  Anyways, the fix was to make `total_difficulty` Optional, and then to update any code paths where it was used.  In doing so, I assume that if the EL doesn't include total difficulty then the chain is already post-merge.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra-ci test improvement Improve tests v7.0.0 New release c. Q1 2025
Projects
None yet
Development

No branches or pull requests

2 participants