Skip to content

fix(kona): unbreak kona-host-client-offline-cannon (target spec + fresh witness)#20167

Merged
wwared merged 5 commits intodevelopfrom
aj/fix/kona-offline-cannon-target-spec
Apr 21, 2026
Merged

fix(kona): unbreak kona-host-client-offline-cannon (target spec + fresh witness)#20167
wwared merged 5 commits intodevelopfrom
aj/fix/kona-offline-cannon-target-spec

Conversation

@ajsutton
Copy link
Copy Markdown
Contributor

Summary

The kona-host-client-offline-cannon CI job has been silently broken: the cannon-builder:v1.0.0 Docker image ships with a MIPS64 target spec that has target-c-int-width: 64, but the n64 ABI specifies 32. The wrong value causes LLVM to miscompile str equality comparisons — str == str always returns false — which breaks kona-client under cannon.

Upstream fix #19730 (commit 04ab310) corrected the JSON in the source tree and plumbed it through cannon-repro.dockerfile's COPY step, but did not reach rust/kona/justfile's build-cannon-client / lint-cannon recipes — those still invoke the baked image directly. Those recipes feed the offline-cannon CI job.

This PR:

  1. build-cannon-client / lint-cannon: bind-mount the corrected rust/kona/docker/cannon/mips64-unknown-none.json over /mips64-unknown-none.json inside the container. Same mechanism cannon-repro.dockerfile uses, adapted for docker run.
  2. Refreshed witness tar: the committed holocene-op-sepolia-26215604-witness.tar.zst was recorded ~1 year ago against a pre-fix kona-client. Regenerated against op-sepolia finalized block #42427365 using a correctly-built kona-host. Updates .circleci/continue/rust-ci.yml (BLOCK_NUMBER, L2_CLAIM, L2_OUTPUT_ROOT, L2_HEAD, L1_HEAD, WITNESS_TAR_NAME). Drops the hardfork prefix from the filename.
  3. New generate-witness-tar just recipe: encapsulates the regeneration procedure (native kona-host run + tar zst package) with arguments mirroring run-client-native. Prints the boundary hashes so operators can sync the CI config when regenerating for a new block.

Reproducer for the str miscompile: https://github.com/ajsutton/rust-mips64-str-miscompile

Test plan

  • kona-host-client-offline-cannon passes on this PR (was hanging/timing out on develop)
  • kona-lint-cannon still passes (same mount path)
  • required-rust-ci gate passes

🤖 Generated with Claude Code

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 11.4%. Comparing base (64dce2f) to head (8641beb).
⚠️ Report is 5 commits behind head on develop.

Additional details and impacted files
@@            Coverage Diff             @@
##           develop   #20167     +/-   ##
==========================================
- Coverage     11.4%    11.4%   -0.1%     
==========================================
  Files          669      669             
  Lines        72658    72658             
==========================================
- Hits          8332     8315     -17     
- Misses       64182    64199     +17     
  Partials       144      144             
Flag Coverage Δ
cannon-go-tests-64 66.3% <ø> (ø)
contracts-bedrock-tests 81.8% <ø> (-0.3%) ⬇️
unit 0.5% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.
see 2 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/kona-offline-cannon-target-spec branch from a72e856 to b4db834 Compare April 21, 2026 00:36
@ajsutton ajsutton marked this pull request as ready for review April 21, 2026 02:40
@ajsutton ajsutton requested a review from a team as a code owner April 21, 2026 02:40
Copy link
Copy Markdown
Contributor

@wwared wwared left a comment

Choose a reason for hiding this comment

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

Seems good to me. I merged #20197 on top of this and rebased on top of latest develop

AGREED_L2_OUTPUT_ROOT=$(cast rpc --rpc-url $OP_NODE_ADDRESS "optimism_outputAtBlock" $(cast 2h $((CLAIMED_L2_BLOCK_NUMBER - 1))) | jq -r .outputRoot)
AGREED_L2_HEAD_HASH=$(cast block --rpc-url $L2_NODE_ADDRESS $((CLAIMED_L2_BLOCK_NUMBER - 1)) --json | jq -r .hash)
L1_ORIGIN_NUM=$(cast rpc --rpc-url $OP_NODE_ADDRESS "optimism_outputAtBlock" $(cast 2h $((CLAIMED_L2_BLOCK_NUMBER - 1))) | jq -r .blockRef.l1origin.number)
L1_HEAD=$(cast block --rpc-url $L1_NODE_ADDRESS $((L1_ORIGIN_NUM + 30)) --json | jq -r .hash)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Question: Is there a particular reason why the "+ 30" offset is used here for L1_HEAD? I know the other kona run-client justfile jobs use the same value

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

honestly, no idea. It does seem very arbitrary and probably not entirely the right approach, but is just following what was previously done (as far as I can tell anyway). It probably should be finding the required L1 head from the safe head db but kona-node didn't support it (maybe still doesn't - not sure if it landed). We may need to circle back around and improve this later but it seems low priority since it has managed to find a working setup.

