RocksDbStorage metadata counters have read-modify-write race conditions

[huitseeker]

The insert_value and remove_value methods perform read-modify-write operations on leaf_count and entry_count metadata without proper synchronization. Concurrent modifications can cause these counters to become corrupted through lost updates.

The root cause is using separate read and write operations instead of atomic transactions. In miden-crypto/src/merkle/smt/large/storage/rocksdb.rs:318-360, the code reads current counts (lines 318-319), modifies them locally (lines 335, 345-347), and writes them back in a batch (lines 356-357). If two threads execute concurrently:

1. Thread A reads entry_count = 5
2. Thread B reads entry_count = 5
3. Thread A increments to 6 and writes
4. Thread B increments to 6 and writes (should be 7)

Result: entry_count = 6 instead of the correct value 7.

Fixing this requires investigation of RocksDB's synchronization and concurrency primitives to determine the right approach. Options include merge operators for atomic counter updates, compare-and-swap operations, RocksDB transactions, or external synchronization if the storage is shared across threads. The investigation should clarify the intended concurrency model and select appropriate RocksDB features.

[bobbinth]

Note that with #1499, RocksDB backend will move into miden-node. So, this issue may be more appropriate there.

[huitseeker]

This issue should move to 0xMiden/node. The RocksDB LargeSmt backend is being copied there in PR #1499, and the same leaf_count/entry_count read-modify-write logic lives in that new crate.

[krushimir]

Thanks for raising this - and sorry for the confusion.

The intended model here is single-writer: mutating paths require &mut self (and LargeSmt itself is not Clone), so if the structure is shared across threads, mutation is expected to be externally synchronized.

The real footgun is RocksDbStorage: Clone (and similarly MemoryStorage: Clone), which allows creating multiple independent mutable handles and makes this look concurrency-safe when it isn’t intended to be.

I tested removing Clone from both storages, and everything still compiles (including tests/benches), so I think enforcing the single-writer model at the type level is the safest/cleanest fix for now.

This addresses the issue for the supported single-writer model; intentionally unsynchronized multi-writer access (outside that contract) can still produce read-modify-write races.