From ed46023a350a7192324c8238bd23a4b7c547e1ce Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 5 May 2026 18:47:17 +0700 Subject: [PATCH] feat(key-wallet): add keep_txs_in_memory Cargo feature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an opt-in `keep_txs_in_memory` Cargo feature (NOT in default) that gates the `ManagedCoreKeysAccount::transactions` field at compile time. With the feature off, processed transactions update UTXOs and balance but no per-tx history is retained, saving memory for callers that only need balance/UTXO state. Always-present `processed_txids: HashSet` keeps `confirm_transaction`'s dedup guard working in either configuration so re-events (mempool→block, late IS-lock arrivals) aren't replayed as new every time. Also adds `has_transaction(&Txid)` / `get_transaction(&Txid)` helpers — the former backed by `processed_txids` works in both configs, the latter is gated. `key-wallet-manager` and `key-wallet-ffi` enable the feature explicitly to keep their existing surface (event matured-coinbase, FFI tx accessors) unchanged. Wallet-level history queries (`transaction_history`, `immature_transactions`, `matured_coinbase_records`) return empty `Vec`s when the feature is off so downstream callers compile cleanly. Test files that exercise per-tx history are gated under the feature, new module `keep_txs_in_memory_tests` covers both configurations (including a regression test for confirm-transaction idempotency). Co-Authored-By: Claude Opus 4.7 (1M context) --- key-wallet-ffi/Cargo.toml | 4 +- key-wallet-manager/Cargo.toml | 2 +- key-wallet/Cargo.toml | 5 +- .../managed_account/managed_account_trait.rs | 18 ++- .../managed_core_funds_account.rs | 25 +++- .../managed_core_keys_account.rs | 107 +++++++++++++++++- key-wallet/src/test_utils/wallet.rs | 14 ++- .../src/tests/keep_txs_in_memory_tests.rs | 102 +++++++++++++++++ key-wallet/src/tests/mod.rs | 5 + .../transaction_router/tests/mod.rs | 4 +- .../transaction_checking/wallet_checker.rs | 32 ++++-- .../wallet_info_interface.rs | 83 ++++++++------ 12 files changed, 345 insertions(+), 56 deletions(-) create mode 100644 key-wallet/src/tests/keep_txs_in_memory_tests.rs diff --git a/key-wallet-ffi/Cargo.toml b/key-wallet-ffi/Cargo.toml index 3237e0579..e85d647c0 100644 --- a/key-wallet-ffi/Cargo.toml +++ b/key-wallet-ffi/Cargo.toml @@ -20,7 +20,7 @@ eddsa = ["dashcore/eddsa", "key-wallet/eddsa"] bls = ["dashcore/bls", "key-wallet/bls"] [dependencies] -key-wallet = { path = "../key-wallet" } +key-wallet = { path = "../key-wallet", features = ["keep_txs_in_memory"] } key-wallet-manager = { path = "../key-wallet-manager" } dashcore = { path = "../dash" } dash-network = { path = "../dash-network", features = ["ffi"] } @@ -33,6 +33,6 @@ hex = "0.4" cbindgen = "0.29" [dev-dependencies] -key-wallet = { path = "../key-wallet", features = ["test-utils"] } +key-wallet = { path = "../key-wallet", features = ["test-utils", "keep_txs_in_memory"] } key-wallet-manager = { path = "../key-wallet-manager", features = ["test-utils"] } hex = "0.4" diff --git a/key-wallet-manager/Cargo.toml b/key-wallet-manager/Cargo.toml index 1b7347c05..1df23c4b4 100644 --- a/key-wallet-manager/Cargo.toml +++ b/key-wallet-manager/Cargo.toml @@ -16,7 +16,7 @@ bls = ["key-wallet/bls"] eddsa = ["key-wallet/eddsa"] [dependencies] -key-wallet = { path = "../key-wallet", default-features = false } +key-wallet = { path = "../key-wallet", default-features = false, features = ["keep_txs_in_memory"] } dashcore = { path = "../dash" } async-trait = "0.1" tokio = { version = "1", features = ["macros", "rt", "sync"] } diff --git a/key-wallet/Cargo.toml b/key-wallet/Cargo.toml index a19209a38..9eaf638ec 100644 --- a/key-wallet/Cargo.toml +++ b/key-wallet/Cargo.toml @@ -16,6 +16,9 @@ bip38 = ["scrypt", "aes", "bs58", "rand"] eddsa = ["dashcore/eddsa"] bls = ["dashcore/bls"] test-utils = ["dashcore/test-utils"] +# Keep per-account transaction history (`ManagedCoreKeysAccount::transactions`) in memory. +# Disable to skip storing transaction records when only UTXOs and balances matter. +keep_txs_in_memory = [] [dependencies] internals = { path = "../internals", package = "dashcore-private" } @@ -46,7 +49,7 @@ async-trait = "0.1" [dev-dependencies] dashcore = { path="../dash", features = ["test-utils"] } hex = "0.4" -key-wallet = { path = ".", features = ["test-utils", "bip38", "serde", "bincode", "eddsa", "bls"] } +key-wallet = { path = ".", default-features = false, features = ["test-utils", "bip38", "serde", "bincode", "eddsa", "bls", "getrandom"] } rand = { version = "0.8", features = ["std", "std_rng"] } test-case = "3.3" tokio = { version = "1", features = ["macros", "rt"] } diff --git a/key-wallet/src/managed_account/managed_account_trait.rs b/key-wallet/src/managed_account/managed_account_trait.rs index 18e8eb04e..779acdf6d 100644 --- a/key-wallet/src/managed_account/managed_account_trait.rs +++ b/key-wallet/src/managed_account/managed_account_trait.rs @@ -5,8 +5,10 @@ //! depend on funds bookkeeping (balance / UTXOs / spent outpoints) lives here //! as default-method implementations so it is written exactly once. +#[cfg(feature = "keep_txs_in_memory")] use std::collections::BTreeMap; +#[cfg(feature = "keep_txs_in_memory")] use crate::account::TransactionRecord; #[cfg(feature = "bls")] use crate::derivation_bls_bip32::ExtendedBLSPubKey; @@ -41,12 +43,24 @@ pub trait ManagedAccountTrait { /// Check if this is a watch-only account fn is_watch_only(&self) -> bool; - /// Get transactions + /// Get transactions. + /// + /// Only available when the `keep_txs_in_memory` Cargo feature is enabled. + #[cfg(feature = "keep_txs_in_memory")] fn transactions(&self) -> &BTreeMap; - /// Get mutable transactions + /// Get mutable transactions. + /// + /// Only available when the `keep_txs_in_memory` Cargo feature is enabled. + #[cfg(feature = "keep_txs_in_memory")] fn transactions_mut(&mut self) -> &mut BTreeMap; + /// Returns `true` if this account has already processed `txid`. + /// + /// Backed by an always-present "processed" set, so this works in both + /// feature configurations. + fn has_transaction(&self, txid: &Txid) -> bool; + /// Return the current monitor revision. /// /// Bumped whenever the monitored address set changes (e.g. new addresses diff --git a/key-wallet/src/managed_account/managed_core_funds_account.rs b/key-wallet/src/managed_account/managed_core_funds_account.rs index 88d3abc15..955346106 100644 --- a/key-wallet/src/managed_account/managed_core_funds_account.rs +++ b/key-wallet/src/managed_account/managed_core_funds_account.rs @@ -235,6 +235,11 @@ impl ManagedCoreFundsAccount { /// Re-process an existing transaction with updated context (e.g., mempool→block confirmation) /// and potentially new address matches from gap limit rescans. + /// + /// Deduplication uses the always-present `processed_txids` set on the + /// inner keys account. With the `keep_txs_in_memory` Cargo feature off, + /// no per-tx record is stored, so we cannot detect a confirmation + /// status transition; we still refresh UTXO state and report no change. pub(crate) fn confirm_transaction( &mut self, tx: &Transaction, @@ -242,12 +247,14 @@ impl ManagedCoreFundsAccount { context: TransactionContext, transaction_type: TransactionType, ) -> bool { - if !self.keys.transactions().contains_key(&tx.txid()) { + if !self.keys.has_transaction(&tx.txid()) { self.record_transaction(tx, account_match, context, transaction_type); return true; } + #[cfg_attr(not(feature = "keep_txs_in_memory"), allow(unused_mut))] let mut changed = false; + #[cfg(feature = "keep_txs_in_memory")] if let Some(tx_record) = self.keys.transactions_mut().get_mut(&tx.txid()) { debug_assert_eq!( tx_record.transaction_type, @@ -265,6 +272,8 @@ impl ManagedCoreFundsAccount { changed = !was_confirmed; } } + #[cfg(not(feature = "keep_txs_in_memory"))] + let _ = transaction_type; self.update_utxos(tx, account_match, context); changed } @@ -372,7 +381,7 @@ impl ManagedCoreFundsAccount { ); let record = tx_record.clone(); - self.keys.transactions_mut().insert(tx.txid(), tx_record); + self.keys.insert_transaction(tx.txid(), tx_record); self.update_utxos(tx, account_match, context); record @@ -608,14 +617,20 @@ impl ManagedAccountTrait for ManagedCoreFundsAccount { self.keys.is_watch_only() } + #[cfg(feature = "keep_txs_in_memory")] fn transactions(&self) -> &BTreeMap { self.keys.transactions() } + #[cfg(feature = "keep_txs_in_memory")] fn transactions_mut(&mut self) -> &mut BTreeMap { self.keys.transactions_mut() } + fn has_transaction(&self, txid: &Txid) -> bool { + self.keys.has_transaction(txid) + } + fn monitor_revision(&self) -> u64 { self.keys.monitor_revision() } @@ -640,6 +655,10 @@ impl<'de> Deserialize<'de> for ManagedCoreFundsAccount { let helper = Helper::deserialize(deserializer)?; + // `spent_outpoints` is rebuilt from stored transactions, which only + // exist when the `keep_txs_in_memory` Cargo feature is enabled. When + // the feature is off, start empty. + #[cfg(feature = "keep_txs_in_memory")] let spent_outpoints = helper .keys .transactions() @@ -647,6 +666,8 @@ impl<'de> Deserialize<'de> for ManagedCoreFundsAccount { .flat_map(|record| &record.transaction.input) .map(|input| input.previous_output) .collect(); + #[cfg(not(feature = "keep_txs_in_memory"))] + let spent_outpoints = HashSet::new(); Ok(ManagedCoreFundsAccount { keys: helper.keys, diff --git a/key-wallet/src/managed_account/managed_core_keys_account.rs b/key-wallet/src/managed_account/managed_core_keys_account.rs index b9d26b632..fc18d22ed 100644 --- a/key-wallet/src/managed_account/managed_core_keys_account.rs +++ b/key-wallet/src/managed_account/managed_core_keys_account.rs @@ -18,7 +18,9 @@ use crate::Network; use dashcore::Txid; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; +#[cfg(any(feature = "keep_txs_in_memory", feature = "serde"))] use std::collections::BTreeMap; +use std::collections::HashSet; /// Managed core keys account with mutable state but no funds tracking. /// @@ -31,7 +33,7 @@ use std::collections::BTreeMap; /// Most behavior comes from [`ManagedAccountTrait`] default methods; this /// type only owns the primitive state. #[derive(Debug, Clone)] -#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +#[cfg_attr(feature = "serde", derive(Serialize))] pub struct ManagedCoreKeysAccount { /// Account type with embedded address pools and index managed_account_type: ManagedAccountType, @@ -39,8 +41,21 @@ pub struct ManagedCoreKeysAccount { network: Network, /// Whether this is a watch-only account is_watch_only: bool, - /// Transaction history for this account + /// Transaction history for this account. + /// + /// Only present when the `keep_txs_in_memory` Cargo feature is enabled. + /// With the feature off, processed transactions update UTXOs and balance + /// (on the funds variant) but no per-tx history is retained here. + #[cfg(feature = "keep_txs_in_memory")] transactions: BTreeMap, + /// Txids of every transaction this account has already processed. + /// + /// Always populated regardless of `keep_txs_in_memory`, so dedup of + /// re-events (mempool→block, late IS-lock arrivals) works in either + /// feature configuration. Rebuilt from `transactions` during + /// deserialization when the feature is enabled. + #[cfg_attr(feature = "serde", serde(skip))] + processed_txids: HashSet, /// Revision counter incremented when the monitored address set changes /// (e.g. new addresses generated). Used to detect bloom filter staleness. #[cfg_attr(feature = "serde", serde(skip))] @@ -58,11 +73,63 @@ impl ManagedCoreKeysAccount { managed_account_type, network, is_watch_only, + #[cfg(feature = "keep_txs_in_memory")] transactions: BTreeMap::new(), + processed_txids: HashSet::new(), monitor_revision: 0, } } + /// Returns `true` if this account has already processed `txid`. + /// + /// Backed by an always-present `processed_txids` set, so this works + /// regardless of whether the `keep_txs_in_memory` Cargo feature is + /// enabled. Use [`Self::get_transaction`] to retrieve the full record + /// (only available when the feature is enabled). + pub fn has_transaction(&self, txid: &Txid) -> bool { + self.processed_txids.contains(txid) + } + + /// Returns the stored transaction record for `txid`, if any. + /// + /// Always returns `None` when the `keep_txs_in_memory` Cargo feature is + /// disabled, since no records are retained. + #[cfg(feature = "keep_txs_in_memory")] + pub fn get_transaction(&self, txid: &Txid) -> Option<&TransactionRecord> { + self.transactions.get(txid) + } + + /// Always returns `None` because the `keep_txs_in_memory` Cargo feature + /// is disabled, so no records are retained. + #[cfg(not(feature = "keep_txs_in_memory"))] + pub fn get_transaction(&self, _txid: &Txid) -> Option<&TransactionRecord> { + None + } + + /// Insert a transaction record. Used by the funds variant; always + /// updates `processed_txids` regardless of feature. + #[cfg(feature = "keep_txs_in_memory")] + pub(crate) fn insert_transaction(&mut self, txid: Txid, record: TransactionRecord) { + self.processed_txids.insert(txid); + self.transactions.insert(txid, record); + } + + /// Insert a transaction record. With the feature off, only the + /// `processed_txids` set is updated. + #[cfg(not(feature = "keep_txs_in_memory"))] + pub(crate) fn insert_transaction(&mut self, txid: Txid, _record: TransactionRecord) { + self.processed_txids.insert(txid); + } + + /// Forget that `txid` was ever processed. Test-only escape hatch for + /// simulating a "lost record" state. + #[cfg(test)] + pub(crate) fn forget_transaction(&mut self, txid: &Txid) { + self.processed_txids.remove(txid); + #[cfg(feature = "keep_txs_in_memory")] + self.transactions.remove(txid); + } + /// Create a `ManagedCoreKeysAccount` from an [`Account`](super::super::Account). pub fn from_account(account: &super::super::Account) -> Self { let key_source = address_pool::KeySource::Public(account.account_xpub); @@ -139,14 +206,20 @@ impl ManagedAccountTrait for ManagedCoreKeysAccount { self.is_watch_only } + #[cfg(feature = "keep_txs_in_memory")] fn transactions(&self) -> &BTreeMap { &self.transactions } + #[cfg(feature = "keep_txs_in_memory")] fn transactions_mut(&mut self) -> &mut BTreeMap { &mut self.transactions } + fn has_transaction(&self, txid: &Txid) -> bool { + self.processed_txids.contains(txid) + } + fn monitor_revision(&self) -> u64 { self.monitor_revision } @@ -155,3 +228,33 @@ impl ManagedAccountTrait for ManagedCoreKeysAccount { self.monitor_revision += 1; } } + +#[cfg(feature = "serde")] +impl<'de> Deserialize<'de> for ManagedCoreKeysAccount { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + #[derive(Deserialize)] + struct Helper { + managed_account_type: ManagedAccountType, + network: Network, + is_watch_only: bool, + #[serde(default)] + transactions: BTreeMap, + } + + let helper = Helper::deserialize(deserializer)?; + let processed_txids: HashSet = helper.transactions.keys().copied().collect(); + + Ok(ManagedCoreKeysAccount { + managed_account_type: helper.managed_account_type, + network: helper.network, + is_watch_only: helper.is_watch_only, + #[cfg(feature = "keep_txs_in_memory")] + transactions: helper.transactions, + processed_txids, + monitor_revision: 0, + }) + } +} diff --git a/key-wallet/src/test_utils/wallet.rs b/key-wallet/src/test_utils/wallet.rs index 2a4756dd9..5ec113583 100644 --- a/key-wallet/src/test_utils/wallet.rs +++ b/key-wallet/src/test_utils/wallet.rs @@ -1,12 +1,17 @@ -use dashcore::{Address, Network, Transaction, Txid}; +#[cfg(feature = "keep_txs_in_memory")] +use dashcore::Txid; +use dashcore::{Address, Network, Transaction}; use crate::{ - account::{ManagedCoreFundsAccount, TransactionRecord}, - managed_account::managed_account_trait::ManagedAccountTrait, + account::ManagedCoreFundsAccount, transaction_checking::{TransactionCheckResult, TransactionContext, WalletTransactionChecker}, wallet::{initialization::WalletAccountCreationOptions, ManagedWalletInfo}, ExtendedPubKey, Utxo, Wallet, }; +#[cfg(feature = "keep_txs_in_memory")] +use crate::{ + account::TransactionRecord, managed_account::managed_account_trait::ManagedAccountTrait, +}; impl ManagedWalletInfo { pub fn dummy(id: u8) -> Self { @@ -61,6 +66,9 @@ impl TestWalletContext { } /// Returns a transaction record by txid from the first BIP44 account. + /// + /// Only available when the `keep_txs_in_memory` Cargo feature is enabled. + #[cfg(feature = "keep_txs_in_memory")] pub fn transaction(&self, txid: &Txid) -> &TransactionRecord { self.bip44_account().transactions().get(txid).expect("Should have transaction") } diff --git a/key-wallet/src/tests/keep_txs_in_memory_tests.rs b/key-wallet/src/tests/keep_txs_in_memory_tests.rs new file mode 100644 index 000000000..94d831119 --- /dev/null +++ b/key-wallet/src/tests/keep_txs_in_memory_tests.rs @@ -0,0 +1,102 @@ +//! Tests for the `keep_txs_in_memory` Cargo feature. +//! +//! When the feature is enabled (the default), `ManagedCoreKeysAccount` keeps +//! a per-account `transactions` map populated as transactions are processed. +//! When the feature is disabled, the field does not exist and processing a +//! transaction only updates UTXOs and balance — but `processed_txids` +//! still tracks which txids have been seen so `confirm_transaction` +//! continues to deduplicate re-events. + +use crate::managed_account::managed_account_trait::ManagedAccountTrait; +use crate::test_utils::TestWalletContext; +use crate::transaction_checking::{BlockInfo, TransactionContext}; +use dashcore::{BlockHash, Transaction}; +use dashcore_hashes::Hash; + +#[tokio::test] +async fn record_transaction_updates_utxos_and_balance() { + let mut ctx = TestWalletContext::new_random(); + + let tx = Transaction::dummy(&ctx.receive_address, 0..1, &[200_000]); + let result = ctx.check_transaction(&tx, TransactionContext::Mempool).await; + assert!(result.is_relevant); + + let account = ctx.bip44_account(); + assert_eq!(account.utxos.len(), 1, "UTXOs are tracked regardless of feature"); + assert_eq!(account.balance.spendable(), 200_000); +} + +#[cfg(feature = "keep_txs_in_memory")] +#[tokio::test] +async fn transactions_are_stored_when_feature_enabled() { + let mut ctx = TestWalletContext::new_random(); + + let tx = Transaction::dummy(&ctx.receive_address, 0..1, &[200_000]); + let _ = ctx.check_transaction(&tx, TransactionContext::Mempool).await; + + let account = ctx.bip44_account(); + assert_eq!(account.transactions().len(), 1); + assert!(account.has_transaction(&tx.txid())); + assert!(account.keys().get_transaction(&tx.txid()).is_some()); +} + +#[cfg(not(feature = "keep_txs_in_memory"))] +#[tokio::test] +async fn transactions_are_not_stored_when_feature_disabled() { + let mut ctx = TestWalletContext::new_random(); + + let tx = Transaction::dummy(&ctx.receive_address, 0..1, &[200_000]); + let _ = ctx.check_transaction(&tx, TransactionContext::Mempool).await; + + let account = ctx.bip44_account(); + // No record is stored, so `get_transaction` returns None ... + assert!(account.keys().get_transaction(&tx.txid()).is_none()); + // ... but `has_transaction` still returns true so re-events dedupe. + assert!(account.has_transaction(&tx.txid())); +} + +/// Re-processing the same transaction (mempool → block) must not double-count +/// UTXOs or change balance, regardless of whether the `keep_txs_in_memory` +/// feature is enabled. Without the always-present `processed_txids` set this +/// regresses when the feature is off because `confirm_transaction`'s only +/// dedup signal disappears with the `transactions` map. +#[tokio::test] +async fn confirm_transaction_dedupes_repeat_events() { + let mut ctx = TestWalletContext::new_random(); + + let tx = Transaction::dummy(&ctx.receive_address, 0..1, &[200_000]); + + // First: mempool + let r1 = ctx.check_transaction(&tx, TransactionContext::Mempool).await; + assert!(r1.is_relevant); + assert!(r1.is_new_transaction); + let utxo_count_after_mempool = ctx.bip44_account().utxos.len(); + let balance_after_mempool = ctx.bip44_account().balance.spendable(); + let revision_after_mempool = ctx.bip44_account().monitor_revision(); + + // Second: same tx confirmed in a block + let block = TransactionContext::InBlock(BlockInfo::new( + 100, + BlockHash::from_slice(&[7u8; 32]).unwrap(), + 1_700_000_000, + )); + let r2 = ctx.check_transaction(&tx, block).await; + assert!(r2.is_relevant); + assert!(!r2.is_new_transaction, "block confirmation must not be flagged as a new tx"); + + let account = ctx.bip44_account(); + assert_eq!( + account.utxos.len(), + utxo_count_after_mempool, + "UTXO count must not grow on confirmation" + ); + assert_eq!( + account.balance.spendable(), + balance_after_mempool, + "spendable balance must not change between mempool and block confirmation" + ); + // monitor_revision can advance once for the block-context UTXO refresh, + // but it must not balloon to N per replay. + let bumped_by = account.monitor_revision().saturating_sub(revision_after_mempool); + assert!(bumped_by <= 1, "monitor_revision bumped {bumped_by} times on a single confirmation"); +} diff --git a/key-wallet/src/tests/mod.rs b/key-wallet/src/tests/mod.rs index d92850c79..238da7a64 100644 --- a/key-wallet/src/tests/mod.rs +++ b/key-wallet/src/tests/mod.rs @@ -24,8 +24,13 @@ mod special_transaction_tests; mod transaction_tests; +// `spent_outpoints_tests` populates `account.transactions` directly, so it +// only compiles with the `keep_txs_in_memory` Cargo feature enabled. +#[cfg(feature = "keep_txs_in_memory")] mod spent_outpoints_tests; +mod keep_txs_in_memory_tests; + mod unit_variant_wallet_tests; mod wallet_tests; diff --git a/key-wallet/src/transaction_checking/transaction_router/tests/mod.rs b/key-wallet/src/transaction_checking/transaction_router/tests/mod.rs index 3fa992fd5..2c5285fff 100644 --- a/key-wallet/src/transaction_checking/transaction_router/tests/mod.rs +++ b/key-wallet/src/transaction_checking/transaction_router/tests/mod.rs @@ -10,7 +10,9 @@ mod helpers; mod asset_unlock; #[cfg(test)] mod classification; -#[cfg(test)] +// `coinbase` exercises in-memory transaction storage; only built when the +// `keep_txs_in_memory` Cargo feature is enabled. +#[cfg(all(test, feature = "keep_txs_in_memory"))] mod coinbase; #[cfg(test)] mod coinjoin; diff --git a/key-wallet/src/transaction_checking/wallet_checker.rs b/key-wallet/src/transaction_checking/wallet_checker.rs index d5d799a0f..3c8361f6a 100644 --- a/key-wallet/src/transaction_checking/wallet_checker.rs +++ b/key-wallet/src/transaction_checking/wallet_checker.rs @@ -72,7 +72,7 @@ impl WalletTransactionChecker for ManagedWalletInfo { if let Some(account) = self.accounts.get_by_account_type_match(&account_match.account_type_match) { - if account.transactions().contains_key(&txid) { + if account.has_transaction(&txid) { is_new = false; break; } @@ -90,7 +90,7 @@ impl WalletTransactionChecker for ManagedWalletInfo { let already_confirmed = result.affected_accounts.iter().any(|am| { self.accounts .get_by_account_type_match(&am.account_type_match) - .and_then(|a| a.transactions().get(&txid)) + .and_then(|a| a.keys().get_transaction(&txid)) .map_or(false, |r| r.is_confirmed()) }); if already_confirmed { @@ -108,8 +108,9 @@ impl WalletTransactionChecker for ManagedWalletInfo { else { continue; }; - if account.transactions().contains_key(&txid) { + if account.has_transaction(&txid) { account.mark_utxos_instant_send(&txid); + #[cfg(feature = "keep_txs_in_memory")] if let Some(record) = account.transactions_mut().get_mut(&txid) { record.update_context(context.clone()); result.updated_records.push(record.clone()); @@ -151,9 +152,10 @@ impl WalletTransactionChecker for ManagedWalletInfo { result.new_records.push(record); result.state_modified = true; } else { - let existed_before = account.transactions().contains_key(&tx.txid()); + let existed_before = account.has_transaction(&tx.txid()); if account.confirm_transaction(tx, &account_match, context.clone(), tx_type) { result.state_modified = true; + #[cfg(feature = "keep_txs_in_memory")] if let Some(record) = account.transactions().get(&tx.txid()) { if existed_before { result.updated_records.push(record.clone()); @@ -161,6 +163,8 @@ impl WalletTransactionChecker for ManagedWalletInfo { result.new_records.push(record.clone()); } } + #[cfg(not(feature = "keep_txs_in_memory"))] + let _ = existed_before; } } @@ -220,7 +224,9 @@ impl WalletTransactionChecker for ManagedWalletInfo { } } -#[cfg(test)] +// These tests rely on per-account transaction history and so only run when +// the `keep_txs_in_memory` Cargo feature is enabled. +#[cfg(all(test, feature = "keep_txs_in_memory"))] mod tests { use super::*; use crate::account::account_type::StandardAccountType; @@ -1057,7 +1063,9 @@ mod tests { let account1 = managed_wallet .bip44_managed_account_at_index_mut(1) .expect("Should have managed account 1"); - account1.transactions_mut().remove(&txid); + // Clear both the record and the always-present `processed_txids` + // entry so the wallet-level `has_transaction` check reports false. + account1.keys_mut().forget_transaction(&txid); account1.utxos.clear(); assert!(!account1.transactions().contains_key(&txid)); assert!(account1.utxos.is_empty()); @@ -1109,13 +1117,15 @@ mod tests { let (mut ctx, tx) = TestWalletContext::new_random().with_mempool_funding(300_000).await; let txid = tx.txid(); - // Simulate the account missing the mempool record by removing it + // Simulate the account missing the mempool record by removing it. + // Forget on `keys_mut()` clears both the record and the always-present + // `processed_txids` entry so `has_transaction` returns false. let account = ctx .managed_wallet .first_bip44_managed_account_mut() .expect("Should have BIP44 account"); assert!(account.transactions().contains_key(&txid)); - account.transactions_mut().remove(&txid); + account.keys_mut().forget_transaction(&txid); assert!(!account.transactions().contains_key(&txid)); // Now process the same tx as a block confirmation. @@ -1159,12 +1169,14 @@ mod tests { assert!(result.is_relevant); let account_match = result.affected_accounts[0].clone(); - // Remove the transaction record (simulating a missing account scenario) + // Remove the transaction record (simulating a missing account scenario). + // `forget_transaction` also clears `processed_txids` so the dedup + // guard reports the tx as new on confirmation. let account = ctx .managed_wallet .first_bip44_managed_account_mut() .expect("Should have BIP44 account"); - account.transactions_mut().remove(&txid); + account.keys_mut().forget_transaction(&txid); account.utxos.clear(); assert!(!account.transactions().contains_key(&txid)); assert!(account.utxos.is_empty()); 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 bc53e771a..26e040063 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 @@ -7,6 +7,7 @@ use std::collections::{BTreeMap, BTreeSet}; use super::managed_account_operations::ManagedAccountOperations; use crate::account::{AccountType, ManagedAccountTrait}; use crate::managed_account::managed_account_collection::ManagedAccountCollection; +#[cfg(feature = "keep_txs_in_memory")] use crate::transaction_checking::TransactionContext; use crate::transaction_checking::WalletTransactionChecker; use crate::wallet::managed_wallet_info::TransactionRecord; @@ -215,11 +216,16 @@ impl WalletInfoInterface for ManagedWalletInfo { } fn transaction_history(&self) -> Vec<&TransactionRecord> { - let mut transactions = Vec::new(); - for account in self.accounts.all_accounts() { - transactions.extend(account.transactions().values()); + #[cfg(feature = "keep_txs_in_memory")] + { + let mut transactions = Vec::new(); + for account in self.accounts.all_accounts() { + transactions.extend(account.transactions().values()); + } + transactions } - transactions + #[cfg(not(feature = "keep_txs_in_memory"))] + Vec::new() } fn accounts_mut(&mut self) -> &mut ManagedAccountCollection { @@ -231,27 +237,32 @@ impl WalletInfoInterface for ManagedWalletInfo { } 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.last_processed_height()) { - immature_txids.insert(utxo.outpoint.txid); + #[cfg(feature = "keep_txs_in_memory")] + { + 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.last_processed_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()); + // 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 } - transactions + #[cfg(not(feature = "keep_txs_in_memory"))] + Vec::new() } fn update_last_processed_height(&mut self, current_height: u32) { @@ -272,22 +283,27 @@ impl WalletInfoInterface for ManagedWalletInfo { if new_height <= old_height { return Vec::new(); } - let mut matured = Vec::new(); - for account in self.accounts.all_accounts() { - for record in account.transactions().values() { - if !record.transaction.is_coin_base() { - continue; - } - let Some(record_height) = record.height() else { - continue; - }; - let maturity_height = record_height.saturating_add(100); - if maturity_height > old_height && maturity_height <= new_height { - matured.push(record.clone()); + #[cfg(feature = "keep_txs_in_memory")] + { + let mut matured = Vec::new(); + for account in self.accounts.all_accounts() { + for record in account.transactions().values() { + if !record.transaction.is_coin_base() { + continue; + } + let Some(record_height) = record.height() else { + continue; + }; + let maturity_height = record_height.saturating_add(100); + if maturity_height > old_height && maturity_height <= new_height { + matured.push(record.clone()); + } } } + matured } - matured + #[cfg(not(feature = "keep_txs_in_memory"))] + Vec::new() } fn mark_instant_send_utxos(&mut self, txid: &Txid, lock: &InstantLock) -> bool { @@ -299,10 +315,13 @@ impl WalletInfoInterface for ManagedWalletInfo { if account.mark_utxos_instant_send(txid) { any_changed = true; } + #[cfg(feature = "keep_txs_in_memory")] if let Some(record) = account.transactions_mut().get_mut(txid) { record.update_context(TransactionContext::InstantSend(lock.clone())); } } + #[cfg(not(feature = "keep_txs_in_memory"))] + let _ = lock; if any_changed { self.update_balance(); }