Skip to content

test(xmtp_mls): randomized bidi delivery fuzz against the live node#3850

Open
tylerhawkes wants to merge 1 commit into
tyler/bidi-ledger-loss-fixesfrom
tyler/bidi-fuzz
Open

test(xmtp_mls): randomized bidi delivery fuzz against the live node#3850
tylerhawkes wants to merge 1 commit into
tyler/bidi-ledger-loss-fixesfrom
tyler/bidi-fuzz

Conversation

@tylerhawkes

@tylerhawkes tylerhawkes commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Randomized no-loss testing for XIP-83 bidi delivery, at three depths. Stacked on #3852 (the ledger fixes this work found).

What's here

1. Live fuzz against the containerized node (bidi_fuzz_tests.rs), two tests:

  • fuzz_server_honors_the_bidi_wave_contract — up to 10 raw BidiConnections, each with its own frame-by-frame checker of the server guarantees (always-ack, per-topic wave cursor order, live-shield until CatchUpComplete, completeness above the lowest floor), racing against up to 10 member-client producers publishing concurrent bursts into the same groups.
  • fuzz_transport_delivery_never_loses_above_the_floor — up to 10 real BidiTransports, one per proxied subscriber client on its own toxiproxy-faultable wire, plus the same racing producer pool. Ops: leases at random floors (yank engine), deliberate kills, stalls-to-backpressure-drop, TCP blips (per-consumer and correlated), suspend/resume, mid-run welcome-minting groups. Invariants per lease: strictly increasing above its floor (order + exactly-once + nothing below the floor in one sweep), alive-at-end completeness, and durable-cursor recovery-chain union completeness — all against unary ground truth.

Seeded and replayable (XMTP_BIDI_FUZZ_SEED, XMTP_BIDI_FUZZ_ROUNDS), retries = 0 in nextest so flakes can't self-heal, and per-run schedule telemetry so a dead chaos lever is visible. Racing member sends settle their one-time post-join KeyUpdate commits serially at setup, so the races contend as app messages at a stable epoch (join-time epoch churn is MLS-layer behavior with its own tests).

2. Deterministic proptest model of the transport ledger (bidi_transport_props.rs): generated op schedules (publish / lease / drop / serve / partial-serve / wire-kill) drive the real BidiTransport over the scripted mock wire against an in-memory server model of the XIP-83 contract, with a reference model asserting strictly-increasing exactly-once delivery and exact log-suffix completeness — no sentinels, no timing, real shrinking. Regression seeds are checked in. PROPTEST_CASES scales the soak (default 16, CI-friendly).

Already earned its keep

Testing

  • Proptest model: 2,000-schedule soak green (~11 min) plus all shrunk regressions.
  • Live fuzz: green across 12+ fresh seeds, spread covering 2–10 producers/consumers/connections; telemetry shows every lever firing (40+ armed yanks, drops→relinks, correlated blips, suspends, mid-run welcomes).
  • just lint green.

🤖 Generated with Claude Code

https://claude.ai/code/session_017AVuJ7VAgen2fFhtJQ6zid

Note

Add randomized fuzz tests for bidi transport delivery against a live node

  • Adds bidi_fuzz_tests.rs with two fuzz tests against a live containerized node: one validates XIP-83 server wire contract invariants per raw BidiConnection, and one validates per-lease delivery ordering and completeness across drops, stalls, suspends, and toxiproxy-injected network faults.
  • Adds bidi_transport_props.rs with a proptest-based harness using an in-memory mock wire to assert cursor ordering, exact suffix delivery, and CatchUpComplete semantics per lease under randomized operation schedules.
  • Disables test retries for bidi_fuzz tests in both default and CI nextest profiles to prevent masking flaky failures.

Macroscope summarized b26bf6d.

@tylerhawkes tylerhawkes requested a review from a team as a code owner July 13, 2026 23:04
@claude

claude Bot commented Jul 13, 2026

Copy link
Copy Markdown

Claude finished @tylerhawkes's task —— View job


Code Review Complete

Review Date: 2026-07-14 09:32 UTC

This is high-quality test code with excellent engineering. The fuzz testing approach is comprehensive and well-architected. Here are the key observations:

Strengths

  • Comprehensive coverage: Two-layer approach (raw connection + transport) thoroughly validates both server and client contracts
  • Deterministic reproducibility: Seed-based RNG with environment variable control (XMTP_BIDI_FUZZ_SEED, XMTP_BIDI_FUZZ_ROUNDS)
  • Real shrinking: The proptest model provides minimal counterexamples (as evidenced by the regression seeds)
  • Good chaos engineering: Multiple fault injection paths (TCP blips, stalls, suspends, kills, toxiproxy)
  • Telemetry: Schedule metrics prove all code paths are exercised

Minor Issues

1. Potential panic in ground_truth (bidi_fuzz_tests.rs:174)

.unwrap_or_else(|_| panic!("ground-truth query failed"))

This loses error context. Consider: .expect("ground-truth query failed: {e}")

2. Magic number without const (bidi_fuzz_tests.rs:183)

