Skip to content

Propagate optimistic head errors - #10296

Open
kediyalua wants to merge 1 commit into
sigp:unstablefrom
kediyalua:propagate-optimistic-head-errors
Open

kediyalua wants to merge 1 commit into
sigp:unstablefrom
kediyalua:propagate-optimistic-head-errors

Conversation

@kediyalua

Copy link
Copy Markdown

Issue Addressed

Closes #10295

Also relates to #3822, which introduced the endpoint with this behaviour.

Proposed Changes

  • POST /eth/v1/beacon/rewards/attestations/{epoch} no longer discards errors from
    BeaconChain::is_optimistic_or_invalid_head. The handler now propagates them with
    map_err(warp_utils::reject::unhandled_error)?, so a head whose execution status cannot be
    determined produces a server error instead of a response asserting
    execution_optimistic: false.
let execution_optimistic = chain
    .is_optimistic_or_invalid_head()
    .map_err(warp_utils::reject::unhandled_error)?;
  • This makes the handler consistent with every other call site of
    is_optimistic_or_invalid_head in beacon_node/http_api (state_id.rs, block_id.rs,
    proposer_duties.rs, sync_committees.rs, and the eth/v1/node/syncing-style handler in
    lib.rs). The attestation rewards handler was the only one using unwrap_or_default().

Additional Info

  • Behaviour is unchanged when is_optimistic_or_invalid_head() returns Ok(_): the endpoint still
    returns HTTP 200 with the same execution_optimistic value. The existing integration test
    test_beacon_attestation_rewards_fulu, which asserts execution_optimistic == Some(false), is
    unaffected.
  • The error path is not covered by a test. It requires the fork choice race described in the
    is_optimistic_or_invalid_head doc comment (head block root pruned while syncing), which the
    test harness cannot easily force. I did not add a test rather than add one that does not exercise
    the path.
  • No changes to the Beacon API schema or to any other endpoint.
  • Verified locally: cargo check -p http_api --locked passes, and
    rustfmt --edition 2024 --check beacon_node/http_api/src/lib.rs reports no diff.
  • Disclosure: this change was prepared with AI assistance; I reviewed the diff and checked it
    against the surrounding call sites.

Signed-off-by: kediyalua <kediyalua@outlook.com>
@kediyalua
kediyalua changed the base branch from stable to unstable October 10, 2026 13:55
@cla-assistant

cla-assistant Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@cla-assistant

cla-assistant Bot commented Oct 10, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@michaelsproul michaelsproul left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

sure

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. HTTP-API low-hanging-fruit Easy to resolve, get it before someone else does! labels Oct 11, 2026

This branch has not been deployed

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

Labels

HTTP-API low-hanging-fruit Easy to resolve, get it before someone else does! ready-for-merge This PR is ready to merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

attestation rewards API reports execution_optimistic: false when the head status is unknown

2 participants