From 1085eb00431d67fe223785b80a9b70486953ef17 Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Wed, 11 Feb 2026 12:12:25 -0300 Subject: [PATCH 01/15] chore: replace in memory with sqlite database fix: proper state transitions chore: move transactions into Db methods chore: remove dead code chore: add raw SQL queries to docs chore: use source deserialization errors --- CHANGELOG.md | 1 + Cargo.lock | 3 +- bin/node/src/commands/bundled.rs | 1 + bin/node/src/commands/mod.rs | 25 +- crates/ntx-builder/Cargo.toml | 5 +- .../ntx-builder/src/actor/account_effect.rs | 73 ++ crates/ntx-builder/src/actor/account_state.rs | 688 +----------------- crates/ntx-builder/src/actor/inflight_note.rs | 9 + crates/ntx-builder/src/actor/mod.rs | 157 ++-- crates/ntx-builder/src/actor/note_state.rs | 235 ------ crates/ntx-builder/src/builder.rs | 73 +- crates/ntx-builder/src/coordinator.rs | 107 +-- crates/ntx-builder/src/db/errors.rs | 18 +- crates/ntx-builder/src/db/manager.rs | 5 + .../db/migrations/2026020900000_setup/up.sql | 3 + crates/ntx-builder/src/db/mod.rs | 166 ++++- crates/ntx-builder/src/db/models/conv.rs | 79 ++ crates/ntx-builder/src/db/models/mod.rs | 3 + .../src/db/models/queries/accounts.rs | 92 +++ .../src/db/models/queries/chain_state.rs | 49 ++ .../ntx-builder/src/db/models/queries/mod.rs | 316 ++++++++ .../src/db/models/queries/notes.rs | 193 +++++ .../src/db/models/queries/tests.rs | 553 ++++++++++++++ crates/ntx-builder/src/lib.rs | 34 +- 24 files changed, 1842 insertions(+), 1046 deletions(-) create mode 100644 crates/ntx-builder/src/actor/account_effect.rs delete mode 100644 crates/ntx-builder/src/actor/note_state.rs create mode 100644 crates/ntx-builder/src/db/models/conv.rs create mode 100644 crates/ntx-builder/src/db/models/mod.rs create mode 100644 crates/ntx-builder/src/db/models/queries/accounts.rs create mode 100644 crates/ntx-builder/src/db/models/queries/chain_state.rs create mode 100644 crates/ntx-builder/src/db/models/queries/mod.rs create mode 100644 crates/ntx-builder/src/db/models/queries/notes.rs create mode 100644 crates/ntx-builder/src/db/models/queries/tests.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 02b443a8e..110a89daa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ - Refactored NTX Builder actor state into `AccountDeltaTracker` and `NotePool` for clarity, and added tracing instrumentation to event broadcasting ([#1611](https://github.com/0xMiden/miden-node/pull/1611)). - Add #[track_caller] to tracing/logging helpers ([#1651](https://github.com/0xMiden/miden-node/pull/1651)). - Improved tracing span fields ([#1650](https://github.com/0xMiden/miden-node/pull/1650)) +- Replaced NTX Builder's in-memory state management with SQLite-backed persistence; account states, notes, and transaction effects are now stored in the database and inflight state is purged on startup ([#1662](https://github.com/0xMiden/miden-node/pull/1662)). ## v0.13.5 (TBD) diff --git a/Cargo.lock b/Cargo.lock index 52c342f35..3f2bc2258 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2812,7 +2812,6 @@ dependencies = [ "diesel", "diesel_migrations", "futures", - "indexmap 2.13.0", "miden-node-proto", "miden-node-test-macro", "miden-node-utils", @@ -2820,6 +2819,8 @@ dependencies = [ "miden-remote-prover-client", "miden-standards", "miden-tx", + "prost", + "rand_chacha 0.9.0", "rstest", "thiserror 2.0.18", "tokio", diff --git a/bin/node/src/commands/bundled.rs b/bin/node/src/commands/bundled.rs index 9cfc654b1..7b8feefe6 100644 --- a/bin/node/src/commands/bundled.rs +++ b/bin/node/src/commands/bundled.rs @@ -315,6 +315,7 @@ impl BundledCommand { store_ntx_builder_url, block_producer_url, validator_url, + &data_directory, ); let id = join_set diff --git a/bin/node/src/commands/mod.rs b/bin/node/src/commands/mod.rs index 5b1e8e52a..cacaf3087 100644 --- a/bin/node/src/commands/mod.rs +++ b/bin/node/src/commands/mod.rs @@ -1,4 +1,5 @@ use std::num::NonZeroUsize; +use std::path::{Path, PathBuf}; use std::time::Duration; use miden_node_block_producer::{ @@ -78,19 +79,37 @@ pub struct NtxBuilderConfig { default_value_t = DEFAULT_NTX_SCRIPT_CACHE_SIZE )] pub script_cache_size: NonZeroUsize, + + /// Directory for the ntx-builder's persistent database. + /// + /// If not set, defaults to the node's data directory. + #[arg(long = "ntx-builder.data-directory", value_name = "DIR")] + pub data_directory: Option, } impl NtxBuilderConfig { /// Converts this CLI config into the ntx-builder's internal config. + /// + /// The `node_data_directory` is used as the default location for the ntx-builder's database + /// if `--ntx-builder.data-directory` is not explicitly set. pub fn into_builder_config( self, store_url: Url, block_producer_url: Url, validator_url: Url, + node_data_directory: &Path, ) -> miden_node_ntx_builder::NtxBuilderConfig { - miden_node_ntx_builder::NtxBuilderConfig::new(store_url, block_producer_url, validator_url) - .with_tx_prover_url(self.tx_prover_url) - .with_script_cache_size(self.script_cache_size) + let data_dir = self.data_directory.unwrap_or_else(|| node_data_directory.to_path_buf()); + let database_filepath = data_dir.join("ntx-builder.sqlite3"); + + miden_node_ntx_builder::NtxBuilderConfig::new( + store_url, + block_producer_url, + validator_url, + database_filepath, + ) + .with_tx_prover_url(self.tx_prover_url) + .with_script_cache_size(self.script_cache_size) } } diff --git a/crates/ntx-builder/Cargo.toml b/crates/ntx-builder/Cargo.toml index e1d6dab84..b31278ddf 100644 --- a/crates/ntx-builder/Cargo.toml +++ b/crates/ntx-builder/Cargo.toml @@ -21,12 +21,12 @@ deadpool-sync = { features = ["tracing"], workspace = true } diesel = { features = ["numeric", "sqlite"], workspace = true } diesel_migrations = { features = ["sqlite"], workspace = true } futures = { workspace = true } -indexmap = { workspace = true } miden-node-proto = { workspace = true } miden-node-utils = { workspace = true } miden-protocol = { default-features = true, workspace = true } miden-remote-prover-client = { features = ["tx-prover"], workspace = true } miden-tx = { default-features = true, workspace = true } +prost = { workspace = true } thiserror = { workspace = true } tokio = { features = ["rt-multi-thread"], workspace = true } tokio-stream = { workspace = true } @@ -39,5 +39,6 @@ url = { workspace = true } miden-node-test-macro = { path = "../test-macro" } miden-node-utils = { features = ["testing"], workspace = true } miden-protocol = { default-features = true, features = ["testing"], workspace = true } -miden-standards = { workspace = true } +miden-standards = { features = ["testing"], workspace = true } +rand_chacha = { workspace = true } rstest = { workspace = true } diff --git a/crates/ntx-builder/src/actor/account_effect.rs b/crates/ntx-builder/src/actor/account_effect.rs new file mode 100644 index 000000000..e7d55ccb6 --- /dev/null +++ b/crates/ntx-builder/src/actor/account_effect.rs @@ -0,0 +1,73 @@ +use miden_node_proto::domain::account::NetworkAccountId; +use miden_protocol::account::delta::AccountUpdateDetails; +use miden_protocol::account::{Account, AccountDelta, AccountId}; + +// NETWORK ACCOUNT EFFECT +// ================================================================================================ + +/// Represents the effect of a transaction on a network account. +#[derive(Clone)] +pub enum NetworkAccountEffect { + Created(Account), + Updated(AccountDelta), +} + +impl NetworkAccountEffect { + pub fn from_protocol(update: &AccountUpdateDetails) -> Option { + let update = match update { + AccountUpdateDetails::Private => return None, + AccountUpdateDetails::Delta(update) if update.is_full_state() => { + NetworkAccountEffect::Created( + Account::try_from(update) + .expect("Account should be derivable by full state AccountDelta"), + ) + }, + AccountUpdateDetails::Delta(update) => NetworkAccountEffect::Updated(update.clone()), + }; + + update.protocol_account_id().is_network().then_some(update) + } + + pub fn network_account_id(&self) -> NetworkAccountId { + // SAFETY: This is a network account by construction. + self.protocol_account_id().try_into().unwrap() + } + + fn protocol_account_id(&self) -> AccountId { + match self { + NetworkAccountEffect::Created(acc) => acc.id(), + NetworkAccountEffect::Updated(delta) => delta.id(), + } + } +} + +#[cfg(test)] +mod tests { + use miden_protocol::block::BlockNumber; + + #[rstest::rstest] + #[test] + #[case::all_zero(Some(BlockNumber::GENESIS), BlockNumber::GENESIS, 0, true)] + #[case::no_attempts(None, BlockNumber::GENESIS, 0, true)] + #[case::one_attempt(Some(BlockNumber::GENESIS), BlockNumber::from(2), 1, true)] + #[case::three_attempts(Some(BlockNumber::GENESIS), BlockNumber::from(3), 3, true)] + #[case::ten_attempts(Some(BlockNumber::GENESIS), BlockNumber::from(13), 10, true)] + #[case::twenty_attempts(Some(BlockNumber::GENESIS), BlockNumber::from(149), 20, true)] + #[case::one_attempt_false(Some(BlockNumber::GENESIS), BlockNumber::from(1), 1, false)] + #[case::three_attempts_false(Some(BlockNumber::GENESIS), BlockNumber::from(2), 3, false)] + #[case::ten_attempts_false(Some(BlockNumber::GENESIS), BlockNumber::from(12), 10, false)] + #[case::twenty_attempts_false(Some(BlockNumber::GENESIS), BlockNumber::from(148), 20, false)] + fn backoff_has_passed( + #[case] last_attempt_block_num: Option, + #[case] current_block_num: BlockNumber, + #[case] attempt_count: usize, + #[case] backoff_should_have_passed: bool, + ) { + use crate::actor::has_backoff_passed; + + assert_eq!( + backoff_should_have_passed, + has_backoff_passed(current_block_num, last_attempt_block_num, attempt_count) + ); + } +} diff --git a/crates/ntx-builder/src/actor/account_state.rs b/crates/ntx-builder/src/actor/account_state.rs index b58cfd692..753dfee8a 100644 --- a/crates/ntx-builder/src/actor/account_state.rs +++ b/crates/ntx-builder/src/actor/account_state.rs @@ -1,24 +1,10 @@ -use std::collections::{BTreeMap, BTreeSet, HashSet}; -use std::num::NonZeroUsize; use std::sync::Arc; -use miden_node_proto::domain::account::NetworkAccountId; -use miden_node_proto::domain::mempool::MempoolEvent; -use miden_node_proto::domain::note::{NetworkNote, SingleTargetNetworkNote}; -use miden_node_utils::tracing::OpenTelemetrySpanExt; use miden_protocol::account::Account; -use miden_protocol::account::delta::AccountUpdateDetails; -use miden_protocol::block::{BlockHeader, BlockNumber}; -use miden_protocol::note::{Note, Nullifier}; -use miden_protocol::transaction::{PartialBlockchain, TransactionId}; -use tracing::instrument; +use miden_protocol::block::BlockHeader; +use miden_protocol::transaction::PartialBlockchain; -use super::ActorShutdownReason; -use super::note_state::{AccountDeltaTracker, NetworkAccountEffect, NotePool}; -use crate::COMPONENT; use crate::actor::inflight_note::InflightNetworkNote; -use crate::builder::ChainState; -use crate::store::{StoreClient, StoreError}; // TRANSACTION CANDIDATE // ================================================================================================ @@ -45,673 +31,3 @@ pub struct TransactionCandidate { /// Wrapped in `Arc` to avoid expensive clones when reading the chain state. pub chain_mmr: Arc, } - -// NETWORK ACCOUNT STATE -// ================================================================================================ - -/// The current state of a network account. -#[derive(Clone)] -pub struct NetworkAccountState { - /// The network account ID this state represents. - account_id: NetworkAccountId, - - /// Tracks committed and inflight account state updates. - account: AccountDeltaTracker, - - /// Manages available and nullified notes. - notes: NotePool, - - /// Uncommitted transactions which have some impact on the network state. - /// - /// This is tracked so we can commit or revert transaction effects. Transactions _without_ an - /// impact are ignored. - inflight_txs: BTreeMap, - - /// Nullifiers of all network notes targeted at this account. - /// - /// Used to filter mempool events: when a `TransactionAdded` event reports consumed nullifiers, - /// only those present in this set are processed. Nullifiers are added when notes are loaded - /// or created, and removed when the consuming transaction is committed. - known_nullifiers: HashSet, -} - -impl NetworkAccountState { - /// Load's all available network notes from the store, along with the required account states. - #[instrument(target = COMPONENT, name = "ntx.state.load", skip_all)] - pub async fn load( - account: Account, - account_id: NetworkAccountId, - store: &StoreClient, - block_num: BlockNumber, - ) -> Result { - let notes = store.get_unconsumed_network_notes(account_id, block_num.as_u32()).await?; - let notes = notes - .into_iter() - .map(|note| { - let NetworkNote::SingleTarget(note) = note; - note - }) - .collect::>(); - - let known_nullifiers: HashSet = - notes.iter().map(SingleTargetNetworkNote::nullifier).collect(); - - let account_tracker = AccountDeltaTracker::new(account); - let mut note_pool = NotePool::default(); - for note in notes { - note_pool.add_note(note); - } - - let state = Self { - account: account_tracker, - notes: note_pool, - account_id, - inflight_txs: BTreeMap::default(), - known_nullifiers, - }; - - state.inject_telemetry(); - - Ok(state) - } - - /// Selects the next candidate network transaction. - /// - /// # Parameters - /// - /// - `limit`: Maximum number of notes to include in the transaction. - /// - `max_note_attempts`: Maximum number of execution attempts before a note is dropped. - /// - `chain_state`: Current chain state for the transaction. - #[instrument(target = COMPONENT, name = "ntx.state.select_candidate", skip_all)] - pub fn select_candidate( - &mut self, - limit: NonZeroUsize, - max_note_attempts: usize, - chain_state: ChainState, - ) -> Option { - // Remove notes that have failed too many times. - self.notes.drop_failing_notes(max_note_attempts); - - // Skip empty accounts, and prune them. - // This is how we keep the number of accounts bounded. - if self.is_empty() { - return None; - } - - // Select notes from the account that can be consumed or are ready for a retry. - let notes = self - .notes - .available_notes(&chain_state.chain_tip_header.block_num()) - .take(limit.get()) - .cloned() - .collect::>(); - - // Skip accounts with no available notes. - if notes.is_empty() { - return None; - } - - let (chain_tip_header, chain_mmr) = chain_state.into_parts(); - TransactionCandidate { - account: self.account.latest_account(), - notes, - chain_tip_header, - chain_mmr, - } - .into() - } - - /// Marks notes of a previously selected candidate as failed. - /// - /// Does not remove the candidate from the in-progress pool. - #[instrument(target = COMPONENT, name = "ntx.state.notes_failed", skip_all)] - pub fn notes_failed(&mut self, notes: &[Note], block_num: BlockNumber) { - let nullifiers = notes.iter().map(Note::nullifier).collect::>(); - self.notes.fail_notes(nullifiers.as_slice(), block_num); - } - - /// Updates state with the mempool event. - #[instrument(target = COMPONENT, name = "ntx.state.mempool_update", skip_all)] - pub fn mempool_update(&mut self, update: &MempoolEvent) -> Option { - let span = tracing::Span::current(); - span.set_attribute("mempool_event.kind", update.kind()); - - match update { - MempoolEvent::TransactionAdded { - id, - nullifiers, - network_notes, - account_delta, - } => { - // Filter network notes relevant to this account. - let network_notes = filter_by_account_id_and_map_to_single_target( - self.account_id, - network_notes.clone(), - ); - self.add_transaction(*id, nullifiers, &network_notes, account_delta.as_ref()); - }, - MempoolEvent::TransactionsReverted(txs) => { - for tx in txs { - let shutdown_reason = self.revert_transaction(*tx); - if shutdown_reason.is_some() { - return shutdown_reason; - } - } - }, - MempoolEvent::BlockCommitted { txs, .. } => { - for tx in txs { - self.commit_transaction(*tx); - } - }, - } - self.inject_telemetry(); - - // No shutdown, continue running actor. - None - } - - /// Returns `true` if there is no inflight state being tracked. - fn is_empty(&self) -> bool { - self.account.has_no_inflight() && self.notes.is_empty() - } - - /// Handles a [`MempoolEvent::TransactionAdded`] event. - fn add_transaction( - &mut self, - id: TransactionId, - nullifiers: &[Nullifier], - network_notes: &[SingleTargetNetworkNote], - account_delta: Option<&AccountUpdateDetails>, - ) { - // Skip transactions we already know about. - // - // This can occur since both ntx builder and the mempool might inform us of the same - // transaction. Once when it was submitted to the mempool, and once by the mempool event. - if self.inflight_txs.contains_key(&id) { - return; - } - - let mut tx_impact = TransactionImpact::default(); - if let Some(update) = account_delta.and_then(NetworkAccountEffect::from_protocol) { - let account_id = update.network_account_id(); - if account_id == self.account_id { - match update { - NetworkAccountEffect::Updated(account_delta) => { - self.account.add_delta(&account_delta); - tx_impact.account_delta = Some(account_id); - }, - NetworkAccountEffect::Created(_) => {}, - } - } - } - for note in network_notes { - assert_eq!( - note.account_id(), - self.account_id, - "note's account ID does not match network account actor's account ID" - ); - tx_impact.notes.insert(note.nullifier()); - self.known_nullifiers.insert(note.nullifier()); - self.notes.add_note(note.clone()); - } - for nullifier in nullifiers { - // Ignore nullifiers that aren't network note nullifiers. - if !self.known_nullifiers.contains(nullifier) { - continue; - } - tx_impact.nullifiers.insert(*nullifier); - let _ = self.notes.nullify(*nullifier); - } - - if !tx_impact.is_empty() { - self.inflight_txs.insert(id, tx_impact); - } - } - - /// Handles [`MempoolEvent::BlockCommitted`] events. - fn commit_transaction(&mut self, tx: TransactionId) { - // We only track transactions which have an impact on the network state. - let Some(impact) = self.inflight_txs.remove(&tx) else { - return; - }; - - if let Some(delta_account_id) = impact.account_delta { - if delta_account_id == self.account_id { - self.account.commit_delta(); - } - } - - for nullifier in impact.nullifiers { - if self.known_nullifiers.remove(&nullifier) { - // Its possible for the account to no longer exist if the transaction creating it - // was reverted. - self.notes.commit_nullifier(nullifier); - } - } - } - - /// Handles [`MempoolEvent::TransactionsReverted`] events. - fn revert_transaction(&mut self, tx: TransactionId) -> Option { - // We only track transactions which have an impact on the network state. - let Some(impact) = self.inflight_txs.remove(&tx) else { - tracing::debug!("transaction {tx} not found in inflight transactions"); - return None; - }; - - // Revert account creation. - if let Some(account_id) = impact.account_delta { - // Account creation reverted, actor must stop. - if account_id == self.account_id && self.account.revert_delta() { - return Some(ActorShutdownReason::AccountReverted(account_id)); - } - } - - // Revert notes. - for note_nullifier in impact.notes { - if self.known_nullifiers.contains(¬e_nullifier) { - self.notes.remove_note(note_nullifier); - self.known_nullifiers.remove(¬e_nullifier); - } - } - - // Revert nullifiers. - for nullifier in impact.nullifiers { - if self.known_nullifiers.contains(&nullifier) { - self.notes.revert_nullifier(nullifier); - self.known_nullifiers.remove(&nullifier); - } - } - - None - } - - /// Adds stats to the current tracing span. - /// - /// Note that these are only visible in the OpenTelemetry context, as conventional tracing - /// does not track fields added dynamically. - fn inject_telemetry(&self) { - let span = tracing::Span::current(); - - span.set_attribute("ntx.state.transactions", self.inflight_txs.len()); - span.set_attribute("ntx.state.notes.total", self.known_nullifiers.len()); - } -} - -/// The impact a transaction has on the state. -#[derive(Clone, Default)] -struct TransactionImpact { - /// The network account this transaction added an account delta to. - account_delta: Option, - - /// Network notes this transaction created. - notes: BTreeSet, - - /// Network notes this transaction consumed. - nullifiers: BTreeSet, -} - -impl TransactionImpact { - fn is_empty(&self) -> bool { - self.account_delta.is_none() && self.notes.is_empty() && self.nullifiers.is_empty() - } -} - -/// Filters network notes by account ID and maps them to single target network notes. -fn filter_by_account_id_and_map_to_single_target( - account_id: NetworkAccountId, - notes: Vec, -) -> Vec { - notes - .into_iter() - .filter_map(|note| match note { - NetworkNote::SingleTarget(note) if note.account_id() == account_id => Some(note), - NetworkNote::SingleTarget(_) => None, - }) - .collect::>() -} - -#[cfg(test)] -mod tests { - use std::collections::HashSet; - use std::sync::{Arc, Mutex}; - - use miden_protocol::account::{AccountBuilder, AccountStorageMode, AccountType}; - use miden_protocol::asset::{Asset, FungibleAsset}; - use miden_protocol::crypto::rand::RpoRandomCoin; - use miden_protocol::note::{Note, NoteAttachment, NoteExecutionHint, NoteType}; - use miden_protocol::testing::account_id::AccountIdBuilder; - use miden_protocol::transaction::TransactionId; - use miden_protocol::{EMPTY_WORD, Felt, Hasher}; - use miden_standards::note::{NetworkAccountTarget, create_p2id_note}; - - use super::*; - - // HELPERS - // ============================================================================================ - - /// Creates a network account for testing. - fn create_network_account(seed: u8) -> Account { - use miden_protocol::testing::noop_auth_component::NoopAuthComponent; - use miden_standards::account::wallets::BasicWallet; - - AccountBuilder::new([seed; 32]) - .account_type(AccountType::RegularAccountUpdatableCode) - .storage_mode(AccountStorageMode::Network) - .with_component(BasicWallet) - .with_auth_component(NoopAuthComponent) - .build_existing() - .expect("should be able to build test account") - } - - /// Creates a faucet account ID for testing. - fn create_faucet_id(seed: u8) -> miden_protocol::account::AccountId { - AccountIdBuilder::new() - .account_type(AccountType::FungibleFaucet) - .storage_mode(AccountStorageMode::Public) - .build_with_seed([seed; 32]) - } - - /// Creates a note targeted at the given network account. - fn create_network_note( - target_account_id: miden_protocol::account::AccountId, - seed: u8, - ) -> Note { - let coin_seed: [u64; 4] = - [u64::from(seed), u64::from(seed) + 1, u64::from(seed) + 2, u64::from(seed) + 3]; - let rng = Arc::new(Mutex::new(RpoRandomCoin::new(coin_seed.map(Felt::new).into()))); - let mut rng = rng.lock().unwrap(); - - let faucet_id = create_faucet_id(seed.wrapping_add(100)); - - let target = NetworkAccountTarget::new(target_account_id, NoteExecutionHint::Always) - .expect("NetworkAccountTarget creation should succeed for network account"); - let attachment: NoteAttachment = target.into(); - - create_p2id_note( - target_account_id, - target_account_id, - vec![Asset::Fungible(FungibleAsset::new(faucet_id, 10).unwrap())], - NoteType::Public, - attachment, - &mut *rng, - ) - .expect("note creation should succeed") - } - - /// Creates a `SingleTargetNetworkNote` from a `Note`. - fn to_single_target_note(note: Note) -> SingleTargetNetworkNote { - SingleTargetNetworkNote::try_from(note).expect("should convert to SingleTargetNetworkNote") - } - - /// Creates a mock `TransactionId` for testing. - fn mock_tx_id(seed: u8) -> TransactionId { - TransactionId::new( - Hasher::hash(&[seed; 32]), - Hasher::hash(&[seed.wrapping_add(1); 32]), - EMPTY_WORD, - EMPTY_WORD, - ) - } - - /// Creates a mock `BlockHeader` for testing. - fn mock_block_header(block_num: u32) -> miden_protocol::block::BlockHeader { - use miden_node_utils::fee::test_fee_params; - use miden_protocol::crypto::dsa::ecdsa_k256_keccak::SecretKey; - - miden_protocol::block::BlockHeader::new( - 0, - EMPTY_WORD, - BlockNumber::from(block_num), - EMPTY_WORD, - EMPTY_WORD, - EMPTY_WORD, - EMPTY_WORD, - EMPTY_WORD, - EMPTY_WORD, - SecretKey::new().public_key(), - test_fee_params(), - 0, - ) - } - - impl NetworkAccountState { - /// Creates a new `NetworkAccountState` for testing. - /// - /// This mirrors the behavior of `load()` but with provided notes instead of - /// fetching from the store. - #[cfg(test)] - pub fn new_for_testing( - account: Account, - account_id: NetworkAccountId, - notes: Vec, - ) -> Self { - let known_nullifiers: HashSet = - notes.iter().map(SingleTargetNetworkNote::nullifier).collect(); - - let account_tracker = AccountDeltaTracker::new(account); - let mut note_pool = NotePool::default(); - for note in notes { - note_pool.add_note(note); - } - - Self { - account: account_tracker, - notes: note_pool, - account_id, - inflight_txs: BTreeMap::default(), - known_nullifiers, - } - } - } - - // TESTS - // ============================================================================================ - - /// Tests that initial notes loaded into `NetworkAccountState` have their nullifiers - /// registered in `known_nullifiers`. - #[test] - fn test_initial_notes_have_nullifiers_indexed() { - let account = create_network_account(1); - let account_id = account.id(); - let network_account_id = - NetworkAccountId::try_from(account_id).expect("should be a network account"); - - let note1 = to_single_target_note(create_network_note(account_id, 1)); - let note2 = to_single_target_note(create_network_note(account_id, 2)); - let nullifier1 = note1.nullifier(); - let nullifier2 = note2.nullifier(); - - let state = - NetworkAccountState::new_for_testing(account, network_account_id, vec![note1, note2]); - - assert!( - state.known_nullifiers.contains(&nullifier1), - "known_nullifiers should contain first note's nullifier" - ); - assert!( - state.known_nullifiers.contains(&nullifier2), - "known_nullifiers should contain second note's nullifier" - ); - assert_eq!( - state.known_nullifiers.len(), - 2, - "known_nullifiers should have exactly 2 entries" - ); - } - - /// Tests that when a `TransactionAdded` event arrives with nullifiers from initial notes, - /// those notes are properly moved from `available_notes` to `nullified_notes`. - #[test] - fn test_mempool_event_nullifies_initial_notes() { - let account = create_network_account(1); - let account_id = account.id(); - let network_account_id = - NetworkAccountId::try_from(account_id).expect("should be a network account"); - - let note1 = to_single_target_note(create_network_note(account_id, 1)); - let note2 = to_single_target_note(create_network_note(account_id, 2)); - let nullifier1 = note1.nullifier(); - let nullifier2 = note2.nullifier(); - - let mut state = - NetworkAccountState::new_for_testing(account, network_account_id, vec![note1, note2]); - - let available_count = state.notes.available_notes(&BlockNumber::from(0)).count(); - assert_eq!(available_count, 2, "both notes should be available initially"); - - let tx_id = mock_tx_id(1); - let event = MempoolEvent::TransactionAdded { - id: tx_id, - nullifiers: vec![nullifier1], - network_notes: vec![], - account_delta: None, - }; - - let shutdown = state.mempool_update(&event); - assert!(shutdown.is_none(), "mempool_update should not trigger shutdown"); - - let available_nullifiers: Vec<_> = state - .notes - .available_notes(&BlockNumber::from(0)) - .map(|n| n.to_inner().nullifier()) - .collect(); - assert!( - !available_nullifiers.contains(&nullifier1), - "note1 should no longer be available" - ); - assert!(available_nullifiers.contains(&nullifier2), "note2 should still be available"); - assert_eq!(available_nullifiers.len(), 1, "only one note should be available"); - - assert!( - state.inflight_txs.contains_key(&tx_id), - "transaction should be tracked in inflight_txs" - ); - } - - /// Tests that after committing a transaction, the nullifier is removed from `known_nullifiers`. - #[test] - fn test_commit_removes_nullifier_from_index() { - let account = create_network_account(1); - let account_id = account.id(); - let network_account_id = - NetworkAccountId::try_from(account_id).expect("should be a network account"); - - let note1 = to_single_target_note(create_network_note(account_id, 1)); - let nullifier1 = note1.nullifier(); - - let mut state = - NetworkAccountState::new_for_testing(account, network_account_id, vec![note1]); - - let tx_id = mock_tx_id(1); - let event = MempoolEvent::TransactionAdded { - id: tx_id, - nullifiers: vec![nullifier1], - network_notes: vec![], - account_delta: None, - }; - state.mempool_update(&event); - - assert!( - state.known_nullifiers.contains(&nullifier1), - "nullifier should still be in index while transaction is inflight" - ); - - let commit_event = MempoolEvent::BlockCommitted { - header: Box::new(mock_block_header(1)), - txs: vec![tx_id], - }; - state.mempool_update(&commit_event); - - assert!( - !state.known_nullifiers.contains(&nullifier1), - "nullifier should be removed from index after commit" - ); - } - - /// Tests that reverting a transaction restores the note to `available_notes`. - #[test] - fn test_revert_restores_note_to_available() { - let account = create_network_account(1); - let account_id = account.id(); - let network_account_id = - NetworkAccountId::try_from(account_id).expect("should be a network account"); - - let note1 = to_single_target_note(create_network_note(account_id, 1)); - let nullifier1 = note1.nullifier(); - - let mut state = - NetworkAccountState::new_for_testing(account, network_account_id, vec![note1]); - - let tx_id = mock_tx_id(1); - let event = MempoolEvent::TransactionAdded { - id: tx_id, - nullifiers: vec![nullifier1], - network_notes: vec![], - account_delta: None, - }; - state.mempool_update(&event); - - // Verify note is not available - let available_count = state.notes.available_notes(&BlockNumber::from(0)).count(); - assert_eq!(available_count, 0, "note should not be available after being consumed"); - - // Revert the transaction - let revert_event = - MempoolEvent::TransactionsReverted(HashSet::from_iter(std::iter::once(tx_id))); - state.mempool_update(&revert_event); - - // Verify note is available again - let available_nullifiers: Vec<_> = state - .notes - .available_notes(&BlockNumber::from(0)) - .map(|n| n.to_inner().nullifier()) - .collect(); - assert!( - available_nullifiers.contains(&nullifier1), - "note should be available again after revert" - ); - } - - /// Tests that nullifiers from dynamically added notes are also indexed. - #[test] - fn test_dynamically_added_notes_are_indexed() { - let account = create_network_account(1); - let account_id = account.id(); - let network_account_id = - NetworkAccountId::try_from(account_id).expect("should be a network account"); - - let mut state = NetworkAccountState::new_for_testing(account, network_account_id, vec![]); - - assert!(state.known_nullifiers.is_empty(), "known_nullifiers should be empty initially"); - - let new_note = to_single_target_note(create_network_note(account_id, 1)); - let new_nullifier = new_note.nullifier(); - - let tx_id = mock_tx_id(1); - let event = MempoolEvent::TransactionAdded { - id: tx_id, - nullifiers: vec![], - network_notes: vec![NetworkNote::SingleTarget(new_note)], - account_delta: None, - }; - - state.mempool_update(&event); - - // Verify the new note's nullifier is now indexed - assert!( - state.known_nullifiers.contains(&new_nullifier), - "dynamically added note's nullifier should be indexed" - ); - - // Verify the note is available - let available_nullifiers: Vec<_> = state - .notes - .available_notes(&BlockNumber::from(0)) - .map(|n| n.to_inner().nullifier()) - .collect(); - assert!( - available_nullifiers.contains(&new_nullifier), - "dynamically added note should be available" - ); - } -} diff --git a/crates/ntx-builder/src/actor/inflight_note.rs b/crates/ntx-builder/src/actor/inflight_note.rs index 23c7d06d7..4cc080862 100644 --- a/crates/ntx-builder/src/actor/inflight_note.rs +++ b/crates/ntx-builder/src/actor/inflight_note.rs @@ -29,6 +29,15 @@ impl InflightNetworkNote { } } + /// Reconstructs an inflight network note from its constituent parts (e.g., from DB rows). + pub fn from_parts( + note: SingleTargetNetworkNote, + attempt_count: usize, + last_attempt: Option, + ) -> Self { + Self { note, attempt_count, last_attempt } + } + /// Consumes the inflight network note and returns the inner network note. pub fn into_inner(self) -> SingleTargetNetworkNote { self.note diff --git a/crates/ntx-builder/src/actor/mod.rs b/crates/ntx-builder/src/actor/mod.rs index dd15c8e0e..03bf1d1c5 100644 --- a/crates/ntx-builder/src/actor/mod.rs +++ b/crates/ntx-builder/src/actor/mod.rs @@ -1,13 +1,13 @@ +pub(crate) mod account_effect; pub mod account_state; mod execute; -mod inflight_note; -mod note_state; +pub(crate) mod inflight_note; use std::num::NonZeroUsize; use std::sync::Arc; use std::time::Duration; -use account_state::{NetworkAccountState, TransactionCandidate}; +use account_state::TransactionCandidate; use futures::FutureExt; use miden_node_proto::clients::{Builder, ValidatorClient}; use miden_node_proto::domain::account::NetworkAccountId; @@ -17,7 +17,7 @@ use miden_node_utils::lru_cache::LruCache; use miden_protocol::Word; use miden_protocol::account::{Account, AccountDelta}; use miden_protocol::block::BlockNumber; -use miden_protocol::note::NoteScript; +use miden_protocol::note::{Note, NoteScript}; use miden_protocol::transaction::TransactionId; use miden_remote_prover_client::RemoteTransactionProver; use tokio::sync::{AcquireError, RwLock, Semaphore, mpsc}; @@ -26,6 +26,7 @@ use url::Url; use crate::block_producer::BlockProducerClient; use crate::builder::ChainState; +use crate::db::Db; use crate::store::StoreClient; // ACTOR SHUTDOWN REASON @@ -33,8 +34,6 @@ use crate::store::StoreClient; /// The reason an actor has shut down. pub enum ActorShutdownReason { - /// Occurs when the transaction that created the actor is reverted. - AccountReverted(NetworkAccountId), /// Occurs when an account actor detects failure in the messaging channel used by the /// coordinator. EventChannelClosed, @@ -71,6 +70,8 @@ pub struct AccountActorContext { pub max_notes_per_tx: NonZeroUsize, /// Maximum number of note execution attempts before dropping a note. pub max_note_attempts: usize, + /// Database for persistent state. + pub db: Db, } // ACCOUNT ORIGIN @@ -132,10 +133,10 @@ enum ActorMode { /// /// ## Core Responsibilities /// -/// - **State Management**: Loads and maintains the current state of network accounts, including -/// available notes, pending transactions, and account commitments. +/// - **State Management**: Queries the database for the current state of network accounts, +/// including available notes and the latest account state. /// - **Transaction Selection**: Selects viable notes and constructs a [`TransactionCandidate`] -/// based on current chain state. +/// based on current chain state and DB queries. /// - **Transaction Execution**: Executes selected transactions using either local or remote /// proving. /// - **Mempool Integration**: Listens for mempool events to stay synchronized with the network @@ -143,11 +144,12 @@ enum ActorMode { /// /// ## Lifecycle /// -/// 1. **Initialization**: Loads account state from the store or uses provided account data. +/// 1. **Initialization**: Checks DB for available notes to determine initial mode. /// 2. **Event Loop**: Continuously processes mempool events and executes transactions. /// 3. **Transaction Processing**: Selects, executes, and proves transactions, and submits them to /// block producer. -/// 4. **State Updates**: Updates internal state based on mempool events and execution results. +/// 4. **State Updates**: Event effects are persisted to DB by the coordinator before actors are +/// notified. /// 5. **Shutdown**: Terminates gracefully when cancelled or encounters unrecoverable errors. /// /// ## Concurrency @@ -158,6 +160,7 @@ enum ActorMode { pub struct AccountActor { origin: AccountOrigin, store: StoreClient, + db: Db, mode: ActorMode, event_rx: mpsc::Receiver>, cancel_token: CancellationToken, @@ -193,6 +196,7 @@ impl AccountActor { Self { origin, store: actor_context.store.clone(), + db: actor_context.db.clone(), mode: ActorMode::NoViableNotes, event_rx, cancel_token, @@ -209,23 +213,19 @@ impl AccountActor { /// Runs the account actor, processing events and managing state until a reason to shutdown is /// encountered. pub async fn run(mut self, semaphore: Arc) -> ActorShutdownReason { - // Load the account state from the store and set up the account actor state. - let account = { - match self.origin { - AccountOrigin::Store(account_id) => self - .store - .get_network_account(account_id) - .await - .expect("actor should be able to load account") - .expect("actor account should exist"), - AccountOrigin::Transaction(ref account) => *(account.clone()), - } - }; + let account_id = self.origin.id(); + + // Determine initial mode by checking DB for available notes. let block_num = self.chain_state.read().await.chain_tip_header.block_num(); - let mut state = - NetworkAccountState::load(account, self.origin.id(), &self.store, block_num) - .await - .expect("actor should be able to load account state"); + let has_notes = self + .db + .has_available_notes(account_id, block_num, self.max_note_attempts) + .await + .expect("actor should be able to check for available notes"); + + if has_notes { + self.mode = ActorMode::NotesAvailable; + } loop { // Enable or disable transaction execution based on actor mode. @@ -239,28 +239,31 @@ impl AccountActor { }; tokio::select! { _ = self.cancel_token.cancelled() => { - return ActorShutdownReason::Cancelled(self.origin.id()); + return ActorShutdownReason::Cancelled(account_id); } // Handle mempool events. event = self.event_rx.recv() => { let Some(event) = event else { return ActorShutdownReason::EventChannelClosed; }; - // Re-enable transaction execution if the transaction being waited on has been - // added to the mempool. + // Re-enable transaction execution if the transaction being waited on has + // been resolved (added to mempool, committed in a block, or reverted). if let ActorMode::TransactionInflight(awaited_id) = self.mode { - if let MempoolEvent::TransactionAdded { id, .. } = *event { - if id == awaited_id { - self.mode = ActorMode::NotesAvailable; - } + let should_wake = match event.as_ref() { + MempoolEvent::TransactionAdded { id, .. } => *id == awaited_id, + MempoolEvent::BlockCommitted { txs, .. } => { + txs.contains(&awaited_id) + }, + MempoolEvent::TransactionsReverted(tx_ids) => { + tx_ids.contains(&awaited_id) + }, + }; + if should_wake { + self.mode = ActorMode::NotesAvailable; } } else { self.mode = ActorMode::NotesAvailable; } - // Update state. - if let Some(shutdown_reason) = state.mempool_update(event.as_ref()) { - return shutdown_reason; - } }, // Execute transactions. permit = tx_permit_acquisition => { @@ -268,13 +271,20 @@ impl AccountActor { Ok(_permit) => { // Read the chain state. let chain_state = self.chain_state.read().await.clone(); - // Find a candidate transaction and execute it. - if let Some(tx_candidate) = state.select_candidate( - self.max_notes_per_tx, - self.max_note_attempts, + + // Drop notes that have failed too many times. + if let Err(err) = self.db.drop_failing_notes(account_id, self.max_note_attempts).await { + tracing::error!(err = %err, "failed to drop failing notes"); + } + + // Query DB for latest account and available notes. + let tx_candidate = self.select_candidate_from_db( + account_id, chain_state, - ) { - self.execute_transactions(&mut state, tx_candidate).await; + ).await; + + if let Some(tx_candidate) = tx_candidate { + self.execute_transactions(account_id, tx_candidate).await; } else { // No transactions to execute, wait for events. self.mode = ActorMode::NoViableNotes; @@ -289,13 +299,44 @@ impl AccountActor { } } + /// Selects a transaction candidate by querying the DB. + async fn select_candidate_from_db( + &self, + account_id: NetworkAccountId, + chain_state: ChainState, + ) -> Option { + let block_num = chain_state.chain_tip_header.block_num(); + let max_notes = self.max_notes_per_tx.get(); + + let (latest_account, notes) = self + .db + .select_candidate(account_id, block_num, self.max_note_attempts) + .await + .expect("actor should be able to query DB for candidate"); + + let account = latest_account?; + + let notes: Vec<_> = notes.into_iter().take(max_notes).collect(); + if notes.is_empty() { + return None; + } + + let (chain_tip_header, chain_mmr) = chain_state.into_parts(); + Some(TransactionCandidate { + account, + notes, + chain_tip_header, + chain_mmr, + }) + } + /// Execute a transaction candidate and mark notes as failed as required. /// /// Updates the state of the actor based on the execution result. - #[tracing::instrument(name = "ntx.actor.execute_transactions", skip(self, state, tx_candidate))] + #[tracing::instrument(name = "ntx.actor.execute_transactions", skip(self, tx_candidate))] async fn execute_transactions( &mut self, - state: &mut NetworkAccountState, + account_id: NetworkAccountId, tx_candidate: TransactionCandidate, ) { let block_num = tx_candidate.chain_tip_header.block_num(); @@ -318,20 +359,34 @@ impl AccountActor { }, // Execution completed with some failed notes. Ok((tx_id, failed)) => { - let notes = failed.into_iter().map(|note| note.note).collect::>(); - state.notes_failed(notes.as_slice(), block_num); + let nullifiers: Vec<_> = + failed.into_iter().map(|note| note.note.nullifier()).collect(); + self.mark_notes_failed(&nullifiers, block_num).await; self.mode = ActorMode::TransactionInflight(tx_id); }, // Transaction execution failed. Err(err) => { tracing::error!(err = err.as_report(), "network transaction failed"); self.mode = ActorMode::NoViableNotes; - let notes = - notes.into_iter().map(|note| note.into_inner().into()).collect::>(); - state.notes_failed(notes.as_slice(), block_num); + let nullifiers: Vec<_> = notes + .into_iter() + .map(|note| Note::from(note.into_inner()).nullifier()) + .collect(); + self.mark_notes_failed(&nullifiers, block_num).await; }, } } + + /// Marks notes as failed in the DB. + async fn mark_notes_failed( + &self, + nullifiers: &[miden_protocol::note::Nullifier], + block_num: BlockNumber, + ) { + if let Err(err) = self.db.notes_failed(nullifiers.to_vec(), block_num).await { + tracing::error!(err = %err, "failed to mark notes as failed"); + } + } } // HELPERS diff --git a/crates/ntx-builder/src/actor/note_state.rs b/crates/ntx-builder/src/actor/note_state.rs deleted file mode 100644 index 610334c67..000000000 --- a/crates/ntx-builder/src/actor/note_state.rs +++ /dev/null @@ -1,235 +0,0 @@ -use std::collections::{HashMap, VecDeque}; - -use miden_node_proto::domain::account::NetworkAccountId; -use miden_node_proto::domain::note::SingleTargetNetworkNote; -use miden_protocol::account::delta::AccountUpdateDetails; -use miden_protocol::account::{Account, AccountDelta, AccountId}; -use miden_protocol::block::BlockNumber; -use miden_protocol::note::Nullifier; - -use crate::actor::inflight_note::InflightNetworkNote; - -// ACCOUNT DELTA TRACKER -// ================================================================================================ - -/// Tracks committed and inflight account state updates. -#[derive(Clone)] -pub struct AccountDeltaTracker { - /// The committed account state, if any. - /// - /// This may be `None` if the account creation transaction is still inflight. - committed: Option, - - /// Inflight account updates in chronological order. - inflight: VecDeque, -} - -impl AccountDeltaTracker { - /// Creates a new tracker with the given committed account state. - pub fn new(account: Account) -> Self { - Self { - committed: Some(account), - inflight: VecDeque::default(), - } - } - - /// Appends a delta to the set of inflight account updates. - pub fn add_delta(&mut self, delta: &AccountDelta) { - let mut state = self.latest_account(); - state - .apply_delta(delta) - .expect("network account delta should apply since it was accepted by the mempool"); - - self.inflight.push_back(state); - } - - /// Commits the oldest account state delta. - /// - /// # Panics - /// - /// Panics if there are no deltas to commit. - pub fn commit_delta(&mut self) { - self.committed = self.inflight.pop_front().expect("must have a delta to commit").into(); - } - - /// Reverts the newest account state delta. - /// - /// Returns `true` if this reverted the account creation delta. The caller _must_ handle - /// cleanup as calls to `latest_account` will panic afterwards. - /// - /// # Panics - /// - /// Panics if there are no deltas to revert. - #[must_use = "must handle account removal if this returns true"] - pub fn revert_delta(&mut self) -> bool { - self.inflight.pop_back().expect("must have a delta to revert"); - self.committed.is_none() && self.inflight.is_empty() - } - - /// Returns the latest inflight account state. - pub fn latest_account(&self) -> Account { - self.inflight - .back() - .or(self.committed.as_ref()) - .expect("account must have either a committed or inflight state") - .clone() - } - - /// Returns `true` if there are no inflight deltas. - pub fn has_no_inflight(&self) -> bool { - self.inflight.is_empty() - } -} - -// NOTE POOL -// ================================================================================================ - -/// Manages available and nullified notes for a network account. -#[derive(Clone, Default)] -pub struct NotePool { - /// Unconsumed notes available for consumption. - available: HashMap, - - /// Notes consumed by inflight transactions (not yet committed). - nullified: HashMap, -} - -impl NotePool { - /// Returns an iterator over notes that are available and not in backoff. - pub fn available_notes( - &self, - block_num: &BlockNumber, - ) -> impl Iterator { - self.available.values().filter(|¬e| note.is_available(*block_num)) - } - - /// Adds a new network note making it available for consumption. - pub fn add_note(&mut self, note: SingleTargetNetworkNote) { - self.available.insert(note.nullifier(), InflightNetworkNote::new(note)); - } - - /// Removes the note completely (used when reverting note creation). - pub fn remove_note(&mut self, nullifier: Nullifier) { - self.available.remove(&nullifier); - self.nullified.remove(&nullifier); - } - - /// Marks a note as being consumed by moving it to the nullified set. - /// - /// Returns `Err(())` if the note does not exist or was already nullified. - pub fn nullify(&mut self, nullifier: Nullifier) -> Result<(), ()> { - if let Some(note) = self.available.remove(&nullifier) { - self.nullified.insert(nullifier, note); - Ok(()) - } else { - tracing::warn!(%nullifier, "note must be available to nullify"); - Err(()) - } - } - - /// Commits a nullifier, removing the associated note entirely. - /// - /// Silently ignores if the nullifier is not present. - pub fn commit_nullifier(&mut self, nullifier: Nullifier) { - let _ = self.nullified.remove(&nullifier); - } - - /// Reverts a nullifier, making the note available again. - pub fn revert_nullifier(&mut self, nullifier: Nullifier) { - // Transactions can be reverted out of order. - if let Some(note) = self.nullified.remove(&nullifier) { - self.available.insert(nullifier, note); - } - } - - /// Drops all notes that have exceeded the maximum attempt count. - pub fn drop_failing_notes(&mut self, max_attempts: usize) { - self.available.retain(|_, note| note.attempt_count() < max_attempts); - } - - /// Marks the specified notes as failed. - pub fn fail_notes(&mut self, nullifiers: &[Nullifier], block_num: BlockNumber) { - for nullifier in nullifiers { - if let Some(note) = self.available.get_mut(nullifier) { - note.fail(block_num); - } else { - tracing::warn!(%nullifier, "failed note is not in account's state"); - } - } - } - - /// Returns `true` if there are no notes being tracked. - pub fn is_empty(&self) -> bool { - self.available.is_empty() && self.nullified.is_empty() - } -} - -// NETWORK ACCOUNT EFFECT -// ================================================================================================ - -/// Represents the effect of a transaction on a network account. -#[derive(Clone)] -pub enum NetworkAccountEffect { - Created(Account), - Updated(AccountDelta), -} - -impl NetworkAccountEffect { - pub fn from_protocol(update: &AccountUpdateDetails) -> Option { - let update = match update { - AccountUpdateDetails::Private => return None, - AccountUpdateDetails::Delta(update) if update.is_full_state() => { - NetworkAccountEffect::Created( - Account::try_from(update) - .expect("Account should be derivable by full state AccountDelta"), - ) - }, - AccountUpdateDetails::Delta(update) => NetworkAccountEffect::Updated(update.clone()), - }; - - update.protocol_account_id().is_network().then_some(update) - } - - pub fn network_account_id(&self) -> NetworkAccountId { - // SAFETY: This is a network account by construction. - self.protocol_account_id().try_into().unwrap() - } - - fn protocol_account_id(&self) -> AccountId { - match self { - NetworkAccountEffect::Created(acc) => acc.id(), - NetworkAccountEffect::Updated(delta) => delta.id(), - } - } -} - -#[cfg(test)] -mod tests { - use miden_protocol::block::BlockNumber; - - #[rstest::rstest] - #[test] - #[case::all_zero(Some(BlockNumber::GENESIS), BlockNumber::GENESIS, 0, true)] - #[case::no_attempts(None, BlockNumber::GENESIS, 0, true)] - #[case::one_attempt(Some(BlockNumber::GENESIS), BlockNumber::from(2), 1, true)] - #[case::three_attempts(Some(BlockNumber::GENESIS), BlockNumber::from(3), 3, true)] - #[case::ten_attempts(Some(BlockNumber::GENESIS), BlockNumber::from(13), 10, true)] - #[case::twenty_attempts(Some(BlockNumber::GENESIS), BlockNumber::from(149), 20, true)] - #[case::one_attempt_false(Some(BlockNumber::GENESIS), BlockNumber::from(1), 1, false)] - #[case::three_attempts_false(Some(BlockNumber::GENESIS), BlockNumber::from(2), 3, false)] - #[case::ten_attempts_false(Some(BlockNumber::GENESIS), BlockNumber::from(12), 10, false)] - #[case::twenty_attempts_false(Some(BlockNumber::GENESIS), BlockNumber::from(148), 20, false)] - fn backoff_has_passed( - #[case] last_attempt_block_num: Option, - #[case] current_block_num: BlockNumber, - #[case] attempt_count: usize, - #[case] backoff_should_have_passed: bool, - ) { - use crate::actor::has_backoff_passed; - - assert_eq!( - backoff_should_have_passed, - has_backoff_passed(current_block_num, last_attempt_block_num, attempt_count) - ); - } -} diff --git a/crates/ntx-builder/src/builder.rs b/crates/ntx-builder/src/builder.rs index 14be4ef31..b642d0379 100644 --- a/crates/ntx-builder/src/builder.rs +++ b/crates/ntx-builder/src/builder.rs @@ -16,6 +16,7 @@ use tonic::Status; use crate::NtxBuilderConfig; use crate::actor::{AccountActorContext, AccountOrigin}; use crate::coordinator::Coordinator; +use crate::db::Db; use crate::store::StoreClient; // CHAIN STATE @@ -89,6 +90,8 @@ pub struct NetworkTransactionBuilder { coordinator: Coordinator, /// Client for the store gRPC API. store: StoreClient, + /// Database for persistent state. + db: Db, /// Shared chain state updated by the event loop and read by actors. chain_state: Arc>, /// Context shared with all account actors. @@ -102,6 +105,7 @@ impl NetworkTransactionBuilder { config: NtxBuilderConfig, coordinator: Coordinator, store: StoreClient, + db: Db, chain_state: Arc>, actor_context: AccountActorContext, mempool_events: MempoolEventStream, @@ -110,6 +114,7 @@ impl NetworkTransactionBuilder { config, coordinator, store, + db, chain_state, actor_context, mempool_events, @@ -177,19 +182,48 @@ impl NetworkTransactionBuilder { } } - /// Handles account IDs loaded from the store by spawning actors for them. + /// Handles account IDs loaded from the store by syncing state to DB and spawning actors. #[tracing::instrument(name = "ntx.builder.handle_loaded_account", skip(self, account_id))] async fn handle_loaded_account( &mut self, account_id: NetworkAccountId, ) -> Result<(), anyhow::Error> { + // Fetch account from store and write to DB. + let account = self + .store + .get_network_account(account_id) + .await + .context("failed to load account from store")? + .context("account should exist in store")?; + + let block_num = self.chain_state.read().await.chain_tip_header.block_num(); + let notes = self + .store + .get_unconsumed_network_notes(account_id, block_num.as_u32()) + .await + .context("failed to load notes from store")?; + + let notes: Vec<_> = notes + .into_iter() + .map(|n| { + let miden_node_proto::domain::note::NetworkNote::SingleTarget(note) = n; + note + }) + .collect(); + + // Write account and notes to DB. + self.db + .sync_account_from_store(account_id, account.clone(), notes.clone()) + .await + .context("failed to sync account to DB")?; + self.coordinator .spawn_actor(AccountOrigin::store(account_id), &self.actor_context) .await?; Ok(()) } - /// Handles mempool events by routing them to actors and spawning new actors as needed. + /// Handles mempool events by writing to DB first, then routing to actors. #[tracing::instrument(name = "ntx.builder.handle_mempool_event", skip(self, event))] async fn handle_mempool_event( &mut self, @@ -197,6 +231,12 @@ impl NetworkTransactionBuilder { ) -> Result<(), anyhow::Error> { match event.as_ref() { MempoolEvent::TransactionAdded { account_delta, .. } => { + // Write event effects to DB first. + self.coordinator + .write_event(&event) + .await + .context("failed to write TransactionAdded to DB")?; + // Handle account deltas in case an account is being created. if let Some(AccountUpdateDetails::Delta(delta)) = account_delta { // Handle account deltas for network accounts only. @@ -214,24 +254,31 @@ impl NetworkTransactionBuilder { Ok(()) }, // Update chain state and broadcast. - MempoolEvent::BlockCommitted { header, txs } => { + MempoolEvent::BlockCommitted { header, .. } => { + // Write event effects to DB first. + self.coordinator + .write_event(&event) + .await + .context("failed to write BlockCommitted to DB")?; + self.update_chain_tip(header.as_ref().clone()).await; self.coordinator.broadcast(event.clone()).await; - - // All transactions pertaining to predating events should now be available - // through the store. So we can now drain them. - for tx_id in txs { - self.coordinator.drain_predating_events(tx_id); - } Ok(()) }, // Broadcast to all actors. - MempoolEvent::TransactionsReverted(txs) => { + MempoolEvent::TransactionsReverted(_) => { + // Write event effects to DB first; returns reverted account IDs. + let reverted_accounts = self + .coordinator + .write_event(&event) + .await + .context("failed to write TransactionsReverted to DB")?; + self.coordinator.broadcast(event.clone()).await; - // Reverted predating transactions need not be processed. - for tx_id in txs { - self.coordinator.drain_predating_events(tx_id); + // Cancel actors for reverted account creations. + for account_id in &reverted_accounts { + self.coordinator.cancel_actor(account_id); } Ok(()) }, diff --git a/crates/ntx-builder/src/coordinator.rs b/crates/ntx-builder/src/coordinator.rs index 673c40106..ea8f36195 100644 --- a/crates/ntx-builder/src/coordinator.rs +++ b/crates/ntx-builder/src/coordinator.rs @@ -2,18 +2,18 @@ use std::collections::HashMap; use std::sync::Arc; use anyhow::Context; -use indexmap::IndexMap; use miden_node_proto::domain::account::NetworkAccountId; use miden_node_proto::domain::mempool::MempoolEvent; -use miden_node_proto::domain::note::NetworkNote; +use miden_node_proto::domain::note::{NetworkNote, SingleTargetNetworkNote}; use miden_protocol::account::delta::AccountUpdateDetails; -use miden_protocol::transaction::TransactionId; use tokio::sync::mpsc::error::SendError; use tokio::sync::{Semaphore, mpsc}; use tokio::task::JoinSet; use tokio_util::sync::CancellationToken; use crate::actor::{AccountActor, AccountActorContext, AccountOrigin, ActorShutdownReason}; +use crate::db::Db; +use crate::db::errors::DatabaseError; // ACTOR HANDLE // ================================================================================================ @@ -87,9 +87,8 @@ pub struct Coordinator { /// ensuring fair resource allocation and system stability under load. semaphore: Arc, - /// Cache of events received from the mempool that predate corresponding network accounts. - /// Grouped by network account ID to allow targeted event delivery to actors upon creation. - predating_events: HashMap>>, + /// Database for persistent state. + db: Db, /// Channel size for each actor's event channel. actor_channel_size: usize, @@ -98,12 +97,12 @@ pub struct Coordinator { impl Coordinator { /// Creates a new coordinator with the specified maximum number of inflight transactions /// and actor channel size. - pub fn new(max_inflight_transactions: usize, actor_channel_size: usize) -> Self { + pub fn new(max_inflight_transactions: usize, actor_channel_size: usize, db: Db) -> Self { Self { actor_registry: HashMap::new(), actor_join_set: JoinSet::new(), semaphore: Arc::new(Semaphore::new(max_inflight_transactions)), - predating_events: HashMap::new(), + db, actor_channel_size, } } @@ -135,17 +134,10 @@ impl Coordinator { let actor = AccountActor::new(origin, actor_context, event_rx, cancel_token.clone()); let handle = ActorHandle::new(event_tx, cancel_token); - // Run the actor. + // Run the actor. Actor reads state from DB on startup. let semaphore = self.semaphore.clone(); self.actor_join_set.spawn(Box::pin(actor.run(semaphore))); - // Send the new actor any events that contain notes that predate account creation. - if let Some(predating_events) = self.predating_events.remove(&account_id) { - for event in predating_events.values() { - Self::send(&handle, event.clone()).await?; - } - } - self.actor_registry.insert(account_id, handle); tracing::info!(account_id = %account_id, "Created actor for account prefix"); Ok(()) @@ -202,11 +194,6 @@ impl Coordinator { tracing::info!(account_id = %account_id, "Account actor cancelled"); Ok(()) }, - ActorShutdownReason::AccountReverted(account_id) => { - tracing::info!(account_id = %account_id, "Account reverted"); - self.actor_registry.remove(&account_id); - Ok(()) - }, ActorShutdownReason::EventChannelClosed => { anyhow::bail!("event channel closed"); }, @@ -226,19 +213,15 @@ impl Coordinator { /// Sends a mempool event to all network account actors that are found in the corresponding /// transaction's notes. /// - /// Caches the mempool event for each network account found in the transaction's notes that does - /// not currently have a corresponding actor. If an actor does not exist for the account, it is - /// assumed that the account has not been created on the chain yet. - /// - /// Cached events will be fed to the corresponding actor when the account creation transaction - /// is processed. + /// Events are sent only to actors that are currently active. Since event effects are already + /// persisted in the DB by `write_event()`, actors that spawn later read their state from the + /// DB and do not need predating events. pub async fn send_targeted( &mut self, event: &Arc, ) -> Result<(), SendError>> { let mut target_actors = HashMap::new(); - if let MempoolEvent::TransactionAdded { id, network_notes, account_delta, .. } = - event.as_ref() + if let MempoolEvent::TransactionAdded { network_notes, account_delta, .. } = event.as_ref() { // We need to inform the account if it was updated. This lets it know that its own // transaction has been applied, and in the future also resolves race conditions with @@ -259,14 +242,7 @@ impl Coordinator { let NetworkNote::SingleTarget(note) = note; let network_account_id = note.account_id(); if let Some(actor) = self.actor_registry.get(&network_account_id) { - // Register actor as target. target_actors.insert(network_account_id, actor); - } else { - // Cache event for every note that doesn't have a corresponding actor. - self.predating_events - .entry(network_account_id) - .or_default() - .insert(*id, event.clone()); } } } @@ -277,16 +253,55 @@ impl Coordinator { Ok(()) } - /// Removes any cached events for a given transaction ID from all account caches. - pub fn drain_predating_events(&mut self, tx_id: &TransactionId) { - // Remove the transaction from all account caches. - // This iterates over all predating events which is fine because the count is expected to be - // low. - self.predating_events.retain(|_, account_events| { - account_events.shift_remove(tx_id); - // Remove entries for accounts with no more cached events. - !account_events.is_empty() - }); + /// Writes mempool event effects to the database. + /// + /// This must be called BEFORE sending notifications to actors. For `TransactionsReverted`, + /// returns the list of account IDs whose creation was reverted. + pub async fn write_event( + &self, + event: &MempoolEvent, + ) -> Result, DatabaseError> { + match event { + MempoolEvent::TransactionAdded { + id, + nullifiers, + network_notes, + account_delta, + } => { + let notes: Vec = network_notes + .iter() + .map(|n| { + let NetworkNote::SingleTarget(note) = n; + note.clone() + }) + .collect(); + + self.db + .handle_transaction_added(*id, account_delta.clone(), notes, nullifiers.clone()) + .await?; + Ok(Vec::new()) + }, + MempoolEvent::BlockCommitted { header, txs } => { + self.db + .handle_block_committed( + txs.clone(), + header.block_num(), + header.as_ref().clone(), + ) + .await?; + Ok(Vec::new()) + }, + MempoolEvent::TransactionsReverted(tx_ids) => { + self.db.handle_transactions_reverted(tx_ids.iter().copied().collect()).await + }, + } + } + + /// Cancels an actor by its account ID. + pub fn cancel_actor(&mut self, account_id: &NetworkAccountId) { + if let Some(handle) = self.actor_registry.remove(account_id) { + handle.cancel_token.cancel(); + } } /// Helper function to send an event to a single account actor. diff --git a/crates/ntx-builder/src/db/errors.rs b/crates/ntx-builder/src/db/errors.rs index 1ea43e382..64a9b3ef1 100644 --- a/crates/ntx-builder/src/db/errors.rs +++ b/crates/ntx-builder/src/db/errors.rs @@ -1,7 +1,5 @@ use deadpool_sync::InteractError; -use crate::db::manager::ConnectionManagerError; - // DATABASE ERRORS // ================================================================================================ @@ -13,13 +11,25 @@ pub enum DatabaseError { Diesel(#[from] diesel::result::Error), #[error("SQLite pool interaction failed: {0}")] InteractError(String), + #[error("deserialization failed: {context}")] + Deserialization { + context: &'static str, + #[source] + source: Box, + }, #[error("schema verification failed")] SchemaVerification(#[from] SchemaVerificationError), - #[error("connection manager error")] - ConnectionManager(#[source] ConnectionManagerError), } impl DatabaseError { + /// Creates a `Deserialization` error with a static context string and the original error. + pub fn deserialization( + context: &'static str, + source: impl std::error::Error + Send + Sync + 'static, + ) -> Self { + Self::Deserialization { context, source: Box::new(source) } + } + /// Converts from `InteractError`. /// /// Required since `InteractError` has at least one enum variant that is _not_ `Send + diff --git a/crates/ntx-builder/src/db/manager.rs b/crates/ntx-builder/src/db/manager.rs index 4234e09dd..aae7b005c 100644 --- a/crates/ntx-builder/src/db/manager.rs +++ b/crates/ntx-builder/src/db/manager.rs @@ -82,5 +82,10 @@ pub(crate) fn configure_connection_on_creation( diesel::sql_query("PRAGMA foreign_keys=ON") .execute(conn) .map_err(ConnectionManagerError::ConnectionParamSetup)?; + + // Set busy timeout so concurrent writers wait instead of immediately failing. + diesel::sql_query("PRAGMA busy_timeout=5000") + .execute(conn) + .map_err(ConnectionManagerError::ConnectionParamSetup)?; Ok(()) } diff --git a/crates/ntx-builder/src/db/migrations/2026020900000_setup/up.sql b/crates/ntx-builder/src/db/migrations/2026020900000_setup/up.sql index 2588a85bd..5ef692d99 100644 --- a/crates/ntx-builder/src/db/migrations/2026020900000_setup/up.sql +++ b/crates/ntx-builder/src/db/migrations/2026020900000_setup/up.sql @@ -27,6 +27,9 @@ CREATE TABLE accounts ( -- At most one committed row per account. CREATE UNIQUE INDEX idx_accounts_committed ON accounts(account_id) WHERE transaction_id IS NULL; +-- At most one inflight row per (account, transaction) pair. +CREATE UNIQUE INDEX idx_accounts_inflight ON accounts(account_id, transaction_id) + WHERE transaction_id IS NOT NULL; CREATE INDEX idx_accounts_account ON accounts(account_id); CREATE INDEX idx_accounts_tx ON accounts(transaction_id) WHERE transaction_id IS NOT NULL; diff --git a/crates/ntx-builder/src/db/mod.rs b/crates/ntx-builder/src/db/mod.rs index febd14f1b..0c23be396 100644 --- a/crates/ntx-builder/src/db/mod.rs +++ b/crates/ntx-builder/src/db/mod.rs @@ -2,15 +2,25 @@ use std::path::PathBuf; use anyhow::Context; use diesel::{Connection, SqliteConnection}; -use tracing::{info, instrument}; +use miden_node_proto::domain::account::NetworkAccountId; +use miden_node_proto::domain::note::SingleTargetNetworkNote; +use miden_protocol::account::Account; +use miden_protocol::account::delta::AccountUpdateDetails; +use miden_protocol::block::{BlockHeader, BlockNumber}; +use miden_protocol::note::Nullifier; +use miden_protocol::transaction::TransactionId; +use tracing::{Instrument, info, instrument}; use crate::COMPONENT; +use crate::actor::inflight_note::InflightNetworkNote; use crate::db::errors::{DatabaseError, DatabaseSetupError}; use crate::db::manager::{ConnectionManager, configure_connection_on_creation}; use crate::db::migrations::apply_migrations; +use crate::db::models::queries; pub mod errors; pub(crate) mod manager; +pub(crate) mod models; mod migrations; mod schema_hash; @@ -20,6 +30,7 @@ pub(crate) mod schema; pub type Result = std::result::Result; +#[derive(Clone)] pub struct Db { pool: deadpool_diesel::Pool>, } @@ -51,7 +62,6 @@ impl Db { } /// Create and commit a transaction with the queries added in the provided closure. - #[allow(dead_code)] pub(crate) async fn transact(&self, msg: M, query: Q) -> std::result::Result where Q: Send @@ -66,10 +76,12 @@ impl Db { let conn = self .pool .get() + .in_current_span() .await .map_err(|e| DatabaseError::ConnectionPoolObtainError(Box::new(e)))?; conn.interact(|conn| <_ as diesel::Connection>::transaction::(conn, query)) + .in_current_span() .await .map_err(|err| E::from(DatabaseError::interact(&msg.to_string(), &err)))? } @@ -86,6 +98,7 @@ impl Db { let conn = self .pool .get() + .in_current_span() .await .map_err(|e| DatabaseError::ConnectionPoolObtainError(Box::new(e)))?; @@ -93,6 +106,7 @@ impl Db { let r = query(conn)?; Ok(r) }) + .in_current_span() .await .map_err(|err| E::from(DatabaseError::interact(&msg.to_string(), &err)))? } @@ -118,4 +132,152 @@ impl Db { me.query("migrations", apply_migrations).await?; Ok(me) } + + // PUBLIC QUERY METHODS + // ============================================================================================ + + /// Returns `true` if there are notes available for consumption by the given account. + pub async fn has_available_notes( + &self, + account_id: NetworkAccountId, + block_num: BlockNumber, + max_attempts: usize, + ) -> Result { + self.query("has_available_notes", move |conn| { + let notes = queries::available_notes(conn, account_id, block_num, max_attempts)?; + Ok(!notes.is_empty()) + }) + .await + } + + /// Drops notes for the given account that have exceeded the maximum attempt count. + pub async fn drop_failing_notes( + &self, + account_id: NetworkAccountId, + max_attempts: usize, + ) -> Result<()> { + self.transact("drop_failing_notes", move |conn| { + queries::drop_failing_notes(conn, account_id, max_attempts) + }) + .await + } + + /// Returns the latest account state and available notes for the given account. + pub async fn select_candidate( + &self, + account_id: NetworkAccountId, + block_num: BlockNumber, + max_note_attempts: usize, + ) -> Result<(Option, Vec)> { + self.query("select_candidate", move |conn| { + let account = queries::latest_account(conn, account_id)?; + let notes = queries::available_notes(conn, account_id, block_num, max_note_attempts)?; + Ok((account, notes)) + }) + .await + } + + /// Marks notes as failed by incrementing `attempt_count` and setting `last_attempt`. + pub async fn notes_failed( + &self, + nullifiers: Vec, + block_num: BlockNumber, + ) -> Result<()> { + self.transact("notes_failed", move |conn| { + queries::notes_failed(conn, &nullifiers, block_num) + }) + .await + } + + /// Handles a `TransactionAdded` mempool event by writing effects to the DB. + pub async fn handle_transaction_added( + &self, + tx_id: TransactionId, + account_delta: Option, + notes: Vec, + nullifiers: Vec, + ) -> Result<()> { + self.transact("handle_transaction_added", move |conn| { + queries::handle_transaction_added( + conn, + &tx_id, + account_delta.as_ref(), + ¬es, + &nullifiers, + ) + }) + .await + } + + /// Handles a `BlockCommitted` mempool event by committing transaction effects. + pub async fn handle_block_committed( + &self, + txs: Vec, + block_num: BlockNumber, + header: BlockHeader, + ) -> Result<()> { + self.transact("handle_block_committed", move |conn| { + queries::handle_block_committed(conn, &txs, block_num, &header) + }) + .await + } + + /// Handles a `TransactionsReverted` mempool event by undoing transaction effects. + /// + /// Returns the list of account IDs whose creation was reverted. + pub async fn handle_transactions_reverted( + &self, + tx_ids: Vec, + ) -> Result> { + self.transact("handle_transactions_reverted", move |conn| { + queries::handle_transactions_reverted(conn, &tx_ids) + }) + .await + } + + /// Purges all inflight state. Called on startup to get a clean slate. + pub async fn purge_inflight(&self) -> Result<()> { + self.transact("purge_inflight", queries::purge_inflight).await + } + + /// Inserts or replaces the singleton chain state row. + pub async fn upsert_chain_state( + &self, + block_num: BlockNumber, + header: BlockHeader, + ) -> Result<()> { + self.transact("upsert_chain_state", move |conn| { + queries::upsert_chain_state(conn, block_num, &header) + }) + .await + } + + /// Syncs an account and its notes from the store into the DB. + pub async fn sync_account_from_store( + &self, + account_id: NetworkAccountId, + account: Account, + notes: Vec, + ) -> Result<()> { + self.transact("sync_account_from_store", move |conn| { + queries::upsert_committed_account(conn, account_id, &account)?; + queries::insert_committed_notes(conn, ¬es)?; + Ok(()) + }) + .await + } + + /// Creates an in-memory SQLite connection for testing with migrations applied. + /// + /// This bypasses the async connection pool entirely, matching the store crate's test pattern. + #[cfg(test)] + pub fn test_conn() -> SqliteConnection { + use crate::db::manager::configure_connection_on_creation; + + let mut conn = + SqliteConnection::establish(":memory:").expect("in-memory sqlite should always work"); + configure_connection_on_creation(&mut conn).expect("connection configuration should work"); + apply_migrations(&mut conn).expect("migrations should apply on empty database"); + conn + } } diff --git a/crates/ntx-builder/src/db/models/conv.rs b/crates/ntx-builder/src/db/models/conv.rs new file mode 100644 index 000000000..f4906c37b --- /dev/null +++ b/crates/ntx-builder/src/db/models/conv.rs @@ -0,0 +1,79 @@ +//! Conversions between Miden domain types and database column types. + +use miden_node_proto::domain::account::NetworkAccountId; +use miden_node_proto::domain::note::SingleTargetNetworkNote; +use miden_node_proto::generated as proto; +use miden_protocol::account::{Account, AccountId}; +use miden_protocol::block::{BlockHeader, BlockNumber}; +use miden_protocol::note::{Note, Nullifier}; +use miden_protocol::transaction::TransactionId; +use miden_tx::utils::{Deserializable, Serializable}; +use prost::Message; + +use crate::db::errors::DatabaseError; + +// SERIALIZATION (domain → DB) +// ================================================================================================ + +pub fn account_to_bytes(account: &Account) -> Vec { + account.to_bytes() +} + +pub fn block_header_to_bytes(header: &BlockHeader) -> Vec { + header.to_bytes() +} + +pub fn network_account_id_to_bytes(id: NetworkAccountId) -> Vec { + id.inner().to_bytes() +} + +pub fn transaction_id_to_bytes(id: &TransactionId) -> Vec { + id.to_bytes() +} + +pub fn nullifier_to_bytes(nullifier: &Nullifier) -> Vec { + nullifier.to_bytes() +} + +#[allow(clippy::cast_possible_wrap)] +pub fn block_num_to_i32(block_num: BlockNumber) -> i32 { + block_num.as_u32() as i32 +} + +#[allow(clippy::cast_sign_loss)] +pub fn block_num_from_i32(val: i32) -> BlockNumber { + BlockNumber::from(val as u32) +} + +/// Serializes a `SingleTargetNetworkNote` to bytes using its protobuf representation. +pub fn single_target_note_to_bytes(note: &SingleTargetNetworkNote) -> Vec { + let proto_note: proto::note::NetworkNote = Note::from(note.clone()).into(); + proto_note.encode_to_vec() +} + +// DESERIALIZATION (DB → domain) +// ================================================================================================ + +pub fn account_from_bytes(bytes: &[u8]) -> Result { + Account::read_from_bytes(bytes).map_err(|e| DatabaseError::deserialization("account", e)) +} + +pub fn account_id_from_bytes(bytes: &[u8]) -> Result { + AccountId::read_from_bytes(bytes).map_err(|e| DatabaseError::deserialization("account id", e)) +} + +pub fn network_account_id_from_bytes(bytes: &[u8]) -> Result { + let account_id = account_id_from_bytes(bytes)?; + NetworkAccountId::try_from(account_id) + .map_err(|e| DatabaseError::deserialization("network account id", e)) +} + +/// Deserializes a `SingleTargetNetworkNote` from its protobuf byte representation. +pub fn single_target_note_from_bytes( + bytes: &[u8], +) -> Result { + let proto_note = proto::note::NetworkNote::decode(bytes) + .map_err(|e| DatabaseError::deserialization("network note proto", e))?; + SingleTargetNetworkNote::try_from(proto_note) + .map_err(|e| DatabaseError::deserialization("network note conversion", e)) +} diff --git a/crates/ntx-builder/src/db/models/mod.rs b/crates/ntx-builder/src/db/models/mod.rs new file mode 100644 index 000000000..405fe0814 --- /dev/null +++ b/crates/ntx-builder/src/db/models/mod.rs @@ -0,0 +1,3 @@ +pub(crate) mod conv; + +pub mod queries; diff --git a/crates/ntx-builder/src/db/models/queries/accounts.rs b/crates/ntx-builder/src/db/models/queries/accounts.rs new file mode 100644 index 000000000..861b60ff8 --- /dev/null +++ b/crates/ntx-builder/src/db/models/queries/accounts.rs @@ -0,0 +1,92 @@ +//! Account-related queries and models. + +use diesel::prelude::*; +use miden_node_proto::domain::account::NetworkAccountId; +use miden_protocol::account::Account; + +use crate::db::errors::DatabaseError; +use crate::db::models::conv as conversions; +use crate::db::schema; + +// MODELS +// ================================================================================================ + +/// Row for inserting into the unified `accounts` table. +/// +/// `transaction_id = None` means committed; `Some(tx_id_bytes)` means inflight. +#[derive(Debug, Clone, Insertable)] +#[diesel(table_name = schema::accounts)] +#[diesel(check_for_backend(diesel::sqlite::Sqlite))] +pub struct AccountInsert { + pub account_id: Vec, + pub account_data: Vec, + pub transaction_id: Option>, +} + +/// Row read from `accounts`. +#[derive(Debug, Clone, Queryable, Selectable)] +#[diesel(table_name = schema::accounts)] +#[diesel(check_for_backend(diesel::sqlite::Sqlite))] +pub struct AccountRow { + pub account_data: Vec, +} + +// QUERIES +// ================================================================================================ + +/// Inserts or replaces the committed account state (`transaction_id = NULL`). +/// +/// Uses `INSERT OR REPLACE` which triggers on the partial unique index +/// `idx_accounts_committed(account_id) WHERE transaction_id IS NULL`. +/// +/// # Raw SQL +/// +/// ```sql +/// INSERT OR REPLACE INTO accounts (account_id, account_data, transaction_id) +/// VALUES (?1, ?2, NULL) +/// ``` +pub fn upsert_committed_account( + conn: &mut SqliteConnection, + account_id: NetworkAccountId, + account: &Account, +) -> Result<(), DatabaseError> { + let row = AccountInsert { + account_id: conversions::network_account_id_to_bytes(account_id), + account_data: conversions::account_to_bytes(account), + transaction_id: None, + }; + diesel::replace_into(schema::accounts::table).values(&row).execute(conn)?; + Ok(()) +} + +/// Returns the latest account state: last inflight row (highest `order_id`), or committed if +/// none. +/// +/// # Raw SQL +/// +/// ```sql +/// SELECT account_data +/// FROM accounts +/// WHERE account_id = ?1 +/// ORDER BY order_id DESC +/// LIMIT 1 +/// ``` +pub fn latest_account( + conn: &mut SqliteConnection, + account_id: NetworkAccountId, +) -> Result, DatabaseError> { + let account_id_bytes = conversions::network_account_id_to_bytes(account_id); + + // ORDER BY order_id DESC returns the latest inflight first, then committed. + let row: Option = schema::accounts::table + .filter(schema::accounts::account_id.eq(&account_id_bytes)) + .order(schema::accounts::order_id.desc()) + .select(AccountRow::as_select()) + .first(conn) + .optional()?; + + match row { + Some(r) => Ok(Some(conversions::account_from_bytes(&r.account_data)?)), + None => Ok(None), + } +} diff --git a/crates/ntx-builder/src/db/models/queries/chain_state.rs b/crates/ntx-builder/src/db/models/queries/chain_state.rs new file mode 100644 index 000000000..61eb24d09 --- /dev/null +++ b/crates/ntx-builder/src/db/models/queries/chain_state.rs @@ -0,0 +1,49 @@ +//! Chain state queries and models. + +use diesel::prelude::*; +use miden_protocol::block::{BlockHeader, BlockNumber}; + +use crate::db::errors::DatabaseError; +use crate::db::models::conv as conversions; +use crate::db::schema; + +// MODELS +// ================================================================================================ + +#[derive(Debug, Clone, Insertable)] +#[diesel(table_name = schema::chain_state)] +#[diesel(check_for_backend(diesel::sqlite::Sqlite))] +pub struct ChainStateInsert { + /// Singleton row ID. Always `Some(0)` to satisfy the `CHECK (id = 0)` constraint. + /// + /// Mapped as `Option` because SQLite's `INTEGER PRIMARY KEY` maps to `Nullable` + /// in diesel's generated schema. + pub id: Option, + pub block_num: i32, + pub block_header: Vec, +} + +// QUERIES +// ================================================================================================ + +/// Inserts or replaces the singleton chain state row. +/// +/// # Raw SQL +/// +/// ```sql +/// INSERT OR REPLACE INTO chain_state (id, block_num, block_header) +/// VALUES (0, ?1, ?2) +/// ``` +pub fn upsert_chain_state( + conn: &mut SqliteConnection, + block_num: BlockNumber, + block_header: &BlockHeader, +) -> Result<(), DatabaseError> { + let row = ChainStateInsert { + id: Some(0), + block_num: conversions::block_num_to_i32(block_num), + block_header: conversions::block_header_to_bytes(block_header), + }; + diesel::replace_into(schema::chain_state::table).values(&row).execute(conn)?; + Ok(()) +} diff --git a/crates/ntx-builder/src/db/models/queries/mod.rs b/crates/ntx-builder/src/db/models/queries/mod.rs new file mode 100644 index 000000000..43f33bc27 --- /dev/null +++ b/crates/ntx-builder/src/db/models/queries/mod.rs @@ -0,0 +1,316 @@ +//! Database query functions for the NTX builder. + +use diesel::prelude::*; +use miden_node_proto::domain::account::NetworkAccountId; +use miden_node_proto::domain::note::SingleTargetNetworkNote; +use miden_protocol::account::delta::AccountUpdateDetails; +use miden_protocol::block::{BlockHeader, BlockNumber}; +use miden_protocol::note::Nullifier; +use miden_protocol::transaction::TransactionId; + +use crate::actor::account_effect::NetworkAccountEffect; +use crate::db::errors::DatabaseError; +use crate::db::models::conv as conversions; +use crate::db::schema; + +mod accounts; +pub use accounts::*; + +mod chain_state; +pub use chain_state::*; + +mod notes; +pub use notes::*; + +#[cfg(test)] +mod tests; + +// STARTUP QUERIES +// ================================================================================================ + +/// Purges all inflight state. Called on startup to get a clean state. +/// +/// - Deletes account rows with `transaction_id IS NOT NULL`. +/// - Deletes note rows with `created_by IS NOT NULL`. +/// - Sets `consumed_by = NULL` on notes consumed by inflight transactions. +/// +/// # Raw SQL +/// +/// ```sql +/// DELETE FROM accounts WHERE transaction_id IS NOT NULL +/// +/// DELETE FROM notes WHERE created_by IS NOT NULL +/// +/// UPDATE notes SET consumed_by = NULL WHERE consumed_by IS NOT NULL +/// ``` +pub fn purge_inflight(conn: &mut SqliteConnection) -> Result<(), DatabaseError> { + // Delete inflight account rows. + diesel::delete(schema::accounts::table.filter(schema::accounts::transaction_id.is_not_null())) + .execute(conn)?; + + // Delete inflight-created notes. + diesel::delete(schema::notes::table.filter(schema::notes::created_by.is_not_null())) + .execute(conn)?; + + // Un-nullify notes consumed by inflight transactions. + diesel::update(schema::notes::table.filter(schema::notes::consumed_by.is_not_null())) + .set(schema::notes::consumed_by.eq(None::>)) + .execute(conn)?; + + Ok(()) +} + +// MEMPOOL EVENT HANDLERS +// ================================================================================================ + +/// Handles a `TransactionAdded` event by writing effects to the DB. +/// +/// # Raw SQL +/// +/// For account updates (applies delta to latest state and inserts inflight row): +/// +/// ```sql +/// -- Fetch latest account (see latest_account) +/// INSERT INTO accounts (account_id, transaction_id, account_data) +/// VALUES (?1, ?2, ?3) +/// ``` +/// +/// Per note (idempotent via `INSERT OR IGNORE`): +/// +/// ```sql +/// INSERT OR IGNORE INTO notes +/// (nullifier, account_id, note_data, attempt_count, last_attempt, created_by, consumed_by) +/// VALUES (?1, ?2, ?3, 0, NULL, ?4, NULL) +/// ``` +/// +/// Per nullifier (marks notes as consumed): +/// +/// ```sql +/// UPDATE notes +/// SET consumed_by = ?1 +/// WHERE nullifier = ?2 AND consumed_by IS NULL +/// ``` +pub fn handle_transaction_added( + conn: &mut SqliteConnection, + tx_id: &TransactionId, + account_delta: Option<&AccountUpdateDetails>, + notes: &[SingleTargetNetworkNote], + nullifiers: &[Nullifier], +) -> Result<(), DatabaseError> { + let tx_id_bytes = conversions::transaction_id_to_bytes(tx_id); + + // Process account delta. + if let Some(update) = account_delta.and_then(NetworkAccountEffect::from_protocol) { + let account_id = update.network_account_id(); + match update { + NetworkAccountEffect::Updated(ref account_delta) => { + // Query latest_account, apply delta, insert inflight row. + let current_account = + latest_account(conn, account_id)?.expect("account must exist to apply delta"); + let mut updated = current_account; + updated.apply_delta(account_delta).expect( + "network account delta should apply since it was accepted by the mempool", + ); + + let insert = AccountInsert { + account_id: conversions::network_account_id_to_bytes(account_id), + transaction_id: Some(tx_id_bytes.clone()), + account_data: conversions::account_to_bytes(&updated), + }; + diesel::insert_into(schema::accounts::table).values(&insert).execute(conn)?; + }, + NetworkAccountEffect::Created(ref account) => { + let insert = AccountInsert { + account_id: conversions::network_account_id_to_bytes(account_id), + transaction_id: Some(tx_id_bytes.clone()), + account_data: conversions::account_to_bytes(account), + }; + diesel::insert_into(schema::accounts::table).values(&insert).execute(conn)?; + }, + } + } + + // Insert notes with created_by = tx_id. + // Uses INSERT OR IGNORE to make this idempotent if the same event is delivered twice + // (the nullifier PK would otherwise cause a constraint violation). + for note in notes { + let insert = NoteInsert { + nullifier: conversions::nullifier_to_bytes(¬e.nullifier()), + account_id: conversions::network_account_id_to_bytes(note.account_id()), + note_data: conversions::single_target_note_to_bytes(note), + attempt_count: 0, + last_attempt: None, + created_by: Some(tx_id_bytes.clone()), + consumed_by: None, + }; + diesel::insert_or_ignore_into(schema::notes::table) + .values(&insert) + .execute(conn)?; + } + + // Mark consumed notes: set consumed_by = tx_id for matching nullifiers. + for nullifier in nullifiers { + let nullifier_bytes = conversions::nullifier_to_bytes(nullifier); + + // Only mark notes that are not already consumed. + diesel::update( + schema::notes::table + .find(&nullifier_bytes) + .filter(schema::notes::consumed_by.is_null()), + ) + .set(schema::notes::consumed_by.eq(Some(&tx_id_bytes))) + .execute(conn)?; + } + + Ok(()) +} + +/// Handles a `BlockCommitted` event by committing transaction effects. +/// +/// # Raw SQL +/// +/// Per committed transaction: +/// +/// ```sql +/// -- Find inflight accounts for this tx +/// SELECT account_id FROM accounts WHERE transaction_id = ?1 +/// +/// -- Delete old committed row +/// DELETE FROM accounts WHERE account_id = ?1 AND transaction_id IS NULL +/// +/// -- Promote inflight row to committed +/// UPDATE accounts SET transaction_id = NULL +/// WHERE account_id = ?1 AND transaction_id = ?2 +/// +/// -- Delete consumed notes +/// DELETE FROM notes WHERE consumed_by = ?1 +/// +/// -- Promote inflight-created notes to committed +/// UPDATE notes SET created_by = NULL WHERE created_by = ?1 +/// ``` +/// +/// Finally updates chain state (see [`upsert_chain_state`]). +pub fn handle_block_committed( + conn: &mut SqliteConnection, + tx_ids: &[TransactionId], + block_num: BlockNumber, + block_header: &BlockHeader, +) -> Result<(), DatabaseError> { + for tx_id in tx_ids { + let tx_id_bytes = conversions::transaction_id_to_bytes(tx_id); + + // Promote inflight account rows: delete old committed, set transaction_id = NULL. + // Find accounts that have an inflight row for this tx. + let inflight_account_ids: Vec> = schema::accounts::table + .filter(schema::accounts::transaction_id.eq(&tx_id_bytes)) + .select(schema::accounts::account_id) + .load(conn)?; + + for account_id_bytes in &inflight_account_ids { + // Delete the old committed row for this account. + diesel::delete( + schema::accounts::table + .filter(schema::accounts::account_id.eq(account_id_bytes)) + .filter(schema::accounts::transaction_id.is_null()), + ) + .execute(conn)?; + + // Promote the inflight row to committed (set transaction_id = NULL). + // Only promote the row for this specific tx. + diesel::update( + schema::accounts::table + .filter(schema::accounts::account_id.eq(account_id_bytes)) + .filter(schema::accounts::transaction_id.eq(&tx_id_bytes)), + ) + .set(schema::accounts::transaction_id.eq(None::>)) + .execute(conn)?; + } + + // Delete consumed notes (consumed_by = tx_id). + diesel::delete(schema::notes::table.filter(schema::notes::consumed_by.eq(&tx_id_bytes))) + .execute(conn)?; + + // Promote inflight-created notes to committed (set created_by = NULL). + diesel::update(schema::notes::table.filter(schema::notes::created_by.eq(&tx_id_bytes))) + .set(schema::notes::created_by.eq(None::>)) + .execute(conn)?; + } + + // Update chain state. + upsert_chain_state(conn, block_num, block_header)?; + + Ok(()) +} + +/// Handles a `TransactionsReverted` event by undoing transaction effects. +/// +/// Returns the list of account IDs whose creation was reverted (no committed row exists for that +/// account after removing the inflight rows). +/// +/// # Raw SQL +/// +/// Per reverted transaction: +/// +/// ```sql +/// -- Find affected accounts +/// SELECT account_id FROM accounts WHERE transaction_id = ?1 +/// +/// -- Delete inflight account rows +/// DELETE FROM accounts WHERE transaction_id = ?1 +/// +/// -- Check if account creation was fully reverted +/// SELECT COUNT(*) FROM accounts WHERE account_id = ?1 +/// +/// -- Delete inflight-created notes +/// DELETE FROM notes WHERE created_by = ?1 +/// +/// -- Restore consumed notes +/// UPDATE notes SET consumed_by = NULL WHERE consumed_by = ?1 +/// ``` +pub fn handle_transactions_reverted( + conn: &mut SqliteConnection, + tx_ids: &[TransactionId], +) -> Result, DatabaseError> { + let mut reverted_accounts = Vec::new(); + + for tx_id in tx_ids { + let tx_id_bytes = conversions::transaction_id_to_bytes(tx_id); + + // Find accounts affected by this transaction. + let affected_account_ids: Vec> = schema::accounts::table + .filter(schema::accounts::transaction_id.eq(&tx_id_bytes)) + .select(schema::accounts::account_id) + .load(conn)?; + + // Delete inflight account rows for this tx. + diesel::delete( + schema::accounts::table.filter(schema::accounts::transaction_id.eq(&tx_id_bytes)), + ) + .execute(conn)?; + + // Check if any affected accounts had their creation fully reverted + // (no committed row and no remaining inflight rows). + for account_id_bytes in &affected_account_ids { + let remaining: i64 = schema::accounts::table + .filter(schema::accounts::account_id.eq(account_id_bytes)) + .count() + .get_result(conn)?; + + if remaining == 0 { + let account_id = conversions::network_account_id_from_bytes(account_id_bytes)?; + reverted_accounts.push(account_id); + } + } + + // Delete inflight-created notes (created_by = tx_id). + diesel::delete(schema::notes::table.filter(schema::notes::created_by.eq(&tx_id_bytes))) + .execute(conn)?; + + // Un-nullify consumed notes (set consumed_by = NULL where consumed_by = tx_id). + diesel::update(schema::notes::table.filter(schema::notes::consumed_by.eq(&tx_id_bytes))) + .set(schema::notes::consumed_by.eq(None::>)) + .execute(conn)?; + } + + Ok(reverted_accounts) +} diff --git a/crates/ntx-builder/src/db/models/queries/notes.rs b/crates/ntx-builder/src/db/models/queries/notes.rs new file mode 100644 index 000000000..c02b6ece3 --- /dev/null +++ b/crates/ntx-builder/src/db/models/queries/notes.rs @@ -0,0 +1,193 @@ +//! Note-related queries and models. + +use diesel::prelude::*; +use miden_node_proto::domain::account::NetworkAccountId; +use miden_node_proto::domain::note::SingleTargetNetworkNote; +use miden_protocol::block::BlockNumber; +use miden_protocol::note::Nullifier; + +use crate::actor::inflight_note::InflightNetworkNote; +use crate::db::errors::DatabaseError; +use crate::db::models::conv as conversions; +use crate::db::schema; + +// MODELS +// ================================================================================================ + +/// Row read from the unified `notes` table. +#[derive(Debug, Clone, Queryable, Selectable)] +#[diesel(table_name = schema::notes)] +#[diesel(check_for_backend(diesel::sqlite::Sqlite))] +pub struct NoteRow { + pub note_data: Vec, + pub attempt_count: i32, + pub last_attempt: Option, +} + +/// Row for inserting into the unified `notes` table. +#[derive(Debug, Clone, Insertable)] +#[diesel(table_name = schema::notes)] +#[diesel(check_for_backend(diesel::sqlite::Sqlite))] +pub struct NoteInsert { + pub nullifier: Vec, + pub account_id: Vec, + pub note_data: Vec, + pub attempt_count: i32, + pub last_attempt: Option, + pub created_by: Option>, + pub consumed_by: Option>, +} + +// QUERIES +// ================================================================================================ + +/// Batch inserts committed notes (`created_by = NULL`, `consumed_by = NULL`). +/// +/// # Raw SQL +/// +/// Per note: +/// +/// ```sql +/// INSERT OR REPLACE INTO notes +/// (nullifier, account_id, note_data, attempt_count, last_attempt, created_by, consumed_by) +/// VALUES (?1, ?2, ?3, 0, NULL, NULL, NULL) +/// ``` +pub fn insert_committed_notes( + conn: &mut SqliteConnection, + notes: &[SingleTargetNetworkNote], +) -> Result<(), DatabaseError> { + for note in notes { + let row = NoteInsert { + nullifier: conversions::nullifier_to_bytes(¬e.nullifier()), + account_id: conversions::network_account_id_to_bytes(note.account_id()), + note_data: conversions::single_target_note_to_bytes(note), + attempt_count: 0, + last_attempt: None, + created_by: None, + consumed_by: None, + }; + diesel::replace_into(schema::notes::table).values(&row).execute(conn)?; + } + Ok(()) +} + +/// Returns notes available for consumption by a given account. +/// +/// Queries unconsumed notes (`consumed_by IS NULL`) for the account that have not exceeded the +/// maximum attempt count, then applies backoff filtering in Rust via +/// `InflightNetworkNote::is_available`. +/// +/// # Raw SQL +/// +/// ```sql +/// SELECT note_data, attempt_count, last_attempt +/// FROM notes +/// WHERE +/// account_id = ?1 +/// AND consumed_by IS NULL +/// AND attempt_count < ?2 +/// ``` +#[allow(clippy::cast_possible_wrap)] +pub fn available_notes( + conn: &mut SqliteConnection, + account_id: NetworkAccountId, + block_num: BlockNumber, + max_attempts: usize, +) -> Result, DatabaseError> { + let account_id_bytes = conversions::network_account_id_to_bytes(account_id); + + // Get unconsumed notes for this account that haven't exceeded the max attempt count. + let rows: Vec = schema::notes::table + .filter(schema::notes::account_id.eq(&account_id_bytes)) + .filter(schema::notes::consumed_by.is_null()) + .filter(schema::notes::attempt_count.lt(max_attempts as i32)) + .select(NoteRow::as_select()) + .load(conn)?; + + let mut result = Vec::new(); + for row in rows { + #[allow(clippy::cast_sign_loss)] + let attempt_count = row.attempt_count as usize; + let note = note_row_to_inflight( + &row.note_data, + attempt_count, + row.last_attempt.map(conversions::block_num_from_i32), + )?; + if note.is_available(block_num) { + result.push(note); + } + } + + Ok(result) +} + +/// Marks notes as failed by incrementing `attempt_count` and setting `last_attempt`. +/// +/// # Raw SQL +/// +/// Per nullifier: +/// +/// ```sql +/// UPDATE notes +/// SET attempt_count = attempt_count + 1, last_attempt = ?1 +/// WHERE nullifier = ?2 +/// ``` +pub fn notes_failed( + conn: &mut SqliteConnection, + nullifiers: &[Nullifier], + block_num: BlockNumber, +) -> Result<(), DatabaseError> { + let block_num_val = conversions::block_num_to_i32(block_num); + + for nullifier in nullifiers { + let nullifier_bytes = conversions::nullifier_to_bytes(nullifier); + + diesel::update(schema::notes::table.find(&nullifier_bytes)) + .set(( + schema::notes::attempt_count.eq(schema::notes::attempt_count + 1), + schema::notes::last_attempt.eq(Some(block_num_val)), + )) + .execute(conn)?; + } + Ok(()) +} + +/// Drops notes for the given account that have exceeded the maximum attempt count. +/// +/// # Raw SQL +/// +/// ```sql +/// DELETE FROM notes +/// WHERE account_id = ?1 AND attempt_count >= ?2 +/// ``` +#[allow(clippy::cast_possible_wrap)] +pub fn drop_failing_notes( + conn: &mut SqliteConnection, + account_id: NetworkAccountId, + max_attempts: usize, +) -> Result<(), DatabaseError> { + let account_id_bytes = conversions::network_account_id_to_bytes(account_id); + let max_attempts = max_attempts as i32; + + diesel::delete( + schema::notes::table + .filter(schema::notes::account_id.eq(&account_id_bytes)) + .filter(schema::notes::attempt_count.ge(max_attempts)), + ) + .execute(conn)?; + + Ok(()) +} + +// HELPERS +// ================================================================================================ + +/// Constructs an `InflightNetworkNote` from DB row fields. +fn note_row_to_inflight( + note_data: &[u8], + attempt_count: usize, + last_attempt: Option, +) -> Result { + let note = conversions::single_target_note_from_bytes(note_data)?; + Ok(InflightNetworkNote::from_parts(note, attempt_count, last_attempt)) +} diff --git a/crates/ntx-builder/src/db/models/queries/tests.rs b/crates/ntx-builder/src/db/models/queries/tests.rs new file mode 100644 index 000000000..30523ae44 --- /dev/null +++ b/crates/ntx-builder/src/db/models/queries/tests.rs @@ -0,0 +1,553 @@ +//! DB-level tests for NTX builder query functions. + +use diesel::prelude::*; +use miden_node_proto::domain::account::NetworkAccountId; +use miden_node_proto::domain::note::SingleTargetNetworkNote; +use miden_protocol::Word; +use miden_protocol::account::{AccountId, AccountStorageMode, AccountType}; +use miden_protocol::block::BlockNumber; +use miden_protocol::note::NoteExecutionHint; +use miden_protocol::testing::account_id::{ + ACCOUNT_ID_REGULAR_NETWORK_ACCOUNT_IMMUTABLE_CODE, + AccountIdBuilder, +}; +use miden_protocol::transaction::TransactionId; +use miden_standards::note::NetworkAccountTarget; +use miden_standards::testing::note::NoteBuilder; +use rand_chacha::ChaCha20Rng; +use rand_chacha::rand_core::SeedableRng; + +use super::*; +use crate::db::models::conv as conversions; +use crate::db::{Db, schema}; + +// TEST HELPERS +// ================================================================================================ + +/// Creates an in-memory SQLite connection with migrations applied. +fn test_conn() -> SqliteConnection { + Db::test_conn() +} + +/// Creates a network account ID from a test constant. +fn mock_network_account_id() -> NetworkAccountId { + let account_id: AccountId = + ACCOUNT_ID_REGULAR_NETWORK_ACCOUNT_IMMUTABLE_CODE.try_into().unwrap(); + NetworkAccountId::try_from(account_id).unwrap() +} + +/// Creates a distinct network account ID using a seeded RNG. +fn mock_network_account_id_seeded(seed: u8) -> NetworkAccountId { + let account_id = AccountIdBuilder::new() + .account_type(AccountType::RegularAccountImmutableCode) + .storage_mode(AccountStorageMode::Network) + .build_with_seed([seed; 32]); + NetworkAccountId::try_from(account_id).unwrap() +} + +/// Creates a unique `TransactionId` from a seed value. +fn mock_tx_id(seed: u64) -> TransactionId { + let w = |n: u64| Word::try_from([n, 0, 0, 0]).unwrap(); + TransactionId::new(w(seed), w(seed + 1), w(seed + 2), w(seed + 3)) +} + +/// Creates a `SingleTargetNetworkNote` targeting the given network account. +fn mock_single_target_note( + network_account_id: NetworkAccountId, + seed: u8, +) -> SingleTargetNetworkNote { + let mut rng = ChaCha20Rng::from_seed([seed; 32]); + let sender = AccountIdBuilder::new() + .account_type(AccountType::RegularAccountImmutableCode) + .storage_mode(AccountStorageMode::Private) + .build_with_rng(&mut rng); + + let target = NetworkAccountTarget::new(network_account_id.inner(), NoteExecutionHint::Always) + .expect("network account should be valid target"); + + let note = NoteBuilder::new(sender, rng).attachment(target).build().unwrap(); + + SingleTargetNetworkNote::try_from(note).expect("note should be single-target network note") +} + +/// Counts the total number of rows in the `notes` table. +fn count_notes(conn: &mut SqliteConnection) -> i64 { + schema::notes::table.count().get_result(conn).unwrap() +} + +/// Counts the total number of rows in the `accounts` table. +fn count_accounts(conn: &mut SqliteConnection) -> i64 { + schema::accounts::table.count().get_result(conn).unwrap() +} + +/// Counts inflight account rows. +fn count_inflight_accounts(conn: &mut SqliteConnection) -> i64 { + schema::accounts::table + .filter(schema::accounts::transaction_id.is_not_null()) + .count() + .get_result(conn) + .unwrap() +} + +/// Counts committed account rows. +fn count_committed_accounts(conn: &mut SqliteConnection) -> i64 { + schema::accounts::table + .filter(schema::accounts::transaction_id.is_null()) + .count() + .get_result(conn) + .unwrap() +} + +// PURGE INFLIGHT TESTS +// ================================================================================================ + +#[test] +fn purge_inflight_clears_all_inflight_state() { + let conn = &mut test_conn(); + + let account_id = mock_network_account_id(); + let tx_id = mock_tx_id(1); + let note = mock_single_target_note(account_id, 10); + + // Insert committed account. + upsert_committed_account(conn, account_id, &mock_account(account_id)).unwrap(); + + // Insert a transaction (creates inflight account row + note + consumption). + handle_transaction_added(conn, &tx_id, None, std::slice::from_ref(¬e), &[]).unwrap(); + + assert!(count_inflight_accounts(conn) == 0); // No account delta, so no inflight account. + assert_eq!(count_notes(conn), 1); + + // Mark note as consumed by another tx. + let tx_id2 = mock_tx_id(2); + handle_transaction_added(conn, &tx_id2, None, &[], &[note.nullifier()]).unwrap(); + + // Verify consumed_by is set. + let consumed_count: i64 = schema::notes::table + .filter(schema::notes::consumed_by.is_not_null()) + .count() + .get_result(conn) + .unwrap(); + assert_eq!(consumed_count, 1); + + // Purge inflight state. + purge_inflight(conn).unwrap(); + + // Inflight accounts should be gone. + assert_eq!(count_inflight_accounts(conn), 0); + // Committed account should remain. + assert_eq!(count_committed_accounts(conn), 1); + // Inflight-created notes should be gone. + assert_eq!(count_notes(conn), 0); +} + +// HANDLE TRANSACTION ADDED TESTS +// ================================================================================================ + +#[test] +fn transaction_added_inserts_notes_and_marks_consumed() { + let conn = &mut test_conn(); + + let account_id = mock_network_account_id(); + let tx_id = mock_tx_id(1); + let note1 = mock_single_target_note(account_id, 10); + let note2 = mock_single_target_note(account_id, 20); + + // Insert committed note first (to test consumption). + insert_committed_notes(conn, std::slice::from_ref(¬e1)).unwrap(); + assert_eq!(count_notes(conn), 1); + + // Add transaction that creates note2 and consumes note1. + handle_transaction_added( + conn, + &tx_id, + None, + std::slice::from_ref(¬e2), + &[note1.nullifier()], + ) + .unwrap(); + + // Should now have 2 notes total. + assert_eq!(count_notes(conn), 2); + + // note1 should be consumed. + let consumed: Option> = schema::notes::table + .find(conversions::nullifier_to_bytes(¬e1.nullifier())) + .select(schema::notes::consumed_by) + .first(conn) + .unwrap(); + assert!(consumed.is_some()); + + // note2 should have created_by set. + let created: Option> = schema::notes::table + .find(conversions::nullifier_to_bytes(¬e2.nullifier())) + .select(schema::notes::created_by) + .first(conn) + .unwrap(); + assert!(created.is_some()); +} + +#[test] +fn transaction_added_is_idempotent_for_notes() { + let conn = &mut test_conn(); + + let account_id = mock_network_account_id(); + let tx_id = mock_tx_id(1); + let note = mock_single_target_note(account_id, 10); + + // Insert the same transaction twice. + handle_transaction_added(conn, &tx_id, None, std::slice::from_ref(¬e), &[]).unwrap(); + handle_transaction_added(conn, &tx_id, None, std::slice::from_ref(¬e), &[]).unwrap(); + + // Should only have one note (INSERT OR IGNORE). + assert_eq!(count_notes(conn), 1); +} + +// HANDLE BLOCK COMMITTED TESTS +// ================================================================================================ + +#[test] +fn block_committed_promotes_inflight_notes_to_committed() { + let conn = &mut test_conn(); + + let account_id = mock_network_account_id(); + let tx_id = mock_tx_id(1); + let note = mock_single_target_note(account_id, 10); + let block_num = BlockNumber::from(1u32); + let header = mock_block_header(block_num); + + // Add a transaction that creates a note. + handle_transaction_added(conn, &tx_id, None, std::slice::from_ref(¬e), &[]).unwrap(); + + // Verify created_by is set. + let created: Option> = schema::notes::table + .find(conversions::nullifier_to_bytes(¬e.nullifier())) + .select(schema::notes::created_by) + .first(conn) + .unwrap(); + assert!(created.is_some()); + + // Commit the block. + handle_block_committed(conn, &[tx_id], block_num, &header).unwrap(); + + // created_by should now be NULL (promoted to committed). + let created: Option> = schema::notes::table + .find(conversions::nullifier_to_bytes(¬e.nullifier())) + .select(schema::notes::created_by) + .first(conn) + .unwrap(); + assert!(created.is_none()); +} + +#[test] +fn block_committed_deletes_consumed_notes() { + let conn = &mut test_conn(); + + let account_id = mock_network_account_id(); + let note = mock_single_target_note(account_id, 10); + + // Insert a committed note. + insert_committed_notes(conn, std::slice::from_ref(¬e)).unwrap(); + assert_eq!(count_notes(conn), 1); + + // Consume it via a transaction. + let tx_id = mock_tx_id(1); + handle_transaction_added(conn, &tx_id, None, &[], &[note.nullifier()]).unwrap(); + + // Commit the block. + let block_num = BlockNumber::from(1u32); + let header = mock_block_header(block_num); + handle_block_committed(conn, &[tx_id], block_num, &header).unwrap(); + + // Consumed note should be deleted. + assert_eq!(count_notes(conn), 0); +} + +#[test] +fn block_committed_promotes_inflight_account_to_committed() { + let conn = &mut test_conn(); + + let account_id = mock_network_account_id(); + let account = mock_account(account_id); + + // Insert committed account. + upsert_committed_account(conn, account_id, &account).unwrap(); + assert_eq!(count_committed_accounts(conn), 1); + + // Insert inflight row. + let tx_id = mock_tx_id(1); + let row = AccountInsert { + account_id: conversions::network_account_id_to_bytes(account_id), + transaction_id: Some(conversions::transaction_id_to_bytes(&tx_id)), + account_data: conversions::account_to_bytes(&account), + }; + diesel::insert_into(schema::accounts::table).values(&row).execute(conn).unwrap(); + + assert_eq!(count_inflight_accounts(conn), 1); + assert_eq!(count_committed_accounts(conn), 1); + + // Commit the block. + let block_num = BlockNumber::from(1u32); + let header = mock_block_header(block_num); + handle_block_committed(conn, &[tx_id], block_num, &header).unwrap(); + + // Should have 1 committed and 0 inflight. + assert_eq!(count_committed_accounts(conn), 1); + assert_eq!(count_inflight_accounts(conn), 0); +} + +// HANDLE TRANSACTIONS REVERTED TESTS +// ================================================================================================ + +#[test] +fn transactions_reverted_restores_consumed_notes() { + let conn = &mut test_conn(); + + let account_id = mock_network_account_id(); + let note = mock_single_target_note(account_id, 10); + + // Insert committed note. + insert_committed_notes(conn, std::slice::from_ref(¬e)).unwrap(); + + // Consume it via a transaction. + let tx_id = mock_tx_id(1); + handle_transaction_added(conn, &tx_id, None, &[], &[note.nullifier()]).unwrap(); + + // Verify consumed. + let consumed: Option> = schema::notes::table + .find(conversions::nullifier_to_bytes(¬e.nullifier())) + .select(schema::notes::consumed_by) + .first(conn) + .unwrap(); + assert!(consumed.is_some()); + + // Revert the transaction. + let reverted = handle_transactions_reverted(conn, &[tx_id]).unwrap(); + assert!(reverted.is_empty()); + + // Note should be un-consumed. + let consumed: Option> = schema::notes::table + .find(conversions::nullifier_to_bytes(¬e.nullifier())) + .select(schema::notes::consumed_by) + .first(conn) + .unwrap(); + assert!(consumed.is_none()); +} + +#[test] +fn transactions_reverted_deletes_inflight_created_notes() { + let conn = &mut test_conn(); + + let account_id = mock_network_account_id(); + let tx_id = mock_tx_id(1); + let note = mock_single_target_note(account_id, 10); + + // Add transaction that creates a note. + handle_transaction_added(conn, &tx_id, None, std::slice::from_ref(¬e), &[]).unwrap(); + assert_eq!(count_notes(conn), 1); + + // Revert the transaction. + handle_transactions_reverted(conn, &[tx_id]).unwrap(); + + // Inflight-created note should be deleted. + assert_eq!(count_notes(conn), 0); +} + +#[test] +fn transactions_reverted_reports_reverted_account_creations() { + let conn = &mut test_conn(); + + let account_id = mock_network_account_id(); + let account = mock_account(account_id); + let tx_id = mock_tx_id(1); + + // Insert an inflight account row (simulating account creation by tx). + let row = AccountInsert { + account_id: conversions::network_account_id_to_bytes(account_id), + transaction_id: Some(conversions::transaction_id_to_bytes(&tx_id)), + account_data: conversions::account_to_bytes(&account), + }; + diesel::insert_into(schema::accounts::table).values(&row).execute(conn).unwrap(); + + // Revert the transaction --- account creation should be reported. + let reverted = handle_transactions_reverted(conn, &[tx_id]).unwrap(); + assert_eq!(reverted.len(), 1); + assert_eq!(reverted[0], account_id); + + // Account should be gone. + assert_eq!(count_accounts(conn), 0); +} + +// AVAILABLE NOTES TESTS +// ================================================================================================ + +#[test] +#[allow(clippy::similar_names)] +fn available_notes_filters_consumed_and_exceeded_attempts() { + let conn = &mut test_conn(); + + let account_id = mock_network_account_id(); + let note_good = mock_single_target_note(account_id, 10); + let note_consumed = mock_single_target_note(account_id, 20); + let note_failed = mock_single_target_note(account_id, 30); + + // Insert all as committed. + insert_committed_notes(conn, &[note_good.clone(), note_consumed.clone(), note_failed.clone()]) + .unwrap(); + + // Consume one note. + let tx_id = mock_tx_id(1); + handle_transaction_added(conn, &tx_id, None, &[], &[note_consumed.nullifier()]).unwrap(); + + // Mark one note as failed many times (exceed max_attempts=3). + let block_num = BlockNumber::from(100u32); + notes_failed(conn, &[note_failed.nullifier()], block_num).unwrap(); + notes_failed(conn, &[note_failed.nullifier()], block_num).unwrap(); + notes_failed(conn, &[note_failed.nullifier()], block_num).unwrap(); + + // Query available notes with max_attempts=3. + let result = available_notes(conn, account_id, block_num, 3).unwrap(); + + // Only note_good should be available (note_consumed is consumed, note_failed exceeded + // attempts). + assert_eq!(result.len(), 1); + assert_eq!(result[0].to_inner().nullifier(), note_good.nullifier()); +} + +#[test] +fn available_notes_only_returns_notes_for_specified_account() { + let conn = &mut test_conn(); + + let account_id_1 = mock_network_account_id(); + let account_id_2 = mock_network_account_id_seeded(42); + + let note_acct1 = mock_single_target_note(account_id_1, 10); + let note_acct2 = mock_single_target_note(account_id_2, 20); + + insert_committed_notes(conn, &[note_acct1.clone(), note_acct2]).unwrap(); + + let block_num = BlockNumber::from(100u32); + let result = available_notes(conn, account_id_1, block_num, 30).unwrap(); + + assert_eq!(result.len(), 1); + assert_eq!(result[0].to_inner().nullifier(), note_acct1.nullifier()); +} + +// DROP FAILING NOTES TESTS +// ================================================================================================ + +#[test] +fn drop_failing_notes_scoped_to_account() { + let conn = &mut test_conn(); + + let account_id_1 = mock_network_account_id(); + let account_id_2 = mock_network_account_id_seeded(42); + + let note_acct1 = mock_single_target_note(account_id_1, 10); + let note_acct2 = mock_single_target_note(account_id_2, 20); + + // Insert both as committed. + insert_committed_notes(conn, &[note_acct1.clone(), note_acct2.clone()]).unwrap(); + + // Fail both notes enough times to exceed max_attempts=2. + let block_num = BlockNumber::from(100u32); + notes_failed(conn, &[note_acct1.nullifier()], block_num).unwrap(); + notes_failed(conn, &[note_acct1.nullifier()], block_num).unwrap(); + notes_failed(conn, &[note_acct2.nullifier()], block_num).unwrap(); + notes_failed(conn, &[note_acct2.nullifier()], block_num).unwrap(); + + // Drop failing notes for account_id_1 only. + drop_failing_notes(conn, account_id_1, 2).unwrap(); + + // note_acct1 should be deleted, note_acct2 should remain. + assert_eq!(count_notes(conn), 1); + let remaining: Vec> = + schema::notes::table.select(schema::notes::nullifier).load(conn).unwrap(); + assert_eq!(remaining[0], conversions::nullifier_to_bytes(¬e_acct2.nullifier())); +} + +// NOTES FAILED TESTS +// ================================================================================================ + +#[test] +fn notes_failed_increments_attempt_count() { + let conn = &mut test_conn(); + + let account_id = mock_network_account_id(); + let note = mock_single_target_note(account_id, 10); + + insert_committed_notes(conn, std::slice::from_ref(¬e)).unwrap(); + + let block_num = BlockNumber::from(5u32); + notes_failed(conn, &[note.nullifier()], block_num).unwrap(); + notes_failed(conn, &[note.nullifier()], block_num).unwrap(); + + let (attempt_count, last_attempt): (i32, Option) = schema::notes::table + .find(conversions::nullifier_to_bytes(¬e.nullifier())) + .select((schema::notes::attempt_count, schema::notes::last_attempt)) + .first(conn) + .unwrap(); + + assert_eq!(attempt_count, 2); + assert_eq!(last_attempt, Some(conversions::block_num_to_i32(block_num))); +} + +// CHAIN STATE TESTS +// ================================================================================================ + +#[test] +fn upsert_chain_state_updates_singleton() { + let conn = &mut test_conn(); + + let block_num_1 = BlockNumber::from(1u32); + let header_1 = mock_block_header(block_num_1); + upsert_chain_state(conn, block_num_1, &header_1).unwrap(); + + // Upsert again with higher block. + let block_num_2 = BlockNumber::from(2u32); + let header_2 = mock_block_header(block_num_2); + upsert_chain_state(conn, block_num_2, &header_2).unwrap(); + + // Should only have one row. + let row_count: i64 = schema::chain_state::table.count().get_result(conn).unwrap(); + assert_eq!(row_count, 1); + + // Should have the latest block number. + let stored_block_num: i32 = schema::chain_state::table + .select(schema::chain_state::block_num) + .first(conn) + .unwrap(); + assert_eq!(stored_block_num, conversions::block_num_to_i32(block_num_2)); +} + +// HELPERS (domain type construction) +// ================================================================================================ + +/// Creates a mock `Account` for a network account. +/// +/// Uses `AccountBuilder` with minimal components needed for serialization. +fn mock_account(_account_id: NetworkAccountId) -> miden_protocol::account::Account { + use miden_protocol::account::auth::PublicKeyCommitment; + use miden_protocol::account::{AccountBuilder, AccountComponent}; + use miden_standards::account::auth::AuthFalcon512Rpo; + + let component_code = miden_standards::code_builder::CodeBuilder::default() + .compile_component_code("test::interface", "pub proc test_proc push.1.2 add end") + .unwrap(); + + let component = + AccountComponent::new(component_code, vec![]).unwrap().with_supports_all_types(); + + AccountBuilder::new([0u8; 32]) + .account_type(AccountType::RegularAccountImmutableCode) + .storage_mode(AccountStorageMode::Network) + .with_component(component) + .with_auth_component(AuthFalcon512Rpo::new(PublicKeyCommitment::from(Word::default()))) + .build_existing() + .unwrap() +} + +/// Creates a mock `BlockHeader` for the given block number. +fn mock_block_header(block_num: BlockNumber) -> miden_protocol::block::BlockHeader { + miden_protocol::block::BlockHeader::mock(block_num, None, None, &[], Word::default()) +} diff --git a/crates/ntx-builder/src/lib.rs b/crates/ntx-builder/src/lib.rs index d77a8dd7d..c86f737f2 100644 --- a/crates/ntx-builder/src/lib.rs +++ b/crates/ntx-builder/src/lib.rs @@ -1,4 +1,5 @@ use std::num::NonZeroUsize; +use std::path::PathBuf; use std::sync::Arc; use actor::AccountActorContext; @@ -6,6 +7,7 @@ use anyhow::Context; use block_producer::BlockProducerClient; use builder::{ChainState, MempoolEventStream}; use coordinator::Coordinator; +use db::Db; use futures::TryStreamExt; use miden_node_utils::lru_cache::LruCache; use store::StoreClient; @@ -16,7 +18,6 @@ mod actor; mod block_producer; mod builder; mod coordinator; -#[expect(dead_code, reason = "will be used as part of follow-up work")] pub(crate) mod db; mod store; @@ -96,10 +97,18 @@ pub struct NtxBuilderConfig { /// Channel size for each actor's event channel. pub actor_channel_size: usize, + + /// Path to the SQLite database file used for persistent state. + pub database_filepath: PathBuf, } impl NtxBuilderConfig { - pub fn new(store_url: Url, block_producer_url: Url, validator_url: Url) -> Self { + pub fn new( + store_url: Url, + block_producer_url: Url, + validator_url: Url, + database_filepath: PathBuf, + ) -> Self { Self { store_url, block_producer_url, @@ -112,6 +121,7 @@ impl NtxBuilderConfig { max_block_count: DEFAULT_MAX_BLOCK_COUNT, account_channel_capacity: DEFAULT_ACCOUNT_CHANNEL_CAPACITY, actor_channel_size: DEFAULT_ACTOR_CHANNEL_SIZE, + database_filepath, } } @@ -195,8 +205,19 @@ impl NtxBuilderConfig { /// - The mempool subscription fails (after retries) /// - The store contains no blocks (not bootstrapped) pub async fn build(self) -> anyhow::Result { + // Bootstrap and load the database. + Db::bootstrap(self.database_filepath.clone()) + .context("failed to bootstrap ntx-builder database")?; + let db = Db::load(self.database_filepath.clone()) + .await + .context("failed to load ntx-builder database")?; + + // Purge inflight state from previous run. + db.purge_inflight().await.context("failed to purge inflight state")?; + let script_cache = LruCache::new(self.script_cache_size); - let coordinator = Coordinator::new(self.max_concurrent_txs, self.actor_channel_size); + let coordinator = + Coordinator::new(self.max_concurrent_txs, self.actor_channel_size, db.clone()); let store = StoreClient::new(self.store_url.clone()); let block_producer = BlockProducerClient::new(self.block_producer_url.clone()); @@ -225,6 +246,11 @@ impl NtxBuilderConfig { } }; + // Store the chain tip in the DB. + db.upsert_chain_state(chain_tip_header.block_num(), chain_tip_header.clone()) + .await + .context("failed to upsert chain state")?; + let chain_state = Arc::new(RwLock::new(ChainState::new(chain_tip_header, chain_mmr))); let actor_context = AccountActorContext { @@ -236,12 +262,14 @@ impl NtxBuilderConfig { script_cache, max_notes_per_tx: self.max_notes_per_tx, max_note_attempts: self.max_note_attempts, + db: db.clone(), }; Ok(NetworkTransactionBuilder::new( self, coordinator, store, + db, chain_state, actor_context, mempool_events, From 3e1b7bb711b3758d9726eaa0b3803f97c3860e5c Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Thu, 12 Feb 2026 12:29:14 -0300 Subject: [PATCH 02/15] review: move backoff_has_passed to proper file --- .../ntx-builder/src/actor/account_effect.rs | 31 ------------------- 1 file changed, 31 deletions(-) diff --git a/crates/ntx-builder/src/actor/account_effect.rs b/crates/ntx-builder/src/actor/account_effect.rs index e7d55ccb6..7a6acf005 100644 --- a/crates/ntx-builder/src/actor/account_effect.rs +++ b/crates/ntx-builder/src/actor/account_effect.rs @@ -40,34 +40,3 @@ impl NetworkAccountEffect { } } } - -#[cfg(test)] -mod tests { - use miden_protocol::block::BlockNumber; - - #[rstest::rstest] - #[test] - #[case::all_zero(Some(BlockNumber::GENESIS), BlockNumber::GENESIS, 0, true)] - #[case::no_attempts(None, BlockNumber::GENESIS, 0, true)] - #[case::one_attempt(Some(BlockNumber::GENESIS), BlockNumber::from(2), 1, true)] - #[case::three_attempts(Some(BlockNumber::GENESIS), BlockNumber::from(3), 3, true)] - #[case::ten_attempts(Some(BlockNumber::GENESIS), BlockNumber::from(13), 10, true)] - #[case::twenty_attempts(Some(BlockNumber::GENESIS), BlockNumber::from(149), 20, true)] - #[case::one_attempt_false(Some(BlockNumber::GENESIS), BlockNumber::from(1), 1, false)] - #[case::three_attempts_false(Some(BlockNumber::GENESIS), BlockNumber::from(2), 3, false)] - #[case::ten_attempts_false(Some(BlockNumber::GENESIS), BlockNumber::from(12), 10, false)] - #[case::twenty_attempts_false(Some(BlockNumber::GENESIS), BlockNumber::from(148), 20, false)] - fn backoff_has_passed( - #[case] last_attempt_block_num: Option, - #[case] current_block_num: BlockNumber, - #[case] attempt_count: usize, - #[case] backoff_should_have_passed: bool, - ) { - use crate::actor::has_backoff_passed; - - assert_eq!( - backoff_should_have_passed, - has_backoff_passed(current_block_num, last_attempt_block_num, attempt_count) - ); - } -} From a592a1843d34330dbfe44a3cb1372f3b84f54650 Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Thu, 12 Feb 2026 12:31:13 -0300 Subject: [PATCH 03/15] review: make ID not nullable --- .../src/db/migrations/2026020900000_setup/up.sql | 4 ++-- crates/ntx-builder/src/db/models/queries/chain_state.rs | 9 +++------ crates/ntx-builder/src/db/schema.rs | 4 ++-- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/crates/ntx-builder/src/db/migrations/2026020900000_setup/up.sql b/crates/ntx-builder/src/db/migrations/2026020900000_setup/up.sql index 5ef692d99..d8da128a9 100644 --- a/crates/ntx-builder/src/db/migrations/2026020900000_setup/up.sql +++ b/crates/ntx-builder/src/db/migrations/2026020900000_setup/up.sql @@ -2,7 +2,7 @@ -- The chain MMR is reconstructed on startup from the store and maintained in memory. CREATE TABLE chain_state ( -- Singleton constraint: only one row allowed. - id INTEGER PRIMARY KEY CHECK (id = 0), + id INTEGER NOT NULL PRIMARY KEY CHECK (id = 0), -- Block number of the chain tip. block_num INTEGER NOT NULL, -- Serialized BlockHeader. @@ -16,7 +16,7 @@ CREATE TABLE chain_state ( -- The auto-incrementing order_id preserves insertion order (VecDeque semantics). CREATE TABLE accounts ( -- Auto-incrementing ID preserves insertion order. - order_id INTEGER PRIMARY KEY AUTOINCREMENT, + order_id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, -- AccountId serialized bytes (8 bytes). account_id BLOB NOT NULL, -- Serialized Account state. diff --git a/crates/ntx-builder/src/db/models/queries/chain_state.rs b/crates/ntx-builder/src/db/models/queries/chain_state.rs index 61eb24d09..035c0d5f2 100644 --- a/crates/ntx-builder/src/db/models/queries/chain_state.rs +++ b/crates/ntx-builder/src/db/models/queries/chain_state.rs @@ -14,11 +14,8 @@ use crate::db::schema; #[diesel(table_name = schema::chain_state)] #[diesel(check_for_backend(diesel::sqlite::Sqlite))] pub struct ChainStateInsert { - /// Singleton row ID. Always `Some(0)` to satisfy the `CHECK (id = 0)` constraint. - /// - /// Mapped as `Option` because SQLite's `INTEGER PRIMARY KEY` maps to `Nullable` - /// in diesel's generated schema. - pub id: Option, + /// Singleton row ID. Always `0` to satisfy the `CHECK (id = 0)` constraint. + pub id: i32, pub block_num: i32, pub block_header: Vec, } @@ -40,7 +37,7 @@ pub fn upsert_chain_state( block_header: &BlockHeader, ) -> Result<(), DatabaseError> { let row = ChainStateInsert { - id: Some(0), + id: 0, block_num: conversions::block_num_to_i32(block_num), block_header: conversions::block_header_to_bytes(block_header), }; diff --git a/crates/ntx-builder/src/db/schema.rs b/crates/ntx-builder/src/db/schema.rs index 74ee8d462..1fc97a217 100644 --- a/crates/ntx-builder/src/db/schema.rs +++ b/crates/ntx-builder/src/db/schema.rs @@ -2,7 +2,7 @@ diesel::table! { accounts (order_id) { - order_id -> Nullable, + order_id -> Integer, account_id -> Binary, account_data -> Binary, transaction_id -> Nullable, @@ -11,7 +11,7 @@ diesel::table! { diesel::table! { chain_state (id) { - id -> Nullable, + id -> Integer, block_num -> Integer, block_header -> Binary, } From b99b2bdc798313955698ce9516c04f3e449504f5 Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Thu, 12 Feb 2026 15:01:40 -0300 Subject: [PATCH 04/15] review: re-add test in proper file --- crates/ntx-builder/src/actor/mod.rs | 31 +++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/crates/ntx-builder/src/actor/mod.rs b/crates/ntx-builder/src/actor/mod.rs index 03bf1d1c5..8ab6070ef 100644 --- a/crates/ntx-builder/src/actor/mod.rs +++ b/crates/ntx-builder/src/actor/mod.rs @@ -423,3 +423,34 @@ fn has_backoff_passed( // Check if the backoff period has passed. blocks_passed.as_usize() > backoff_threshold } + +#[cfg(test)] +mod tests { + use miden_protocol::block::BlockNumber; + + use super::has_backoff_passed; + + #[rstest::rstest] + #[test] + #[case::all_zero(Some(BlockNumber::GENESIS), BlockNumber::GENESIS, 0, true)] + #[case::no_attempts(None, BlockNumber::GENESIS, 0, true)] + #[case::one_attempt(Some(BlockNumber::GENESIS), BlockNumber::from(2), 1, true)] + #[case::three_attempts(Some(BlockNumber::GENESIS), BlockNumber::from(3), 3, true)] + #[case::ten_attempts(Some(BlockNumber::GENESIS), BlockNumber::from(13), 10, true)] + #[case::twenty_attempts(Some(BlockNumber::GENESIS), BlockNumber::from(149), 20, true)] + #[case::one_attempt_false(Some(BlockNumber::GENESIS), BlockNumber::from(1), 1, false)] + #[case::three_attempts_false(Some(BlockNumber::GENESIS), BlockNumber::from(2), 3, false)] + #[case::ten_attempts_false(Some(BlockNumber::GENESIS), BlockNumber::from(12), 10, false)] + #[case::twenty_attempts_false(Some(BlockNumber::GENESIS), BlockNumber::from(148), 20, false)] + fn backoff_has_passed( + #[case] last_attempt_block_num: Option, + #[case] current_block_num: BlockNumber, + #[case] attempt_count: usize, + #[case] backoff_should_have_passed: bool, + ) { + assert_eq!( + backoff_should_have_passed, + has_backoff_passed(current_block_num, last_attempt_block_num, attempt_count) + ); + } +} From 1441d167e3d929227910c6e281b06b24030d88fa Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Thu, 12 Feb 2026 15:06:37 -0300 Subject: [PATCH 05/15] review: merge Db initialization methods --- crates/ntx-builder/src/db/mod.rs | 63 +++++++++++++++++--------------- crates/ntx-builder/src/lib.rs | 8 +--- 2 files changed, 35 insertions(+), 36 deletions(-) diff --git a/crates/ntx-builder/src/db/mod.rs b/crates/ntx-builder/src/db/mod.rs index 0c23be396..276d8073a 100644 --- a/crates/ntx-builder/src/db/mod.rs +++ b/crates/ntx-builder/src/db/mod.rs @@ -36,26 +36,51 @@ pub struct Db { } impl Db { - /// Creates a new database file, configures it, and applies migrations. - /// - /// This is a synchronous one-shot setup used during node initialization. - /// For runtime access with a connection pool, use [`Db::load`]. + /// Creates and initializes the database, then opens an async connection pool. #[instrument( target = COMPONENT, - name = "ntx_builder.database.bootstrap", + name = "ntx_builder.database.setup", skip_all, fields(path=%database_filepath.display()), err, )] - pub fn bootstrap(database_filepath: PathBuf) -> anyhow::Result<()> { + pub async fn setup(database_filepath: PathBuf) -> anyhow::Result { + // Run sync bootstrap (create file, configure, migrate) on a blocking thread. + let path = database_filepath.clone(); + tokio::task::spawn_blocking(move || Self::bootstrap_sync(&path)) + .await + .context("bootstrap task panicked")? + .context("failed to bootstrap ntx-builder database")?; + + // Open async connection pool. + let manager = ConnectionManager::new(database_filepath.to_str().unwrap()); + let pool = deadpool_diesel::Pool::builder(manager) + .max_size(16) + .build() + .map_err(DatabaseSetupError::PoolBuild) + .context("failed to build connection pool")?; + + info!( + target: COMPONENT, + sqlite = %database_filepath.display(), + "Connected to the database" + ); + + let me = Db { pool }; + me.query("migrations", apply_migrations) + .await + .context("failed to apply migrations on pool connection")?; + Ok(me) + } + + /// Opens a connection to the database, then configures it, and applies migrations. + fn bootstrap_sync(database_filepath: &std::path::Path) -> anyhow::Result<()> { let mut conn: SqliteConnection = diesel::sqlite::SqliteConnection::establish( database_filepath.to_str().context("database filepath is invalid")?, ) .context("failed to open a database connection")?; configure_connection_on_creation(&mut conn)?; - - // Run migrations. apply_migrations(&mut conn).context("failed to apply database migrations")?; Ok(()) @@ -111,28 +136,6 @@ impl Db { .map_err(|err| E::from(DatabaseError::interact(&msg.to_string(), &err)))? } - /// Opens a connection pool to an existing database and re-applies pending migrations. - /// - /// Use [`Db::bootstrap`] first to create and initialize the database file. - #[instrument(target = COMPONENT, skip_all)] - pub async fn load(database_filepath: PathBuf) -> Result { - let manager = ConnectionManager::new(database_filepath.to_str().unwrap()); - let pool = deadpool_diesel::Pool::builder(manager) - .max_size(16) - .build() - .map_err(DatabaseSetupError::PoolBuild)?; - - info!( - target: COMPONENT, - sqlite = %database_filepath.display(), - "Connected to the database" - ); - - let me = Db { pool }; - me.query("migrations", apply_migrations).await?; - Ok(me) - } - // PUBLIC QUERY METHODS // ============================================================================================ diff --git a/crates/ntx-builder/src/lib.rs b/crates/ntx-builder/src/lib.rs index c86f737f2..5732cb43f 100644 --- a/crates/ntx-builder/src/lib.rs +++ b/crates/ntx-builder/src/lib.rs @@ -205,12 +205,8 @@ impl NtxBuilderConfig { /// - The mempool subscription fails (after retries) /// - The store contains no blocks (not bootstrapped) pub async fn build(self) -> anyhow::Result { - // Bootstrap and load the database. - Db::bootstrap(self.database_filepath.clone()) - .context("failed to bootstrap ntx-builder database")?; - let db = Db::load(self.database_filepath.clone()) - .await - .context("failed to load ntx-builder database")?; + // Set up the database (bootstrap + connection pool). + let db = Db::setup(self.database_filepath.clone()).await?; // Purge inflight state from previous run. db.purge_inflight().await.context("failed to purge inflight state")?; From 964302c9a4b44fc75df7acea8741a65a5eef5ea6 Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Thu, 12 Feb 2026 15:46:46 -0300 Subject: [PATCH 06/15] review: move to DELETE+INSERT --- .../src/db/models/queries/accounts.rs | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/crates/ntx-builder/src/db/models/queries/accounts.rs b/crates/ntx-builder/src/db/models/queries/accounts.rs index 861b60ff8..911dd2d6c 100644 --- a/crates/ntx-builder/src/db/models/queries/accounts.rs +++ b/crates/ntx-builder/src/db/models/queries/accounts.rs @@ -36,13 +36,14 @@ pub struct AccountRow { /// Inserts or replaces the committed account state (`transaction_id = NULL`). /// -/// Uses `INSERT OR REPLACE` which triggers on the partial unique index -/// `idx_accounts_committed(account_id) WHERE transaction_id IS NULL`. +/// Deletes any existing committed row first, then inserts a fresh one. /// /// # Raw SQL /// /// ```sql -/// INSERT OR REPLACE INTO accounts (account_id, account_data, transaction_id) +/// DELETE FROM accounts WHERE account_id = ?1 AND transaction_id IS NULL +/// +/// INSERT INTO accounts (account_id, account_data, transaction_id) /// VALUES (?1, ?2, NULL) /// ``` pub fn upsert_committed_account( @@ -50,12 +51,23 @@ pub fn upsert_committed_account( account_id: NetworkAccountId, account: &Account, ) -> Result<(), DatabaseError> { + let account_id_bytes = conversions::network_account_id_to_bytes(account_id); + + // Delete the existing committed row (if any). + diesel::delete( + schema::accounts::table + .filter(schema::accounts::account_id.eq(&account_id_bytes)) + .filter(schema::accounts::transaction_id.is_null()), + ) + .execute(conn)?; + + // Insert the new committed row. let row = AccountInsert { - account_id: conversions::network_account_id_to_bytes(account_id), + account_id: account_id_bytes, account_data: conversions::account_to_bytes(account), transaction_id: None, }; - diesel::replace_into(schema::accounts::table).values(&row).execute(conn)?; + diesel::insert_into(schema::accounts::table).values(&row).execute(conn)?; Ok(()) } From 325de14ec9d83519305e43f4b15a8bf7b03887f2 Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Fri, 13 Feb 2026 11:48:25 -0300 Subject: [PATCH 07/15] review: use i64 for block_num --- crates/ntx-builder/src/db/models/conv.rs | 7 +++---- crates/ntx-builder/src/db/models/queries/chain_state.rs | 4 ++-- crates/ntx-builder/src/db/models/queries/notes.rs | 8 ++++---- crates/ntx-builder/src/db/models/queries/tests.rs | 8 ++++---- crates/ntx-builder/src/db/schema.rs | 4 ++-- 5 files changed, 15 insertions(+), 16 deletions(-) diff --git a/crates/ntx-builder/src/db/models/conv.rs b/crates/ntx-builder/src/db/models/conv.rs index f4906c37b..9e01c361e 100644 --- a/crates/ntx-builder/src/db/models/conv.rs +++ b/crates/ntx-builder/src/db/models/conv.rs @@ -35,13 +35,12 @@ pub fn nullifier_to_bytes(nullifier: &Nullifier) -> Vec { nullifier.to_bytes() } -#[allow(clippy::cast_possible_wrap)] -pub fn block_num_to_i32(block_num: BlockNumber) -> i32 { - block_num.as_u32() as i32 +pub fn block_num_to_i64(block_num: BlockNumber) -> i64 { + i64::from(block_num.as_u32()) } #[allow(clippy::cast_sign_loss)] -pub fn block_num_from_i32(val: i32) -> BlockNumber { +pub fn block_num_from_i64(val: i64) -> BlockNumber { BlockNumber::from(val as u32) } diff --git a/crates/ntx-builder/src/db/models/queries/chain_state.rs b/crates/ntx-builder/src/db/models/queries/chain_state.rs index 035c0d5f2..cb9bb546b 100644 --- a/crates/ntx-builder/src/db/models/queries/chain_state.rs +++ b/crates/ntx-builder/src/db/models/queries/chain_state.rs @@ -16,7 +16,7 @@ use crate::db::schema; pub struct ChainStateInsert { /// Singleton row ID. Always `0` to satisfy the `CHECK (id = 0)` constraint. pub id: i32, - pub block_num: i32, + pub block_num: i64, pub block_header: Vec, } @@ -38,7 +38,7 @@ pub fn upsert_chain_state( ) -> Result<(), DatabaseError> { let row = ChainStateInsert { id: 0, - block_num: conversions::block_num_to_i32(block_num), + block_num: conversions::block_num_to_i64(block_num), block_header: conversions::block_header_to_bytes(block_header), }; diesel::replace_into(schema::chain_state::table).values(&row).execute(conn)?; diff --git a/crates/ntx-builder/src/db/models/queries/notes.rs b/crates/ntx-builder/src/db/models/queries/notes.rs index c02b6ece3..bce6c5ab2 100644 --- a/crates/ntx-builder/src/db/models/queries/notes.rs +++ b/crates/ntx-builder/src/db/models/queries/notes.rs @@ -21,7 +21,7 @@ use crate::db::schema; pub struct NoteRow { pub note_data: Vec, pub attempt_count: i32, - pub last_attempt: Option, + pub last_attempt: Option, } /// Row for inserting into the unified `notes` table. @@ -33,7 +33,7 @@ pub struct NoteInsert { pub account_id: Vec, pub note_data: Vec, pub attempt_count: i32, - pub last_attempt: Option, + pub last_attempt: Option, pub created_by: Option>, pub consumed_by: Option>, } @@ -111,7 +111,7 @@ pub fn available_notes( let note = note_row_to_inflight( &row.note_data, attempt_count, - row.last_attempt.map(conversions::block_num_from_i32), + row.last_attempt.map(conversions::block_num_from_i64), )?; if note.is_available(block_num) { result.push(note); @@ -137,7 +137,7 @@ pub fn notes_failed( nullifiers: &[Nullifier], block_num: BlockNumber, ) -> Result<(), DatabaseError> { - let block_num_val = conversions::block_num_to_i32(block_num); + let block_num_val = conversions::block_num_to_i64(block_num); for nullifier in nullifiers { let nullifier_bytes = conversions::nullifier_to_bytes(nullifier); diff --git a/crates/ntx-builder/src/db/models/queries/tests.rs b/crates/ntx-builder/src/db/models/queries/tests.rs index 30523ae44..f8f6e7de0 100644 --- a/crates/ntx-builder/src/db/models/queries/tests.rs +++ b/crates/ntx-builder/src/db/models/queries/tests.rs @@ -482,14 +482,14 @@ fn notes_failed_increments_attempt_count() { notes_failed(conn, &[note.nullifier()], block_num).unwrap(); notes_failed(conn, &[note.nullifier()], block_num).unwrap(); - let (attempt_count, last_attempt): (i32, Option) = schema::notes::table + let (attempt_count, last_attempt): (i32, Option) = schema::notes::table .find(conversions::nullifier_to_bytes(¬e.nullifier())) .select((schema::notes::attempt_count, schema::notes::last_attempt)) .first(conn) .unwrap(); assert_eq!(attempt_count, 2); - assert_eq!(last_attempt, Some(conversions::block_num_to_i32(block_num))); + assert_eq!(last_attempt, Some(conversions::block_num_to_i64(block_num))); } // CHAIN STATE TESTS @@ -513,11 +513,11 @@ fn upsert_chain_state_updates_singleton() { assert_eq!(row_count, 1); // Should have the latest block number. - let stored_block_num: i32 = schema::chain_state::table + let stored_block_num: i64 = schema::chain_state::table .select(schema::chain_state::block_num) .first(conn) .unwrap(); - assert_eq!(stored_block_num, conversions::block_num_to_i32(block_num_2)); + assert_eq!(stored_block_num, conversions::block_num_to_i64(block_num_2)); } // HELPERS (domain type construction) diff --git a/crates/ntx-builder/src/db/schema.rs b/crates/ntx-builder/src/db/schema.rs index 1fc97a217..6a70ee121 100644 --- a/crates/ntx-builder/src/db/schema.rs +++ b/crates/ntx-builder/src/db/schema.rs @@ -12,7 +12,7 @@ diesel::table! { diesel::table! { chain_state (id) { id -> Integer, - block_num -> Integer, + block_num -> BigInt, block_header -> Binary, } } @@ -23,7 +23,7 @@ diesel::table! { account_id -> Binary, note_data -> Binary, attempt_count -> Integer, - last_attempt -> Nullable, + last_attempt -> Nullable, created_by -> Nullable, consumed_by -> Nullable, } From 15668a7a5b0021d93387d3d2023f4fd4d7dfeb53 Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Fri, 13 Feb 2026 12:01:26 -0300 Subject: [PATCH 08/15] review: remove bootstrap_sync --- crates/ntx-builder/src/db/mod.rs | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/crates/ntx-builder/src/db/mod.rs b/crates/ntx-builder/src/db/mod.rs index 276d8073a..0b584d16a 100644 --- a/crates/ntx-builder/src/db/mod.rs +++ b/crates/ntx-builder/src/db/mod.rs @@ -1,7 +1,7 @@ use std::path::PathBuf; use anyhow::Context; -use diesel::{Connection, SqliteConnection}; +use diesel::SqliteConnection; use miden_node_proto::domain::account::NetworkAccountId; use miden_node_proto::domain::note::SingleTargetNetworkNote; use miden_protocol::account::Account; @@ -14,7 +14,7 @@ use tracing::{Instrument, info, instrument}; use crate::COMPONENT; use crate::actor::inflight_note::InflightNetworkNote; use crate::db::errors::{DatabaseError, DatabaseSetupError}; -use crate::db::manager::{ConnectionManager, configure_connection_on_creation}; +use crate::db::manager::ConnectionManager; use crate::db::migrations::apply_migrations; use crate::db::models::queries; @@ -45,14 +45,6 @@ impl Db { err, )] pub async fn setup(database_filepath: PathBuf) -> anyhow::Result { - // Run sync bootstrap (create file, configure, migrate) on a blocking thread. - let path = database_filepath.clone(); - tokio::task::spawn_blocking(move || Self::bootstrap_sync(&path)) - .await - .context("bootstrap task panicked")? - .context("failed to bootstrap ntx-builder database")?; - - // Open async connection pool. let manager = ConnectionManager::new(database_filepath.to_str().unwrap()); let pool = deadpool_diesel::Pool::builder(manager) .max_size(16) @@ -73,19 +65,6 @@ impl Db { Ok(me) } - /// Opens a connection to the database, then configures it, and applies migrations. - fn bootstrap_sync(database_filepath: &std::path::Path) -> anyhow::Result<()> { - let mut conn: SqliteConnection = diesel::sqlite::SqliteConnection::establish( - database_filepath.to_str().context("database filepath is invalid")?, - ) - .context("failed to open a database connection")?; - - configure_connection_on_creation(&mut conn)?; - apply_migrations(&mut conn).context("failed to apply database migrations")?; - - Ok(()) - } - /// Create and commit a transaction with the queries added in the provided closure. pub(crate) async fn transact(&self, msg: M, query: Q) -> std::result::Result where @@ -275,6 +254,8 @@ impl Db { /// This bypasses the async connection pool entirely, matching the store crate's test pattern. #[cfg(test)] pub fn test_conn() -> SqliteConnection { + use diesel::Connection; + use crate::db::manager::configure_connection_on_creation; let mut conn = From c2d18f75fd0c3a864c8bc27d16699c3bbed3c458 Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Fri, 13 Feb 2026 12:14:12 -0300 Subject: [PATCH 09/15] review: remove in memory db for tests --- Cargo.lock | 1 + crates/ntx-builder/Cargo.toml | 1 + crates/ntx-builder/src/db/mod.rs | 14 ++++---- .../src/db/models/queries/tests.rs | 32 +++++++++---------- 4 files changed, 25 insertions(+), 23 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d13d254da..6382c8ea2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2823,6 +2823,7 @@ dependencies = [ "prost", "rand_chacha 0.9.0", "rstest", + "tempfile", "thiserror 2.0.18", "tokio", "tokio-stream", diff --git a/crates/ntx-builder/Cargo.toml b/crates/ntx-builder/Cargo.toml index b31278ddf..233262cd6 100644 --- a/crates/ntx-builder/Cargo.toml +++ b/crates/ntx-builder/Cargo.toml @@ -42,3 +42,4 @@ miden-protocol = { default-features = true, features = ["testing"], works miden-standards = { features = ["testing"], workspace = true } rand_chacha = { workspace = true } rstest = { workspace = true } +tempfile = { version = "3.20" } diff --git a/crates/ntx-builder/src/db/mod.rs b/crates/ntx-builder/src/db/mod.rs index 0b584d16a..399433baf 100644 --- a/crates/ntx-builder/src/db/mod.rs +++ b/crates/ntx-builder/src/db/mod.rs @@ -249,19 +249,19 @@ impl Db { .await } - /// Creates an in-memory SQLite connection for testing with migrations applied. - /// - /// This bypasses the async connection pool entirely, matching the store crate's test pattern. + /// Creates a file-backed SQLite test connection with migrations applied. #[cfg(test)] - pub fn test_conn() -> SqliteConnection { + pub fn test_conn() -> (SqliteConnection, tempfile::TempDir) { use diesel::Connection; use crate::db::manager::configure_connection_on_creation; - let mut conn = - SqliteConnection::establish(":memory:").expect("in-memory sqlite should always work"); + let dir = tempfile::tempdir().expect("failed to create temp directory"); + let db_path = dir.path().join("test.sqlite3"); + let mut conn = SqliteConnection::establish(db_path.to_str().unwrap()) + .expect("temp file sqlite should always work"); configure_connection_on_creation(&mut conn).expect("connection configuration should work"); apply_migrations(&mut conn).expect("migrations should apply on empty database"); - conn + (conn, dir) } } diff --git a/crates/ntx-builder/src/db/models/queries/tests.rs b/crates/ntx-builder/src/db/models/queries/tests.rs index f8f6e7de0..a8dfcc8c0 100644 --- a/crates/ntx-builder/src/db/models/queries/tests.rs +++ b/crates/ntx-builder/src/db/models/queries/tests.rs @@ -24,8 +24,8 @@ use crate::db::{Db, schema}; // TEST HELPERS // ================================================================================================ -/// Creates an in-memory SQLite connection with migrations applied. -fn test_conn() -> SqliteConnection { +/// Creates a file-backed SQLite connection with migrations applied. +fn test_conn() -> (SqliteConnection, tempfile::TempDir) { Db::test_conn() } @@ -103,7 +103,7 @@ fn count_committed_accounts(conn: &mut SqliteConnection) -> i64 { #[test] fn purge_inflight_clears_all_inflight_state() { - let conn = &mut test_conn(); + let (conn, _dir) = &mut test_conn(); let account_id = mock_network_account_id(); let tx_id = mock_tx_id(1); @@ -146,7 +146,7 @@ fn purge_inflight_clears_all_inflight_state() { #[test] fn transaction_added_inserts_notes_and_marks_consumed() { - let conn = &mut test_conn(); + let (conn, _dir) = &mut test_conn(); let account_id = mock_network_account_id(); let tx_id = mock_tx_id(1); @@ -189,7 +189,7 @@ fn transaction_added_inserts_notes_and_marks_consumed() { #[test] fn transaction_added_is_idempotent_for_notes() { - let conn = &mut test_conn(); + let (conn, _dir) = &mut test_conn(); let account_id = mock_network_account_id(); let tx_id = mock_tx_id(1); @@ -208,7 +208,7 @@ fn transaction_added_is_idempotent_for_notes() { #[test] fn block_committed_promotes_inflight_notes_to_committed() { - let conn = &mut test_conn(); + let (conn, _dir) = &mut test_conn(); let account_id = mock_network_account_id(); let tx_id = mock_tx_id(1); @@ -241,7 +241,7 @@ fn block_committed_promotes_inflight_notes_to_committed() { #[test] fn block_committed_deletes_consumed_notes() { - let conn = &mut test_conn(); + let (conn, _dir) = &mut test_conn(); let account_id = mock_network_account_id(); let note = mock_single_target_note(account_id, 10); @@ -265,7 +265,7 @@ fn block_committed_deletes_consumed_notes() { #[test] fn block_committed_promotes_inflight_account_to_committed() { - let conn = &mut test_conn(); + let (conn, _dir) = &mut test_conn(); let account_id = mock_network_account_id(); let account = mock_account(account_id); @@ -301,7 +301,7 @@ fn block_committed_promotes_inflight_account_to_committed() { #[test] fn transactions_reverted_restores_consumed_notes() { - let conn = &mut test_conn(); + let (conn, _dir) = &mut test_conn(); let account_id = mock_network_account_id(); let note = mock_single_target_note(account_id, 10); @@ -336,7 +336,7 @@ fn transactions_reverted_restores_consumed_notes() { #[test] fn transactions_reverted_deletes_inflight_created_notes() { - let conn = &mut test_conn(); + let (conn, _dir) = &mut test_conn(); let account_id = mock_network_account_id(); let tx_id = mock_tx_id(1); @@ -355,7 +355,7 @@ fn transactions_reverted_deletes_inflight_created_notes() { #[test] fn transactions_reverted_reports_reverted_account_creations() { - let conn = &mut test_conn(); + let (conn, _dir) = &mut test_conn(); let account_id = mock_network_account_id(); let account = mock_account(account_id); @@ -384,7 +384,7 @@ fn transactions_reverted_reports_reverted_account_creations() { #[test] #[allow(clippy::similar_names)] fn available_notes_filters_consumed_and_exceeded_attempts() { - let conn = &mut test_conn(); + let (conn, _dir) = &mut test_conn(); let account_id = mock_network_account_id(); let note_good = mock_single_target_note(account_id, 10); @@ -416,7 +416,7 @@ fn available_notes_filters_consumed_and_exceeded_attempts() { #[test] fn available_notes_only_returns_notes_for_specified_account() { - let conn = &mut test_conn(); + let (conn, _dir) = &mut test_conn(); let account_id_1 = mock_network_account_id(); let account_id_2 = mock_network_account_id_seeded(42); @@ -438,7 +438,7 @@ fn available_notes_only_returns_notes_for_specified_account() { #[test] fn drop_failing_notes_scoped_to_account() { - let conn = &mut test_conn(); + let (conn, _dir) = &mut test_conn(); let account_id_1 = mock_network_account_id(); let account_id_2 = mock_network_account_id_seeded(42); @@ -471,7 +471,7 @@ fn drop_failing_notes_scoped_to_account() { #[test] fn notes_failed_increments_attempt_count() { - let conn = &mut test_conn(); + let (conn, _dir) = &mut test_conn(); let account_id = mock_network_account_id(); let note = mock_single_target_note(account_id, 10); @@ -497,7 +497,7 @@ fn notes_failed_increments_attempt_count() { #[test] fn upsert_chain_state_updates_singleton() { - let conn = &mut test_conn(); + let (conn, _dir) = &mut test_conn(); let block_num_1 = BlockNumber::from(1u32); let header_1 = mock_block_header(block_num_1); From 80a84c93c7b90578b35029940c06c420a57a791e Mon Sep 17 00:00:00 2001 From: Santiago Pittella <87827390+SantiagoPittella@users.noreply.github.com> Date: Thu, 19 Feb 2026 11:44:13 -0300 Subject: [PATCH 10/15] review: remove match with map Co-authored-by: Mirko <48352201+Mirko-von-Leipzig@users.noreply.github.com> --- crates/ntx-builder/src/db/models/queries/accounts.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/ntx-builder/src/db/models/queries/accounts.rs b/crates/ntx-builder/src/db/models/queries/accounts.rs index 911dd2d6c..cd19f60bc 100644 --- a/crates/ntx-builder/src/db/models/queries/accounts.rs +++ b/crates/ntx-builder/src/db/models/queries/accounts.rs @@ -97,8 +97,5 @@ pub fn latest_account( .first(conn) .optional()?; - match row { - Some(r) => Ok(Some(conversions::account_from_bytes(&r.account_data)?)), - None => Ok(None), - } + row.map(conversions::account_from_bytes).transpose() } From 6fd47ecfec201520309c66497598f26e4d5d9180 Mon Sep 17 00:00:00 2001 From: Santiago Pittella <87827390+SantiagoPittella@users.noreply.github.com> Date: Thu, 19 Feb 2026 11:44:47 -0300 Subject: [PATCH 11/15] review: rename to get_account Co-authored-by: Mirko <48352201+Mirko-von-Leipzig@users.noreply.github.com> --- crates/ntx-builder/src/db/models/queries/accounts.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ntx-builder/src/db/models/queries/accounts.rs b/crates/ntx-builder/src/db/models/queries/accounts.rs index cd19f60bc..a8555c6b3 100644 --- a/crates/ntx-builder/src/db/models/queries/accounts.rs +++ b/crates/ntx-builder/src/db/models/queries/accounts.rs @@ -83,7 +83,7 @@ pub fn upsert_committed_account( /// ORDER BY order_id DESC /// LIMIT 1 /// ``` -pub fn latest_account( +pub fn get_account( conn: &mut SqliteConnection, account_id: NetworkAccountId, ) -> Result, DatabaseError> { From 0b0c33768c31b4928947ab1dc075ca0d890ce3ef Mon Sep 17 00:00:00 2001 From: Santiago Pittella <87827390+SantiagoPittella@users.noreply.github.com> Date: Thu, 19 Feb 2026 11:45:03 -0300 Subject: [PATCH 12/15] review: rename to add_transaction Co-authored-by: Mirko <48352201+Mirko-von-Leipzig@users.noreply.github.com> --- crates/ntx-builder/src/db/models/queries/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ntx-builder/src/db/models/queries/mod.rs b/crates/ntx-builder/src/db/models/queries/mod.rs index 43f33bc27..f22215f17 100644 --- a/crates/ntx-builder/src/db/models/queries/mod.rs +++ b/crates/ntx-builder/src/db/models/queries/mod.rs @@ -90,7 +90,7 @@ pub fn purge_inflight(conn: &mut SqliteConnection) -> Result<(), DatabaseError> /// SET consumed_by = ?1 /// WHERE nullifier = ?2 AND consumed_by IS NULL /// ``` -pub fn handle_transaction_added( +pub fn add_transaction( conn: &mut SqliteConnection, tx_id: &TransactionId, account_delta: Option<&AccountUpdateDetails>, From b1ba616a01dbddf98cdab42f26ad69a6b1109671 Mon Sep 17 00:00:00 2001 From: Santiago Pittella <87827390+SantiagoPittella@users.noreply.github.com> Date: Thu, 19 Feb 2026 11:45:17 -0300 Subject: [PATCH 13/15] review: rename to commit_block Co-authored-by: Mirko <48352201+Mirko-von-Leipzig@users.noreply.github.com> --- crates/ntx-builder/src/db/models/queries/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ntx-builder/src/db/models/queries/mod.rs b/crates/ntx-builder/src/db/models/queries/mod.rs index f22215f17..e18eab34d 100644 --- a/crates/ntx-builder/src/db/models/queries/mod.rs +++ b/crates/ntx-builder/src/db/models/queries/mod.rs @@ -190,7 +190,7 @@ pub fn add_transaction( /// ``` /// /// Finally updates chain state (see [`upsert_chain_state`]). -pub fn handle_block_committed( +pub fn commit_block( conn: &mut SqliteConnection, tx_ids: &[TransactionId], block_num: BlockNumber, From 1d6544f2af38621328ea791cf34f89cd4ce78d07 Mon Sep 17 00:00:00 2001 From: Santiago Pittella <87827390+SantiagoPittella@users.noreply.github.com> Date: Thu, 19 Feb 2026 11:45:32 -0300 Subject: [PATCH 14/15] review: rename to revert_transaction Co-authored-by: Mirko <48352201+Mirko-von-Leipzig@users.noreply.github.com> --- crates/ntx-builder/src/db/models/queries/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ntx-builder/src/db/models/queries/mod.rs b/crates/ntx-builder/src/db/models/queries/mod.rs index e18eab34d..5b99f3eaa 100644 --- a/crates/ntx-builder/src/db/models/queries/mod.rs +++ b/crates/ntx-builder/src/db/models/queries/mod.rs @@ -267,7 +267,7 @@ pub fn commit_block( /// -- Restore consumed notes /// UPDATE notes SET consumed_by = NULL WHERE consumed_by = ?1 /// ``` -pub fn handle_transactions_reverted( +pub fn revert_transaction( conn: &mut SqliteConnection, tx_ids: &[TransactionId], ) -> Result, DatabaseError> { From c2ad2a367fcf4958b032db9b9f9377275012a9f7 Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Thu, 19 Feb 2026 11:53:31 -0300 Subject: [PATCH 15/15] chore: rename method in caller sites --- crates/ntx-builder/src/db/mod.rs | 14 ++----- .../src/db/models/queries/accounts.rs | 3 +- .../ntx-builder/src/db/models/queries/mod.rs | 2 +- .../src/db/models/queries/tests.rs | 40 ++++++++----------- 4 files changed, 24 insertions(+), 35 deletions(-) diff --git a/crates/ntx-builder/src/db/mod.rs b/crates/ntx-builder/src/db/mod.rs index 399433baf..c323c355d 100644 --- a/crates/ntx-builder/src/db/mod.rs +++ b/crates/ntx-builder/src/db/mod.rs @@ -152,7 +152,7 @@ impl Db { max_note_attempts: usize, ) -> Result<(Option, Vec)> { self.query("select_candidate", move |conn| { - let account = queries::latest_account(conn, account_id)?; + let account = queries::get_account(conn, account_id)?; let notes = queries::available_notes(conn, account_id, block_num, max_note_attempts)?; Ok((account, notes)) }) @@ -180,13 +180,7 @@ impl Db { nullifiers: Vec, ) -> Result<()> { self.transact("handle_transaction_added", move |conn| { - queries::handle_transaction_added( - conn, - &tx_id, - account_delta.as_ref(), - ¬es, - &nullifiers, - ) + queries::add_transaction(conn, &tx_id, account_delta.as_ref(), ¬es, &nullifiers) }) .await } @@ -199,7 +193,7 @@ impl Db { header: BlockHeader, ) -> Result<()> { self.transact("handle_block_committed", move |conn| { - queries::handle_block_committed(conn, &txs, block_num, &header) + queries::commit_block(conn, &txs, block_num, &header) }) .await } @@ -212,7 +206,7 @@ impl Db { tx_ids: Vec, ) -> Result> { self.transact("handle_transactions_reverted", move |conn| { - queries::handle_transactions_reverted(conn, &tx_ids) + queries::revert_transaction(conn, &tx_ids) }) .await } diff --git a/crates/ntx-builder/src/db/models/queries/accounts.rs b/crates/ntx-builder/src/db/models/queries/accounts.rs index a8555c6b3..8cc5a2245 100644 --- a/crates/ntx-builder/src/db/models/queries/accounts.rs +++ b/crates/ntx-builder/src/db/models/queries/accounts.rs @@ -97,5 +97,6 @@ pub fn get_account( .first(conn) .optional()?; - row.map(conversions::account_from_bytes).transpose() + row.map(|AccountRow { account_data, .. }| conversions::account_from_bytes(&account_data)) + .transpose() } diff --git a/crates/ntx-builder/src/db/models/queries/mod.rs b/crates/ntx-builder/src/db/models/queries/mod.rs index 5b99f3eaa..4b6fd7ffd 100644 --- a/crates/ntx-builder/src/db/models/queries/mod.rs +++ b/crates/ntx-builder/src/db/models/queries/mod.rs @@ -106,7 +106,7 @@ pub fn add_transaction( NetworkAccountEffect::Updated(ref account_delta) => { // Query latest_account, apply delta, insert inflight row. let current_account = - latest_account(conn, account_id)?.expect("account must exist to apply delta"); + get_account(conn, account_id)?.expect("account must exist to apply delta"); let mut updated = current_account; updated.apply_delta(account_delta).expect( "network account delta should apply since it was accepted by the mempool", diff --git a/crates/ntx-builder/src/db/models/queries/tests.rs b/crates/ntx-builder/src/db/models/queries/tests.rs index d447c09d8..6ef55f9a2 100644 --- a/crates/ntx-builder/src/db/models/queries/tests.rs +++ b/crates/ntx-builder/src/db/models/queries/tests.rs @@ -113,14 +113,14 @@ fn purge_inflight_clears_all_inflight_state() { upsert_committed_account(conn, account_id, &mock_account(account_id)).unwrap(); // Insert a transaction (creates inflight account row + note + consumption). - handle_transaction_added(conn, &tx_id, None, std::slice::from_ref(¬e), &[]).unwrap(); + add_transaction(conn, &tx_id, None, std::slice::from_ref(¬e), &[]).unwrap(); assert!(count_inflight_accounts(conn) == 0); // No account delta, so no inflight account. assert_eq!(count_notes(conn), 1); // Mark note as consumed by another tx. let tx_id2 = mock_tx_id(2); - handle_transaction_added(conn, &tx_id2, None, &[], &[note.nullifier()]).unwrap(); + add_transaction(conn, &tx_id2, None, &[], &[note.nullifier()]).unwrap(); // Verify consumed_by is set. let consumed_count: i64 = schema::notes::table @@ -158,14 +158,8 @@ fn transaction_added_inserts_notes_and_marks_consumed() { assert_eq!(count_notes(conn), 1); // Add transaction that creates note2 and consumes note1. - handle_transaction_added( - conn, - &tx_id, - None, - std::slice::from_ref(¬e2), - &[note1.nullifier()], - ) - .unwrap(); + add_transaction(conn, &tx_id, None, std::slice::from_ref(¬e2), &[note1.nullifier()]) + .unwrap(); // Should now have 2 notes total. assert_eq!(count_notes(conn), 2); @@ -196,8 +190,8 @@ fn transaction_added_is_idempotent_for_notes() { let note = mock_single_target_note(account_id, 10); // Insert the same transaction twice. - handle_transaction_added(conn, &tx_id, None, std::slice::from_ref(¬e), &[]).unwrap(); - handle_transaction_added(conn, &tx_id, None, std::slice::from_ref(¬e), &[]).unwrap(); + add_transaction(conn, &tx_id, None, std::slice::from_ref(¬e), &[]).unwrap(); + add_transaction(conn, &tx_id, None, std::slice::from_ref(¬e), &[]).unwrap(); // Should only have one note (INSERT OR IGNORE). assert_eq!(count_notes(conn), 1); @@ -217,7 +211,7 @@ fn block_committed_promotes_inflight_notes_to_committed() { let header = mock_block_header(block_num); // Add a transaction that creates a note. - handle_transaction_added(conn, &tx_id, None, std::slice::from_ref(¬e), &[]).unwrap(); + add_transaction(conn, &tx_id, None, std::slice::from_ref(¬e), &[]).unwrap(); // Verify created_by is set. let created: Option> = schema::notes::table @@ -228,7 +222,7 @@ fn block_committed_promotes_inflight_notes_to_committed() { assert!(created.is_some()); // Commit the block. - handle_block_committed(conn, &[tx_id], block_num, &header).unwrap(); + commit_block(conn, &[tx_id], block_num, &header).unwrap(); // created_by should now be NULL (promoted to committed). let created: Option> = schema::notes::table @@ -252,12 +246,12 @@ fn block_committed_deletes_consumed_notes() { // Consume it via a transaction. let tx_id = mock_tx_id(1); - handle_transaction_added(conn, &tx_id, None, &[], &[note.nullifier()]).unwrap(); + add_transaction(conn, &tx_id, None, &[], &[note.nullifier()]).unwrap(); // Commit the block. let block_num = BlockNumber::from(1u32); let header = mock_block_header(block_num); - handle_block_committed(conn, &[tx_id], block_num, &header).unwrap(); + commit_block(conn, &[tx_id], block_num, &header).unwrap(); // Consumed note should be deleted. assert_eq!(count_notes(conn), 0); @@ -289,7 +283,7 @@ fn block_committed_promotes_inflight_account_to_committed() { // Commit the block. let block_num = BlockNumber::from(1u32); let header = mock_block_header(block_num); - handle_block_committed(conn, &[tx_id], block_num, &header).unwrap(); + commit_block(conn, &[tx_id], block_num, &header).unwrap(); // Should have 1 committed and 0 inflight. assert_eq!(count_committed_accounts(conn), 1); @@ -311,7 +305,7 @@ fn transactions_reverted_restores_consumed_notes() { // Consume it via a transaction. let tx_id = mock_tx_id(1); - handle_transaction_added(conn, &tx_id, None, &[], &[note.nullifier()]).unwrap(); + add_transaction(conn, &tx_id, None, &[], &[note.nullifier()]).unwrap(); // Verify consumed. let consumed: Option> = schema::notes::table @@ -322,7 +316,7 @@ fn transactions_reverted_restores_consumed_notes() { assert!(consumed.is_some()); // Revert the transaction. - let reverted = handle_transactions_reverted(conn, &[tx_id]).unwrap(); + let reverted = revert_transaction(conn, &[tx_id]).unwrap(); assert!(reverted.is_empty()); // Note should be un-consumed. @@ -343,11 +337,11 @@ fn transactions_reverted_deletes_inflight_created_notes() { let note = mock_single_target_note(account_id, 10); // Add transaction that creates a note. - handle_transaction_added(conn, &tx_id, None, std::slice::from_ref(¬e), &[]).unwrap(); + add_transaction(conn, &tx_id, None, std::slice::from_ref(¬e), &[]).unwrap(); assert_eq!(count_notes(conn), 1); // Revert the transaction. - handle_transactions_reverted(conn, &[tx_id]).unwrap(); + revert_transaction(conn, &[tx_id]).unwrap(); // Inflight-created note should be deleted. assert_eq!(count_notes(conn), 0); @@ -370,7 +364,7 @@ fn transactions_reverted_reports_reverted_account_creations() { diesel::insert_into(schema::accounts::table).values(&row).execute(conn).unwrap(); // Revert the transaction --- account creation should be reported. - let reverted = handle_transactions_reverted(conn, &[tx_id]).unwrap(); + let reverted = revert_transaction(conn, &[tx_id]).unwrap(); assert_eq!(reverted.len(), 1); assert_eq!(reverted[0], account_id); @@ -396,7 +390,7 @@ fn available_notes_filters_consumed_and_exceeded_attempts() { // Consume one note. let tx_id = mock_tx_id(1); - handle_transaction_added(conn, &tx_id, None, &[], &[note_consumed.nullifier()]).unwrap(); + add_transaction(conn, &tx_id, None, &[], &[note_consumed.nullifier()]).unwrap(); // Mark one note as failed many times (exceed max_attempts=3). let block_num = BlockNumber::from(100u32);