fix: serialize layered DB write transaxtions

[kstoykov]

Summary

• Serialize LayeredDatabase write transactions through the mem_db write lock instead of refcounting overlapping transactions in the background writer.
• Enforce the one-open-write-txn protocol: StartTxn while a txn is already open and CommitTxn with no open txn now poison the writer instead of leaving ambiguous transaction state.
• Update transactional remove handling and cold-producer comments so writer-side operations do not re-enter the producer-held mem lock.

Closes # — no linked issue.

Surface areas touched

• Consensus protocol (primary / worker / network / state-sync)
• Execution / EVM
• JSON-RPC (eth_*, rayls_*, faucet)
• Middleware (orchestrator / processor / bridge)
• Infrastructure (types / storage / config / network-cli)
• On-chain contracts (rayls-contracts/)
• Operations (etc/, scripts, Docker, compose)
• CI / build (.github/workflows/, Makefile)
• Documentation only (doc/, in-crate READMEs, root docs)
• Tests only

Breaking / compatibility

None.

Test plan

• cargo fmt --check
• cargo clippy -p rayls-infrastructure-storage --features redb --all-targets
• cargo test -p rayls-infrastructure-storage --features redb
• cargo check --workspace --all-targets
• Targeted regression tests:
  • dropped_write_txn_poisons_the_next_write_txn
  • write_txn_serializes_writers_until_commit
  • txn_remove_survives_sync_persist_before_commit
  • txn_remove_and_insert_apply_in_channel_order

[github-actions]

Security Scan - Code

Severity: HIGH, CRITICAL

No vulnerabilities found

View scan results

Report Summary

┌─────────────────────────────────────────────┬────────────┬─────────────────┬───────────────────┐
│                   Target                    │    Type    │ Vulnerabilities │ Misconfigurations │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ Cargo.lock                                  │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ crates/testing/fuzz-targets/Cargo.lock      │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ crates/testing/fuzz-targets/fuzz/Cargo.lock │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/state-sum/Cargo.lock                    │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/tps/package-lock.json                   │    npm     │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ rayls-contracts/package-lock.json           │    npm     │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/chaos-network/Dockerfile                │ dockerfile │        -        │         0         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/docker-network/Dockerfile               │ dockerfile │        -        │         0         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/docker-replay/Dockerfile                │ dockerfile │        -        │         0         │
└─────────────────────────────────────────────┴────────────┴─────────────────┴───────────────────┘
Legend:
- '-': Not scanned
- '0': Clean (no security findings detected)

[github-actions]

Claude finished @kstoykov's task in 4m 7s —— View job

---

PR Review: fix: serialize layered DB write transactions

• Gathering context and reading changed files
• Analyzing layered_db.rs changes (serialization, protocol enforcement, eviction)
• Analyzing lib.rs, mem_db.rs and cold/producer.rs changes
• Reviewing tests
• Posting final review

---

Overall Assessment

This is a well-reasoned, correctness-critical refactor. It fixes two latent bugs (a deadlock and a transactional ordering hazard) while simplifying the transaction model from a fragile refcount to a strict mutex-based serialization. The tests are comprehensive and the comments accurately describe the invariants.

---

Core Design Analysis

Serialization mechanism (write_txn / commit ordering, layered_db.rs:1294-1301, layered_db.rs:635-648)

The key insight: acquiring the mem write lock before enqueuing StartTxn and enqueuing CommitTxn before releasing the lock creates a hard guarantee that the writer's channel will always see StartTxn → ops → CommitTxn as a contiguous, non-overlapping sequence. This correctly replaces the old refcount design that allowed multiple logical transactions to share one MDBX transaction.

Deadlock fix in remove_txn (layered_db.rs:1615-1618)

The removal of mem_db.contains_key() from remove_txn (and remove_txn for KeyRemoveBatch) is a genuine deadlock fix. The producer holds the parking_lot write lock for the entire transaction duration. The writer calling mem_db.contains_key() would try to acquire the read lock, which parking_lot::RwLock blocks until no writers hold it — deadlock. The new code applies ops in strict channel order within the MDBX transaction, which is semantically equivalent for well-formed txn sequences.

Eviction guard (layered_db.rs:789-808)

Correct. mem_db.evict_if_needed() takes the write lock (mem_db.rs:490). By returning early from evict_and_log when has_open_txn=true, the writer never contends the lock the producer holds.

---

