feat: Lock-free apply_block refactor by sergerad · Pull Request #2345 · 0xMiden/node

sergerad · 2026-07-15T03:05:41Z

Summary

Closes #1539.

Closes #1853.

Makes the store's block-write path lock-free for readers, and makes the resulting read-consistency rules type-enforced. Reads previously contended on RwLocks over the in-memory trees (and were blocked during the DB-commit window of apply_block); they now load an immutable snapshot via ArcSwap and are never blocked by writes. All reads flow through a request-scoped StateView, so a query that combines tree and DB data at different chain heights is no longer expressible.

Why:

Read endpoints (sync, account/nullifier proofs, chain tip) no longer stall while a block is being applied.
Removes the cross-task lock choreography in apply_block (oneshot handshakes between the DB task and the in-memory update), which was fragile and hard to reason about.
Snapshot-scoped reads were previously enforced by convention (comments and caller-side checks); several paths read the tip and the data from different snapshots. The StateView makes the scoping structural.

How:

Lock-free write path (state/writer/):

A single WriteWorker task owns the mutable nullifier tree, account tree, blockchain MMR, and account-state forest, processing blocks serially from an mpsc channel — no locks anywhere. In-flight writes always complete: shutdown is only observed between requests.
After each DB commit the worker builds an immutable StateSnapshot (trees backed by read-only RocksDB snapshot views) and publishes it atomically via ArcSwap. Readers keep a consistent frozen view even while the next block commits.
Db::apply_block is now a plain transaction — the oneshot allow_acquire/acquire_done synchronization is removed.

Write capabilities (state/lifecycle.rs):

LoadedState::start spawns the worker and returns the read-only Arc<State> plus non-cloneable BlockWriter and ProofWriter capabilities and a WriterTask join handle — statically limiting each write path to one task. The capabilities expose no read access; tasks that read and write receive the Arc<State> alongside their capability.
BlockWriter::stop drains and joins the worker so tree storage is released deterministically before the data directory is re-opened or deleted (used by recover and stress-test seeding).

Type-enforced reads (state/view/):

