Skip to content

fix(evmexec): classify syncing payload preparation correctly#1886

Open
peter941221 wants to merge 2 commits into
alpenlabs:mainfrom
peter941221:fix/evmexec-prepare-payload-syncing
Open

fix(evmexec): classify syncing payload preparation correctly#1886
peter941221 wants to merge 2 commits into
alpenlabs:mainfrom
peter941221:fix/evmexec-prepare-payload-syncing

Conversation

@peter941221
Copy link
Copy Markdown

@peter941221 peter941221 commented May 30, 2026

Description

Fixes payload preparation error classification in build_block_from_mempool.

Before this change, the function called forkchoiceUpdated with payload attributes and then immediately required payload_id. That treated a legal SYNCING response as payload_id missing, even though payload building has not started yet in that state.

This patch checks payload_status.status before reading payload_id, keeps the existing VALID + payload_id path, and returns explicit errors for SYNCING, INVALID, and unexpected ACCEPTED.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature/Enhancement (non-breaking change which adds functionality or enhances an existing one)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactor
  • New or updated tests
  • Dependency Update

Notes to Reviewers

The happy path is unchanged.

The important change is that the function now classifies the full response surface explicitly instead of assuming every success response must carry a payload id. The current branch tests VALID, SYNCING, INVALID, unexpected ACCEPTED, and VALID without a payload id.

AI was used to assist in this PR.

Is this PR addressing any specification, design doc or external reference document?

  • Yes
  • No

If yes, please add relevant links:

Checklist

  • I have performed a self-review of my code.
  • I have commented my code where necessary.
  • I have updated the documentation if needed.
  • My changes do not introduce new warnings.
  • I have added (where necessary) tests that prove my changes are effective or that my feature works.
  • New and existing tests pass with my changes.
  • I have disclosed my use of AI in the body of this PR.

Related Issues

@peter941221
Copy link
Copy Markdown
Author

peter941221 commented May 30, 2026

  1. Follow-up hardening

I kept the production fix unchanged and tightened the status-boundary tests around build_block_from_mempool.

The branch now covers:

  • VALID with a payload id
  • SYNCING
  • INVALID { validation_error }
  • unexpected ACCEPTED
  • VALID without a payload id

That closes the main review gap in this PR: the status classification is no longer justified by one branch and one example. The tests now pin the whole response surface that this function handles.

  1. Validation

Linux-side validation passed in a temporary WSL verification copy:

cargo fmt --all --check
cargo test -p strata-evmexec test_build_block_from_mempool -- --nocapture
cargo test -p strata-evmexec -- --nocapture

strata-evmexec result: 11 passed, 0 failed.

  1. Branch update

Pushed follow-up commit:

29f44e63 fix(evmexec): harden payload preparation status tests

  1. AI disclosure

I used AI assistance for code investigation, patch drafting, PR text, and test targeting. I reviewed the final patch and validation results before posting.

@peter941221 peter941221 marked this pull request as ready for review May 30, 2026 08:56
@peter941221
Copy link
Copy Markdown
Author

peter941221 commented Jun 1, 2026

This branch is still at the hardened test boundary from 29f44e6.

It now covers the full status surface handled by build_block_from_mempool, so I think the review unit is in good shape for another look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant