Skip to content

fix(store/state): perform potentially blocking RocksDB access in a blocking context #2076

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kkovaacs merged 5 commits into from
May 18, 2026
Merged

fix(store/state): perform potentially blocking RocksDB access in a blocking context #2076

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cd2971c
fix(store/state): perform nullifier and account tree access in a bloc…
kkovaacs May 14, 2026
7a3c458
fix(store/state): perform account state forest access in a blocking task
kkovaacs May 14, 2026
9be817d
chore: update changelog
kkovaacs May 14, 2026
9531ca7
Merge remote-tracking branch 'origin/next' into krisztian/move-store-…
kkovaacs May 15, 2026
9068823
Merge branch 'next' into krisztian/move-store-rocksdb-access-to-block…
kkovaacs May 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
- [BREAKING] Renamed `--url` CLI flags and `*_URL` env vars to `--listen` / `*_LISTEN` across all components.
- [BREAKING] Removed `miden-node validator` subcommand and created a separate `miden-validator` binary ([#2053](https://github.com/0xMiden/node/pull/2053)).
- [BREAKING] Removed `miden-node ntx-builder` subcommand and created a separate `miden-ntx-builder` binary ([#2067](https://github.com/0xMiden/node/pull/2067)).
- Replaced blocking-in-async LargeSmt and account state forest operations in the store with wrappers using Tokio's `block_in_place()` ([#2076](https://github.com/0xMiden/node/pull/2076)).

## v0.14.10 (2026-05-29)

Expand Down
2 changes: 0 additions & 2 deletions crates/store/src/server/ntx_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,6 @@ impl ntx_builder_server::NtxBuilder for StoreApi {
let asset_witnesses = self
.state
.get_vault_asset_witnesses(account_id, block_num, vault_keys)
.await
.map_err(internal_error)?;

// Convert AssetWitness to protobuf format by extracting witness data.
Expand Down Expand Up @@ -307,7 +306,6 @@ impl ntx_builder_server::NtxBuilder for StoreApi {
let storage_witness = self
.state
.get_storage_map_witness(account_id, &slot_name, block_num, map_key)
.await
.map_err(internal_error)?;

// Convert StorageMapWitness to protobuf format by extracting witness data.
Expand Down
158 changes: 70 additions & 88 deletions crates/store/src/state/apply_block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,17 +123,7 @@ impl State {
// Awaiting the block saving task to complete without errors.
block_save_task.await??;

// Scope to update the in-memory data.
async move {
// We need to hold the write lock here to prevent inconsistency between the in-memory
// state and the DB state. Thus, we need to wait for the DB update task to complete
// successfully.
let mut inner = self
.inner
.write()
.instrument(info_span!(target: COMPONENT, "acquire_inner_write_lock"))
.await;

self.with_inner_write_blocking(|inner| {
// We need to check that neither the nullifier tree nor the account tree have changed
// while we were waiting for the DB preparation task to complete. If either of them
// did change, we do not proceed with in-memory and database updates, since it may
Expand All @@ -154,8 +144,8 @@ impl State {
// Await for successful commit of the DB transaction. If the commit fails, we mustn't
// change in-memory state, so we return a block applying error and don't proceed with
// in-memory updates.
db_update_task
.await?
tokio::runtime::Handle::current()
.block_on(db_update_task)?
Comment on lines +134 to +135
Copy link
Copy Markdown
Collaborator

@Mirko-von-Leipzig Mirko-von-Leipzig May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be using block_on? I guess we're sort of saved because we know this will only happen once, and always sequentially?

Copy link
Copy Markdown
Contributor Author

@kkovaacs kkovaacs May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative would be using a oneshot channel for explicit synchronization on the database commit -- that would avoid this ugly block_on() call here.

Copy link
Copy Markdown
Collaborator

@Mirko-von-Leipzig Mirko-von-Leipzig May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can live with these as is until we get proper write isolation.

.map_err(|err| ApplyBlockError::DbUpdateTaskFailed(err.as_report()))?;

// Update the in-memory data structures after successful commit of the DB transaction
Expand All @@ -171,15 +161,11 @@ impl State {
inner.blockchain.push(block_commitment);

Ok(())
}
.in_current_span()
.await?;
})?;

self.forest
.write()
.instrument(info_span!(target: COMPONENT, "acquire_forest_write_lock"))
.await
.apply_block_updates(block_num, account_deltas)?;
self.with_forest_write_blocking(|forest| {
forest.apply_block_updates(block_num, account_deltas)
})?;

// Push to cache and notify replica subscribers.
self.block_cache.push(block_num, BlockNotification::new(block_num, cache_bytes));
Expand Down Expand Up @@ -237,78 +223,74 @@ impl State {
header: &BlockHeader,
body: &BlockBody,
) -> Result<(Word, NullifierMutationSet, Word, AccountMutationSet), ApplyBlockError> {
let inner = self
.inner
.read()
.instrument(info_span!(target: COMPONENT, "acquire_inner_read_lock"))
.await;

let block_num = header.block_num();

// nullifiers can be produced only once
let duplicate_nullifiers: Vec<_> = body
.created_nullifiers()
.iter()
.filter(|&nullifier| inner.nullifier_tree.get_block_num(nullifier).is_some())
.copied()
.collect();
if !duplicate_nullifiers.is_empty() {
return Err(InvalidBlockError::DuplicatedNullifiers(duplicate_nullifiers).into());
}
self.with_inner_read_blocking(|inner| {
let block_num = header.block_num();

// nullifiers can be produced only once
let duplicate_nullifiers: Vec<_> = body
.created_nullifiers()
.iter()
.filter(|&nullifier| inner.nullifier_tree.get_block_num(nullifier).is_some())
.copied()
.collect();
if !duplicate_nullifiers.is_empty() {
return Err(InvalidBlockError::DuplicatedNullifiers(duplicate_nullifiers).into());
}

// new_block.chain_root must be equal to the chain MMR root prior to the update
let peaks = inner.blockchain.peaks();
if peaks.hash_peaks() != header.chain_commitment() {
return Err(InvalidBlockError::NewBlockInvalidChainCommitment.into());
}
// new_block.chain_root must be equal to the chain MMR root prior to the update
let peaks = inner.blockchain.peaks();
if peaks.hash_peaks() != header.chain_commitment() {
return Err(InvalidBlockError::NewBlockInvalidChainCommitment.into());
}

// compute update for nullifier tree
let nullifier_tree_update = inner
.nullifier_tree
.compute_mutations(
body.created_nullifiers().iter().map(|nullifier| (*nullifier, block_num)),
)
.map_err(InvalidBlockError::NewBlockNullifierAlreadySpent)?;

if nullifier_tree_update.as_mutation_set().root() != header.nullifier_root() {
// We do our best here to notify the serve routine, if it doesn't care (dropped the
// receiver) we can't do much.
let _ = self.termination_ask.try_send(ApplyBlockError::InvalidBlockError(
InvalidBlockError::NewBlockInvalidNullifierRoot,
));
return Err(InvalidBlockError::NewBlockInvalidNullifierRoot.into());
}
// compute update for nullifier tree
let nullifier_tree_update = inner
.nullifier_tree
.compute_mutations(
body.created_nullifiers().iter().map(|nullifier| (*nullifier, block_num)),
)
.map_err(InvalidBlockError::NewBlockNullifierAlreadySpent)?;

if nullifier_tree_update.as_mutation_set().root() != header.nullifier_root() {
// We do our best here to notify the serve routine, if it doesn't care (dropped the
// receiver) we can't do much.
let _ = self.termination_ask.try_send(ApplyBlockError::InvalidBlockError(
InvalidBlockError::NewBlockInvalidNullifierRoot,
));
return Err(InvalidBlockError::NewBlockInvalidNullifierRoot.into());
}

// compute update for account tree
let account_tree_update = inner
.account_tree
.compute_mutations(
body.updated_accounts()
.iter()
.map(|update| (update.account_id(), update.final_state_commitment())),
)
.map_err(|e| match e {
HistoricalError::AccountTreeError(err) => {
InvalidBlockError::NewBlockDuplicateAccountIdPrefix(err)
},
HistoricalError::MerkleError(_) => {
panic!("Unexpected MerkleError during account tree mutation computation")
},
})?;
// compute update for account tree
let account_tree_update = inner
.account_tree
.compute_mutations(
body.updated_accounts()
.iter()
.map(|update| (update.account_id(), update.final_state_commitment())),
)
.map_err(|e| match e {
HistoricalError::AccountTreeError(err) => {
InvalidBlockError::NewBlockDuplicateAccountIdPrefix(err)
},
HistoricalError::MerkleError(_) => {
panic!("Unexpected MerkleError during account tree mutation computation")
},
})?;

if account_tree_update.as_mutation_set().root() != header.account_root() {
let _ = self.termination_ask.try_send(ApplyBlockError::InvalidBlockError(
InvalidBlockError::NewBlockInvalidAccountRoot,
));
return Err(InvalidBlockError::NewBlockInvalidAccountRoot.into());
}
if account_tree_update.as_mutation_set().root() != header.account_root() {
let _ = self.termination_ask.try_send(ApplyBlockError::InvalidBlockError(
InvalidBlockError::NewBlockInvalidAccountRoot,
));
return Err(InvalidBlockError::NewBlockInvalidAccountRoot.into());
}

Ok((
inner.nullifier_tree.root(),
nullifier_tree_update,
inner.account_tree.root_latest(),
account_tree_update,
))
Ok((
inner.nullifier_tree.root(),
nullifier_tree_update,
inner.account_tree.root_latest(),
account_tree_update,
))
})
}

/// Builds note records with inclusion proofs from the block body.
Expand Down
Loading
Loading