Adds a just recipe that runs kona-host in native mode against real RPCs
to collect preimages and packages them into a zstd-compressed tar in the
layout expected by the kona-host-client-offline CI job. Arguments mirror
run-client-native for consistency. Prints the computed boundary hashes so
operators can sync .circleci/continue/rust-ci.yml when regenerating for a
new block.

Requires RPC endpoints with archive state available for the target block;
pruned EL nodes will reject optimism_outputAtBlock for old blocks.
The committed witness tar at holocene-op-sepolia-26215604-witness.tar.zst
is ~1 year old and tracks a post-Holocene test block chosen when the
offline host+client test was introduced. kona-host-client-offline-cannon
has been failing because the rebuilt kona-client's preimage access
pattern no longer matches what was recorded back then.

Regenerates against a recent op-sepolia finalized block so the test
exercises current hardfork code paths (through Jovian). Uses the new
generate-witness-tar just recipe. Drops the hardfork prefix from the
tar filename since the block is past multiple hardforks.

Boundaries (see .circleci/continue/rust-ci.yml):
  BLOCK_NUMBER=42427365
  L2_CLAIM=0xaaf7cb1725cb9766899cbd78866270d8561e79b91c00be5c53adb7699e0e7715
  L2_OUTPUT_ROOT=0xac90a2e8cfee33ecb47b73f3e83f9aa18ff7ecf28bc8d34b2de857919e50b6c6
  L2_HEAD=0xc5fa07d52e83f7a2e68de356e7a7bfc9306a9b28666ec6c9cc6e4f8e1b268278
  L1_HEAD=0xab33322985684f2c6fbcf375bdcb69cc0f10547e58cb5a7b889172ee9ea77aef
  L2_CHAIN_ID=11155420
… path

The cannon-builder:v1.0.0 Docker image ships with a MIPS64 target spec
that has target-c-int-width=64. The correct n64 ABI value is 32, and the
wrong value caused LLVM to miscompile str equality comparisons — the
branch on the memcmp return was eliminated, making str == str always
return false. Fixed upstream in commit 04ab310 for the reproducible
prestate path, but kona-host-client-offline-cannon goes through
rust/kona/justfile's build-cannon-client and lint-cannon recipes which
invoke the baked image directly and never picked up the corrected spec.

Mount the already-corrected spec from rust/kona/docker/cannon/ over
/mips64-unknown-none.json inside the container so cargo resolves it
instead of the broken baked-in file. Same mechanism cannon-repro.dockerfile
uses at its COPY step, adapted for docker run.

Reproducer: https://github.com/ajsutton/rust-mips64-str-miscompile
The offline-cannon witness is ~7MB and changes every time the test block
number is refreshed, which bloats git history with binary churn. Mirror
op-program's compat-data approach: host the tar as a GitHub release asset
on ethereum-optimism/chain-test-data and download it on demand with etag
caching.

- Add scripts/fetch-witness-tar.sh — curl with --etag-save/--etag-compare
  so repeated local runs skip re-download when the release asset is
  unchanged (same pattern as op-program/scripts/run-compat.sh).
- Tag convention is kona-YYYY-MM-DD, namespaced to avoid colliding with
  the op-program compat data tags already hosted in the same repo.
- CI gets a Fetch witness data step and a WITNESS_TAR_URL env var,
  inserted before the existing Decompress witness data step.
- generate-witness-tar now prints the exact gh release create/upload
  commands so regenerators follow the same ritual.
- Drop the committed tar and gitignore *.tar.zst in testdata/ so
  regenerated artefacts cannot sneak back into the tree.
…ffline

The CI Fetch + Decompress steps and WITNESS_TAR_NAME/URL env vars were
really an implementation detail of running the offline cannon test.
Moving them into a `prepare-witness-data` recipe wired as a dependency
of `run-client-cannon-offline` means:

- CI shrinks to just the boundary env vars + one just invocation.
- Local runs of `just run-client-cannon-offline ...` automatically
  fetch and decompress (no separate command to remember).
- The tar name and release URL live next to the block/claim/etc.
  boundary metadata in a single recipe, so refreshing for a new block
  only touches the justfile (plus a new chain-test-data release).

Also drops the fetch script's hardcoded URL default — the just recipe
is now the single source of truth for the canonical release URL.
@wwared wwared force-pushed the aj/fix/kona-offline-cannon-target-spec branch from c225233 to 8641beb Compare April 21, 2026 13:56
@wwared wwared enabled auto-merge April 21, 2026 20:07
@wwared wwared added this pull request to the merge queue Apr 21, 2026
Merged via the queue into develop with commit 3fc8c8a Apr 21, 2026
180 checks passed
@wwared wwared deleted the aj/fix/kona-offline-cannon-target-spec branch April 21, 2026 20:39
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