Skip to content

STR 2066 Revise and harden OL STF tests#1760

Merged
storopoli merged 34 commits into
mainfrom
STR-2066
May 22, 2026
Merged

STR 2066 Revise and harden OL STF tests#1760
storopoli merged 34 commits into
mainfrom
STR-2066

Conversation

@krsnapaudel
Copy link
Copy Markdown
Contributor

@krsnapaudel krsnapaudel commented May 6, 2026

Description

Refactor, revise and harden OL STF tests. Add tests to fill existing gaps.

  • Module reorganization: splits tests/stf.rs and tests/edge_cases.rs into themed modules (verify_header, verify_body, staging_layers, sau_transfers, sau_messages, sau_limits, sau_sequences, sau_logs, chain_processing, asm_manifests, tx_constraints, partial_execution, stress, sau_validation).
  • Behavior fixture layer: introduces OLStfFixture::builder() + OLStfFixtureBuilder for pre-genesis setup, FixtureBlockBuilder for child-block sequencing and tx construction, SAU/GAM/manifest fixture builders, StateSnapshot::assert_unchanged for state-unchanged pinning, and StateTestExt: IStateAccessor for state-lookup ergonomics. Raw verifier/staging/tamper helpers stay as free functions over &mut impl IStateAccessorMut.
  • Production fixes:
    • Rejects SAU updates with unconsumed proofs in transaction_processing.
    • Replaces broad ExecError::ChainIntegrity with finer variants: AsmManifestHeightMismatch { expected, actual, index } in manifest_processing and HeaderEpochMismatch / ContextEpochMismatch in chain_processing. Downstream callers updated.
  • Existing-test hardening: positive controls on tamper tests, state-unchanged assertions via snapshots on SAU/inbox/ledger-reference validation tests, bridge-gateway silent-drop pinned across 5 drop variants (including limbo-amount checks), body-root mismatch revived, mid-block partial execution semantics pinned, zero-balance boundary asserted including transfer-after-drain, supply-cap tests use BitcoinAmount::MAX_MONEY and BitcoinAmount::MAX with intent comments, limbo bookkeeping asserted in deposit/withdrawal and ASM manifest tests.
  • New coverage: chain processing error branches, GAM structural validation, ASM manifest processing boundaries (height gap, height ≤ last L1, max manifest count, malformed deposit log, unknown-serial deposit limboing, checkpoint tip update), log-root padding parity, epoch preseal and post-epoch diff verification, dependent SAU sequences, replay rejection across blocks, TxConstraints slot boundaries, ledger-reference proof rejection paths, multi-SAU same-block scenarios (distinct senders, sequential seqnos, duplicate seqno failure), staging-layer execution paths (WriteTrackingState, IndexerState), SAU and log boundary limits, SAU target-type and sequence-number bounds, block terminality verification, output message delivery to snark inboxes, SAU log emission contract, ignored stress tests for large inbox/tx batches.

This PR was created with help from Claude Code and Codex.

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

Please review the doc below and the test added.

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

  • Yes
  • No

If yes, please add relevant links:
OL STF test cases

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

STR-2066

@krsnapaudel krsnapaudel requested review from a team as code owners May 6, 2026 19:23
@krsnapaudel krsnapaudel marked this pull request as draft May 6, 2026 19:23
@krsnapaudel krsnapaudel requested review from barakshani and bewakes May 6, 2026 19:25
@krsnapaudel
Copy link
Copy Markdown
Contributor Author

@barakshani Added tests with tx staging. See staging_layers.rs.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

Commit: 1ce1f16

SP1 Execution Results

program cycles gas
EVM EE Chunk 565,505 771,876
EVM EE Account 421,125 526,509
Checkpoint 2,241,010 2,582,478

@codecov
Copy link
Copy Markdown

codecov Bot commented May 6, 2026

Codecov Report

❌ Patch coverage is 92.54120% with 86 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.01%. Comparing base (d1d88bd) to head (4d6250c).

Files with missing lines Patch % Lines
crates/ol/stf/src/test_utils.rs 89.42% 86 Missing ⚠️
@@            Coverage Diff             @@
##             main    #1760      +/-   ##
==========================================
+ Coverage   79.76%   80.01%   +0.25%     
==========================================
  Files         674      674              
  Lines       74805    75685     +880     
==========================================
+ Hits        59665    60559     +894     
+ Misses      15140    15126      -14     
Flag Coverage Δ
functional 60.25% <58.06%> (+0.06%) ⬆️
unit 66.12% <92.54%> (+0.45%) ⬆️

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

