Skip to content

engine: return -38003 for FCUv2 payloadAttributes mismatch#10014

Merged
macfarla merged 4 commits into
besu-eth:mainfrom
Muzry:fix-fcuv2-return-38003
Apr 2, 2026
Merged

engine: return -38003 for FCUv2 payloadAttributes mismatch#10014
macfarla merged 4 commits into
besu-eth:mainfrom
Muzry:fix-fcuv2-return-38003

Conversation

@Muzry
Copy link
Copy Markdown
Contributor

@Muzry Muzry commented Mar 10, 2026

This PR updates engine_forkchoiceUpdatedV2 to return -38003: Invalid payload attributes when the wrong payloadAttributes version is used.

In particular, FCUv2 payload-attribute version mismatches such as:

  • missing withdrawals at or after Shanghai
  • unexpected withdrawals before Shanghai

should be treated as Invalid payload attributes, not Invalid params.

Why

This change aligns the client with the latest Engine API spec update in:

It also follows the implementation discussion and prior client-side change in:

The spec was clarified so that FCUv2 now behaves consistently with newer forkchoiceUpdated versions for payloadAttributes structure/version
mismatches.

What changed

  • Updated FCUv2 payload attributes validation to return -38003 for payloadAttributes version mismatches.
  • Added/updated regression coverage for the affected FCUv2 cases.

Hive impact

This fixes the Hive engine-withdrawals failure caused by returning the wrong error code for FCUv2 payloadAttributes mismatches.

Relevant Hive failure:

After this change, the client returns the expected error code for the affected FCUv2 cases.

If my understanding or interpretation of the spec change is incorrect, please let me know and I can adjust the implementation accordingly.

@macfarla macfarla self-assigned this Mar 11, 2026
@macfarla macfarla added the hive relating to hive tests label Mar 11, 2026
@macfarla
Copy link
Copy Markdown
Contributor

@Muzry you'll need to add signoff to your commits to satisfy the DCO check. I need to check what's going on with hive tests as I'm not seeing any engine-withdrawal failures when running on main locally. what specific hive test is your PR addressing?

@macfarla macfarla assigned Muzry and unassigned macfarla Mar 18, 2026
@macfarla macfarla marked this pull request as draft March 18, 2026 03:48
@Muzry Muzry force-pushed the fix-fcuv2-return-38003 branch 2 times, most recently from 5202660 to 5eac4f5 Compare March 20, 2026 02:05
@Muzry
Copy link
Copy Markdown
Contributor Author

Muzry commented Mar 20, 2026

@macfarla Thanks, I’ve added signoff to my commit and re-ran the full engine-withdrawals suite locally against hive main using the Besu client from hive.

Hive commit used for the run: 1fc72ff41ba0903b46c4e210ead80f55a63e1718

commands: ./hive --sim ethereum/engine --sim.limit 'engine-withdrawals/' --client besu --sim.parallelism 50 --docker.output --docker.pull

The run produced 34 test cases: 20 passed and 14 failed.

The main failure cluster is exactly what this PR addresses: for engine_forkchoiceUpdatedV2, hive expects -38003, while the Besu version currently used by hive still returns -32602.

So this PR is addressing the engine-withdrawals FCUv2 error-code failures, rather than a withdrawal execution failure in engine_newPayload.

You can also check: https://hive.ethpandaops.io/#/test/generic/1773131670-9be21c1a6d01e061c51d963e43511cc1. The recent test runs there are also failing.

@macfarla macfarla assigned macfarla and unassigned Muzry Mar 20, 2026
@Muzry Muzry marked this pull request as ready for review March 24, 2026 07:41
@macfarla
Copy link
Copy Markdown
Contributor

running hive tests locally,

  • 10 engine-withdrawals failures running on main branch
  • failures look like: FAIL (Withdrawals Fork On Genesis): Expected error code on EngineForkchoiceUpdatedV2: want=-38003, got=-32602
  • 0 engine-withdrawals failures running on this PR
Screenshot 2026-03-25 at 12 58 46 pm

@macfarla
Copy link
Copy Markdown
Contributor

macfarla commented Mar 25, 2026

@Muzry this does fix a bunch of hive tests which is great. There are some unit tests (maybe only 1) that need updating with this change as well - if you can run the unit tests locally that will make the feedback loop faster - eg

> Task :ethereum:api:test

EngineForkchoiceUpdatedV1Test > shouldReturnInvalidIfPayloadTimestampNotGreaterThanHead() FAILED
    org.opentest4j.AssertionFailedError at AbstractEngineForkchoiceUpdatedTest.java:788

