From dfafbc4b68b37cc2e9ba2f8866057f5b35dc3576 Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Thu, 27 Feb 2025 19:56:34 -0300 Subject: [PATCH 1/4] feat: support account rollbacks --- crates/rust-client/src/store/mod.rs | 2 + .../src/store/sqlite_store/account.rs | 11 ++++++ .../src/store/sqlite_store/sync.rs | 37 ++++++++++++++++++- .../src/store/sqlite_store/transaction.rs | 9 +++++ 4 files changed, 58 insertions(+), 1 deletion(-) diff --git a/crates/rust-client/src/store/mod.rs b/crates/rust-client/src/store/mod.rs index 484b6f37e..aeaac6e6e 100644 --- a/crates/rust-client/src/store/mod.rs +++ b/crates/rust-client/src/store/mod.rs @@ -345,6 +345,8 @@ pub enum TransactionFilter { /// Filter by transactions that haven't yet been committed to the blockchain as per the last /// sync. Uncomitted, + /// Return a list of the transaction that matches the provided [`TransactionId`]s. + Ids(Vec), } // NOTE FILTER diff --git a/crates/rust-client/src/store/sqlite_store/account.rs b/crates/rust-client/src/store/sqlite_store/account.rs index a09d784e8..85804fd7e 100644 --- a/crates/rust-client/src/store/sqlite_store/account.rs +++ b/crates/rust-client/src/store/sqlite_store/account.rs @@ -207,6 +207,17 @@ impl SqliteStore { }) .collect::, _>>() } + + pub fn delete_accounts( + tx: &Transaction<'_>, + account_ids: &[AccountId], + ) -> Result<(), StoreError> { + const QUERY: &str = "DELETE FROM accounts WHERE id = ?"; + for account_id in account_ids { + tx.execute(QUERY, params![account_id.to_hex()])?; + } + Ok(()) + } } // HELPERS diff --git a/crates/rust-client/src/store/sqlite_store/sync.rs b/crates/rust-client/src/store/sqlite_store/sync.rs index 129b860a2..20077f4f0 100644 --- a/crates/rust-client/src/store/sqlite_store/sync.rs +++ b/crates/rust-client/src/store/sqlite_store/sync.rs @@ -14,7 +14,7 @@ use crate::{ account::{lock_account, update_account}, note::apply_note_updates_tx, }, - StoreError, + StoreError, TransactionFilter, }, sync::{NoteTagRecord, NoteTagSource, StateSyncUpdate}, }; @@ -139,6 +139,41 @@ impl SqliteStore { // Marc transactions as discarded Self::mark_transactions_as_discarded(&tx, &discarded_transactions)?; + // TODO: here we need to remove the `accounts` table entries that are originated from the + // discarded transactions + + // First we need the `transaction` entries from the `transactions` table that matches the + // `transactions_to_discard` + let transactions_to_discard = + Self::get_transactions(conn, &TransactionFilter::Ids(discarded_transactions.clone()))?; + + let outdated_accounts = transactions_to_discard + .iter() + .map(|transaction_record| { + Self::get_account(conn, transaction_record.account_id).unwrap().unwrap() + }) + .collect::>(); + + // Transaction records have a final_account_state field, which is the hash of the account in + // the final state after the transaction is applied. We can use this field to + // identify the accounts that are originated from the discarded transactions. + + let accounts_to_remove = transactions_to_discard + .iter() + .map(|tx| { + let final_account_state = tx.final_account_state; + + let account = outdated_accounts + .iter() + .find(|account| account.account().hash() == final_account_state)?; + + account.account().id() + }) + .collect::>(); + + // Remove the accounts that are originated from the discarded transactions + Self::delete_accounts(&tx, &accounts_to_remove)?; + // Update public accounts on the db that have been updated onchain for account in updated_accounts.updated_public_accounts() { update_account(&tx, account)?; diff --git a/crates/rust-client/src/store/sqlite_store/transaction.rs b/crates/rust-client/src/store/sqlite_store/transaction.rs index 5fdcff601..e794ac8cf 100644 --- a/crates/rust-client/src/store/sqlite_store/transaction.rs +++ b/crates/rust-client/src/store/sqlite_store/transaction.rs @@ -10,6 +10,7 @@ use miden_objects::{ account::AccountId, block::BlockNumber, crypto::utils::{Deserializable, Serializable}, + testing::account_id, transaction::{ ExecutedTransaction, OutputNotes, ToInputNoteCommitments, TransactionId, TransactionScript, }, @@ -49,6 +50,14 @@ impl TransactionFilter { match self { TransactionFilter::All => QUERY.to_string(), TransactionFilter::Uncomitted => format!("{QUERY} WHERE tx.commit_height IS NULL"), + TransactionFilter::Ids(ids) => { + let ids = ids + .iter() + .map(|id| format!("'{}'", id.to_string())) + .collect::>() + .join(", "); + format!("{QUERY} WHERE tx.id IN ({})", ids) + }, } } } From 05782f006744dbedc98593ba9e8ac82e0b859822 Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Fri, 28 Feb 2025 15:19:06 -0300 Subject: [PATCH 2/4] wip: fix deletion --- .../src/store/sqlite_store/sync.rs | 32 +++++++++++-------- .../src/store/sqlite_store/transaction.rs | 1 - 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/crates/rust-client/src/store/sqlite_store/sync.rs b/crates/rust-client/src/store/sqlite_store/sync.rs index 20077f4f0..5c07d180c 100644 --- a/crates/rust-client/src/store/sqlite_store/sync.rs +++ b/crates/rust-client/src/store/sqlite_store/sync.rs @@ -2,7 +2,9 @@ use alloc::{collections::BTreeSet, vec::Vec}; -use miden_objects::{block::BlockNumber, note::NoteTag, transaction::TransactionId}; +use miden_objects::{ + account::AccountId, block::BlockNumber, note::NoteTag, transaction::TransactionId, +}; use miden_tx::utils::{Deserializable, Serializable}; use rusqlite::{params, Connection, Transaction}; @@ -114,6 +116,19 @@ impl SqliteStore { tags_to_remove, } = state_sync_update; + // First we need the `transaction` entries from the `transactions` table that matches the + // `transactions_to_discard` + + let transactions_to_discard = + Self::get_transactions(conn, &TransactionFilter::Ids(discarded_transactions.clone()))?; + + let outdated_accounts = transactions_to_discard + .iter() + .map(|transaction_record| { + Self::get_account(conn, transaction_record.account_id).unwrap().unwrap() + }) + .collect::>(); + let tx = conn.transaction()?; // Update state sync block number @@ -142,18 +157,6 @@ impl SqliteStore { // TODO: here we need to remove the `accounts` table entries that are originated from the // discarded transactions - // First we need the `transaction` entries from the `transactions` table that matches the - // `transactions_to_discard` - let transactions_to_discard = - Self::get_transactions(conn, &TransactionFilter::Ids(discarded_transactions.clone()))?; - - let outdated_accounts = transactions_to_discard - .iter() - .map(|transaction_record| { - Self::get_account(conn, transaction_record.account_id).unwrap().unwrap() - }) - .collect::>(); - // Transaction records have a final_account_state field, which is the hash of the account in // the final state after the transaction is applied. We can use this field to // identify the accounts that are originated from the discarded transactions. @@ -165,7 +168,8 @@ impl SqliteStore { let account = outdated_accounts .iter() - .find(|account| account.account().hash() == final_account_state)?; + .find(|account| account.account().hash() == final_account_state) + .unwrap(); account.account().id() }) diff --git a/crates/rust-client/src/store/sqlite_store/transaction.rs b/crates/rust-client/src/store/sqlite_store/transaction.rs index e794ac8cf..77304decc 100644 --- a/crates/rust-client/src/store/sqlite_store/transaction.rs +++ b/crates/rust-client/src/store/sqlite_store/transaction.rs @@ -10,7 +10,6 @@ use miden_objects::{ account::AccountId, block::BlockNumber, crypto::utils::{Deserializable, Serializable}, - testing::account_id, transaction::{ ExecutedTransaction, OutputNotes, ToInputNoteCommitments, TransactionId, TransactionScript, }, From 80b35ecba7f404f3f1606cc79054bea6105aa295 Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Fri, 28 Feb 2025 16:25:00 -0300 Subject: [PATCH 3/4] wip: move discarded_tx changes to apply_nullifiers --- crates/rust-client/src/store/account.rs | 2 + .../src/store/sqlite_store/sync.rs | 85 +++++++++++-------- 2 files changed, 50 insertions(+), 37 deletions(-) diff --git a/crates/rust-client/src/store/account.rs b/crates/rust-client/src/store/account.rs index 9bfd4a330..bdabd21e5 100644 --- a/crates/rust-client/src/store/account.rs +++ b/crates/rust-client/src/store/account.rs @@ -13,6 +13,7 @@ use miden_objects::{ /// The account should be stored in the database with its parts normalized. Meaning that the /// account header, vault, storage and code are stored separately. This is done to avoid data /// duplication as the header can reference the same elements if they have equal roots. +#[derive(Debug)] pub struct AccountRecord { /// Full account object. account: Account, @@ -54,6 +55,7 @@ impl From for Account { /// Represents the status of an account tracked by the client. /// /// The status of an account may change by local or external factors. +#[derive(Debug)] pub enum AccountStatus { /// The account is new and hasn't been used yet. The seed used to create the account is /// stored in this state. diff --git a/crates/rust-client/src/store/sqlite_store/sync.rs b/crates/rust-client/src/store/sqlite_store/sync.rs index 5c07d180c..5a9f13797 100644 --- a/crates/rust-client/src/store/sqlite_store/sync.rs +++ b/crates/rust-client/src/store/sqlite_store/sync.rs @@ -7,6 +7,7 @@ use miden_objects::{ }; use miden_tx::utils::{Deserializable, Serializable}; use rusqlite::{params, Connection, Transaction}; +use tracing::info; use super::SqliteStore; use crate::{ @@ -116,19 +117,6 @@ impl SqliteStore { tags_to_remove, } = state_sync_update; - // First we need the `transaction` entries from the `transactions` table that matches the - // `transactions_to_discard` - - let transactions_to_discard = - Self::get_transactions(conn, &TransactionFilter::Ids(discarded_transactions.clone()))?; - - let outdated_accounts = transactions_to_discard - .iter() - .map(|transaction_record| { - Self::get_account(conn, transaction_record.account_id).unwrap().unwrap() - }) - .collect::>(); - let tx = conn.transaction()?; // Update state sync block number @@ -154,30 +142,6 @@ impl SqliteStore { // Marc transactions as discarded Self::mark_transactions_as_discarded(&tx, &discarded_transactions)?; - // TODO: here we need to remove the `accounts` table entries that are originated from the - // discarded transactions - - // Transaction records have a final_account_state field, which is the hash of the account in - // the final state after the transaction is applied. We can use this field to - // identify the accounts that are originated from the discarded transactions. - - let accounts_to_remove = transactions_to_discard - .iter() - .map(|tx| { - let final_account_state = tx.final_account_state; - - let account = outdated_accounts - .iter() - .find(|account| account.account().hash() == final_account_state) - .unwrap(); - - account.account().id() - }) - .collect::>(); - - // Remove the accounts that are originated from the discarded transactions - Self::delete_accounts(&tx, &accounts_to_remove)?; - // Update public accounts on the db that have been updated onchain for account in updated_accounts.updated_public_accounts() { update_account(&tx, account)?; @@ -198,12 +162,59 @@ impl SqliteStore { note_updates: &NoteUpdates, transactions_to_discard: &[TransactionId], ) -> Result<(), StoreError> { + info!("APPLY NULLIFIERS"); + // First we need the `transaction` entries from the `transactions` table that matches the + // `transactions_to_discard` + + info!("discarded_transactions: {:?}", transactions_to_discard); + let transactions_records_to_discard = Self::get_transactions( + conn, + &TransactionFilter::Ids(transactions_to_discard.to_vec()), + )?; + + info!("transactions_to_discard: {:?}", transactions_to_discard); + let outdated_accounts = transactions_records_to_discard + .iter() + .map(|transaction_record| { + Self::get_account(conn, transaction_record.account_id).unwrap().unwrap() + }) + .collect::>(); + + info!("outdated_accounts: {:?}", outdated_accounts); + let tx = conn.transaction()?; apply_note_updates_tx(&tx, note_updates)?; Self::mark_transactions_as_discarded(&tx, transactions_to_discard)?; + // TODO: here we need to remove the `accounts` table entries that are originated from the + // discarded transactions + + // Transaction records have a final_account_state field, which is the hash of the account in + // the final state after the transaction is applied. We can use this field to + // identify the accounts that are originated from the discarded transactions. + + let accounts_to_remove = transactions_records_to_discard + .iter() + .map(|tx| { + let final_account_state = tx.final_account_state; + info!("final_account_state: {:?}", final_account_state); + let account = outdated_accounts + .iter() + .find(|account| account.account().hash() == final_account_state) + .unwrap(); + + info!("account: {:?}", account); + account.account().id() + }) + .collect::>(); + + info!("accounts_to_remove: {:?}", accounts_to_remove); + + // Remove the accounts that are originated from the discarded transactions + Self::delete_accounts(&tx, &accounts_to_remove)?; + tx.commit()?; Ok(()) From a4bdec9be2f95d6f9cd2efeaba08ad4a36f75b85 Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Wed, 5 Mar 2025 16:24:11 -0300 Subject: [PATCH 4/4] fix: remove unwraps, add missing code --- .../src/store/sqlite_store/account.rs | 6 +-- .../src/store/sqlite_store/sync.rs | 54 ++++++++----------- .../src/store/sqlite_store/transaction.rs | 8 +-- .../src/store/web_store/js/transactions.js | 14 ++++- .../src/store/web_store/transaction/mod.rs | 10 +++- 5 files changed, 50 insertions(+), 42 deletions(-) diff --git a/crates/rust-client/src/store/sqlite_store/account.rs b/crates/rust-client/src/store/sqlite_store/account.rs index 85804fd7e..937cf51ea 100644 --- a/crates/rust-client/src/store/sqlite_store/account.rs +++ b/crates/rust-client/src/store/sqlite_store/account.rs @@ -210,10 +210,10 @@ impl SqliteStore { pub fn delete_accounts( tx: &Transaction<'_>, - account_ids: &[AccountId], + account_hashes: &[Digest], ) -> Result<(), StoreError> { - const QUERY: &str = "DELETE FROM accounts WHERE id = ?"; - for account_id in account_ids { + const QUERY: &str = "DELETE FROM accounts WHERE account_hash = ?"; + for account_id in account_hashes { tx.execute(QUERY, params![account_id.to_hex()])?; } Ok(()) diff --git a/crates/rust-client/src/store/sqlite_store/sync.rs b/crates/rust-client/src/store/sqlite_store/sync.rs index 5a9f13797..2b25be180 100644 --- a/crates/rust-client/src/store/sqlite_store/sync.rs +++ b/crates/rust-client/src/store/sqlite_store/sync.rs @@ -2,12 +2,9 @@ use alloc::{collections::BTreeSet, vec::Vec}; -use miden_objects::{ - account::AccountId, block::BlockNumber, note::NoteTag, transaction::TransactionId, -}; +use miden_objects::{block::BlockNumber, note::NoteTag, transaction::TransactionId}; use miden_tx::utils::{Deserializable, Serializable}; use rusqlite::{params, Connection, Transaction}; -use tracing::info; use super::SqliteStore; use crate::{ @@ -162,25 +159,22 @@ impl SqliteStore { note_updates: &NoteUpdates, transactions_to_discard: &[TransactionId], ) -> Result<(), StoreError> { - info!("APPLY NULLIFIERS"); // First we need the `transaction` entries from the `transactions` table that matches the // `transactions_to_discard` - info!("discarded_transactions: {:?}", transactions_to_discard); let transactions_records_to_discard = Self::get_transactions( conn, &TransactionFilter::Ids(transactions_to_discard.to_vec()), )?; - info!("transactions_to_discard: {:?}", transactions_to_discard); - let outdated_accounts = transactions_records_to_discard - .iter() - .map(|transaction_record| { - Self::get_account(conn, transaction_record.account_id).unwrap().unwrap() - }) - .collect::>(); - - info!("outdated_accounts: {:?}", outdated_accounts); + // Get the outdated accounts, handling potential errors + let mut outdated_accounts = Vec::new(); + for transaction_record in &transactions_records_to_discard { + let account = Self::get_account(conn, transaction_record.account_id) + .map_err(|err| StoreError::QueryError(format!("Failed to get account: {err}")))? + .ok_or_else(|| StoreError::AccountDataNotFound(transaction_record.account_id))?; + outdated_accounts.push(account); + } let tx = conn.transaction()?; @@ -195,22 +189,20 @@ impl SqliteStore { // the final state after the transaction is applied. We can use this field to // identify the accounts that are originated from the discarded transactions. - let accounts_to_remove = transactions_records_to_discard - .iter() - .map(|tx| { - let final_account_state = tx.final_account_state; - info!("final_account_state: {:?}", final_account_state); - let account = outdated_accounts - .iter() - .find(|account| account.account().hash() == final_account_state) - .unwrap(); - - info!("account: {:?}", account); - account.account().id() - }) - .collect::>(); - - info!("accounts_to_remove: {:?}", accounts_to_remove); + let mut accounts_to_remove = Vec::new(); + for tx_record in &transactions_records_to_discard { + let final_account_state = tx_record.final_account_state; + let account = outdated_accounts + .iter() + .find(|account| account.account().hash() == final_account_state) + .ok_or_else(|| { + StoreError::QueryError(format!( + "Could not find account with hash {final_account_state}" + )) + })?; + + accounts_to_remove.push(account.account().hash()); + } // Remove the accounts that are originated from the discarded transactions Self::delete_accounts(&tx, &accounts_to_remove)?; diff --git a/crates/rust-client/src/store/sqlite_store/transaction.rs b/crates/rust-client/src/store/sqlite_store/transaction.rs index 77304decc..c7410fa6d 100644 --- a/crates/rust-client/src/store/sqlite_store/transaction.rs +++ b/crates/rust-client/src/store/sqlite_store/transaction.rs @@ -50,12 +50,8 @@ impl TransactionFilter { TransactionFilter::All => QUERY.to_string(), TransactionFilter::Uncomitted => format!("{QUERY} WHERE tx.commit_height IS NULL"), TransactionFilter::Ids(ids) => { - let ids = ids - .iter() - .map(|id| format!("'{}'", id.to_string())) - .collect::>() - .join(", "); - format!("{QUERY} WHERE tx.id IN ({})", ids) + let ids = ids.iter().map(|id| format!("'{id}'")).collect::>().join(", "); + format!("{QUERY} WHERE tx.id IN ({ids})") }, } } diff --git a/crates/rust-client/src/store/web_store/js/transactions.js b/crates/rust-client/src/store/web_store/js/transactions.js index e13dcf7b2..11bfc0626 100644 --- a/crates/rust-client/src/store/web_store/js/transactions.js +++ b/crates/rust-client/src/store/web_store/js/transactions.js @@ -10,6 +10,18 @@ export async function getTransactions(filter) { (tx) => tx.commitHeight === undefined || tx.commitHeight === null ) .toArray(); + } else if (filter.startsWith("Ids:")) { + const idsString = filter.substring(4); // Remove "Ids:" prefix + const ids = idsString.split(","); + + if (ids.length > 0) { + transactionRecords = await transactions + .where("id") + .anyOf(ids) + .toArray(); + } else { + transactionRecords = []; + } } else { transactionRecords = await transactions.toArray(); } @@ -81,7 +93,7 @@ export async function getTransactions(filter) { ); return processedTransactions; - } catch { + } catch (err) { console.error("Failed to get transactions: ", err); throw err; } diff --git a/crates/rust-client/src/store/web_store/transaction/mod.rs b/crates/rust-client/src/store/web_store/transaction/mod.rs index 1cfa82c19..587c8c578 100644 --- a/crates/rust-client/src/store/web_store/transaction/mod.rs +++ b/crates/rust-client/src/store/web_store/transaction/mod.rs @@ -1,4 +1,7 @@ -use alloc::{string::ToString, vec::Vec}; +use alloc::{ + string::{String, ToString}, + vec::Vec, +}; use miden_objects::{ account::AccountId, @@ -33,6 +36,11 @@ impl WebStore { let filter_as_str = match filter { TransactionFilter::All => "All", TransactionFilter::Uncomitted => "Uncomitted", + TransactionFilter::Ids(ids) => &{ + let ids_str = + ids.iter().map(ToString::to_string).collect::>().join(","); + format!("Ids:{ids_str}") + }, }; let promise = idxdb_get_transactions(filter_as_str.to_string());