assert!(ids.len() < 95, ...

Should be a named constant like MAX_QUERY_SIZE to clarify relationship to PUBLISH_CAP=60

3. Unbounded retry loop (bidi_fuzz_tests.rs:936-946)

assert!(attempts < 40, "lease kept failing: {e} (seed={seed})");

Hard limit of 40 attempts * 250ms = 10s max. Consider deriving from SETTLE constant for consistency.

4. Race condition comment (bidi_fuzz_tests.rs:1234-1236)
The comment about "drop landing during wind-down" suggests this is a known edge case. Consider adding an assertion that relinks stabilize within a bounded window.

5. Proptest config could be more visible (bidi_transport_props.rs:701-707)
The PROPTEST_CASES default of 16 is quite low. Consider adding a comment explaining why (CI-friendly) and suggesting higher values for local testing.

Performance Considerations

  • Both tests have 300s timeouts but typical runs complete in ~60-90s based on the rounds configuration
  • The PUBLISH_CAP=60 limit keeps ground-truth queries single-page, which is good
  • Concurrent producer bursts via JoinSet is efficient

Security

  • No security concerns: this is test-only code with no production exposure
  • Toxiproxy fault injection properly isolated to test consumers

Test Configuration

The nextest config change (lines 48-54 in .config/nextest.toml) correctly disables retries for fuzz tests to prevent masking failures — this is critical and well-justified.

Regression Tests

The two checked-in proptest regressions show the tests are working as intended — they found real bugs and captured minimal repro cases.

Summary

Excellent work. The architecture is sound, the invariants are well-chosen, and the implementation is thorough. The minor issues are primarily about code clarity rather than correctness. This will be a valuable long-term asset for catching regressions.


macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Jul 13, 2026
@macroscopeapp

macroscopeapp Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

Test-only changes adding fuzz and property-based tests for bidi delivery. All new code is gated behind #[cfg(test)] and has no runtime impact. Author owns all changed files.

No code changes detected at b26bf6d. Prior analysis still applies.

You can customize Macroscope's approvability policy. Learn more.

@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.96438% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.53%. Comparing base (11acadd) to head (b26bf6d).

Files with missing lines Patch % Lines
.../xmtp_api_d14n/src/queries/bidi_transport_props.rs 97.96% 8 Missing ⚠️
Additional details and impacted files
@@                       Coverage Diff                        @@
##           tyler/bidi-ledger-loss-fixes    #3850      +/-   ##
================================================================
+ Coverage                         85.40%   85.53%   +0.12%     
================================================================
  Files                               413      414       +1     
  Lines                             66012    66405     +393     
================================================================
+ Hits                              56379    56799     +420     
+ Misses                             9633     9606      -27     

☔ View full report in Codecov by Harness.
📢 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.

Base automatically changed from tyler/xip83-delivery-tags to main July 13, 2026 23:26
@macroscopeapp macroscopeapp Bot dismissed their stale review July 13, 2026 23:26

Dismissing prior approval to re-evaluate ef8afbd

@tylerhawkes tylerhawkes changed the base branch from main to tyler/bidi-ledger-loss-fixes July 14, 2026 01:14
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Jul 14, 2026
@tylerhawkes tylerhawkes force-pushed the tyler/bidi-ledger-loss-fixes branch from 4b8388e to 977d84a Compare July 14, 2026 05:54
@macroscopeapp macroscopeapp Bot dismissed their stale review July 14, 2026 05:54

Dismissing prior approval to re-evaluate 438a3e9

@tylerhawkes

Copy link
Copy Markdown
Contributor Author

CI triage for the failures on the previous head:

  • lint-config / lint-workspace: treefmt formatting on the two new Rust files (whitespace-level) — fixed, just lint green locally.
  • test-workspace: the failures were a burst of ConnectionReset errors across unrelated xmtp_mls client tests (should_reconnect first, then a fast-fail cluster). Reproduced the setup locally — full workspace suite with both fuzz tests running concurrently: 2497/2497 passed, with should_reconnect flaking once and passing on retry. That test opens a raw subscription and treats any connection hiccup as fatal, so it's load-sensitive; the CI runner amplifies it. Nothing in the fuzz suite is implicated — the cascade signature doesn't appear in any of the fuzz tests' own assertions, and the node stayed healthy under the same concurrency locally.

Also rebased onto #3852's latest (the withheld-buffer cap + Arc sharing) — the proptest model and both live fuzz tests are green against it.

…roptest ledger model

Two layers of randomized no-loss checking for XIP-83 bidi delivery:

- Live fuzz against the containerized node: up to 10 racing member
  producers x up to 10 consumers (raw connections in the server-contract
  test; per-consumer proxied BidiTransports in the transport test), with
  per-lease strictly-increasing delivery, chain-union completeness, and
  toxiproxy network faults. Seeded and replayable; schedule telemetry
  proves every lever fires.
- Deterministic proptest model of the transport ledger over the scripted
  mock wire: generated op schedules (publish/lease/drop/serve/partial-
  serve/wire-kill) against an in-memory XIP-83 server model, with real
  shrinking. Found the virgin-topic snapshot loss and the cross-wave
  watermark gap-jump fixed in the tags commit; regression seeds checked in.
@tylerhawkes tylerhawkes force-pushed the tyler/bidi-ledger-loss-fixes branch from 977d84a to 11acadd Compare July 14, 2026 06:21
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.

1 participant