Skip to content

test(outbox): expand outbox tests with invalid cases#940

Merged
zcabter merged 3 commits intomainfrom
ryan/rv-910
Mar 30, 2026
Merged

test(outbox): expand outbox tests with invalid cases#940
zcabter merged 3 commits intomainfrom
ryan/rv-910

Conversation

@zcabter
Copy link
Copy Markdown

@zcabter zcabter commented Mar 24, 2026

Closes RV-910

What

Expand outbox proof tests with invalid and adversarial cases.

Why

The existing outbox proof tests only covered the happy path - producing, verifying valid proofs and serialization round trip. There was no coverage for invalid inputs (stale/future levels) or malicious proof manipulation (crafting custom Merkle proofs). These negative tests ensure the proof system rejects invalid proofs with the correct errors.

How

Added negative/adversarial tests for outbox proofs.

  • Invalid levels - asserts produce_outbox_proof returns LevelNotFound for stale (before outbox window) and future levels.
  • Swapped Merkle proofs - produces two valid proofs at different levels, swaps their inner Merkle proofs, and asserts verification rejects both with AbsentDataAccess.
  • Tampered message size - directly mutates the serialized length prefix (zero, incoherent, oversized) and asserts deserialisation fails.
  • Oversized message replacement - replaces the 4096 B message payload with 8192 B and asserts deserialisation fails. (Ignored pending RV-950.)
  • Zero-sized message replacement - replaces the message with an empty payload; deserialisation and verification both pass (zero-sized messages are valid).

Manually Testing

make all
cargo nextest run -p octez-riscv --test test_outbox_proofs

Tasks for the Author

  • Link all Linear issues related to this MR using magic words (e.g. part of, relates to, closes).
  • Eliminate dead code and other spurious artefacts introduced in your changes.
  • Document new public functions, methods and types.
  • Make sure the documentation for updated functions, methods, and types is correct.
  • Add tests for bugs that have been fixed.
  • Explain changes to regression test captures when applicable.
  • Write commit messages in agreement with our guidelines.
  • Self-review your changes to ensure they are high-quality.
  • Complete all of the above before assigning this MR to reviewers.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 24, 2026

Benchmark results for revision 726318b:

Metric Duration TPS
Mean 1.5178199s 26.354
Worst 1.530666693s 26.132
Best 1.512254497s 26.451
Standard Deviation ±4.439112ms ±0.077
Full results
Run Transfers Duration TPS
1 40 1.530666693s 26.132
2 40 1.519157016s 26.330
3 40 1.519014174s 26.333
4 40 1.525087069s 26.228
5 40 1.515481292s 26.394
6 40 1.513075773s 26.436
7 40 1.515604355s 26.392
8 40 1.51297689s 26.438
9 40 1.520588504s 26.306
10 40 1.517226526s 26.364
11 40 1.518764831s 26.337
12 40 1.513346565s 26.431
13 40 1.522108985s 26.279
14 40 1.516284783s 26.380
15 40 1.520087662s 26.314
16 40 1.51386594s 26.422
17 40 1.512254497s 26.451
18 40 1.515893146s 26.387
19 40 1.515078748s 26.401
20 40 1.519834546s 26.319

Compare the results above with those for the default branch.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.74%. Comparing base (702c29a) to head (5315a59).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #940      +/-   ##
==========================================
+ Coverage   89.71%   89.74%   +0.03%     
==========================================
  Files         110      110              
  Lines       22136    22136              
  Branches    22136    22136              
==========================================
+ Hits        19859    19866       +7     
- Misses       1882     1888       +6     
+ Partials      395      382      -13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 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.

Copy link
Copy Markdown
Contributor

@victor-dumitrescu victor-dumitrescu left a comment

Choose a reason for hiding this comment

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

I think there are a few more scenarios worth testing to do with manipulating the proof itself:

  • proof where the size of the message has been tampered with (both too small and too large), resulting in a deserialisation error
  • proof which contains a message larger than the maximum message size, resulting in deserialisation error

There are a few more interesting scenarios but which we can't easily test with the current dummy kernel in which we fill every outbox level with the same message. We'd probably need to bring in the echo kernel and control the inbox for each test. These could be:

  • valid proof with correct level but wrong index
  • valid proof with correct index but wrong level
  • try to produce proof for index which doesn't contain a message

The latter can be left as future work for now.

Copy link
Copy Markdown
Contributor

@thomasathorne thomasathorne left a comment

Choose a reason for hiding this comment

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

LGTM, modulo the comments from Victor.

@zcabter zcabter force-pushed the ryan/rv-910 branch 2 times, most recently from df0f024 to f1a1cdf Compare March 30, 2026 09:47
@zcabter zcabter enabled auto-merge March 30, 2026 12:42
@zcabter zcabter added this pull request to the merge queue Mar 30, 2026
Merged via the queue into main with commit 2e4c20a Mar 30, 2026
9 checks passed
@zcabter zcabter deleted the ryan/rv-910 branch March 30, 2026 14:24
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.

3 participants