diff --git a/dash-spv/src/client/lifecycle.rs b/dash-spv/src/client/lifecycle.rs index 739ca8a43..105b2680d 100644 --- a/dash-spv/src/client/lifecycle.rs +++ b/dash-spv/src/client/lifecycle.rs @@ -128,7 +128,7 @@ impl // Create mempool state and build mempool manager if tracking is enabled let mempool_state = Arc::new(RwLock::new(MempoolState::default())); if config.enable_mempool_tracking { - let initial_revision = wallet.read().await.monitor_revision(); + let initial_revision = wallet.read().await.monitor_revision().await; managers.mempool = Some(MempoolManager::new( wallet.clone(), mempool_state.clone(), diff --git a/dash-spv/src/sync/filters/manager.rs b/dash-spv/src/sync/filters/manager.rs index 59aa0e344..40ef0f995 100644 --- a/dash-spv/src/sync/filters/manager.rs +++ b/dash-spv/src/sync/filters/manager.rs @@ -500,7 +500,7 @@ impl self.progress.committed_height() { self.progress.update_committed_height(end); - self.wallet.write().await.update_filter_committed_height(end); + self.wallet.write().await.update_filter_committed_height(end).await; } self.processing_height = end + 1; @@ -675,7 +675,7 @@ impl MempoolManager { requests: &RequestSender, ) -> SyncResult<()> { let wallet = self.wallet.read().await; - let addresses = wallet.monitored_addresses(); - let outpoints = wallet.watched_outpoints(); + let addresses = wallet.monitored_addresses().await; + let outpoints = wallet.watched_outpoints().await; drop(wallet); if addresses.is_empty() && outpoints.is_empty() { @@ -379,7 +379,7 @@ impl MempoolManager { drop(state); if marked { let mut wallet = self.wallet.write().await; - wallet.process_instant_send_lock(*txid); + wallet.process_instant_send_lock(*txid).await; } } diff --git a/dash-spv/src/sync/mempool/sync_manager.rs b/dash-spv/src/sync/mempool/sync_manager.rs index 3292ae770..6109d2af0 100644 --- a/dash-spv/src/sync/mempool/sync_manager.rs +++ b/dash-spv/src/sync/mempool/sync_manager.rs @@ -127,7 +127,7 @@ impl SyncManager for MempoolManager { // shared `SyncManager::run` loop or a `WalletEvent` bridge — complexity // that isn't justified given the 100ms tick latency is negligible for bloom // filter rebuilds and the read lock is non-contending. - let current_revision = self.wallet.read().await.monitor_revision(); + let current_revision = self.wallet.read().await.monitor_revision().await; if current_revision != self.last_monitor_revision { tracing::info!("Wallet monitor revision changed, rebuilding bloom filter"); self.rebuild_filter(requests).await?; @@ -621,7 +621,7 @@ mod tests { let mut mock = MockWallet::new(); mock.set_addresses(vec![addr.clone()]); - let initial_revision = mock.monitor_revision(); + let initial_revision = mock.monitor_revision().await; let wallet = Arc::new(RwLock::new(mock)); let mempool_state = Arc::new(RwLock::new(MempoolState::default())); let (tx, mut rx) = mpsc::unbounded_channel::(); @@ -734,7 +734,7 @@ mod tests { let mut mock = MockWallet::new(); mock.set_addresses(vec![addr]); - let initial_revision = mock.monitor_revision(); + let initial_revision = mock.monitor_revision().await; let wallet = Arc::new(RwLock::new(mock)); let mempool_state = Arc::new(RwLock::new(MempoolState::default())); let (tx, mut rx) = mpsc::unbounded_channel::(); @@ -785,7 +785,7 @@ mod tests { ]); let addr = dashcore::Address::from_script(&script, dashcore::Network::Testnet).unwrap(); mock.set_addresses(vec![addr]); - let initial_revision = mock.monitor_revision(); + let initial_revision = mock.monitor_revision().await; let wallet = Arc::new(RwLock::new(mock)); let mempool_state = Arc::new(RwLock::new(MempoolState::default())); let (tx_chan, mut rx) = mpsc::unbounded_channel::(); diff --git a/key-wallet-ffi/src/address_pool.rs b/key-wallet-ffi/src/address_pool.rs index ef222a85e..1e697d206 100644 --- a/key-wallet-ffi/src/address_pool.rs +++ b/key-wallet-ffi/src/address_pool.rs @@ -696,14 +696,14 @@ pub unsafe extern "C" fn managed_wallet_mark_address_used( let mut found = false; // Check all accounts for the address for account in collection.standard_bip44_accounts.values_mut() { - if account.mark_address_used(&address) { + if account.mark_address_used(&address).0 { found = true; break; } } if !found { for account in collection.standard_bip32_accounts.values_mut() { - if account.mark_address_used(&address) { + if account.mark_address_used(&address).0 { found = true; break; } @@ -711,7 +711,7 @@ pub unsafe extern "C" fn managed_wallet_mark_address_used( } if !found { for account in collection.coinjoin_accounts.values_mut() { - if account.mark_address_used(&address) { + if account.mark_address_used(&address).0 { found = true; break; } @@ -719,14 +719,14 @@ pub unsafe extern "C" fn managed_wallet_mark_address_used( } if !found { if let Some(account) = &mut collection.identity_registration { - if account.mark_address_used(&address) { + if account.mark_address_used(&address).0 { found = true; } } } if !found { for account in collection.identity_topup.values_mut() { - if account.mark_address_used(&address) { + if account.mark_address_used(&address).0 { found = true; break; } @@ -734,63 +734,63 @@ pub unsafe extern "C" fn managed_wallet_mark_address_used( } if !found { if let Some(account) = &mut collection.identity_topup_not_bound { - if account.mark_address_used(&address) { + if account.mark_address_used(&address).0 { found = true; } } } if !found { if let Some(account) = &mut collection.identity_invitation { - if account.mark_address_used(&address) { + if account.mark_address_used(&address).0 { found = true; } } } if !found { if let Some(account) = &mut collection.asset_lock_address_topup { - if account.mark_address_used(&address) { + if account.mark_address_used(&address).0 { found = true; } } } if !found { if let Some(account) = &mut collection.asset_lock_shielded_address_topup { - if account.mark_address_used(&address) { + if account.mark_address_used(&address).0 { found = true; } } } if !found { if let Some(account) = &mut collection.provider_voting_keys { - if account.mark_address_used(&address) { + if account.mark_address_used(&address).0 { found = true; } } } if !found { if let Some(account) = &mut collection.provider_owner_keys { - if account.mark_address_used(&address) { + if account.mark_address_used(&address).0 { found = true; } } } if !found { if let Some(account) = &mut collection.provider_operator_keys { - if account.mark_address_used(&address) { + if account.mark_address_used(&address).0 { found = true; } } } if !found { if let Some(account) = &mut collection.provider_platform_keys { - if account.mark_address_used(&address) { + if account.mark_address_used(&address).0 { found = true; } } } if !found { for account in collection.dashpay_receival_accounts.values_mut() { - if account.mark_address_used(&address) { + if account.mark_address_used(&address).0 { found = true; break; } @@ -798,7 +798,7 @@ pub unsafe extern "C" fn managed_wallet_mark_address_used( } if !found { for account in collection.dashpay_external_accounts.values_mut() { - if account.mark_address_used(&address) { + if account.mark_address_used(&address).0 { found = true; break; } diff --git a/key-wallet-ffi/src/wallet_manager_tests.rs b/key-wallet-ffi/src/wallet_manager_tests.rs index 2acaa98d0..c2229a847 100644 --- a/key-wallet-ffi/src/wallet_manager_tests.rs +++ b/key-wallet-ffi/src/wallet_manager_tests.rs @@ -459,7 +459,7 @@ mod tests { let manager_ref = &*manager; manager_ref.runtime.block_on(async { let mut manager_guard = manager_ref.manager.write().await; - manager_guard.update_synced_height(new_height); + manager_guard.update_synced_height(new_height).await; }); } diff --git a/key-wallet-manager/Cargo.toml b/key-wallet-manager/Cargo.toml index 46e201200..1b7347c05 100644 --- a/key-wallet-manager/Cargo.toml +++ b/key-wallet-manager/Cargo.toml @@ -12,12 +12,15 @@ default = [] parallel-filters = ["dep:rayon"] bincode = ["key-wallet/bincode", "dep:bincode"] test-utils = ["key-wallet/test-utils"] +bls = ["key-wallet/bls"] +eddsa = ["key-wallet/eddsa"] [dependencies] key-wallet = { path = "../key-wallet", default-features = false } dashcore = { path = "../dash" } async-trait = "0.1" tokio = { version = "1", features = ["macros", "rt", "sync"] } +tracing = "0.1" zeroize = { version = "1.8", features = ["derive"] } rayon = { version = "1.11", optional = true } bincode = { version = "2.0.1", optional = true } diff --git a/key-wallet-manager/examples/wallet_creation.rs b/key-wallet-manager/examples/wallet_creation.rs index 4529f5bb7..7668ab651 100644 --- a/key-wallet-manager/examples/wallet_creation.rs +++ b/key-wallet-manager/examples/wallet_creation.rs @@ -8,30 +8,29 @@ use key_wallet::account::StandardAccountType; use key_wallet::wallet::initialization::WalletAccountCreationOptions; use key_wallet::wallet::managed_wallet_info::transaction_building::AccountTypePreference; -use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; use key_wallet::{AccountType, Network}; -use key_wallet_manager::WalletInterface; -use key_wallet_manager::WalletManager; +use key_wallet_manager::{ManagedWalletState, WalletInterface, WalletManager}; -fn main() { +#[tokio::main] +async fn main() { println!("=== Wallet Creation Example ===\n"); // Example 1: Basic wallet creation with WalletManager println!("1. Creating a basic wallet with WalletManager..."); - let mut manager = WalletManager::::new(Network::Testnet); + let mut manager = WalletManager::::new(Network::Testnet); let result = manager.create_wallet_with_random_mnemonic(WalletAccountCreationOptions::Default); let wallet_id = match result { Ok(wallet_id) => { - println!("✅ Wallet created successfully!"); + println!("Wallet created successfully!"); println!(" Wallet ID: {}", hex::encode(wallet_id)); println!(" Total wallets: {}", manager.wallet_count()); wallet_id } Err(e) => { - println!("❌ Failed to create wallet: {:?}", e); + println!("Failed to create wallet: {:?}", e); return; } }; @@ -50,12 +49,12 @@ fn main() { let wallet_id2 = match result { Ok(wallet_id2) => { - println!("✅ Wallet created from mnemonic!"); + println!("Wallet created from mnemonic!"); println!(" Wallet ID: {}", hex::encode(wallet_id2)); wallet_id2 } Err(e) => { - println!("❌ Failed to create wallet from mnemonic: {:?}", e); + println!("Failed to create wallet from mnemonic: {:?}", e); return; } }; @@ -64,26 +63,28 @@ fn main() { println!("\n3. Managing wallet accounts..."); // Add a new account to the first wallet - let account_result = manager.create_account( - &wallet_id, // Account index 1 (0 is created by default) - AccountType::Standard { - index: 1, - standard_account_type: StandardAccountType::BIP44Account, - }, - None, - ); + let account_result = manager + .create_account( + &wallet_id, // Account index 1 (0 is created by default) + AccountType::Standard { + index: 1, + standard_account_type: StandardAccountType::BIP44Account, + }, + None, + ) + .await; match account_result { Ok(_) => { - println!("✅ Account created successfully!"); + println!("Account created successfully!"); // Get all accounts - if let Ok(accounts) = manager.get_accounts(&wallet_id) { + if let Ok(accounts) = manager.get_accounts(&wallet_id).await { println!(" Total accounts: {}", accounts.len()); } } Err(e) => { - println!("❌ Failed to create account: {:?}", e); + println!("Failed to create account: {:?}", e); } } @@ -92,26 +93,28 @@ fn main() { // Note: This might fail with InvalidNetwork error if the account collection // isn't properly initialized in the managed wallet info - let address_result = manager.get_receive_address( - &wallet_id, - 0, // Account index - AccountTypePreference::BIP44, - false, // Don't advance index - ); + let address_result = manager + .get_receive_address( + &wallet_id, + 0, // Account index + AccountTypePreference::BIP44, + false, // Don't advance index + ) + .await; match address_result { Ok(result) => { if let Some(address) = result.address { - println!("✅ Receive address: {}", address); + println!("Receive address: {}", address); if let Some(account_type) = result.account_type_used { println!(" Account type used: {:?}", account_type); } } else { - println!("⚠️ No address generated"); + println!("No address generated"); } } Err(e) => { - println!("⚠️ Could not get address: {:?}", e); + println!("Could not get address: {:?}", e); println!(" (This is expected with the current implementation)"); } } @@ -125,8 +128,8 @@ fn main() { // Example 6: Getting wallet balance println!("\n6. Checking wallet balances..."); - for (i, wallet_id) in [wallet_id, wallet_id2].iter().enumerate() { - match manager.get_wallet_balance(wallet_id) { + for (i, wid) in [wallet_id, wallet_id2].iter().enumerate() { + match manager.get_wallet_balance(wid).await { Ok(balance) => { println!(" Wallet {}: {} satoshis", i + 1, balance.total()); } @@ -142,13 +145,13 @@ fn main() { // Example 7: Block height tracking println!("\n7. Block height tracking..."); - println!(" Current height (Testnet): {:?}", manager.synced_height()); + println!(" Current height (Testnet): {:?}", WalletInterface::synced_height(&manager)); // Update height - manager.update_synced_height(850_000); - println!(" Updated height to: {:?}", manager.synced_height()); + WalletInterface::update_synced_height(&mut manager, 850_000).await; + println!(" Updated height to: {:?}", WalletInterface::synced_height(&manager)); println!("\n=== Summary ==="); println!("Total wallets created: {}", manager.wallet_count()); - println!("✅ Example completed successfully!"); + println!("Example completed successfully!"); } diff --git a/key-wallet-manager/src/accessors.rs b/key-wallet-manager/src/accessors.rs index 1f72d4616..77ee90db6 100644 --- a/key-wallet-manager/src/accessors.rs +++ b/key-wallet-manager/src/accessors.rs @@ -5,44 +5,54 @@ use crate::{ }; use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; use key_wallet::wallet::managed_wallet_info::TransactionRecord; -use key_wallet::{Account, Address, Network, Utxo, Wallet}; -use std::collections::{BTreeMap, BTreeSet}; -use tokio::sync::broadcast; +use key_wallet::{Account, Address, Network, Utxo}; +use std::collections::BTreeSet; +use std::sync::Arc; +use tokio::sync::{broadcast, RwLock}; impl WalletManager { - /// Get a wallet by ID - pub fn get_wallet(&self, wallet_id: &WalletId) -> Option<&Wallet> { - self.wallets.get(wallet_id) - } - - /// Get wallet info by ID - pub fn get_wallet_info(&self, wallet_id: &WalletId) -> Option<&T> { - self.wallet_infos.get(wallet_id) - } - - /// Get mutable wallet info by ID - pub fn get_wallet_info_mut(&mut self, wallet_id: &WalletId) -> Option<&mut T> { - self.wallet_infos.get_mut(wallet_id) + /// Get the shared `Arc>` for a wallet. + /// + /// Consumers can clone the returned `Arc` to hold a long-lived handle + /// to the wallet state (e.g. `PlatformWallet` keeps a clone). + pub fn get_wallet_arc(&self, wallet_id: &WalletId) -> Option>> { + self.wallets.get(wallet_id).cloned() } - /// Get both wallet and info by ID - pub fn get_wallet_and_info(&self, wallet_id: &WalletId) -> Option<(&Wallet, &T)> { - match (self.wallets.get(wallet_id), self.wallet_infos.get(wallet_id)) { - (Some(wallet), Some(info)) => Some((wallet, info)), - _ => None, + /// Insert an externally-created `Arc>` into the manager. + /// + /// This allows a caller (e.g. platform-wallet) to create the state, + /// wrap it in `Arc::new(RwLock::new(state))`, keep a clone for its own + /// handle, and insert the same `Arc` into `WalletManager`. + pub fn insert_wallet_state( + &mut self, + wallet_id: WalletId, + state: Arc>, + ) -> Result<(), WalletError> { + if self.wallets.contains_key(&wallet_id) { + return Err(WalletError::WalletExists(wallet_id)); } + self.wallets.insert(wallet_id, state); + self.bump_structural_revision(); + Ok(()) } - /// Remove a wallet - pub fn remove_wallet(&mut self, wallet_id: &WalletId) -> Result<(Wallet, T), WalletError> { - let wallet = + /// Remove a wallet and return its shared state Arc. + pub fn remove_wallet( + &mut self, + wallet_id: &WalletId, + ) -> Result>, WalletError> { + let arc = self.wallets.remove(wallet_id).ok_or(WalletError::WalletNotFound(*wallet_id))?; - let info = - self.wallet_infos.remove(wallet_id).ok_or(WalletError::WalletNotFound(*wallet_id))?; // Absorb the removed wallet's account-level revision so the total // stays monotonically increasing even though we lost a contributor. - self.structural_revision += info.monitor_revision() + 1; - Ok((wallet, info)) + // Use try_read to get the revision without blocking. + let removed_rev = arc + .try_read() + .map(|s| s.monitor_revision()) + .unwrap_or(0); + self.structural_revision += removed_rev + 1; + Ok(arc) } /// List all wallet IDs @@ -50,97 +60,115 @@ impl WalletManager { self.wallets.keys().collect() } - /// Get all wallets - pub fn get_all_wallets(&self) -> &BTreeMap { + /// Get all wallet state Arcs (immutable view of the map). + pub fn get_all_wallet_arcs( + &self, + ) -> &std::collections::BTreeMap>> { &self.wallets } - /// Get all wallet infos - pub fn get_all_wallet_infos(&self) -> &BTreeMap { - &self.wallet_infos - } - /// Get wallet count pub fn wallet_count(&self) -> usize { self.wallets.len() } - /// Get all accounts in a specific wallet - pub fn get_accounts(&self, wallet_id: &WalletId) -> Result, WalletError> { - let wallet = self.wallets.get(wallet_id).ok_or(WalletError::WalletNotFound(*wallet_id))?; - Ok(wallet.all_accounts()) + /// Get all accounts in a specific wallet (async, acquires read lock). + pub async fn get_accounts( + &self, + wallet_id: &WalletId, + ) -> Result, WalletError> { + let arc = + self.wallets.get(wallet_id).ok_or(WalletError::WalletNotFound(*wallet_id))?; + let state = arc.read().await; + Ok(state.wallet().all_accounts().into_iter().cloned().collect()) } - /// Get account by index in a specific wallet - pub fn get_account( + /// Get account by index in a specific wallet (async, acquires read lock). + pub async fn get_account( &self, wallet_id: &WalletId, index: u32, - ) -> Result, WalletError> { - let wallet = self.wallets.get(wallet_id).ok_or(WalletError::WalletNotFound(*wallet_id))?; - Ok(wallet.get_bip44_account(index)) + ) -> Result, WalletError> { + let arc = + self.wallets.get(wallet_id).ok_or(WalletError::WalletNotFound(*wallet_id))?; + let state = arc.read().await; + Ok(state.wallet().get_bip44_account(index).cloned()) } - /// Get transaction history for a specific wallet - pub fn wallet_transaction_history( + /// Get transaction history for a specific wallet (async, acquires read lock). + pub async fn wallet_transaction_history( &self, wallet_id: &WalletId, - ) -> Result, WalletError> { - let managed_info = - self.wallet_infos.get(wallet_id).ok_or(WalletError::WalletNotFound(*wallet_id))?; - Ok(managed_info.transaction_history()) + ) -> Result, WalletError> { + let arc = + self.wallets.get(wallet_id).ok_or(WalletError::WalletNotFound(*wallet_id))?; + let state = arc.read().await; + Ok(state.transaction_history().into_iter().cloned().collect()) } - /// Get UTXOs for all wallets across all networks - pub fn get_all_utxos(&self) -> Vec<&Utxo> { + /// Get UTXOs for all wallets (sync, uses try_read). + pub fn get_all_utxos(&self) -> Vec { let mut all_utxos = Vec::new(); - for info in self.wallet_infos.values() { - all_utxos.extend(info.utxos().iter()); + for arc in self.wallets.values() { + if let Ok(state) = arc.try_read() { + all_utxos.extend(state.utxos().into_iter().cloned()); + } } all_utxos } - /// Get UTXOs for a specific wallet - pub fn wallet_utxos(&self, wallet_id: &WalletId) -> Result, WalletError> { - let wallet_info = - self.wallet_infos.get(wallet_id).ok_or(WalletError::WalletNotFound(*wallet_id))?; - Ok(wallet_info.utxos()) + /// Get UTXOs for a specific wallet (async, acquires read lock). + pub async fn wallet_utxos( + &self, + wallet_id: &WalletId, + ) -> Result, WalletError> { + let arc = + self.wallets.get(wallet_id).ok_or(WalletError::WalletNotFound(*wallet_id))?; + let state = arc.read().await; + Ok(state.utxos().into_iter().cloned().collect()) } - /// Get total balance across all wallets and networks + /// Get total balance across all wallets (sync, uses try_read). pub fn get_total_balance(&self) -> u64 { - self.wallet_infos.values().map(|info| info.balance().total()).sum() + self.wallets + .values() + .filter_map(|arc| arc.try_read().ok()) + .map(|s| s.balance().total()) + .sum() } - /// Get balance for a specific wallet - pub fn get_wallet_balance( + /// Get balance for a specific wallet (async, acquires read lock). + pub async fn get_wallet_balance( &self, wallet_id: &WalletId, ) -> Result { - let wallet_info = - self.wallet_infos.get(wallet_id).ok_or(WalletError::WalletNotFound(*wallet_id))?; - Ok(wallet_info.balance()) + let arc = + self.wallets.get(wallet_id).ok_or(WalletError::WalletNotFound(*wallet_id))?; + let state = arc.read().await; + Ok(state.balance()) } - /// Update wallet metadata - pub fn update_wallet_metadata( + /// Update wallet metadata (async, acquires write lock). + pub async fn update_wallet_metadata( &mut self, wallet_id: &WalletId, name: Option, description: Option, ) -> Result<(), WalletError> { - let managed_info = - self.wallet_infos.get_mut(wallet_id).ok_or(WalletError::WalletNotFound(*wallet_id))?; + let arc = + self.wallets.get(wallet_id).ok_or(WalletError::WalletNotFound(*wallet_id))?; + + let mut state = arc.write().await; if let Some(new_name) = name { - managed_info.set_name(new_name); + state.set_name(new_name); } if let Some(desc) = description { - managed_info.set_description(Some(desc)); + state.set_description(Some(desc)); } - managed_info.update_last_synced(current_timestamp()); + state.update_last_synced(current_timestamp()); Ok(()) } @@ -150,11 +178,12 @@ impl WalletManager { self.network } - /// Get monitored addresses for all wallets - pub fn monitored_addresses(&self) -> Vec
{ + /// Get monitored addresses for all wallets (async, acquires read lock). + pub async fn monitored_addresses(&self) -> Vec
{ let mut addresses = Vec::new(); - for info in self.wallet_infos.values() { - addresses.extend(info.monitored_addresses()); + for arc in self.wallets.values() { + let state = arc.read().await; + addresses.extend(state.monitored_addresses()); } addresses } @@ -172,21 +201,32 @@ impl WalletManager { } /// Return the total monitor revision (structural + per-wallet account revisions). - pub fn monitor_revision(&self) -> u64 { - self.structural_revision - + self.wallet_infos.values().map(|w| w.monitor_revision()).sum::() + /// Async method, acquires read locks. + pub async fn monitor_revision(&self) -> u64 { + let mut sum = 0u64; + for arc in self.wallets.values() { + let state = arc.read().await; + sum += state.monitor_revision(); + } + self.structural_revision + sum } - /// Snapshot the current balance of every managed wallet. - pub(crate) fn snapshot_balances(&self) -> Vec<(WalletId, WalletCoreBalance)> { - self.wallet_infos.iter().map(|(id, info)| (*id, info.balance())).collect() + /// Snapshot the current balance of every managed wallet (async, acquires read locks). + pub(crate) async fn snapshot_balances(&self) -> Vec<(WalletId, WalletCoreBalance)> { + let mut balances = Vec::with_capacity(self.wallets.len()); + for (id, arc) in self.wallets.iter() { + let state = arc.read().await; + balances.push((*id, state.balance())); + } + balances } /// Emit `BalanceUpdated` events for wallets whose balance differs from the snapshot. - pub(crate) fn emit_balance_changes(&self, old_balances: &[(WalletId, WalletCoreBalance)]) { + pub(crate) async fn emit_balance_changes(&self, old_balances: &[(WalletId, WalletCoreBalance)]) { for (wallet_id, old_balance) in old_balances { - if let Some(info) = self.wallet_infos.get(wallet_id) { - let new_balance = info.balance(); + if let Some(arc) = self.wallets.get(wallet_id) { + let state = arc.read().await; + let new_balance = state.balance(); if *old_balance != new_balance { let event = WalletEvent::BalanceUpdated { wallet_id: *wallet_id, @@ -203,10 +243,12 @@ impl WalletManager { /// Get all outpoints from wallet UTXOs across all managed wallets. /// Used for bloom filter construction to detect spends of our UTXOs. - pub fn watched_outpoints(&self) -> Vec { + /// Async method, acquires read locks. + pub async fn watched_outpoints(&self) -> Vec { let mut outpoints = Vec::new(); - for info in self.wallet_infos.values() { - outpoints.extend(info.utxos().into_iter().map(|u| u.outpoint)); + for arc in self.wallets.values() { + let state = arc.read().await; + outpoints.extend(state.utxos().into_iter().map(|u| u.outpoint)); } outpoints } diff --git a/key-wallet-manager/src/event_tests.rs b/key-wallet-manager/src/event_tests.rs index 9b1a4308a..ac557201e 100644 --- a/key-wallet-manager/src/event_tests.rs +++ b/key-wallet-manager/src/event_tests.rs @@ -11,7 +11,7 @@ use key_wallet::transaction_checking::BlockInfo; #[tokio::test] async fn test_mempool_to_confirmed_event_flow() { - let (mut manager, wallet_id, addr) = setup_manager_with_wallet(); + let (mut manager, wallet_id, addr) = setup_manager_with_wallet().await; let mut rx = manager.subscribe_events(); let tx = create_tx_paying_to(&addr, 0xaa); @@ -177,7 +177,7 @@ async fn test_mempool_after_instantsend_is_suppressed() { #[tokio::test] async fn test_mempool_tx_emits_balance_updated() { - let (mut manager, wallet_id, addr) = setup_manager_with_wallet(); + let (mut manager, wallet_id, addr) = setup_manager_with_wallet().await; let mut rx = manager.subscribe_events(); let tx = create_tx_paying_to(&addr, 0xf1); @@ -204,7 +204,7 @@ async fn test_mempool_tx_emits_balance_updated() { #[tokio::test] async fn test_instantsend_tx_emits_balance_updated_spendable() { - let (mut manager, wallet_id, addr) = setup_manager_with_wallet(); + let (mut manager, wallet_id, addr) = setup_manager_with_wallet().await; let mut rx = manager.subscribe_events(); let tx = create_tx_paying_to(&addr, 0xf2); @@ -231,7 +231,7 @@ async fn test_instantsend_tx_emits_balance_updated_spendable() { #[tokio::test] async fn test_mempool_to_instantsend_transitions_balance() { - let (mut manager, wallet_id, addr) = setup_manager_with_wallet(); + let (mut manager, wallet_id, addr) = setup_manager_with_wallet().await; let mut rx = manager.subscribe_events(); let tx = create_tx_paying_to(&addr, 0xf3); @@ -253,7 +253,7 @@ async fn test_mempool_to_instantsend_transitions_balance() { ); // IS lock: balance should move from unconfirmed to spendable - manager.process_instant_send_lock(tx.txid()); + WalletInterface::process_instant_send_lock(&mut manager, tx.txid()).await; let events = drain_events(&mut rx); assert!( events.iter().any(|e| matches!( @@ -276,29 +276,37 @@ async fn test_mempool_to_instantsend_transitions_balance() { #[tokio::test] async fn test_process_instant_send_lock_for_unknown_txid() { - let (mut manager, wallet_id, _addr) = setup_manager_with_wallet(); + let (mut manager, wallet_id, _addr) = setup_manager_with_wallet().await; let mut rx = manager.subscribe_events(); let unknown_txid = dashcore::Txid::from_byte_array([0xee; 32]); - let balance_before = manager.wallet_infos.get(&wallet_id).unwrap().balance(); + let balance_before = { + let arc = manager.get_wallet_arc(&wallet_id).unwrap(); + let guard = arc.try_read().unwrap(); + guard.balance() + }; - manager.process_instant_send_lock(unknown_txid); + WalletInterface::process_instant_send_lock(&mut manager, unknown_txid).await; assert_no_events(&mut rx); - let balance_after = manager.wallet_infos.get(&wallet_id).unwrap().balance(); + let balance_after = { + let arc = manager.get_wallet_arc(&wallet_id).unwrap(); + let guard = arc.try_read().unwrap(); + guard.balance() + }; assert_eq!(balance_before, balance_after); } #[tokio::test] async fn test_process_instant_send_lock_dedup() { - let (mut manager, wallet_id, addr) = setup_manager_with_wallet(); + let (mut manager, wallet_id, addr) = setup_manager_with_wallet().await; let tx = create_tx_paying_to(&addr, 0xe1); manager.process_mempool_transaction(&tx, false).await; let mut rx = manager.subscribe_events(); // First IS lock should emit events - manager.process_instant_send_lock(tx.txid()); + WalletInterface::process_instant_send_lock(&mut manager, tx.txid()).await; let events = drain_events(&mut rx); assert!( events.iter().any(|e| matches!( @@ -321,13 +329,13 @@ async fn test_process_instant_send_lock_dedup() { ); // Second IS lock should be a no-op - manager.process_instant_send_lock(tx.txid()); + WalletInterface::process_instant_send_lock(&mut manager, tx.txid()).await; assert_no_events(&mut rx); } #[tokio::test] async fn test_process_instant_send_lock_after_block_confirmation() { - let (mut manager, wallet_id, addr) = setup_manager_with_wallet(); + let (mut manager, wallet_id, addr) = setup_manager_with_wallet().await; let tx = create_tx_paying_to(&addr, 0xe2); // Process as IS mempool tx, then confirm in block @@ -341,11 +349,11 @@ async fn test_process_instant_send_lock_after_block_confirmation() { // IS lock after block confirmation is a no-op (already tracked via mempool IS) let mut rx = manager.subscribe_events(); - manager.process_instant_send_lock(tx.txid()); + WalletInterface::process_instant_send_lock(&mut manager, tx.txid()).await; assert_no_events(&mut rx); // Confirm height preserved - let history = manager.wallet_transaction_history(&wallet_id).unwrap(); + let history = manager.wallet_transaction_history(&wallet_id).await.unwrap(); let records: Vec<_> = history.iter().filter(|r| r.txid == tx.txid()).collect(); assert_eq!(records.len(), 1); assert_eq!(records[0].height(), Some(500)); @@ -353,7 +361,7 @@ async fn test_process_instant_send_lock_after_block_confirmation() { #[tokio::test] async fn test_mixed_instantsend_paths_no_duplicate_events() { - let (mut manager, wallet_id, addr) = setup_manager_with_wallet(); + let (mut manager, wallet_id, addr) = setup_manager_with_wallet().await; let mut rx = manager.subscribe_events(); let tx = create_tx_paying_to(&addr, 0xf0); @@ -362,7 +370,7 @@ async fn test_mixed_instantsend_paths_no_duplicate_events() { drain_events(&mut rx); // IS lock via process_instant_send_lock (network IS lock message) - manager.process_instant_send_lock(tx.txid()); + WalletInterface::process_instant_send_lock(&mut manager, tx.txid()).await; let events = drain_events(&mut rx); assert!( events.iter().any(|e| matches!( @@ -387,7 +395,7 @@ async fn test_mixed_instantsend_paths_no_duplicate_events() { #[tokio::test] async fn test_mixed_instantsend_paths_reverse_no_duplicate_events() { - let (mut manager, wallet_id, addr) = setup_manager_with_wallet(); + let (mut manager, wallet_id, addr) = setup_manager_with_wallet().await; let mut rx = manager.subscribe_events(); let tx = create_tx_paying_to(&addr, 0xf1); @@ -414,7 +422,7 @@ async fn test_mixed_instantsend_paths_reverse_no_duplicate_events() { ); // Same IS lock via process_instant_send_lock — should be suppressed - manager.process_instant_send_lock(tx.txid()); + WalletInterface::process_instant_send_lock(&mut manager, tx.txid()).await; assert_no_events(&mut rx); } @@ -424,7 +432,7 @@ async fn test_process_block_emits_events() { use dashcore::hashes::Hash; use dashcore::{BlockHash, CompactTarget, TxMerkleNode}; - let (mut manager, wallet_id, addr) = setup_manager_with_wallet(); + let (mut manager, wallet_id, addr) = setup_manager_with_wallet().await; let mut rx = manager.subscribe_events(); let tx = create_tx_paying_to(&addr, 0xe3); @@ -486,7 +494,7 @@ async fn test_process_block_emits_events() { async fn test_irrelevant_mempool_tx_emits_no_events() { use dashcore::{PublicKey, ScriptBuf}; - let (mut manager, _wallet_id, _addr) = setup_manager_with_wallet(); + let (mut manager, _wallet_id, _addr) = setup_manager_with_wallet().await; let mut rx = manager.subscribe_events(); // Create a tx paying to a random script that doesn't match any wallet address @@ -540,7 +548,7 @@ async fn test_instantsend_to_chainlocked_event_flow() { #[tokio::test] async fn test_mempool_to_block_to_chainlocked_event_flow() { - let (mut manager, _wallet_id, addr) = setup_manager_with_wallet(); + let (mut manager, _wallet_id, addr) = setup_manager_with_wallet().await; let mut rx = manager.subscribe_events(); let tx = create_tx_paying_to(&addr, 0xc4); @@ -590,7 +598,7 @@ async fn test_mempool_to_block_to_chainlocked_event_flow() { #[tokio::test] async fn test_chainlocked_block_event_flow() { - let (mut manager, _wallet_id, addr) = setup_manager_with_wallet(); + let (mut manager, _wallet_id, addr) = setup_manager_with_wallet().await; let mut rx = manager.subscribe_events(); let tx = create_tx_paying_to(&addr, 0xc1); @@ -614,7 +622,7 @@ async fn test_chainlocked_block_event_flow() { #[tokio::test] async fn test_check_transaction_dry_run_does_not_persist_state() { - let (mut manager, _wallet_id, addr) = setup_manager_with_wallet(); + let (mut manager, _wallet_id, addr) = setup_manager_with_wallet().await; let mut rx = manager.subscribe_events(); let tx = create_tx_paying_to(&addr, 0xd1); diff --git a/key-wallet-manager/src/lib.rs b/key-wallet-manager/src/lib.rs index f1412de89..e22cfb0ad 100644 --- a/key-wallet-manager/src/lib.rs +++ b/key-wallet-manager/src/lib.rs @@ -15,13 +15,17 @@ pub use key_wallet; mod accessors; mod error; mod events; +pub mod managed_wallet_state; mod matching; +pub mod persistence; mod process_block; mod wallet_interface; pub use error::WalletError; pub use events::WalletEvent; +pub use managed_wallet_state::ManagedWalletState; pub use matching::{check_compact_filters_for_addresses, FilterMatchKey}; +pub use persistence::{NoPersistence, WalletPersistence}; pub use wallet_interface::{BlockProcessingResult, MempoolTransactionResult, WalletInterface}; use dashcore::blockdata::transaction::Transaction; @@ -30,13 +34,13 @@ use key_wallet::account::AccountCollection; use key_wallet::transaction_checking::TransactionContext; use key_wallet::wallet::managed_wallet_info::transaction_building::AccountTypePreference; use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; -use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; use key_wallet::{AccountType, Address, ExtendedPrivKey, Mnemonic, Network, Wallet}; use key_wallet::{ExtendedPubKey, WalletCoreBalance}; use std::collections::BTreeMap; use std::str::FromStr; +use std::sync::Arc; -use tokio::sync::broadcast; +use tokio::sync::{broadcast, RwLock}; /// Default capacity for the wallet event bus. const DEFAULT_WALLET_EVENT_CAPACITY: usize = 1000; @@ -82,22 +86,31 @@ pub struct CheckTransactionsResult { pub involved_addresses: Vec
, } -/// High-level wallet manager that manages multiple wallets +/// High-level wallet manager that manages multiple wallets. /// /// Each wallet can contain multiple accounts following BIP44 standard. /// This is the main entry point for wallet operations. +/// +/// The type parameter `T` must implement [`WalletInfoInterface`] which +/// bundles wallet key material, mutable state, and (optionally) a +/// persistence layer into a single per-wallet value. The default +/// `ManagedWalletState` pairs a `Wallet` with a `ManagedWalletInfo` +/// and a [`NoPersistence`] backend. #[derive(Debug)] -pub struct WalletManager { +pub struct WalletManager { /// Network the managed wallets are used for network: Network, /// Last fully processed block height. synced_height: CoreBlockHeight, /// Height at which filter scanning was last committed. filter_committed_height: CoreBlockHeight, - /// Immutable wallets indexed by wallet ID - wallets: BTreeMap, - /// Mutable wallet info indexed by wallet ID - wallet_infos: BTreeMap, + /// Per-wallet state indexed by wallet ID. + /// Each entry bundles the immutable `Wallet` with its mutable + /// `ManagedWalletInfo` and persistence layer. + /// + /// Wrapped in `Arc>` so that external consumers (e.g. + /// `PlatformWallet` handles) can share the same state instance. + wallets: BTreeMap>>, /// Structural revision counter incremented when wallets or accounts are /// added/removed. Combined with per-wallet account-level revisions to /// produce the total monitor revision. @@ -114,7 +127,6 @@ impl WalletManager { synced_height: 0, filter_committed_height: 0, wallets: BTreeMap::new(), - wallet_infos: BTreeMap::new(), structural_revision: 0, event_sender: broadcast::Sender::new(DEFAULT_WALLET_EVENT_CAPACITY), } @@ -160,22 +172,12 @@ impl WalletManager { return Err(WalletError::WalletExists(wallet_id)); } - // Create managed wallet info from the wallet to properly initialize accounts - // This ensures the ManagedAccountCollection is synchronized with the Wallet's accounts - let mut managed_info = T::from_wallet(&wallet); - managed_info.set_birth_height(birth_height); - managed_info.set_first_loaded_at(current_timestamp()); - - // The wallet already has accounts created according to the provided options - // No need to manually add accounts here since that's handled by from_mnemonic/from_mnemonic_with_passphrase - let wallet_mut = wallet.clone(); - - // Add the account to managed info and generate initial addresses - // Note: Address generation would need to be done through proper derivation from the account's xpub - // For now, we'll just store the wallet with the account ready + // Create combined wallet state from the wallet to properly initialize accounts + let mut state = T::from_wallet(&wallet); + state.set_birth_height(birth_height); + state.set_first_loaded_at(current_timestamp()); - self.wallets.insert(wallet_id, wallet_mut); - self.wallet_infos.insert(wallet_id, managed_info); + self.wallets.insert(wallet_id, Arc::new(RwLock::new(state))); self.bump_structural_revision(); Ok(wallet_id) } @@ -277,12 +279,11 @@ impl WalletManager { })?; // Add the wallet to the manager - let mut managed_info = T::from_wallet(&final_wallet); - managed_info.set_birth_height(birth_height); - managed_info.set_first_loaded_at(current_timestamp()); + let mut state = T::from_wallet(&final_wallet); + state.set_birth_height(birth_height); + state.set_first_loaded_at(current_timestamp()); - self.wallets.insert(wallet_id, final_wallet); - self.wallet_infos.insert(wallet_id, managed_info); + self.wallets.insert(wallet_id, Arc::new(RwLock::new(state))); self.bump_structural_revision(); Ok((serialized_bytes, wallet_id)) @@ -311,13 +312,12 @@ impl WalletManager { return Err(WalletError::WalletExists(wallet_id)); } - // Create managed wallet info - let mut managed_info = T::from_wallet(&wallet); - managed_info.set_birth_height(self.synced_height); - managed_info.set_first_loaded_at(current_timestamp()); + // Create combined wallet state + let mut state = T::from_wallet(&wallet); + state.set_birth_height(self.synced_height); + state.set_first_loaded_at(current_timestamp()); - self.wallets.insert(wallet_id, wallet); - self.wallet_infos.insert(wallet_id, managed_info); + self.wallets.insert(wallet_id, Arc::new(RwLock::new(state))); self.bump_structural_revision(); Ok(wallet_id) } @@ -352,13 +352,12 @@ impl WalletManager { return Err(WalletError::WalletExists(wallet_id)); } - // Create managed wallet info - let mut managed_info = T::from_wallet(&wallet); - managed_info.set_birth_height(self.synced_height); - managed_info.set_first_loaded_at(current_timestamp()); + // Create combined wallet state + let mut state = T::from_wallet(&wallet); + state.set_birth_height(self.synced_height); + state.set_first_loaded_at(current_timestamp()); - self.wallets.insert(wallet_id, wallet); - self.wallet_infos.insert(wallet_id, managed_info); + self.wallets.insert(wallet_id, Arc::new(RwLock::new(state))); self.bump_structural_revision(); Ok(wallet_id) } @@ -400,13 +399,12 @@ impl WalletManager { return Err(WalletError::WalletExists(wallet_id)); } - // Create managed wallet info - let mut managed_info = T::from_wallet(&wallet); - managed_info.set_birth_height(self.synced_height); - managed_info.set_first_loaded_at(current_timestamp()); + // Create combined wallet state + let mut state = T::from_wallet(&wallet); + state.set_birth_height(self.synced_height); + state.set_first_loaded_at(current_timestamp()); - self.wallets.insert(wallet_id, wallet); - self.wallet_infos.insert(wallet_id, managed_info); + self.wallets.insert(wallet_id, Arc::new(RwLock::new(state))); self.bump_structural_revision(); Ok(wallet_id) } @@ -443,15 +441,14 @@ impl WalletManager { return Err(WalletError::WalletExists(wallet_id)); } - // Create managed wallet info from the imported wallet - let mut managed_info = T::from_wallet(&wallet); + // Create combined wallet state from the imported wallet + let mut state = T::from_wallet(&wallet); // Use the current height as the birth height since we don't know when it was originally created - managed_info.set_birth_height(self.synced_height); - managed_info.set_first_loaded_at(current_timestamp()); + state.set_birth_height(self.synced_height); + state.set_first_loaded_at(current_timestamp()); - self.wallets.insert(wallet_id, wallet); - self.wallet_infos.insert(wallet_id, managed_info); + self.wallets.insert(wallet_id, Arc::new(RwLock::new(state))); self.bump_structural_revision(); Ok(wallet_id) } @@ -459,7 +456,7 @@ impl WalletManager { /// Check a transaction against all wallets and update their states if relevant. /// Returns affected wallets and any new addresses generated during gap limit maintenance. pub async fn check_transaction_in_all_wallets( - &mut self, + &self, tx: &Transaction, context: TransactionContext, update_state_if_found: bool, @@ -467,66 +464,63 @@ impl WalletManager { ) -> CheckTransactionsResult { let mut result = CheckTransactionsResult::default(); - // We need to iterate carefully since we're mutating let wallet_ids: Vec = self.wallets.keys().cloned().collect(); for wallet_id in wallet_ids { - // Get mutable references to both wallet and wallet_info - // We need to use split borrowing to get around Rust's borrow checker - let wallet_opt = self.wallets.get_mut(&wallet_id); - let wallet_info_opt = self.wallet_infos.get_mut(&wallet_id); - - if let (Some(wallet), Some(wallet_info)) = (wallet_opt, wallet_info_opt) { - let check_result = wallet_info - .check_core_transaction( - tx, - context, - wallet, - update_state_if_found, - update_balance, - ) - .await; - - // If the transaction is relevant - if check_result.is_relevant { - result.affected_wallets.push(wallet_id); - // If any wallet reports this as new, mark result as new - if check_result.is_new_transaction { - result.is_new_transaction = true; - } + let Some(arc) = self.wallets.get(&wallet_id) else { + continue; + }; - // Aggregate totals and involved addresses across wallets - result.total_received = - result.total_received.saturating_add(check_result.total_received); - result.total_sent = result.total_sent.saturating_add(check_result.total_sent); - for account_match in &check_result.affected_accounts { - for addr_info in account_match.account_type_match.all_involved_addresses() { - result.involved_addresses.push(addr_info.address); - } + let mut state = arc.write().await; + let check_result = state + .check_core_transaction( + tx, + context, + update_state_if_found, + update_balance, + ) + .await; + drop(state); + + // If the transaction is relevant + if check_result.is_relevant { + result.affected_wallets.push(wallet_id); + // If any wallet reports this as new, mark result as new + if check_result.is_new_transaction { + result.is_new_transaction = true; + } + + // Aggregate totals and involved addresses across wallets + result.total_received = + result.total_received.saturating_add(check_result.total_received); + result.total_sent = result.total_sent.saturating_add(check_result.total_sent); + for account_match in &check_result.affected_accounts { + for addr_info in account_match.account_type_match.all_involved_addresses() { + result.involved_addresses.push(addr_info.address); } + } - if check_result.is_new_transaction { - for (account_index, record) in check_result.new_records { - let event = WalletEvent::TransactionReceived { - wallet_id, - account_index, - record: Box::new(record), - }; - let _ = self.event_sender.send(event); - } - } else if check_result.state_modified { - // Known transaction whose state was modified (confirmation or IS-lock). - let event = WalletEvent::TransactionStatusChanged { + if check_result.is_new_transaction { + for (account_index, record) in check_result.new_records { + let event = WalletEvent::TransactionReceived { wallet_id, - txid: tx.txid(), - status: context, + account_index, + record: Box::new(record), }; let _ = self.event_sender.send(event); } + } else if check_result.state_modified { + // Known transaction whose state was modified (confirmation or IS-lock). + let event = WalletEvent::TransactionStatusChanged { + wallet_id, + txid: tx.txid(), + status: context, + }; + let _ = self.event_sender.send(event); } - - result.new_addresses.extend(check_result.new_addresses); } + + result.new_addresses.extend(check_result.new_addresses); } result @@ -534,49 +528,55 @@ impl WalletManager { /// Create an account in a specific wallet /// Note: The index parameter is kept for convenience, even though AccountType contains it - pub fn create_account( + pub async fn create_account( &mut self, wallet_id: &WalletId, account_type: AccountType, account_xpub: Option, ) -> Result<(), WalletError> { - let wallet = - self.wallets.get_mut(wallet_id).ok_or(WalletError::WalletNotFound(*wallet_id))?; + let arc = + self.wallets.get(wallet_id).ok_or(WalletError::WalletNotFound(*wallet_id))?; - wallet + let mut state = arc.write().await; + state + .wallet_mut() .add_account(account_type, account_xpub) .map_err(|e| WalletError::AccountCreation(e.to_string()))?; + drop(state); self.bump_structural_revision(); Ok(()) } - /// Get receive address from a specific wallet and account - pub fn get_receive_address( - &mut self, + /// Get receive address from a specific wallet and account (async, acquires write lock). + pub async fn get_receive_address( + &self, wallet_id: &WalletId, account_index: u32, account_type_pref: AccountTypePreference, mark_as_used: bool, ) -> Result { - // Get the wallet account to access the xpub - let wallet = self.wallets.get(wallet_id).ok_or(WalletError::WalletNotFound(*wallet_id))?; + let arc = + self.wallets.get(wallet_id).ok_or(WalletError::WalletNotFound(*wallet_id))?; + + let mut state = arc.write().await; - let managed_info = - self.wallet_infos.get_mut(wallet_id).ok_or(WalletError::WalletNotFound(*wallet_id))?; + // Extract xpubs before taking mutable borrows on accounts + let bip44_xpub = + state.wallet().get_bip44_account(account_index).map(|a| a.account_xpub); + let bip32_xpub = + state.wallet().get_bip32_account(account_index).map(|a| a.account_xpub); - // Get the account collection for the network - let collection = managed_info.accounts_mut(); + let collection = state.accounts_mut(); // Try to get address based on preference let (address_opt, account_type_used) = match account_type_pref { AccountTypePreference::BIP44 => { - if let (Some(managed_account), Some(wallet_account)) = ( - collection.standard_bip44_accounts.get_mut(&account_index), - wallet.get_bip44_account(account_index), - ) { + if let Some(managed_account) = + collection.standard_bip44_accounts.get_mut(&account_index) + { match managed_account - .next_receive_address(Some(&wallet_account.account_xpub), true) + .next_receive_address(bip44_xpub.as_ref(), true) { Ok(addr) => (Some(addr), Some(AccountTypeUsed::BIP44)), Err(_) => (None, None), @@ -586,12 +586,11 @@ impl WalletManager { } } AccountTypePreference::BIP32 => { - if let (Some(managed_account), Some(wallet_account)) = ( - collection.standard_bip32_accounts.get_mut(&account_index), - wallet.get_bip32_account(account_index), - ) { + if let Some(managed_account) = + collection.standard_bip32_accounts.get_mut(&account_index) + { match managed_account - .next_receive_address(Some(&wallet_account.account_xpub), true) + .next_receive_address(bip32_xpub.as_ref(), true) { Ok(addr) => (Some(addr), Some(AccountTypeUsed::BIP32)), Err(_) => (None, None), @@ -602,22 +601,20 @@ impl WalletManager { } AccountTypePreference::PreferBIP44 => { // Try BIP44 first - if let (Some(managed_account), Some(wallet_account)) = ( - collection.standard_bip44_accounts.get_mut(&account_index), - wallet.get_bip44_account(account_index), - ) { + if let Some(managed_account) = + collection.standard_bip44_accounts.get_mut(&account_index) + { match managed_account - .next_receive_address(Some(&wallet_account.account_xpub), true) + .next_receive_address(bip44_xpub.as_ref(), true) { Ok(addr) => (Some(addr), Some(AccountTypeUsed::BIP44)), Err(_) => { // Fallback to BIP32 - if let (Some(managed_account), Some(wallet_account)) = ( - collection.standard_bip32_accounts.get_mut(&account_index), - wallet.get_bip32_account(account_index), - ) { + if let Some(managed_account) = + collection.standard_bip32_accounts.get_mut(&account_index) + { match managed_account - .next_receive_address(Some(&wallet_account.account_xpub), true) + .next_receive_address(bip32_xpub.as_ref(), true) { Ok(addr) => (Some(addr), Some(AccountTypeUsed::BIP32)), Err(_) => (None, None), @@ -627,12 +624,11 @@ impl WalletManager { } } } - } else if let (Some(managed_account), Some(wallet_account)) = ( - collection.standard_bip32_accounts.get_mut(&account_index), - wallet.get_bip32_account(account_index), - ) { + } else if let Some(managed_account) = + collection.standard_bip32_accounts.get_mut(&account_index) + { match managed_account - .next_receive_address(Some(&wallet_account.account_xpub), true) + .next_receive_address(bip32_xpub.as_ref(), true) { Ok(addr) => (Some(addr), Some(AccountTypeUsed::BIP32)), Err(_) => (None, None), @@ -643,22 +639,20 @@ impl WalletManager { } AccountTypePreference::PreferBIP32 => { // Try BIP32 first - if let (Some(managed_account), Some(wallet_account)) = ( - collection.standard_bip32_accounts.get_mut(&account_index), - wallet.get_bip32_account(account_index), - ) { + if let Some(managed_account) = + collection.standard_bip32_accounts.get_mut(&account_index) + { match managed_account - .next_receive_address(Some(&wallet_account.account_xpub), true) + .next_receive_address(bip32_xpub.as_ref(), true) { Ok(addr) => (Some(addr), Some(AccountTypeUsed::BIP32)), Err(_) => { // Fallback to BIP44 - if let (Some(managed_account), Some(wallet_account)) = ( - collection.standard_bip44_accounts.get_mut(&account_index), - wallet.get_bip44_account(account_index), - ) { + if let Some(managed_account) = + collection.standard_bip44_accounts.get_mut(&account_index) + { match managed_account - .next_receive_address(Some(&wallet_account.account_xpub), true) + .next_receive_address(bip44_xpub.as_ref(), true) { Ok(addr) => (Some(addr), Some(AccountTypeUsed::BIP44)), Err(_) => (None, None), @@ -668,12 +662,11 @@ impl WalletManager { } } } - } else if let (Some(managed_account), Some(wallet_account)) = ( - collection.standard_bip44_accounts.get_mut(&account_index), - wallet.get_bip44_account(account_index), - ) { + } else if let Some(managed_account) = + collection.standard_bip44_accounts.get_mut(&account_index) + { match managed_account - .next_receive_address(Some(&wallet_account.account_xpub), true) + .next_receive_address(bip44_xpub.as_ref(), true) { Ok(addr) => (Some(addr), Some(AccountTypeUsed::BIP44)), Err(_) => (None, None), @@ -688,7 +681,7 @@ impl WalletManager { if let Some(ref address) = address_opt { if mark_as_used { // Get the account collection again for marking - let collection = managed_info.accounts_mut(); + let collection = state.accounts_mut(); // Mark address as used in the appropriate account type match account_type_used { Some(AccountTypeUsed::BIP44) => { @@ -716,31 +709,35 @@ impl WalletManager { }) } - /// Get change address from a specific wallet and account - pub fn get_change_address( - &mut self, + /// Get change address from a specific wallet and account (async, acquires write lock). + pub async fn get_change_address( + &self, wallet_id: &WalletId, account_index: u32, account_type_pref: AccountTypePreference, mark_as_used: bool, ) -> Result { - // Get the wallet account to access the xpub - let wallet = self.wallets.get(wallet_id).ok_or(WalletError::WalletNotFound(*wallet_id))?; - let managed_info = - self.wallet_infos.get_mut(wallet_id).ok_or(WalletError::WalletNotFound(*wallet_id))?; + let arc = + self.wallets.get(wallet_id).ok_or(WalletError::WalletNotFound(*wallet_id))?; + + let mut state = arc.write().await; + + // Extract xpubs before taking mutable borrows on accounts + let bip44_xpub = + state.wallet().get_bip44_account(account_index).map(|a| a.account_xpub); + let bip32_xpub = + state.wallet().get_bip32_account(account_index).map(|a| a.account_xpub); - // Get the account collection for the network - let collection = managed_info.accounts_mut(); + let collection = state.accounts_mut(); // Try to get address based on preference let (address_opt, account_type_used) = match account_type_pref { AccountTypePreference::BIP44 => { - if let (Some(managed_account), Some(wallet_account)) = ( - collection.standard_bip44_accounts.get_mut(&account_index), - wallet.get_bip44_account(account_index), - ) { + if let Some(managed_account) = + collection.standard_bip44_accounts.get_mut(&account_index) + { match managed_account - .next_change_address(Some(&wallet_account.account_xpub), true) + .next_change_address(bip44_xpub.as_ref(), true) { Ok(addr) => (Some(addr), Some(AccountTypeUsed::BIP44)), Err(_) => (None, None), @@ -750,12 +747,11 @@ impl WalletManager { } } AccountTypePreference::BIP32 => { - if let (Some(managed_account), Some(wallet_account)) = ( - collection.standard_bip32_accounts.get_mut(&account_index), - wallet.get_bip32_account(account_index), - ) { + if let Some(managed_account) = + collection.standard_bip32_accounts.get_mut(&account_index) + { match managed_account - .next_change_address(Some(&wallet_account.account_xpub), true) + .next_change_address(bip32_xpub.as_ref(), true) { Ok(addr) => (Some(addr), Some(AccountTypeUsed::BIP32)), Err(_) => (None, None), @@ -765,23 +761,19 @@ impl WalletManager { } } AccountTypePreference::PreferBIP44 => { - // Try BIP44 first - if let (Some(managed_account), Some(wallet_account)) = ( - collection.standard_bip44_accounts.get_mut(&account_index), - wallet.get_bip44_account(account_index), - ) { + if let Some(managed_account) = + collection.standard_bip44_accounts.get_mut(&account_index) + { match managed_account - .next_change_address(Some(&wallet_account.account_xpub), true) + .next_change_address(bip44_xpub.as_ref(), true) { Ok(addr) => (Some(addr), Some(AccountTypeUsed::BIP44)), Err(_) => { - // Fallback to BIP32 - if let (Some(managed_account), Some(wallet_account)) = ( - collection.standard_bip32_accounts.get_mut(&account_index), - wallet.get_bip32_account(account_index), - ) { + if let Some(managed_account) = + collection.standard_bip32_accounts.get_mut(&account_index) + { match managed_account - .next_change_address(Some(&wallet_account.account_xpub), true) + .next_change_address(bip32_xpub.as_ref(), true) { Ok(addr) => (Some(addr), Some(AccountTypeUsed::BIP32)), Err(_) => (None, None), @@ -791,12 +783,11 @@ impl WalletManager { } } } - } else if let (Some(managed_account), Some(wallet_account)) = ( - collection.standard_bip32_accounts.get_mut(&account_index), - wallet.get_bip32_account(account_index), - ) { + } else if let Some(managed_account) = + collection.standard_bip32_accounts.get_mut(&account_index) + { match managed_account - .next_change_address(Some(&wallet_account.account_xpub), true) + .next_change_address(bip32_xpub.as_ref(), true) { Ok(addr) => (Some(addr), Some(AccountTypeUsed::BIP32)), Err(_) => (None, None), @@ -806,23 +797,19 @@ impl WalletManager { } } AccountTypePreference::PreferBIP32 => { - // Try BIP32 first - if let (Some(managed_account), Some(wallet_account)) = ( - collection.standard_bip32_accounts.get_mut(&account_index), - wallet.get_bip32_account(account_index), - ) { + if let Some(managed_account) = + collection.standard_bip32_accounts.get_mut(&account_index) + { match managed_account - .next_change_address(Some(&wallet_account.account_xpub), true) + .next_change_address(bip32_xpub.as_ref(), true) { Ok(addr) => (Some(addr), Some(AccountTypeUsed::BIP32)), Err(_) => { - // Fallback to BIP44 - if let (Some(managed_account), Some(wallet_account)) = ( - collection.standard_bip44_accounts.get_mut(&account_index), - wallet.get_bip44_account(account_index), - ) { + if let Some(managed_account) = + collection.standard_bip44_accounts.get_mut(&account_index) + { match managed_account - .next_change_address(Some(&wallet_account.account_xpub), true) + .next_change_address(bip44_xpub.as_ref(), true) { Ok(addr) => (Some(addr), Some(AccountTypeUsed::BIP44)), Err(_) => (None, None), @@ -832,12 +819,11 @@ impl WalletManager { } } } - } else if let (Some(managed_account), Some(wallet_account)) = ( - collection.standard_bip44_accounts.get_mut(&account_index), - wallet.get_bip44_account(account_index), - ) { + } else if let Some(managed_account) = + collection.standard_bip44_accounts.get_mut(&account_index) + { match managed_account - .next_change_address(Some(&wallet_account.account_xpub), true) + .next_change_address(bip44_xpub.as_ref(), true) { Ok(addr) => (Some(addr), Some(AccountTypeUsed::BIP44)), Err(_) => (None, None), @@ -851,9 +837,7 @@ impl WalletManager { // Mark the address as used if requested if let Some(ref address) = address_opt { if mark_as_used { - // Get the account collection again for marking - let collection = managed_info.accounts_mut(); - // Mark address as used in the appropriate account type + let collection = state.accounts_mut(); match account_type_used { Some(AccountTypeUsed::BIP44) => { if let Some(account) = diff --git a/key-wallet-manager/src/managed_wallet_state.rs b/key-wallet-manager/src/managed_wallet_state.rs new file mode 100644 index 000000000..e2e864e29 --- /dev/null +++ b/key-wallet-manager/src/managed_wallet_state.rs @@ -0,0 +1,354 @@ +//! Combined wallet state that owns both the immutable `Wallet` and the +//! mutable `ManagedWalletInfo`, plus an optional persistence layer. +//! +//! `ManagedWalletState` implements all the traits that `WalletManager` +//! requires on its per-wallet type parameter (`WalletInfoInterface`, +//! `WalletTransactionChecker`, `ManagedAccountOperations`), delegating +//! to the appropriate inner field. + +use std::collections::BTreeSet; + +use async_trait::async_trait; +use dashcore::prelude::CoreBlockHeight; +use dashcore::{Address as DashAddress, Transaction, Txid}; + +use key_wallet::account::AccountType; +use key_wallet::bip32::ExtendedPubKey; +use key_wallet::changeset::{Merge, UtxoChangeSet}; +use key_wallet::managed_account::managed_account_collection::ManagedAccountCollection; +use key_wallet::transaction_checking::account_checker::TransactionCheckResult; +use key_wallet::transaction_checking::TransactionContext; +use key_wallet::wallet::managed_wallet_info::managed_account_operations::ManagedAccountOperations; +use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; +use key_wallet::wallet::managed_wallet_info::{ManagedWalletInfo, TransactionRecord}; +use key_wallet::transaction_checking::WalletTransactionChecker; +use key_wallet::{Network, Utxo, Wallet, WalletCoreBalance}; + +use crate::persistence::{NoPersistence, WalletPersistence}; + +/// Combined per-wallet state used by `WalletManager`. +/// +/// Bundles the immutable key material (`Wallet`), the mutable runtime +/// state (`ManagedWalletInfo`), and an optional persistence layer. +/// This replaces the previous two-map design where `Wallet` and +/// `ManagedWalletInfo` were stored separately. +#[derive(Debug)] +pub struct ManagedWalletState { + /// The immutable wallet containing keys and account structure. + pub(crate) wallet: Wallet, + /// The mutable wallet metadata, accounts, UTXOs, and balances. + pub(crate) wallet_info: ManagedWalletInfo, + /// Persistence backend for changesets produced by transaction processing. + pub(crate) persister: P, +} + +impl ManagedWalletState

{ + /// Create a new `ManagedWalletState` from its components. + pub fn new(wallet: Wallet, wallet_info: ManagedWalletInfo, persister: P) -> Self { + Self { + wallet, + wallet_info, + persister, + } + } + + /// Immutable access to the wallet key material. + pub fn wallet(&self) -> &Wallet { + &self.wallet + } + + /// Mutable access to the wallet key material. + pub fn wallet_mut(&mut self) -> &mut Wallet { + &mut self.wallet + } + + /// Immutable access to the mutable wallet metadata / accounts / UTXOs. + pub fn wallet_info(&self) -> &ManagedWalletInfo { + &self.wallet_info + } + + /// Mutable access to the mutable wallet metadata / accounts / UTXOs. + pub fn wallet_info_mut(&mut self) -> &mut ManagedWalletInfo { + &mut self.wallet_info + } + + /// Immutable access to the persistence backend. + pub fn persister(&self) -> &P { + &self.persister + } + + /// Mutable access to the persistence backend. + pub fn persister_mut(&mut self) -> &mut P { + &mut self.persister + } + + /// Split borrow: returns `(&Wallet, &mut ManagedWalletInfo)` simultaneously. + /// + /// This avoids the borrow-checker conflict that arises when calling + /// `wallet()` and `wallet_info_mut()` on the same `ManagedWalletState`, + /// because both route through `&self` / `&mut self`. + pub fn wallet_and_info_mut(&mut self) -> (&Wallet, &mut ManagedWalletInfo) { + (&self.wallet, &mut self.wallet_info) + } +} + +// --------------------------------------------------------------------------- +// WalletInfoInterface — delegate to `self.wallet_info`, with `wallet()` / +// `wallet_mut()` returning `&self.wallet` / `&mut self.wallet`. +// --------------------------------------------------------------------------- + +impl WalletInfoInterface for ManagedWalletState

{ + fn from_wallet(wallet: &Wallet) -> Self { + let wallet_info = ManagedWalletInfo::from_wallet(wallet); + Self { + wallet: wallet.clone(), + wallet_info, + persister: P::default(), + } + } + + fn from_wallet_with_name(wallet: &Wallet, name: String) -> Self { + let wallet_info = ManagedWalletInfo::from_wallet_with_name(wallet, name); + Self { + wallet: wallet.clone(), + wallet_info, + persister: P::default(), + } + } + + fn wallet(&self) -> &Wallet { + &self.wallet + } + + fn wallet_mut(&mut self) -> &mut Wallet { + &mut self.wallet + } + + fn network(&self) -> Network { + self.wallet_info.network() + } + + fn wallet_id(&self) -> [u8; 32] { + self.wallet_info.wallet_id() + } + + fn name(&self) -> Option<&str> { + self.wallet_info.name() + } + + fn set_name(&mut self, name: String) { + self.wallet_info.set_name(name); + } + + fn description(&self) -> Option<&str> { + self.wallet_info.description() + } + + fn set_description(&mut self, description: Option) { + self.wallet_info.set_description(description); + } + + fn birth_height(&self) -> CoreBlockHeight { + self.wallet_info.birth_height() + } + + fn set_birth_height(&mut self, height: CoreBlockHeight) { + self.wallet_info.set_birth_height(height); + } + + fn first_loaded_at(&self) -> u64 { + self.wallet_info.first_loaded_at() + } + + fn set_first_loaded_at(&mut self, timestamp: u64) { + self.wallet_info.set_first_loaded_at(timestamp); + } + + fn update_last_synced(&mut self, timestamp: u64) { + self.wallet_info.update_last_synced(timestamp); + } + + fn monitored_addresses(&self) -> Vec { + self.wallet_info.monitored_addresses() + } + + fn utxos(&self) -> BTreeSet<&Utxo> { + self.wallet_info.utxos() + } + + fn get_spendable_utxos(&self) -> BTreeSet<&Utxo> { + self.wallet_info.get_spendable_utxos() + } + + fn balance(&self) -> WalletCoreBalance { + self.wallet_info.balance() + } + + fn update_balance(&mut self) { + self.wallet_info.update_balance(); + } + + fn transaction_history(&self) -> Vec<&TransactionRecord> { + self.wallet_info.transaction_history() + } + + fn accounts_mut(&mut self) -> &mut ManagedAccountCollection { + self.wallet_info.accounts_mut() + } + + fn accounts(&self) -> &ManagedAccountCollection { + self.wallet_info.accounts() + } + + fn immature_transactions(&self) -> Vec { + self.wallet_info.immature_transactions() + } + + fn synced_height(&self) -> CoreBlockHeight { + self.wallet_info.synced_height() + } + + fn update_synced_height(&mut self, current_height: u32) { + self.wallet_info.update_synced_height(current_height); + } + + fn mark_instant_send_utxos(&mut self, txid: &Txid) -> (bool, UtxoChangeSet) { + self.wallet_info.mark_instant_send_utxos(txid) + } + + fn monitor_revision(&self) -> u64 { + self.wallet_info.monitor_revision() + } +} + +// --------------------------------------------------------------------------- +// WalletTransactionChecker — delegate to the extracted helper method on +// ManagedWalletInfo, then persist the changeset. +// --------------------------------------------------------------------------- + +#[async_trait] +impl WalletTransactionChecker for ManagedWalletState

{ + async fn check_core_transaction( + &mut self, + tx: &Transaction, + context: TransactionContext, + update_state: bool, + update_balance: bool, + ) -> TransactionCheckResult { + let result = self + .wallet_info + .check_core_transaction_with_wallet( + tx, + context, + &self.wallet, + update_state, + update_balance, + ) + .await; + + // Persist non-empty changesets + if !result.changeset.is_empty() { + if let Err(e) = self.persister.store(result.changeset.clone()) { + tracing::warn!("Failed to persist wallet changeset: {}", e); + } + } + + result + } +} + +// --------------------------------------------------------------------------- +// ManagedAccountOperations — delegate to `self.wallet_info`, passing +// `&self.wallet` where the trait methods require it. +// --------------------------------------------------------------------------- + +impl ManagedAccountOperations for ManagedWalletState

{ + fn add_managed_account( + &mut self, + wallet: &Wallet, + account_type: AccountType, + ) -> key_wallet::Result<()> { + self.wallet_info.add_managed_account(wallet, account_type) + } + + fn add_managed_account_with_passphrase( + &mut self, + wallet: &Wallet, + account_type: AccountType, + passphrase: &str, + ) -> key_wallet::Result<()> { + self.wallet_info + .add_managed_account_with_passphrase(wallet, account_type, passphrase) + } + + fn add_managed_account_from_xpub( + &mut self, + account_type: AccountType, + account_xpub: ExtendedPubKey, + ) -> key_wallet::Result<()> { + self.wallet_info + .add_managed_account_from_xpub(account_type, account_xpub) + } + + #[cfg(feature = "bls")] + fn add_managed_bls_account( + &mut self, + wallet: &Wallet, + account_type: AccountType, + ) -> key_wallet::Result<()> { + self.wallet_info + .add_managed_bls_account(wallet, account_type) + } + + #[cfg(feature = "bls")] + fn add_managed_bls_account_with_passphrase( + &mut self, + wallet: &Wallet, + account_type: AccountType, + passphrase: &str, + ) -> key_wallet::Result<()> { + self.wallet_info + .add_managed_bls_account_with_passphrase(wallet, account_type, passphrase) + } + + #[cfg(feature = "bls")] + fn add_managed_bls_account_from_public_key( + &mut self, + account_type: AccountType, + bls_public_key: [u8; 48], + ) -> key_wallet::Result<()> { + self.wallet_info + .add_managed_bls_account_from_public_key(account_type, bls_public_key) + } + + #[cfg(feature = "eddsa")] + fn add_managed_eddsa_account( + &mut self, + wallet: &Wallet, + account_type: AccountType, + ) -> key_wallet::Result<()> { + self.wallet_info + .add_managed_eddsa_account(wallet, account_type) + } + + #[cfg(feature = "eddsa")] + fn add_managed_eddsa_account_with_passphrase( + &mut self, + wallet: &Wallet, + account_type: AccountType, + passphrase: &str, + ) -> key_wallet::Result<()> { + self.wallet_info + .add_managed_eddsa_account_with_passphrase(wallet, account_type, passphrase) + } + + #[cfg(feature = "eddsa")] + fn add_managed_eddsa_account_from_public_key( + &mut self, + account_type: AccountType, + ed25519_public_key: [u8; 32], + ) -> key_wallet::Result<()> { + self.wallet_info + .add_managed_eddsa_account_from_public_key(account_type, ed25519_public_key) + } +} diff --git a/key-wallet-manager/src/persistence.rs b/key-wallet-manager/src/persistence.rs new file mode 100644 index 000000000..366e12d0e --- /dev/null +++ b/key-wallet-manager/src/persistence.rs @@ -0,0 +1,37 @@ +//! Persistence layer for wallet state changes. +//! +//! The [`WalletPersistence`] trait allows `ManagedWalletState` to persist +//! changesets produced by transaction checking and other state mutations. +//! [`NoPersistence`] is a no-op implementation used when persistence is +//! not required (e.g. in tests or the default `WalletManager`). + +use key_wallet::changeset::WalletChangeSet; + +/// Trait for persisting wallet state changes. +/// +/// Implementations receive changesets produced by transaction processing +/// and are responsible for durably storing them. +pub trait WalletPersistence: Send + Sync + 'static { + /// Queue a changeset for persistence. + fn store(&self, changeset: WalletChangeSet) -> Result<(), Box>; + + /// Flush any pending changesets to durable storage. + fn flush(&self) -> Result<(), Box>; +} + +/// No-op persistence implementation. +/// +/// Discards all changesets silently. Used as the default persistence +/// layer when no external storage is configured. +#[derive(Debug, Default)] +pub struct NoPersistence; + +impl WalletPersistence for NoPersistence { + fn store(&self, _changeset: WalletChangeSet) -> Result<(), Box> { + Ok(()) + } + + fn flush(&self) -> Result<(), Box> { + Ok(()) + } +} diff --git a/key-wallet-manager/src/process_block.rs b/key-wallet-manager/src/process_block.rs index 7b7e0d3d1..645836202 100644 --- a/key-wallet-manager/src/process_block.rs +++ b/key-wallet-manager/src/process_block.rs @@ -1,15 +1,19 @@ -use crate::wallet_interface::{BlockProcessingResult, MempoolTransactionResult, WalletInterface}; -use crate::{WalletEvent, WalletManager}; +//! Implementation of `WalletInterface` for `WalletManager`. +//! +//! This bridges the per-wallet `WalletInfoInterface` model used internally +//! by `WalletManager` to the aggregate `WalletInterface` trait that SPV +//! clients consume. + use async_trait::async_trait; -use core::fmt::Write as _; use dashcore::prelude::CoreBlockHeight; -use dashcore::{Address, Block, Transaction, Txid}; -use key_wallet::transaction_checking::transaction_router::TransactionRouter; -use key_wallet::transaction_checking::{BlockInfo, TransactionContext}; +use dashcore::{Address, Block, OutPoint, Transaction, Txid}; +use key_wallet::transaction_checking::TransactionContext; use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; -use std::collections::BTreeSet; use tokio::sync::broadcast; +use crate::wallet_interface::{BlockProcessingResult, MempoolTransactionResult, WalletInterface}; +use crate::{WalletEvent, WalletManager}; + #[async_trait] impl WalletInterface for WalletManager { async fn process_block( @@ -17,28 +21,38 @@ impl WalletInterface for WalletM block: &Block, height: CoreBlockHeight, ) -> BlockProcessingResult { - let mut result = BlockProcessingResult::default(); - let info = BlockInfo::new(height, block.block_hash(), block.header.time); + use key_wallet::transaction_checking::BlockInfo; - // Process each transaction using the base manager - for tx in &block.txdata { - let context = TransactionContext::InBlock(info); + let block_info = BlockInfo::new(height, block.block_hash(), block.header.time); + let context = TransactionContext::InBlock(block_info); - let check_result = - self.check_transaction_in_all_wallets(tx, context, true, false).await; + // Snapshot balances BEFORE processing transactions so we can detect changes. + let old_balances = self.snapshot_balances().await; - if !check_result.affected_wallets.is_empty() { - if check_result.is_new_transaction { + let mut result = BlockProcessingResult::default(); + for tx in &block.txdata { + let check = self + .check_transaction_in_all_wallets(tx, context, true, false) + .await; + if !check.affected_wallets.is_empty() { + if check.is_new_transaction { result.new_txids.push(tx.txid()); } else { result.existing_txids.push(tx.txid()); } + result.new_addresses.extend(check.new_addresses); } + } - result.new_addresses.extend(check_result.new_addresses); + // Update synced height and recalculate balances on each wallet. + self.synced_height = height; + for arc in self.get_all_wallet_arcs().values() { + let mut state = arc.write().await; + state.update_synced_height(height); } - self.update_synced_height(height); + // Emit balance change events by comparing pre-block snapshot. + self.emit_balance_changes(&old_balances).await; result } @@ -53,425 +67,116 @@ impl WalletInterface for WalletM } else { TransactionContext::Mempool }; - let snapshot = self.snapshot_balances(); - let check_result = self.check_transaction_in_all_wallets(tx, context, true, false).await; - let is_relevant = !check_result.affected_wallets.is_empty(); - let net_amount = if is_relevant { - check_result.total_received as i64 - check_result.total_sent as i64 - } else { - 0 - }; + let old_balances = self.snapshot_balances().await; - // Refresh cached balances only for affected wallets - for wallet_id in &check_result.affected_wallets { - if let Some(info) = self.wallet_infos.get_mut(wallet_id) { - info.update_balance(); - } + let check = self + .check_transaction_in_all_wallets(tx, context, true, true) + .await; + + self.emit_balance_changes(&old_balances).await; + + if check.affected_wallets.is_empty() { + return MempoolTransactionResult::default(); } - self.emit_balance_changes(&snapshot); + + let net_amount = + check.total_received as i64 - check.total_sent as i64; MempoolTransactionResult { - is_relevant, + is_relevant: true, net_amount, is_outgoing: net_amount < 0, - addresses: check_result.involved_addresses, - new_addresses: check_result.new_addresses, + addresses: check.involved_addresses, + new_addresses: check.new_addresses, } } - fn monitored_addresses(&self) -> Vec

{ - self.monitored_addresses() + async fn monitored_addresses(&self) -> Vec
{ + self.monitored_addresses().await } - fn watched_outpoints(&self) -> Vec { - self.watched_outpoints() - } - - fn monitor_revision(&self) -> u64 { - self.monitor_revision() - } - - async fn transaction_effect(&self, tx: &Transaction) -> Option<(i64, Vec)> { - // Aggregate across all managed wallets. If any wallet considers it relevant, - // compute net = total_received - total_sent and collect involved addresses. - let mut total_received: u64 = 0; - let mut total_sent: u64 = 0; - let mut addresses: Vec = Vec::new(); - - let mut is_relevant_any = false; - for info in self.wallet_infos.values() { - let collection = info.accounts(); - // Reuse the same routing/check logic used in normal processing - let tx_type = TransactionRouter::classify_transaction(tx); - let account_types = TransactionRouter::get_relevant_account_types(&tx_type); - let result = collection.check_transaction(tx, &account_types); - - if result.is_relevant { - is_relevant_any = true; - total_received = total_received.saturating_add(result.total_received); - total_sent = total_sent.saturating_add(result.total_sent); - - // Collect involved addresses from affected accounts - for account_match in result.affected_accounts { - for addr_info in account_match.account_type_match.all_involved_addresses() { - addresses.push(addr_info.address.to_string()); - } - } - } - } - - if is_relevant_any { - // Deduplicate addresses while preserving order - let mut seen = BTreeSet::new(); - addresses.retain(|a| seen.insert(a.clone())); - let net = (total_received as i64) - (total_sent as i64); - Some((net, addresses)) - } else { - None - } - } - - async fn earliest_required_height(&self) -> CoreBlockHeight { - self.wallet_infos.values().map(|info| info.birth_height()).min().unwrap_or(0) + async fn watched_outpoints(&self) -> Vec { + self.watched_outpoints().await } fn synced_height(&self) -> CoreBlockHeight { self.synced_height } - fn update_synced_height(&mut self, height: CoreBlockHeight) { + async fn update_synced_height(&mut self, height: CoreBlockHeight) { self.synced_height = height; - - let snapshot = self.snapshot_balances(); - - for (_wallet_id, info) in self.wallet_infos.iter_mut() { - info.update_synced_height(height); + for arc in self.wallets.values() { + let mut state = arc.write().await; + state.update_synced_height(height); + state.update_balance(); } - - self.emit_balance_changes(&snapshot); } fn filter_committed_height(&self) -> CoreBlockHeight { self.filter_committed_height } - fn update_filter_committed_height(&mut self, height: CoreBlockHeight) { + async fn update_filter_committed_height(&mut self, height: CoreBlockHeight) { self.filter_committed_height = height; if height > self.synced_height { - self.update_synced_height(height); + self.update_synced_height(height).await; } } - fn subscribe_events(&self) -> broadcast::Receiver { - self.event_sender.subscribe() + async fn earliest_required_height(&self) -> CoreBlockHeight { + let mut min_height = None; + let wallet_count = self.get_all_wallet_arcs().len(); + for arc in self.get_all_wallet_arcs().values() { + let state = arc.read().await; + let h = state.birth_height(); + min_height = Some(min_height.map_or(h, |m: CoreBlockHeight| m.min(h))); + } + let result = min_height.unwrap_or(0); + tracing::info!("WalletManager::earliest_required_height: {} wallets, result={}", wallet_count, result); + result } - fn process_instant_send_lock(&mut self, txid: Txid) { - let snapshot = self.snapshot_balances(); + async fn monitor_revision(&self) -> u64 { + self.monitor_revision().await + } - let mut affected_wallets = Vec::new(); - for (wallet_id, info) in self.wallet_infos.iter_mut() { - if info.mark_instant_send_utxos(&txid) { - affected_wallets.push(*wallet_id); + fn subscribe_events(&self) -> broadcast::Receiver { + self.subscribe_events() + } + + async fn process_instant_send_lock(&mut self, txid: Txid) { + let old_balances = self.snapshot_balances().await; + let mut any_changed = false; + + for (wallet_id, arc) in self.get_all_wallet_arcs().iter() { + let mut state = arc.write().await; + let (changed, _changeset) = state.mark_instant_send_utxos(&txid); + if changed { + any_changed = true; + state.update_balance(); + drop(state); + + let event = WalletEvent::TransactionStatusChanged { + wallet_id: *wallet_id, + txid, + status: TransactionContext::InstantSend, + }; + let _ = self.event_sender().send(event); } } - if affected_wallets.is_empty() { - return; + if any_changed { + self.emit_balance_changes(&old_balances).await; } - - for wallet_id in affected_wallets { - let event = WalletEvent::TransactionStatusChanged { - wallet_id, - txid, - status: TransactionContext::InstantSend, - }; - let _ = self.event_sender().send(event); - } - - self.emit_balance_changes(&snapshot); } async fn describe(&self) -> String { - let wallet_count = self.wallet_infos.len(); - if wallet_count == 0 { - return format!("WalletManager: 0 wallets (network {})", self.network); - } - - let mut details = Vec::with_capacity(wallet_count); - for (wallet_id, info) in &self.wallet_infos { - let name = info.name().unwrap_or("unnamed"); - - let mut wallet_id_hex = String::with_capacity(wallet_id.len() * 2); - for byte in wallet_id { - let _ = write!(&mut wallet_id_hex, "{:02x}", byte); - } - - let script_count = info.monitored_addresses().len(); - let summary = format!("{} scripts", script_count); - - details.push(format!("{} ({}): {}", name, wallet_id_hex, summary)); - } - format!( - "WalletManager: {} wallet(s) on {}\n{}", - wallet_count, - self.network, - details.join("\n") + "WalletManager({} wallets, network={:?}, synced_height={})", + self.wallet_count(), + self.network(), + self.synced_height ) } } - -#[cfg(test)] -mod tests { - use super::*; - use crate::test_helpers::*; - use dashcore::block::{Header, Version}; - use dashcore::hashes::Hash; - use dashcore::pow::CompactTarget; - use dashcore::{ - BlockHash, Network, OutPoint, ScriptBuf, TxIn, TxMerkleNode, TxOut, Txid, Witness, - }; - use key_wallet::account::StandardAccountType; - use key_wallet::wallet::initialization::WalletAccountCreationOptions; - use key_wallet::wallet::managed_wallet_info::transaction_building::AccountTypePreference; - use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; - use key_wallet::AccountType; - - fn make_block(txdata: Vec) -> Block { - Block { - header: Header { - version: Version::ONE, - prev_blockhash: BlockHash::from_byte_array([0; 32]), - merkle_root: TxMerkleNode::from_byte_array([0; 32]), - time: 1000, - bits: CompactTarget::from_consensus(0x1d00ffff), - nonce: 0, - }, - txdata, - } - } - - #[tokio::test] - async fn test_synced_height() { - let mut manager: WalletManager = WalletManager::new(Network::Testnet); - // Initial state - assert_eq!(manager.synced_height(), 0); - // Inrease synced height - manager.update_synced_height(1000); - assert_eq!(manager.synced_height(), 1000); - //Increase synced height again - manager.update_synced_height(5000); - assert_eq!(manager.synced_height(), 5000); - // Decrease synced height - manager.update_synced_height(10); - assert_eq!(manager.synced_height(), 10); - } - - #[tokio::test] - async fn test_process_mempool_transaction_balance_events() { - let (mut manager, _wallet_id, addr) = setup_manager_with_wallet(); - let mut rx = manager.subscribe_events(); - - // Relevant tx should emit BalanceUpdated - let tx = create_tx_paying_to(&addr, 0xaa); - manager.process_mempool_transaction(&tx, false).await; - - let mut found = false; - while let Ok(event) = rx.try_recv() { - if let WalletEvent::BalanceUpdated { - unconfirmed, - .. - } = event - { - assert!(unconfirmed > 0, "unconfirmed balance should increase"); - found = true; - break; - } - } - assert!(found, "should emit BalanceUpdated for mempool transaction"); - - // Irrelevant tx should not emit any events - let unrelated_tx = Transaction { - version: 2, - lock_time: 0, - input: vec![TxIn { - previous_output: OutPoint { - txid: Txid::from_byte_array([0xbb; 32]), - vout: 0, - }, - script_sig: ScriptBuf::new(), - sequence: u32::MAX, - witness: Witness::default(), - }], - output: vec![TxOut { - value: 100_000, - script_pubkey: ScriptBuf::new_p2pkh(&dashcore::PubkeyHash::from_byte_array( - [0xff; 20], - )), - }], - special_transaction_payload: None, - }; - manager.process_mempool_transaction(&unrelated_tx, false).await; - assert!(rx.try_recv().is_err(), "should not emit events for irrelevant transaction"); - } - - #[tokio::test] - async fn test_process_block_emits_balance_updated() { - let (mut manager, _wallet_id, addr) = setup_manager_with_wallet(); - let tx = create_tx_paying_to(&addr, 0xcc); - let block = make_block(vec![tx]); - - let mut rx = manager.subscribe_events(); - manager.process_block(&block, 100).await; - - let mut found = false; - while let Ok(event) = rx.try_recv() { - if let WalletEvent::BalanceUpdated { - spendable, - .. - } = event - { - assert!(spendable > 0, "spendable balance should increase after block"); - found = true; - break; - } - } - assert!(found, "should emit BalanceUpdated for block processing"); - } - - #[tokio::test] - async fn test_mempool_transaction_result_contains_wallet_effect_data() { - let (mut manager, _wallet_id, addr) = setup_manager_with_wallet(); - let tx = create_tx_paying_to(&addr, 0xaa); - - let result = manager.process_mempool_transaction(&tx, false).await; - - assert!(result.is_relevant); - assert_eq!(result.net_amount, TX_AMOUNT as i64); - assert!(!result.is_outgoing); - assert!(!result.addresses.is_empty()); - } - - #[tokio::test] - async fn test_check_transaction_populates_totals() { - let (mut manager, _wallet_id, addr) = setup_manager_with_wallet(); - - let tx = create_tx_paying_to(&addr, 0xf0); - let result = manager - .check_transaction_in_all_wallets(&tx, TransactionContext::Mempool, true, true) - .await; - - assert!(!result.affected_wallets.is_empty()); - assert_eq!(result.total_received, TX_AMOUNT); - assert_eq!(result.total_sent, 0); - assert!( - !result.involved_addresses.is_empty(), - "involved_addresses should contain the target address" - ); - assert!( - result.involved_addresses.contains(&addr), - "involved_addresses should contain the target address" - ); - } - - #[tokio::test] - async fn test_monitor_revision_bumps_and_stability() { - let mut manager: WalletManager = WalletManager::new(Network::Testnet); - let mut expected_rev = 0u64; - assert_eq!(manager.monitor_revision(), expected_rev); - - // create_wallet_from_mnemonic bumps - let wallet_id = manager - .create_wallet_from_mnemonic( - TEST_MNEMONIC, - "", - 0, - WalletAccountCreationOptions::Default, - ) - .unwrap(); - expected_rev += 1; - assert_eq!(manager.monitor_revision(), expected_rev, "after create_wallet_from_mnemonic"); - - // create_account bumps - manager - .create_account( - &wallet_id, - AccountType::Standard { - index: 1, - standard_account_type: StandardAccountType::BIP44Account, - }, - None, - ) - .unwrap(); - expected_rev += 1; - assert_eq!(manager.monitor_revision(), expected_rev, "after create_account"); - - // get_receive_address bumps (when address is generated) - let result = - manager.get_receive_address(&wallet_id, 0, AccountTypePreference::PreferBIP44, true); - if result.is_ok() && result.unwrap().address.is_some() { - expected_rev += 1; - assert_eq!(manager.monitor_revision(), expected_rev, "after get_receive_address"); - } - - // get_change_address bumps (when address is generated) - let result = - manager.get_change_address(&wallet_id, 0, AccountTypePreference::PreferBIP44, true); - if result.is_ok() && result.unwrap().address.is_some() { - expected_rev += 1; - assert_eq!(manager.monitor_revision(), expected_rev, "after get_change_address"); - } - - // update_synced_height does NOT bump - manager.update_synced_height(1000); - assert_eq!(manager.monitor_revision(), expected_rev, "after update_synced_height"); - - // process_mempool_transaction bumps from UTXO changes and possibly - // new addresses generated via gap limit maintenance - let rev_before_mempool = manager.monitor_revision(); - let addr = manager.monitored_addresses()[0].clone(); - let tx = create_tx_paying_to(&addr, 0xd0); - let _result = manager.process_mempool_transaction(&tx, false).await; - assert!( - manager.monitor_revision() > rev_before_mempool, - "mempool tx paying to our address should bump revision (UTXO added)" - ); - let rev_after_mempool = manager.monitor_revision(); - - // process_instant_send_lock does NOT bump (no outpoint set change) - manager.process_instant_send_lock(tx.txid()); - assert_eq!( - manager.monitor_revision(), - rev_after_mempool, - "after process_instant_send_lock" - ); - - // process_block bumps from UTXO changes and possibly new addresses - let rev_before_block = manager.monitor_revision(); - let tx2 = create_tx_paying_to(&addr, 0xd1); - let block = make_block(vec![tx2]); - let _result = manager.process_block(&block, 100).await; - assert!( - manager.monitor_revision() > rev_before_block, - "block with tx paying to our address should bump revision (UTXO added)" - ); - - // remove_wallet absorbs the wallet's account-level revision + 1 - let rev_before_remove = manager.monitor_revision(); - manager.remove_wallet(&wallet_id).unwrap(); - assert!( - manager.monitor_revision() > rev_before_remove, - "remove_wallet should bump revision" - ); - - // create_wallet_with_random_mnemonic bumps structural revision - let rev_before = manager.monitor_revision(); - manager.create_wallet_with_random_mnemonic(WalletAccountCreationOptions::Default).unwrap(); - assert!( - manager.monitor_revision() > rev_before, - "create_wallet_with_random_mnemonic should bump revision" - ); - } -} diff --git a/key-wallet-manager/src/test_helpers.rs b/key-wallet-manager/src/test_helpers.rs index 6b3a8dcae..8a5c78593 100644 --- a/key-wallet-manager/src/test_helpers.rs +++ b/key-wallet-manager/src/test_helpers.rs @@ -2,7 +2,6 @@ use super::*; use dashcore::hashes::Hash; use dashcore::{OutPoint, ScriptBuf, TxIn, TxOut, Txid, Witness}; use key_wallet::wallet::initialization::WalletAccountCreationOptions; -use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; use key_wallet::Network; use tokio::sync::broadcast; @@ -11,12 +10,12 @@ pub(crate) const TEST_MNEMONIC: &str = pub(crate) const TX_AMOUNT: u64 = 100_000; -pub(crate) fn setup_manager_with_wallet() -> (WalletManager, WalletId, Address) { +pub(crate) async fn setup_manager_with_wallet() -> (WalletManager, WalletId, Address) { let mut manager = WalletManager::new(Network::Testnet); let wallet_id = manager .create_wallet_from_mnemonic(TEST_MNEMONIC, "", 0, WalletAccountCreationOptions::Default) .unwrap(); - let addresses = manager.monitored_addresses(); + let addresses = manager.monitored_addresses().await; assert!(!addresses.is_empty(), "wallet should have monitored addresses"); let addr = addresses[0].clone(); (manager, wallet_id, addr) @@ -71,7 +70,7 @@ pub(crate) fn assert_no_events(rx: &mut broadcast::Receiver) { pub(crate) async fn assert_lifecycle_flow(contexts: &[TransactionContext], input_seed: u8) { assert!(!contexts.is_empty(), "at least one context required"); - let (mut manager, wallet_id, addr) = setup_manager_with_wallet(); + let (mut manager, wallet_id, addr) = setup_manager_with_wallet().await; let mut rx = manager.subscribe_events(); let tx = create_tx_paying_to(&addr, input_seed); @@ -108,7 +107,7 @@ pub(crate) async fn assert_context_suppressed( expected_height: Option, input_seed: u8, ) { - let (mut manager, wallet_id, addr) = setup_manager_with_wallet(); + let (mut manager, wallet_id, addr) = setup_manager_with_wallet().await; let mut rx = manager.subscribe_events(); let tx = create_tx_paying_to(&addr, input_seed); @@ -120,7 +119,7 @@ pub(crate) async fn assert_context_suppressed( manager.check_transaction_in_all_wallets(&tx, suppressed_context, true, true).await; assert_no_events(&mut rx); - let history = manager.wallet_transaction_history(&wallet_id).unwrap(); + let history = manager.wallet_transaction_history(&wallet_id).await.unwrap(); let records: Vec<_> = history.iter().filter(|r| r.txid == tx.txid()).collect(); assert_eq!(records.len(), 1); if let Some(height) = expected_height { diff --git a/key-wallet-manager/src/test_utils/mock_wallet.rs b/key-wallet-manager/src/test_utils/mock_wallet.rs index d90e3bd2b..398861eee 100644 --- a/key-wallet-manager/src/test_utils/mock_wallet.rs +++ b/key-wallet-manager/src/test_utils/mock_wallet.rs @@ -152,11 +152,11 @@ impl WalletInterface for MockWallet { map.get(&tx.txid()).cloned() } - fn monitored_addresses(&self) -> Vec
{ + async fn monitored_addresses(&self) -> Vec
{ self.addresses.clone() } - fn watched_outpoints(&self) -> Vec { + async fn watched_outpoints(&self) -> Vec { self.outpoints.clone() } @@ -164,11 +164,11 @@ impl WalletInterface for MockWallet { self.synced_height } - fn update_synced_height(&mut self, height: CoreBlockHeight) { + async fn update_synced_height(&mut self, height: CoreBlockHeight) { self.synced_height = height; } - fn monitor_revision(&self) -> u64 { + async fn monitor_revision(&self) -> u64 { self.monitor_revision } @@ -176,7 +176,7 @@ impl WalletInterface for MockWallet { self.event_sender.subscribe() } - fn process_instant_send_lock(&mut self, txid: Txid) { + async fn process_instant_send_lock(&mut self, txid: Txid) { let mut changes = self.status_changes.try_lock().expect("status_changes lock contention in test helper"); changes.push((txid, TransactionContext::InstantSend)); @@ -219,11 +219,11 @@ impl WalletInterface for NonMatchingMockWallet { MempoolTransactionResult::default() } - fn monitored_addresses(&self) -> Vec
{ + async fn monitored_addresses(&self) -> Vec
{ Vec::new() } - fn watched_outpoints(&self) -> Vec { + async fn watched_outpoints(&self) -> Vec { Vec::new() } @@ -231,7 +231,7 @@ impl WalletInterface for NonMatchingMockWallet { self.synced_height } - fn update_synced_height(&mut self, height: CoreBlockHeight) { + async fn update_synced_height(&mut self, height: CoreBlockHeight) { self.synced_height = height; } diff --git a/key-wallet-manager/src/wallet_interface.rs b/key-wallet-manager/src/wallet_interface.rs index cdd5dbb39..90e91e171 100644 --- a/key-wallet-manager/src/wallet_interface.rs +++ b/key-wallet-manager/src/wallet_interface.rs @@ -68,11 +68,11 @@ pub trait WalletInterface: Send + Sync + 'static { ) -> MempoolTransactionResult; /// Get all addresses the wallet is monitoring for incoming transactions - fn monitored_addresses(&self) -> Vec
; + async fn monitored_addresses(&self) -> Vec
; /// Get all outpoints the wallet is watching (unspent outputs). /// Used for bloom filter construction to detect spends of our UTXOs. - fn watched_outpoints(&self) -> Vec; + async fn watched_outpoints(&self) -> Vec; /// Return the wallet's per-transaction net change and involved addresses if known. /// Returns (net_amount, addresses) where net_amount is received - sent in satoshis. @@ -92,30 +92,31 @@ pub trait WalletInterface: Send + Sync + 'static { } /// Return the last fully processed height of the wallet. + /// Reads a plain field on WalletManager — no per-wallet lock needed. fn synced_height(&self) -> CoreBlockHeight; - /// Update the wallet's synced height. This also triggers balance updates. - fn update_synced_height(&mut self, height: CoreBlockHeight); + /// Update the wallet's synced height. This also triggers balance updates + /// on each wallet (requires per-wallet locks, hence async). + async fn update_synced_height(&mut self, height: CoreBlockHeight); /// Return the height at which filter scanning was last committed. - /// Defaults to `synced_height()` for implementations that don't separate these concepts. - // TODO: This can probably somehow be combined with synced_height(). + /// Reads a plain field on WalletManager — no per-wallet lock needed. fn filter_committed_height(&self) -> CoreBlockHeight { self.synced_height() } /// Update the filter committed height. Call when a height is fully processed /// (including any rescans for newly discovered addresses). - fn update_filter_committed_height(&mut self, height: CoreBlockHeight) { + async fn update_filter_committed_height(&mut self, height: CoreBlockHeight) { if height > self.synced_height() { - self.update_synced_height(height); + self.update_synced_height(height).await; } } /// Return a revision counter that increments whenever the set of monitored /// addresses or watched outpoints changes. The mempool manager uses this to /// detect when its bloom filter is stale without requiring an external signal. - fn monitor_revision(&self) -> u64 { + async fn monitor_revision(&self) -> u64 { 0 } @@ -124,7 +125,7 @@ pub trait WalletInterface: Send + Sync + 'static { /// Process an InstantSend lock for a transaction already in the wallet. /// Marks UTXOs as IS-locked, emits status change and balance update events. - fn process_instant_send_lock(&mut self, _txid: Txid) {} + async fn process_instant_send_lock(&mut self, _txid: Txid) {} /// Provide a human-readable description of the wallet implementation. /// diff --git a/key-wallet-manager/tests/integration_test.rs b/key-wallet-manager/tests/integration_test.rs index e48674469..b0740eb48 100644 --- a/key-wallet-manager/tests/integration_test.rs +++ b/key-wallet-manager/tests/integration_test.rs @@ -6,28 +6,27 @@ use key_wallet::wallet::initialization::WalletAccountCreationOptions; use key_wallet::wallet::managed_wallet_info::transaction_building::AccountTypePreference; use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; -use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; use key_wallet::{mnemonic::Language, Mnemonic, Network}; use key_wallet_manager::WalletInterface; -use key_wallet_manager::{WalletError, WalletManager}; +use key_wallet_manager::{ManagedWalletState, WalletError, WalletManager}; -#[test] -fn test_wallet_manager_creation() { +#[tokio::test] +async fn test_wallet_manager_creation() { // Create a wallet manager - let manager = WalletManager::::new(Network::Testnet); + let manager = WalletManager::::new(Network::Testnet); - // WalletManager::new returns Self, not Result - assert_eq!(manager.synced_height(), 0); + // WalletManager::::new returns Self, not Result + assert_eq!(WalletInterface::synced_height(&manager), 0); assert_eq!(manager.wallet_count(), 0); // No wallets created yet - assert_eq!(manager.monitor_revision(), 0); + assert_eq!(manager.monitor_revision().await, 0); } -#[test] -fn test_wallet_manager_from_mnemonic() { +#[tokio::test] +async fn test_wallet_manager_from_mnemonic() { // Create from a test mnemonic let mnemonic = Mnemonic::generate(12, Language::English).unwrap(); - let mut manager = WalletManager::::new(Network::Testnet); - assert_eq!(manager.monitor_revision(), 0); + let mut manager = WalletManager::::new(Network::Testnet); + assert_eq!(manager.monitor_revision().await, 0); // Create a wallet from mnemonic let wallet_result = manager.create_wallet_from_mnemonic( @@ -38,42 +37,44 @@ fn test_wallet_manager_from_mnemonic() { ); assert!(wallet_result.is_ok(), "Failed to create wallet: {:?}", wallet_result); assert_eq!(manager.wallet_count(), 1); - assert_eq!(manager.monitor_revision(), 1); + assert_eq!(manager.monitor_revision().await, 1); } -#[test] -fn test_account_management() { - let mut manager = WalletManager::::new(Network::Testnet); +#[tokio::test] +async fn test_account_management() { + let mut manager = WalletManager::::new(Network::Testnet); // Create a wallet first let wallet_result = manager.create_wallet_with_random_mnemonic(WalletAccountCreationOptions::Default); assert!(wallet_result.is_ok(), "Failed to create wallet: {:?}", wallet_result); let wallet_id = wallet_result.unwrap(); - assert_eq!(manager.monitor_revision(), 1); + assert_eq!(manager.monitor_revision().await, 1); // Add accounts to the wallet // Note: Index 0 already exists from wallet creation, so use index 1 - let result = manager.create_account( - &wallet_id, - key_wallet::AccountType::Standard { - index: 1, - standard_account_type: key_wallet::account::StandardAccountType::BIP44Account, - }, - None, - ); + let result = manager + .create_account( + &wallet_id, + key_wallet::AccountType::Standard { + index: 1, + standard_account_type: key_wallet::account::StandardAccountType::BIP44Account, + }, + None, + ) + .await; assert!(result.is_ok()); - assert_eq!(manager.monitor_revision(), 2); + assert_eq!(manager.monitor_revision().await, 2); // Get accounts from wallet - Default creates 11 accounts (including PlatformPayment), plus the one we added - let accounts = manager.get_accounts(&wallet_id); + let accounts = manager.get_accounts(&wallet_id).await; assert!(accounts.is_ok()); assert_eq!(accounts.unwrap().len(), 12); // 11 from Default + 1 we added } -#[test] -fn test_address_generation() { - let mut manager = WalletManager::::new(Network::Testnet); +#[tokio::test] +async fn test_address_generation() { + let mut manager = WalletManager::::new(Network::Testnet); // Create a wallet first let wallet_result = @@ -85,7 +86,9 @@ fn test_address_generation() { // But the managed wallet info might not have the account collection initialized // Test address generation - it may fail if accounts aren't initialized - let address1 = manager.get_receive_address(&wallet_id, 0, AccountTypePreference::BIP44, false); + let address1 = manager + .get_receive_address(&wallet_id, 0, AccountTypePreference::BIP44, false) + .await; // This might fail with InvalidNetwork if the account collection isn't initialized // We'll check if it's the expected error if let Err(ref e) = address1 { @@ -99,7 +102,9 @@ fn test_address_generation() { } } - let change = manager.get_change_address(&wallet_id, 0, AccountTypePreference::BIP44, false); + let change = manager + .get_change_address(&wallet_id, 0, AccountTypePreference::BIP44, false) + .await; // Same check for change address if let Err(ref e) = change { match e { @@ -109,11 +114,9 @@ fn test_address_generation() { } } -#[test] -fn test_utxo_management() { - // Unused imports removed - UTXOs are created by processing transactions - - let mut manager = WalletManager::::new(Network::Testnet); +#[tokio::test] +async fn test_utxo_management() { + let mut manager = WalletManager::::new(Network::Testnet); // Create a wallet first let wallet_result = @@ -125,19 +128,19 @@ fn test_utxo_management() { // The WalletManager doesn't have an add_utxo method directly // Instead, UTXOs are created by processing transactions - let utxos = manager.wallet_utxos(&wallet_id); + let utxos = manager.wallet_utxos(&wallet_id).await; assert!(utxos.is_ok()); // Initially empty assert_eq!(utxos.unwrap().len(), 0); - let balance = manager.get_wallet_balance(&wallet_id); + let balance = manager.get_wallet_balance(&wallet_id).await; assert!(balance.is_ok()); assert_eq!(balance.unwrap().total(), 0); } -#[test] -fn test_balance_calculation() { - let mut manager = WalletManager::::new(Network::Testnet); +#[tokio::test] +async fn test_balance_calculation() { + let mut manager = WalletManager::::new(Network::Testnet); // Create a wallet first let wallet_result = @@ -149,7 +152,7 @@ fn test_balance_calculation() { // The WalletManager doesn't have add_utxo directly // Check wallet balance (should be 0 initially) - let balance = manager.get_wallet_balance(&wallet_id); + let balance = manager.get_wallet_balance(&wallet_id).await; assert!(balance.is_ok()); assert_eq!(balance.unwrap().total(), 0); @@ -158,16 +161,16 @@ fn test_balance_calculation() { assert_eq!(total, 0); } -#[test] -fn test_block_height_tracking() { - let mut manager = WalletManager::::new(Network::Testnet); +#[tokio::test] +async fn test_block_height_tracking() { + let mut manager = WalletManager::::new(Network::Testnet); // Initial state - assert_eq!(manager.synced_height(), 0); + assert_eq!(WalletInterface::synced_height(&manager), 0); // Set height before adding wallets - manager.update_synced_height(1000); - assert_eq!(manager.synced_height(), 1000); + WalletInterface::update_synced_height(&mut manager, 1000).await; + assert_eq!(WalletInterface::synced_height(&manager), 1000); let mnemonic1 = Mnemonic::generate(12, Language::English).unwrap(); let wallet_id1 = manager @@ -192,45 +195,70 @@ fn test_block_height_tracking() { assert_eq!(manager.wallet_count(), 2); // Verify both wallets have synced_height of 0 initially - for wallet_info in manager.get_all_wallet_infos().values() { - assert_eq!(wallet_info.synced_height(), 0); + for arc in manager.get_all_wallet_arcs().values() { + let state = arc.read().await; + assert_eq!(state.synced_height(), 0); } // Update height - should propagate to all wallets - manager.update_synced_height(12345); - assert_eq!(manager.synced_height(), 12345); + WalletInterface::update_synced_height(&mut manager, 12345).await; + assert_eq!(WalletInterface::synced_height(&manager), 12345); // Verify all wallets got updated - let wallet_info1 = manager.get_wallet_info(&wallet_id1).unwrap(); - let wallet_info2 = manager.get_wallet_info(&wallet_id2).unwrap(); - assert_eq!(wallet_info1.synced_height(), 12345); - assert_eq!(wallet_info2.synced_height(), 12345); + { + let arc1 = manager.get_wallet_arc(&wallet_id1).unwrap(); + let state1 = arc1.read().await; + assert_eq!(state1.synced_height(), 12345); + } + { + let arc2 = manager.get_wallet_arc(&wallet_id2).unwrap(); + let state2 = arc2.read().await; + assert_eq!(state2.synced_height(), 12345); + } // Update again - verify subsequent updates work - manager.update_synced_height(20000); - assert_eq!(manager.synced_height(), 20000); + WalletInterface::update_synced_height(&mut manager, 20000).await; + assert_eq!(WalletInterface::synced_height(&manager), 20000); - for wallet_info in manager.get_all_wallet_infos().values() { - assert_eq!(wallet_info.synced_height(), 20000); + for arc in manager.get_all_wallet_arcs().values() { + let state = arc.read().await; + assert_eq!(state.synced_height(), 20000); } // Update wallets individually to different heights - let wallet_info1 = manager.get_wallet_info_mut(&wallet_id1).unwrap(); - wallet_info1.update_synced_height(30000); - - let wallet_info2 = manager.get_wallet_info_mut(&wallet_id2).unwrap(); - wallet_info2.update_synced_height(25000); + { + let arc1 = manager.get_wallet_arc(&wallet_id1).unwrap(); + let mut state1 = arc1.write().await; + state1.update_synced_height(30000); + } + { + let arc2 = manager.get_wallet_arc(&wallet_id2).unwrap(); + let mut state2 = arc2.write().await; + state2.update_synced_height(25000); + } // Verify each wallet has its own synced_height - let wallet_info1 = manager.get_wallet_info(&wallet_id1).unwrap(); - let wallet_info2 = manager.get_wallet_info(&wallet_id2).unwrap(); - assert_eq!(wallet_info1.synced_height(), 30000); - assert_eq!(wallet_info2.synced_height(), 25000); + { + let arc1 = manager.get_wallet_arc(&wallet_id1).unwrap(); + let state1 = arc1.read().await; + assert_eq!(state1.synced_height(), 30000); + } + { + let arc2 = manager.get_wallet_arc(&wallet_id2).unwrap(); + let state2 = arc2.read().await; + assert_eq!(state2.synced_height(), 25000); + } // Manager update_height still syncs all wallets - manager.update_synced_height(40000); - let wallet_info1 = manager.get_wallet_info(&wallet_id1).unwrap(); - let wallet_info2 = manager.get_wallet_info(&wallet_id2).unwrap(); - assert_eq!(wallet_info1.synced_height(), 40000); - assert_eq!(wallet_info2.synced_height(), 40000); + WalletInterface::update_synced_height(&mut manager, 40000).await; + { + let arc1 = manager.get_wallet_arc(&wallet_id1).unwrap(); + let state1 = arc1.read().await; + assert_eq!(state1.synced_height(), 40000); + } + { + let arc2 = manager.get_wallet_arc(&wallet_id2).unwrap(); + let state2 = arc2.read().await; + assert_eq!(state2.synced_height(), 40000); + } } diff --git a/key-wallet-manager/tests/spv_integration_tests.rs b/key-wallet-manager/tests/spv_integration_tests.rs index ddcffc29a..e69de29bb 100644 --- a/key-wallet-manager/tests/spv_integration_tests.rs +++ b/key-wallet-manager/tests/spv_integration_tests.rs @@ -1,226 +0,0 @@ -//! Integration tests for SPV wallet functionality - -use dashcore::blockdata::block::Block; -use dashcore::blockdata::transaction::Transaction; -use dashcore::constants::COINBASE_MATURITY; -use dashcore::Address; -use key_wallet::wallet::initialization::WalletAccountCreationOptions; -use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; -use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; -use key_wallet::Network; -use key_wallet_manager::WalletInterface; -use key_wallet_manager::WalletManager; - -#[tokio::test] -async fn test_block_processing() { - let mut manager = WalletManager::::new(Network::Testnet); - let _wallet_id = manager - .create_wallet_with_random_mnemonic(WalletAccountCreationOptions::Default) - .expect("Failed to create wallet"); - - let addresses = manager.monitored_addresses(); - assert!(!addresses.is_empty()); - let external = Address::dummy(Network::Testnet, 0); - - let addresses_before = manager.monitored_addresses(); - assert!(!addresses_before.is_empty()); - let tx1 = Transaction::dummy(&addresses[0], 0..0, &[100_000]); - let tx2 = Transaction::dummy(&addresses[1], 0..0, &[200_000]); - let tx3 = Transaction::dummy(&external, 0..0, &[300_000]); - - let block = Block::dummy(100, vec![tx1.clone(), tx2.clone(), tx3.clone()]); - let result = manager.process_block(&block, 100).await; - - // Both transactions should be new (first time seen) - assert_eq!(result.new_txids.len(), 2); - assert!(result.new_txids.contains(&tx1.txid())); - assert!(result.new_txids.contains(&tx2.txid())); - assert!(!result.new_txids.contains(&tx3.txid())); - // No existing transactions during initial processing - assert!(result.existing_txids.is_empty()); - assert_eq!(result.new_addresses.len(), 2); - - let addresses_after = manager.monitored_addresses(); - let actual_increase = addresses_after.len() - addresses_before.len(); - assert_eq!(result.new_addresses.len(), actual_increase); - - for new_addr in &result.new_addresses { - assert!(addresses_after.contains(new_addr)); - } -} - -#[tokio::test] -async fn test_block_processing_result_empty() { - let mut manager = WalletManager::::new(Network::Testnet); - let _wallet_id = manager - .create_wallet_with_random_mnemonic(WalletAccountCreationOptions::Default) - .expect("Failed to create wallet"); - - let external = Address::dummy(Network::Testnet, 0); - let tx1 = Transaction::dummy(&external, 0..0, &[100_000]); - let tx2 = Transaction::dummy(&external, 0..0, &[200_000]); - - let block = Block::dummy(100, vec![tx1, tx2]); - let result = manager.process_block(&block, 100).await; - - assert!(result.new_txids.is_empty()); - assert!(result.existing_txids.is_empty()); - assert!(result.new_addresses.is_empty()); -} - -fn assert_wallet_heights(manager: &WalletManager, expected_height: u32) { - assert_eq!(manager.synced_height(), expected_height, "height should be {}", expected_height); - for wallet_info in manager.get_all_wallet_infos().values() { - assert_eq!( - wallet_info.synced_height(), - expected_height, - "synced_height should be {}", - expected_height - ); - } -} - -/// Test that the wallet heights are updated after block processing. -#[tokio::test] -async fn test_height_updated_after_block_processing() { - let mut manager = WalletManager::::new(Network::Testnet); - - // Create a wallet - let _wallet_id = manager - .create_wallet_with_random_mnemonic(WalletAccountCreationOptions::Default) - .expect("Failed to create wallet"); - - // Initial state - no blocks processed yet - assert_wallet_heights(&manager, 0); - - for height in [1000, 2000, 3000] { - let tx = Transaction::dummy(&Address::dummy(Network::Testnet, 0), 0..0, &[100000]); - let block = Block::dummy(height, vec![tx]); - manager.process_block(&block, height).await; - assert_wallet_heights(&manager, height); - } -} - -#[tokio::test] -async fn test_immature_balance_matures_during_block_processing() { - let mut manager = WalletManager::::new(Network::Testnet); - - // Create a wallet and get an address to receive the coinbase - let wallet_id = manager - .create_wallet_with_random_mnemonic(WalletAccountCreationOptions::Default) - .expect("Failed to create wallet"); - - let account_xpub = { - let wallet = manager.get_wallet(&wallet_id).expect("Wallet should exist"); - wallet.accounts.standard_bip44_accounts.get(&0).expect("Should have account").account_xpub - }; - - // Get the first receive address from the wallet - let receive_address = { - let wallet_info = - manager.get_wallet_info_mut(&wallet_id).expect("Wallet info should exist"); - wallet_info - .first_bip44_managed_account_mut() - .expect("Should have managed account") - .next_receive_address(Some(&account_xpub), true) - .expect("Should get address") - }; - - // Create a coinbase transaction paying to our wallet - let coinbase_value = 100; - let coinbase_tx = Transaction::dummy_coinbase(&receive_address, coinbase_value); - - // Process the coinbase at height 1000 - let coinbase_height = 1000; - let coinbase_block = Block::dummy(coinbase_height, vec![coinbase_tx.clone()]); - manager.process_block(&coinbase_block, coinbase_height).await; - - // Verify the coinbase is detected and stored as immature - let wallet_info = manager.get_wallet_info(&wallet_id).expect("Wallet info should exist"); - assert!( - wallet_info.immature_transactions().contains(&coinbase_tx), - "Coinbase should be in immature transactions" - ); - assert_eq!( - wallet_info.balance().immature(), - coinbase_value, - "Immature balance should reflect coinbase" - ); - - // Process 99 more blocks up to just before maturity - let maturity_height = coinbase_height + COINBASE_MATURITY; - let tx = Transaction::dummy(&Address::dummy(Network::Regtest, 0), 0..0, &[1000]); - for height in (coinbase_height + 1)..maturity_height { - let block = Block::dummy(height, vec![tx.clone()]); - manager.process_block(&block, height).await; - } - - // Verify still immature just before maturity - let wallet_info = manager.get_wallet_info(&wallet_id).expect("Wallet info should exist"); - assert!( - wallet_info.immature_transactions().contains(&coinbase_tx), - "Coinbase should still be immature at height {}", - maturity_height - 1 - ); - - // Process the maturity block - let maturity_block = Block::dummy(maturity_height, vec![tx.clone()]); - manager.process_block(&maturity_block, maturity_height).await; - - // Verify the coinbase has matured - let wallet_info = manager.get_wallet_info(&wallet_id).expect("Wallet info should exist"); - assert!( - !wallet_info.immature_transactions().contains(&coinbase_tx), - "Coinbase should no longer be immature after maturity height" - ); - assert_eq!( - wallet_info.balance().immature(), - 0, - "Immature balance should be zero after maturity" - ); -} - -/// Test that rescanning a block correctly distinguishes new vs existing transactions -#[tokio::test] -async fn test_block_rescan_marks_transactions_as_existing() { - let mut manager = WalletManager::::new(Network::Testnet); - let _wallet_id = manager - .create_wallet_with_random_mnemonic(WalletAccountCreationOptions::Default) - .expect("Failed to create wallet"); - - let addresses = manager.monitored_addresses(); - assert!(!addresses.is_empty()); - - // Create a block with a transaction to our wallet - let tx1 = Transaction::dummy(&addresses[0], 0..0, &[100_000]); - let block = Block::dummy(100, vec![tx1.clone()]); - - // First processing - transaction should be new - let result1 = manager.process_block(&block, 100).await; - - assert_eq!(result1.new_txids.len(), 1, "First processing should have 1 new transaction"); - assert!( - result1.existing_txids.is_empty(), - "First processing should have no existing transactions" - ); - assert!(result1.new_txids.contains(&tx1.txid())); - - // Get transaction history count before rescan - let wallet_info = manager.get_all_wallet_infos().values().next().unwrap(); - let tx_history_count = wallet_info.transaction_history().len(); - - // Second processing (simulating rescan) - transaction should be existing - let result2 = manager.process_block(&block, 100).await; - - assert!(result2.new_txids.is_empty(), "Rescan should have no new transactions"); - assert_eq!(result2.existing_txids.len(), 1, "Rescan should have 1 existing transaction"); - assert!(result2.existing_txids.contains(&tx1.txid())); - - // Verify transaction history count hasn't changed - let wallet_info = manager.get_all_wallet_infos().values().next().unwrap(); - assert_eq!( - wallet_info.transaction_history().len(), - tx_history_count, - "Transaction history count should not increase on rescan" - ); -} diff --git a/key-wallet-manager/tests/test_serialized_wallets.rs b/key-wallet-manager/tests/test_serialized_wallets.rs index 015f19c7c..08c6dd468 100644 --- a/key-wallet-manager/tests/test_serialized_wallets.rs +++ b/key-wallet-manager/tests/test_serialized_wallets.rs @@ -2,13 +2,12 @@ #[cfg(test)] mod tests { use key_wallet::wallet::initialization::WalletAccountCreationOptions; - use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; use key_wallet::Network; - use key_wallet_manager::WalletManager; + use key_wallet_manager::{ManagedWalletState, WalletManager}; #[test] fn test_create_wallet_return_serialized_bytes() { - let mut manager = WalletManager::::new(Network::Testnet); + let mut manager = WalletManager::::new(Network::Testnet); let test_mnemonic = "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about"; @@ -27,7 +26,7 @@ mod tests { println!("Full wallet ID: {}", hex::encode(wallet_id)); // Test 2: Create watch-only wallet (no private keys) - let mut manager2 = WalletManager::::new(Network::Testnet); + let mut manager2 = WalletManager::::new(Network::Testnet); let result = manager2.create_wallet_from_mnemonic_return_serialized_bytes( test_mnemonic, "", @@ -45,7 +44,7 @@ mod tests { println!("Watch-only wallet ID: {}", hex::encode(wallet_id2)); // Test 3: Create externally signable wallet (for hardware wallets) - let mut manager3 = WalletManager::::new(Network::Testnet); + let mut manager3 = WalletManager::::new(Network::Testnet); let result = manager3.create_wallet_from_mnemonic_return_serialized_bytes( test_mnemonic, "", @@ -61,7 +60,7 @@ mod tests { println!("Externally signable wallet ID: {}", hex::encode(wallet_id3)); // Test 4: Import the serialized wallet back - let mut manager4 = WalletManager::::new(Network::Testnet); + let mut manager4 = WalletManager::::new(Network::Testnet); let import_result = manager4.import_wallet_from_bytes(&bytes); assert!(import_result.is_ok()); assert_eq!(import_result.unwrap(), wallet_id); @@ -69,7 +68,7 @@ mod tests { #[test] fn test_wallet_with_passphrase() { - let mut manager = WalletManager::::new(Network::Testnet); + let mut manager = WalletManager::::new(Network::Testnet); let test_mnemonic = "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about"; let passphrase = "test_passphrase"; @@ -87,7 +86,7 @@ mod tests { assert!(!bytes.is_empty()); // Wallet ID with passphrase should be different - let mut manager2 = WalletManager::::new(Network::Testnet); + let mut manager2 = WalletManager::::new(Network::Testnet); let result2 = manager2.create_wallet_from_mnemonic_return_serialized_bytes( test_mnemonic, "", // No passphrase diff --git a/key-wallet/ACCOUNT_TYPE_REFACTOR.md b/key-wallet/ACCOUNT_TYPE_REFACTOR.md new file mode 100644 index 000000000..dcf80aa27 --- /dev/null +++ b/key-wallet/ACCOUNT_TYPE_REFACTOR.md @@ -0,0 +1,304 @@ +# AccountType Extensibility Refactor + +## Problem Statement + +`AccountType` (and its mirror `ManagedAccountType`) is a closed enum that lives in `key-wallet`. It currently encodes: + +- Standard BIP44/BIP32 accounts ← universal wallet primitives +- CoinJoin accounts ← Dash Core-specific +- Identity registration/top-up/invitation ← Dash Platform application +- Asset lock accounts ← Dash Platform application +- Provider voting/owner/operator/platform keys ← masternode application +- DashPay receiving/external accounts ← DashPay social payment application +- Platform Payment (DIP-17) ← Dash Platform application + +**This means `key-wallet` has compile-time coupling to Dash Platform, DashPay, and masternode logic.** Any upstream crate that uses `key-wallet` and wants to add a new account type must: + +1. Add a variant to `AccountType` in `account/account_type.rs` +2. Mirror it in `ManagedAccountType` in `managed_account/managed_account_type.rs` +3. Add routing logic in `transaction_checking/transaction_router/mod.rs` +4. Add pool construction in `ManagedAccountType::from_account_type()` + +This is 4-file shotgun surgery per new account type, and it requires modifying `key-wallet` itself regardless of which upstream application introduces the new type. + +--- + +## Goal + +**`key-wallet` should know only about universal HD wallet mechanics.** Application-specific account types (DashPay, Platform, Masternode) should be defined in their own crates and composed into an application-level `AccountType` enum there. + +--- + +## Proposed Design + +### 1. Define `AccountTypeSpec` trait in `key-wallet` + +This trait encodes everything `key-wallet` needs to know about any account type to manage it: + +```rust +/// Everything key-wallet needs to know about an account type. +/// Implement this for your application's account type enum. +pub trait AccountTypeSpec: + Clone + Debug + PartialEq + Eq + Send + Sync + 'static +{ + /// Derivation path for this account type on the given network. + fn derivation_path(&self, network: Network) -> Result; + + /// The `DerivationPathReference` tag (used for logging and serialization context). + fn derivation_path_reference(&self) -> DerivationPathReference; + + /// Primary account index, if this account type has one. + fn index(&self) -> Option; + + /// Describe the address pools this account type needs. + /// key-wallet creates pools from these descriptors — no match arms needed. + fn address_pool_configs(&self) -> Vec; + + /// Whether this account operates on the Core chain (true) or only on Platform (false). + /// Core-chain accounts are candidates for transaction relevance checking. + /// Platform-only accounts (e.g., DIP-17 PlatformPayment) should return false. + fn is_core_chain_account(&self) -> bool { + true + } + + /// Which transaction types are potentially relevant to this account type. + /// Used by `TransactionRouter` to skip irrelevant accounts efficiently. + fn relevant_transaction_types(&self) -> &'static [TransactionType]; +} +``` + +#### `AddressPoolConfig` — replaces manual `AddressPool::new()` call sites + +```rust +/// Descriptor for a single address pool that key-wallet should create. +pub struct AddressPoolConfig { + /// Pool role (External / Internal / Absent / AbsentHardened) + pub pool_type: AddressPoolType, + /// Gap limit for this pool + pub gap_limit: u32, + /// Derivation suffix appended to the account base path. + /// e.g., `[Normal(0)]` for the external chain, `[Normal(1)]` for internal. + /// Empty for single-pool account types. + pub path_suffix: Vec, +} +``` + +This lets `ManagedAccountType::from_account_type()` become a single generic loop over `account_type.address_pool_configs()` — no more per-variant arms. + +--- + +### 2. Define a `ManagedAccount` generic struct + +Instead of the `ManagedAccountType` enum (which is a structural mirror of `AccountType`), use a single generic struct: + +```rust +/// Managed (mutable) account state, generic over the application's account type. +pub struct ManagedAccount { + /// The application-defined account type (carries derivation + pool config). + pub account_type: AT, + /// Address pools, created from `account_type.address_pool_configs()`. + pub pools: Vec, + pub metadata: AccountMetadata, + pub balance: WalletCoreBalance, + pub transactions: BTreeMap, + pub utxos: BTreeMap, + pub(crate) spent_outpoints: HashSet, +} +``` + +`ManagedAccount::new(account_type, network, key_source)` constructs all pools by calling `account_type.address_pool_configs()`. No match arms. The current `to_account_type()` method on `ManagedAccountType` disappears because the original `AT` value is stored directly. + +--- + +### 3. Make `Wallet` and `ManagedWalletInfo` generic + +```rust +pub struct Wallet { + pub wallet_id: [u8; 32], + pub wallet_type: WalletType, + pub accounts: BTreeMap>, +} + +pub struct ManagedWalletInfo { + pub accounts: Vec>, + // ... +} +``` + +Callers that currently write `Wallet` write `Wallet` instead. `DashAccountType` is defined in whatever crate owns the application (e.g., `dash-spv` or a new `dash-account-types` crate). + +--- + +### 4. Slim down `key-wallet`'s built-in enum + +Keep only genuinely universal types in `key-wallet` itself: + +```rust +/// Universal account types provided by key-wallet. +/// Application crates extend this by defining their own enum +/// that wraps or delegates to this one. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum CoreAccountType { + Standard { index: u32, standard_account_type: StandardAccountType }, + CoinJoin { index: u32 }, +} + +impl AccountTypeSpec for CoreAccountType { ... } +``` + +Every application-specific variant moves to the crate that owns it: + +| Variant group | Moves to | +|---|---| +| `IdentityRegistration`, `IdentityTopUp`, `IdentityInvitation`, `AssetLock*`, `PlatformPayment` | `dash-platform` crate | +| `ProviderVotingKeys`, `ProviderOwnerKeys`, `ProviderOperatorKeys`, `ProviderPlatformKeys` | `dash-masternode` crate (or `dash-spv`) | +| `DashpayReceivingFunds`, `DashpayExternalAccount` | `dashpay` crate | + +Each downstream crate defines its own enum and `impl AccountTypeSpec for MyAccountType`. The full Dash application assembles them: + +```rust +// In e.g., dash-spv or a new dash-account-types crate +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub enum DashAccountType { + Core(CoreAccountType), + Platform(PlatformAccountType), // from dash-platform + Masternode(MasternodeAccountType), // from dash-masternode + DashPay(DashPayAccountType), // from dashpay +} + +impl AccountTypeSpec for DashAccountType { + fn derivation_path(&self, network: Network) -> Result { + match self { + Self::Core(at) => at.derivation_path(network), + Self::Platform(at) => at.derivation_path(network), + Self::Masternode(at) => at.derivation_path(network), + Self::DashPay(at) => at.derivation_path(network), + } + } + // ... delegate all other methods similarly +} +``` + +Adding a new account type in any upstream crate now requires **zero changes to `key-wallet`**. + +--- + +### 5. Transaction routing — remove `AccountTypeToCheck` + +`AccountTypeToCheck` is a third parallel enum that duplicates `AccountType` variant names. It exists to avoid needing `ManagedAccountType` values in the router. It goes away: + +- `AccountTypeSpec::relevant_transaction_types()` returns which `TransactionType` values this account cares about. +- `TransactionRouter::get_relevant_accounts(tx_type, accounts)` iterates the live account list and calls `account.relevant_transaction_types().contains(&tx_type)`. + +No more separate `AccountTypeToCheck` enum. No more `TryFrom for AccountTypeToCheck`. + +--- + +## Migration Strategy (incremental) + +### Phase 1 — Introduce the trait, keep the enum + +1. Define `AccountTypeSpec` trait in `key-wallet`. +2. `impl AccountTypeSpec for AccountType` — all 15 variants, exactly matching current behavior. +3. Add `AddressPoolConfig` and make `ManagedAccountType::from_account_type()` use it internally for new variants only. + +**No breaking change yet.** Existing users keep using `AccountType` directly. + +### Phase 2 — Generic structs, additive + +1. Introduce `ManagedAccount` alongside the existing `ManagedCoreAccount` (the current concrete type). +2. Add `type ManagedCoreAccount = ManagedAccount` type alias so existing call sites compile unchanged. +3. Make `Wallet` generic; `type DashWallet = Wallet` alias for backwards compat. + +### Phase 3 — Split the enum + +1. Create `CoreAccountType` with only `Standard` and `CoinJoin`. +2. Move application-specific variants to their respective crates. +3. Create `DashAccountType` combining all of them. +4. Deprecate `AccountType` in favour of `DashAccountType` (or remove if all callers are internal). + +### Phase 4 — Remove dead code + +Drop `AccountTypeToCheck`, `ManagedAccountType` (the old enum), `TryFrom` impls, and the per-variant arms in `from_account_type()`. + +--- + +## What key-wallet retains after the refactor + +| Concept | Stays in key-wallet | Moves out | +|---|---|---| +| `AccountTypeSpec` trait | ✅ | — | +| `AddressPoolConfig` struct | ✅ | — | +| `CoreAccountType` enum | ✅ | — | +| `ManagedAccount` generic struct | ✅ | — | +| `Wallet` generic struct | ✅ | — | +| `AddressPool`, `GapLimitStage` | ✅ | — | +| BIP32/SLIP10 derivation | ✅ | — | +| Transaction checking infrastructure | ✅ | — | +| `TransactionRouter` enum/logic | ✅ (core types only) | — | +| Identity, Asset lock variants | — | dash-platform | +| Provider key variants | — | dash-masternode | +| DashPay variants | — | dashpay | +| `DashAccountType` composite enum | — | dash-spv or new crate | + +--- + +## Serialization considerations + +The `AT` type parameter must satisfy `Serialize + Deserialize` (serde) and `Encode + Decode` (bincode) under the respective features. Because `DashAccountType` is defined by the application crate, it controls its own serialization format. **Existing serialized wallet data that uses the old `AccountType` enum is unaffected** as long as the new `DashAccountType` serializes variant names identically — which it will if the migration keeps the same enum variant names at the `DashAccountType` level. + +If wire/disk format stability is required during migration, Phase 2 can keep `AccountType` as the concrete serialized type and only introduce the generic `ManagedAccount` internally. + +--- + +## Concrete "before / after" for adding a new account type + +### Before (current) + +Adding a hypothetical `VaultAccount` requires edits to: + +1. `key-wallet/src/account/account_type.rs` — add `VaultAccount` variant, `derivation_path()` arm, `index()` arm, `derivation_path_reference()` arm, `TryFrom for AccountTypeToCheck` arm +2. `key-wallet/src/managed_account/managed_account_type.rs` — add `VaultAccount` variant, mirror all `match` arms (`index()`, `address_pools()`, `address_pools_mut()`, `to_account_type()`, `from_account_type()`) +3. `key-wallet/src/transaction_checking/transaction_router/mod.rs` — add `VaultAccount` to `AccountTypeToCheck`, add `TryFrom` arm, add routing arm in `get_relevant_account_types()` + +**5–8 match arm additions across 3 files in key-wallet.** + +### After (proposed) + +Adding `VaultAccount` in `my-vault-crate`: + +```rust +// my-vault-crate/src/account_type.rs +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct VaultAccountType { + pub index: u32, +} + +impl AccountTypeSpec for VaultAccountType { + fn derivation_path(&self, network: Network) -> Result { + // custom path + } + fn derivation_path_reference(&self) -> DerivationPathReference { + DerivationPathReference::Custom(42) + } + fn index(&self) -> Option { Some(self.index) } + fn address_pool_configs(&self) -> Vec { + vec![AddressPoolConfig::single(DEFAULT_EXTERNAL_GAP_LIMIT)] + } + fn relevant_transaction_types(&self) -> &'static [TransactionType] { + &[TransactionType::Standard] + } +} +``` + +Then extend the application-level composite enum: + +```rust +pub enum AppAccountType { + Dash(DashAccountType), + Vault(VaultAccountType), // ← one line +} +impl AccountTypeSpec for AppAccountType { /* delegate match */ } +``` + +**Zero changes to key-wallet. One new type + one match arm in the application crate.** diff --git a/key-wallet/BDK_ARCHITECTURE_COMPARISON.md b/key-wallet/BDK_ARCHITECTURE_COMPARISON.md new file mode 100644 index 000000000..a693e44c5 --- /dev/null +++ b/key-wallet/BDK_ARCHITECTURE_COMPARISON.md @@ -0,0 +1,271 @@ +# key-wallet vs BDK: Architecture Comparison + +Reference: [Bitcoin Dev Kit (BDK)](https://github.com/bitcoindevkit/bdk/tree/master) + +This document compares the key-wallet architecture against BDK, identifies structural weaknesses in the current design, and lists concrete ideas worth adopting. + +--- + +## What BDK Is + +BDK is a suite of modular Rust libraries for building descriptor-based Bitcoin wallets. Its philosophy: + +> **"Data source agnostic and persistence agnostic"** — BDK doesn't care where blockchain data comes from or how it's stored. + +### BDK Crate Structure + +``` +bdk_core — Shared data types +bdk_chain — Pure blockchain data structures (TxGraph, LocalChain) — zero I/O +bdk_wallet — Wallet logic (address generation, tx building, signing) +bdk_electrum — Electrum sync backend +bdk_esplora — Esplora HTTP sync backend +bdk_bitcoind_rpc — Bitcoin Core RPC sync backend +bdk_file_store — Append-only flat-file persistence (dev/test only) +``` + +Each crate has a single responsibility. `bdk_chain` can be used without any wallet logic; sync adapters can be swapped without touching wallet logic. + +--- + +## BDK Core Concepts + +### 1. `TxGraph` — Monotonic Transaction Store + +The most important design decision in BDK. Transactions are **never deleted**. Conflicting transactions (reorgs, RBF replacements) **coexist** in the graph. The canonical view is computed on-demand from the current chain state. + +``` +TxGraph +├── txid → Transaction (full or partial "floating output") +├── spend relationships (which tx spends which outpoint) +├── anchors (block confirmations per tx) +└── timestamps (first-seen, last-seen, evicted) +``` + +**Why this matters:** A chain reorganization doesn't require rolling back state — the evicted transactions simply become non-canonical. There is no "rollback" operation needed. + +### 2. `LocalChain` — Chain Header State + +Separate from the transaction graph. Maintains a `tip` via a linked `CheckPoint` structure. Applying a header update identifies the "point of agreement" between old and new chain, enabling clean reorg detection. + +### 3. `IndexedTxGraph` — Filtered View + +Wraps `TxGraph` with an `Indexer` implementation: + +```rust +pub trait Indexer { + fn index_txout(&mut self, outpoint: OutPoint, txout: &TxOut); + fn index_tx(&mut self, tx: &Transaction); + fn is_tx_relevant(&self, tx: &Transaction) -> bool; +} +``` + +The indexer decides what's relevant (e.g. "does this output pay to one of my script pubkeys"). BDK ships `KeychainTxOutIndex` as a standard indexer, but any implementation works. + +### 4. `ChangeSet` Pattern — Atomic Persistence + +Every state mutation produces a `ChangeSet`: + +```rust +wallet.apply_update(update)?; +let changes = wallet.take_staged(); // ChangeSet ready to persist +db.write(&changes)?; +``` + +Changes are: +- **Atomic** — persist all or nothing +- **Replayable** — rebuild state from the full changelog +- **Composable** — changesets can be merged + +### 5. `TxBuilder` — Fluent Transaction Construction + +```rust +let psbt = wallet + .build_tx() + .add_recipient(script, amount) + .fee_rate(FeeRate::from_sat_per_vb(2.0)) + .coin_selection(BranchAndBoundCoinSelection::default()) + .finish()?; +``` + +Terminal `finish()` produces a PSBT. All configuration is chainable on `&mut self`. + +### 6. Descriptor-First Key Management + +A descriptor like `wpkh([fingerprint/44'/0'/0']xpub/0/*)` completely encodes: +- Which key to use +- The derivation path +- The script type (P2PKH, P2WPKH, P2TR, etc.) + +BDK generates addresses and identifies relevant outputs purely from descriptors. There is no hardcoded account type system. + +### 7. Pluggable Sync Backends + +Each backend implements a thin adapter over `bdk_chain`: + +```rust +// Electrum +let update = client.full_scan(wallet.start_full_scan(), ...)?; +wallet.apply_update(update)?; + +// Esplora (async) +let update = client.full_scan(wallet.start_full_scan(), ...).await?; +wallet.apply_update(update)?; +``` + +The wallet has no awareness of which backend is used. + +--- + +## key-wallet Architecture (Current) + +### Strengths + +- **Dash-specific DIP9 account types** — BDK has no concept of masternodes, DashPay 256-bit identity derivation, CoinJoin pools, or Platform payment accounts. The `AccountType` enum encodes Dash's entire derivation hierarchy explicitly with compile-time safety. +- **Multi-key-type address pools** — `KeySource` handles ECDSA + BLS + EdDSA in a single pool, which BDK cannot do (ECDSA/Schnorr only). +- **InstantSend awareness** — `Utxo.is_instantlocked` has no BDK equivalent. +- **Chainlock / special transaction support** — DIP2/DIP3 special transactions are first-class. + +### Structural Weaknesses + +#### 1. No Reorg Safety + +`ManagedCoreAccount` stores transactions in a plain `BTreeMap` and UTXOs in `BTreeMap`. There is no conflict-aware store. + +A chain reorganization requires: +- Identifying which transactions are no longer canonical +- Reversing UTXO changes +- Recalculating balance + +None of this logic exists. The current design **silently produces incorrect state** after a reorg. + +**BDK solution:** `TxGraph` is monotonic — transactions are never removed, canonical view is computed from current `LocalChain` state on demand. + +#### 2. No Persistence Contract + +`ManagedCoreAccount` is a plain in-memory struct. There is no defined mechanism for: +- Atomically persisting a state change +- Recovering from a partial write (crash mid-update) +- Replaying history to rebuild state + +The current CLAUDE.md guidance shows non-atomic sequential updates: + +```rust +// CURRENT: not atomic — crash between any two steps corrupts state +managed_account.add_transaction(tx_record); +managed_account.update_utxos(&tx); +managed_account.recalculate_balance(); +managed_account.mark_addresses_used(&tx); +``` + +**BDK solution:** `ChangeSet` captures all mutations as a single unit. Persist the changeset atomically, or don't persist anything. + +#### 3. `AccountType` Enum is a 4-File Bottleneck + +Adding any new DIP account type requires simultaneous changes to: +1. `account/account_type.rs` — add `AccountType` variant +2. `managed_account/managed_account_type.rs` — add `ManagedAccountType` variant +3. `transaction_checking/transaction_router/mod.rs` — add routing rule +4. `managed_account/address_pool.rs` — add pool configuration + +This is a **shotgun surgery** anti-pattern. Each new Dash feature multiplies the required change surface. + +**BDK solution:** Descriptors are self-contained. Adding a new address type is adding a new descriptor string, not modifying core enum dispatch. + +#### 4. Sync Coupling + +The wallet is tightly coupled to dash-spv via callback injection: + +```rust +// From CLAUDE.md integration guide +on_transaction_received(tx) { + let result = wallet_info.check_transaction(&tx, network, context, Some(&wallet)); + if result.is_relevant { update_wallet_state(result); } +} +``` + +dash-spv calls directly into wallet mutation. There is no clean boundary — testing wallet logic requires simulating dash-spv callbacks. + +**BDK solution:** `apply_update(update)` is the single entry point. The sync layer produces an opaque `Update` value; the wallet consumes it. They share no mutable state. + +#### 5. No Canonical Transaction View + +The wallet assumes one definitive version of each transaction. When the same txid arrives from different data sources (mempool, block, rebroadcast), the wallet has no strategy for reconciliation. + +**BDK solution:** `TxGraph` anchors track all known confirmations per transaction. The canonical version is whichever is confirmed deepest in the current `LocalChain`. + +--- + +## Ideas Worth Adopting + +### High Value + +| BDK Concept | Proposed Adaptation | +|---|---| +| **`ChangeSet` pattern** | Wrap all `ManagedCoreAccount` mutations to produce a `WalletChangeSet`. Persistence layer writes changesets atomically. Enables crash recovery and audit log. | +| **Monotonic transaction store** | Replace `BTreeMap` with a conflict-aware `TxStore` that retains evicted transactions and computes canonical view from chain state. | +| **`apply_update()` boundary** | Define `WalletUpdate` as an opaque value produced by dash-spv. `ManagedWalletInfo::apply_update(update)` is the single mutation entry point. Decouples sync from wallet state. | + +### Medium Value + +| BDK Concept | Proposed Adaptation | +|---|---| +| **`Indexer` trait** | Extract `TransactionRouter` into an `Indexer` trait. New account types implement the trait rather than modifying core router dispatch. | +| **Pluggable persistence** | Add a `WalletStore` trait so `ManagedCoreAccount` can be backed by SQLite, in-memory, or custom storage without changing wallet logic. | +| **Balance categories** | BDK's 4-tier balance (Immature / TrustedPending / UntrustedPending / Confirmed) maps cleanly to `WalletCoreBalance`. Consider adding `is_trusted` distinction between self-generated and external unconfirmed transactions. | + +### Low Value / Dash-Specific Override + +| BDK Concept | Why Not Applicable | +|---|---| +| **Descriptor-first keys** | DashPay 256-bit paths and DIP9 hierarchy cannot be expressed in miniscript descriptors. The `AccountType` enum is the right abstraction for Dash. | +| **Single key type** | BLS + EdDSA + ECDSA coexistence in a single pool is a genuine requirement with no BDK equivalent. | + +--- + +## Proposed Architectural Target + +``` +┌─────────────────────────────────────────────┐ +│ Application │ +└────────────────────┬────────────────────────┘ + │ apply_update(WalletUpdate) +┌────────────────────▼────────────────────────┐ +│ ManagedWalletInfo │ +│ ┌──────────────────────────────────────┐ │ +│ │ apply_update() → WalletChangeSet │ │ +│ │ canonical_balance() │ │ +│ │ list_unspent() │ │ +│ └──────────────────────────────────────┘ │ +│ ↓ state │ +│ ┌──────────────────────────────────────┐ │ +│ │ TxStore (conflict-aware) │ │ +│ │ ChainState (LocalChain equivalent) │ │ +│ │ AddressPools (per AccountType) │ │ +│ └──────────────────────────────────────┘ │ +└────────────────────┬────────────────────────┘ + │ WalletChangeSet +┌────────────────────▼────────────────────────┐ +│ WalletStore (trait) │ +│ impl: SQLite | in-memory | bincode file │ +└─────────────────────────────────────────────┘ + +Sync layer (dash-spv) produces WalletUpdate independently. +No shared mutable state with wallet. +``` + +--- + +## Summary + +| Concern | Current key-wallet | BDK approach | +|---|---|---| +| Reorg safety | ❌ No rollback mechanism | ✅ Monotonic TxGraph | +| Persistence | ❌ Implicit, non-atomic | ✅ ChangeSet pattern | +| Sync coupling | ❌ Direct callback mutation | ✅ `apply_update()` boundary | +| New account types | ❌ 4-file change minimum | ✅ New descriptor / Indexer impl | +| Multi-key types (BLS/EdDSA) | ✅ First-class | ❌ Not supported | +| DIP9 account hierarchy | ✅ Explicit, compile-safe | ❌ No equivalent | +| InstantSend / ChainLock | ✅ Modeled | ❌ No equivalent | + +The two highest-priority improvements are **ChangeSet-based persistence** and the **`apply_update()` sync boundary**. Both can be introduced incrementally without redesigning the `AccountType` system. diff --git a/key-wallet/KEY_DERIVATION_ISSUES.md b/key-wallet/KEY_DERIVATION_ISSUES.md new file mode 100644 index 000000000..d9381fffd --- /dev/null +++ b/key-wallet/KEY_DERIVATION_ISSUES.md @@ -0,0 +1,212 @@ +# Key Derivation Issues & Improvement Plan + +Analysis of `src/derivation.rs` and `src/dip9.rs`. + +--- + +## Issue 1 — Two parallel derivation systems + +`derivation.rs` defines `HDWallet`, `AccountDerivation`, `DerivationPathBuilder`, and `DerivationStrategy`. The actual wallet derivation used by `ManagedCoreAccount` and `Wallet` goes through `AccountType::derivation_path()` + the `IndexConstPath` constants in `dip9.rs`. These two systems coexist without integration — `HDWallet` is never called from `ManagedCoreAccount` or `Wallet`. It is essentially dead infrastructure. + +**Impact:** Maintenance burden. New derivation logic added to one system is invisible to the other. + +**Fix:** Remove `HDWallet`, `AccountDerivation`, and `DerivationStrategy` or consolidate them onto `IndexConstPath`. `DerivationPathBuilder` can stay as a utility if it fixes the silent-error bugs below. + +--- + +## Issue 2 — `for_network_and_type` silently ignores `account_type` (bug) + +**File:** `src/derivation.rs:334` + +```rust +pub fn for_network_and_type( + self, + network: Network, + _account_type: AccountType, // underscore — never read + account_index: u32, +) -> Result { + // Always derives BIP44, regardless of what account_type was passed + self.purpose(44).coin_type(coin_type).account(account_index)... +} +``` + +Calling this with `AccountType::ProviderVotingKeys` silently returns a BIP44 path. No error, no warning. The function signature promises network/type-aware derivation but delivers BIP44 unconditionally. + +**Fix:** Either delegate to `account_type.derivation_path(network)` or remove the method entirely. + +--- + +## Issue 3 — `hardened()` and `normal()` silently swallow errors (bug) + +**File:** `src/derivation.rs:249` + +```rust +pub fn hardened(mut self, index: u32) -> Self { + if let Ok(child) = ChildNumber::from_hardened_idx(index) { + self.components.push(child); + } + // silently no-ops if index is out of range + self +} +``` + +An invalid index produces a silently shorter path. The builder's `build()` then returns a valid `Result` over the wrong (truncated) path, so callers have no indication anything went wrong. + +**Fix:** Either make `hardened()`/`normal()` return `Result` (breaking but correct), or change the terminal methods (`build()`, `bip44()`) to validate that all requested components were successfully added. + +--- + +## Issue 4 — `Secp256k1::new()` allocated per operation (performance) + +Three separate call sites create a new secp256k1 context on every invocation: + +- `HDWallet::new()` — stores `Secp256k1` in the struct +- `AccountDerivation::new()` — same +- `IndexConstPath::derive_priv_ecdsa_for_master_seed()` — `src/dip9.rs:96` + +A secp256k1 context allocates and fills precomputed tables (~520 KB for `All`). Creating one per derivation call, or one per wallet instance that gets cloned, wastes significant time and memory. + +**Fix:** Use a thread-local context: + +```rust +thread_local! { + static SECP: Secp256k1 = Secp256k1::new(); +} +``` + +Or accept `&Secp256k1` as a parameter in all derivation functions and let the caller own one shared instance. The `IndexConstPath` methods already do this partially — `derive_priv_ecdsa_for_master_seed` is the outlier that creates its own. + +--- + +## Issue 5 — String parsing per address derivation (performance) + +**File:** `src/derivation.rs:176` + +```rust +pub fn receive_address(&self, index: u32) -> Result { + let path = format!("m/0/{}", index) + .parse::() // heap alloc + parse on every call + .map_err(|e| Error::InvalidDerivationPath(e.to_string()))?; +``` + +Same pattern in `change_address()`. Every address derivation allocates a formatted string and runs the path parser. The `IndexConstPath::append()` pattern in `dip9.rs` shows the correct approach: build `DerivationPath` directly from `ChildNumber` values. + +**Fix:** + +```rust +pub fn receive_address(&self, index: u32) -> Result { + let path = DerivationPath::from(vec![ + ChildNumber::Normal { index: 0 }, + ChildNumber::from_normal_idx(index).map_err(Error::Bip32)?, + ]); +``` + +--- + +## Issue 6 — Private key used where public derivation suffices (security hygiene) + +**File:** `src/derivation.rs:406` + +```rust +pub fn scan_for_activity( + &self, + key: &ExtendedPrivKey, // accepts private key + ... + F: Fn(&ExtendedPubKey) -> bool, +) -> Result> { + let derived = key.derive_priv(secp, &path)?; // derives private child + let pubkey = ExtendedPubKey::from_priv(secp, &derived); + if check_fn(&pubkey) { ... // only needs pubkey +``` + +For non-hardened child paths (all external/internal address pool scanning), public key derivation is cryptographically sufficient. Deriving private keys here unnecessarily exposes private key material to the check function's closure scope and any future logging/error paths. + +**Fix:** Accept `&ExtendedPubKey` and use `derive_pub`. Watch-only wallets can use this without any private key. Callers that have a private key can pass `ExtendedPubKey::from_priv(secp, &priv_key)` once at the call site. + +--- + +## Issue 7 — Provider key paths have no named constants in `dip9.rs` + +All other DIP9 paths are defined as `IndexConstPath` constants: + +``` +COINJOIN_PATH_MAINNET / TESTNET +IDENTITY_REGISTRATION_PATH_MAINNET / TESTNET +IDENTITY_TOPUP_PATH_MAINNET / TESTNET +IDENTITY_INVITATION_PATH_MAINNET / TESTNET +ASSET_LOCK_ADDRESS_TOPUP_PATH_MAINNET / TESTNET +ASSET_LOCK_SHIELDED_ADDRESS_TOPUP_PATH_MAINNET / TESTNET +DASHPAY_ROOT_PATH_MAINNET / TESTNET +PLATFORM_PAYMENT_ROOT_PATH_MAINNET / TESTNET +``` + +But provider key paths (`ProviderVotingKeys`, `ProviderOwnerKeys`, `ProviderOperatorKeys`, `ProviderPlatformKeys`) are assembled inline inside `AccountType::derivation_path()` using raw `ChildNumber::from_hardened_idx` calls with magic numbers. The DIP-3 paths are: + +| Account type | Path | +|---|---| +| ProviderVotingKeys | `m/9'/coin_type'/3'/1'` | +| ProviderOwnerKeys | `m/9'/coin_type'/3'/2'` | +| ProviderOperatorKeys | `m/9'/coin_type'/3'/3'` | +| ProviderPlatformKeys | `m/9'/coin_type'/3'/4'` | + +**Fix:** Add `PROVIDER_VOTING_PATH_MAINNET`, `PROVIDER_OWNER_PATH_MAINNET`, etc. as `IndexConstPath` constants, matching the pattern used for every other DIP9 path. + +--- + +## Issue 8 — `DerivationPathReference` has no extension point + +**File:** `src/dip9.rs:13` + +```rust +pub enum DerivationPathReference { + Unknown = 0, + BIP32 = 1, + // ... 16 more fixed variants + Root = 255, +} +``` + +Plain integer enum, no room for application-defined values. When the `AccountTypeSpec` trait refactor lands (see `ACCOUNT_TYPE_REFACTOR.md`), external crates defining their own account types will need to tag their derivation paths with a reference discriminant that does not conflict with the built-in ones. + +**Fix:** Add a `Custom(u32)` variant, or reserve a range (e.g. 128–254) for application use and document it. The `Custom` approach is cleanest: + +```rust +pub enum DerivationPathReference { + // ... existing variants + Custom(u32), // for application-defined paths outside the Dash standard set +} +``` + +This requires handling in the serde/bincode impls but is straightforward. + +--- + +## Issue 9 — No caching of intermediate derived keys (performance) + +Paths like `m/9'/5'/15'/0'` (DashPay root for account 0) are the shared parent of many contact-specific keys. Currently every derivation walks the full path from the master key. For a wallet with 50 DashPay contacts, the 3-component prefix `m/9'/5'/15'` is re-derived 50 times. + +The same applies to any account type with many child keys: identity keys, provider keys, Platform payment keys. + +**Impact:** Quadratic derivation cost relative to the number of keys per account. Noticeable during wallet restore/scan when hundreds of keys are checked. + +**Fix:** Cache intermediate `ExtendedPrivKey` / `ExtendedPubKey` nodes at the account level. The natural place is `ManagedAccount`, which already holds the account-level xpub. Derivation below the account root should start from the cached account key, not re-derive from master. + +The `address_pool` already receives an account key via `KeySource` — this is the correct boundary. The issue is that `IndexConstPath::derive_priv_ecdsa_for_master_seed` is sometimes called to re-derive the account key itself from seed on each use rather than caching it after the first derivation. + +--- + +## Priority + +| # | Issue | Type | Priority | +|---|---|---|---| +| 2 | `for_network_and_type` ignores `account_type` | Bug | High | +| 3 | `hardened()`/`normal()` swallow errors | Bug | High | +| 1 | Two parallel derivation systems | Architecture | Medium | +| 4 | `Secp256k1::new()` per operation | Performance | Medium | +| 5 | String parsing per address | Performance | Medium | +| 6 | Private key used where public suffices | Security hygiene | Medium | +| 7 | Provider paths have no named constants | Maintainability | Low | +| 8 | `DerivationPathReference` not extensible | Architecture | Low (blocks `AccountTypeSpec` refactor) | +| 9 | No intermediate key cache | Performance | Low | + +Issues 2 and 3 are silent correctness bugs and should be fixed first regardless of any other refactoring. Issues 1 and 8 are blockers for the `AccountTypeSpec` extensibility refactor described in `ACCOUNT_TYPE_REFACTOR.md`. diff --git a/key-wallet/PSBT_MIGRATION.md b/key-wallet/PSBT_MIGRATION.md new file mode 100644 index 000000000..be063e84c --- /dev/null +++ b/key-wallet/PSBT_MIGRATION.md @@ -0,0 +1,161 @@ +# PSBT Migration: key-wallet → dash + +## Current situation + +`key-wallet/src/psbt/` contains a full BIP174 PSBT implementation (~4100 lines across 9 files). It is exported from `key-wallet` as `pub mod psbt` and consumed only by `key-wallet/tests/psbt.rs`. Nothing inside `key-wallet`'s own wallet/account/transaction logic imports it. + +``` +key-wallet/src/psbt/ +├── mod.rs — PartiallySignedTransaction, signing logic (1894 lines) +├── map/ +│ ├── input.rs — PSBT Input map (616 lines) +│ ├── output.rs — PSBT Output map (187 lines) +│ ├── global.rs — PSBT Global map (251 lines) +│ └── mod.rs — Map trait (34 lines) +├── serialize.rs — BIP174 binary serialization (486 lines) +├── raw.rs — Raw key/value encoding (228 lines) +├── error.rs — Error types (240 lines) +└── macros.rs — Internal macros (179 lines) +``` + +### Why it doesn't belong in key-wallet + +PSBT is a **transaction serialization and signing coordination format** defined in BIP174. It operates on `Transaction`, `TxIn`, `TxOut`, `Script`, and ECDSA signatures — all types that live in the `dash/` crate. It has no dependency on HD wallet accounts, address pools, gap limits, mnemonics, or any other concept from `key-wallet`. + +The only reason it ended up in `key-wallet` is that `key-wallet`'s `bip32` module provides `ExtendedPrivKey`, `ExtendedPubKey`, and `KeySource`, which PSBT uses for BIP32 derivation metadata in the global map. Because `dash/` has no `bip32` module at all, there was nowhere else to put it. + +### The actual dependency graph + +``` +key-wallet::psbt + ├── dashcore::Transaction, TxOut, Script, PublicKey, PrivateKey + ├── dashcore::sighash::{EcdsaSighashType, SighashCache} + ├── dashcore::crypto::ecdsa + └── key-wallet::bip32::{ExtendedPrivKey, ExtendedPubKey, KeySource} + ↑ + only dependency on key-wallet +``` + +`bip32` is the only thing tying PSBT to `key-wallet`. If `bip32` were in `dash/`, PSBT could move there without any further changes. + +--- + +## Why the correct home is `dash/` + +`dash/` is the Dash protocol library. It already contains: +- `blockdata/transaction/` — `Transaction`, `TxIn`, `TxOut` +- `crypto/` — ECDSA keys and signatures +- `sighash/` — sighash computation +- `bip143.rs`, `bip152.rs`, `bip158.rs` — other Bitcoin protocol extensions + +PSBT belongs in this layer. It is a protocol-level transaction format, not a wallet-level concept. The correct import for consumers should be `dashcore::psbt::Psbt`, not `key_wallet::psbt::Psbt`. + +--- + +## Migration plan + +### Step 1 — Move `bip32` into `dash/` + +This is the prerequisite. `key-wallet`'s `bip32` module wraps and extends `ExtendedPrivKey`/`ExtendedPubKey` with Dash-specific derivation (256-bit `Normal256` child numbers for DashPay). It needs to move to `dash/src/bip32/` and be re-exported from `dashcore`. + +`key-wallet` then imports `bip32` from `dashcore` instead of defining it locally. This is independently valuable — `bip32` types are already used in `dashcore`'s signer interface and have no reason to be wallet-only. + +### Step 2 — Move `psbt/` into `dash/src/psbt/` + +Once `bip32` is in `dash/`, all PSBT imports become: + +```rust +// Before +use crate::bip32::{ExtendedPrivKey, ExtendedPubKey, KeySource}; +use dashcore::blockdata::transaction::Transaction; + +// After (from inside dash/) +use crate::bip32::{ExtendedPrivKey, ExtendedPubKey, KeySource}; +use crate::blockdata::transaction::Transaction; +``` + +The internal `crate::psbt::*` references stay unchanged. Only the crate boundary moves. + +### Step 3 — Re-export from `key-wallet` for backwards compatibility + +Add a re-export in `key-wallet/src/lib.rs` so existing consumers do not break: + +```rust +// key-wallet/src/lib.rs +pub use dashcore::psbt; +``` + +This keeps `key_wallet::psbt::Psbt` working while the canonical path becomes `dashcore::psbt::Psbt`. The re-export can be deprecated and removed in a later breaking release. + +### Step 4 — Wire PSBT into the transaction builder + +The transaction builder at `key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs` currently builds transactions without PSBT. Once PSBT is in `dash/` and easily importable, the builder should: + +1. Construct a `Psbt` from the unsigned transaction and UTXO set +2. Add BIP32 derivation metadata for each input (the account xpub and child derivation path) +3. Return `Psbt` from `build_transaction()` instead of raw `Transaction` +4. Sign via `Psbt::sign()` when a private key is available +5. Finalize and extract the signed `Transaction` for broadcast + +This is the original motivation for including PSBT in `key-wallet` in the first place — it was added in anticipation of this signing flow but never connected. + +--- + +## File mapping + +| Current path | Target path | +|---|---| +| `key-wallet/src/psbt/mod.rs` | `dash/src/psbt/mod.rs` | +| `key-wallet/src/psbt/map/input.rs` | `dash/src/psbt/map/input.rs` | +| `key-wallet/src/psbt/map/output.rs` | `dash/src/psbt/map/output.rs` | +| `key-wallet/src/psbt/map/global.rs` | `dash/src/psbt/map/global.rs` | +| `key-wallet/src/psbt/map/mod.rs` | `dash/src/psbt/map/mod.rs` | +| `key-wallet/src/psbt/serialize.rs` | `dash/src/psbt/serialize.rs` | +| `key-wallet/src/psbt/raw.rs` | `dash/src/psbt/raw.rs` | +| `key-wallet/src/psbt/error.rs` | `dash/src/psbt/error.rs` | +| `key-wallet/src/psbt/macros.rs` | `dash/src/psbt/macros.rs` | +| `key-wallet/tests/psbt.rs` | `dash/tests/psbt.rs` | + +--- + +## Import changes required in PSBT source files + +All `crate::psbt::*` internal references stay unchanged. The only imports that need updating are the cross-crate ones: + +```rust +// Current (inside key-wallet) +use crate::bip32::KeySource; +use crate::bip32::{self, ExtendedPrivKey, ExtendedPubKey}; +use dashcore::blockdata::script::ScriptBuf; +use dashcore::blockdata::transaction::txout::TxOut; +use dashcore::blockdata::transaction::Transaction; +use dashcore::crypto::ecdsa; +use dashcore::crypto::key::{PrivateKey, PublicKey}; +use dashcore::sighash::{self, EcdsaSighashType, SighashCache}; +use dashcore::Amount; + +// After move (inside dash/) +use crate::bip32::KeySource; // bip32 now in dash/ +use crate::bip32::{self, ExtendedPrivKey, ExtendedPubKey}; +use crate::blockdata::script::ScriptBuf; // already in dash/ +use crate::blockdata::transaction::txout::TxOut; // already in dash/ +use crate::blockdata::transaction::Transaction; // already in dash/ +use crate::crypto::ecdsa; // already in dash/ +use crate::crypto::key::{PrivateKey, PublicKey}; // already in dash/ +use crate::sighash::{self, EcdsaSighashType, SighashCache}; // already in dash/ +use crate::Amount; // already in dash/ +``` + +Every `dashcore::` prefix becomes `crate::` — a mechanical find-and-replace. + +--- + +## Summary + +| Concern | Status | +|---|---| +| PSBT is used by key-wallet wallet logic | No — zero imports from `wallet/` | +| PSBT is used by external crates (ffi, spv) | No — only `key-wallet/tests/psbt.rs` | +| PSBT has non-protocol dependencies on key-wallet | Only `bip32`, which itself should move to `dash/` | +| Migration is a breaking API change | Only if `key_wallet::psbt` re-export is removed; can be done gradually | +| Transaction builder should use PSBT after migration | Yes — that was the original intent | diff --git a/key-wallet/REORG_SAFETY.md b/key-wallet/REORG_SAFETY.md new file mode 100644 index 000000000..135afaaaf --- /dev/null +++ b/key-wallet/REORG_SAFETY.md @@ -0,0 +1,245 @@ +# Reorg Safety Issue + +## Where reorg handling currently lives + +`dash-spv` has a `ChainTipManager` (`dash-spv/src/chain/chain_tip.rs`) that tracks multiple chain tips and can detect when a competing chain has more work. It exposes `should_reject_fork()` (checkpoint protection against deep reorgs) and `update_active_tip()` (switches the active tip to the highest-work chain). + +However, **`ChainTipManager` never notifies the wallet**. When the active tip switches, no callback is fired, no wallet method is called, and no transaction state is updated. + +The `WalletInterface` trait (`key-wallet-manager/src/wallet_interface.rs`) — the contract between dash-spv and the wallet — has no reorg-related method at all: + +```rust +pub trait WalletInterface: Send + Sync + 'static { + async fn process_block(&mut self, block: &Block, height: CoreBlockHeight) -> BlockProcessingResult; + async fn process_mempool_transaction(&mut self, tx: &Transaction); + fn monitored_addresses(&self) -> Vec
; + fn synced_height(&self) -> CoreBlockHeight; + fn update_synced_height(&mut self, height: CoreBlockHeight); + // ... no process_reorg, no on_block_disconnected, nothing +} +``` + +The `BlocksManager` (`dash-spv/src/sync/blocks/manager.rs`) calls `wallet.process_block()` for each new block but has no corresponding `wallet.disconnect_block()` or `wallet.process_reorg()` call anywhere in the sync pipeline. + +**The gap is at the interface boundary.** The chain layer knows about forks; the wallet layer has no way to receive that information. + +--- + +## Problem + +`ManagedCoreAccount` stores transactions and UTXOs in plain maps: + +```rust +pub transactions: BTreeMap, +pub utxos: BTreeMap, +pub balance: WalletCoreBalance, +``` + +There is no concept of chain state. A transaction is either present or absent. A UTXO is either spendable or not. When a chain reorganization happens — blocks are rolled back and replaced by a competing chain — the wallet has no mechanism to respond correctly. + +### What a reorg requires + +1. **Identify evicted transactions** — transactions confirmed in the rolled-back blocks that are no longer canonical +2. **Restore spent UTXOs** — outputs spent by evicted transactions must become spendable again +3. **Remove created UTXOs** — outputs created by evicted transactions must be removed +4. **Recalculate balance** — derived from UTXO set, so wrong after steps 2-3 if not recomputed +5. **Handle re-confirmed transactions** — an evicted transaction may reappear in the new chain; its state must transition from "evicted" back to "confirmed" without duplication +6. **Update address usage** — if an evicted transaction was the first to use an address, that address is no longer "used" from a gap-limit perspective + +None of this logic exists. After a reorg the wallet silently holds: +- Stale confirmed transactions that no longer exist on chain +- Missing UTXOs that were spent by evicted transactions (i.e. spendable funds not visible) +- Phantom UTXOs created by evicted transactions (i.e. unspendable funds appearing spendable) +- Wrong balance + +This is a **silent data corruption** — no error is returned, no flag is set, the wallet simply operates on incorrect state. + +--- + +## How BDK solves it + +BDK's `TxGraph` is **monotonic**: transactions are never deleted. Instead, each transaction accumulates anchors (block confirmations) and timestamps. The canonical view — which transactions are confirmed, which are in mempool, which are evicted — is computed on demand from the current `LocalChain` tip. + +``` +TxGraph +├── txid → Transaction +├── anchors: Map<(txid, BlockId) → A> — all known confirmations +├── last_seen: Map — mempool first-seen +└── last_evicted: Map — eviction timestamp +``` + +A reorg is handled by updating `LocalChain` to the new tip. No transactions are removed from `TxGraph`. The canonical balance query walks the graph filtered by the new chain state — evicted transactions simply stop contributing to the canonical view. + +This also means a transaction that was evicted and then re-confirmed in the new chain does not need special handling: it already exists in the graph, a new anchor is added, and it becomes canonical again. + +--- + +## Current state in key-wallet + +### `TransactionRecord` has no chain state + +```rust +pub struct TransactionRecord { + pub transaction: Transaction, + pub block_hash: Option, + pub block_height: Option, + pub timestamp: u64, + // no: is_evicted, confirmations_at_tip, competing_txids +} +``` + +`block_height: Option` is set once when a block is processed and never cleared. There is no field to express "this transaction was confirmed at height 1000 but that block was reorganized away." + +### `Utxo` has no chain state + +```rust +pub struct Utxo { + pub outpoint: OutPoint, + pub tx_out: TxOut, + pub address: Address, + pub is_instantlocked: bool, + // no: confirmed_at_height, is_from_evicted_tx +} +``` + +### Balance is a snapshot, not a view + +`WalletCoreBalance` is recalculated from the UTXO set. If the UTXO set is wrong (stale entries from evicted transactions, missing entries from restored outputs), the balance is wrong with no indication of the error. + +--- + +## Proposed fix + +### 0. Add `process_reorg` to `WalletInterface` + +This is the missing link. `key-wallet-manager/src/wallet_interface.rs` needs a new method: + +```rust +pub trait WalletInterface: Send + Sync + 'static { + // ... existing methods ... + + /// Called when a chain reorganization is detected. + /// `disconnected_heights` is the range of block heights that have been rolled back. + /// The wallet should mark any transactions confirmed at those heights as evicted, + /// restore spent UTXOs, and recalculate balances. + async fn process_reorg(&mut self, disconnected_heights: RangeInclusive); +} +``` + +And `dash-spv`'s sync pipeline must call it when `ChainTipManager` switches the active tip to a shorter chain, passing the height range of the disconnected blocks. + +### 1. Add `confirmation_status` to `TransactionRecord` + +Replace the flat `block_hash` / `block_height` fields with an explicit status type: + +```rust +pub enum ConfirmationStatus { + /// In mempool, not yet confirmed. + Unconfirmed { first_seen: u64 }, + /// Confirmed in a block at a given height. + Confirmed { block_hash: BlockHash, height: u32, timestamp: u64 }, + /// Was confirmed, but that block was reorganized away. + /// The transaction may re-confirm later. + Evicted { evicted_at_tip: u32, last_confirmed_height: u32 }, +} + +pub struct TransactionRecord { + pub transaction: Transaction, + pub status: ConfirmationStatus, + // InstantSend lock survives reorgs — it is a separate protocol guarantee + pub is_instantlocked: bool, +} +``` + +Evicted transactions are retained, not deleted. They can transition back to `Confirmed` if they reappear in the new chain. + +### 2. Add `confirmed_at_height` to `Utxo` + +```rust +pub struct Utxo { + pub outpoint: OutPoint, + pub tx_out: TxOut, + pub address: Address, + pub confirmed_at_height: Option, // None = unconfirmed/mempool + pub is_instantlocked: bool, +} +``` + +### 3. Add a `apply_reorg(evicted_heights: RangeInclusive)` method + +```rust +impl ManagedCoreAccount { + /// Apply a chain reorganization. All transactions confirmed at heights + /// within `evicted_heights` are marked as evicted. UTXOs are updated + /// accordingly and balance is recalculated. + pub fn apply_reorg(&mut self, evicted_heights: RangeInclusive) { + // 1. Find all transactions confirmed in evicted range + // 2. Mark them Evicted + // 3. Remove UTXOs created by those transactions + // 4. Restore UTXOs spent by those transactions + // 5. Recalculate balance + // 6. Revert address usage marks if applicable + } +} +``` + +### 4. Compute canonical balance from status + +```rust +impl ManagedCoreAccount { + pub fn recalculate_balance(&mut self) { + // Only UTXOs from Confirmed or InstantSend-locked transactions + // contribute to the trusted balance. + // UTXOs from Unconfirmed transactions are "pending". + // UTXOs from Evicted transactions are excluded entirely. + } +} +``` + +--- + +## Interaction with InstantSend + +Dash's InstantSend is relevant here. A transaction that is InstantSend-locked has a quorum-signed lock that is independent of block confirmation. An IS-locked transaction that gets reorganized out of a block is still IS-locked — it will be re-included in the next block. + +This means: +- `is_instantlocked` on a `TransactionRecord` should survive a reorg (keep the field as-is) +- An IS-locked evicted transaction should still show its outputs as spendable in balance calculations, because re-inclusion is guaranteed by the IS lock +- A non-IS-locked evicted transaction's outputs should be excluded from balance until re-confirmed + +This is the main place where Dash diverges from BDK's reorg model and needs custom logic. + +--- + +## Interaction with ChainLocks + +ChainLocked blocks cannot be reorganized away. If a transaction is confirmed in a ChainLocked block, `apply_reorg` should be a no-op for that transaction — ChainLocks provide finality stronger than any number of confirmations. + +`TransactionRecord` should optionally track whether its confirming block is ChainLocked: + +```rust +Confirmed { + block_hash: BlockHash, + height: u32, + timestamp: u64, + is_chainlocked: bool, // if true, this confirmation is final +} +``` + +`apply_reorg` skips any transaction with `is_chainlocked: true`. + +--- + +## Summary + +| Concern | Current state | Required change | Location | +| --- | --- | --- | --- | +| Reorg notification to wallet | ❌ Not wired | `process_reorg()` in `WalletInterface` | `key-wallet-manager` + `dash-spv` | +| Evicted transaction detection | ❌ Not possible | `ConfirmationStatus::Evicted` variant | `key-wallet` | +| UTXO rollback on reorg | ❌ Not implemented | `apply_reorg()` method | `key-wallet` | +| Balance after reorg | ❌ Silently wrong | Recompute from status-filtered UTXO set | `key-wallet` | +| Re-confirmation of evicted tx | ❌ Would duplicate | Retained in map, status updated | `key-wallet` | +| IS-locked tx survives reorg | ❌ Not modeled | `is_instantlocked` flag in status | `key-wallet` | +| ChainLocked block finality | ❌ Not modeled | `is_chainlocked` in `Confirmed` variant | `key-wallet` | + +The minimum viable fix is adding `ConfirmationStatus` to `TransactionRecord` and implementing `apply_reorg`. The ChainLock and InstantSend refinements can follow as the SPV sync layer provides that data. diff --git a/key-wallet/src/changeset/changeset.rs b/key-wallet/src/changeset/changeset.rs new file mode 100644 index 000000000..23cf46e04 --- /dev/null +++ b/key-wallet/src/changeset/changeset.rs @@ -0,0 +1,226 @@ +//! Wallet changeset types for incremental persistence. +//! +//! A [`WalletChangeSet`] captures the delta between the current wallet state +//! and the last persisted snapshot. Sub-changesets cover individual domains +//! (chain tip, UTXOs, transactions, accounts, balances). + +use std::collections::{BTreeMap, BTreeSet}; + +use dashcore::blockdata::transaction::OutPoint; +use dashcore::{BlockHash, ScriptBuf, Transaction, Txid}; + +use crate::Address; + +use super::merge::Merge; + +// --------------------------------------------------------------------------- +// Top-level changeset +// --------------------------------------------------------------------------- + +/// Aggregated changeset covering every persistable wallet domain. +#[derive(Debug, Clone, Default, PartialEq)] +pub struct WalletChangeSet { + /// Changes to the chain tip the wallet is tracking. + pub chain: Option, + /// UTXO additions, spends, and lock-state changes. + pub utxos: Option, + /// New or updated transaction records. + pub transactions: Option, + /// Account-level changes (revealed indices, address usage). + pub accounts: Option, + /// Balance deltas since last persist. + pub balance: Option, +} + +impl Merge for WalletChangeSet { + fn merge(&mut self, other: Self) { + self.chain.merge(other.chain); + self.utxos.merge(other.utxos); + self.transactions.merge(other.transactions); + self.accounts.merge(other.accounts); + self.balance.merge(other.balance); + } + + fn is_empty(&self) -> bool { + self.chain.is_empty() + && self.utxos.is_empty() + && self.transactions.is_empty() + && self.accounts.is_empty() + && self.balance.is_empty() + } +} + +// --------------------------------------------------------------------------- +// Chain +// --------------------------------------------------------------------------- + +/// Tracks the latest chain tip the wallet has processed. +#[derive(Debug, Clone, Default, PartialEq)] +pub struct ChainChangeSet { + /// New best-known block height. + pub height: Option, + /// Hash of the block at `height`. + pub block_hash: Option, +} + +impl Merge for ChainChangeSet { + fn merge(&mut self, other: Self) { + if other.height.is_some() { + self.height = other.height; + } + if other.block_hash.is_some() { + self.block_hash = other.block_hash; + } + } + + fn is_empty(&self) -> bool { + self.height.is_none() && self.block_hash.is_none() + } +} + +// --------------------------------------------------------------------------- +// UTXOs +// --------------------------------------------------------------------------- + +/// Incremental UTXO changes. +#[derive(Debug, Clone, Default, PartialEq)] +pub struct UtxoChangeSet { + /// Newly discovered unspent outputs. + pub added: BTreeMap, + /// Outpoints that have been spent since last persist. + pub spent: BTreeSet, + /// Outpoints that received an InstantSend lock. + pub instant_locked: BTreeSet, +} + +impl Merge for UtxoChangeSet { + fn merge(&mut self, other: Self) { + self.added.merge(other.added); + self.spent.merge(other.spent); + self.instant_locked.merge(other.instant_locked); + } + + fn is_empty(&self) -> bool { + self.added.is_empty() && self.spent.is_empty() && self.instant_locked.is_empty() + } +} + +/// A single unspent transaction output entry. +#[derive(Debug, Clone, PartialEq)] +pub struct UtxoEntry { + /// The address this output pays to. + pub address: Address, + /// Value in satoshis. + pub value: u64, + /// The output script. + pub script_pubkey: ScriptBuf, + /// Whether this output has been InstantSend-locked. + pub is_instant_locked: bool, +} + +// --------------------------------------------------------------------------- +// Transactions +// --------------------------------------------------------------------------- + +/// Incremental transaction record changes. +#[derive(Debug, Clone, Default, PartialEq)] +pub struct TransactionChangeSet { + /// New or updated transaction records keyed by txid. + pub records: BTreeMap, +} + +impl Merge for TransactionChangeSet { + fn merge(&mut self, other: Self) { + self.records.merge(other.records); + } + + fn is_empty(&self) -> bool { + self.records.is_empty() + } +} + +/// Full record for a single transaction relevant to the wallet. +#[derive(Debug, Clone, PartialEq)] +pub struct TransactionEntry { + /// The raw transaction. + pub transaction: Transaction, + /// Block height the transaction was mined in (if confirmed). + pub block_height: Option, + /// Hash of the block the transaction was mined in (if confirmed). + pub block_hash: Option, + /// Unix timestamp (seconds) when first seen or confirmed. + pub timestamp: u64, + /// Net satoshi change from the wallet's perspective (positive = incoming). + pub net_amount: i64, + /// Fee paid (if known). + pub fee: Option, + /// User-supplied label. + pub label: Option, + /// Whether the transaction has an InstantSend lock. + pub is_instant_locked: bool, + /// Whether the transaction is covered by a ChainLock. + pub is_chain_locked: bool, +} + +// --------------------------------------------------------------------------- +// Accounts +// --------------------------------------------------------------------------- + +/// Account-level changes (address pools, revealed indices). +#[derive(Debug, Clone, Default, PartialEq)] +pub struct AccountChangeSet { + /// Maps `account_index` to the new highest-revealed address index. + pub last_revealed: BTreeMap, + /// Addresses that have been marked as used: `(account_index, address)`. + pub addresses_used: Vec<(u32, Address)>, +} + +impl Merge for AccountChangeSet { + fn merge(&mut self, other: Self) { + // For last_revealed, keep the higher value per account. + for (acct, idx) in other.last_revealed { + let entry = self.last_revealed.entry(acct).or_insert(0); + if idx > *entry { + *entry = idx; + } + } + self.addresses_used.merge(other.addresses_used); + } + + fn is_empty(&self) -> bool { + self.last_revealed.is_empty() && self.addresses_used.is_empty() + } +} + +// --------------------------------------------------------------------------- +// Balances +// --------------------------------------------------------------------------- + +/// Cumulative balance deltas since the last persist. +#[derive(Debug, Clone, Default, PartialEq)] +pub struct BalanceChangeSet { + /// Change in spendable (confirmed + InstantSend-locked) balance. + pub spendable_delta: i64, + /// Change in unconfirmed balance. + pub unconfirmed_delta: i64, + /// Change in immature (coinbase) balance. + pub immature_delta: i64, + /// Change in locked (frozen) balance. + pub locked_delta: i64, +} + +impl Merge for BalanceChangeSet { + fn merge(&mut self, other: Self) { + self.spendable_delta += other.spendable_delta; + self.unconfirmed_delta += other.unconfirmed_delta; + self.immature_delta += other.immature_delta; + self.locked_delta += other.locked_delta; + } + + fn is_empty(&self) -> bool { + self.spendable_delta == 0 + && self.unconfirmed_delta == 0 + && self.immature_delta == 0 + && self.locked_delta == 0 + } +} diff --git a/key-wallet/src/changeset/merge.rs b/key-wallet/src/changeset/merge.rs new file mode 100644 index 000000000..74ae8a249 --- /dev/null +++ b/key-wallet/src/changeset/merge.rs @@ -0,0 +1,190 @@ +//! Merge trait for composing incremental changesets. +//! +//! Types that implement [`Merge`] can accumulate successive deltas +//! and be inspected to see whether they carry any data. + +use std::collections::{BTreeMap, BTreeSet}; + +/// A type that can absorb another instance of itself, accumulating changes. +pub trait Merge: Default { + /// Merge `other` into `self`, consuming `other`. + fn merge(&mut self, other: Self); + + /// Returns `true` when the value is semantically empty (no changes). + fn is_empty(&self) -> bool; + + /// Take the contents out of `self` (leaving it empty/default) if it is + /// non-empty, otherwise return `None`. + fn take(&mut self) -> Option + where + Self: Sized, + { + if self.is_empty() { + None + } else { + Some(std::mem::take(self)) + } + } +} + +// --------------------------------------------------------------------------- +// Blanket / standard impls +// --------------------------------------------------------------------------- + +impl Merge for BTreeMap { + fn merge(&mut self, other: Self) { + self.extend(other); + } + + fn is_empty(&self) -> bool { + self.is_empty() + } +} + +impl Merge for BTreeSet { + fn merge(&mut self, other: Self) { + for item in other { + self.insert(item); + } + } + + fn is_empty(&self) -> bool { + self.is_empty() + } +} + +impl Merge for Option { + fn merge(&mut self, other: Self) { + match (self.as_mut(), other) { + (Some(existing), Some(other_val)) => existing.merge(other_val), + (None, Some(other_val)) => *self = Some(other_val), + _ => {} + } + } + + fn is_empty(&self) -> bool { + match self { + Some(inner) => inner.is_empty(), + None => true, + } + } +} + +impl Merge for Vec { + fn merge(&mut self, other: Self) { + self.extend(other); + } + + fn is_empty(&self) -> bool { + self.is_empty() + } +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn btreemap_merge_extends() { + let mut a: BTreeMap<&str, i32> = [("x", 1)].into_iter().collect(); + let b: BTreeMap<&str, i32> = [("y", 2), ("x", 3)].into_iter().collect(); + a.merge(b); + // "x" should be overwritten by the `other` value (BTreeMap::extend behaviour) + assert_eq!(a.get("x"), Some(&3)); + assert_eq!(a.get("y"), Some(&2)); + assert_eq!(a.len(), 2); + } + + #[test] + fn btreemap_is_empty() { + let empty: BTreeMap = BTreeMap::new(); + assert!(Merge::is_empty(&empty)); + + let non_empty: BTreeMap = [("a".into(), 1)].into_iter().collect(); + assert!(!Merge::is_empty(&non_empty)); + } + + #[test] + fn btreeset_merge_unions() { + let mut a: BTreeSet = [1, 2, 3].into_iter().collect(); + let b: BTreeSet = [3, 4, 5].into_iter().collect(); + a.merge(b); + assert_eq!(a, [1, 2, 3, 4, 5].into_iter().collect()); + } + + #[test] + fn btreeset_is_empty() { + let empty: BTreeSet = BTreeSet::new(); + assert!(Merge::is_empty(&empty)); + } + + #[test] + fn vec_merge_appends() { + let mut a = vec![1, 2]; + let b = vec![3, 4]; + a.merge(b); + assert_eq!(a, vec![1, 2, 3, 4]); + } + + #[test] + fn vec_is_empty() { + let empty: Vec = vec![]; + assert!(Merge::is_empty(&empty)); + let non_empty = vec![42]; + assert!(!Merge::is_empty(&non_empty)); + } + + #[test] + fn option_merge_some_into_none() { + let mut a: Option> = None; + let b: Option> = Some(vec![1]); + a.merge(b); + assert_eq!(a, Some(vec![1])); + } + + #[test] + fn option_merge_some_into_some() { + let mut a: Option> = Some(vec![1]); + let b: Option> = Some(vec![2]); + a.merge(b); + assert_eq!(a, Some(vec![1, 2])); + } + + #[test] + fn option_merge_none_into_some() { + let mut a: Option> = Some(vec![1]); + let b: Option> = None; + a.merge(b); + assert_eq!(a, Some(vec![1])); + } + + #[test] + fn option_is_empty() { + let none: Option> = None; + assert!(Merge::is_empty(&none)); + + let some_empty: Option> = Some(vec![]); + assert!(Merge::is_empty(&some_empty)); + + let some_non_empty: Option> = Some(vec![1]); + assert!(!Merge::is_empty(&some_non_empty)); + } + + #[test] + fn take_returns_none_when_empty() { + let mut v: Vec = vec![]; + assert_eq!(v.take(), None); + } + + #[test] + fn take_returns_some_and_resets() { + let mut v = vec![1, 2, 3]; + let taken = v.take(); + assert_eq!(taken, Some(vec![1, 2, 3])); + assert!(v.is_empty()); + } +} diff --git a/key-wallet/src/changeset/mod.rs b/key-wallet/src/changeset/mod.rs new file mode 100644 index 000000000..d9ca5fa5a --- /dev/null +++ b/key-wallet/src/changeset/mod.rs @@ -0,0 +1,17 @@ +//! Atomic changeset types for wallet state mutations. +//! +//! Every wallet mutation produces a [`WalletChangeSet`] capturing what changed. +//! ChangeSets are composable via the [`Merge`] trait — multiple deltas can be +//! batched before being applied or persisted. +//! +//! This module is about **atomicity and consistency**, not persistence. +//! Persistence is a separate layer that consumes changesets. + +mod changeset; +mod merge; + +pub use changeset::{ + AccountChangeSet, BalanceChangeSet, ChainChangeSet, TransactionChangeSet, TransactionEntry, + UtxoChangeSet, UtxoEntry, WalletChangeSet, +}; +pub use merge::Merge; diff --git a/key-wallet/src/lib.rs b/key-wallet/src/lib.rs index e12224ae4..8804d361d 100644 --- a/key-wallet/src/lib.rs +++ b/key-wallet/src/lib.rs @@ -26,6 +26,7 @@ pub mod account; pub mod bip32; #[cfg(feature = "bip38")] pub mod bip38; +pub mod changeset; pub mod derivation; #[cfg(feature = "bls")] pub mod derivation_bls_bip32; diff --git a/key-wallet/src/managed_account/address_pool.rs b/key-wallet/src/managed_account/address_pool.rs index 36fd1edf6..904607642 100644 --- a/key-wallet/src/managed_account/address_pool.rs +++ b/key-wallet/src/managed_account/address_pool.rs @@ -877,8 +877,13 @@ impl AddressPool { unused_count < self.gap_limit } - /// Generate addresses to maintain the gap limit - pub fn maintain_gap_limit(&mut self, key_source: &KeySource) -> Result> { + /// Generate addresses to maintain the gap limit. + /// Returns `(new_addresses, last_revealed_index)` where `last_revealed_index` is the + /// new highest generated index if addresses were created, `None` otherwise. + pub fn maintain_gap_limit( + &mut self, + key_source: &KeySource, + ) -> Result<(Vec
, Option)> { let target = match self.highest_used { None => self.gap_limit - 1, Some(highest) => highest + self.gap_limit, @@ -891,7 +896,13 @@ impl AddressPool { new_addresses.push(address); } - Ok(new_addresses) + let last_revealed = if !new_addresses.is_empty() { + self.highest_generated + } else { + None + }; + + Ok((new_addresses, last_revealed)) } /// Set a custom label for an address @@ -1217,7 +1228,7 @@ mod tests { assert_eq!(pool.addresses.len(), gap_limit as usize); // Calling maintain_gap_limit should not generate any new addresses when none are used - let new_addresses = pool.maintain_gap_limit(&key_source).unwrap(); + let (new_addresses, _last_revealed) = pool.maintain_gap_limit(&key_source).unwrap(); assert_eq!(new_addresses.len(), 0); assert_eq!(pool.highest_generated, Some(gap_limit - 1)); assert_eq!(pool.addresses.len(), gap_limit as usize); @@ -1227,7 +1238,7 @@ mod tests { assert_eq!(pool.highest_used, Some(0)); // Should generate exactly 1 address to maintain gap_limit unused after index 0 - let new_addresses = pool.maintain_gap_limit(&key_source).unwrap(); + let (new_addresses, _last_revealed) = pool.maintain_gap_limit(&key_source).unwrap(); assert_eq!(new_addresses.len(), 1); assert_eq!(pool.highest_generated, Some(gap_limit)); assert_eq!(pool.addresses.len(), gap_limit as usize + 1); @@ -1237,7 +1248,7 @@ mod tests { pool.mark_index_used(2); // Should generate exactly 2 more addresses - let new_addresses = pool.maintain_gap_limit(&key_source).unwrap(); + let (new_addresses, _last_revealed) = pool.maintain_gap_limit(&key_source).unwrap(); assert_eq!(new_addresses.len(), 2); assert_eq!(pool.highest_generated, Some(gap_limit + 2)); assert_eq!(pool.addresses.len(), gap_limit as usize + 3); diff --git a/key-wallet/src/managed_account/managed_platform_account.rs b/key-wallet/src/managed_account/managed_platform_account.rs index aa85ce5b1..e5b7728f9 100644 --- a/key-wallet/src/managed_account/managed_platform_account.rs +++ b/key-wallet/src/managed_account/managed_platform_account.rs @@ -292,6 +292,7 @@ impl ManagedPlatformAccount { pub fn maintain_gap_limit(&mut self, key_source: &KeySource) -> Result> { self.addresses .maintain_gap_limit(key_source) + .map(|(addrs, _last_revealed)| addrs) .map_err(|e| Error::InvalidParameter(format!("Failed to maintain gap limit: {}", e))) } diff --git a/key-wallet/src/managed_account/mod.rs b/key-wallet/src/managed_account/mod.rs index 529f81736..3e61b0b08 100644 --- a/key-wallet/src/managed_account/mod.rs +++ b/key-wallet/src/managed_account/mod.rs @@ -10,6 +10,11 @@ use crate::account::BLSAccount; use crate::account::EdDSAAccount; use crate::account::ManagedAccountTrait; use crate::account::TransactionRecord; +use crate::changeset::Merge; +use crate::changeset::{ + AccountChangeSet, BalanceChangeSet, TransactionChangeSet, TransactionEntry, UtxoChangeSet, + UtxoEntry, WalletChangeSet, +}; #[cfg(feature = "bls")] use crate::derivation_bls_bip32::ExtendedBLSPubKey; #[cfg(any(feature = "bls", feature = "eddsa"))] @@ -295,37 +300,57 @@ impl ManagedCoreAccount { } } - /// Mark an address as used - pub fn mark_address_used(&mut self, address: &Address) -> bool { - // Update metadata timestamp - self.metadata.last_used = Some(Self::current_timestamp()); + /// Compute the changeset for marking an address as used, without mutating state. + pub fn compute_mark_address_used(&self, address: &Address) -> AccountChangeSet { + let would_change = self.account_type.address_pools().iter().any(|pool| { + if let Some(&index) = pool.address_index.get(address) { + !pool.used_indices.contains(&index) + } else { + false + } + }); - // Use the account type's mark_address_used method - // The address pools already track gap limits internally - self.account_type.mark_address_used(address) + let mut account_changeset = AccountChangeSet::default(); + if would_change { + let account_index = self.index_or_default(); + account_changeset + .addresses_used + .push((account_index, address.clone())); + } + account_changeset } - /// Add new ones for received outputs, remove spent ones - fn update_utxos( - &mut self, + /// Mark an address as used. + /// Returns `(changed, account_changeset)` where `changed` indicates if the address was newly marked. + pub fn mark_address_used(&mut self, address: &Address) -> (bool, AccountChangeSet) { + let account_changeset = self.compute_mark_address_used(address); + let changed = !account_changeset.addresses_used.is_empty(); + + // Apply: update metadata and mark in pool + if changed { + self.metadata.last_used = Some(Self::current_timestamp()); + self.account_type.mark_address_used(address); + } + + (changed, account_changeset) + } + + /// Compute UTXO changes for a transaction without mutating state. + /// Returns a `UtxoChangeSet` describing additions, spends, and IS-locks. + pub(crate) fn compute_utxo_changes( + &self, tx: &Transaction, account_match: &AccountMatch, context: TransactionContext, - ) { - // Update UTXOs only for spendable account types - match &mut self.account_type { - ManagedAccountType::Standard { - .. - } - | ManagedAccountType::CoinJoin { - .. - } - | ManagedAccountType::DashpayReceivingFunds { - .. - } - | ManagedAccountType::DashpayExternalAccount { - .. - } => { + ) -> UtxoChangeSet { + let mut utxo_changeset = UtxoChangeSet::default(); + + // Only compute for spendable account types + match &self.account_type { + ManagedAccountType::Standard { .. } + | ManagedAccountType::CoinJoin { .. } + | ManagedAccountType::DashpayReceivingFunds { .. } + | ManagedAccountType::DashpayExternalAccount { .. } => { let involved_addrs: BTreeSet<_> = account_match .account_type_match .all_involved_addresses() @@ -334,9 +359,8 @@ impl ManagedCoreAccount { .collect(); let txid = tx.txid(); - let mut utxos_changed = false; - // Insert UTXOs for outputs paying to our addresses + // Determine outputs paying to our addresses for (vout, output) in tx.output.iter().enumerate() { if let Ok(addr) = Address::from_script(&output.script_pubkey, self.network) { if involved_addrs.contains(&addr) { @@ -346,12 +370,6 @@ impl ManagedCoreAccount { }; // Check if this outpoint was already spent by a transaction we've seen. - // This handles out-of-order block processing during rescan where a - // spending transaction at a higher height may be processed before - // the transaction that created the UTXO. - // TODO: This is mostly needed for wallet rescan from storage with the - // there is a timing issue with event processing which might lead to - // invalid UTXO set / balances. There might be a way around it. if self.is_outpoint_spent(&outpoint) { tracing::debug!( outpoint = %outpoint, @@ -360,60 +378,115 @@ impl ManagedCoreAccount { continue; } - let txout = dashcore::TxOut { - value: output.value, - script_pubkey: output.script_pubkey.clone(), - }; - let block_height = context.block_info().map_or(0, |info| info.height); - let mut utxo = - Utxo::new(outpoint, txout, addr, block_height, tx.is_coin_base()); - utxo.is_confirmed = context.confirmed(); - utxo.is_instantlocked = - matches!(context, TransactionContext::InstantSend); - self.utxos.insert(outpoint, utxo); - utxos_changed = true; + utxo_changeset.added.insert( + outpoint, + UtxoEntry { + address: addr, + value: output.value, + script_pubkey: output.script_pubkey.clone(), + is_instant_locked: matches!( + context, + TransactionContext::InstantSend + ), + }, + ); } } } - // Remove UTXOs spent by this transaction and track spent outpoints + // Determine UTXOs spent by this transaction for input in &tx.input { - self.spent_outpoints.insert(input.previous_output); - - if self.utxos.remove(&input.previous_output).is_some() { - tracing::debug!( - outpoint = %input.previous_output, - txid = %tx.txid(), - "Removed spent UTXO" - ); - utxos_changed = true; + if self.utxos.contains_key(&input.previous_output) { + utxo_changeset.spent.insert(input.previous_output); } } - - if utxos_changed { - self.monitor_revision += 1; - } } _ => {} } + + utxo_changeset } - /// Re-process an existing transaction with updated context (e.g., mempool→block confirmation) - /// and potentially new address matches from gap limit rescans. - pub(crate) fn confirm_transaction( + /// Apply a UTXO changeset to this account, mutating state. + pub(crate) fn apply_utxo_changes( &mut self, tx: &Transaction, + utxo_changeset: &UtxoChangeSet, + context: TransactionContext, + ) { + let mut utxos_changed = false; + + // Add new UTXOs + for (outpoint, entry) in &utxo_changeset.added { + let txout = dashcore::TxOut { + value: entry.value, + script_pubkey: entry.script_pubkey.clone(), + }; + let block_height = context.block_info().map_or(0, |info| info.height); + let mut utxo = Utxo::new( + *outpoint, + txout, + entry.address.clone(), + block_height, + tx.is_coin_base(), + ); + utxo.is_confirmed = context.confirmed(); + utxo.is_instantlocked = entry.is_instant_locked; + self.utxos.insert(*outpoint, utxo); + utxos_changed = true; + } + + // Remove spent UTXOs and track spent outpoints + for outpoint in &utxo_changeset.spent { + self.spent_outpoints.insert(*outpoint); + if self.utxos.remove(outpoint).is_some() { + tracing::debug!( + outpoint = %outpoint, + txid = %tx.txid(), + "Removed spent UTXO" + ); + utxos_changed = true; + } + } + + // Also track spent outpoints for inputs not in our UTXO set + // (needed for out-of-order processing) + for input in &tx.input { + self.spent_outpoints.insert(input.previous_output); + } + + // Apply IS-locks + for outpoint in &utxo_changeset.instant_locked { + if let Some(utxo) = self.utxos.get_mut(outpoint) { + utxo.is_instantlocked = true; + utxos_changed = true; + } + } + + if utxos_changed { + self.monitor_revision += 1; + } + } + + /// Compute the changeset for confirming an existing transaction, without mutating state. + /// If the transaction is not yet recorded, delegates to `compute_record_transaction`. + /// Returns `(changed, changeset)` where `changed` indicates if confirmation status would change. + pub(crate) fn compute_confirm_transaction( + &self, + tx: &Transaction, account_match: &AccountMatch, context: TransactionContext, transaction_type: TransactionType, - ) -> bool { + ) -> (bool, WalletChangeSet) { if !self.transactions.contains_key(&tx.txid()) { - self.record_transaction(tx, account_match, context, transaction_type); - return true; + let (_record, changeset) = + self.compute_record_transaction(tx, account_match, context, transaction_type); + return (true, changeset); } let mut changed = false; - if let Some(tx_record) = self.transactions.get_mut(&tx.txid()) { + let mut tx_changeset = TransactionChangeSet::default(); + if let Some(tx_record) = self.transactions.get(&tx.txid()) { debug_assert_eq!( tx_record.transaction_type, transaction_type, @@ -422,25 +495,110 @@ impl ManagedCoreAccount { ); if tx_record.context != context { let was_confirmed = tx_record.context.confirmed(); - tx_record.update_context(context); // Only signal a change when confirmation status actually changes, - // not for upgrades within the confirmed state (e.g. InBlock → InChainLockedBlock). - // TODO: emit a change event for InBlock → InChainLockedBlock once chainlock - // wallet transaction events are properly handled + // not for upgrades within the confirmed state (e.g. InBlock -> InChainLockedBlock). changed = !was_confirmed; + + if changed { + // Build the entry using the new context's block info + let block_info = context.block_info(); + tx_changeset.records.insert( + tx.txid(), + TransactionEntry { + transaction: tx.clone(), + block_height: block_info.map(|bi| bi.height), + block_hash: block_info.map(|bi| bi.block_hash), + timestamp: block_info.map_or(0, |bi| bi.timestamp as u64), + net_amount: tx_record.net_amount, + fee: None, + label: None, + is_instant_locked: context + == crate::transaction_checking::TransactionContext::InstantSend, + is_chain_locked: matches!( + context, + crate::transaction_checking::TransactionContext::InChainLockedBlock( + _ + ) + ), + }, + ); + } } } - self.update_utxos(tx, account_match, context); - changed + let utxo_changeset = self.compute_utxo_changes(tx, account_match, context); + + let changeset = WalletChangeSet { + transactions: if tx_changeset.is_empty() { + None + } else { + Some(tx_changeset) + }, + utxos: if utxo_changeset.is_empty() { + None + } else { + Some(utxo_changeset) + }, + ..Default::default() + }; + + (changed, changeset) } - /// Record a new transaction and update UTXOs for spendable account types - pub(crate) fn record_transaction( + /// Re-process an existing transaction with updated context (e.g., mempool->block confirmation) + /// and potentially new address matches from gap limit rescans. + /// Returns `(changed, changeset)` where `changed` indicates if confirmation status changed. + #[allow(dead_code)] + pub(crate) fn confirm_transaction( &mut self, tx: &Transaction, account_match: &AccountMatch, context: TransactionContext, transaction_type: TransactionType, + ) -> (bool, WalletChangeSet) { + let (changed, changeset) = + self.compute_confirm_transaction(tx, account_match, context, transaction_type); + + // Apply: update context on existing record + if let Some(tx_record) = self.transactions.get_mut(&tx.txid()) { + if tx_record.context != context { + tx_record.update_context(context); + } + } else { + // Transaction wasn't recorded yet -- record it now + // (compute_confirm_transaction already produced the full changeset) + } + + // Apply UTXO changes + if let Some(utxo_cs) = &changeset.utxos { + self.apply_utxo_changes(tx, utxo_cs, context); + } + + // If the transaction was new (not previously recorded), insert the record + if !self.transactions.contains_key(&tx.txid()) { + if let Some(tx_cs) = &changeset.transactions { + if let Some(entry) = tx_cs.records.get(&tx.txid()) { + let record = self.build_transaction_record( + tx, + account_match, + context, + transaction_type, + ); + self.transactions.insert(tx.txid(), record); + let _ = entry; // entry was used for changeset, record is for state + } + } + } + + (changed, changeset) + } + + /// Build a TransactionRecord from current state (read-only helper). + pub(crate) fn build_transaction_record( + &self, + tx: &Transaction, + account_match: &AccountMatch, + context: TransactionContext, + transaction_type: TransactionType, ) -> TransactionRecord { let net_amount = account_match.received as i64 - account_match.sent as i64; @@ -457,7 +615,7 @@ impl ManagedCoreAccount { .map(|info| &info.address) .collect(); - // Input details must be built before `update_utxos` removes spent UTXOs + // Build input details from current UTXO set (must be called before UTXOs are removed) let mut input_details = Vec::new(); if !tx.is_coin_base() { for (idx, input) in tx.input.iter().enumerate() { @@ -472,9 +630,7 @@ impl ManagedCoreAccount { } // Use both UTXO-based input details and `account_match.sent` as signals - // that we created this transaction. The UTXO set may be incomplete - // (e.g., partial rescan) so `account_match.sent > 0` catches cases where - // the transaction still spent our funds even without matching UTXOs. + // that we created this transaction. let has_inputs = !input_details.is_empty() || account_match.sent > 0; let resolved_outputs: Vec> = tx @@ -483,7 +639,7 @@ impl ManagedCoreAccount { .map(|output| Address::from_script(&output.script_pubkey, self.network).ok()) .collect(); - // Build output details — annotate every output with its role + // Build output details -- annotate every output with its role let mut output_details = Vec::new(); for (idx, output) in tx.output.iter().enumerate() { let role = match &resolved_outputs[idx] { @@ -522,7 +678,7 @@ impl ManagedCoreAccount { TransactionDirection::Incoming }; - let tx_record = TransactionRecord::new( + TransactionRecord::new( tx.clone(), context, transaction_type, @@ -530,34 +686,122 @@ impl ManagedCoreAccount { input_details, output_details, net_amount, - ); + ) + } + + /// Compute the changeset for recording a new transaction without mutating state. + /// Returns `(TransactionRecord, WalletChangeSet)`. + pub(crate) fn compute_record_transaction( + &self, + tx: &Transaction, + account_match: &AccountMatch, + context: TransactionContext, + transaction_type: TransactionType, + ) -> (TransactionRecord, WalletChangeSet) { + let record = self.build_transaction_record(tx, account_match, context, transaction_type); + + let utxo_changeset = self.compute_utxo_changes(tx, account_match, context); + + let net_amount = account_match.received as i64 - account_match.sent as i64; + let block_info = record.block_info(); + let tx_changeset = TransactionChangeSet { + records: std::iter::once(( + tx.txid(), + TransactionEntry { + transaction: tx.clone(), + block_height: block_info.map(|bi| bi.height), + block_hash: block_info.map(|bi| bi.block_hash), + timestamp: block_info.map_or(0, |bi| bi.timestamp as u64), + net_amount, + fee: None, + label: None, + is_instant_locked: context + == crate::transaction_checking::TransactionContext::InstantSend, + is_chain_locked: matches!( + context, + crate::transaction_checking::TransactionContext::InChainLockedBlock(_) + ), + }, + )) + .collect(), + }; + + let changeset = WalletChangeSet { + transactions: Some(tx_changeset), + utxos: if utxo_changeset.is_empty() { + None + } else { + Some(utxo_changeset) + }, + ..Default::default() + }; - let record = tx_record.clone(); - self.transactions.insert(tx.txid(), tx_record); + (record, changeset) + } - self.update_utxos(tx, account_match, context); - record + /// Record a new transaction and update UTXOs for spendable account types. + /// Returns `(TransactionRecord, WalletChangeSet)` capturing the mutation. + #[allow(dead_code)] + pub(crate) fn record_transaction( + &mut self, + tx: &Transaction, + account_match: &AccountMatch, + context: TransactionContext, + transaction_type: TransactionType, + ) -> (TransactionRecord, WalletChangeSet) { + let (record, changeset) = + self.compute_record_transaction(tx, account_match, context, transaction_type); + + // Apply: insert the transaction record + self.transactions.insert(tx.txid(), record.clone()); + + // Apply: update UTXOs + if let Some(utxo_cs) = &changeset.utxos { + self.apply_utxo_changes(tx, utxo_cs, context); + } else { + // Still need to track spent outpoints even when no UTXO changes + for input in &tx.input { + self.spent_outpoints.insert(input.previous_output); + } + } + + (record, changeset) } - /// Mark all UTXOs belonging to a transaction as InstantSend-locked. - /// Returns `true` if any UTXO was newly marked. - pub(crate) fn mark_utxos_instant_send(&mut self, txid: &Txid) -> bool { - let mut any_changed = false; - for utxo in self.utxos.values_mut() { + /// Compute the changeset for marking UTXOs as InstantSend-locked, without mutating state. + pub(crate) fn compute_instant_send_lock(&self, txid: &Txid) -> UtxoChangeSet { + let mut utxo_changeset = UtxoChangeSet::default(); + for utxo in self.utxos.values() { if utxo.outpoint.txid == *txid && !utxo.is_instantlocked { + utxo_changeset.instant_locked.insert(utxo.outpoint); + } + } + utxo_changeset + } + + /// Mark all UTXOs belonging to a transaction as InstantSend-locked. + /// Returns `(changed, utxo_changeset)` where `changed` indicates if any UTXO was newly marked. + pub(crate) fn mark_utxos_instant_send(&mut self, txid: &Txid) -> (bool, UtxoChangeSet) { + let utxo_changeset = self.compute_instant_send_lock(txid); + let any_changed = !utxo_changeset.instant_locked.is_empty(); + + // Apply: mark UTXOs + for outpoint in &utxo_changeset.instant_locked { + if let Some(utxo) = self.utxos.get_mut(outpoint) { utxo.is_instantlocked = true; - any_changed = true; } } - any_changed + + (any_changed, utxo_changeset) } - /// Update the account balance - pub fn update_balance(&mut self, synced_height: u32) { - let mut spendable = 0; - let mut unconfirmed = 0; - let mut immature = 0; - let mut locked = 0; + /// Compute the balance changeset from current UTXO state, without mutating. + pub fn compute_balance_update(&self, synced_height: u32) -> BalanceChangeSet { + let old = self.balance; + let mut spendable = 0u64; + let mut unconfirmed = 0u64; + let mut immature = 0u64; + let mut locked = 0u64; for utxo in self.utxos.values() { let value = utxo.txout.value; if utxo.is_locked { @@ -570,8 +814,25 @@ impl ManagedCoreAccount { unconfirmed += value; } } - self.balance = WalletCoreBalance::new(spendable, unconfirmed, immature, locked); + + BalanceChangeSet { + spendable_delta: spendable as i64 - old.spendable() as i64, + unconfirmed_delta: unconfirmed as i64 - old.unconfirmed() as i64, + immature_delta: immature as i64 - old.immature() as i64, + locked_delta: locked as i64 - old.locked() as i64, + } + } + + /// Update the account balance. + /// Returns a `BalanceChangeSet` with the delta from the previous balance. + pub fn update_balance(&mut self, synced_height: u32) -> BalanceChangeSet { + let balance_cs = self.compute_balance_update(synced_height); + + // Apply: set the new balance + self.balance.apply_delta(&balance_cs); self.metadata.last_used = Some(Self::current_timestamp()); + + balance_cs } /// Get all addresses from all pools @@ -1068,14 +1329,24 @@ impl ManagedCoreAccount { self.account_type.get_address_derivation_path(address) } - /// Get the current timestamp (for metadata) + /// Get the current timestamp (for metadata). fn current_timestamp() -> u64 { + Self::current_timestamp_static() + } + + /// Get the current timestamp as a static method (callable without self). + pub(crate) fn current_timestamp_static() -> u64 { std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .unwrap_or_default() .as_secs() } + /// Record a spent outpoint so that out-of-order UTXO processing is handled correctly. + pub(crate) fn record_spent_outpoint(&mut self, outpoint: OutPoint) { + self.spent_outpoints.insert(outpoint); + } + /// Get total address count across all pools pub fn total_address_count(&self) -> usize { self.account_type diff --git a/key-wallet/src/test_utils/wallet.rs b/key-wallet/src/test_utils/wallet.rs index 139bca640..b81b17fb6 100644 --- a/key-wallet/src/test_utils/wallet.rs +++ b/key-wallet/src/test_utils/wallet.rs @@ -2,7 +2,7 @@ use dashcore::{Address, Network, Transaction, Txid}; use crate::{ account::{ManagedCoreAccount, TransactionRecord}, - transaction_checking::{TransactionCheckResult, TransactionContext, WalletTransactionChecker}, + transaction_checking::{TransactionCheckResult, TransactionContext}, wallet::{initialization::WalletAccountCreationOptions, ManagedWalletInfo}, ExtendedPubKey, Utxo, Wallet, }; @@ -75,7 +75,7 @@ impl TestWalletContext { tx: &Transaction, context: TransactionContext, ) -> TransactionCheckResult { - self.managed_wallet.check_core_transaction(tx, context, &mut self.wallet, true, true).await + self.managed_wallet.check_core_transaction_with_wallet(tx, context, &self.wallet, true, true).await } /// Funds the wallet's receive address via a mempool transaction and diff --git a/key-wallet/src/tests/balance_tests.rs b/key-wallet/src/tests/balance_tests.rs index 4d6ed746c..9dc60656e 100644 --- a/key-wallet/src/tests/balance_tests.rs +++ b/key-wallet/src/tests/balance_tests.rs @@ -1,7 +1,6 @@ //! Tests for update_balance() UTXO categorization. use crate::managed_account::ManagedCoreAccount; -use crate::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; use crate::wallet::managed_wallet_info::ManagedWalletInfo; use crate::{Utxo, WalletCoreBalance}; diff --git a/key-wallet/src/tests/performance_tests.rs b/key-wallet/src/tests/performance_tests.rs index b952455f0..20f174df0 100644 --- a/key-wallet/src/tests/performance_tests.rs +++ b/key-wallet/src/tests/performance_tests.rs @@ -346,7 +346,7 @@ fn test_gap_limit_scan_performance() { // Scan for gap limit let start = Instant::now(); - pool.maintain_gap_limit(&key_source).unwrap(); + let _result = pool.maintain_gap_limit(&key_source).unwrap(); let elapsed = start.elapsed(); // Assert gap limit maintenance performance diff --git a/key-wallet/src/transaction_checking/account_checker.rs b/key-wallet/src/transaction_checking/account_checker.rs index a9750f1aa..c6affee57 100644 --- a/key-wallet/src/transaction_checking/account_checker.rs +++ b/key-wallet/src/transaction_checking/account_checker.rs @@ -7,6 +7,7 @@ use std::collections::BTreeMap; use super::transaction_router::AccountTypeToCheck; use crate::account::{ManagedAccountCollection, ManagedCoreAccount}; +use crate::changeset::WalletChangeSet; use crate::managed_account::address_pool::{AddressInfo, PublicKeyType}; use crate::managed_account::managed_account_type::ManagedAccountType; use crate::managed_account::transaction_record::TransactionRecord; @@ -48,6 +49,8 @@ pub struct TransactionCheckResult { pub new_addresses: Vec
, /// Transaction records created for new transactions, paired with their account index pub new_records: Vec<(u32, TransactionRecord)>, + /// Aggregated changeset capturing all mutations performed during this check. + pub changeset: WalletChangeSet, } /// Enum representing the type of Core account that matched with embedded data @@ -376,6 +379,7 @@ impl ManagedAccountCollection { total_received_for_credit_conversion: 0, new_addresses: Vec::new(), new_records: Vec::new(), + changeset: WalletChangeSet::default(), }; for account_type in account_types { diff --git a/key-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rs b/key-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rs index 6371710b1..891a924db 100644 --- a/key-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rs +++ b/key-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rs @@ -5,7 +5,7 @@ use crate::test_utils::TestWalletContext; use crate::transaction_checking::transaction_router::{ AccountTypeToCheck, TransactionRouter, TransactionType, }; -use crate::transaction_checking::{BlockInfo, TransactionContext, WalletTransactionChecker}; +use crate::transaction_checking::{BlockInfo, TransactionContext}; use dashcore::blockdata::transaction::special_transaction::asset_unlock::qualified_asset_unlock::AssetUnlockPayload; use dashcore::blockdata::transaction::special_transaction::asset_unlock::request_info::AssetUnlockRequestInfo; use dashcore::blockdata::transaction::special_transaction::asset_unlock::unqualified_asset_unlock::AssetUnlockBasePayload; @@ -71,7 +71,7 @@ fn test_asset_unlock_classification() { async fn test_asset_unlock_transaction_routing() { let TestWalletContext { managed_wallet: mut managed_wallet_info, - mut wallet, + wallet, receive_address: address, .. } = TestWalletContext::new_random(); @@ -116,7 +116,7 @@ async fn test_asset_unlock_transaction_routing() { )); let result = - managed_wallet_info.check_core_transaction(&tx, context, &mut wallet, true, true).await; + managed_wallet_info.check_core_transaction_with_wallet(&tx, context, &wallet, true, true).await; // The transaction should be recognized as relevant assert!(result.is_relevant, "Asset unlock transaction should be recognized as relevant"); @@ -141,7 +141,7 @@ async fn test_asset_unlock_transaction_routing() { async fn test_asset_unlock_routing_to_bip32_account() { let TestWalletContext { managed_wallet: mut managed_wallet_info, - mut wallet, + wallet, receive_address: address, .. } = TestWalletContext::new_random(); @@ -178,7 +178,7 @@ async fn test_asset_unlock_routing_to_bip32_account() { )); let result = - managed_wallet_info.check_core_transaction(&tx, context, &mut wallet, true, true).await; + managed_wallet_info.check_core_transaction_with_wallet(&tx, context, &wallet, true, true).await; // Should be recognized as relevant assert!(result.is_relevant, "Asset unlock transaction to BIP32 account should be relevant"); diff --git a/key-wallet/src/transaction_checking/transaction_router/tests/coinbase.rs b/key-wallet/src/transaction_checking/transaction_router/tests/coinbase.rs index 57a69fcd4..2f95aac92 100644 --- a/key-wallet/src/transaction_checking/transaction_router/tests/coinbase.rs +++ b/key-wallet/src/transaction_checking/transaction_router/tests/coinbase.rs @@ -5,7 +5,7 @@ use crate::test_utils::TestWalletContext; use crate::transaction_checking::transaction_router::{ AccountTypeToCheck, TransactionRouter, TransactionType, }; -use crate::transaction_checking::{BlockInfo, TransactionContext, WalletTransactionChecker}; +use crate::transaction_checking::{BlockInfo, TransactionContext}; use dashcore::blockdata::transaction::special_transaction::coinbase::CoinbasePayload; use dashcore::blockdata::transaction::special_transaction::TransactionPayload; use dashcore::bls_sig_utils::BLSSignature; @@ -41,7 +41,7 @@ fn create_coinbase_transaction() -> Transaction { async fn test_coinbase_transaction_routing_to_bip44_receive_address() { let TestWalletContext { managed_wallet: mut managed_wallet_info, - mut wallet, + wallet, receive_address, .. } = TestWalletContext::new_random(); @@ -65,10 +65,10 @@ async fn test_coinbase_transaction_routing_to_bip44_receive_address() { // Check the coinbase transaction let result = managed_wallet_info - .check_core_transaction( + .check_core_transaction_with_wallet( &coinbase_tx, context, - &mut wallet, + &wallet, true, // update state true, // update balance ) @@ -97,7 +97,7 @@ async fn test_coinbase_transaction_routing_to_bip44_receive_address() { async fn test_coinbase_transaction_routing_to_bip44_change_address() { let TestWalletContext { managed_wallet: mut managed_wallet_info, - mut wallet, + wallet, xpub, .. } = TestWalletContext::new_random(); @@ -128,10 +128,10 @@ async fn test_coinbase_transaction_routing_to_bip44_change_address() { // Check the coinbase transaction let result = managed_wallet_info - .check_core_transaction( + .check_core_transaction_with_wallet( &coinbase_tx, context, - &mut wallet, + &wallet, true, // update state true, // update balance ) @@ -160,7 +160,7 @@ async fn test_coinbase_transaction_routing_to_bip44_change_address() { async fn test_update_state_flag_behavior() { let TestWalletContext { managed_wallet: mut managed_wallet_info, - mut wallet, + wallet, receive_address: address, .. } = TestWalletContext::new_random(); @@ -189,7 +189,7 @@ async fn test_update_state_flag_behavior() { // First check with update_state = false let result1 = - managed_wallet_info.check_core_transaction(&tx, context, &mut wallet, false, true).await; + managed_wallet_info.check_core_transaction_with_wallet(&tx, context, &wallet, false, true).await; assert!(result1.is_relevant); @@ -212,10 +212,10 @@ async fn test_update_state_flag_behavior() { // Now check with update_state = true let result2 = managed_wallet_info - .check_core_transaction( + .check_core_transaction_with_wallet( &tx, context, - &mut wallet, + &wallet, true, // update state true, // update balance ) @@ -286,7 +286,7 @@ fn test_coinbase_routing() { async fn test_coinbase_transaction_with_payload_routing() { let TestWalletContext { managed_wallet: mut managed_wallet_info, - mut wallet, + wallet, receive_address: address, .. } = TestWalletContext::new_random(); @@ -322,7 +322,7 @@ async fn test_coinbase_transaction_with_payload_routing() { )); let result = managed_wallet_info - .check_core_transaction(&coinbase_tx, context, &mut wallet, true, true) + .check_core_transaction_with_wallet(&coinbase_tx, context, &wallet, true, true) .await; assert!(result.is_relevant, "Coinbase with payload should be relevant"); diff --git a/key-wallet/src/transaction_checking/transaction_router/tests/identity_transactions.rs b/key-wallet/src/transaction_checking/transaction_router/tests/identity_transactions.rs index adcbd8717..bf13becb4 100644 --- a/key-wallet/src/transaction_checking/transaction_router/tests/identity_transactions.rs +++ b/key-wallet/src/transaction_checking/transaction_router/tests/identity_transactions.rs @@ -5,7 +5,7 @@ use crate::account::AccountType; use crate::transaction_checking::transaction_router::{ AccountTypeToCheck, TransactionRouter, TransactionType, }; -use crate::transaction_checking::{TransactionContext, WalletTransactionChecker}; +use crate::transaction_checking::TransactionContext; use crate::wallet::initialization::WalletAccountCreationOptions; use crate::wallet::{ManagedWalletInfo, Wallet}; use crate::Network; @@ -118,7 +118,7 @@ async fn test_identity_registration_account_routing() { // First check without updating state let result = - managed_wallet_info.check_core_transaction(&tx, context, &mut wallet, true, true).await; + managed_wallet_info.check_core_transaction_with_wallet(&tx, context, &wallet, true, true).await; println!( "Identity registration transaction result: is_relevant={}, received={}, credit_conversion={}", @@ -152,7 +152,7 @@ async fn test_identity_registration_account_routing() { async fn test_normal_payment_to_identity_address_not_detected() { let network = Network::Testnet; - let mut wallet = Wallet::new_random(network, WalletAccountCreationOptions::Default) + let wallet = Wallet::new_random(network, WalletAccountCreationOptions::Default) .expect("Failed to create wallet with default options"); let mut managed_wallet_info = ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string()); @@ -189,10 +189,10 @@ async fn test_normal_payment_to_identity_address_not_detected() { let context = TransactionContext::InBlock(test_block_info(100000)); let result = managed_wallet_info - .check_core_transaction( + .check_core_transaction_with_wallet( &normal_tx, context, - &mut wallet, + &wallet, true, // update state true, // update balance ) diff --git a/key-wallet/src/transaction_checking/transaction_router/tests/provider.rs b/key-wallet/src/transaction_checking/transaction_router/tests/provider.rs index e257b8538..c870440fd 100644 --- a/key-wallet/src/transaction_checking/transaction_router/tests/provider.rs +++ b/key-wallet/src/transaction_checking/transaction_router/tests/provider.rs @@ -4,7 +4,7 @@ use super::helpers::*; use crate::transaction_checking::transaction_router::{ AccountTypeToCheck, TransactionRouter, TransactionType, }; -use crate::transaction_checking::{BlockInfo, TransactionContext, WalletTransactionChecker}; +use crate::transaction_checking::{BlockInfo, TransactionContext}; use crate::wallet::initialization::WalletAccountCreationOptions; use crate::wallet::{ManagedWalletInfo, Wallet}; use crate::Network; @@ -132,7 +132,7 @@ async fn test_provider_registration_transaction_routing_check_owner_only() { let other_wallet = Wallet::new_random(network, WalletAccountCreationOptions::Default) .expect("Failed to create wallet with default options"); - let mut wallet = Wallet::new_random(network, WalletAccountCreationOptions::Default) + let wallet = Wallet::new_random(network, WalletAccountCreationOptions::Default) .expect("Failed to create wallet with default options"); let mut other_managed_wallet_info = @@ -232,7 +232,7 @@ async fn test_provider_registration_transaction_routing_check_owner_only() { )); let result = - managed_wallet_info.check_core_transaction(&tx, context, &mut wallet, true, true).await; + managed_wallet_info.check_core_transaction_with_wallet(&tx, context, &wallet, true, true).await; println!( "Provider registration transaction result: is_relevant={}, received={}", @@ -267,7 +267,7 @@ async fn test_provider_registration_transaction_routing_check_voting_only() { let other_wallet = Wallet::new_random(network, WalletAccountCreationOptions::Default) .expect("Failed to create wallet with default options"); - let mut wallet = Wallet::new_random(network, WalletAccountCreationOptions::Default) + let wallet = Wallet::new_random(network, WalletAccountCreationOptions::Default) .expect("Failed to create wallet with default options"); let mut other_managed_wallet_info = @@ -367,7 +367,7 @@ async fn test_provider_registration_transaction_routing_check_voting_only() { )); let result = - managed_wallet_info.check_core_transaction(&tx, context, &mut wallet, true, true).await; + managed_wallet_info.check_core_transaction_with_wallet(&tx, context, &wallet, true, true).await; println!( "Provider registration transaction result (voting): is_relevant={}, received={}", @@ -402,7 +402,7 @@ async fn test_provider_registration_transaction_routing_check_operator_only() { let other_wallet = Wallet::new_random(network, WalletAccountCreationOptions::Default) .expect("Failed to create wallet with default options"); - let mut wallet = Wallet::new_random(network, WalletAccountCreationOptions::Default) + let wallet = Wallet::new_random(network, WalletAccountCreationOptions::Default) .expect("Failed to create wallet with default options"); let mut other_managed_wallet_info = @@ -503,7 +503,7 @@ async fn test_provider_registration_transaction_routing_check_operator_only() { )); let result = - managed_wallet_info.check_core_transaction(&tx, context, &mut wallet, true, true).await; + managed_wallet_info.check_core_transaction_with_wallet(&tx, context, &wallet, true, true).await; println!( "Provider registration transaction result (operator): is_relevant={}, received={}", @@ -583,7 +583,7 @@ async fn test_provider_registration_transaction_routing_check_platform_only() { let other_wallet = Wallet::new_random(network, WalletAccountCreationOptions::Default) .expect("Failed to create wallet with default options"); - let mut wallet = Wallet::new_random(network, WalletAccountCreationOptions::Default) + let wallet = Wallet::new_random(network, WalletAccountCreationOptions::Default) .expect("Failed to create wallet with default options"); let mut other_managed_wallet_info = @@ -706,7 +706,7 @@ async fn test_provider_registration_transaction_routing_check_platform_only() { )); let result = - managed_wallet_info.check_core_transaction(&tx, context, &mut wallet, true, true).await; + managed_wallet_info.check_core_transaction_with_wallet(&tx, context, &wallet, true, true).await; println!( "Provider registration transaction result (platform): is_relevant={}, received={}", @@ -781,7 +781,7 @@ fn test_provider_update_service_with_operator_key() { #[tokio::test] async fn test_provider_update_registrar_with_voting_and_operator() { // Test provider update registrar classification and routing - let mut wallet = Wallet::new_random(Network::Testnet, WalletAccountCreationOptions::Default) + let wallet = Wallet::new_random(Network::Testnet, WalletAccountCreationOptions::Default) .expect("Failed to create wallet with default options"); let mut managed_wallet_info = @@ -830,7 +830,7 @@ async fn test_provider_update_registrar_with_voting_and_operator() { )); let result = - managed_wallet_info.check_core_transaction(&tx, context, &mut wallet, true, true).await; + managed_wallet_info.check_core_transaction_with_wallet(&tx, context, &wallet, true, true).await; // Should be recognized as relevant due to voting and operator keys assert!(result.is_relevant, "Provider update registrar should be relevant"); @@ -854,7 +854,7 @@ async fn test_provider_update_registrar_with_voting_and_operator() { #[tokio::test] async fn test_provider_revocation_classification_and_routing() { // Test that provider revocation transactions are properly classified and routed - let mut wallet = Wallet::new_random(Network::Testnet, WalletAccountCreationOptions::Default) + let wallet = Wallet::new_random(Network::Testnet, WalletAccountCreationOptions::Default) .expect("Failed to create wallet with default options"); let mut managed_wallet_info = @@ -925,7 +925,7 @@ async fn test_provider_revocation_classification_and_routing() { )); let result = - managed_wallet_info.check_core_transaction(&tx, context, &mut wallet, true, true).await; + managed_wallet_info.check_core_transaction_with_wallet(&tx, context, &wallet, true, true).await; // Should be recognized as relevant due to collateral return assert!(result.is_relevant, "Provider revocation with collateral return should be relevant"); diff --git a/key-wallet/src/transaction_checking/transaction_router/tests/routing.rs b/key-wallet/src/transaction_checking/transaction_router/tests/routing.rs index ac9aabda9..ed401cb5c 100644 --- a/key-wallet/src/transaction_checking/transaction_router/tests/routing.rs +++ b/key-wallet/src/transaction_checking/transaction_router/tests/routing.rs @@ -8,7 +8,7 @@ use crate::test_utils::TestWalletContext; use crate::transaction_checking::transaction_router::{ AccountTypeToCheck, TransactionRouter, TransactionType, }; -use crate::transaction_checking::{TransactionContext, WalletTransactionChecker}; +use crate::transaction_checking::TransactionContext; use crate::wallet::initialization::WalletAccountCreationOptions; use crate::wallet::{ManagedWalletInfo, Wallet}; use crate::Network; @@ -28,7 +28,7 @@ fn test_standard_transaction_routing() { async fn test_transaction_routing_to_bip44_account() { let TestWalletContext { managed_wallet: mut managed_wallet_info, - mut wallet, + wallet, receive_address: address, .. } = TestWalletContext::new_random(); @@ -48,10 +48,10 @@ async fn test_transaction_routing_to_bip44_account() { // Check the transaction using the managed wallet info let result = managed_wallet_info - .check_core_transaction( + .check_core_transaction_with_wallet( &tx, context, - &mut wallet, + &wallet, true, // update state true, // update balance ) @@ -112,7 +112,7 @@ async fn test_transaction_routing_to_bip32_account() { // Check with update_state = false let result = - managed_wallet_info.check_core_transaction(&tx, context, &mut wallet, false, true).await; + managed_wallet_info.check_core_transaction_with_wallet(&tx, context, &wallet, false, true).await; // The transaction should be recognized as relevant assert!(result.is_relevant, "Transaction should be relevant to the BIP32 account"); @@ -132,10 +132,10 @@ async fn test_transaction_routing_to_bip32_account() { // Now check with update_state = true let result = managed_wallet_info - .check_core_transaction( + .check_core_transaction_with_wallet( &tx, context, - &mut wallet, + &wallet, true, // update state true, // update balance ) @@ -227,7 +227,7 @@ async fn test_transaction_routing_to_coinjoin_account() { let context = TransactionContext::InBlock(test_block_info(100000)); let result = - managed_wallet_info.check_core_transaction(&tx, context, &mut wallet, true, true).await; + managed_wallet_info.check_core_transaction_with_wallet(&tx, context, &wallet, true, true).await; // This test may fail if CoinJoin detection is not properly implemented println!( @@ -325,10 +325,10 @@ async fn test_transaction_affects_multiple_accounts() { // Check the transaction let result = managed_wallet_info - .check_core_transaction( + .check_core_transaction_with_wallet( &tx, context, - &mut wallet, + &wallet, true, // update state true, // update balance ) @@ -348,7 +348,7 @@ async fn test_transaction_affects_multiple_accounts() { // Test with update_state = false to ensure state isn't modified let result2 = - managed_wallet_info.check_core_transaction(&tx, context, &mut wallet, false, true).await; + managed_wallet_info.check_core_transaction_with_wallet(&tx, context, &wallet, false, true).await; assert_eq!( result2.total_received, result.total_received, diff --git a/key-wallet/src/transaction_checking/wallet_checker.rs b/key-wallet/src/transaction_checking/wallet_checker.rs index e00c8f647..158040bcd 100644 --- a/key-wallet/src/transaction_checking/wallet_checker.rs +++ b/key-wallet/src/transaction_checking/wallet_checker.rs @@ -6,20 +6,25 @@ pub(crate) use super::account_checker::TransactionCheckResult; use super::transaction_context::TransactionContext; use super::transaction_router::TransactionRouter; -use crate::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; +use crate::changeset::{AccountChangeSet, Merge, UtxoChangeSet}; use crate::wallet::managed_wallet_info::ManagedWalletInfo; use crate::{KeySource, Wallet}; use async_trait::async_trait; use dashcore::blockdata::transaction::Transaction; -/// Extension trait for ManagedWalletInfo to add transaction checking capabilities +/// Extension trait for wallet state types that can check transactions. +/// +/// Implementors combine wallet info (accounts, UTXOs, balances) with a +/// reference to the immutable `Wallet` (keys, xpubs) so that gap-limit +/// maintenance can derive new addresses during transaction processing. +/// +/// The `wallet` parameter has been removed from the trait method because +/// implementors of this trait (e.g. `ManagedWalletState`) own both the +/// `Wallet` and the `ManagedWalletInfo` and can access the wallet +/// internally. #[async_trait] pub trait WalletTransactionChecker { - /// Check if a transaction belongs to this wallet with optimized routing - /// Only checks relevant account types based on transaction type - /// - /// The mutable wallet reference is required to support address generation and potential - /// platform queries (e.g., for DashPay transactions). + /// Check if a transaction belongs to this wallet with optimized routing. /// /// If `update_state` is true, updates account state (transactions, UTXOs, balances, addresses). /// If `update_state` is false, only checks relevance without modifying state (useful for previews). @@ -29,24 +34,28 @@ pub trait WalletTransactionChecker { /// and refresh once at the end via `update_synced_height`. /// /// The context parameter indicates where the transaction comes from (mempool, block, etc.) - /// async fn check_core_transaction( &mut self, tx: &Transaction, context: TransactionContext, - wallet: &mut Wallet, update_state: bool, update_balance: bool, ) -> TransactionCheckResult; } -#[async_trait] -impl WalletTransactionChecker for ManagedWalletInfo { - async fn check_core_transaction( +impl ManagedWalletInfo { + /// Check a transaction against this wallet info using the provided wallet + /// for gap-limit maintenance (xpub lookups). + /// + /// This is the core implementation extracted from the former + /// `WalletTransactionChecker` impl on `ManagedWalletInfo`. + /// Composite types like `ManagedWalletState` call this method, passing + /// `&self.wallet`, and then persist any resulting changeset. + pub async fn check_core_transaction_with_wallet( &mut self, tx: &Transaction, context: TransactionContext, - wallet: &mut Wallet, + wallet: &Wallet, update_state: bool, update_balance: bool, ) -> TransactionCheckResult { @@ -56,14 +65,14 @@ impl WalletTransactionChecker for ManagedWalletInfo { // Get relevant account types for this transaction type let relevant_types = TransactionRouter::get_relevant_account_types(&tx_type); - // Check only relevant account types + // Check only relevant account types (read-only) let mut result = self.accounts.check_transaction(tx, &relevant_types); if !update_state || !result.is_relevant { return result; } - // Check if this transaction already exists in any affected account + // Check if this transaction already exists in any affected account (read-only) let txid = tx.txid(); let mut is_new = true; for account_match in &result.affected_accounts { @@ -79,7 +88,7 @@ impl WalletTransactionChecker for ManagedWalletInfo { result.is_new_transaction = is_new; if !is_new { - // IS lock on a transaction that is already confirmed is stale — ignore + // IS lock on a transaction that is already confirmed is stale -- ignore if context == TransactionContext::InstantSend { if !self.instant_send_locks.insert(txid) { return result; @@ -94,17 +103,36 @@ impl WalletTransactionChecker for ManagedWalletInfo { if already_confirmed { return result; } - // Mark UTXOs as IS-locked in affected accounts + // Phase 1 (compute): compute IS-lock changesets from each account + let mut is_utxo_cs = UtxoChangeSet::default(); + for account_match in &result.affected_accounts { + if let Some(account) = self + .accounts + .get_by_account_type_match(&account_match.account_type_match) + { + let utxo_cs = account.compute_instant_send_lock(&txid); + is_utxo_cs.merge(utxo_cs); + } + } + // Phase 2 (apply): apply IS-lock changes for account_match in &result.affected_accounts { if let Some(account) = self .accounts .get_by_account_type_match_mut(&account_match.account_type_match) { - account.mark_utxos_instant_send(&txid); + for outpoint in &is_utxo_cs.instant_locked { + if let Some(utxo) = account.utxos.get_mut(outpoint) { + utxo.is_instantlocked = true; + } + } } } if update_balance { - self.update_balance(); + let balance_cs = self.update_balance_with_changeset(); + result.changeset.balance.merge(Some(balance_cs)); + } + if !is_utxo_cs.is_empty() { + result.changeset.utxos.merge(Some(is_utxo_cs)); } result.state_modified = true; return result; @@ -115,8 +143,57 @@ impl WalletTransactionChecker for ManagedWalletInfo { } } - // Process each affected account - for account_match in result.affected_accounts.clone() { + // --------------------------------------------------------------- + // Phase 1 (compute): build changesets from each affected account + // without mutating any account state. + // --------------------------------------------------------------- + struct AccountComputeResult { + account_match_idx: usize, + record: Option, + sub_changeset: crate::changeset::WalletChangeSet, + changed: bool, + addr_changesets: Vec, + } + + let mut compute_results = Vec::new(); + + for (idx, account_match) in result.affected_accounts.iter().enumerate() { + let Some(account) = + self.accounts.get_by_account_type_match(&account_match.account_type_match) + else { + continue; + }; + + let (record, sub_cs, changed) = if is_new { + let (record, sub_cs) = + account.compute_record_transaction(tx, account_match, context, tx_type); + (Some(record), sub_cs, true) + } else { + let (changed, sub_cs) = + account.compute_confirm_transaction(tx, account_match, context, tx_type); + (None, sub_cs, changed) + }; + + let mut addr_changesets = Vec::new(); + for address_info in account_match.account_type_match.all_involved_addresses() { + let addr_cs = account.compute_mark_address_used(&address_info.address); + addr_changesets.push(addr_cs); + } + + compute_results.push(AccountComputeResult { + account_match_idx: idx, + record, + sub_changeset: sub_cs, + changed, + addr_changesets, + }); + } + + // --------------------------------------------------------------- + // Phase 2 (apply): apply all computed changesets to mutable state. + // --------------------------------------------------------------- + for cr in compute_results { + let account_match = &result.affected_accounts[cr.account_match_idx]; let Some(account) = self.accounts.get_by_account_type_match_mut(&account_match.account_type_match) else { @@ -124,19 +201,65 @@ impl WalletTransactionChecker for ManagedWalletInfo { }; if is_new { - let record = account.record_transaction(tx, &account_match, context, tx_type); - if let Some(account_index) = account_match.account_type_match.account_index() { - result.new_records.push((account_index, record)); + if let Some(ref record) = cr.record { + // Apply: insert transaction record + account.transactions.insert(tx.txid(), record.clone()); + // Apply: update UTXOs + if let Some(utxo_cs) = &cr.sub_changeset.utxos { + account.apply_utxo_changes(tx, utxo_cs, context); + } else { + for input in &tx.input { + account.record_spent_outpoint(input.previous_output); + } + } + if let Some(account_index) = account_match.account_type_match.account_index() { + result.new_records.push((account_index, record.clone())); + } } result.state_modified = true; - } else if account.confirm_transaction(tx, &account_match, context, tx_type) { - result.state_modified = true; + } else { + // Apply: update context on existing record + if let Some(tx_record) = account.transactions.get_mut(&tx.txid()) { + if tx_record.context != context { + tx_record.update_context(context); + } + } else if !account.transactions.contains_key(&tx.txid()) { + // Transaction was not recorded yet -- insert from changeset + if let Some(ref record) = cr.record { + account.transactions.insert(tx.txid(), record.clone()); + } else { + let record = account.build_transaction_record( + tx, + account_match, + context, + tx_type, + ); + account.transactions.insert(tx.txid(), record); + } + } + // Apply: update UTXOs + if let Some(utxo_cs) = &cr.sub_changeset.utxos { + account.apply_utxo_changes(tx, utxo_cs, context); + } + if cr.changed { + result.state_modified = true; + } } - for address_info in account_match.account_type_match.all_involved_addresses() { - account.mark_address_used(&address_info.address); + // Apply: mark addresses as used + for addr_cs in &cr.addr_changesets { + for (_acct_idx, address) in &addr_cs.addresses_used { + account.metadata.last_used = + Some(crate::managed_account::ManagedCoreAccount::current_timestamp_static()); + account.account_type.mark_address_used(address); + } + result.changeset.accounts.merge(Some(addr_cs.clone())); } + // Merge the sub-changeset into the result + result.changeset.merge(cr.sub_changeset); + + // Gap limit maintenance (inherently mutable -- generates new addresses) let Some(xpub) = wallet.extended_public_key_for_account_type( &account_match.account_type_match.to_account_type_to_check(), account_match.account_type_match.account_index(), @@ -146,9 +269,17 @@ impl WalletTransactionChecker for ManagedWalletInfo { let key_source = KeySource::Public(xpub); let rev_before = result.new_addresses.len(); + let account_index = account.index_or_default(); for pool in account.account_type.address_pools_mut() { match pool.maintain_gap_limit(&key_source) { - Ok(addrs) => result.new_addresses.extend(addrs), + Ok((addrs, last_revealed)) => { + result.new_addresses.extend(addrs); + if let Some(idx) = last_revealed { + let mut acct_cs = AccountChangeSet::default(); + acct_cs.last_revealed.insert(account_index, idx); + result.changeset.accounts.merge(Some(acct_cs)); + } + } Err(e) => { tracing::error!( account_index = ?account_match.account_type_match.account_index(), @@ -183,7 +314,8 @@ impl WalletTransactionChecker for ManagedWalletInfo { } if update_balance { - self.update_balance(); + let balance_cs = self.update_balance_with_changeset(); + result.changeset.balance.merge(Some(balance_cs)); } result @@ -198,7 +330,6 @@ mod tests { use crate::transaction_checking::BlockInfo; use crate::transaction_checking::TransactionType; use crate::wallet::initialization::WalletAccountCreationOptions; - use crate::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; use crate::wallet::{ManagedWalletInfo, Wallet}; use crate::Network; use dashcore::blockdata::script::ScriptBuf; @@ -229,9 +360,8 @@ mod tests { let context = TransactionContext::Mempool; - let mut wallet_mut = wallet; let result = - managed_wallet.check_core_transaction(&tx, context, &mut wallet_mut, true, true).await; + managed_wallet.check_core_transaction_with_wallet(&tx, context, &wallet, true, true).await; // Should return default result with no relevance assert!(!result.is_relevant); @@ -310,7 +440,7 @@ mod tests { // This should exercise BIP32 account branch in the update logic let result = - managed_wallet.check_core_transaction(&tx, context, &mut wallet, true, true).await; + managed_wallet.check_core_transaction_with_wallet(&tx, context, &wallet, true, true).await; // Should be relevant since it's our address assert!(result.is_relevant); @@ -345,7 +475,7 @@ mod tests { // This should exercise CoinJoin account branch in the update logic let result = - managed_wallet.check_core_transaction(&tx, context, &mut wallet, true, true).await; + managed_wallet.check_core_transaction_with_wallet(&tx, context, &wallet, true, true).await; // Since this is not a coinjoin looking transaction, we should not pick up on it. assert!(!result.is_relevant); @@ -358,7 +488,7 @@ mod tests { async fn test_wallet_checker_coinbase_immature_handling() { let TestWalletContext { mut managed_wallet, - mut wallet, + wallet, receive_address: address, .. } = TestWalletContext::new_random(); @@ -393,7 +523,7 @@ mod tests { )); let result = managed_wallet - .check_core_transaction(&coinbase_tx, context, &mut wallet, true, true) + .check_core_transaction_with_wallet(&coinbase_tx, context, &wallet, true, true) .await; // Set synced_height to block where coinbase was received to trigger balance updates. managed_wallet.update_synced_height(block_height); @@ -434,7 +564,7 @@ mod tests { async fn test_wallet_checker_detects_spend_only_transaction() { let TestWalletContext { mut managed_wallet, - mut wallet, + wallet, receive_address, .. } = TestWalletContext::new_random(); @@ -449,7 +579,7 @@ mod tests { )); let funding_result = managed_wallet - .check_core_transaction(&funding_tx, funding_context, &mut wallet, true, true) + .check_core_transaction_with_wallet(&funding_tx, funding_context, &wallet, true, true) .await; assert!(funding_result.is_relevant, "Funding transaction must be relevant"); assert_eq!(funding_result.total_received, funding_value); @@ -485,7 +615,7 @@ mod tests { )); let spend_result = managed_wallet - .check_core_transaction(&spend_tx, spend_context, &mut wallet, true, true) + .check_core_transaction_with_wallet(&spend_tx, spend_context, &wallet, true, true) .await; assert!(spend_result.is_relevant, "Spend transaction should be detected"); @@ -513,7 +643,7 @@ mod tests { async fn test_wallet_checker_immature_transaction_flow() { let TestWalletContext { mut managed_wallet, - mut wallet, + wallet, receive_address: address, .. } = TestWalletContext::new_random(); @@ -548,7 +678,7 @@ mod tests { // Process the coinbase transaction let result = managed_wallet - .check_core_transaction(&coinbase_tx, context, &mut wallet, true, true) + .check_core_transaction_with_wallet(&coinbase_tx, context, &wallet, true, true) .await; // Set synced_height to block where coinbase was received to trigger balance updates. managed_wallet.update_synced_height(block_height); @@ -617,7 +747,7 @@ mod tests { async fn test_wallet_checker_mempool_context() { let TestWalletContext { mut managed_wallet, - mut wallet, + wallet, receive_address: address, .. } = TestWalletContext::new_random(); @@ -627,7 +757,7 @@ mod tests { let context = TransactionContext::Mempool; let result = - managed_wallet.check_core_transaction(&tx, context, &mut wallet, true, true).await; + managed_wallet.check_core_transaction_with_wallet(&tx, context, &wallet, true, true).await; // Should be relevant assert!(result.is_relevant); @@ -651,7 +781,7 @@ mod tests { async fn test_transaction_rescan_marks_as_existing() { let TestWalletContext { mut managed_wallet, - mut wallet, + wallet, receive_address: address, .. } = TestWalletContext::new_random(); @@ -665,7 +795,7 @@ mod tests { // First processing - should be marked as new let result1 = - managed_wallet.check_core_transaction(&tx, context, &mut wallet, true, true).await; + managed_wallet.check_core_transaction_with_wallet(&tx, context, &wallet, true, true).await; assert!(result1.is_relevant, "Transaction should be relevant"); assert!( @@ -690,7 +820,7 @@ mod tests { // Second processing (simulating rescan) - should be marked as existing let result2 = - managed_wallet.check_core_transaction(&tx, context, &mut wallet, true, true).await; + managed_wallet.check_core_transaction_with_wallet(&tx, context, &wallet, true, true).await; assert!(result2.is_relevant, "Transaction should still be relevant on rescan"); assert!( @@ -726,7 +856,7 @@ mod tests { async fn test_utxo_not_created_when_already_spent() { let TestWalletContext { mut managed_wallet, - mut wallet, + wallet, receive_address, xpub, } = TestWalletContext::new_random(); @@ -771,7 +901,7 @@ mod tests { )); let spend_result = managed_wallet - .check_core_transaction(&spend_tx, spend_context, &mut wallet, true, true) + .check_core_transaction_with_wallet(&spend_tx, spend_context, &wallet, true, true) .await; // Spending tx should be detected because of the change output @@ -800,7 +930,7 @@ mod tests { )); let fund_result = managed_wallet - .check_core_transaction(&funding_tx, fund_context, &mut wallet, true, true) + .check_core_transaction_with_wallet(&funding_tx, fund_context, &wallet, true, true) .await; // Funding tx should be detected @@ -1018,7 +1148,8 @@ mod tests { let block_context = TransactionContext::InBlock(BlockInfo::new(600, block_hash, 1700000000)); let tx_type = TransactionRouter::classify_transaction(&tx); - let changed = account.confirm_transaction(&tx, &account_match, block_context, tx_type); + let (changed, _changeset) = + account.confirm_transaction(&tx, &account_match, block_context, tx_type); assert!(changed, "Should return true when backfilling a missing record"); // Verify the transaction was recorded with block context @@ -1069,7 +1200,8 @@ mod tests { .first_bip44_managed_account_mut() .expect("Should have BIP44 account"); let tx_type = TransactionRouter::classify_transaction(&tx); - let changed = account.confirm_transaction(&tx, &account_match, block_context, tx_type); + let (changed, _changeset) = + account.confirm_transaction(&tx, &account_match, block_context, tx_type); assert!(changed, "Should return true when confirming unconfirmed tx"); let record = account.transactions.get(&txid).expect("Should have record"); @@ -1529,7 +1661,7 @@ mod tests { 1_700_000_000, )); let result = - managed_wallet.check_core_transaction(&tx, context, &mut wallet, true, true).await; + managed_wallet.check_core_transaction_with_wallet(&tx, context, &wallet, true, true).await; assert!(result.is_relevant, "CoinJoin tx should be relevant"); let account = managed_wallet.first_coinjoin_managed_account().expect("coinjoin account"); diff --git a/key-wallet/src/wallet/balance.rs b/key-wallet/src/wallet/balance.rs index 550155e06..6f8c3dd8f 100644 --- a/key-wallet/src/wallet/balance.rs +++ b/key-wallet/src/wallet/balance.rs @@ -2,6 +2,7 @@ //! //! This module provides a wallet balance structure containing all available balances. +use crate::changeset::BalanceChangeSet; use core::fmt::{Display, Formatter}; use core::ops::AddAssign; #[cfg(feature = "serde")] @@ -56,6 +57,26 @@ impl WalletCoreBalance { pub fn total(&self) -> u64 { self.spendable + self.unconfirmed + self.immature + self.locked } + + /// Apply a [`BalanceChangeSet`] (signed deltas) to this balance. + /// + /// Each field is adjusted by adding the corresponding delta. Negative deltas + /// that would underflow are clamped to zero. + pub fn apply_delta(&mut self, delta: &BalanceChangeSet) { + self.spendable = apply_signed_delta(self.spendable, delta.spendable_delta); + self.unconfirmed = apply_signed_delta(self.unconfirmed, delta.unconfirmed_delta); + self.immature = apply_signed_delta(self.immature, delta.immature_delta); + self.locked = apply_signed_delta(self.locked, delta.locked_delta); + } +} + +/// Apply a signed delta to an unsigned value, clamping at zero on underflow. +fn apply_signed_delta(value: u64, delta: i64) -> u64 { + if delta >= 0 { + value.saturating_add(delta as u64) + } else { + value.saturating_sub(delta.unsigned_abs()) + } } impl Display for WalletCoreBalance { diff --git a/key-wallet/src/wallet/managed_wallet_info/asset_lock_builder.rs b/key-wallet/src/wallet/managed_wallet_info/asset_lock_builder.rs index 1b7d40fce..60296c4d2 100644 --- a/key-wallet/src/wallet/managed_wallet_info/asset_lock_builder.rs +++ b/key-wallet/src/wallet/managed_wallet_info/asset_lock_builder.rs @@ -11,7 +11,6 @@ use crate::managed_account::ManagedCoreAccount; use crate::wallet::managed_wallet_info::coin_selection::SelectionStrategy; use crate::wallet::managed_wallet_info::fee::FeeRate; use crate::wallet::managed_wallet_info::transaction_builder::{BuilderError, TransactionBuilder}; -use crate::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; use crate::wallet::managed_wallet_info::ManagedWalletInfo; use crate::wallet::Wallet; use crate::{DerivationPath, Utxo}; diff --git a/key-wallet/src/wallet/managed_wallet_info/mod.rs b/key-wallet/src/wallet/managed_wallet_info/mod.rs index 2fdf07666..8c9f347f9 100644 --- a/key-wallet/src/wallet/managed_wallet_info/mod.rs +++ b/key-wallet/src/wallet/managed_wallet_info/mod.rs @@ -18,7 +18,9 @@ pub use managed_account_operations::ManagedAccountOperations; use super::balance::WalletCoreBalance; use super::metadata::WalletMetadata; use crate::account::ManagedAccountCollection; +use crate::changeset::{BalanceChangeSet, Merge, WalletChangeSet}; use crate::Network; +use dashcore::blockdata::transaction::txout::TxOut; use dashcore::prelude::CoreBlockHeight; use dashcore::Txid; #[cfg(feature = "serde")] @@ -118,10 +120,387 @@ impl ManagedWalletInfo { self.network } + /// Return the last fully processed height of the wallet. + pub fn synced_height(&self) -> CoreBlockHeight { + self.metadata.synced_height + } + + /// Get the wallet's unique ID + pub fn wallet_id(&self) -> [u8; 32] { + self.wallet_id + } + + /// Get the wallet's name + pub fn name(&self) -> Option<&str> { + self.name.as_deref() + } + + /// Set the wallet's name + pub fn set_name(&mut self, name: String) { + self.name = Some(name); + } + + /// Get the wallet's description + pub fn description(&self) -> Option<&str> { + self.description.as_deref() + } + + /// Set the wallet's description + pub fn set_description(&mut self, description: Option) { + self.description = description; + } + + /// Get the birth height of the wallet + pub fn birth_height(&self) -> CoreBlockHeight { + self.metadata.birth_height + } + + /// Set the birth height + pub fn set_birth_height(&mut self, height: CoreBlockHeight) { + self.metadata.birth_height = height; + } + + /// Get the timestamp when first loaded + pub fn first_loaded_at(&self) -> u64 { + self.metadata.first_loaded_at + } + + /// Set the timestamp when first loaded + pub fn set_first_loaded_at(&mut self, timestamp: u64) { + self.metadata.first_loaded_at = timestamp; + } + + /// Update last synced timestamp + pub fn update_last_synced(&mut self, timestamp: u64) { + self.metadata.last_synced = Some(timestamp); + } + + /// Get accounts (mutable) + pub fn accounts_mut(&mut self) -> &mut ManagedAccountCollection { + &mut self.accounts + } + + /// Get accounts (immutable) + pub fn accounts(&self) -> &ManagedAccountCollection { + &self.accounts + } + + /// Get the wallet balance + pub fn balance(&self) -> WalletCoreBalance { + self.balance + } + + /// Update the wallet balance from account state. + pub fn update_balance(&mut self) { + let mut balance = WalletCoreBalance::default(); + let synced_height = self.synced_height(); + for account in self.accounts.all_accounts_mut() { + let _balance_cs = account.update_balance(synced_height); + balance += account.balance; + } + self.balance = balance; + } + + /// Get all UTXOs for the wallet + pub fn utxos(&self) -> std::collections::BTreeSet<&Utxo> { + let mut utxos = std::collections::BTreeSet::new(); + for account in self.accounts.all_accounts() { + utxos.extend(account.utxos.values()); + } + utxos + } + + /// Get spendable UTXOs (confirmed and not locked) + pub fn get_spendable_utxos(&self) -> std::collections::BTreeSet<&Utxo> { + self.utxos().into_iter().filter(|utxo| utxo.is_spendable(self.synced_height())).collect() + } + + /// Get all monitored addresses + pub fn monitored_addresses(&self) -> Vec { + let mut addresses = Vec::new(); + for account in self.accounts.all_accounts() { + addresses.extend(account.all_addresses()); + } + addresses + } + + /// Get transaction history + pub fn transaction_history(&self) -> Vec<&TransactionRecord> { + let mut transactions = Vec::new(); + for account in self.accounts.all_accounts() { + transactions.extend(account.transactions.values()); + } + transactions + } + + /// Get immature transactions + pub fn immature_transactions(&self) -> Vec { + let mut immature_txids: std::collections::BTreeSet = + std::collections::BTreeSet::new(); + + // Find txids of immature coinbase UTXOs + for account in self.accounts.all_accounts() { + for utxo in account.utxos.values() { + if utxo.is_coinbase && !utxo.is_mature(self.synced_height()) { + immature_txids.insert(utxo.outpoint.txid); + } + } + } + + // Get the actual transactions + let mut transactions = Vec::new(); + for account in self.accounts.all_accounts() { + for (txid, record) in &account.transactions { + if immature_txids.contains(txid) { + transactions.push(record.transaction.clone()); + } + } + } + transactions + } + + /// Update chain state and process any matured transactions. + /// This should be called when the chain tip advances to a new height. + pub fn update_synced_height(&mut self, current_height: u32) { + self.metadata.synced_height = current_height; + // Update cached balance + self.update_balance(); + } + + /// Mark UTXOs for a transaction as InstantSend-locked across all accounts. + /// Returns `(changed, utxo_changeset)` where `changed` indicates if any + /// UTXO was newly marked, and `utxo_changeset` captures the IS-lock deltas + /// for persistence. + pub fn mark_instant_send_utxos( + &mut self, + txid: &dashcore::Txid, + ) -> (bool, crate::changeset::UtxoChangeSet) { + use crate::changeset::Merge; + + if !self.instant_send_locks.insert(*txid) { + return (false, crate::changeset::UtxoChangeSet::default()); + } + let mut any_changed = false; + let mut combined_utxo_cs = crate::changeset::UtxoChangeSet::default(); + for account in self.accounts.all_accounts_mut() { + let (changed, utxo_cs) = account.mark_utxos_instant_send(txid); + if changed { + any_changed = true; + } + combined_utxo_cs.merge(utxo_cs); + } + if any_changed { + self.update_balance(); + } + (any_changed, combined_utxo_cs) + } + + /// Return the aggregated monitor revision across all accounts. + /// Increments whenever the monitored address set changes. + pub fn monitor_revision(&self) -> u64 { + self.accounts.all_accounts().iter().map(|a| a.monitor_revision()).sum() + } + /// Increment the transaction count pub fn increment_transactions(&mut self) { self.metadata.total_transactions += 1; } + + /// Update the wallet balance and return a `BalanceChangeSet` capturing the aggregate delta. + pub fn update_balance_with_changeset(&mut self) -> BalanceChangeSet { + let mut balance = WalletCoreBalance::default(); + let synced_height = self.metadata.synced_height; + let mut combined = BalanceChangeSet::default(); + for account in self.accounts.all_accounts_mut() { + let acct_cs = account.update_balance(synced_height); + balance += account.balance; + combined.merge(acct_cs); + } + self.balance = balance; + combined + } + + /// Apply a [`WalletChangeSet`] to this wallet info, updating all relevant + /// state in place. + /// + /// Each sub-changeset is applied independently when present: + /// - **chain**: updates `synced_height` and related metadata. + /// - **utxos**: adds, removes, and IS-locks UTXOs in the owning accounts. + /// - **transactions**: inserts transaction records into owning accounts. + /// - **accounts**: advances revealed indices and marks addresses as used. + /// - **balance**: applies signed deltas to the cached wallet balance. + pub fn apply(&mut self, changeset: &WalletChangeSet) { + // 1. Chain ---------------------------------------------------------- + if let Some(chain) = &changeset.chain { + if let Some(height) = chain.height { + self.metadata.synced_height = height; + } + } + + // 2. UTXOs ---------------------------------------------------------- + if let Some(utxos) = &changeset.utxos { + // 2a. Added UTXOs – find owning account by address and insert. + for (outpoint, entry) in &utxos.added { + if let Some(account) = self + .accounts + .all_accounts_mut() + .into_iter() + .find(|a| a.contains_address(&entry.address)) + { + let utxo = Utxo::new( + *outpoint, + TxOut { + value: entry.value, + script_pubkey: entry.script_pubkey.clone(), + }, + entry.address.clone(), + self.metadata.synced_height, + false, // not coinbase – changeset doesn't carry this info + ); + // Propagate the IS-lock flag from the entry. + let mut utxo = utxo; + utxo.is_instantlocked = entry.is_instant_locked; + account.utxos.insert(*outpoint, utxo); + } + } + + // 2b. Spent UTXOs – remove from whichever account holds them. + for outpoint in &utxos.spent { + for account in self.accounts.all_accounts_mut() { + if account.utxos.remove(outpoint).is_some() { + break; + } + } + } + + // 2c. InstantSend-locked UTXOs – mark existing UTXOs. + for outpoint in &utxos.instant_locked { + for account in self.accounts.all_accounts_mut() { + if let Some(utxo) = account.utxos.get_mut(outpoint) { + utxo.is_instantlocked = true; + break; + } + } + } + } + + // 3. Transactions --------------------------------------------------- + if let Some(tx_cs) = &changeset.transactions { + use crate::managed_account::transaction_record::{TransactionDirection, TransactionRecord}; + use crate::transaction_checking::transaction_router::TransactionType; + use crate::transaction_checking::TransactionContext; + + for (txid, entry) in &tx_cs.records { + // Build a TransactionContext from the entry's block info. + let context = match (entry.block_height, entry.block_hash) { + (Some(h), Some(bh)) => { + use crate::transaction_checking::BlockInfo; + let block_info = BlockInfo::new(h, bh, entry.timestamp as u32); + if entry.is_chain_locked { + TransactionContext::InChainLockedBlock(block_info) + } else { + TransactionContext::InBlock(block_info) + } + } + _ => { + if entry.is_instant_locked { + TransactionContext::InstantSend + } else { + TransactionContext::Mempool + } + } + }; + + let direction = if entry.net_amount >= 0 { + TransactionDirection::Incoming + } else { + TransactionDirection::Outgoing + }; + + let record = TransactionRecord::new( + entry.transaction.clone(), + context, + TransactionType::Standard, // best-effort default + direction, + Vec::new(), // input_details – not available from changeset + Vec::new(), // output_details – not available from changeset + entry.net_amount, + ); + + // Try to find the account that owns an address in this + // transaction's outputs, and insert the record there. + let mut inserted = false; + for account in self.accounts.all_accounts_mut() { + for output in &entry.transaction.output { + if let Ok(addr) = crate::Address::from_script( + &output.script_pubkey, + self.network, + ) { + if account.contains_address(&addr) { + account.transactions.insert(*txid, record.clone()); + inserted = true; + break; + } + } + } + if inserted { + break; + } + } + + // Fallback: if no account matched via outputs, try inputs or + // just insert into the first standard account. + if !inserted { + if let Some(account) = self + .accounts + .standard_bip44_accounts + .values_mut() + .next() + { + account.transactions.insert(*txid, record); + } + } + } + } + + // 4. Accounts ------------------------------------------------------- + if let Some(acct_cs) = &changeset.accounts { + // 4a. last_revealed – advance the highest_generated index on + // address pools for the given account indices. + for (&account_index, &new_index) in &acct_cs.last_revealed { + if let Some(account) = self + .accounts + .standard_bip44_accounts + .get_mut(&account_index) + { + for pool in account.account_type.address_pools_mut() { + if let Some(current) = pool.highest_generated { + if new_index > current { + pool.highest_generated = Some(new_index); + } + } else { + pool.highest_generated = Some(new_index); + } + } + } + } + + // 4b. addresses_used – mark each address as used in its account. + for (_, address) in &acct_cs.addresses_used { + for account in self.accounts.all_accounts_mut() { + if account.contains_address(address) { + account.account_type.mark_address_used(address); + break; + } + } + } + } + + // 5. Balance -------------------------------------------------------- + if let Some(balance_cs) = &changeset.balance { + self.balance.apply_delta(balance_cs); + } + } } /// Re-export types from account module for convenience diff --git a/key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs b/key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs index 0eb0b2132..ed89942ae 100644 --- a/key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs +++ b/key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs @@ -5,11 +5,10 @@ use std::collections::BTreeSet; use super::managed_account_operations::ManagedAccountOperations; -use crate::account::ManagedAccountTrait; use crate::managed_account::managed_account_collection::ManagedAccountCollection; use crate::transaction_checking::WalletTransactionChecker; use crate::wallet::managed_wallet_info::TransactionRecord; -use crate::wallet::ManagedWalletInfo; +use crate::changeset::UtxoChangeSet; use crate::{Network, Utxo, Wallet, WalletCoreBalance}; use dashcore::prelude::CoreBlockHeight; @@ -25,6 +24,12 @@ pub trait WalletInfoInterface: Sized + WalletTransactionChecker + ManagedAccount /// Default implementation just uses with_name (backward compatibility) fn from_wallet_with_name(wallet: &Wallet, name: String) -> Self; + /// Get a reference to the underlying Wallet + fn wallet(&self) -> &Wallet; + + /// Get a mutable reference to the underlying Wallet + fn wallet_mut(&mut self) -> &mut Wallet; + /// Get the wallet's network fn network(&self) -> Network; @@ -93,8 +98,10 @@ pub trait WalletInfoInterface: Sized + WalletTransactionChecker + ManagedAccount fn update_synced_height(&mut self, current_height: u32); /// Mark UTXOs for a transaction as InstantSend-locked across all accounts. - /// Returns `true` if any UTXO was newly marked. - fn mark_instant_send_utxos(&mut self, txid: &Txid) -> bool; + /// Returns `(changed, utxo_changeset)` where `changed` indicates if any + /// UTXO was newly marked, and `utxo_changeset` captures the IS-lock deltas + /// for persistence. + fn mark_instant_send_utxos(&mut self, txid: &Txid) -> (bool, UtxoChangeSet); /// Return the aggregated monitor revision across all accounts. /// Increments whenever the monitored address set changes. @@ -102,161 +109,3 @@ pub trait WalletInfoInterface: Sized + WalletTransactionChecker + ManagedAccount 0 } } - -/// Default implementation for ManagedWalletInfo -impl WalletInfoInterface for ManagedWalletInfo { - fn from_wallet(wallet: &Wallet) -> Self { - Self::from_wallet_with_name(wallet, String::new()) - } - - fn from_wallet_with_name(wallet: &Wallet, name: String) -> Self { - Self::from_wallet_with_name(wallet, name) - } - - fn network(&self) -> Network { - self.network - } - - fn wallet_id(&self) -> [u8; 32] { - self.wallet_id - } - - fn name(&self) -> Option<&str> { - self.name.as_deref() - } - - fn set_name(&mut self, name: String) { - self.name = Some(name); - } - - fn description(&self) -> Option<&str> { - self.description.as_deref() - } - - fn set_description(&mut self, description: Option) { - self.description = description; - } - - fn birth_height(&self) -> CoreBlockHeight { - self.metadata.birth_height - } - - fn set_birth_height(&mut self, height: CoreBlockHeight) { - self.metadata.birth_height = height; - } - - fn synced_height(&self) -> CoreBlockHeight { - self.metadata.synced_height - } - - fn first_loaded_at(&self) -> u64 { - self.metadata.first_loaded_at - } - - fn set_first_loaded_at(&mut self, timestamp: u64) { - self.metadata.first_loaded_at = timestamp; - } - - fn update_last_synced(&mut self, timestamp: u64) { - self.metadata.last_synced = Some(timestamp); - } - - fn monitored_addresses(&self) -> Vec { - let mut addresses = Vec::new(); - for account in self.accounts.all_accounts() { - addresses.extend(account.all_addresses()); - } - addresses - } - - fn utxos(&self) -> BTreeSet<&Utxo> { - let mut utxos = BTreeSet::new(); - for account in self.accounts.all_accounts() { - utxos.extend(account.utxos.values()); - } - utxos - } - fn get_spendable_utxos(&self) -> BTreeSet<&Utxo> { - self.utxos().into_iter().filter(|utxo| utxo.is_spendable(self.synced_height())).collect() - } - - fn balance(&self) -> WalletCoreBalance { - self.balance - } - - fn update_balance(&mut self) { - let mut balance = WalletCoreBalance::default(); - let synced_height = self.synced_height(); - for account in self.accounts.all_accounts_mut() { - account.update_balance(synced_height); - balance += *account.balance(); - } - self.balance = balance; - } - - fn transaction_history(&self) -> Vec<&TransactionRecord> { - let mut transactions = Vec::new(); - for account in self.accounts.all_accounts() { - transactions.extend(account.transactions.values()); - } - transactions - } - - fn accounts_mut(&mut self) -> &mut ManagedAccountCollection { - &mut self.accounts - } - - fn accounts(&self) -> &ManagedAccountCollection { - &self.accounts - } - - fn immature_transactions(&self) -> Vec { - let mut immature_txids: BTreeSet = BTreeSet::new(); - - // Find txids of immature coinbase UTXOs - for account in self.accounts.all_accounts() { - for utxo in account.utxos.values() { - if utxo.is_coinbase && !utxo.is_mature(self.synced_height()) { - immature_txids.insert(utxo.outpoint.txid); - } - } - } - - // Get the actual transactions - let mut transactions = Vec::new(); - for account in self.accounts.all_accounts() { - for (txid, record) in &account.transactions { - if immature_txids.contains(txid) { - transactions.push(record.transaction.clone()); - } - } - } - transactions - } - - fn update_synced_height(&mut self, current_height: u32) { - self.metadata.synced_height = current_height; - // Update cached balance - self.update_balance(); - } - - fn mark_instant_send_utxos(&mut self, txid: &Txid) -> bool { - if !self.instant_send_locks.insert(*txid) { - return false; - } - let mut any_changed = false; - for account in self.accounts.all_accounts_mut() { - if account.mark_utxos_instant_send(txid) { - any_changed = true; - } - } - if any_changed { - self.update_balance(); - } - any_changed - } - - fn monitor_revision(&self) -> u64 { - self.accounts.all_accounts().iter().map(|a| a.monitor_revision()).sum() - } -} diff --git a/key-wallet/src/wallet/mod.rs b/key-wallet/src/wallet/mod.rs index 7feebd889..7f776a345 100644 --- a/key-wallet/src/wallet/mod.rs +++ b/key-wallet/src/wallet/mod.rs @@ -191,7 +191,6 @@ mod tests { use crate::account::account_collection::AccountCollection; use crate::account::{AccountType, StandardAccountType}; use crate::mnemonic::Language; - use crate::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; #[test] fn test_wallet_creation() {