From 99bbfdadf26918c1ccb1877673aba5b0626a030c Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 24 Apr 2026 16:11:26 +0800 Subject: [PATCH 1/2] feat(key-wallet-manager): add mutable-pair accessors + insert_wallet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds three accessors on WalletManager that exploit the split borrow between the independent `wallets` and `wallet_infos` maps: - `get_wallet_and_info_mut` — `(&Wallet, &mut T)` for mutations that read wallet config while writing info (UTXOs, balance, tx history). - `get_wallet_mut_and_info_mut` — `(&mut Wallet, &mut T)` for cases that need to mutate the wallet itself (e.g. idempotent re-derivation of HD accounts during changeset replay). - `insert_wallet` — register a pre-built `(Wallet, T)` pair, returns `WalletExists` on duplicate and bumps `structural_revision` on success. Enables restore paths that reconstruct wallets from persistence without going through the mnemonic-based builder. Unit tests cover dedup error, split-borrow mutation across both maps, and the monitor-revision bump on insert. Co-Authored-By: Claude Opus 4.7 (1M context) --- key-wallet-manager/src/accessors.rs | 125 ++++++++++++++++++++++++++++ 1 file changed, 125 insertions(+) diff --git a/key-wallet-manager/src/accessors.rs b/key-wallet-manager/src/accessors.rs index 46e5cf842..f1def0093 100644 --- a/key-wallet-manager/src/accessors.rs +++ b/key-wallet-manager/src/accessors.rs @@ -33,6 +33,44 @@ impl WalletManager { } } + /// Immutable wallet + mutable info — split borrow on two maps. + pub fn get_wallet_and_info_mut(&mut self, wallet_id: &WalletId) -> Option<(&Wallet, &mut T)> { + match (self.wallets.get(wallet_id), self.wallet_infos.get_mut(wallet_id)) { + (Some(wallet), Some(info)) => Some((wallet, info)), + _ => None, + } + } + + /// Mutable wallet + mutable info — split borrow on two maps. + /// + /// Used when the caller needs to mutate both the `Wallet` (e.g. to + /// idempotently re-derive HD accounts via `Wallet::add_account` during + /// changeset replay) and the associated info in the same scope. + pub fn get_wallet_mut_and_info_mut( + &mut self, + wallet_id: &WalletId, + ) -> Option<(&mut Wallet, &mut T)> { + match (self.wallets.get_mut(wallet_id), self.wallet_infos.get_mut(wallet_id)) { + (Some(wallet), Some(info)) => Some((wallet, info)), + _ => None, + } + } + + /// Insert a pre-built wallet and info pair. + /// + /// Errors with [`WalletError::WalletExists`] if a wallet with the same ID is + /// already registered. On success bumps the structural revision. + pub fn insert_wallet(&mut self, wallet: Wallet, info: T) -> Result { + let wallet_id = wallet.compute_wallet_id(); + if self.wallets.contains_key(&wallet_id) { + return Err(WalletError::WalletExists(wallet_id)); + } + self.wallets.insert(wallet_id, wallet); + self.wallet_infos.insert(wallet_id, info); + self.bump_structural_revision(); + Ok(wallet_id) + } + /// Remove a wallet pub fn remove_wallet(&mut self, wallet_id: &WalletId) -> Result<(Wallet, T), WalletError> { let wallet = @@ -211,3 +249,90 @@ impl WalletManager { outpoints } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::test_helpers::TEST_MNEMONIC; + use key_wallet::mnemonic::{Language, Mnemonic}; + use key_wallet::wallet::initialization::WalletAccountCreationOptions; + use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; + + fn build_wallet() -> Wallet { + let mnemonic = Mnemonic::from_phrase(TEST_MNEMONIC, Language::English).unwrap(); + Wallet::from_mnemonic(mnemonic, Network::Testnet, WalletAccountCreationOptions::Default) + .expect("wallet from mnemonic") + } + + #[test] + fn insert_wallet_rejects_duplicate() { + let mut manager: WalletManager = WalletManager::new(Network::Testnet); + let wallet = build_wallet(); + let info = ManagedWalletInfo::from_wallet(&wallet); + + let id = + manager.insert_wallet(wallet.clone(), info.clone()).expect("first insert succeeds"); + + match manager.insert_wallet(wallet, info) { + Err(WalletError::WalletExists(dup_id)) => assert_eq!(dup_id, id), + other => panic!("expected WalletExists, got {:?}", other), + } + } + + #[test] + fn get_wallet_and_info_mut_returns_none_then_some() { + let mut manager: WalletManager = WalletManager::new(Network::Testnet); + let wallet = build_wallet(); + let id = wallet.compute_wallet_id(); + + assert!(manager.get_wallet_and_info_mut(&id).is_none()); + + let info = ManagedWalletInfo::from_wallet(&wallet); + manager.insert_wallet(wallet, info).expect("insert"); + + let (_wallet_ref, info_mut) = + manager.get_wallet_and_info_mut(&id).expect("present after insert"); + info_mut.set_name("renamed".to_string()); + + assert_eq!(manager.get_wallet_info(&id).unwrap().name(), Some("renamed")); + } + + #[test] + fn get_wallet_mut_and_info_mut_allows_split_borrow() { + let mut manager: WalletManager = WalletManager::new(Network::Testnet); + let wallet = build_wallet(); + let info = ManagedWalletInfo::from_wallet(&wallet); + let id = manager.insert_wallet(wallet, info).expect("insert"); + + // Compile-time proof that &mut Wallet and &mut T coexist: call a + // `&mut self` method on each within the same borrow scope. + let (wallet_mut, info_mut) = manager.get_wallet_mut_and_info_mut(&id).expect("present"); + let _account = wallet_mut.get_bip44_account_mut(0); + info_mut.set_name("touched".to_string()); + + assert_eq!(manager.get_wallet_info(&id).unwrap().name(), Some("touched")); + } + + #[test] + fn insert_wallet_bumps_monitor_revision() { + let mut manager: WalletManager = WalletManager::new(Network::Testnet); + let before = manager.monitor_revision(); + + let wallet = build_wallet(); + let info = ManagedWalletInfo::from_wallet(&wallet); + let info_revision = info.monitor_revision(); + manager.insert_wallet(wallet, info).expect("insert"); + + let after = manager.monitor_revision(); + // `monitor_revision` is structural_revision + sum of infos' revisions. + // After insert, the delta is 1 (the structural bump) plus whatever the + // fresh info's revision contributes — so the delta must be >= 1. + assert!( + after > before + info_revision, + "expected monitor_revision > {} + {}, got {}", + before, + info_revision, + after + ); + } +} From 503e41aecfc46041673439f8299f257b0221e804 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 28 Apr 2026 12:32:58 +0800 Subject: [PATCH 2/2] test(key-wallet-manager): drop redundant accessor tests Per ZocoLini's review on #685: the type system already proves the mutable-borrow accessors return the right shape, and monitor_revision is covered elsewhere. Keep insert_wallet_rejects_duplicate since it exercises actual error-path logic. Co-Authored-By: Claude Opus 4.7 (1M context) --- key-wallet-manager/src/accessors.rs | 57 ----------------------------- 1 file changed, 57 deletions(-) diff --git a/key-wallet-manager/src/accessors.rs b/key-wallet-manager/src/accessors.rs index f1def0093..e1388608f 100644 --- a/key-wallet-manager/src/accessors.rs +++ b/key-wallet-manager/src/accessors.rs @@ -278,61 +278,4 @@ mod tests { other => panic!("expected WalletExists, got {:?}", other), } } - - #[test] - fn get_wallet_and_info_mut_returns_none_then_some() { - let mut manager: WalletManager = WalletManager::new(Network::Testnet); - let wallet = build_wallet(); - let id = wallet.compute_wallet_id(); - - assert!(manager.get_wallet_and_info_mut(&id).is_none()); - - let info = ManagedWalletInfo::from_wallet(&wallet); - manager.insert_wallet(wallet, info).expect("insert"); - - let (_wallet_ref, info_mut) = - manager.get_wallet_and_info_mut(&id).expect("present after insert"); - info_mut.set_name("renamed".to_string()); - - assert_eq!(manager.get_wallet_info(&id).unwrap().name(), Some("renamed")); - } - - #[test] - fn get_wallet_mut_and_info_mut_allows_split_borrow() { - let mut manager: WalletManager = WalletManager::new(Network::Testnet); - let wallet = build_wallet(); - let info = ManagedWalletInfo::from_wallet(&wallet); - let id = manager.insert_wallet(wallet, info).expect("insert"); - - // Compile-time proof that &mut Wallet and &mut T coexist: call a - // `&mut self` method on each within the same borrow scope. - let (wallet_mut, info_mut) = manager.get_wallet_mut_and_info_mut(&id).expect("present"); - let _account = wallet_mut.get_bip44_account_mut(0); - info_mut.set_name("touched".to_string()); - - assert_eq!(manager.get_wallet_info(&id).unwrap().name(), Some("touched")); - } - - #[test] - fn insert_wallet_bumps_monitor_revision() { - let mut manager: WalletManager = WalletManager::new(Network::Testnet); - let before = manager.monitor_revision(); - - let wallet = build_wallet(); - let info = ManagedWalletInfo::from_wallet(&wallet); - let info_revision = info.monitor_revision(); - manager.insert_wallet(wallet, info).expect("insert"); - - let after = manager.monitor_revision(); - // `monitor_revision` is structural_revision + sum of infos' revisions. - // After insert, the delta is 1 (the structural bump) plus whatever the - // fresh info's revision contributes — so the delta must be >= 1. - assert!( - after > before + info_revision, - "expected monitor_revision > {} + {}, got {}", - before, - info_revision, - after - ); - } }