Files with missing lines Coverage Δ
bin/prover-perf/src/programs/checkpoint.rs 84.78% <100.00%> (ø)
crates/consensus-logic/src/fcm/service.rs 94.25% <100.00%> (-0.01%) ⬇️
crates/ledger-types/src/errors.rs 75.00% <ø> (+31.25%) ⬆️
crates/ol/da/src/scheme.rs 100.00% <100.00%> (ø)
crates/ol/da/src/types/payload.rs 91.01% <100.00%> (+3.65%) ⬆️
crates/ol/stf/src/chain_processing.rs 100.00% <100.00%> (+25.53%) ⬆️
crates/ol/stf/src/context.rs 90.64% <100.00%> (+0.64%) ⬆️
crates/ol/stf/src/manifest_processing.rs 94.07% <100.00%> (+6.95%) ⬆️
crates/ol/stf/src/proof_verification.rs 98.75% <100.00%> (+3.65%) ⬆️
crates/ol/stf/src/transaction_processing.rs 99.14% <100.00%> (+3.69%) ⬆️
... and 3 more

... and 10 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AlpenCodex
Copy link
Copy Markdown
Collaborator

@codex Please review this PR

(This is a test being run by @sampkaALP (security))

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@krsnapaudel krsnapaudel marked this pull request as ready for review May 7, 2026 17:51
@delbonis
Copy link
Copy Markdown
Contributor

delbonis commented May 7, 2026

Merge conflict because of the limbo PR that just went in.

@delbonis
Copy link
Copy Markdown
Contributor

delbonis commented May 7, 2026

This is good work, but there's still a lot of repeating patterns with tests. I would love it if we could think of some declaritive model for describing all the different test scenarios which would make it easier to write tests by handling the setup/asserts better, and when we make changes to how things work we don't have to change tons of tests that interact with it. I am not sure what that would look like at the moment.

@krsnapaudel
Copy link
Copy Markdown
Contributor Author

This is good work, but there's still a lot of repeating patterns with tests. I would love it if we could think of some declaritive model for describing all the different test scenarios which would make it easier to write tests by handling the setup/asserts better, and when we make changes to how things work we don't have to change tons of tests that interact with it. I am not sure what that would look like at the moment.

@delbonis Yes I am aware. I will try to come up with some ways to reduce repetition with AI help.

@krsnapaudel krsnapaudel force-pushed the STR-2066 branch 3 times, most recently from c8c18a8 to beec3cd Compare May 9, 2026 04:19
@krsnapaudel krsnapaudel force-pushed the STR-2066 branch 2 times, most recently from 4990930 to 75017a3 Compare May 13, 2026 18:48
@delbonis
Copy link
Copy Markdown
Contributor

wow I would have assumed this would have caused more conflicts

Copy link
Copy Markdown
Contributor

@delbonis delbonis left a comment

Choose a reason for hiding this comment

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

There's a lot of good things here to improve ergonomics, however there's also a lot of smelly problems (like "being too specific without reason") and unidiomatic/misleading names that should be improved.

Also in most of the tests we can probably just do use ...::test_utils::*; instead of importing each and every item, since that just creates more work to keep updated (either for humans or LLMs).

Comment thread crates/ol/stf/src/test_utils.rs
Comment thread crates/ol/stf/src/test_utils.rs Outdated
Comment thread crates/ol/stf/src/test_utils.rs Outdated
Comment thread crates/ol/stf/src/test_utils.rs Outdated
Comment thread crates/ol/stf/src/test_utils.rs
Comment thread crates/ol/stf/src/test_utils.rs Outdated
Comment thread crates/ol/stf/src/test_utils.rs Outdated
Comment thread crates/ol/stf/src/test_utils.rs Outdated
Comment thread crates/ol/stf/src/test_utils.rs Outdated
Copy link
Copy Markdown
Contributor

@bewakes bewakes left a comment

Choose a reason for hiding this comment

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

Great work! The tests are much cleaner. Some comments, nothing major blocking.
You might want to also update this doc as necessary.

Comment thread crates/ol/stf/src/tests/asm_manifests.rs Outdated
Comment thread crates/ol/stf/src/test_utils.rs Outdated
Comment thread crates/ol/stf/src/tests/asm_manifests.rs
Comment thread crates/ol/stf/src/tests/asm_manifests.rs
Comment thread crates/ol/stf/src/tests/asm_manifests.rs
Comment thread crates/ol/stf/src/tests/sau_limits.rs
Comment thread crates/ol/stf/src/tests/sau_limits.rs Outdated
Comment thread crates/ol/stf/src/test_utils.rs Outdated
Comment thread crates/ol/stf/src/test_utils.rs Outdated
Comment thread crates/ol/stf/src/tests/stress.rs Outdated
Copy link
Copy Markdown
Contributor

@prajwolrg prajwolrg left a comment

Choose a reason for hiding this comment

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

utACK

@storopoli storopoli added this pull request to the merge queue May 22, 2026
Merged via the queue into main with commit 5d6d955 May 22, 2026
31 checks passed
@storopoli storopoli deleted the STR-2066 branch May 22, 2026 13:51
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.

6 participants