Skip to content

fix: use UintBn64 for gloas ExecutionPayloadBid.executionPayment - #9749

Merged
nflaig merged 1 commit into
ChainSafe:unstablefrom
lodekeeper:fix/gloas-bid-execution-payment-uintbn64
Aug 3, 2026
Merged

fix: use UintBn64 for gloas ExecutionPayloadBid.executionPayment#9749
nflaig merged 1 commit into
ChainSafe:unstablefrom
lodekeeper:fix/gloas-bid-execution-payment-uintbn64

Conversation

@lodekeeper

Copy link
Copy Markdown
Contributor

ExecutionPayloadBid.executionPayment is a uint64 with no bound in process_execution_payload_bid. Back it with UintBn64 (exact) instead of UintNum64 (float64, lossy above 2**53) so the bid hashTreeRoot stays exact across the full uint64 range. Gloas-only, no mainnet impact.

executionPayment is an unbounded uint64 in process_execution_payload_bid,
so a block can carry any value; UintBn64 keeps the bid hashTreeRoot exact
for values above 2**53 (matching exact-uint64 clients).

🤖 Generated with AI assistance

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@lodekeeper
lodekeeper requested a review from a team as a code owner August 2, 2026 20:58
@nflaig nflaig changed the title fix(types): use UintBn64 for gloas ExecutionPayloadBid.executionPayment fix: use UintBn64 for gloas ExecutionPayloadBid.executionPayment Aug 2, 2026

@nflaig nflaig 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.

this seems valid to me, that's why adding useless fields like execution_payment to the spec containers is bad, at least found this before any public testnet/mainnet, the reason why this is problematic is because we would produce a different HTR if such a bid was in a canonical block, causing a consensus split, not sure why we use number here, this field can be arbitrarily controlled by a proposer (or builder if bid is accepted)

@markolazic01

Copy link
Copy Markdown
Contributor

looks good, I checked that changes properly cover all places where executionPayment appears.

@lodekeeper

Copy link
Copy Markdown
Contributor Author

Thanks @markolazic01 for double-checking the executionPayment usages 🙏

@nflaig — agreed on all counts. The number (UintNum64) here was almost certainly a reflexive mirror of how we type the other uint64 payload fields — ExecutionPayload.gasLimit/gasUsed/blockNumber/timestamp are all UintNum64 too. That's fine for those: they're bounded well below 2^53 by validity rules every client enforces, so no client ever hashes an out-of-range value. The bid is the exception — executionPayment is proposer/builder-supplied on a signed container with no such bound, so its hash_tree_root is taken over whatever value they put in. UintNum64 silently truncates anything ≥ 2^53, so Lodestar would compute a different HTR than clients using exact uint64 → a canonical block carrying such a bid → block-root disagreement → consensus split. Same class as the Eth1Data.depositCount fix (#9747): proposer-controlled uint64 + no validity bound + float64 decode. UintBn64 keeps the exact bytes through SSZ/HTR. And agreed — much better to catch it pre-testnet/mainnet.

@nflaig nflaig 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.

must be done, this field can be controlled by the proposer or builder, gonna merge this

@nflaig
nflaig merged commit 5aeaa12 into ChainSafe:unstable Aug 3, 2026
20 checks passed
markolazic01 pushed a commit to markolazic01/lodestar that referenced this pull request Aug 3, 2026
…inSafe#9749)

`ExecutionPayloadBid.executionPayment` is a `uint64` with no bound in
`process_execution_payload_bid`. Back it with `UintBn64` (exact) instead
of `UintNum64` (float64, lossy above 2**53) so the bid hashTreeRoot
stays exact across the full uint64 range. Gloas-only, no mainnet impact.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants