From 2ae39335ea30d6892b43d94f7b42ba3b64f617cb Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Wed, 19 Aug 2026 11:13:45 -0300 Subject: [PATCH] chore: Database simplificactions --- CHANGELOG.md | 3 + crates/rust-client/src/store/mod.rs | 3 +- crates/sqlite-store/src/account/accounts.rs | 307 ++++++++---------- crates/sqlite-store/src/account/mod.rs | 2 +- .../src/account/{helpers.rs => rows.rs} | 124 +++---- crates/sqlite-store/src/account/storage.rs | 2 +- crates/sqlite-store/src/chain_data.rs | 178 +++++----- .../src/db_management/migration_tests.rs | 2 +- .../db_management/{utils.rs => migrations.rs} | 87 +---- crates/sqlite-store/src/db_management/mod.rs | 3 +- .../src/db_management/settings.rs | 57 ++++ crates/sqlite-store/src/forest.rs | 24 +- crates/sqlite-store/src/lib.rs | 149 +++++++-- crates/sqlite-store/src/macros.rs | 41 +++ .../{store.sql => migrations/0001_init.sql} | 0 crates/sqlite-store/src/note/filters.rs | 143 +++----- crates/sqlite-store/src/note/mod.rs | 162 +++++---- crates/sqlite-store/src/note/tests.rs | 16 + crates/sqlite-store/src/sync.rs | 76 +++-- crates/sqlite-store/src/transaction.rs | 79 ++--- 20 files changed, 725 insertions(+), 733 deletions(-) rename crates/sqlite-store/src/account/{helpers.rs => rows.rs} (76%) rename crates/sqlite-store/src/db_management/{utils.rs => migrations.rs} (69%) create mode 100644 crates/sqlite-store/src/db_management/settings.rs create mode 100644 crates/sqlite-store/src/macros.rs rename crates/sqlite-store/src/{store.sql => migrations/0001_init.sql} (100%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 94473f4945..19244d2178 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ * [BREAKING][type][rust] Added the `NoteFilter::ScriptRoots` variant, so exhaustive matches on `NoteFilter` in `Store` implementations must handle it ([#2335](https://github.com/0xMiden/rust-sdk/pull/2335)). * [BREAKING][behavior][rpc] The `SyncNotes` response now carries a reduced note metadata message: instead of the note's attachments commitment it carries one entry per attachment, with single-word attachments sent verbatim and larger ones sent as commitments. The client reconstructs the protocol-level `NoteMetadata` from those entries, so it requires a node that speaks this format. * [BREAKING][behavior][store] The SQLite base schema now declares an index on `input_notes(script_root)`. This changes the schema fingerprint, so opening a database created before this change fails with `SchemaHashMismatch` and existing stores must be recreated ([#2335](https://github.com/0xMiden/rust-sdk/pull/2335)). +* [BREAKING][removal][store] `miden-client-sqlite-store` no longer exposes the internal helpers `column_value_as_u64` and `u64_to_value`, nor the connection-taking `SqliteStore` methods (`get_transactions`, `apply_transaction`, `apply_transaction_batch`, `get_foreign_account_code`, `get_account_vault`, `get_account_storage`, `upsert_foreign_account_code`, `prune_account_history`); they are now crate-private. Use the `Store` trait methods instead ([#2351](https://github.com/0xMiden/rust-sdk/issues/2351)). * [BREAKING][removal][rust] `miden_client::agglayer::create_bridge_account` and `miden_client::agglayer::create_agglayer_faucet` are removed. Build the accounts with `AggLayerBridge::account_builder` and `AggLayerFaucet::account_builder`, which return an `AccountBuilder` and take the account's `FeePolicyManager` explicitly; its active policy must be a `BasicConstantFeePolicy` scheduling every root in the account's `allowed_notes()`. The faucet builder additionally takes the initial token supply and the account seeding its `ADMIN` role. ### Enhancements @@ -44,6 +45,8 @@ ### Fixes +* [FIX][store] Corrupted database contents now surface as `StoreError`s instead of panicking: undecodable account IDs, nonces, and note-script blobs, a missing blockchain-checkpoint row, and a zero MMR node id all return errors, and rusqlite errors on parameterized note/account queries are no longer converted through panicking `expect`s ([#2351](https://github.com/0xMiden/rust-sdk/issues/2351)). +* [FIX][store] `u64` columns written with the top bit set (stored as negative SQL INTEGERs) are now read back through the shared bit-cast helper everywhere; two read sites previously errored on such values ([#2351](https://github.com/0xMiden/rust-sdk/issues/2351)). * [FIX][cli] `miden-client init` now reports invalid remote prover endpoints instead of silently writing a local-prover config ([#2376](https://github.com/0xMiden/rust-sdk/pull/2376)). * [FIX][rust] `VerifyingRpcClient::sync_transactions` now validates that every returned transaction record's account ID was actually requested, rejecting mismatches with `RpcError::InvalidResponse` ([#2372](https://github.com/0xMiden/rust-sdk/issues/2372)). * [FIX][rust] `Client::prove_transaction_with` now checks that the `TransactionProver` returned a proof of the transaction it was asked to prove, rejecting a mismatch with the new `ClientError::MismatchedProvenTransaction` ([#2391](https://github.com/0xMiden/rust-sdk/pull/2391)). diff --git a/crates/rust-client/src/store/mod.rs b/crates/rust-client/src/store/mod.rs index efabb8ff5a..7d266b8f6d 100644 --- a/crates/rust-client/src/store/mod.rs +++ b/crates/rust-client/src/store/mod.rs @@ -762,7 +762,8 @@ pub enum TransactionFilter { impl TransactionFilter { /// Returns a [String] containing the query for this Filter. pub fn to_query(&self) -> String { - const QUERY: &str = "SELECT tx.id, script.script, tx.details, tx.status \ + const QUERY: &str = "SELECT tx.id AS id, script.script AS script, tx.details AS details, \ + tx.status AS status \ FROM transactions AS tx LEFT JOIN transaction_scripts AS script ON tx.script_root = script.script_root"; match self { TransactionFilter::All => QUERY.to_string(), diff --git a/crates/sqlite-store/src/account/accounts.rs b/crates/sqlite-store/src/account/accounts.rs index fe1c841b43..09819ceef4 100644 --- a/crates/sqlite-store/src/account/accounts.rs +++ b/crates/sqlite-store/src/account/accounts.rs @@ -1,7 +1,6 @@ //! Account-related database operations. use std::collections::BTreeMap; -use std::rc::Rc; use std::string::ToString; use std::vec::Vec; @@ -35,17 +34,9 @@ use miden_client::{AccountError, Felt, Word}; use miden_protocol::account::{AccountStorageHeader, StorageMapWitness, StorageSlotHeader}; use miden_protocol::asset::{AssetId, PartialVault}; use miden_protocol::crypto::merkle::MerkleError; -use rusqlite::types::Value; -use rusqlite::{ - Connection, - OptionalExtension, - Transaction, - TransactionBehavior, - named_params, - params, -}; +use rusqlite::{Connection, OptionalExtension, Transaction, named_params, params}; -use crate::account::helpers::{ +use crate::account::rows::{ query_account_addresses, query_account_code, query_historical_account_headers, @@ -56,7 +47,17 @@ use crate::account::helpers::{ }; use crate::forest::{ScopedAccountForest, SqliteForestBackend, allocate_forest_revision}; use crate::sql_error::SqlResultExt; -use crate::{SqliteStore, column_value_as_u64, insert_sql, subst, u64_to_value}; +use crate::{ + SqliteStore, + blob_array, + column_value_as_u64, + insert_sql, + int_array, + subst, + u64_to_value, + with_immediate_write_tx, + with_write_tx, +}; impl SqliteStore { // READER METHODS @@ -70,8 +71,8 @@ impl SqliteStore { .query_map([], |row| row.get(0)) .expect("no binding parameters used in query") .map(|result| { - let id: Vec = result.map_err(|e| StoreError::ParsingError(e.to_string()))?; - Ok(AccountId::read_from_bytes(&id).expect("account id is valid")) + let id: Vec = result.into_store_error()?; + Ok(AccountId::read_from_bytes(&id)?) }) .collect::, StoreError>>() } @@ -195,12 +196,11 @@ impl SqliteStore { Ok(Some(AccountRecord::new(account_record_data, status, client_account_type))) } - pub fn get_foreign_account_code( + pub(crate) fn get_foreign_account_code( conn: &mut Connection, account_ids: Vec, ) -> Result, StoreError> { - let params: Vec = - account_ids.into_iter().map(|id| Value::Blob(id.to_bytes())).collect(); + let account_id_list = blob_array(account_ids); const QUERY: &str = " SELECT account_id, code FROM foreign_account_code JOIN account_code ON foreign_account_code.code_commitment = account_code.commitment @@ -208,25 +208,17 @@ impl SqliteStore { conn.prepare_cached(QUERY) .into_store_error()? - .query_map([Rc::new(params)], |row| Ok((row.get(0)?, row.get(1)?))) - .expect("no binding parameters used in query") + .query_map([account_id_list], |row| Ok((row.get("account_id")?, row.get("code")?))) + .into_store_error()? .map(|result| { - result.map_err(|err| StoreError::ParsingError(err.to_string())).and_then( - |(id, code): (Vec, Vec)| { - Ok(( - AccountId::read_from_bytes(&id) - .map_err(StoreError::DataDeserializationError)?, - AccountCode::read_from_bytes(&code) - .map_err(StoreError::DataDeserializationError)?, - )) - }, - ) + let (id, code): (Vec, Vec) = result.into_store_error()?; + Ok((AccountId::read_from_bytes(&id)?, AccountCode::read_from_bytes(&code)?)) }) .collect::, _>>() } /// Retrieves the full asset vault for a specific account. - pub fn get_account_vault( + pub(crate) fn get_account_vault( conn: &Connection, account_id: AccountId, ) -> Result { @@ -235,7 +227,7 @@ impl SqliteStore { } /// Retrieves the full storage for a specific account. - pub fn get_account_storage( + pub(crate) fn get_account_storage( conn: &Connection, account_id: AccountId, filter: &AccountStorageFilter, @@ -328,85 +320,64 @@ impl SqliteStore { initial_address: &Address, client_account_type: ClientAccountType, ) -> Result<(), StoreError> { - let db_tx = conn - .transaction_with_behavior(TransactionBehavior::Immediate) - .into_store_error()?; - { - let mut smt_forest = ScopedAccountForest::new(SqliteForestBackend::new(&db_tx))?; - Self::insert_account_code(&db_tx, account.code())?; + with_immediate_write_tx(conn, |tx| { + let mut smt_forest = ScopedAccountForest::new(SqliteForestBackend::new(tx))?; + Self::insert_account_code(tx, account.code())?; let account_id = account.id(); - Self::insert_storage_slots(&db_tx, account_id, account.storage().slots().iter())?; - Self::insert_assets(&db_tx, account_id, account.vault().assets())?; + Self::insert_storage_slots(tx, account_id, account.storage().slots().iter())?; + Self::insert_assets(tx, account_id, account.vault().assets())?; let watched = matches!(client_account_type, ClientAccountType::Watched); - Self::insert_new_account_header(&db_tx, &account.into(), account.seed(), watched)?; - Self::insert_address(&db_tx, initial_address, account.id())?; + Self::insert_new_account_header(tx, &account.into(), account.seed(), watched)?; + Self::insert_address_tx(tx, initial_address, account.id())?; Self::reconcile_account_forest( - &db_tx, + tx, &mut smt_forest, account_id, account.vault(), account.storage(), - )?; - } - db_tx.commit().into_store_error() + ) + }) } pub(crate) fn update_account( conn: &mut Connection, new_account_state: &Account, ) -> Result<(), StoreError> { - const QUERY: &str = "SELECT id FROM latest_account_headers WHERE id = ?"; - if conn - .prepare(QUERY) - .into_store_error()? - .query_map(params![new_account_state.id().to_bytes()], |row| row.get(0)) - .into_store_error()? - .map(|result| { - result.map_err(|err| StoreError::ParsingError(err.to_string())).and_then( - |id: Vec| { - AccountId::read_from_bytes(&id) - .map_err(StoreError::DataDeserializationError) - }, - ) - }) - .next() - .is_none() - { - return Err(StoreError::AccountDataNotFound(new_account_state.id())); - } - - let db_tx = conn - .transaction_with_behavior(TransactionBehavior::Immediate) - .into_store_error()?; - { - let mut smt_forest = ScopedAccountForest::new(SqliteForestBackend::new(&db_tx))?; - Self::update_account_state(&db_tx, &mut smt_forest, new_account_state)?; - } - db_tx.commit().into_store_error() + with_immediate_write_tx(conn, |tx| { + let mut smt_forest = ScopedAccountForest::new(SqliteForestBackend::new(tx))?; + Self::update_account_state(tx, &mut smt_forest, new_account_state) + }) } - pub fn upsert_foreign_account_code( + pub(crate) fn upsert_foreign_account_code( conn: &mut Connection, account_id: AccountId, code: &AccountCode, ) -> Result<(), StoreError> { - let tx = conn.transaction().into_store_error()?; - - Self::insert_account_code(&tx, code)?; + with_write_tx(conn, |tx| { + Self::insert_account_code(tx, code)?; - const QUERY: &str = - insert_sql!(foreign_account_code { account_id, code_commitment } | REPLACE); + const QUERY: &str = + insert_sql!(foreign_account_code { account_id, code_commitment } | REPLACE); - tx.execute(QUERY, params![account_id.to_bytes(), code.commitment().to_bytes()]) - .into_store_error()?; + tx.execute(QUERY, params![account_id.to_bytes(), code.commitment().to_bytes()]) + .into_store_error()?; - Self::insert_account_code(&tx, code)?; - tx.commit().into_store_error() + Ok(()) + }) } pub(crate) fn insert_address( + conn: &mut Connection, + address: &Address, + account_id: AccountId, + ) -> Result<(), StoreError> { + with_write_tx(conn, |tx| Self::insert_address_tx(tx, address, account_id)) + } + + pub(crate) fn insert_address_tx( tx: &Transaction<'_>, address: &Address, account_id: AccountId, @@ -423,12 +394,12 @@ impl SqliteStore { conn: &mut Connection, address: &Address, ) -> Result<(), StoreError> { - let tx = conn.transaction().into_store_error()?; - let serialized_address = address.to_bytes(); - const DELETE_QUERY: &str = "DELETE FROM addresses WHERE address = ?"; - tx.execute(DELETE_QUERY, params![serialized_address]).into_store_error()?; - - tx.commit().into_store_error() + with_write_tx(conn, |tx| { + let serialized_address = address.to_bytes(); + const DELETE_QUERY: &str = "DELETE FROM addresses WHERE address = ?"; + tx.execute(DELETE_QUERY, params![serialized_address]).into_store_error()?; + Ok(()) + }) } /// Inserts an [`AccountCode`]. @@ -630,12 +601,8 @@ impl SqliteStore { return Ok(()); } - let commitment_params = Rc::new( - discarded_states - .iter() - .map(|(_, commitment)| Value::Blob(commitment.to_bytes())) - .collect::>(), - ); + let commitment_params = + blob_array(discarded_states.iter().map(|(_, commitment)| commitment)); // Step 1: Resolve (account_id, nonce) pairs from both latest and historical headers. // The most recent discarded state is in latest, older ones are in historical. @@ -644,17 +611,18 @@ impl SqliteStore { "SELECT id, nonce FROM latest_account_headers WHERE account_commitment IN rarray(?)", "SELECT id, nonce FROM historical_account_headers WHERE account_commitment IN rarray(?)", ] { - id_nonce_pairs.extend( - tx.prepare(query) - .into_store_error()? - .query_map(params![commitment_params.clone()], |row| { - let id: Vec = row.get(0)?; - let nonce: u64 = column_value_as_u64(row, 1)?; - Ok((id, nonce)) - }) - .into_store_error()? - .filter_map(Result::ok), - ); + let pairs = tx + .prepare(query) + .into_store_error()? + .query_map(params![commitment_params.clone()], |row| { + let id: Vec = row.get("id")?; + let nonce: u64 = column_value_as_u64(row, "nonce")?; + Ok((id, nonce)) + }) + .into_store_error()? + .collect::, _>>() + .into_store_error()?; + id_nonce_pairs.extend(pairs); } // Step 2: Group nonces by account, sort descending (undo most recent first). @@ -753,7 +721,7 @@ impl SqliteStore { } // Step 5: Delete all consumed historical entries at the discarded nonces - let nonce_params = Rc::new(nonces.iter().map(|n| u64_to_value(*n)).collect::>()); + let nonce_params = int_array(nonces.iter().copied()); for table in [ "historical_account_storage", "historical_storage_map_entries", @@ -1120,7 +1088,7 @@ impl SqliteStore { .query_row( "SELECT account_seed, locked, watched FROM latest_account_headers WHERE id = ?", params![&id_bytes], - |row| Ok((row.get(0)?, row.get(1)?, row.get(2)?)), + |row| Ok((row.get("account_seed")?, row.get("locked")?, row.get("watched")?)), ) .optional() .into_store_error()? @@ -1174,88 +1142,91 @@ impl SqliteStore { /// Deletes all historical entries with `replaced_at_nonce <= up_to_nonce` /// (see DESIGN.md for why this threshold is safe), then removes any account /// code that was only referenced by the deleted headers. - pub fn prune_account_history( + pub(crate) fn prune_account_history( conn: &mut Connection, account_id: AccountId, up_to_nonce: Felt, ) -> Result { - let tx = conn.transaction().into_store_error()?; - let account_id_bytes = account_id.to_bytes(); - let boundary_val = u64_to_value(up_to_nonce.as_canonical_u64()); - let mut total_deleted: usize = 0; - - // Collect code commitments from headers we are about to delete. - let candidate_code_commitments: Vec> = { - let mut stmt = tx - .prepare( - "SELECT DISTINCT code_commitment FROM historical_account_headers \ + with_write_tx(conn, |tx| { + let account_id_bytes = account_id.to_bytes(); + let boundary_val = u64_to_value(up_to_nonce.as_canonical_u64()); + let mut total_deleted: usize = 0; + + // Collect code commitments from headers we are about to delete. + let candidate_code_commitments: Vec> = { + let mut stmt = tx + .prepare( + "SELECT DISTINCT code_commitment FROM historical_account_headers \ WHERE id = ? AND replaced_at_nonce <= ?", - ) - .into_store_error()?; - let rows = stmt - .query_map(params![&account_id_bytes, &boundary_val], |row| row.get(0)) - .into_store_error()?; - rows.collect::>, _>>().into_store_error()? - }; + ) + .into_store_error()?; + let rows = stmt + .query_map(params![&account_id_bytes, &boundary_val], |row| row.get(0)) + .into_store_error()?; + rows.collect::>, _>>().into_store_error()? + }; - // Delete historical entries. - total_deleted += tx - .execute( - "DELETE FROM historical_account_headers \ + // Delete historical entries. + total_deleted += tx + .execute( + "DELETE FROM historical_account_headers \ WHERE id = ? AND replaced_at_nonce <= ?", - params![&account_id_bytes, &boundary_val], - ) - .into_store_error()?; + params![&account_id_bytes, &boundary_val], + ) + .into_store_error()?; - total_deleted += tx - .execute( - "DELETE FROM historical_account_storage \ + total_deleted += tx + .execute( + "DELETE FROM historical_account_storage \ WHERE account_id = ? AND replaced_at_nonce <= ?", - params![&account_id_bytes, &boundary_val], - ) - .into_store_error()?; + params![&account_id_bytes, &boundary_val], + ) + .into_store_error()?; - total_deleted += tx - .execute( - "DELETE FROM historical_storage_map_entries \ + total_deleted += tx + .execute( + "DELETE FROM historical_storage_map_entries \ WHERE account_id = ? AND replaced_at_nonce <= ?", - params![&account_id_bytes, &boundary_val], - ) - .into_store_error()?; + params![&account_id_bytes, &boundary_val], + ) + .into_store_error()?; - total_deleted += tx - .execute( - "DELETE FROM historical_account_assets \ + total_deleted += tx + .execute( + "DELETE FROM historical_account_assets \ WHERE account_id = ? AND replaced_at_nonce <= ?", - params![&account_id_bytes, &boundary_val], - ) - .into_store_error()?; + params![&account_id_bytes, &boundary_val], + ) + .into_store_error()?; - // Delete orphaned code: only check commitments from the deleted headers, - // and only if they are not referenced by any remaining header or foreign code. - for commitment in &candidate_code_commitments { - let still_referenced: bool = tx - .query_row( - "SELECT EXISTS( + // Delete orphaned code: only check commitments from the deleted headers, + // and only if they are not referenced by any remaining header or foreign code. + for commitment in &candidate_code_commitments { + let still_referenced: bool = tx + .query_row( + "SELECT EXISTS( SELECT 1 FROM latest_account_headers WHERE code_commitment = ?1 UNION ALL SELECT 1 FROM historical_account_headers WHERE code_commitment = ?1 UNION ALL SELECT 1 FROM foreign_account_code WHERE code_commitment = ?1 )", - params![commitment], - |row| row.get(0), - ) - .into_store_error()?; - - if !still_referenced { - total_deleted += tx - .execute("DELETE FROM account_code WHERE commitment = ?", params![commitment]) + params![commitment], + |row| row.get(0), + ) .into_store_error()?; + + if !still_referenced { + total_deleted += tx + .execute( + "DELETE FROM account_code WHERE commitment = ?", + params![commitment], + ) + .into_store_error()?; + } } - } - tx.commit().into_store_error()?; - Ok(total_deleted) + Ok(total_deleted) + }) } } diff --git a/crates/sqlite-store/src/account/mod.rs b/crates/sqlite-store/src/account/mod.rs index f0f39a97e7..87f3db0b21 100644 --- a/crates/sqlite-store/src/account/mod.rs +++ b/crates/sqlite-store/src/account/mod.rs @@ -3,7 +3,7 @@ #![allow(clippy::items_after_statements)] mod accounts; -pub(crate) mod helpers; +pub(crate) mod rows; mod storage; mod vault; diff --git a/crates/sqlite-store/src/account/helpers.rs b/crates/sqlite-store/src/account/rows.rs similarity index 76% rename from crates/sqlite-store/src/account/helpers.rs rename to crates/sqlite-store/src/account/rows.rs index 242253744d..389eed5074 100644 --- a/crates/sqlite-store/src/account/helpers.rs +++ b/crates/sqlite-store/src/account/rows.rs @@ -1,4 +1,4 @@ -//! Helper functions for account operations. +//! Row mappers and query helpers for the account tables. use std::collections::BTreeMap; @@ -17,10 +17,10 @@ use miden_client::asset::{Asset, AssetId}; use miden_client::store::{AccountStatus, AccountStorageFilter, ClientAccountType, StoreError}; use miden_client::{Deserializable, Serializable, Word}; use rusqlite::types::Value; -use rusqlite::{Connection, Params, params, params_from_iter}; +use rusqlite::{Connection, Params, ToSql, params, params_from_iter}; -use crate::column_value_as_u64; use crate::sql_error::SqlResultExt; +use crate::{column_value_as_u64, text_array}; pub(crate) struct SerializedHeaderData { pub id: Vec, @@ -53,11 +53,12 @@ pub(crate) fn parse_accounts( _ => AccountStatus::Tracked, }; - let nonce = miden_client::Felt::new(nonce).expect("stored nonce must be a valid Felt"); + let nonce = miden_client::Felt::new(nonce).map_err(|err| { + StoreError::ParsingError(format!("stored nonce is not a valid Felt: {err}")) + })?; Ok(( AccountHeader::new( - AccountId::read_from_bytes(&id) - .expect("Conversion from stored AccountID should not panic"), + AccountId::read_from_bytes(&id)?, nonce, Word::read_from_bytes(&vault_root)?, Word::read_from_bytes(&storage_commitment)?, @@ -67,6 +68,24 @@ pub(crate) fn parse_accounts( )) } +/// Columns shared by `latest_account_headers` and `historical_account_headers` that make up a +/// [`SerializedHeaderData`] row; the mappers below read exactly these columns by name. +const ACCOUNT_HEADER_COLUMNS: &str = + "id, nonce, vault_root, storage_commitment, code_commitment, account_seed, locked"; + +/// Reads the [`SerializedHeaderData`] columns from a header row. +fn parse_header_row(row: &rusqlite::Row<'_>) -> Result { + Ok(SerializedHeaderData { + id: row.get("id")?, + nonce: column_value_as_u64(row, "nonce")?, + vault_root: row.get("vault_root")?, + storage_commitment: row.get("storage_commitment")?, + code_commitment: row.get("code_commitment")?, + account_seed: row.get("account_seed")?, + locked: row.get("locked")?, + }) +} + /// Fetches rows from `latest_account_headers`. Each row includes the [`ClientAccountType`], /// which `historical_account_headers` doesn't carry — that's why this query lives separately /// from [`query_historical_account_headers`]. @@ -76,33 +95,16 @@ pub(crate) fn query_latest_account_headers( params: impl Params, ) -> Result, StoreError> { let query = format!( - "SELECT id, nonce, vault_root, storage_commitment, code_commitment, account_seed, locked, watched \ + "SELECT {ACCOUNT_HEADER_COLUMNS}, watched \ FROM latest_account_headers WHERE {where_clause}" ); conn.prepare(&query) .into_store_error()? .query_map(params, |row| { - let id: Vec = row.get(0)?; - let nonce: u64 = column_value_as_u64(row, 1)?; - let vault_root: Vec = row.get(2)?; - let storage_commitment: Vec = row.get(3)?; - let code_commitment: Vec = row.get(4)?; - let account_seed: Option> = row.get(5)?; - let locked: bool = row.get(6)?; - let watched: bool = row.get(7)?; + let parts = parse_header_row(row)?; + let watched: bool = row.get("watched")?; - Ok(( - SerializedHeaderData { - id, - nonce, - vault_root, - storage_commitment, - code_commitment, - account_seed, - locked, - }, - watched, - )) + Ok((parts, watched)) }) .into_store_error()? .map(|result| { @@ -124,30 +126,12 @@ pub(crate) fn query_historical_account_headers( params: impl Params, ) -> Result, StoreError> { let query = format!( - "SELECT id, nonce, vault_root, storage_commitment, code_commitment, account_seed, locked \ + "SELECT {ACCOUNT_HEADER_COLUMNS} \ FROM historical_account_headers WHERE {where_clause}" ); conn.prepare(&query) .into_store_error()? - .query_map(params, |row| { - let id: Vec = row.get(0)?; - let nonce: u64 = column_value_as_u64(row, 1)?; - let vault_root: Vec = row.get(2)?; - let storage_commitment: Vec = row.get(3)?; - let code_commitment: Vec = row.get(4)?; - let account_seed: Option> = row.get(5)?; - let locked: bool = row.get(6)?; - - Ok(SerializedHeaderData { - id, - nonce, - vault_root, - storage_commitment, - code_commitment, - account_seed, - locked, - }) - }) + .query_map(params, parse_header_row) .into_store_error()? .map(|result| parse_accounts(result.into_store_error()?)) .collect::, StoreError>>() @@ -207,8 +191,8 @@ pub(crate) fn query_vault_assets( conn.prepare(VAULT_QUERY) .into_store_error()? .query_map(params![account_id.to_bytes()], |row| { - let asset_id: Vec = row.get(0)?; - let asset: Vec = row.get(1)?; + let asset_id: Vec = row.get("asset_id")?; + let asset: Vec = row.get("asset")?; Ok((asset_id, asset)) }) .into_store_error()? @@ -229,26 +213,22 @@ pub(crate) fn query_storage_slots( // Build storage values query with filter pushed to SQL let base_query = "SELECT slot_name, slot_value, slot_type FROM latest_account_storage WHERE account_id = ?1"; - let mut values_params: Vec = vec![Value::Blob(account_id.to_bytes())]; + let mut values_params: Vec> = vec![Box::new(Value::Blob(account_id.to_bytes()))]; let query = match filter { AccountStorageFilter::All => base_query.to_string(), AccountStorageFilter::SlotName(name) => { - values_params.push(Value::Text(name.to_string())); + values_params.push(Box::new(Value::Text(name.to_string()))); format!("{base_query} AND slot_name = ?2") }, AccountStorageFilter::SlotNames(names) => { if names.is_empty() { return Ok(BTreeMap::new()); } - let placeholders = - (0..names.len()).map(|i| format!("?{}", i + 2)).collect::>().join(", "); - for name in names { - values_params.push(Value::Text(name.to_string())); - } - format!("{base_query} AND slot_name IN ({placeholders})") + values_params.push(Box::new(text_array(names.iter().map(StorageSlotName::to_string)))); + format!("{base_query} AND slot_name IN rarray(?2)") }, AccountStorageFilter::Root(root) => { - values_params.push(Value::Blob(root.to_bytes())); + values_params.push(Box::new(Value::Blob(root.to_bytes()))); format!("{base_query} AND slot_value = ?2") }, }; @@ -256,9 +236,9 @@ pub(crate) fn query_storage_slots( let mut stmt = conn.prepare(&query).into_store_error()?; let storage_values = stmt .query_map(params_from_iter(values_params.iter()), |row| { - let slot_name: String = row.get(0)?; - let value: Vec = row.get(1)?; - let slot_type: u8 = row.get(2)?; + let slot_name: String = row.get("slot_name")?; + let value: Vec = row.get("slot_value")?; + let slot_type: u8 = row.get("slot_type")?; Ok((slot_name, value, slot_type)) }) .into_store_error()? @@ -312,18 +292,14 @@ pub(crate) fn query_storage_maps( ) -> Result, StoreError> { let base_query = "SELECT slot_name, key, value FROM latest_storage_map_entries WHERE account_id = ?1"; - let mut map_params: Vec = vec![Value::Blob(account_id.to_bytes())]; + let mut map_params: Vec> = vec![Box::new(Value::Blob(account_id.to_bytes()))]; let query = match slot_name_filter { Some(names) => { if names.is_empty() { return Ok(BTreeMap::new()); } - let placeholders = - (0..names.len()).map(|i| format!("?{}", i + 2)).collect::>().join(", "); - for name in names { - map_params.push(Value::Text(name.clone())); - } - format!("{base_query} AND slot_name IN ({placeholders})") + map_params.push(Box::new(text_array(names.iter().cloned()))); + format!("{base_query} AND slot_name IN rarray(?2)") }, None => base_query.to_string(), }; @@ -331,9 +307,9 @@ pub(crate) fn query_storage_maps( let mut stmt = conn.prepare(&query).into_store_error()?; let map_entries = stmt .query_map(params_from_iter(map_params.iter()), |row| { - let slot_name: String = row.get(0)?; - let key: Vec = row.get(1)?; - let value: Vec = row.get(2)?; + let slot_name: String = row.get("slot_name")?; + let key: Vec = row.get("key")?; + let value: Vec = row.get("value")?; Ok((slot_name, key, value)) }) @@ -369,9 +345,9 @@ pub(crate) fn query_storage_values( conn.prepare(STORAGE_QUERY) .into_store_error()? .query_map(params![account_id.to_bytes()], |row| { - let slot_name: String = row.get(0)?; - let value: Vec = row.get(1)?; - let slot_type: u8 = row.get(2)?; + let slot_name: String = row.get("slot_name")?; + let value: Vec = row.get("slot_value")?; + let slot_type: u8 = row.get("slot_type")?; Ok((slot_name, value, slot_type)) }) .into_store_error()? diff --git a/crates/sqlite-store/src/account/storage.rs b/crates/sqlite-store/src/account/storage.rs index dd51f33fa7..08a651d02a 100644 --- a/crates/sqlite-store/src/account/storage.rs +++ b/crates/sqlite-store/src/account/storage.rs @@ -281,7 +281,7 @@ impl SqliteStore { let mut read_stmt = tx.prepare_cached(READ_ALL_MAP_ENTRIES).into_store_error()?; let rows = read_stmt .query_map(params![account_id_bytes, slot_name_str], |row| { - Ok((row.get::<_, Vec>(0)?, row.get::<_, Vec>(1)?)) + Ok((row.get::<_, Vec>("key")?, row.get::<_, Vec>("value")?)) }) .into_store_error()?; rows.collect::>().into_store_error()? diff --git a/crates/sqlite-store/src/chain_data.rs b/crates/sqlite-store/src/chain_data.rs index c61ac6d5f4..2245fb6521 100644 --- a/crates/sqlite-store/src/chain_data.rs +++ b/crates/sqlite-store/src/chain_data.rs @@ -2,7 +2,6 @@ use std::collections::{BTreeMap, BTreeSet}; use std::num::NonZeroUsize; -use std::rc::Rc; use std::vec::Vec; use miden_client::Word; @@ -11,12 +10,11 @@ use miden_client::crypto::{Forest, InOrderIndex, MmrPeaks}; use miden_client::note::BlockNumber; use miden_client::store::{BlockRelevance, PartialBlockchainFilter, StoreError}; use miden_client::utils::{Deserializable, Serializable}; -use rusqlite::types::Value; use rusqlite::{Connection, OptionalExtension, Transaction, params, params_from_iter}; use super::SqliteStore; use crate::sql_error::SqlResultExt; -use crate::{insert_sql, subst}; +use crate::{column_value_as_u64, insert_sql, int_array, subst, with_write_tx}; struct SerializedBlockHeaderData { block_num: u32, @@ -24,7 +22,6 @@ struct SerializedBlockHeaderData { has_client_notes: bool, } struct SerializedBlockHeaderParts { - _block_num: u64, header: Vec, has_client_notes: bool, } @@ -43,16 +40,15 @@ impl SqliteStore { conn: &mut Connection, block_numbers: &BTreeSet, ) -> Result, StoreError> { - let block_number_list = block_numbers - .iter() - .map(|block_number| Value::Integer(i64::from(block_number.as_u32()))) - .collect::>(); + let block_number_list = + int_array(block_numbers.iter().map(|block_number| u64::from(block_number.as_u32()))); - const QUERY: &str = "SELECT block_num, header, has_client_notes FROM block_headers WHERE block_num IN rarray(?)"; + const QUERY: &str = + "SELECT header, has_client_notes FROM block_headers WHERE block_num IN rarray(?)"; conn.prepare(QUERY) .into_store_error()? - .query_map(params![Rc::new(block_number_list)], parse_block_headers_columns) + .query_map(params![block_number_list], parse_block_headers_columns) .into_store_error()? .map(|result| { let serialized_block_header_parts: SerializedBlockHeaderParts = @@ -65,7 +61,8 @@ impl SqliteStore { pub(crate) fn get_tracked_block_headers( conn: &mut Connection, ) -> Result, StoreError> { - const QUERY: &str = "SELECT block_num, header, has_client_notes FROM block_headers WHERE has_client_notes=true"; + const QUERY: &str = + "SELECT header, has_client_notes FROM block_headers WHERE has_client_notes=true"; conn.prepare(QUERY) .into_store_error()? .query_map(params![], parse_block_headers_columns) @@ -106,15 +103,12 @@ impl SqliteStore { PartialBlockchainFilter::List(ids) if ids.is_empty() => Ok(BTreeMap::new()), PartialBlockchainFilter::List(ids) => { - let id_values = ids - .iter() - .map(|id| Value::Integer(i64::try_from(id.inner()).expect("id is a valid i64"))) - .collect::>(); + let id_values = int_array(ids.iter().map(|id| id.inner() as u64)); query_partial_blockchain_nodes( conn, "SELECT id, node FROM partial_blockchain_nodes WHERE id IN rarray(?)", - params_from_iter([Rc::new(id_values)]), + params_from_iter([id_values]), ) }, @@ -141,7 +135,9 @@ impl SqliteStore { let row: Option<(u32, Vec)> = conn .prepare(QUERY) .into_store_error()? - .query_row(params![], |row| Ok((row.get(0)?, row.get(1)?))) + .query_row(params![], |row| { + Ok((row.get("block_num")?, row.get("partial_blockchain_peaks")?)) + }) .optional() .into_store_error()?; @@ -159,21 +155,26 @@ impl SqliteStore { nodes: &[(InOrderIndex, Word)], has_client_notes: bool, ) -> Result<(), StoreError> { - let tx = conn.transaction().into_store_error()?; - - Self::insert_block_header_tx(&tx, block_header, has_client_notes)?; - Self::insert_partial_blockchain_nodes_tx(&tx, nodes)?; - tx.commit().into_store_error()?; - Ok(()) + with_write_tx(conn, |tx| { + Self::insert_block_header_tx(tx, block_header, has_client_notes)?; + Self::insert_partial_blockchain_nodes_tx(tx, nodes) + }) } /// Inserts a list of MMR authentication nodes to the Partial Blockchain nodes table. + /// + /// The insert statement is prepared once (through the statement cache) and reused for every + /// node, since sync regularly inserts many nodes per block. pub(crate) fn insert_partial_blockchain_nodes_tx( tx: &Transaction<'_>, nodes: &[(InOrderIndex, Word)], ) -> Result<(), StoreError> { + const QUERY: &str = insert_sql!(partial_blockchain_nodes { id, node } | IGNORE); + let mut stmt = tx.prepare_cached(QUERY).into_store_error()?; for (index, node) in nodes { - insert_partial_blockchain_node(tx, *index, *node)?; + let SerializedPartialBlockchainNodeData { id, node } = + serialize_partial_blockchain_node(*index, *node); + stmt.execute(params![id, node]).into_store_error()?; } Ok(()) } @@ -205,80 +206,63 @@ impl SqliteStore { /// 2. Sets `has_client_notes = false` for `blocks_to_untrack`. /// 3. Deletes block headers with `has_client_notes = false` that are not the genesis or /// sync-height block. - pub fn prune_irrelevant_blocks( + pub(crate) fn untrack_and_prune_irrelevant_blocks( conn: &mut Connection, blocks_to_untrack: &[BlockNumber], node_indices_to_remove: &[InOrderIndex], ) -> Result<(), StoreError> { - let tx = conn.transaction().into_store_error()?; - - // 1. Delete stale MMR authentication nodes. - if !node_indices_to_remove.is_empty() { - let id_values = node_indices_to_remove - .iter() - .map(|id| Value::Integer(i64::try_from(id.inner()).expect("id is a valid i64"))) - .collect::>(); - - tx.execute( - "DELETE FROM partial_blockchain_nodes WHERE id IN rarray(?)", - params![Rc::new(id_values)], - ) - .into_store_error()?; - } + with_write_tx(conn, |tx| { + // 1. Delete stale MMR authentication nodes. + if !node_indices_to_remove.is_empty() { + let id_values = + int_array(node_indices_to_remove.iter().map(|id| id.inner() as u64)); - // 2. Mark untracked blocks as irrelevant. - if !blocks_to_untrack.is_empty() { - let block_values = blocks_to_untrack - .iter() - .map(|b| Value::Integer(i64::from(b.as_u32()))) - .collect::>(); - - tx.execute( - "UPDATE block_headers SET has_client_notes = 0 WHERE block_num IN rarray(?)", - params![Rc::new(block_values)], - ) - .into_store_error()?; - } + tx.execute( + "DELETE FROM partial_blockchain_nodes WHERE id IN rarray(?)", + params![id_values], + ) + .into_store_error()?; + } - // 3. Delete irrelevant block headers. - let genesis: u32 = BlockNumber::GENESIS.as_u32(); + // 2. Mark untracked blocks as irrelevant. + if !blocks_to_untrack.is_empty() { + let block_values = + int_array(blocks_to_untrack.iter().map(|b| u64::from(b.as_u32()))); - let sync_block: Option = tx - .query_row("SELECT block_num FROM blockchain_checkpoint LIMIT 1", [], |r| r.get(0)) - .optional() - .into_store_error()?; + tx.execute( + "UPDATE block_headers SET has_client_notes = 0 WHERE block_num IN rarray(?)", + params![block_values], + ) + .into_store_error()?; + } - if let Some(sync_height) = sync_block { - tx.execute( - "DELETE FROM block_headers \ - WHERE has_client_notes = 0 \ - AND block_num > ?1 \ - AND block_num < ?2", - rusqlite::params![genesis, sync_height], - ) - .into_store_error()?; - } + // 3. Delete irrelevant block headers. + let genesis: u32 = BlockNumber::GENESIS.as_u32(); + + let sync_block: Option = tx + .query_row("SELECT block_num FROM blockchain_checkpoint LIMIT 1", [], |r| r.get(0)) + .optional() + .into_store_error()?; + + if let Some(sync_height) = sync_block { + tx.execute( + "DELETE FROM block_headers \ + WHERE has_client_notes = 0 \ + AND block_num > ?1 \ + AND block_num < ?2", + rusqlite::params![genesis, sync_height], + ) + .into_store_error()?; + } - tx.commit().into_store_error() + Ok(()) + }) } } // HELPERS // ================================================================================================ -/// Inserts a node represented by its in-order index and the node value. -fn insert_partial_blockchain_node( - tx: &Transaction<'_>, - id: InOrderIndex, - node: Word, -) -> Result<(), StoreError> { - let SerializedPartialBlockchainNodeData { id, node } = - serialize_partial_blockchain_node(id, node); - const QUERY: &str = insert_sql!(partial_blockchain_nodes { id, node } | IGNORE); - tx.execute(QUERY, params![id, node]).into_store_error()?; - Ok(()) -} - fn query_partial_blockchain_nodes( conn: &mut Connection, sql: &str, @@ -326,15 +310,10 @@ fn serialize_block_header( fn parse_block_headers_columns( row: &rusqlite::Row<'_>, ) -> Result { - let block_num: u32 = row.get(0)?; - let header: Vec = row.get(1)?; - let has_client_notes: bool = row.get(2)?; + let header: Vec = row.get("header")?; + let has_client_notes: bool = row.get("has_client_notes")?; - Ok(SerializedBlockHeaderParts { - _block_num: u64::from(block_num), - header, - has_client_notes, - }) + Ok(SerializedBlockHeaderParts { header, has_client_notes }) } fn parse_block_header( @@ -358,23 +337,20 @@ fn serialize_partial_blockchain_node( fn parse_partial_blockchain_nodes_columns( row: &rusqlite::Row<'_>, ) -> Result { - let id: u64 = row.get(0)?; - let node = row.get(1)?; + let id = column_value_as_u64(row, "id")?; + let node = row.get("node")?; Ok(SerializedPartialBlockchainNodeParts { id, node }) } fn parse_partial_blockchain_nodes( serialized_partial_blockchain_node_parts: &SerializedPartialBlockchainNodeParts, ) -> Result<(InOrderIndex, Word), StoreError> { - let id = InOrderIndex::new( - NonZeroUsize::new( - usize::try_from(serialized_partial_blockchain_node_parts.id) - .expect("id is u64, should not fail"), - ) - .unwrap(), - ); + let id = usize::try_from(serialized_partial_blockchain_node_parts.id)?; + let id = NonZeroUsize::new(id).ok_or_else(|| { + StoreError::ParsingError("stored partial blockchain node id must be non-zero".to_string()) + })?; let node: Word = Word::read_from_bytes(&serialized_partial_blockchain_node_parts.node)?; - Ok((id, node)) + Ok((InOrderIndex::new(id), node)) } pub(crate) fn set_block_header_has_client_notes( diff --git a/crates/sqlite-store/src/db_management/migration_tests.rs b/crates/sqlite-store/src/db_management/migration_tests.rs index f3be2061ac..fe35f868c8 100644 --- a/crates/sqlite-store/src/db_management/migration_tests.rs +++ b/crates/sqlite-store/src/db_management/migration_tests.rs @@ -4,7 +4,7 @@ use rusqlite::{Connection, params}; use rusqlite_migration::{M, Migrations, SchemaVersion}; use crate::db_management::errors::SqliteStoreError; -use crate::db_management::utils::{ +use crate::db_management::migrations::{ EXPECTED_SCHEMA_HASHES, apply_migrations, apply_migrations_with, diff --git a/crates/sqlite-store/src/db_management/utils.rs b/crates/sqlite-store/src/db_management/migrations.rs similarity index 69% rename from crates/sqlite-store/src/db_management/utils.rs rename to crates/sqlite-store/src/db_management/migrations.rs index 21340da104..b1552b1fee 100644 --- a/crates/sqlite-store/src/db_management/utils.rs +++ b/crates/sqlite-store/src/db_management/migrations.rs @@ -1,58 +1,14 @@ +//! Database migrations and schema-fingerprint verification. + use std::string::String; use std::sync::LazyLock; use std::vec::Vec; -use miden_client::store::StoreError; use miden_protocol::crypto::hash::blake::{Blake3_256, Blake3Digest}; -use rusqlite::types::FromSql; -use rusqlite::{Connection, OptionalExtension, Result, ToSql, params}; +use rusqlite::{Connection, Result}; use rusqlite_migration::{M, Migrations, SchemaVersion}; use super::errors::SqliteStoreError; -use crate::sql_error::SqlResultExt; - -// MACROS -// ================================================================================================ - -/// Auxiliary macro which substitutes `$src` token by `$dst` expression. -#[macro_export] -macro_rules! subst { - ($src:tt, $dst:expr_2021) => { - $dst - }; -} - -/// Generates a simple insert SQL statement with parameters for the provided table name and fields. -/// Supports optional conflict resolution (adding "| REPLACE" or "| IGNORE" at the end will generate -/// "OR REPLACE" and "OR IGNORE", correspondingly). -/// -/// # Usage: -/// -/// ```ignore -/// insert_sql!(users { id, first_name, last_name, age } | REPLACE); -/// ``` -/// -/// which generates: -/// ```sql -/// INSERT OR REPLACE INTO `users` (`id`, `first_name`, `last_name`, `age`) VALUES (?, ?, ?, ?) -/// ``` -#[macro_export] -macro_rules! insert_sql { - ($table:ident { $first_field:ident $(, $($field:ident),+)? $(,)? } $(| $on_conflict:expr)?) => { - concat!( - stringify!(INSERT $(OR $on_conflict)? INTO ), - "`", - stringify!($table), - "` (`", - stringify!($first_field), - $($(concat!("`, `", stringify!($field))),+ ,)? - "`) VALUES (", - subst!($first_field, "?"), - $($(subst!($field, ", ?")),+ ,)? - ")" - ) - }; -} // MIGRATIONS // ================================================================================================ @@ -61,7 +17,7 @@ type Hash = Blake3Digest<32>; const SCHEMA_HASH_DOMAIN: &[u8] = b"miden-client-sqlite-schema-v1"; -const MIGRATION_SCRIPTS: [&str; 1] = [include_str!("../store.sql")]; +const MIGRATION_SCRIPTS: [&str; 1] = [include_str!("../migrations/0001_init.sql")]; static MIGRATIONS: LazyLock = LazyLock::new(prepare_migrations); pub(crate) static EXPECTED_SCHEMA_HASHES: LazyLock> = LazyLock::new(compute_expected_schema_hashes); @@ -172,41 +128,6 @@ fn normalize_sql(sql: &str) -> String { .join(" ") } -pub fn get_setting(conn: &mut Connection, name: &str) -> Result, StoreError> { - conn.transaction() - .into_store_error()? - .query_row("SELECT value FROM settings WHERE name = $1", params![name], |row| row.get(0)) - .optional() - .into_store_error() -} - -pub fn set_setting(conn: &Connection, name: &str, value: &T) -> Result<()> { - let count = - conn.execute(insert_sql!(settings { name, value } | REPLACE), params![name, value])?; - - debug_assert_eq!(count, 1); - - Ok(()) -} - -pub fn remove_setting(conn: &Connection, name: &str) -> Result<(), StoreError> { - let count = conn - .execute("DELETE FROM settings WHERE name = $1", params![name]) - .into_store_error()?; - - debug_assert_eq!(count, 1); - - Ok(()) -} - -pub fn list_setting_keys(conn: &Connection) -> Result, StoreError> { - let mut stmt = conn.prepare("SELECT name FROM settings").into_store_error()?; - stmt.query_map([], |row| row.get::<_, String>(0)) - .into_store_error()? - .collect::, _>>() - .into_store_error() -} - // TESTS // ================================================================================================ diff --git a/crates/sqlite-store/src/db_management/mod.rs b/crates/sqlite-store/src/db_management/mod.rs index 3f80965323..59719b8964 100644 --- a/crates/sqlite-store/src/db_management/mod.rs +++ b/crates/sqlite-store/src/db_management/mod.rs @@ -1,6 +1,7 @@ pub(crate) mod errors; +pub(crate) mod migrations; pub(crate) mod pool_manager; -pub(crate) mod utils; +pub(crate) mod settings; #[cfg(test)] mod migration_tests; diff --git a/crates/sqlite-store/src/db_management/settings.rs b/crates/sqlite-store/src/db_management/settings.rs new file mode 100644 index 0000000000..79dc088ae8 --- /dev/null +++ b/crates/sqlite-store/src/db_management/settings.rs @@ -0,0 +1,57 @@ +//! Persistence for the client's key-value settings table. + +use std::string::String; +use std::vec::Vec; + +use miden_client::store::{SettingMutation, StoreError}; +use rusqlite::types::FromSql; +use rusqlite::{Connection, OptionalExtension, Result, ToSql, params}; + +use crate::sql_error::SqlResultExt; +use crate::{SqliteStore, insert_sql, subst, with_write_tx}; + +impl SqliteStore { + /// Applies the provided setting mutations atomically. + pub(crate) fn apply_settings_mutations( + conn: &mut Connection, + mutations: &[SettingMutation], + ) -> Result<(), StoreError> { + with_write_tx(conn, |tx| { + for mutation in mutations { + match mutation { + SettingMutation::Set { key, value } => set_setting(tx, key, value)?, + SettingMutation::Remove { key } => remove_setting(tx, key)?, + } + } + Ok(()) + }) + } +} + +pub fn get_setting(conn: &Connection, name: &str) -> Result, StoreError> { + conn.query_row("SELECT value FROM settings WHERE name = $1", params![name], |row| row.get(0)) + .optional() + .into_store_error() +} + +pub fn set_setting(conn: &Connection, name: &str, value: &T) -> Result<(), StoreError> { + conn.execute(insert_sql!(settings { name, value } | REPLACE), params![name, value]) + .into_store_error()?; + + Ok(()) +} + +pub fn remove_setting(conn: &Connection, name: &str) -> Result<(), StoreError> { + conn.execute("DELETE FROM settings WHERE name = $1", params![name]) + .into_store_error()?; + + Ok(()) +} + +pub fn list_setting_keys(conn: &Connection) -> Result, StoreError> { + let mut stmt = conn.prepare("SELECT name FROM settings").into_store_error()?; + stmt.query_map([], |row| row.get::<_, String>(0)) + .into_store_error()? + .collect::, _>>() + .into_store_error() +} diff --git a/crates/sqlite-store/src/forest.rs b/crates/sqlite-store/src/forest.rs index 05a350ed9b..e2a9e23d36 100644 --- a/crates/sqlite-store/src/forest.rs +++ b/crates/sqlite-store/src/forest.rs @@ -152,9 +152,9 @@ fn tree_meta(conn: &Connection, lineage: LineageId) -> Result>(1)?, - column_value_as_u64(row, 2)?, + column_value_as_u64(row, "version")?, + row.get::<_, Vec>("root")?, + column_value_as_u64(row, "entry_count")?, )) }, ) @@ -186,7 +186,7 @@ fn load_entries(conn: &Connection, lineage: LineageId) -> Result>(0)?, row.get::<_, Vec>(1)?)) + Ok((row.get::<_, Vec>("key")?, row.get::<_, Vec>("value")?)) }) .map_err(internal)?; @@ -239,7 +239,7 @@ fn load_leaf_entries( .map_err(internal)?; let rows = stmt .query_map(params![lineage.as_bytes().as_slice(), u64_to_value(position)], |row| { - Ok((row.get::<_, Vec>(0)?, row.get::<_, Vec>(1)?)) + Ok((row.get::<_, Vec>("key")?, row.get::<_, Vec>("value")?)) }) .map_err(internal)?; @@ -401,9 +401,9 @@ fn compute_update_mutations( let rows = stmt .query_map(params![lineage.as_bytes().as_slice()], |row| { Ok(( - row.get::<_, Vec>(0)?, - row.get::<_, Vec>(1)?, - column_value_as_u64(row, 2)?, + row.get::<_, Vec>("key")?, + row.get::<_, Vec>("value")?, + column_value_as_u64(row, "leaf_position")?, )) }) .map_err(internal)?; @@ -769,9 +769,9 @@ impl BackendReader for SqliteForestBackend<'_, '_> { let rows = stmt .query_map([], |row| { Ok(( - row.get::<_, Vec>(0)?, - column_value_as_u64(row, 1)?, - row.get::<_, Vec>(2)?, + row.get::<_, Vec>("lineage")?, + column_value_as_u64(row, "version")?, + row.get::<_, Vec>("root")?, )) }) .map_err(internal)?; @@ -1040,7 +1040,7 @@ mod tests { use miden_protocol::{Felt, ONE, ZERO}; use super::*; - use crate::db_management::utils::apply_migrations; + use crate::db_management::migrations::apply_migrations; fn setup_conn() -> Connection { let mut conn = Connection::open_in_memory().unwrap(); diff --git a/crates/sqlite-store/src/lib.rs b/crates/sqlite-store/src/lib.rs index e63b498197..02fe54ec17 100644 --- a/crates/sqlite-store/src/lib.rs +++ b/crates/sqlite-store/src/lib.rs @@ -7,17 +7,13 @@ use std::boxed::Box; use std::collections::{BTreeMap, BTreeSet}; use std::path::PathBuf; +use std::rc::Rc; use std::string::{String, ToString}; use std::vec::Vec; +use db_management::migrations::apply_migrations; use db_management::pool_manager::{Pool, SqlitePoolManager}; -use db_management::utils::{ - apply_migrations, - get_setting, - list_setting_keys, - remove_setting, - set_setting, -}; +use db_management::settings::{get_setting, list_setting_keys, remove_setting, set_setting}; use miden_client::Word; use miden_client::account::{ Account, @@ -50,6 +46,7 @@ use miden_client::store::{ }; use miden_client::sync::{NoteTagRecord, StateSyncUpdate}; use miden_client::transaction::{TransactionRecord, TransactionStoreUpdate}; +use miden_client::utils::Serializable; use miden_protocol::Felt; use miden_protocol::account::StorageMapWitness; use miden_protocol::asset::AssetId; @@ -62,6 +59,7 @@ mod builder; mod chain_data; mod db_management; mod forest; +mod macros; mod note; mod sql_error; mod sync; @@ -75,7 +73,7 @@ pub use builder::ClientBuilderSqliteExt; /// Represents a pool of connections with an `SQLite` database. The pool is used to interact /// concurrently with the underlying database in a safe and efficient manner. /// -/// Current table definitions can be found at `store.sql` migration file. +/// Current table definitions can be found in the `migrations/` SQL files. pub struct SqliteStore { pub(crate) pool: Pool, database_filepath: String, @@ -275,7 +273,11 @@ impl Store for SqliteStore { let blocks_to_untrack = blocks_to_untrack.to_vec(); let node_indices_to_remove = node_indices_to_remove.to_vec(); self.interact_with_connection(move |conn| { - SqliteStore::prune_irrelevant_blocks(conn, &blocks_to_untrack, &node_indices_to_remove) + SqliteStore::untrack_and_prune_irrelevant_blocks( + conn, + &blocks_to_untrack, + &node_indices_to_remove, + ) }) .await } @@ -296,11 +298,10 @@ impl Store for SqliteStore { block_numbers: &BTreeSet, ) -> Result, StoreError> { let block_numbers = block_numbers.clone(); - Ok(self - .interact_with_connection(move |conn| { - SqliteStore::get_block_headers(conn, &block_numbers) - }) - .await?) + self.interact_with_connection(move |conn| { + SqliteStore::get_block_headers(conn, &block_numbers) + }) + .await } async fn get_tracked_block_headers(&self) -> Result, StoreError> { @@ -420,10 +421,7 @@ impl Store for SqliteStore { } async fn set_setting(&self, key: String, value: Vec) -> Result<(), StoreError> { - self.interact_with_connection(move |conn| { - set_setting(conn, &key, &value).into_store_error() - }) - .await + self.interact_with_connection(move |conn| set_setting(conn, &key, &value)).await } async fn get_setting(&self, key: String) -> Result>, StoreError> { @@ -443,17 +441,7 @@ impl Store for SqliteStore { mutations: Vec, ) -> Result<(), StoreError> { self.interact_with_connection(move |conn| { - let tx = conn.transaction().into_store_error()?; - for mutation in &mutations { - match mutation { - SettingMutation::Set { key, value } => { - set_setting(&tx, key, value).into_store_error()?; - }, - SettingMutation::Remove { key } => remove_setting(&tx, key)?, - } - } - tx.commit().into_store_error()?; - Ok(()) + SqliteStore::apply_settings_mutations(conn, &mutations) }) .await } @@ -518,9 +506,7 @@ impl Store for SqliteStore { account_id: AccountId, ) -> Result<(), StoreError> { self.interact_with_connection(move |conn| { - let tx = conn.transaction().into_store_error()?; - SqliteStore::insert_address(&tx, &address, account_id)?; - tx.commit().into_store_error() + SqliteStore::insert_address(conn, &address, account_id) }) .await } @@ -554,7 +540,7 @@ pub(crate) fn current_timestamp_u64() -> u64 { /// /// `Sqlite` uses `i64` as its internal representation format, and so when retrieving /// we need to make sure we cast as `u64` to get the original value -pub fn column_value_as_u64( +pub(crate) fn column_value_as_u64( row: &rusqlite::Row<'_>, index: I, ) -> rusqlite::Result { @@ -570,7 +556,7 @@ pub fn column_value_as_u64( /// /// `Sqlite` uses `i64` as its internal representation format. Note that the `as` operator performs /// a lossless conversion from `u64` to `i64`. -pub fn u64_to_value(v: u64) -> Value { +pub(crate) fn u64_to_value(v: u64) -> Value { #[allow( clippy::cast_possible_wrap, reason = "We store u64 as i64 as sqlite only allows the latter." @@ -578,6 +564,56 @@ pub fn u64_to_value(v: u64) -> Value { Value::Integer(v as i64) } +/// Builds the value list for a `rarray(?)` parameter from serializable items, each stored as a +/// BLOB of its canonical byte encoding. +/// +/// Binding the list as a single table-valued parameter keeps the SQL text constant, so the +/// prepared statement stays cacheable regardless of the list length (and the list is not subject +/// to `SQLite`'s bound-parameter limit). +pub(crate) fn blob_array(items: impl IntoIterator) -> Rc> { + Rc::new(items.into_iter().map(|item| Value::Blob(item.to_bytes())).collect()) +} + +/// Builds the value list for a `rarray(?)` parameter from `u64` values, stored as SQL INTEGERs +/// through the same bit-cast as [`u64_to_value`]. +pub(crate) fn int_array(items: impl IntoIterator) -> Rc> { + Rc::new(items.into_iter().map(u64_to_value).collect()) +} + +/// Builds the value list for a `rarray(?)` parameter from string values, stored as SQL TEXT. +pub(crate) fn text_array(items: impl IntoIterator) -> Rc> { + Rc::new(items.into_iter().map(Value::Text).collect()) +} + +/// Runs `f` inside a rusqlite transaction, committing on `Ok` and rolling back on `Err`. +pub(crate) fn with_write_tx( + conn: &mut Connection, + f: impl FnOnce(&rusqlite::Transaction<'_>) -> Result, +) -> Result { + with_write_tx_behavior(conn, rusqlite::TransactionBehavior::Deferred, f) +} + +/// Runs `f` inside an `IMMEDIATE` rusqlite transaction, committing on `Ok` and rolling back on +/// `Err`. Immediate transactions take the write lock up front, so writes that read current state +/// first cannot be invalidated by a concurrent writer between the read and the write. +pub(crate) fn with_immediate_write_tx( + conn: &mut Connection, + f: impl FnOnce(&rusqlite::Transaction<'_>) -> Result, +) -> Result { + with_write_tx_behavior(conn, rusqlite::TransactionBehavior::Immediate, f) +} + +fn with_write_tx_behavior( + conn: &mut Connection, + behavior: rusqlite::TransactionBehavior, + f: impl FnOnce(&rusqlite::Transaction<'_>) -> Result, +) -> Result { + let tx = conn.transaction_with_behavior(behavior).into_store_error()?; + let result = f(&tx)?; + tx.commit().into_store_error()?; + Ok(result) +} + // TESTS // ================================================================================================ @@ -588,10 +624,53 @@ pub mod tests { use miden_client::store::Store; use miden_client::testing::common::create_test_store_path; - use super::SqliteStore; + use super::{SqliteStore, StoreError, column_value_as_u64, u64_to_value, with_write_tx}; fn assert_send_sync() {} + /// The write path bit-casts `u64` to `i64` and the read path must bit-cast it back, including + /// for values whose top bit is set (which are stored as negative SQL INTEGERs). + #[test] + fn u64_column_round_trip() { + let conn = rusqlite::Connection::open_in_memory().unwrap(); + for value in [0u64, 1, 1 << 63, u64::MAX] { + let read: u64 = conn + .query_row("SELECT ?1", [u64_to_value(value)], |row| column_value_as_u64(row, 0)) + .unwrap(); + assert_eq!(read, value); + } + } + + #[test] + fn with_write_tx_rolls_back_on_error() { + let mut conn = rusqlite::Connection::open_in_memory().unwrap(); + conn.execute_batch("CREATE TABLE t (id INTEGER PRIMARY KEY);").unwrap(); + + let result = with_write_tx(&mut conn, |tx| { + tx.execute("INSERT INTO t (id) VALUES (1)", []).unwrap(); + Err::<(), _>(StoreError::DatabaseError("forced failure".into())) + }); + assert!(result.is_err()); + + let count: i64 = conn.query_row("SELECT COUNT(*) FROM t", [], |row| row.get(0)).unwrap(); + assert_eq!(count, 0, "the insert must roll back when the closure errors"); + } + + #[test] + fn with_write_tx_commits_on_success() { + let mut conn = rusqlite::Connection::open_in_memory().unwrap(); + conn.execute_batch("CREATE TABLE t (id INTEGER PRIMARY KEY);").unwrap(); + + with_write_tx(&mut conn, |tx| { + tx.execute("INSERT INTO t (id) VALUES (1)", []).unwrap(); + Ok(()) + }) + .unwrap(); + + let count: i64 = conn.query_row("SELECT COUNT(*) FROM t", [], |row| row.get(0)).unwrap(); + assert_eq!(count, 1); + } + #[test] fn is_send_sync() { assert_send_sync::(); diff --git a/crates/sqlite-store/src/macros.rs b/crates/sqlite-store/src/macros.rs new file mode 100644 index 0000000000..fb783cc010 --- /dev/null +++ b/crates/sqlite-store/src/macros.rs @@ -0,0 +1,41 @@ +//! SQL-generation macros shared across the store's modules. + +/// Auxiliary macro which substitutes `$src` token by `$dst` expression. +#[macro_export] +macro_rules! subst { + ($src:tt, $dst:expr_2021) => { + $dst + }; +} + +/// Generates a simple insert SQL statement with parameters for the provided table name and fields. +/// Supports optional conflict resolution (adding "| REPLACE" or "| IGNORE" at the end will generate +/// "OR REPLACE" and "OR IGNORE", correspondingly). +/// +/// # Usage: +/// +/// ```ignore +/// insert_sql!(users { id, first_name, last_name, age } | REPLACE); +/// ``` +/// +/// which generates: +/// ```sql +/// INSERT OR REPLACE INTO `users` (`id`, `first_name`, `last_name`, `age`) VALUES (?, ?, ?, ?) +/// ``` +#[macro_export] +macro_rules! insert_sql { + ($table:ident { $first_field:ident $(, $($field:ident),+)? $(,)? } $(| $on_conflict:expr)?) => { + concat!( + stringify!(INSERT $(OR $on_conflict)? INTO ), + "`", + stringify!($table), + "` (`", + stringify!($first_field), + $($(concat!("`, `", stringify!($field))),+ ,)? + "`) VALUES (", + subst!($first_field, "?"), + $($(subst!($field, ", ?")),+ ,)? + ")" + ) + }; +} diff --git a/crates/sqlite-store/src/store.sql b/crates/sqlite-store/src/migrations/0001_init.sql similarity index 100% rename from crates/sqlite-store/src/store.sql rename to crates/sqlite-store/src/migrations/0001_init.sql diff --git a/crates/sqlite-store/src/note/filters.rs b/crates/sqlite-store/src/note/filters.rs index 51d520458a..13df1dbf64 100644 --- a/crates/sqlite-store/src/note/filters.rs +++ b/crates/sqlite-store/src/note/filters.rs @@ -1,29 +1,38 @@ -// NOTE FILTER (OUTPUT NOTES) +// NOTE FILTER QUERIES // ================================================================================================ use std::rc::Rc; use miden_client::account::AccountId; -use miden_client::note::BlockNumber; +use miden_client::note::{BlockNumber, NoteId}; use miden_client::store::{InputNoteState, NoteFilter, OutputNoteState}; -use miden_client::utils::Serializable; use rusqlite::types::Value; +use super::{INPUT_NOTE_COLUMNS, OUTPUT_NOTE_COLUMNS}; +use crate::blob_array; + type NoteQueryParams = Vec>>; +/// Builds a `column IN rarray(?)` condition, pushing the bound value list onto `params`. +/// +/// The list is bound as a single table-valued parameter so the SQL text stays constant no matter +/// how many values the filter carries. +fn in_rarray_condition( + column: &str, + values: Rc>, + params: &mut NoteQueryParams, +) -> String { + params.push(values); + format!("({column} IN rarray(?))") +} + +// NOTE FILTER (OUTPUT NOTES) +// ================================================================================================ + /// Returns the output notes query for a specific `NoteFilter` pub(super) fn note_filter_to_query_output_notes(filter: &NoteFilter) -> (String, NoteQueryParams) { - let base = "SELECT - note.recipient_digest, - note.assets, - note.metadata, - note.expected_height, - note.state, - note.attachments - from output_notes AS note"; - let (condition, params) = note_filter_output_notes_condition(filter); - let query = format!("{base} WHERE {condition}"); + let query = format!("SELECT {OUTPUT_NOTE_COLUMNS} from output_notes AS note WHERE {condition}"); (query, params) } @@ -54,36 +63,18 @@ pub(super) fn note_filter_output_notes_condition(filter: &NoteFilter) -> (String "1 = 0".to_string() }, NoteFilter::Unique(note_id) => { - let note_ids_list = vec![Value::Blob(note_id.as_word().to_bytes())]; - params.push(Rc::new(note_ids_list)); - "note.note_id IN rarray(?)".to_string() - }, - NoteFilter::List(note_ids) => { - let note_ids_list = note_ids - .iter() - .map(|note_id| Value::Blob(note_id.as_word().to_bytes())) - .collect::>(); - - params.push(Rc::new(note_ids_list)); - "note.note_id IN rarray(?)".to_string() + in_rarray_condition("note.note_id", blob_array([note_id.as_word()]), &mut params) }, + NoteFilter::List(note_ids) => in_rarray_condition( + "note.note_id", + blob_array(note_ids.iter().map(NoteId::as_word)), + &mut params, + ), NoteFilter::DetailsCommitments(commitments) => { - let commitments_list = commitments - .iter() - .map(|commitment| Value::Blob(commitment.to_bytes())) - .collect::>(); - - params.push(Rc::new(commitments_list)); - "note.details_commitment IN rarray(?)".to_string() + in_rarray_condition("note.details_commitment", blob_array(commitments), &mut params) }, NoteFilter::Nullifiers(nullifiers) => { - let nullifiers_list = nullifiers - .iter() - .map(|nullifier| Value::Blob(nullifier.to_bytes())) - .collect::>(); - - params.push(Rc::new(nullifiers_list)); - "note.nullifier IN rarray(?)".to_string() + in_rarray_condition("note.nullifier", blob_array(nullifiers), &mut params) }, NoteFilter::Unspent => { format!( @@ -102,29 +93,25 @@ pub(super) fn note_filter_output_notes_condition(filter: &NoteFilter) -> (String // NOTE FILTER (INPUT NOTES) // ================================================================================================ -const INPUT_NOTES_BASE_QUERY: &str = "SELECT - note.assets, - note.serial_number, - note.inputs, - script.serialized_note_script, - note.state, - note.created_at, - note.attachments - from input_notes AS note - LEFT OUTER JOIN notes_scripts AS script - ON note.script_root = script.script_root"; +fn input_notes_base_query() -> String { + format!( + "SELECT {INPUT_NOTE_COLUMNS} from input_notes AS note \ + LEFT OUTER JOIN notes_scripts AS script ON note.script_root = script.script_root" + ) +} pub(super) fn note_filter_to_query_input_notes(filter: &NoteFilter) -> (String, NoteQueryParams) { + let base_query = input_notes_base_query(); let (condition, params) = note_filter_input_notes_condition(filter); let query = if matches!(filter, NoteFilter::Consumed) { format!( - "{INPUT_NOTES_BASE_QUERY} WHERE {condition} \ + "{base_query} WHERE {condition} \ ORDER BY note.consumed_block_height ASC, \ note.consumed_tx_order IS NULL, note.consumed_tx_order ASC, \ note.note_id ASC" ) } else { - format!("{INPUT_NOTES_BASE_QUERY} WHERE {condition}") + format!("{base_query} WHERE {condition}") }; (query, params) @@ -142,8 +129,9 @@ pub(super) fn note_filter_to_query_input_note_by_offset( use core::fmt::Write; let (mut condition, mut params) = note_filter_input_notes_condition(filter); - params.push(Rc::new(vec![Value::Blob(consumer.to_bytes())])); - condition.push_str(" AND note.consumer_account_id IN rarray(?)"); + let consumer_condition = + in_rarray_condition("note.consumer_account_id", blob_array([&consumer]), &mut params); + let _ = write!(condition, " AND {consumer_condition}"); condition.push_str(" AND note.consumed_tx_order IS NOT NULL"); if let Some(start) = block_start { @@ -154,9 +142,10 @@ pub(super) fn note_filter_to_query_input_note_by_offset( } let query = format!( - "{INPUT_NOTES_BASE_QUERY} WHERE {condition} \ + "{} WHERE {condition} \ ORDER BY note.consumed_block_height ASC, note.consumed_tx_order ASC, note.note_id ASC \ - LIMIT 1 OFFSET {offset}" + LIMIT 1 OFFSET {offset}", + input_notes_base_query() ); (query, params) @@ -189,45 +178,21 @@ pub(super) fn note_filter_input_notes_condition(filter: &NoteFilter) -> (String, ) }, NoteFilter::Unique(note_id) => { - let note_ids_list = vec![Value::Blob(note_id.as_word().to_bytes())]; - params.push(Rc::new(note_ids_list)); - "(note.note_id IN rarray(?))".to_string() - }, - NoteFilter::List(note_ids) => { - let note_ids_list = note_ids - .iter() - .map(|note_id| Value::Blob(note_id.as_word().to_bytes())) - .collect::>(); - - params.push(Rc::new(note_ids_list)); - "(note.note_id IN rarray(?))".to_string() + in_rarray_condition("note.note_id", blob_array([note_id.as_word()]), &mut params) }, + NoteFilter::List(note_ids) => in_rarray_condition( + "note.note_id", + blob_array(note_ids.iter().map(NoteId::as_word)), + &mut params, + ), NoteFilter::DetailsCommitments(commitments) => { - let commitments_list = commitments - .iter() - .map(|commitment| Value::Blob(commitment.to_bytes())) - .collect::>(); - - params.push(Rc::new(commitments_list)); - "(note.details_commitment IN rarray(?))".to_string() + in_rarray_condition("note.details_commitment", blob_array(commitments), &mut params) }, NoteFilter::Nullifiers(nullifiers) => { - let nullifiers_list = nullifiers - .iter() - .map(|nullifier| Value::Blob(nullifier.to_bytes())) - .collect::>(); - - params.push(Rc::new(nullifiers_list)); - "(note.nullifier IN rarray(?))".to_string() + in_rarray_condition("note.nullifier", blob_array(nullifiers), &mut params) }, NoteFilter::ScriptRoots(script_roots) => { - let script_roots_list = script_roots - .iter() - .map(|script_root| Value::Blob(script_root.to_bytes())) - .collect::>(); - - params.push(Rc::new(script_roots_list)); - "(note.script_root IN rarray(?))".to_string() + in_rarray_condition("note.script_root", blob_array(script_roots), &mut params) }, NoteFilter::Unverified => { format!("(state_discriminant = {})", InputNoteState::STATE_UNVERIFIED) diff --git a/crates/sqlite-store/src/note/mod.rs b/crates/sqlite-store/src/note/mod.rs index c12adf5c17..a09cc238f6 100644 --- a/crates/sqlite-store/src/note/mod.rs +++ b/crates/sqlite-store/src/note/mod.rs @@ -2,7 +2,6 @@ use std::collections::BTreeMap; use std::rc::Rc; -use std::string::ToString; use std::vec::Vec; use miden_client::Word; @@ -30,13 +29,13 @@ use miden_client::store::{ use miden_client::utils::{Deserializable, Serializable}; use miden_protocol::note::NoteStorage; use rusqlite::types::Value; -use rusqlite::{Connection, Transaction, params, params_from_iter}; +use rusqlite::{Connection, OptionalExtension, Transaction, params, params_from_iter}; use super::SqliteStore; use crate::chain_data::set_block_header_has_client_notes; use crate::note::filters::{note_filter_to_query_input_notes, note_filter_to_query_output_notes}; use crate::sql_error::SqlResultExt; -use crate::{insert_sql, subst}; +use crate::{column_value_as_u64, insert_sql, subst, u64_to_value, with_write_tx}; mod filters; @@ -126,10 +125,29 @@ struct SerializedOutputNoteStateUpdate { pub state: Vec, } -/// Represents the pars retrieved form the database to build a `NoteScript` -struct SerializedNoteScriptPars { - pub script: Vec, -} +// COLUMN LISTS +// ================================================================================================ + +// Each SELECT list below is the single source of truth for its row mapper: the filter modules +// build their queries from these constants and the mappers read the same columns by name, so the +// two cannot drift apart. + +/// Columns read by [`parse_input_note_columns`]. +pub(super) const INPUT_NOTE_COLUMNS: &str = "note.assets AS assets, \ + note.serial_number AS serial_number, \ + note.inputs AS inputs, \ + script.serialized_note_script AS serialized_note_script, \ + note.state AS state, \ + note.created_at AS created_at, \ + note.attachments AS attachments"; + +/// Columns read by [`parse_output_note_columns`]. +pub(super) const OUTPUT_NOTE_COLUMNS: &str = "note.recipient_digest AS recipient_digest, \ + note.assets AS assets, \ + note.metadata AS metadata, \ + note.expected_height AS expected_height, \ + note.state AS state, \ + note.attachments AS attachments"; // NOTES STORE METHODS // ================================================================================================ @@ -144,7 +162,7 @@ impl SqliteStore { .prepare(query.as_str()) .into_store_error()? .query_map(params_from_iter(params), parse_input_note_columns) - .expect("no binding parameters used in query") + .into_store_error()? .map(|result| Ok(result.into_store_error()?).and_then(parse_input_note)) .collect::, _>>()?; @@ -161,7 +179,7 @@ impl SqliteStore { .prepare(&query) .into_store_error()? .query_map(params_from_iter(params), parse_output_note_columns) - .expect("no binding parameters used in query") + .into_store_error()? .map(|result| Ok(result.into_store_error()?).and_then(parse_output_note)) .collect::, _>>()?; @@ -189,7 +207,7 @@ impl SqliteStore { .prepare(&query) .into_store_error()? .query_map(params_from_iter(params), parse_input_note_columns) - .expect("no binding parameters used in query") + .into_store_error()? .map(|result| Ok(result.into_store_error()?).and_then(parse_input_note)) .next() .transpose()?; @@ -201,22 +219,21 @@ impl SqliteStore { conn: &mut Connection, notes: &[InputNoteRecord], ) -> Result<(), StoreError> { - let tx = conn.transaction().into_store_error()?; - - for note in notes { - upsert_input_note_tx(&tx, note)?; - - // Whenever we insert a note, we also update block relevance - if let Some(inclusion_proof) = note.inclusion_proof() { - set_block_header_has_client_notes( - &tx, - inclusion_proof.location().block_num().as_u64(), - true, - )?; + with_write_tx(conn, |tx| { + for note in notes { + upsert_input_note_tx(tx, note)?; + + // Whenever we insert a note, we also update block relevance + if let Some(inclusion_proof) = note.inclusion_proof() { + set_block_header_has_client_notes( + tx, + inclusion_proof.location().block_num().as_u64(), + true, + )?; + } } - } - - tx.commit().into_store_error() + Ok(()) + }) } pub(crate) fn get_unspent_input_note_nullifiers( @@ -232,11 +249,10 @@ impl SqliteStore { conn.prepare(QUERY) .into_store_error()? .query_map([unspent_filters], |row| row.get(0)) - .expect("no binding parameters used in query") + .into_store_error()? .map(|result| { - result - .map_err(|err| StoreError::ParsingError(err.to_string())) - .and_then(|v: Vec| Ok(Nullifier::read_from_bytes(&v)?)) + let v: Vec = result.into_store_error()?; + Ok(Nullifier::read_from_bytes(&v)?) }) .collect::, _>>() } @@ -245,33 +261,32 @@ impl SqliteStore { conn: &mut Connection, note_scripts: &[NoteScript], ) -> Result<(), StoreError> { - let tx = conn.transaction().into_store_error()?; - - for note_script in note_scripts { - upsert_note_script_tx(&tx, note_script)?; - } - - tx.commit().into_store_error() + with_write_tx(conn, |tx| { + for note_script in note_scripts { + upsert_note_script_tx(tx, note_script)?; + } + Ok(()) + }) } - /// Retrieves the note scripts from the database. + /// Retrieves a note script by its root from the database. pub(crate) fn get_note_script( conn: &mut Connection, script_root: Word, ) -> Result { - let query = "SELECT * FROM notes_scripts WHERE script_root = ?"; - let note_script = conn - .prepare(query) + const QUERY: &str = + "SELECT serialized_note_script FROM notes_scripts WHERE script_root = ?"; + let script_bytes: Option> = conn + .prepare_cached(QUERY) .into_store_error()? - .query_map([script_root.to_bytes()], parse_note_scripts_columns) - .expect("no binding parameters used in query") - .map(|result| Ok(result.into_store_error()?).and_then(|s| parse_note_script(&s))) - .collect::, _>>()? - .first() - .cloned() - .ok_or(StoreError::NoteScriptNotFound(script_root.to_hex()))?; - - Ok(note_script) + .query_row([script_root.to_bytes()], |row| row.get(0)) + .optional() + .into_store_error()?; + + match script_bytes { + Some(bytes) => Ok(NoteScript::from_bytes(&bytes)?), + None => Err(StoreError::NoteScriptNotFound(script_root.to_hex())), + } } } @@ -355,13 +370,13 @@ pub(super) fn upsert_input_note_tx( fn parse_input_note_columns( row: &rusqlite::Row<'_>, ) -> Result { - let assets: Vec = row.get(0)?; - let serial_number: Vec = row.get(1)?; - let inputs: Vec = row.get(2)?; - let script: Vec = row.get(3)?; - let state: Vec = row.get(4)?; - let created_at: u64 = row.get(5)?; - let attachments: Vec = row.get(6)?; + let assets: Vec = row.get("assets")?; + let serial_number: Vec = row.get("serial_number")?; + let inputs: Vec = row.get("inputs")?; + let script: Vec = row.get("serialized_note_script")?; + let state: Vec = row.get("state")?; + let created_at: u64 = column_value_as_u64(row, "created_at")?; + let attachments: Vec = row.get("attachments")?; Ok(SerializedInputNoteParts { assets, @@ -457,12 +472,12 @@ fn serialize_input_note(note: &InputNoteRecord) -> SerializedInputNoteData { fn parse_output_note_columns( row: &rusqlite::Row<'_>, ) -> Result { - let recipient_digest: Vec = row.get(0)?; - let assets: Vec = row.get(1)?; - let metadata: Vec = row.get(2)?; - let expected_height: u32 = row.get(3)?; - let state: Vec = row.get(4)?; - let attachments: Vec = row.get(5)?; + let recipient_digest: Vec = row.get("recipient_digest")?; + let assets: Vec = row.get("assets")?; + let metadata: Vec = row.get("metadata")?; + let expected_height: u32 = row.get("expected_height")?; + let state: Vec = row.get("state")?; + let attachments: Vec = row.get("attachments")?; Ok(SerializedOutputNoteParts { assets, @@ -676,8 +691,7 @@ fn batch_insert_input_notes( } param_values.push(Value::Integer(i64::from(note.state_discriminant))); param_values.push(Value::Blob(note.state.clone())); - #[allow(clippy::cast_possible_wrap)] - param_values.push(Value::Integer(note.created_at as i64)); + param_values.push(u64_to_value(note.created_at)); match note.consumed_block_height { Some(h) => param_values.push(Value::Integer(i64::from(h))), None => param_values.push(Value::Null), @@ -806,23 +820,3 @@ pub(super) fn upsert_note_script_tx( Ok(()) } - -/// Parse note script columns from the provided row into native types. -fn parse_note_scripts_columns( - row: &rusqlite::Row<'_>, -) -> Result { - // The script root can be derived from the script itself. - // There's no need to retrieve it separately. - // let script_root = row.get(0)?; - let script = row.get(1)?; - - Ok(SerializedNoteScriptPars { script }) -} - -/// Parse a note script from the provided parts. -fn parse_note_script( - serialized_note_script_parts: &SerializedNoteScriptPars, -) -> Result { - let note_script = NoteScript::from_bytes(&serialized_note_script_parts.script)?; - Ok(note_script) -} diff --git a/crates/sqlite-store/src/note/tests.rs b/crates/sqlite-store/src/note/tests.rs index fe5c223ae3..dae28a9893 100644 --- a/crates/sqlite-store/src/note/tests.rs +++ b/crates/sqlite-store/src/note/tests.rs @@ -518,3 +518,19 @@ async fn output_notes_never_match_script_root_filter() { .unwrap(); assert!(notes.is_empty()); } + +/// `get_note_script` returns the stored script when present and `NoteScriptNotFound` otherwise. +#[tokio::test] +async fn get_note_script_by_root() { + let store = create_test_store().await; + let script = StandardNote::SWAP.script(); + + store.upsert_note_scripts(std::slice::from_ref(&script)).await.unwrap(); + + let stored = store.get_note_script(script.root().into()).await.unwrap(); + assert_eq!(stored.root(), script.root()); + + let missing_root = Word::default(); + let err = store.get_note_script(missing_root).await.unwrap_err(); + assert!(matches!(err, miden_client::store::StoreError::NoteScriptNotFound(_))); +} diff --git a/crates/sqlite-store/src/sync.rs b/crates/sqlite-store/src/sync.rs index 6cd6e9548b..2409385b54 100644 --- a/crates/sqlite-store/src/sync.rs +++ b/crates/sqlite-store/src/sync.rs @@ -9,14 +9,14 @@ use miden_client::note::{BlockNumber, NoteTag}; use miden_client::store::StoreError; use miden_client::sync::{NoteTagRecord, NoteTagSource, PublicAccountUpdate, StateSyncUpdate}; use miden_client::utils::{Deserializable, Serializable}; -use rusqlite::{Connection, Transaction, TransactionBehavior, params}; +use rusqlite::{Connection, Transaction, params}; use super::SqliteStore; use crate::forest::{ScopedAccountForest, SqliteForestBackend}; use crate::note::apply_note_updates_tx; use crate::sql_error::SqlResultExt; use crate::transaction::upsert_transaction_record; -use crate::{insert_sql, subst}; +use crate::{insert_sql, subst, with_immediate_write_tx, with_write_tx}; impl SqliteStore { pub(crate) fn get_note_tags(conn: &mut Connection) -> Result, StoreError> { @@ -24,7 +24,7 @@ impl SqliteStore { conn.prepare_cached(QUERY) .into_store_error()? - .query_map([], |row| Ok((row.get(0)?, row.get(1)?))) + .query_map([], |row| Ok((row.get("tag")?, row.get("source")?))) .expect("no binding parameters used in query") .map(|result| { let (tag, source): (Vec, Vec) = result.into_store_error()?; @@ -58,24 +58,14 @@ impl SqliteStore { conn: &mut Connection, tag: NoteTagRecord, ) -> Result { - let tx = conn.transaction().into_store_error()?; - let inserted = add_note_tag_tx(&tx, &tag)?; - - tx.commit().into_store_error()?; - - Ok(inserted) + with_write_tx(conn, |tx| add_note_tag_tx(tx, &tag)) } pub(super) fn remove_note_tag( conn: &mut Connection, tag: NoteTagRecord, ) -> Result { - let tx = conn.transaction().into_store_error()?; - let removed_tags = remove_note_tag_tx(&tx, tag)?; - - tx.commit().into_store_error()?; - - Ok(removed_tags) + with_write_tx(conn, |tx| remove_note_tag_tx(tx, tag)) } pub(super) fn get_sync_height(conn: &mut Connection) -> Result { @@ -87,10 +77,12 @@ impl SqliteStore { .expect("no binding parameters used in query") .map(|result| { let v: i64 = result.into_store_error()?; - Ok(BlockNumber::from(u32::try_from(v).expect("block number is always positive"))) + Ok(BlockNumber::from(u32::try_from(v)?)) }) .next() - .expect("state sync block number exists") + .unwrap_or_else(|| { + Err(StoreError::QueryError("the blockchain checkpoint row is missing".to_string())) + }) } pub(super) fn apply_state_sync( @@ -105,11 +97,8 @@ impl SqliteStore { account_updates, ) = state_sync_update.into_parts(); - let db_tx = conn - .transaction_with_behavior(TransactionBehavior::Immediate) - .into_store_error()?; - { - let mut smt_forest = ScopedAccountForest::new(SqliteForestBackend::new(&db_tx))?; + with_immediate_write_tx(conn, |db_tx| { + let mut smt_forest = ScopedAccountForest::new(SqliteForestBackend::new(db_tx))?; // Update blockchain checkpoint (block number and peaks) only if moving forward. let new_peaks_bytes = partial_blockchain_updates.new_peaks.peaks().to_vec().to_bytes(); const BLOCKCHAIN_CHECKPOINT_QUERY: &str = "UPDATE blockchain_checkpoint SET block_num = ?, partial_blockchain_peaks = ? WHERE block_num < ?"; @@ -127,17 +116,17 @@ impl SqliteStore { for (block_header, is_relevant) in partial_blockchain_updates.block_headers_to_store(block_num) { - Self::insert_block_header_tx(&db_tx, block_header, *is_relevant)?; + Self::insert_block_header_tx(db_tx, block_header, *is_relevant)?; } // Insert new authentication nodes (inner nodes of the PartialBlockchain) Self::insert_partial_blockchain_nodes_tx( - &db_tx, + db_tx, partial_blockchain_updates.new_authentication_nodes(), )?; // Update notes - apply_note_updates_tx(&db_tx, ¬e_updates)?; + apply_note_updates_tx(db_tx, ¬e_updates)?; // Remove tags of input notes whose inclusion settled in this sync (committed, // consumed during catch-up, or invalidated): their tag no longer drives note sync. @@ -158,14 +147,14 @@ impl SqliteStore { .collect::>(); for tag in tags_to_remove { - remove_note_tag_tx(&db_tx, tag)?; + remove_note_tag_tx(db_tx, tag)?; } for transaction_record in transaction_updates .committed_transactions() .chain(transaction_updates.discarded_transactions()) { - upsert_transaction_record(&db_tx, transaction_record)?; + upsert_transaction_record(db_tx, transaction_record)?; } // Remove the accounts that are originated from the discarded transactions @@ -174,25 +163,26 @@ impl SqliteStore { .map(|tx| (tx.details.account_id, tx.details.final_account_state)) .collect(); - Self::undo_account_state(&db_tx, &mut smt_forest, &discarded_states)?; + Self::undo_account_state(db_tx, &mut smt_forest, &discarded_states)?; // Update public accounts on the db that have been updated onchain for update in account_updates.updated_public_accounts() { match update { PublicAccountUpdate::Full(account) => { - Self::update_account_state(&db_tx, &mut smt_forest, account)?; + Self::update_account_state(db_tx, &mut smt_forest, account)?; }, PublicAccountUpdate::Patch { new_header, patch } => { - Self::apply_sync_account_patch(&db_tx, &mut smt_forest, new_header, patch)?; + Self::apply_sync_account_patch(db_tx, &mut smt_forest, new_header, patch)?; }, } } for (account_id, digest) in account_updates.mismatched_private_accounts() { - Self::lock_account_on_unexpected_commitment(&db_tx, account_id, digest)?; + Self::lock_account_on_unexpected_commitment(db_tx, account_id, digest)?; } - } - db_tx.commit().into_store_error() + + Ok(()) + }) } } @@ -221,3 +211,23 @@ pub(super) fn remove_note_tag_tx( Ok(removed_tags) } + +#[cfg(test)] +mod tests { + use rusqlite::Connection; + + use crate::SqliteStore; + use crate::db_management::migrations::apply_migrations; + + /// A missing checkpoint row is only reachable through corruption (the initial migration seeds + /// it); it must surface as an error, not a panic. + #[test] + fn get_sync_height_errors_when_checkpoint_is_missing() { + let mut conn = Connection::open_in_memory().unwrap(); + apply_migrations(&mut conn).unwrap(); + conn.execute("DELETE FROM blockchain_checkpoint", []).unwrap(); + + let err = SqliteStore::get_sync_height(&mut conn).unwrap_err(); + assert!(matches!(err, miden_client::store::StoreError::QueryError(_))); + } +} diff --git a/crates/sqlite-store/src/transaction.rs b/crates/sqlite-store/src/transaction.rs index 13963cc1a3..8826e36480 100644 --- a/crates/sqlite-store/src/transaction.rs +++ b/crates/sqlite-store/src/transaction.rs @@ -1,6 +1,5 @@ #![allow(clippy::items_after_statements)] -use std::rc::Rc; use std::vec::Vec; use miden_client::Word; @@ -15,15 +14,14 @@ use miden_client::transaction::{ TransactionStoreUpdate, }; use miden_client::utils::{Deserializable as _, Serializable as _}; -use rusqlite::types::Value; -use rusqlite::{Connection, Transaction, TransactionBehavior, params}; +use rusqlite::{Connection, Transaction, params}; use super::SqliteStore; use super::note::apply_note_updates_tx; use super::sync::add_note_tag_tx; use crate::forest::{ScopedAccountForest, SqliteForestBackend}; use crate::sql_error::SqlResultExt; -use crate::{insert_sql, subst}; +use crate::{blob_array, insert_sql, subst, with_immediate_write_tx}; pub(crate) const UPSERT_TRANSACTION_QUERY: &str = insert_sql!( transactions { @@ -72,69 +70,52 @@ struct SerializedTransactionParts { impl SqliteStore { /// Retrieves tracked transactions, filtered by [`TransactionFilter`]. - pub fn get_transactions( + pub(crate) fn get_transactions( conn: &mut Connection, filter: &TransactionFilter, ) -> Result, StoreError> { - match filter { - TransactionFilter::Ids(ids) => { - let id_blobs = ids.iter().map(|id| Value::Blob(id.to_bytes())).collect::>(); - - // Create a prepared statement and bind the array parameter - conn.prepare(filter.to_query().as_ref()) - .into_store_error()? - .query_map(params![Rc::new(id_blobs)], parse_transaction_columns) - .into_store_error()? - .map(|result| Ok(result.into_store_error()?).and_then(parse_transaction)) - .collect::, _>>() - }, - _ => { - // For other filters, no parameters are needed - conn.prepare(filter.to_query().as_ref()) - .into_store_error()? - .query_map([], parse_transaction_columns) - .into_store_error()? - .map(|result| Ok(result.into_store_error()?).and_then(parse_transaction)) - .collect::, _>>() - }, - } + // Only the `Ids` filter binds a parameter (the id list, as a single rarray value). + let id_list = match filter { + TransactionFilter::Ids(ids) => Some(blob_array(ids)), + _ => None, + }; + + conn.prepare(filter.to_query().as_ref()) + .into_store_error()? + .query_map(rusqlite::params_from_iter(id_list), parse_transaction_columns) + .into_store_error()? + .map(|result| Ok(result.into_store_error()?).and_then(parse_transaction)) + .collect::, _>>() } /// Inserts a transaction and updates the current state based on the `tx_result` changes. /// /// SQL writes and forest mutations go through the same rusqlite transaction, so they commit /// or roll back atomically. - pub fn apply_transaction( + pub(crate) fn apply_transaction( conn: &mut Connection, tx_update: &TransactionStoreUpdate, ) -> Result<(), StoreError> { - let db_tx = conn - .transaction_with_behavior(TransactionBehavior::Immediate) - .into_store_error()?; - { - let mut forest = ScopedAccountForest::new(SqliteForestBackend::new(&db_tx))?; - Self::apply_transaction_in_txn(&db_tx, &mut forest, tx_update)?; - } - db_tx.commit().into_store_error() + with_immediate_write_tx(conn, |tx| { + let mut forest = ScopedAccountForest::new(SqliteForestBackend::new(tx))?; + Self::apply_transaction_in_txn(tx, &mut forest, tx_update) + }) } /// Applies a batch of [`TransactionStoreUpdate`]s atomically. Either every update in the /// slice is persisted or none are. Executes in order inside a single /// [`rusqlite::Transaction`]. - pub fn apply_transaction_batch( + pub(crate) fn apply_transaction_batch( conn: &mut Connection, tx_updates: &[TransactionStoreUpdate], ) -> Result<(), StoreError> { - let db_tx = conn - .transaction_with_behavior(TransactionBehavior::Immediate) - .into_store_error()?; - { - let mut forest = ScopedAccountForest::new(SqliteForestBackend::new(&db_tx))?; + with_immediate_write_tx(conn, |tx| { + let mut forest = ScopedAccountForest::new(SqliteForestBackend::new(tx))?; for update in tx_updates { - Self::apply_transaction_in_txn(&db_tx, &mut forest, update)?; + Self::apply_transaction_in_txn(tx, &mut forest, update)?; } - } - db_tx.commit().into_store_error() + Ok(()) + }) } /// Applies a transaction's store update within the provided rusqlite transaction. @@ -252,10 +233,10 @@ fn serialize_transaction_data(transaction_record: &TransactionRecord) -> Seriali fn parse_transaction_columns( row: &rusqlite::Row<'_>, ) -> Result { - let id: Vec = row.get(0)?; - let tx_script: Option> = row.get(1)?; - let details: Vec = row.get(2)?; - let status: Vec = row.get(3)?; + let id: Vec = row.get("id")?; + let tx_script: Option> = row.get("script")?; + let details: Vec = row.get("details")?; + let status: Vec = row.get("status")?; Ok(SerializedTransactionParts { id, tx_script, details, status }) }