feat: e2e fuzz tests

[ameya-deshmukh]

No description provided.

[github-actions]

PR Review Summary

Changes

Adds a new E2E adversarial fuzz test (crates/seismic/node/tests/e2e/fuzz.rs) that spins up a real dev node and bombards it with malformed, adversarial, and signature-corrupted transactions across raw bytes, EIP-1559, and seismic (0x4a) tx types, asserting the node stays alive after each batch. Adds alloy-signer-local and rand as dev dependencies.

---

Blocking Issues

1. No timeout on node readiness wait — fuzz.rs:447

   rx.recv().await.unwrap();  // hangs forever if node never signals ready

   If the child process crashes at startup, never emits "Starting consensus engine", or the binary needs a full rebuild (first CI run), this line waits indefinitely. CI will eventually hit its own job timeout with a cryptic message rather than a clear test failure. Use tokio::time::timeout:

   tokio::time::timeout(Duration::from_secs(120), rx.recv())
       .await
       .expect("Timed out waiting for node to become ready")
       .expect("Node readiness channel closed");

---

Suggestions

1. assert_node_alive is a misleading no-op — fuzz.rs:38-40

   async fn assert_node_alive(client: &jsonrpsee::http_client::HttpClient, wallet_addr: Address) {
       let _ = get_nonce(client, wallet_addr).await;  // result is u64, let _ is pointless
   }

   The function name implies an assertion, but the actual check relies on get_nonce panicking internally via its .unwrap(). The let _ = is redundant since get_nonce returns u64, not Result. This is confusing — the call is only useful for its panic side-effect. Drop the let _ =: get_nonce(client, wallet_addr).await;. Better still, add an explicit assertion so the intent is obvious.

2. thread::sleep in async context — fuzz.rs:464

   thread::sleep(Duration::from_secs(WAIT));

   This blocks a tokio thread pool thread rather than yielding to the executor. Should be tokio::time::sleep(Duration::from_secs(WAIT)).await. (The same pattern exists in integration.rs — worth fixing both.)

3. Child process leaks on test panic — fuzz.rs:463

   shutdown_tx.try_send(()).unwrap();

   If any of the four fuzz batches panics (e.g., the node crashes and get_nonce inside assert_node_alive panics), this line is never reached and the seismic-reth child process is left running. Consider a RAII guard or tokio::spawn with the child handle captured directly, so cleanup runs even on panic.

4. Three identical typos in function-level comments — fuzz.rs:108, 145, 248

   "randomly-randomerated" → "randomly-generated"

---

Positive Notes

• Good variety of adversarial cases: empty payloads, type-prefix-only payloads, overflowed nonces/values/fees, wrong chain IDs, zero and random block hashes, large calldata, signature corruptions (single byte, full r+s, all-zero, all-FF).
• The seismic adversarial path correctly goes through get_signed_seismic_tx_bytes → get_unsigned_seismic_tx_request which encrypts the plaintext via the mock enclave before submission — no plaintext leakage in the fuzzing path.
• --enclave.mock-server is properly gated to the dev node spawned for testing.
• send_raw silently swallows rejections — correct, since the goal is to observe node liveness, not transaction success.
• The corrupt_last_n / corrupt_tail_random helpers are clean and reusable.
• Random cases per batch (RANDOM_CASES = 50) give reasonable coverage without excessive runtime.

[github-actions]

PR Review Summary

Changes

Adds an E2E fuzz test suite (fuzz.rs) that spins up a dev node and bombards it with adversarial inputs across four categories: malformed raw bytes, adversarial EIP-1559 parameters, adversarial seismic transactions, and signature-corrupted transactions. Verifies the node stays alive after each batch. Adds alloy-signer-local and rand to dev-dependencies.

Blocking Issues

None found.

Suggestions