Issues Found

1. Fragile infallibility assumption in commit (layered_db.rs:639-648)

// Enqueue the commit before releasing the mem write lock...
// This is safe because `MemDbTxMut::commit` is an infallible no-op that only drops the
// `parking_lot` write guard; it performs no I/O. If that ever becomes fallible, do not
// enqueue `CommitTxn` before the mem commit can fail.
self.tx.send(DBMessage::CommitTxn).map_err(|_| eyre::eyre!("DB thread gone, FATAL!"))?;
self.mem_db.commit()

The comment documents this correctly, but there is no compiler-level enforcement. If MemDbTxMut::commit ever gains fallibility, a future author may not see this comment in time. Consider adding a const _: () = assert!(...); or a doc-test that panics on error, or at minimum an unreachable!() path in MemDbTxMut::commit. This is a low-severity concern but it would be painful to debug. Fix this →

2. write_txn can acquire the write lock then fail sending StartTxn (layered_db.rs:1298-1300)

let mem_db = self.mem_db.write_txn()?;
self.tx.send(DBMessage::StartTxn).map_err(|_| eyre::eyre!("DB thread gone, FATAL!"))?;

If send(StartTxn) fails (writer thread gone), mem_db (which holds the write guard) is dropped and the lock is released — correct. However, this error path does NOT send a CommitTxn, so the writer (if it's still alive but temporarily slow) could process a StartTxn with no matching CommitTxn, which would only be resolved on the next successful write_txn/CommitTxn — or it would poison the DB when the next StartTxn arrives. In practice, when send fails the writer is gone, so this is a safe no-op, but the comment "DB thread gone, FATAL!" could be clarified to note no orphan CommitTxn is needed. Minor documentation concern.

3. write_txn_serializes_writers_until_commit timing sensitivity (layered_db.rs:3049-3055)

second_rx.recv_timeout(std::time::Duration::from_millis(500)),
Err(std::sync::mpsc::RecvTimeoutError::Timeout)

500ms is a negative assertion (proving a thread is blocked). This is reasonable on typical hardware but could be flaky on memory-constrained CI runners where thread scheduling can be very slow. Consider using a condvar or a smaller sentinel to confirm the second thread has started trying to acquire the lock before the timeout check, making the test less dependent on scheduling latency. This is a low-risk concern given the current 500ms margin.

4. remove_txn removes keys that may not exist in the MDBX transaction (layered_db.rs:1616-1618)

fn remove_txn(&self, txn: &mut <DB as Database>::TXMut<'_>) -> eyre::Result<()> {
    txn.remove::<T>(&self.key)
}

Without the old contains_key guard, remove_txn now issues a no-op MDBX delete if the key was never in the persistent layer. Both redb and mdbx treat deleting a non-existent key as Ok(()), so this is safe in practice. Worth noting the assumption is now implicit. The txn_remove_and_insert_apply_in_channel_order test covers the ordering semantics, but there's no explicit test that a stray remove_txn on a never-inserted key doesn't produce an error. This is informational — no action required.

---

Minor / Nits

• The non-storage files (chainspec.rs, block.rs, admin_transfer.rs, faucet.rs, restarts.rs) are purely cargo fmt comment-rewrap changes. No correctness impact.

• The WRITE_BATCH_ROWS comment update in cold/producer.rs correctly describes the new behavior: each with_write_txn is now a real commit point because the mem-lock serialization prevents merging. The old comment ("refcounts overlapping transactions") was stale and could have misled future readers of the archiver; removing it is good.

• The evict_and_log log field change from open_txns (usize) to has_open_txn (bool) is a breaking metrics/log field rename. If any monitoring dashboards or log parsers key on the old open_txns field name, they will need updating.

---

Tests

The four new regression tests cover exactly the right scenarios:

• dropped_write_txn_poisons_the_next_write_txn: protocol violation (double StartTxn) → poison ✓
• write_txn_serializes_writers_until_commit: mutual exclusion via mem lock ✓
• txn_remove_survives_sync_persist_before_commit: barrier during open txn does not deadlock ✓
• txn_remove_and_insert_apply_in_channel_order: channel-order semantics for remove/insert within txn ✓

---

Summary

The serialization correctness, the deadlock fix in remove_txn, and the eviction guard are all sound. The most actionable item is the fragile infallibility assumption (finding 1) — worth a small guard to prevent future regression. Everything else is low-severity or informational.