All tree and DB reads live on StateView, pinned to one snapshot per request (State::view()). DB queries are scoped by the view's tip internally; callers cannot supply their own. RocksDB-backed trees are only reachable through block_in_place helpers; snapshot fields are only visible inside the view module.
Tip-scoped Db queries require view-issued proof types (ScopedBlockNum / ScopedBlockRange), constructible only by a StateView after validating the bound against its tip — extending the enforcement to the DB boundary itself.
Range-scoped sync queries validate range.end() <= tip themselves via a new RangeBeyondTip error (same InvalidArgument response as before); the RPC layer's range_bounds_check is deleted and pagination's chain_tip is now the tip the query actually ran against.
Fixes paths that previously took two snapshots per request (get_account, the block producer's get_tx_inputs); sync_chain_mmr clamps the proven tip to the view's tip.

Live tips (state/tip.rs):

State::committed_tip() / proven_tip() read the watch channels their writers publish to (mirroring subscribe_committed_tip / subscribe_proven_tip); the Finality enum is removed. The committed tip is published after the snapshot, so it never reports a block a fresh view cannot serve.

Observability: SnapshotGuard tracks live snapshot generations and lifetimes; warns when a snapshot outlives 10s or more than 4 generations are pinned (a leaked/slow reader pins a RocksDB snapshot).

Supporting changes: read-only reader() views for AccountStateForest / AccountTreeWithHistory (relaxed to BackendReader/SmtStorageReader bounds); state module restructured into view/ (read endpoints) and writer/ (worker + capabilities); new tracing field names allowlisted.

Changelog

[[entry]]
scope       = "node"
impact      = "changed"
description = "Store reads are lock-free: readers use atomically published in-memory snapshots and are no longer blocked while blocks are applied."

…ckfree-store-state

This reverts commit c528f19.

…ckfree-store-state

Mirko-von-Leipzig

LGTM, thank you; this is already much better.

I left some comments regarding splitting the read and write portion sooner somehow; but thinking on it some more I'm not sure its tractable to implement at the moment.

Mirko-von-Leipzig · 2026-07-28T08:59:55Z

+    pub fn reader(&self) -> AccountTreeWithHistory<S::Reader> {
+        let latest = self.latest.reader().expect("snapshot creation should not fail");
+        AccountTreeWithHistory {
+            block_number: self.block_number,
+            latest,
+            overlays: self.overlays.clone(),
+        }
+    }


I think the overall API shape here is incorrect, but perhaps that can be part of the followup.

What I would like is a guarantee that readers can never access writing. But with this API everybody has access to AccountTreeWithHistory and can therefore invoke apply_mutations if they so wish.

Something similar to what I suggested here #2353 (comment).

i.e. on Smt::load you get a single SmtWriter (non-clone), and a SmtReader. This would go together with a single WriteConnection and a ReadOnlyConnectionPool for the database.

The write task would get StateWriter (1 db connection, 1 smt writer per smt), and everything else gets ReadOnlyState for queries. This way we guarantee very early on that nothing can escalate their privilege.

So ito this code, reader would become SmtReader::snapshot(BlockNumber) -> AccountTreeSnapshot, or something similar.

What I would like is a guarantee that readers can never access writing. But with this API everybody has access to AccountTreeWithHistory and can therefore invoke apply_mutations if they so wish.

The apply_mutations fn lives in impl<S: SmtStorage> AccountTreeWithHistory<S> { so it is not callable from the AccountTreeWithHistory<SmtStorageReader> which is returned by the reader() here.

Yes but readers already have access to super struct i.e. they could elect to write.

I don't want to audit this by reading every read query and checking they do the correct thing.

Ideally, right at the very start, before anything even gets launched, we already split things up. But perhaps that isn't possible because apply_block method lives together with the read methods so we cannot do that.

As an example, if we had the write methods in a separate gRPC service, we could give the write service state the write struct. And the readers would only ever get the read struct. But this isn't possible unless we split the services, which seems a bit silly (? perhaps ?).

But lets ignore this for.

Have unresolved to keep track of this. Might add to a stacked PR.

Mirko-von-Leipzig · 2026-07-28T09:10:20Z

I would create a separate file per function instead.

I also didn't review the contents under the assumption that this was a copy-paste move.

Mirko-von-Leipzig · 2026-07-28T09:47:58Z

+#[must_use = "call `start` to spawn the block writer and obtain the state"]
+pub struct LoadedState {
+    state: State,
+    writer: BlockWriter,
+}
+
+impl LoadedState {
+    /// Spawns the block writer onto the current runtime and returns the state together with the
+    /// writer task's handle.
+    ///
+    /// The writer exits once the shutdown token passed to [`State::load`] is cancelled or the last
+    /// state reference (holding the only write handle) is dropped — an in-flight block write
+    /// always completes first. Awaiting the returned handle after either event guarantees the
+    /// writer has released the tree storage it owns; a join error carries a writer panic.
+    ///
+    /// Callers without a token to cancel should stop the store via [`State::stop`] rather than
+    /// dropping and joining by hand.
+    pub fn start(self) -> (Arc<State>, WriterTask) {
+        let writer_task = tokio::spawn(self.writer.run());
+        (Arc::new(self.state), WriterTask(writer_task))
+    }
+}
+
+// WRITER TASK
+// ================================================================================================
+
+/// Handle of the store's block writer task, returned by [`LoadedState::start`].
+///
+/// Awaiting it resolves once the writer has exited and released the tree storage it owns; a join
+/// error carries a writer panic. The newtype ensures [`State::stop`] can only be given the store's
+/// own writer task, and deliberately does not expose [`tokio::task::JoinHandle::abort`]: aborting
+/// the writer mid-write could leave the trees lagging the committed database state, voiding the
+/// guarantee that an in-flight block write always completes.
+#[must_use = "await the writer task to observe its exit, or pass it to `State::stop`"]
+pub struct WriterTask(tokio::task::JoinHandle<()>);
+
+impl Future for WriterTask {
+    type Output = Result<(), tokio::task::JoinError>;
+
+    fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
+        Pin::new(&mut self.0).poll(cx)
+    }
+}


As mentioned; I think this is the wrong abstraction. I haven't checked how WriterTask works yet though.

I feel like State::load could return ReadOnlyState(Arc<State>) + StateWriter. The task spawning etc and what that means in various contexts should be more obvious at the call site, and not hidden imo.

e.g. I would expect to see, at the command level:

full node's to pass WriteState to an sync-via-rpc thingy, and

sequencer to pass it.. to apply_block somehow.

Have updated to use a readonly State with BlockWriter and ProofWriter

Mirko-von-Leipzig · 2026-07-29T06:36:25Z

+/// Readers are expected to be request-scoped, so snapshot lifetimes should be well under a block
+/// interval. A lifetime exceeding [`SNAPSHOT_LIFETIME_WARN_THRESHOLD`] is logged at warn level.
+pub(crate) struct SnapshotGuard {
+    live: Arc<AtomicUsize>,


This could probably be

Suggested change

live: Arc<AtomicUsize>,

live: Arc<()>,

because count is tracked by arc already via Arc::strong_count.

I think I prefer AtomicUsize for clarity. The strong_count approach feels a bit too implicit. And potentially our warn/debug logs could be misleading / erroneous regarding the live count if multiple drops occur concurrently?

One would have to prematurely "drop" the arc e.g.:

let count = live.downgrade().strong_count();

I figured you'd want to actually store something in the Arc eventually. But if you don't like relying on arc semantics, then lets not block on this. It just stood out to me that we were replicating arc internals.

Mirko-von-Leipzig · 2026-07-29T06:37:08Z

Thank you, I think this is really cool!

Mirko-von-Leipzig · 2026-07-29T06:41:37Z

+        if snapshots_live > SNAPSHOTS_LIVE_WARN_THRESHOLD {
+            tracing::warn!(
+                target: COMPONENT,
+                block_num = block_num.as_u32(),
+                snapshots.live = snapshots_live,
+                "too many live state snapshots; slow readers are pinning old generations",
+            );
+        }


I like this.

I would like to explore some additional stuff we could diagnose here. For example if we could figure out a way to have each snapshot given the read request name as a static string.

And that here we could lookup the oldest snapshot and log that.

An alternative to this would be including a timeout task to each snapshot, which automatically does this logging instead. And the timeout task gets cancelled/aborted when the snapshot is dropped.

But lets leave that for a follow-up PR to explore this space.

Mirko-von-Leipzig · 2026-07-29T06:42:52Z

I didn't inspect this code too deeply; @kkovaacs if you could perhaps do a more thorough job here.

Mirko-von-Leipzig

Blocking merge until we are done with the demo

…ckfree-store-state

sergerad added 18 commits July 15, 2026 15:04

Initial apply_block lock-free refactor

7195581

Add snapshot count observability

5c7dbeb

Merge branch 'next' of github.com:0xMiden/miden-node into sergerad-lo…

b7699fc

…ckfree-store-state

Fix ui tests

46b8df6

Lint

bb3cf85

Warn about long lived snapshots

891d702

Warn on snapshot count

8c2bc02

Scape all requests

8373d9b

Lint

3a27431

Merge branch 'next' of github.com:0xMiden/miden-node into sergerad-lo…

8fa1502

…ckfree-store-state

Replace result with control flow

c21a950

Compartmentalize apply_block

0b562a8

snapshots_live fn

ddf4f15

Shutdown writer

6556067

Lint

2ad2bbf

Merge branch 'next' of github.com:0xMiden/miden-node into sergerad-lo…

09b11f8

…ckfree-store-state

Fix comments

dc1e08a

Lint

d7e6c52

sergerad requested review from Mirko-von-Leipzig and kkovaacs July 23, 2026 01:18

sergerad marked this pull request as ready for review July 23, 2026 01:18

sergerad added 8 commits July 23, 2026 09:31

Add branch comment

d14362e

Fix shutdown

d6bf7bd

Shutdown integration without token

220f911

integrate cancellation token

8a531c2

Refactor writer task management

3b648ff

Split module

aea62ce

WriterTask

7f8ebc5

Add benchmarks

c528f19

sergerad mentioned this pull request Jul 23, 2026

test: Benchmarks for lock free apply_block #2370

Merged

sergerad added 5 commits July 24, 2026 08:12

Revert "Add benchmarks"

3ceabd1

This reverts commit c528f19.

Merge branch 'next' of github.com:0xMiden/miden-node into sergerad-lo…

0a7f05a

…ckfree-store-state

Merge branch 'next' of github.com:0xMiden/miden-node into sergerad-lo…

927a777

…ckfree-store-state

Llint

974aa12

Merge branch 'next' of github.com:0xMiden/miden-node into sergerad-lo…

29765f0

…ckfree-store-state

Mirko-von-Leipzig approved these changes Jul 29, 2026

View reviewed changes

Mirko-von-Leipzig self-requested a review July 29, 2026 07:03

sergerad mentioned this pull request Jul 29, 2026

Dedicated core allocation for apply_block #2375

Open

Mirko-von-Leipzig requested review from Mirko-von-Leipzig and removed request for Mirko-von-Leipzig July 29, 2026 08:20

Mirko-von-Leipzig requested changes Jul 29, 2026

View reviewed changes

sergerad added 4 commits July 30, 2026 11:57

RM deadcode

5b6f022

State::for_tests

4369f8d

BlockWriter and WriteWorker

858ee24

Merge branch 'next' of github.com:0xMiden/miden-node into sergerad-lo…

9b40e4b

…ckfree-store-state

sergerad mentioned this pull request Jul 30, 2026

Enforce block-scoped DB reads through a view type #2414

Open

sergerad and others added 4 commits July 31, 2026 09:36

Merge branch 'next' of github.com:0xMiden/miden-node into sergerad-lo…

d49626f

…ckfree-store-state

StateView (#2415)

082ed21

Add ScopedBlockNum

d9b0f04

Lint

db5fe1e

Uh oh!

Conversation

sergerad commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changelog

Uh oh!

Mirko-von-Leipzig left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sergerad Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mirko-von-Leipzig left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sergerad commented Jul 15, 2026 •

edited

Loading

sergerad Jul 30, 2026 •

edited

Loading