From 0a49ef440e2486f623942d4bb856ac8004845619 Mon Sep 17 00:00:00 2001 From: xdustinface Date: Sun, 26 Apr 2026 00:28:59 +1000 Subject: [PATCH 1/4] fix(key-wallet): seed sync checkpoints from `birth_height` in `ManagedWalletInfo` ctors `WalletInfoInterface::from_wallet` and `from_wallet_with_name` (plus the inherent `ManagedWalletInfo` constructors) now take `birth_height: CoreBlockHeight` and seed both checkpoint heights (`synced_height`, `last_processed_height`) to `birth_height.saturating_sub(1)` so the next block to scan is always `birth_height`. Without this, every wallet-add path had to remember `set_birth_height` after construction, and even when called it only set `birth_height`, leaving the checkpoints at `0`. With the per-wallet sync introduced in #694, that dragged `WalletManager::synced_height` (a min across wallets) back to genesis on every add, forcing a re-scan from `0`. Moving the seed into construction makes the invariant a property of the type. `ManagedWalletInfo::with_birth_height` is removed. It set only `birth_height` and had no callers. --- key-wallet-ffi/src/managed_wallet.rs | 6 ++-- key-wallet-ffi/src/transaction.rs | 2 +- key-wallet-ffi/src/transaction_checking.rs | 2 +- key-wallet-manager/src/accessors.rs | 2 +- key-wallet-manager/src/lib.rs | 23 +++++-------- key-wallet-manager/tests/integration_test.rs | 27 +++++++++------ .../tests/test_serialized_wallets.rs | 7 ++++ key-wallet/src/test_utils/wallet.rs | 2 +- .../tests/identity_transactions.rs | 4 +-- .../transaction_router/tests/provider.rs | 20 +++++------ .../transaction_router/tests/routing.rs | 8 ++--- .../transaction_checking/wallet_checker.rs | 8 ++--- .../managed_wallet_info/asset_lock_builder.rs | 2 +- .../src/wallet/managed_wallet_info/mod.rs | 34 +++++++++++-------- .../wallet_info_interface.rs | 25 ++++++++------ key-wallet/src/wallet/mod.rs | 2 +- 16 files changed, 94 insertions(+), 80 deletions(-) diff --git a/key-wallet-ffi/src/managed_wallet.rs b/key-wallet-ffi/src/managed_wallet.rs index ee37b7cdb..a9165f4f8 100644 --- a/key-wallet-ffi/src/managed_wallet.rs +++ b/key-wallet-ffi/src/managed_wallet.rs @@ -501,7 +501,7 @@ mod tests { // Create managed wallet info from the wallet heap-allocated like C would do let wallet_rust = unsafe { &(*wallet).wallet }; - let managed_info = ManagedWalletInfo::from_wallet(wallet_rust); + let managed_info = ManagedWalletInfo::from_wallet(wallet_rust, 0); let ffi_managed = Box::into_raw(Box::new(FFIManagedWalletInfo::new(managed_info))); // Test get_next_receive_address with valid pointers @@ -581,7 +581,7 @@ mod tests { // We need to work with the existing wallet structure // Create managed wallet info from the existing wallet - let mut managed_info = ManagedWalletInfo::from_wallet(wallet_arc); + let mut managed_info = ManagedWalletInfo::from_wallet(wallet_arc, 0); let network = key_wallet::Network::Testnet; @@ -759,7 +759,7 @@ mod tests { // Create managed wallet info let wallet_arc = unsafe { &(*wallet_ptr).wallet }; - let mut managed_info = ManagedWalletInfo::from_wallet(wallet_arc); + let mut managed_info = ManagedWalletInfo::from_wallet(wallet_arc, 0); // Set some test balance values managed_info.balance = WalletCoreBalance::new(1000000, 50000, 10000, 25000); diff --git a/key-wallet-ffi/src/transaction.rs b/key-wallet-ffi/src/transaction.rs index e7d801ed3..a00b28ee5 100644 --- a/key-wallet-ffi/src/transaction.rs +++ b/key-wallet-ffi/src/transaction.rs @@ -306,7 +306,7 @@ pub unsafe extern "C" fn wallet_check_transaction( use key_wallet::transaction_checking::WalletTransactionChecker; use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; - let mut managed_info = ManagedWalletInfo::from_wallet(wallet.inner()); + let mut managed_info = ManagedWalletInfo::from_wallet(wallet.inner(), 0); // Check the transaction - wallet is always required now let wallet_mut = unwrap_or_return!(wallet.inner_mut(), error); diff --git a/key-wallet-ffi/src/transaction_checking.rs b/key-wallet-ffi/src/transaction_checking.rs index 1875a89f0..774273d26 100644 --- a/key-wallet-ffi/src/transaction_checking.rs +++ b/key-wallet-ffi/src/transaction_checking.rs @@ -79,7 +79,7 @@ pub unsafe extern "C" fn wallet_create_managed_wallet( error: *mut FFIError, ) -> *mut FFIManagedWalletInfo { let wallet = deref_ptr!(wallet, error); - let managed_info = ManagedWalletInfo::from_wallet(wallet.inner()); + let managed_info = ManagedWalletInfo::from_wallet(wallet.inner(), 0); Box::into_raw(Box::new(FFIManagedWalletInfo::new(managed_info))) } diff --git a/key-wallet-manager/src/accessors.rs b/key-wallet-manager/src/accessors.rs index ff7353c05..db66cb763 100644 --- a/key-wallet-manager/src/accessors.rs +++ b/key-wallet-manager/src/accessors.rs @@ -249,7 +249,7 @@ mod tests { fn insert_wallet_rejects_duplicate() { let mut manager: WalletManager = WalletManager::new(Network::Testnet); let wallet = build_wallet(); - let info = ManagedWalletInfo::from_wallet(&wallet); + let info = ManagedWalletInfo::from_wallet(&wallet, 0); let id = manager.insert_wallet(wallet.clone(), info.clone()).expect("first insert succeeds"); diff --git a/key-wallet-manager/src/lib.rs b/key-wallet-manager/src/lib.rs index 4e9ed5056..9f60bf125 100644 --- a/key-wallet-manager/src/lib.rs +++ b/key-wallet-manager/src/lib.rs @@ -170,8 +170,7 @@ impl WalletManager { // 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); + let mut managed_info = T::from_wallet(&wallet, birth_height); managed_info.set_first_loaded_at(current_timestamp()); // The wallet already has accounts created according to the provided options @@ -276,8 +275,7 @@ impl WalletManager { })?; // Add the wallet to the manager - let mut managed_info = T::from_wallet(&final_wallet); - managed_info.set_birth_height(birth_height); + let mut managed_info = T::from_wallet(&final_wallet, birth_height); managed_info.set_first_loaded_at(current_timestamp()); self.wallets.insert(wallet_id, final_wallet); @@ -311,8 +309,7 @@ impl WalletManager { } // Create managed wallet info - let mut managed_info = T::from_wallet(&wallet); - managed_info.set_birth_height(self.last_processed_height()); + let mut managed_info = T::from_wallet(&wallet, self.last_processed_height()); managed_info.set_first_loaded_at(current_timestamp()); self.wallets.insert(wallet_id, wallet); @@ -352,8 +349,7 @@ impl WalletManager { } // Create managed wallet info - let mut managed_info = T::from_wallet(&wallet); - managed_info.set_birth_height(self.last_processed_height()); + let mut managed_info = T::from_wallet(&wallet, self.last_processed_height()); managed_info.set_first_loaded_at(current_timestamp()); self.wallets.insert(wallet_id, wallet); @@ -400,8 +396,7 @@ impl WalletManager { } // Create managed wallet info - let mut managed_info = T::from_wallet(&wallet); - managed_info.set_birth_height(self.last_processed_height()); + let mut managed_info = T::from_wallet(&wallet, self.last_processed_height()); managed_info.set_first_loaded_at(current_timestamp()); self.wallets.insert(wallet_id, wallet); @@ -442,11 +437,9 @@ impl WalletManager { return Err(WalletError::WalletExists(wallet_id)); } - // Create managed wallet info from the imported wallet - let mut managed_info = 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.last_processed_height()); + // Create managed wallet info from the imported wallet, using the current chain + // tip as the birth height since the serialized form does not preserve it. + let mut managed_info = T::from_wallet(&wallet, self.last_processed_height()); managed_info.set_first_loaded_at(current_timestamp()); self.wallets.insert(wallet_id, wallet); diff --git a/key-wallet-manager/tests/integration_test.rs b/key-wallet-manager/tests/integration_test.rs index fe0047564..43a5e8b34 100644 --- a/key-wallet-manager/tests/integration_test.rs +++ b/key-wallet-manager/tests/integration_test.rs @@ -181,19 +181,24 @@ fn test_block_height_tracking() { .create_wallet_from_mnemonic( &mnemonic2.to_string(), "", - 0, + 5000, WalletAccountCreationOptions::Default, ) .unwrap(); assert_eq!(manager.wallet_count(), 2); - // Both wallets initialized with `synced_height = birth_height - 1 = 0`, - // so neither has been processed past genesis. - for wallet_info in manager.get_all_wallet_infos().values() { - assert_eq!(wallet_info.last_processed_height(), 0); - assert_eq!(wallet_info.synced_height(), 0); - } + // wallet1 was created with birth_height = 0 so its sync checkpoint stays at 0; + // wallet2 was created with birth_height = 5000 and is seeded to birth_height - 1. + let wallet_info1 = manager.get_wallet_info(&wallet_id1).unwrap(); + let wallet_info2 = manager.get_wallet_info(&wallet_id2).unwrap(); + assert_eq!(wallet_info1.last_processed_height(), 0); + assert_eq!(wallet_info1.synced_height(), 0); + assert_eq!(wallet_info2.last_processed_height(), 4999); + assert_eq!(wallet_info2.synced_height(), 4999); + // Manager aggregates: synced_height is the min, last_processed_height is the max. + assert_eq!(manager.synced_height(), 0); + assert_eq!(manager.last_processed_height(), 4999); // Per-wallet last-processed updates only touch the addressed wallet. manager.update_wallet_last_processed_height(&wallet_id1, 12345); @@ -201,16 +206,16 @@ fn test_block_height_tracking() { let wallet_info1 = manager.get_wallet_info(&wallet_id1).unwrap(); let wallet_info2 = manager.get_wallet_info(&wallet_id2).unwrap(); assert_eq!(wallet_info1.last_processed_height(), 12345); - assert_eq!(wallet_info2.last_processed_height(), 0); + assert_eq!(wallet_info2.last_processed_height(), 4999); // Per-wallet synced-height updates only touch the addressed wallet. manager.update_wallet_synced_height(&wallet_id1, 12000); 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(), 12000); - assert_eq!(wallet_info2.synced_height(), 0); - // Aggregate `synced_height()` is `min` across wallets, so wallet 2 holds it at 0. - assert_eq!(manager.synced_height(), 0); + assert_eq!(wallet_info2.synced_height(), 4999); + // Aggregate `synced_height()` is `min` across wallets, so wallet 2 holds it at 4999. + assert_eq!(manager.synced_height(), 4999); // Advance wallet 2 too. Aggregate min jumps to wallet 2's new value. manager.update_wallet_synced_height(&wallet_id2, 11000); diff --git a/key-wallet-manager/tests/test_serialized_wallets.rs b/key-wallet-manager/tests/test_serialized_wallets.rs index 015f19c7c..b79d5d84b 100644 --- a/key-wallet-manager/tests/test_serialized_wallets.rs +++ b/key-wallet-manager/tests/test_serialized_wallets.rs @@ -2,6 +2,7 @@ #[cfg(test)] mod tests { 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::WalletManager; @@ -26,6 +27,12 @@ mod tests { assert!(!bytes.is_empty()); println!("Full wallet ID: {}", hex::encode(wallet_id)); + // The wallet's sync checkpoint should be seeded to birth_height - 1. + let info = manager.get_wallet_info(&wallet_id).unwrap(); + assert_eq!(info.birth_height(), 100_000); + assert_eq!(info.synced_height(), 99_999); + assert_eq!(info.last_processed_height(), 99_999); + // Test 2: Create watch-only wallet (no private keys) let mut manager2 = WalletManager::::new(Network::Testnet); let result = manager2.create_wallet_from_mnemonic_return_serialized_bytes( diff --git a/key-wallet/src/test_utils/wallet.rs b/key-wallet/src/test_utils/wallet.rs index 139bca640..2d342d3bf 100644 --- a/key-wallet/src/test_utils/wallet.rs +++ b/key-wallet/src/test_utils/wallet.rs @@ -31,7 +31,7 @@ impl TestWalletContext { let wallet = Wallet::new_random(Network::Testnet, WalletAccountCreationOptions::Default) .expect("Should create wallet"); let mut managed_wallet = - ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string()); + ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0); let xpub = wallet .accounts 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..0f4491534 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 @@ -44,7 +44,7 @@ async fn test_identity_registration_account_routing() { wallet.add_account(account_type, None).expect("Failed to add account to wallet"); let mut managed_wallet_info = - ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string()); + ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0); // Get the identity registration account let account = wallet @@ -155,7 +155,7 @@ async fn test_normal_payment_to_identity_address_not_detected() { let mut 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()); + ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0); let account = wallet .accounts 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..2a62257d4 100644 --- a/key-wallet/src/transaction_checking/transaction_router/tests/provider.rs +++ b/key-wallet/src/transaction_checking/transaction_router/tests/provider.rs @@ -136,10 +136,10 @@ async fn test_provider_registration_transaction_routing_check_owner_only() { .expect("Failed to create wallet with default options"); let mut other_managed_wallet_info = - ManagedWalletInfo::from_wallet_with_name(&other_wallet, "Other".to_string()); + ManagedWalletInfo::from_wallet_with_name(&other_wallet, "Other".to_string(), 0); let mut managed_wallet_info = - ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string()); + ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0); // Get addresses from provider accounts let managed_owner = managed_wallet_info @@ -271,10 +271,10 @@ async fn test_provider_registration_transaction_routing_check_voting_only() { .expect("Failed to create wallet with default options"); let mut other_managed_wallet_info = - ManagedWalletInfo::from_wallet_with_name(&other_wallet, "Other".to_string()); + ManagedWalletInfo::from_wallet_with_name(&other_wallet, "Other".to_string(), 0); let mut managed_wallet_info = - ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string()); + ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0); // Get addresses from provider accounts let owner_address = other_managed_wallet_info @@ -406,10 +406,10 @@ async fn test_provider_registration_transaction_routing_check_operator_only() { .expect("Failed to create wallet with default options"); let mut other_managed_wallet_info = - ManagedWalletInfo::from_wallet_with_name(&other_wallet, "Other".to_string()); + ManagedWalletInfo::from_wallet_with_name(&other_wallet, "Other".to_string(), 0); let mut managed_wallet_info = - ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string()); + ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0); // Get addresses from provider accounts let owner_address = other_managed_wallet_info @@ -587,10 +587,10 @@ async fn test_provider_registration_transaction_routing_check_platform_only() { .expect("Failed to create wallet with default options"); let mut other_managed_wallet_info = - ManagedWalletInfo::from_wallet_with_name(&other_wallet, "Other".to_string()); + ManagedWalletInfo::from_wallet_with_name(&other_wallet, "Other".to_string(), 0); let mut managed_wallet_info = - ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string()); + ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0); // Get addresses from provider accounts let owner_address = other_managed_wallet_info @@ -785,7 +785,7 @@ async fn test_provider_update_registrar_with_voting_and_operator() { .expect("Failed to create wallet with default options"); let mut managed_wallet_info = - ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string()); + ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0); // Get voting address let voting_address = managed_wallet_info @@ -858,7 +858,7 @@ async fn test_provider_revocation_classification_and_routing() { .expect("Failed to create wallet with default options"); let mut managed_wallet_info = - ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string()); + ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0); // Get a standard address for collateral return let account = wallet 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 85a9ce6df..0e58de50d 100644 --- a/key-wallet/src/transaction_checking/transaction_router/tests/routing.rs +++ b/key-wallet/src/transaction_checking/transaction_router/tests/routing.rs @@ -77,7 +77,7 @@ async fn test_transaction_routing_to_bip32_account() { wallet.add_account(account_type, None).expect("Failed to add account to wallet"); let mut managed_wallet_info = - ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string()); + ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0); // Get the account's xpub for address derivation let account = wallet @@ -159,7 +159,7 @@ async fn test_transaction_routing_to_coinjoin_account() { wallet.add_account(account_type, None).expect("Failed to add account to wallet"); let mut managed_wallet_info = - ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string()); + ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0); // Get the account's xpub let account = wallet @@ -258,7 +258,7 @@ async fn test_transaction_affects_multiple_accounts() { wallet.add_account(account_type, None).expect("Failed to add account to wallet"); let mut managed_wallet_info = - ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string()); + ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0); // Get addresses from different accounts @@ -362,7 +362,7 @@ fn test_next_address_method_restrictions() { let wallet = Wallet::new_random(Network::Testnet, WalletAccountCreationOptions::Default) .expect("Failed to create wallet with default options"); let mut managed_wallet_info = - ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string()); + ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0); // Test that standard BIP44 accounts reject next_address { diff --git a/key-wallet/src/transaction_checking/wallet_checker.rs b/key-wallet/src/transaction_checking/wallet_checker.rs index 5872a46c0..0d237464e 100644 --- a/key-wallet/src/transaction_checking/wallet_checker.rs +++ b/key-wallet/src/transaction_checking/wallet_checker.rs @@ -249,7 +249,7 @@ mod tests { .expect("Should create wallet"); let mut managed_wallet = - ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string()); + ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0); // Create a transaction to an external address let dummy_address = Address::p2pkh( @@ -311,7 +311,7 @@ mod tests { .expect("Should add identity registration account"); let mut managed_wallet = - ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string()); + ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0); // Get BIP32 account address - scope the immutable borrow let (bip32_xpub, bip32_address) = { @@ -1029,7 +1029,7 @@ mod tests { .expect("Should add second BIP44 account"); let mut managed_wallet = - ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string()); + ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0); let xpub0 = wallet .accounts @@ -1638,7 +1638,7 @@ mod tests { .expect("add coinjoin"); let mut managed_wallet = - ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string()); + ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0); let xpub = wallet.accounts.coinjoin_accounts.get(&0).expect("coinjoin account").account_xpub; 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 8dc0b1a9c..85e1e3e19 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 @@ -553,7 +553,7 @@ mod tests { fn test_wallet_and_info() -> (Wallet, ManagedWalletInfo) { let wallet = Wallet::new_random(Network::Testnet, WalletAccountCreationOptions::Default).unwrap(); - let info = ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string()); + let info = ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0); (wallet, info) } diff --git a/key-wallet/src/wallet/managed_wallet_info/mod.rs b/key-wallet/src/wallet/managed_wallet_info/mod.rs index 2fdf07666..9c0da2e17 100644 --- a/key-wallet/src/wallet/managed_wallet_info/mod.rs +++ b/key-wallet/src/wallet/managed_wallet_info/mod.rs @@ -82,34 +82,38 @@ impl ManagedWalletInfo { } /// Create managed wallet info from a Wallet - pub fn from_wallet(wallet: &super::super::Wallet) -> Self { + /// Create managed wallet info from a Wallet, seeding the sync checkpoint at `birth_height`. + /// + /// Sets `birth_height` and seeds both `synced_height` and `last_processed_height` to + /// `birth_height.saturating_sub(1)` so that the next block to scan is `birth_height`. + pub fn from_wallet(wallet: &super::super::Wallet, birth_height: CoreBlockHeight) -> Self { + let initial_height = birth_height.saturating_sub(1); Self { network: wallet.network, wallet_id: wallet.wallet_id, name: None, description: None, - metadata: WalletMetadata::default(), + metadata: WalletMetadata { + birth_height, + synced_height: initial_height, + last_processed_height: initial_height, + ..WalletMetadata::default() + }, accounts: ManagedAccountCollection::from_account_collection(&wallet.accounts), balance: WalletCoreBalance::default(), instant_send_locks: HashSet::new(), } } - /// Create managed wallet info from a Wallet with a name - pub fn from_wallet_with_name(wallet: &super::super::Wallet, name: String) -> Self { - let mut info = Self::from_wallet(wallet); - info.name = Some(name); - info - } - - /// Create managed wallet info with birth height - pub fn with_birth_height( - network: Network, - wallet_id: [u8; 32], + /// Create managed wallet info from a Wallet with a name, seeding the sync checkpoint + /// at `birth_height` (see `from_wallet` for details). + pub fn from_wallet_with_name( + wallet: &super::super::Wallet, + name: String, birth_height: CoreBlockHeight, ) -> Self { - let mut info = Self::new(network, wallet_id); - info.metadata.birth_height = birth_height; + let mut info = Self::from_wallet(wallet, birth_height); + info.name = Some(name); info } 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 548d12fea..52d6848f2 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 @@ -18,13 +18,18 @@ use dashcore::{Address as DashAddress, Transaction, Txid}; /// Trait that wallet info types must implement to work with WalletManager pub trait WalletInfoInterface: Sized + WalletTransactionChecker + ManagedAccountOperations { - /// Create a wallet info from an existing wallet - /// This properly initializes the wallet info from the wallet's state - fn from_wallet(wallet: &Wallet) -> Self; + /// Create a wallet info from an existing wallet, seeding the sync checkpoint at + /// `birth_height`. + /// + /// Both `synced_height` and `last_processed_height` are seeded to + /// `birth_height.saturating_sub(1)` so the next block to scan is `birth_height`. + /// Taking `birth_height` at construction makes the sync checkpoint a required + /// invariant of the type rather than something callers have to remember to set. + fn from_wallet(wallet: &Wallet, birth_height: CoreBlockHeight) -> Self; - /// Create a wallet info from an existing wallet with proper account initialization - /// Default implementation just uses with_name (backward compatibility) - fn from_wallet_with_name(wallet: &Wallet, name: String) -> Self; + /// Create a wallet info with a name, seeding the sync checkpoint at `birth_height` + /// (see `from_wallet` for details). + fn from_wallet_with_name(wallet: &Wallet, name: String, birth_height: CoreBlockHeight) -> Self; /// Get the wallet's network fn network(&self) -> Network; @@ -125,12 +130,12 @@ pub trait WalletInfoInterface: Sized + WalletTransactionChecker + ManagedAccount /// 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(wallet: &Wallet, birth_height: CoreBlockHeight) -> Self { + Self::from_wallet(wallet, birth_height) } - fn from_wallet_with_name(wallet: &Wallet, name: String) -> Self { - Self::from_wallet_with_name(wallet, name) + fn from_wallet_with_name(wallet: &Wallet, name: String, birth_height: CoreBlockHeight) -> Self { + Self::from_wallet_with_name(wallet, name, birth_height) } fn network(&self) -> Network { diff --git a/key-wallet/src/wallet/mod.rs b/key-wallet/src/wallet/mod.rs index 91281bfa5..f2d044b09 100644 --- a/key-wallet/src/wallet/mod.rs +++ b/key-wallet/src/wallet/mod.rs @@ -413,7 +413,7 @@ mod tests { .unwrap(); // Create managed info from the wallet - let mut managed_info = ManagedWalletInfo::from_wallet(&wallet); + let mut managed_info = ManagedWalletInfo::from_wallet(&wallet, 0); managed_info.set_name("Test Wallet".to_string()); managed_info.set_description(Some("A test wallet".to_string())); From 0fa8a19e9fc92bc891c38b83ae36dfd952b65ba0 Mon Sep 17 00:00:00 2001 From: xdustinface Date: Wed, 29 Apr 2026 09:40:49 +1000 Subject: [PATCH 2/4] docs(key-wallet-manager): clarify fallback `birth_height` source on wallet import Addresses CodeRabbit review comment on PR #692 https://github.com/dashpay/rust-dashcore/pull/692#discussion_r3154441733 --- key-wallet-manager/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/key-wallet-manager/src/lib.rs b/key-wallet-manager/src/lib.rs index 9f60bf125..1f050e7c2 100644 --- a/key-wallet-manager/src/lib.rs +++ b/key-wallet-manager/src/lib.rs @@ -437,8 +437,9 @@ impl WalletManager { return Err(WalletError::WalletExists(wallet_id)); } - // Create managed wallet info from the imported wallet, using the current chain - // tip as the birth height since the serialized form does not preserve it. + // Create managed wallet info from the imported wallet, using the manager's + // current aggregated last-processed height as the fallback birth height + // since the serialized form does not preserve it. let mut managed_info = T::from_wallet(&wallet, self.last_processed_height()); managed_info.set_first_loaded_at(current_timestamp()); From 6235f1527e65dcd589e5b592fa6d137f93fa66b9 Mon Sep 17 00:00:00 2001 From: xdustinface Date: Wed, 29 Apr 2026 09:42:25 +1000 Subject: [PATCH 3/4] refactor(key-wallet): drop `WalletInfoInterface::set_birth_height` The setter only updated `birth_height`, leaving `synced_height` and `last_processed_height` at their previous values, so a caller using it after construction would silently bypass the seeding invariant established by `from_wallet`. With the per-wallet height tracking in #694 that aggregate to a min/max across wallets, this could drag the manager's `synced_height` back to genesis. After the constructor change, the setter has zero callers in-tree, so removing it is the cleanest way to enforce the invariant: `birth_height` is fixed at construction via `from_wallet`. Addresses CodeRabbit review comment on PR #692 https://github.com/dashpay/rust-dashcore/pull/692#discussion_r3154441754 --- .../wallet/managed_wallet_info/wallet_info_interface.rs | 7 ------- 1 file changed, 7 deletions(-) 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 52d6848f2..568e228c1 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 @@ -52,9 +52,6 @@ pub trait WalletInfoInterface: Sized + WalletTransactionChecker + ManagedAccount /// Get the birth height of the wallet fn birth_height(&self) -> CoreBlockHeight; - /// Set the birth height - fn set_birth_height(&mut self, height: CoreBlockHeight); - /// Get the timestamp when first loaded fn first_loaded_at(&self) -> u64; @@ -166,10 +163,6 @@ impl WalletInfoInterface for ManagedWalletInfo { self.metadata.birth_height } - fn set_birth_height(&mut self, height: CoreBlockHeight) { - self.metadata.birth_height = height; - } - fn last_processed_height(&self) -> CoreBlockHeight { self.metadata.last_processed_height } From 77abbbbe50395e108769fdeb5667dc0300fb18bb Mon Sep 17 00:00:00 2001 From: xdustinface Date: Wed, 29 Apr 2026 09:43:14 +1000 Subject: [PATCH 4/4] test(key-wallet): pin off-by-one seed contract in `test_wallet_with_managed_info` Use a non-zero `birth_height` and assert that `metadata.synced_height` and `metadata.last_processed_height` land at `birth_height - 1`, locking the seeding invariant in place. Addresses CodeRabbit review comment on PR #692 https://github.com/dashpay/rust-dashcore/pull/692#pullrequestreview-4189162764 --- key-wallet/src/wallet/mod.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/key-wallet/src/wallet/mod.rs b/key-wallet/src/wallet/mod.rs index f2d044b09..8e79f2682 100644 --- a/key-wallet/src/wallet/mod.rs +++ b/key-wallet/src/wallet/mod.rs @@ -412,8 +412,9 @@ mod tests { ) .unwrap(); - // Create managed info from the wallet - let mut managed_info = ManagedWalletInfo::from_wallet(&wallet, 0); + // Create managed info from the wallet with a non-zero birth height so the + // seeded checkpoints are observable. + let mut managed_info = ManagedWalletInfo::from_wallet(&wallet, 100); managed_info.set_name("Test Wallet".to_string()); managed_info.set_description(Some("A test wallet".to_string())); @@ -421,6 +422,9 @@ mod tests { assert_eq!(managed_info.wallet_id, wallet.wallet_id); assert_eq!(managed_info.name.as_ref().unwrap(), "Test Wallet"); assert_eq!(managed_info.description.as_ref().unwrap(), "A test wallet"); + assert_eq!(managed_info.metadata.birth_height, 100); + assert_eq!(managed_info.metadata.synced_height, 99); + assert_eq!(managed_info.metadata.last_processed_height, 99); assert_eq!(managed_info.metadata.first_loaded_at, 0); // Default value assert!(managed_info.metadata.last_synced.is_none());