Skip to content

fix(kona): ensure chain_id is always set for trie hints in interop provider#20164

Merged
ajsutton merged 2 commits intodevelopfrom
aj/fix/interop-provider-chain-id
Apr 22, 2026
Merged

fix(kona): ensure chain_id is always set for trie hints in interop provider#20164
ajsutton merged 2 commits intodevelopfrom
aj/fix/interop-provider-chain-id

Conversation

@ajsutton
Copy link
Copy Markdown
Contributor

@ajsutton ajsutton commented Apr 20, 2026

Summary

Fixes a bug where OracleInteropProvider could emit TrieHinter hints without a chain_id when header_by_hash() was called before header_by_number(). Then refactors to eliminate the hidden mutable state so the bug class cannot recur.

Context

Cantina audit finding.

Fixes ethereum-optimism/optimism-private#502

Originally opened privately as ethereum-optimism/optimism-private#523 (fix) and ethereum-optimism/optimism-private#526 (refactor); moved to the public repo as a combined PR on top of #20163.

@ajsutton ajsutton requested a review from a team as a code owner April 20, 2026 00:51
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 20, 2026

Codecov Report

❌ Patch coverage is 66.66667% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.8%. Comparing base (8e0c70e) to head (8de77c3).
⚠️ Report is 18 commits behind head on develop.

Files with missing lines Patch % Lines
...st/kona/crates/proof/proof-interop/src/provider.rs 66.6% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #20164      +/-   ##
===========================================
  Coverage     76.7%    76.8%              
===========================================
  Files          691      508     -183     
  Lines        76359    65817   -10542     
===========================================
- Hits         58584    50559    -8025     
+ Misses       17631    15258    -2373     
+ Partials       144        0     -144     
Flag Coverage Δ
cannon-go-tests-64 ?
contracts-bedrock-tests ?
unit 76.8% <66.6%> (-0.1%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...st/kona/crates/proof/proof-interop/src/provider.rs 66.1% <66.6%> (+0.3%) ⬆️

... and 191 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ajsutton ajsutton force-pushed the aj/fix/interop-provider-chain-id branch from 820ffdc to b8df94c Compare April 20, 2026 02:49
@ajsutton ajsutton changed the base branch from develop to aj/fix/l2-account-proof-hint-hash April 20, 2026 02:50
@ajsutton ajsutton force-pushed the aj/fix/l2-account-proof-hint-hash branch from e9f12b0 to a84314d Compare April 20, 2026 20:15
@ajsutton ajsutton force-pushed the aj/fix/interop-provider-chain-id branch from b8df94c to ad2f78a Compare April 20, 2026 20:16
Base automatically changed from aj/fix/l2-account-proof-hint-hash to develop April 21, 2026 03:35
ajsutton and others added 2 commits April 21, 2026 16:57
If header_by_hash() is called before header_by_number(), the chain_id
field is never set, causing all subsequent TrieHinter calls to emit
hints without a chain_id. Extract a set_chain_id() method (matching the
non-interop provider pattern) and call it from both header_by_hash()
and header_by_number().

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the `Arc<RwLock<Option<u64>>>` chain_id field on OracleInteropProvider
with an explicit ChainScopedHinter wrapper. This eliminates hidden mutable state
and makes the chain ID binding explicit at each call site.

The ChainScopedHinter borrows the oracle and holds a fixed chain_id, implementing
TrieHinter with that chain_id always set. The provider's scoped_hinter() method
creates instances as needed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ajsutton ajsutton force-pushed the aj/fix/interop-provider-chain-id branch from ad2f78a to 8de77c3 Compare April 21, 2026 07:10
Copy link
Copy Markdown
Contributor

@digorithm digorithm left a comment

Choose a reason for hiding this comment

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

Very clean! Is this worth adding a regression test (since it was a Cantina finding)? I'm ok either way given that the scope is small and now this is strictly enforced by the type system.

@ajsutton
Copy link
Copy Markdown
Contributor Author

It always worked and we have tests that effectively guarantee it work because otherwise interop can't supply the required preimages. So I don't think we need more testing, it just makes it a lot easier to understand where the chain ID in hints comes from.

@ajsutton ajsutton added this pull request to the merge queue Apr 22, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 22, 2026
@ajsutton ajsutton added this pull request to the merge queue Apr 22, 2026
Merged via the queue into develop with commit ef4820a Apr 22, 2026
128 checks passed
@ajsutton ajsutton deleted the aj/fix/interop-provider-chain-id branch April 22, 2026 02:02
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.

2 participants