1. Excessive RPC calls for nonces in send_adversarial_seismic_txs (fuzz.rs ~L230-300)
   Each of the 6 hardcoded seismic cases (plus 50 random cases) independently calls get_nonce before building the tx. Since all adversarial seismic txs should be rejected the nonce never advances, so get_nonce returns the same value 56 times. Consider fetching nonce once at the top of the function and reusing it, mirroring the pattern in send_adversarial_eip1559_txs and send_signature_corrupted_txs.

2. send_raw silently discards errors, hiding accidental acceptances (fuzz.rs L47-49)
   The function intentionally swallows rejections but also swallows accidental successes. If a malformed tx were mistakenly accepted (a bug), the test would not catch it. Consider logging errors at trace level for CI log visibility, or introducing a send_raw_expect_reject helper for the clearly-invalid cases (zero gas, truncated RLP, wrong chain ID, corrupted signatures) that asserts the node returns an error.

3. "128 KB calldata" EIP-1559 case may actually be a valid mempool tx (fuzz.rs L163-174)
   gas: Some(30_000_000) combined with a valid chain ID, fee, and current nonce produces a well-formed transaction that could be accepted into the mempool and mined. This would silently advance the nonce counter. Add a comment clarifying whether acceptance is intentional, or assign it an invalid nonce (e.g. u64::MAX) so it is definitively rejected like the rest of the adversarial cases.

4. Missing seismic-specific field corruption (fuzz.rs ~L220-340)
   The seismic fuzz cases test calldata content and block-hash validation but do not corrupt seismic-specific fields. Consider adding cases for:

   • Malformed encryption_pubkey (wrong length, all-zero bytes, point not on curve)
   • Malformed encryption nonce (wrong length, zeros)
   • Replayed seismic tx (duplicate block_hash + encryption_nonce pair)

   These are the highest-value attack surfaces for confidential-tx handling.

5. WAIT constant (1 s) may be too short on loaded CI runners (fuzz.rs L31, L468)
   The sleep after shutdown_tx is only best-effort cleanup; it is harmless for correctness but may leave background tasks running on slow CI machines. Consider bumping to 2-3 s or adding a comment that it is intentionally best-effort.

6. Verbose override-merge in build_signed_1559 (fuzz.rs L65-96)
   The manual field-by-field if is_some() copying is correct but verbose. Minor readability note only.

Positive Notes

• Follows the established test-file convention of #![allow(clippy::unwrap_used, ...)] consistently with integration.rs.
• Uses std::panic::AssertUnwindSafe to guarantee the shutdown signal is sent even on panic — good cleanup hygiene.
• Good spread of hard-coded edge cases (zero gas, max nonce, wrong chain ID, zero block hash, empty calldata) complemented by parameterized random cases (RANDOM_CASES = 50 per batch).
• Both EIP-1559 and seismic tx types are fuzzed, including signature corruption for both.
• Reuses the existing SeismicRethTestCommand infrastructure cleanly with no bespoke node-setup code.

[github-actions]

PR Review Summary

Changes

Adds an E2E fuzz test suite (fuzz.rs) that spins up a full dev node via SeismicRethTestCommand and bombards it with malformed raw bytes, adversarial EIP-1559 transactions, adversarial Seismic (0x4a) transactions, and signature-corrupted transactions. Registers the module in main.rs. Adds alloy-signer-local and rand as dev-dependencies.

---

Blocking Issues

1. Non-reproducible RNG — failures cannot be replayed (fuzz.rs lines 109, 135–142, 233–247, 361–378, 433–437)
   Every helper uses rand::rng() (OS-seeded thread-local). A CI failure involving a specific random input is impossible to reproduce without capturing the seed.
   Fix: use a seeded RNG, print the seed before each test run, and allow overriding it via an env var:

   let seed: u64 = std::env::var("FUZZ_SEED")
       .ok()
       .and_then(|s| s.parse().ok())
       .unwrap_or_else(|| rand::rng().random());
   eprintln\!("FUZZ_SEED={seed}  (re-run with FUZZ_SEED={seed} to reproduce)");
   let mut rng = rand::rngs::SmallRng::seed_from_u64(seed);

2. Node process may leak on timeout / startup failure (fuzz.rs lines 447–455)
   If rx.recv() times out (120 s), the test returns an error but the child cargo run process that was spawned inside SeismicRethTestCommand::run is never killed — the shutdown_tx is never sent. Subsequent test runs (or parallel jobs) may fail due to port conflicts.
   Fix: send the shutdown signal in an on_error guard or wrap startup in a scopeguard:

   let ready = tokio::time::timeout(Duration::from_secs(120), rx.recv()).await;
   if ready.is_err() {
       let _ = shutdown_tx.try_send(());
       panic\!("Timed out waiting for node to become ready");
   }
   ready.unwrap().expect("Node readiness channel closed");

---

Suggestions

1. Missing seismic-field corruption cases — acknowledged in TODO but should be tracked (fuzz.rs lines 257–259)
   The TODO notes that encryption_pubkey and encryption_nonce are not corrupted post-encoding. These fields are the core of Seismic's confidential-tx security model. This is the most important gap for the security testing goals of this file. Consider opening a follow-up issue and linking it in the comment so it doesn't stall.

2. shutdown_tx.try_send(()).unwrap() can silently succeed even if the node loop already exited (fuzz.rs line 473)
   try_send returns Err if the channel is full (capacity 1) or disconnected. The current code ignores the distinction. Use let _ = shutdown_tx.send(()).await; (async send) to avoid the silent-discard risk and to let the child's kill future complete before proceeding.

3. Nonce not refreshed between random-case loops (fuzz.rs send_adversarial_seismic_txs / send_signature_corrupted_txs)
   The random loop iterations all sign with the same nonce fetched once at function entry. A tx with a valid nonce that happens to be accepted (e.g., valid-enough calldata or a block_hash that matches) will advance the on-chain nonce, causing all subsequent same-nonce cases to be rejected as nonce-too-low rather than for the intended adversarial reason. This can mask bugs. Either use a fresh nonce fetch before each send or intentionally use nonce + i as u64 for sequential ordering.

4. Blanket #\![allow(...)]) suppresses the whole file (fuzz.rs line 6)
   clippy::indexing_slicing is only needed for two sites (bytes[0] and bytes[offset]). Moving the allows to the specific callsites keeps the rest of the file under the project's stricter linting policy. The CI enforces these as errors project-wide; even test files should minimise the blast radius of suppression.

5. build_signed_1559 override logic is verbose (fuzz.rs lines 71–89)
   The series of if overrides.X.is_some() { tx.X = overrides.X } blocks are correct but can be collapsed:

   tx.gas = overrides.gas.or(tx.gas);
   tx.max_fee_per_gas = overrides.max_fee_per_gas.or(tx.max_fee_per_gas);
   // etc.

6. WAIT sleep (1 s) after shutdown signal is fragile (fuzz.rs line 27, 474)
   A fixed 1-second delay before the test exits gives no guarantee the child process has actually died, especially on slow CI runners. Consider adding a one-shot acknowledgment channel that the child's kill-and-exit task sends before the test returns.

---

Positive Notes

• Good structure: hardcoded edge-cases first, then random fuzz cases, with assert_node_alive health-checks separating each category.
• The catch_unwind + resume_unwind pattern correctly ensures the shutdown signal is sent even when assertions fail inside the test body.
• Covering all three transaction types (legacy-via-EIP-1559, seismic 0x4a, signature-corrupted) in a single test gives broad rejection-path coverage.
• The helper functions (corrupt_last_n, corrupt_tail_random) are clean and reusable.
• Using trace\! for send outcomes keeps CI output clean while still being available with RUST_LOG=trace.
• send_malformed_raw_bytes correctly tests the RLP/EIP-2718 decoder boundary with the full range of type-prefix bytes including 0x4A.