@macfarla macfarla assigned Muzry and unassigned macfarla Mar 25, 2026
@macfarla macfarla marked this pull request as draft March 25, 2026 03:49
@Muzry Muzry force-pushed the fix-fcuv2-return-38003 branch from 32f84a2 to 26088ed Compare March 30, 2026 06:39
@Muzry
Copy link
Copy Markdown
Contributor Author

Muzry commented Mar 30, 2026

@macfarla Thanks, sorry for the delay. I've updated the unit tests for this change. Could you take another look?

@macfarla
Copy link
Copy Markdown
Contributor

@macfarla Thanks, sorry for the delay. I've updated the unit tests for this change. Could you take another look?

@Muzry can you amend your latest commits to include signoff - to fix the DCO check

@Muzry Muzry force-pushed the fix-fcuv2-return-38003 branch from 26088ed to 74601e5 Compare March 30, 2026 07:18
@Muzry
Copy link
Copy Markdown
Contributor Author

Muzry commented Mar 30, 2026

@macfarla I’ve updated the commit to include signoff. Could you please take another look?

@macfarla macfarla assigned macfarla and unassigned Muzry Mar 31, 2026
Copy link
Copy Markdown
Contributor

@macfarla macfarla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@macfarla macfarla marked this pull request as ready for review March 31, 2026 00:29
Copilot AI review requested due to automatic review settings March 31, 2026 00:29
@macfarla macfarla assigned Muzry and unassigned macfarla Mar 31, 2026
@macfarla macfarla marked this pull request as draft March 31, 2026 22:07
@macfarla
Copy link
Copy Markdown
Contributor

@Muzry there's an acceptance test failing as well https://github.com/besu-eth/besu/actions/runs/23820570721/job/69433410658?pr=10014 - you can run these tests locally, it will massively speed up the feedback loop for getting your PR in.

@Muzry Muzry force-pushed the fix-fcuv2-return-38003 branch 3 times, most recently from 6bc9ecd to 480f287 Compare April 1, 2026 07:24
@Muzry
Copy link
Copy Markdown
Contributor Author

Muzry commented Apr 1, 2026

@macfarla I updated the Paris acceptance test fixture for engine_forkchoiceUpdatedV1 to match the current V1 behavior (-32602 / Invalid withdrawals) and re-ran the acceptance test locally. It is now passing. Could you please take another look?

@macfarla macfarla marked this pull request as ready for review April 1, 2026 19:07
@macfarla
Copy link
Copy Markdown
Contributor

macfarla commented Apr 1, 2026

@macfarla I updated the Paris acceptance test fixture for engine_forkchoiceUpdatedV1 to match the current V1 behavior (-32602 / Invalid withdrawals) and re-ran the acceptance test locally. It is now passing. Could you please take another look?

@macfarla I updated the Paris acceptance test fixture for engine_forkchoiceUpdatedV1 to match the current V1 behavior (-32602 / Invalid withdrawals) and re-ran the acceptance test locally. It is now passing. Could you please take another look?

ExecutionEngineShanghaiAcceptanceTest is failing

@macfarla macfarla marked this pull request as draft April 1, 2026 19:20
@macfarla macfarla marked this pull request as draft April 1, 2026 19:20
@macfarla
Copy link
Copy Markdown
Contributor

macfarla commented Apr 1, 2026

@Muzry run this locally, it takes ~ 20 min, then you can be sure all the tests are passing
./gradlew build

Muzry added 3 commits April 2, 2026 21:53
Signed-off-by: muzry <muzrry@gmail.com>
Signed-off-by: muzry <muzrry@gmail.com>
Signed-off-by: muzry <muzrry@gmail.com>
@Muzry Muzry force-pushed the fix-fcuv2-return-38003 branch from bc72ee0 to 14c4027 Compare April 2, 2026 13:53
Signed-off-by: muzry <muzrry@gmail.com>
@Muzry Muzry force-pushed the fix-fcuv2-return-38003 branch from 14c4027 to 8281b41 Compare April 2, 2026 13:54
@Muzry
Copy link
Copy Markdown
Contributor Author

Muzry commented Apr 2, 2026

@macfarla I updated the related unit tests and acceptance fixtures for this change, and re-ran the relevant tests locally. The FCU V1/V2 unit tests are passing, and the Paris / Shanghai acceptance cases related to this change are also now passing locally. Could you please take another look?

@macfarla macfarla marked this pull request as ready for review April 2, 2026 20:05
@macfarla macfarla merged commit 1df6b70 into besu-eth:main Apr 2, 2026
46 of 48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hive relating to hive tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants