diff --git a/dash-spv-ffi/src/bin/ffi_cli.rs b/dash-spv-ffi/src/bin/ffi_cli.rs index 3c412eb93..42a766943 100644 --- a/dash-spv-ffi/src/bin/ffi_cli.rs +++ b/dash-spv-ffi/src/bin/ffi_cli.rs @@ -172,12 +172,15 @@ fn read_balance(balance: *const FFIBalance) -> FFIBalance { unsafe { *balance } } +#[allow(clippy::too_many_arguments)] extern "C" fn on_transaction_detected( wallet_id: *const c_char, record: *const FFITransactionRecord, balance: *const FFIBalance, _account_balances: *const dash_spv_ffi::FFIAccountBalance, account_balances_count: u32, + _addresses_derived: *const dash_spv_ffi::FFIDerivedAddress, + addresses_derived_count: u32, _user_data: *mut c_void, ) { let wallet_short = short_wallet(wallet_id); @@ -189,7 +192,7 @@ extern "C" fn on_transaction_detected( let b = read_balance(balance); let txid_hex = hex::encode(r.txid); println!( - "[Wallet] TX detected: wallet={}..., txid={}, account_kind={:?}, account_index={}, amount={} duffs, balance[confirmed={}, unconfirmed={}], changed_accounts={}", + "[Wallet] TX detected: wallet={}..., txid={}, account_kind={:?}, account_index={}, amount={} duffs, balance[confirmed={}, unconfirmed={}], changed_accounts={}, derived={}", wallet_short, txid_hex, r.account_type.kind, @@ -198,6 +201,7 @@ extern "C" fn on_transaction_detected( b.confirmed, b.unconfirmed, account_balances_count, + addresses_derived_count, ); } @@ -243,12 +247,14 @@ extern "C" fn on_wallet_block_processed( balance: *const FFIBalance, _account_balances: *const dash_spv_ffi::FFIAccountBalance, account_balances_count: u32, + _addresses_derived: *const dash_spv_ffi::FFIDerivedAddress, + addresses_derived_count: u32, _user_data: *mut c_void, ) { let wallet_short = short_wallet(wallet_id); let b = read_balance(balance); println!( - "[Wallet] Block processed: wallet={}..., height={}, inserted={}, updated={}, matured={}, balance[confirmed={}, unconfirmed={}, immature={}, locked={}], changed_accounts={}", + "[Wallet] Block processed: wallet={}..., height={}, inserted={}, updated={}, matured={}, balance[confirmed={}, unconfirmed={}, immature={}, locked={}], changed_accounts={}, derived={}", wallet_short, height, inserted_count, @@ -259,6 +265,7 @@ extern "C" fn on_wallet_block_processed( b.immature, b.locked, account_balances_count, + addresses_derived_count, ); } diff --git a/dash-spv-ffi/src/callbacks.rs b/dash-spv-ffi/src/callbacks.rs index 794dba2e0..cad8c20d0 100644 --- a/dash-spv-ffi/src/callbacks.rs +++ b/dash-spv-ffi/src/callbacks.rs @@ -567,6 +567,106 @@ impl FFIAccountBalance { } } +// ============================================================================ +// FFIDerivedAddress - One address derived during gap-limit maintenance +// ============================================================================ + +/// Pool the derived address belongs to. +/// +/// Mirrors `key_wallet::managed_account::address_pool::AddressPoolType` +/// 1:1 — kept distinct from the existing `FFIAddressPoolType` (which +/// collapses Absent / AbsentHardened into a single `Single` variant) so +/// event consumers can distinguish hardened single-pool variants +/// (Provider operator keys, etc.) from non-hardened ones. +#[repr(C)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum FFIDerivedAddressPoolType { + External = 0, + Internal = 1, + Absent = 2, + AbsentHardened = 3, +} + +impl From + for FFIDerivedAddressPoolType +{ + fn from(t: key_wallet::managed_account::address_pool::AddressPoolType) -> Self { + use key_wallet::managed_account::address_pool::AddressPoolType as P; + match t { + P::External => FFIDerivedAddressPoolType::External, + P::Internal => FFIDerivedAddressPoolType::Internal, + P::Absent => FFIDerivedAddressPoolType::Absent, + P::AbsentHardened => FFIDerivedAddressPoolType::AbsentHardened, + } + } +} + +/// One address derived as a side effect of gap-limit maintenance during +/// transaction or block processing. +/// +/// Wallet events deliver an array of these so persisters can mirror the +/// on-disk address pool transactionally with the tx/block records that +/// triggered the derivation. Without this, UTXOs landing on freshly +/// derived addresses orphan their parent address row at the persister. +/// +/// `account_type` follows the same memory rules as on +/// [`FFIAccountBalance`]: the embedded `identity_user` / `identity_friend` +/// pointers are owned by the `FFIAccountType` and freed when the array is +/// dropped after the callback returns. `address` is a heap-allocated +/// null-terminated UTF-8 string, owned by this struct and freed on drop. +/// Consumers that need to retain the data past the callback must copy +/// every owning field — not just retain pointers. +#[repr(C)] +pub struct FFIDerivedAddress { + /// Owning-account descriptor (discriminant + indices + identity ids). + pub account_type: FFIAccountType, + /// Pool within the account that derived this address. + pub pool_type: FFIDerivedAddressPoolType, + /// Derivation index within the pool. Combined with `account_type` + /// and `pool_type`, this fully determines the derivation path — + /// consumers that need a rendered path can recompute it + /// deterministically. + pub derivation_index: u32, + /// Heap-allocated null-terminated UTF-8 string. Owned by this + /// struct; freed when the struct is dropped. + pub address: *mut c_char, + /// 33-byte compressed ECDSA public key (inline, no allocation). + pub public_key: [u8; 33], +} + +impl FFIDerivedAddress { + fn from_slice(addresses: &[key_wallet_manager::DerivedAddress]) -> Vec { + addresses + .iter() + .map(|d| { + let address_str = d.address.to_string(); + let c_address = CString::new(address_str).unwrap_or_else(|_| CString::default()); + FFIDerivedAddress { + account_type: FFIAccountType::from(&d.account_type), + pool_type: FFIDerivedAddressPoolType::from(d.pool_type), + derivation_index: d.derivation_index, + address: c_address.into_raw(), + public_key: d.public_key, + } + }) + .collect() + } +} + +impl Drop for FFIDerivedAddress { + fn drop(&mut self) { + if !self.address.is_null() { + // SAFETY: `address` was constructed via `CString::into_raw` in + // `FFIDerivedAddress::from_slice`, so reclaiming via + // `CString::from_raw` is the matching free. + let _ = unsafe { CString::from_raw(self.address) }; + self.address = std::ptr::null_mut(); + } + // `account_type` has its own Drop impl that frees the + // identity_user / identity_friend allocations when applicable. + } +} + // ============================================================================ // FFIWalletEventCallbacks - One callback per WalletEvent variant // ============================================================================ @@ -584,6 +684,13 @@ impl FFIAccountBalance { /// entries for a normal transaction); accounts whose balance is unchanged /// are omitted. The array is null with a zero count when no per-account /// balance changed. +/// +/// `addresses_derived` is an array of size `addresses_derived_count` of +/// addresses derived as a side effect of gap-limit maintenance while +/// processing this transaction, attributed to the same account as +/// `record`. Empty in the common case (null pointer with zero count). +/// Persisters should write these rows transactionally with `record` so +/// UTXOs landing on freshly-derived addresses retain a parent row. pub type OnTransactionDetectedCallback = Option< extern "C" fn( wallet_id: *const c_char, @@ -591,6 +698,8 @@ pub type OnTransactionDetectedCallback = Option< balance: *const FFIBalance, account_balances: *const FFIAccountBalance, account_balances_count: u32, + addresses_derived: *const FFIDerivedAddress, + addresses_derived_count: u32, user_data: *mut c_void, ), >; @@ -629,6 +738,13 @@ pub type OnTransactionInstantLockedCallback = Option< /// balance *after* the block was processed. `account_balances` follows the /// same contract as on [`OnTransactionDetectedCallback`]. /// +/// `addresses_derived` is an array of size `addresses_derived_count` of +/// addresses derived as a side effect of gap-limit maintenance across +/// every record in the block, deduplicated by +/// `(account_type, pool_type, derivation_index)`. Empty in the common +/// case (null pointer with zero count). Persisters should write these +/// rows transactionally with the inserted/updated records. +/// /// All array pointers and their contents are borrowed and only valid for the /// duration of the callback. pub type OnWalletBlockProcessedCallback = Option< @@ -644,6 +760,8 @@ pub type OnWalletBlockProcessedCallback = Option< balance: *const FFIBalance, account_balances: *const FFIAccountBalance, account_balances_count: u32, + addresses_derived: *const FFIDerivedAddress, + addresses_derived_count: u32, user_data: *mut c_void, ), >; @@ -784,6 +902,7 @@ impl FFIWalletEventCallbacks { record, balance, account_balances, + addresses_derived, } => { if let Some(cb) = self.on_transaction_detected { let wallet_id_hex = hex::encode(wallet_id); @@ -791,11 +910,17 @@ impl FFIWalletEventCallbacks { let ffi_record = FFITransactionRecord::from(record.as_ref()); let ffi_balance = FFIBalance::from(*balance); let ffi_account_balances = FFIAccountBalance::from_map(account_balances); + let ffi_addresses_derived = FFIDerivedAddress::from_slice(addresses_derived); let account_balances_ptr = if ffi_account_balances.is_empty() { ptr::null() } else { ffi_account_balances.as_ptr() }; + let addresses_derived_ptr = if ffi_addresses_derived.is_empty() { + ptr::null() + } else { + ffi_addresses_derived.as_ptr() + }; cb( c_wallet_id.as_ptr(), @@ -803,10 +928,13 @@ impl FFIWalletEventCallbacks { &ffi_balance as *const FFIBalance, account_balances_ptr, ffi_account_balances.len() as u32, + addresses_derived_ptr, + ffi_addresses_derived.len() as u32, self.user_data, ); drop(ffi_account_balances); + drop(ffi_addresses_derived); } } WalletEvent::TransactionInstantLocked { @@ -851,6 +979,7 @@ impl FFIWalletEventCallbacks { matured, balance, account_balances, + addresses_derived, } => { if let Some(cb) = self.on_block_processed { let wallet_id_hex = hex::encode(wallet_id); @@ -863,6 +992,7 @@ impl FFIWalletEventCallbacks { matured.iter().map(FFITransactionRecord::from).collect(); let ffi_balance = FFIBalance::from(*balance); let ffi_account_balances = FFIAccountBalance::from_map(account_balances); + let ffi_addresses_derived = FFIDerivedAddress::from_slice(addresses_derived); // Pass a null pointer when an array is empty so C/Swift // consumers that null-check before reading don't see a @@ -887,6 +1017,11 @@ impl FFIWalletEventCallbacks { } else { ffi_account_balances.as_ptr() }; + let addresses_derived_ptr = if ffi_addresses_derived.is_empty() { + ptr::null() + } else { + ffi_addresses_derived.as_ptr() + }; cb( c_wallet_id.as_ptr(), @@ -900,6 +1035,8 @@ impl FFIWalletEventCallbacks { &ffi_balance as *const FFIBalance, account_balances_ptr, ffi_account_balances.len() as u32, + addresses_derived_ptr, + ffi_addresses_derived.len() as u32, self.user_data, ); @@ -907,6 +1044,7 @@ impl FFIWalletEventCallbacks { drop(ffi_updated); drop(ffi_matured); drop(ffi_account_balances); + drop(ffi_addresses_derived); } } WalletEvent::SyncHeightAdvanced { diff --git a/dash-spv-ffi/tests/dashd_sync/callbacks.rs b/dash-spv-ffi/tests/dashd_sync/callbacks.rs index fd18ea14e..b207a5977 100644 --- a/dash-spv-ffi/tests/dashd_sync/callbacks.rs +++ b/dash-spv-ffi/tests/dashd_sync/callbacks.rs @@ -421,6 +421,8 @@ extern "C" fn on_transaction_detected( balance: *const FFIBalance, account_balances: *const FFIAccountBalance, account_balances_count: u32, + _addresses_derived: *const dash_spv_ffi::FFIDerivedAddress, + _addresses_derived_count: u32, user_data: *mut c_void, ) { let Some(tracker) = (unsafe { tracker_from(user_data) }) else { @@ -493,6 +495,8 @@ extern "C" fn on_wallet_block_processed( balance: *const FFIBalance, account_balances: *const FFIAccountBalance, account_balances_count: u32, + _addresses_derived: *const dash_spv_ffi::FFIDerivedAddress, + _addresses_derived_count: u32, user_data: *mut c_void, ) { let Some(tracker) = (unsafe { tracker_from(user_data) }) else { diff --git a/key-wallet-manager/src/event_tests.rs b/key-wallet-manager/src/event_tests.rs index fed3ced25..37f2752a3 100644 --- a/key-wallet-manager/src/event_tests.rs +++ b/key-wallet-manager/src/event_tests.rs @@ -14,6 +14,8 @@ use dashcore::{ BlockHash, CompactTarget, OutPoint, ScriptBuf, TxIn, TxMerkleNode, TxOut, Txid, Witness, }; use key_wallet::account::StandardAccountType; +use key_wallet::managed_account::address_pool::AddressPoolType; +use key_wallet::managed_account::managed_account_type::ManagedAccountType; use key_wallet::AccountType; use std::collections::BTreeSet; @@ -72,6 +74,7 @@ async fn test_mempool_tx_emits_single_event_with_balance() { record, balance, account_balances, + addresses_derived: _, } => { assert_eq!(*wid, wallet_id); assert_eq!(record.txid, tx.txid()); @@ -124,6 +127,7 @@ async fn test_mempool_tx_with_instant_lock_emits_detected_event_with_locked_bala record, balance, account_balances, + addresses_derived: _, } => { assert_eq!(*wid, wallet_id); assert!(matches!(record.context, TransactionContext::InstantSend(_))); @@ -338,6 +342,7 @@ async fn test_block_with_new_tx_emits_inserted_record() { matured, balance, account_balances, + addresses_derived: _, } => { assert_eq!(*wid, wallet_id); assert_eq!(*height, 100); @@ -402,6 +407,7 @@ async fn test_block_confirming_known_mempool_tx_emits_updated_record() { matured, balance, account_balances, + addresses_derived: _, } => { assert_eq!(*wid, wallet_id); assert_eq!(*height, 200); @@ -669,3 +675,369 @@ async fn test_check_transaction_dry_run_does_not_persist_state() { .await; assert!(result.is_new_transaction); } + +// --------------------------------------------------------------------------- +// addresses_derived (gap-limit extension piggy-backed on the event) +// --------------------------------------------------------------------------- + +/// Pull `(highest_generated_index, gap_limit)` and the address at the given +/// index for the BIP44 account 0 pool of the given type. +fn pool_state( + manager: &WalletManager, + wallet_id: &WalletId, + pool_type: AddressPoolType, +) -> (u32, u32, Address) { + let info = manager.get_wallet_info(wallet_id).expect("wallet info"); + let acct = info + .accounts + .standard_bip44_accounts + .get(&0) + .expect("BIP44 account 0 should exist on the default test wallet"); + let pool = match (acct.managed_account_type(), pool_type) { + ( + ManagedAccountType::Standard { + external_addresses, + .. + }, + AddressPoolType::External, + ) => external_addresses, + ( + ManagedAccountType::Standard { + internal_addresses, + .. + }, + AddressPoolType::Internal, + ) => internal_addresses, + _ => panic!("unexpected pool type {:?}", pool_type), + }; + let highest = pool.highest_generated.expect("pre-generated pool must have addresses"); + let gap_limit = pool.gap_limit; + let addr = pool.address_at_index(highest).expect("highest index must exist"); + (highest, gap_limit, addr) +} + +/// Build a tx whose only output pays to `addr`. `seed` differentiates the +/// input prevout so two different txs can pay to the same address without +/// being deduped on `txid`. +fn create_tx_paying_to_with_input_seed(addr: &Address, txid_seed: u8, vout: u32) -> Transaction { + Transaction { + version: 2, + lock_time: 0, + input: vec![TxIn { + previous_output: OutPoint { + txid: Txid::from_byte_array([txid_seed; 32]), + vout, + }, + script_sig: ScriptBuf::new(), + sequence: u32::MAX, + witness: Witness::default(), + }], + output: vec![TxOut { + value: TX_AMOUNT, + script_pubkey: addr.script_pubkey(), + }], + special_transaction_payload: None, + } +} + +#[tokio::test] +async fn test_mempool_tx_to_highest_external_carries_addresses_derived() { + let (mut manager, wallet_id, _addr) = setup_manager_with_wallet(); + let (highest_before, gap_limit, highest_addr) = + pool_state(&manager, &wallet_id, AddressPoolType::External); + + let mut rx = manager.subscribe_events(); + let tx = create_tx_paying_to(&highest_addr, 0xa0); + manager.process_mempool_transaction(&tx, None).await; + + let events = drain_events(&mut rx); + let detected = events + .iter() + .find(|e| matches!(e, WalletEvent::TransactionDetected { .. })) + .unwrap_or_else(|| panic!("expected TransactionDetected, got {:?}", events)); + let WalletEvent::TransactionDetected { + addresses_derived, + .. + } = detected + else { + unreachable!() + }; + + // Receiving to the highest pre-generated External address must extend + // the pool by exactly `gap_limit`, with all entries on the External + // pool of the BIP44 account 0, and contiguous derivation indices + // starting just past `highest_before`. + assert_eq!( + addresses_derived.len() as u32, + gap_limit, + "expected gap_limit ({}) new addresses, got {}", + gap_limit, + addresses_derived.len() + ); + let expected_account = AccountType::Standard { + index: 0, + standard_account_type: StandardAccountType::BIP44Account, + }; + + // Snapshot the pool state *after* extension so we can pin every + // emitted (address, public_key) pair against what the wallet + // actually stored. The persistence contract this PR enforces is + // that each `DerivedAddress` row matches the wallet's + // `AddressInfo` for the same `(account, pool, index)` — drift + // here would silently corrupt downstream `CoreAddress` rows. + let info_after = manager.get_wallet_info(&wallet_id).expect("wallet info"); + let acct_after = info_after.accounts.standard_bip44_accounts.get(&0).expect("BIP44 0"); + let external_pool_after = match acct_after.managed_account_type() { + ManagedAccountType::Standard { + external_addresses, + .. + } => external_addresses, + _ => panic!("expected Standard account"), + }; + + for (i, derived) in addresses_derived.iter().enumerate() { + assert_eq!(derived.account_type, expected_account); + assert_eq!(derived.pool_type, AddressPoolType::External); + assert_eq!( + derived.derivation_index, + highest_before + 1 + i as u32, + "derivation indices must be contiguous starting just past the prior highest" + ); + + // Pin the persistence-critical payload against the wallet's + // own AddressInfo for the same index. + let stored = external_pool_after + .info_at_index(derived.derivation_index) + .unwrap_or_else(|| panic!("pool missing index {}", derived.derivation_index)); + assert_eq!( + derived.address, stored.address, + "address mismatch at index {}", + derived.derivation_index + ); + let stored_pubkey = match stored.public_key.as_ref().expect("ECDSA pool stores pubkey") { + key_wallet::managed_account::address_pool::PublicKeyType::ECDSA(b) => b, + other => panic!("BIP44 external pool produced non-ECDSA key: {:?}", other), + }; + assert_eq!( + stored_pubkey.len(), + 33, + "BIP44 external pool must store 33-byte compressed keys" + ); + assert_eq!( + &derived.public_key[..], + stored_pubkey.as_slice(), + "public key mismatch at index {}", + derived.derivation_index + ); + } +} + +#[tokio::test] +async fn test_mempool_tx_to_already_buffered_external_carries_no_addresses_derived() { + // After a first hit on the highest-index External address, the pool + // is already extended by `gap_limit` past that index. A subsequent + // tx to a LOWER index sits well inside the buffer and must not + // trigger any further derivation. + let (mut manager, wallet_id, _addr) = setup_manager_with_wallet(); + let (highest_before, _gap, highest_addr) = + pool_state(&manager, &wallet_id, AddressPoolType::External); + + // Prime: first tx pushes the boundary and extends the pool. + let tx_prime = create_tx_paying_to(&highest_addr, 0xa1); + manager.process_mempool_transaction(&tx_prime, None).await; + + // Now send a second tx to an index that is well within the new + // buffer (e.g. index 0). The pool is already at highest_before + + // gap_limit; using a lower index does not push it further. + let info = manager.get_wallet_info(&wallet_id).expect("wallet info"); + let acct = info.accounts.standard_bip44_accounts.get(&0).expect("BIP44 0"); + let buffered_addr = match acct.managed_account_type() { + ManagedAccountType::Standard { + external_addresses, + .. + } => external_addresses.address_at_index(0).expect("low-index external address must exist"), + _ => panic!("expected Standard account type"), + }; + assert!( + buffered_addr != highest_addr, + "test setup mismatch: low-index addr should differ from highest" + ); + let _ = highest_before; + + let mut rx = manager.subscribe_events(); + let tx = create_tx_paying_to(&buffered_addr, 0xa2); + manager.process_mempool_transaction(&tx, None).await; + + let events = drain_events(&mut rx); + let detected = events + .iter() + .find(|e| matches!(e, WalletEvent::TransactionDetected { .. })) + .unwrap_or_else(|| panic!("expected TransactionDetected, got {:?}", events)); + let WalletEvent::TransactionDetected { + addresses_derived, + .. + } = detected + else { + unreachable!() + }; + assert!( + addresses_derived.is_empty(), + "no derivation expected when the second hit sits inside the existing buffer, got {:?}", + addresses_derived + ); +} + +#[tokio::test] +async fn test_block_with_external_and_internal_high_index_extends_both_pools() { + let (mut manager, wallet_id, _addr) = setup_manager_with_wallet(); + let (ext_highest_before, ext_gap, ext_highest_addr) = + pool_state(&manager, &wallet_id, AddressPoolType::External); + let (int_highest_before, int_gap, int_highest_addr) = + pool_state(&manager, &wallet_id, AddressPoolType::Internal); + + let tx_ext = create_tx_paying_to(&ext_highest_addr, 0xb0); + let tx_int = create_tx_paying_to(&int_highest_addr, 0xb1); + let block = make_block(vec![tx_ext, tx_int], 0xb2, 6000); + + let mut rx = manager.subscribe_events(); + let wallets = BTreeSet::from([wallet_id]); + manager.process_block_for_wallets(&block, 700, &wallets).await; + + let events = drain_events(&mut rx); + let block_event = events + .iter() + .find(|e| matches!(e, WalletEvent::BlockProcessed { .. })) + .unwrap_or_else(|| panic!("expected BlockProcessed, got {:?}", events)); + let WalletEvent::BlockProcessed { + addresses_derived, + .. + } = block_event + else { + unreachable!() + }; + + let ext_count = + addresses_derived.iter().filter(|d| d.pool_type == AddressPoolType::External).count() + as u32; + let int_count = + addresses_derived.iter().filter(|d| d.pool_type == AddressPoolType::Internal).count() + as u32; + assert_eq!(ext_count, ext_gap, "External pool must extend by gap_limit"); + assert_eq!(int_count, int_gap, "Internal pool must extend by gap_limit"); + + // De-dup invariant: each (pool, index) appears once. + let mut ext_indices: Vec = addresses_derived + .iter() + .filter(|d| d.pool_type == AddressPoolType::External) + .map(|d| d.derivation_index) + .collect(); + ext_indices.sort_unstable(); + ext_indices.dedup(); + assert_eq!(ext_indices.len() as u32, ext_gap); + let expected_first_ext = ext_highest_before + 1; + assert_eq!(ext_indices.first().copied(), Some(expected_first_ext)); + + let mut int_indices: Vec = addresses_derived + .iter() + .filter(|d| d.pool_type == AddressPoolType::Internal) + .map(|d| d.derivation_index) + .collect(); + int_indices.sort_unstable(); + int_indices.dedup(); + assert_eq!(int_indices.len() as u32, int_gap); + let expected_first_int = int_highest_before + 1; + assert_eq!(int_indices.first().copied(), Some(expected_first_int)); +} + +#[tokio::test] +async fn test_block_with_two_records_pushing_external_boundary_dedupes() { + let (mut manager, wallet_id, _addr) = setup_manager_with_wallet(); + let (_, gap_limit, highest_addr) = pool_state(&manager, &wallet_id, AddressPoolType::External); + + // Two distinct txs (different prevouts → different txids) both paying + // to the same highest-index External address. Both records will be + // processed within the same block. The first triggers gap-limit + // extension; the second hits an already-extended boundary and must not + // double-extend. + let tx1 = create_tx_paying_to_with_input_seed(&highest_addr, 0xc0, 0); + let tx2 = create_tx_paying_to_with_input_seed(&highest_addr, 0xc1, 0); + let block = make_block(vec![tx1, tx2], 0xc2, 7000); + + let mut rx = manager.subscribe_events(); + let wallets = BTreeSet::from([wallet_id]); + manager.process_block_for_wallets(&block, 800, &wallets).await; + + let events = drain_events(&mut rx); + let block_event = events + .iter() + .find(|e| matches!(e, WalletEvent::BlockProcessed { .. })) + .unwrap_or_else(|| panic!("expected BlockProcessed, got {:?}", events)); + let WalletEvent::BlockProcessed { + addresses_derived, + .. + } = block_event + else { + unreachable!() + }; + + // Exactly `gap_limit` entries — not `2 * gap_limit`, despite both + // records nominally pushing the boundary. + assert_eq!( + addresses_derived.len() as u32, + gap_limit, + "two records pushing the same boundary must dedup to gap_limit, got {}", + addresses_derived.len() + ); + // And every entry must be distinct on (account, pool, index). + let mut keys: Vec<(AccountType, AddressPoolType, u32)> = addresses_derived + .iter() + .map(|d| (d.account_type, d.pool_type, d.derivation_index)) + .collect(); + keys.sort(); + let total = keys.len(); + keys.dedup(); + assert_eq!(keys.len(), total, "duplicate (account, pool, index) entries leaked through"); +} + +#[tokio::test] +async fn test_instant_send_lock_event_does_not_carry_addresses_derived_field() { + // IS-lock application doesn't extend the pool — addresses are + // already marked used at mempool time. The InstantLocked event + // intentionally has no `addresses_derived` field; this test pins + // that down so a future "defensively add the field everywhere" + // refactor surfaces here. + let (mut manager, wallet_id, addr) = setup_manager_with_wallet(); + let tx = create_tx_paying_to(&addr, 0xd0); + manager.process_mempool_transaction(&tx, None).await; + + let mut rx = manager.subscribe_events(); + let lock = InstantLock { + txid: tx.txid(), + cyclehash: CycleHash::from_byte_array([0xab; 32]), + signature: BLSSignature::from([0xcd; 96]), + ..InstantLock::default() + }; + manager.process_instant_send_lock(lock); + + let events = drain_events(&mut rx); + let lock_event = events + .iter() + .find(|e| matches!(e, WalletEvent::TransactionInstantLocked { .. })) + .unwrap_or_else(|| panic!("expected TransactionInstantLocked, got {:?}", events)); + // Pattern-match on every field; if a future change adds + // `addresses_derived`, this fails to compile and forces a + // deliberate decision. + match lock_event { + WalletEvent::TransactionInstantLocked { + wallet_id: wid, + txid, + instant_lock: _, + balance: _, + account_balances: _, + } => { + assert_eq!(*wid, wallet_id); + assert_eq!(*txid, tx.txid()); + } + _ => unreachable!(), + } +} diff --git a/key-wallet-manager/src/events.rs b/key-wallet-manager/src/events.rs index c43aa64f0..248caf45e 100644 --- a/key-wallet-manager/src/events.rs +++ b/key-wallet-manager/src/events.rs @@ -10,11 +10,132 @@ use dashcore::ephemerealdata::instant_lock::InstantLock; use dashcore::prelude::CoreBlockHeight; use dashcore::Txid; use key_wallet::account::AccountType; +use key_wallet::managed_account::address_pool::{AddressPoolType, PublicKeyType}; use key_wallet::managed_account::transaction_record::TransactionRecord; +use key_wallet::transaction_checking::DerivedAddressInfo; use key_wallet::WalletCoreBalance; use crate::WalletId; +/// One address derived as a side effect of gap-limit maintenance during +/// transaction processing. +/// +/// Emitted on [`WalletEvent::TransactionDetected`] / +/// [`WalletEvent::BlockProcessed`] so persisters can mirror the on-disk +/// address pool transactionally with the tx/block records that triggered +/// the derivation. Keeping the derivation here (rather than as a +/// stand-alone event) is what lets consumers store +/// `Wallet → Account → CoreAddress → Txo` without breaking the +/// `CoreAddress` link for UTXOs landing on freshly derived addresses. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DerivedAddress { + /// The account that derived this address. + pub account_type: AccountType, + /// Which pool of the account the address belongs to (External / + /// Internal / Absent / AbsentHardened). + pub pool_type: AddressPoolType, + /// Derivation index within the pool. Combined with `account_type` + /// (which carries any account-level indices like the Dashpay + /// `user_identity_id` / `friend_identity_id`) and `pool_type`, this + /// fully determines the derivation path — consumers that need a + /// rendered path can recompute it deterministically rather than + /// shipping a redundant string on every event. + pub derivation_index: u32, + /// The derived address. + pub address: dashcore::Address, + /// Compressed ECDSA public key (33 bytes). Non-ECDSA pools + /// (BLS / EdDSA) are skipped during projection. + pub public_key: [u8; 33], +} + +impl DerivedAddress { + /// Project a [`DerivedAddressInfo`] from key-wallet into a + /// `DerivedAddress` event payload. Returns `None` for non-ECDSA pools + /// (BLS / EdDSA) since the event field carries a 33-byte compressed + /// key — those pools don't trigger gap-limit extension on Core + /// transactions in practice, but skip rather than panic if they do. + /// Drops are logged so a future change that wires gap-limit extension + /// into a BLS / EdDSA pool surfaces in traces rather than silently + /// orphaning UTXOs at the persister. + pub(crate) fn from_info(derived: DerivedAddressInfo) -> Option { + let account_type = derived.account_type; + let pool_type = derived.pool_type; + let index = derived.info.index; + let public_key = match derived.info.public_key.as_ref() { + None => { + tracing::warn!( + ?account_type, + ?pool_type, + index, + "dropping derived address with no public key from event projection" + ); + return None; + } + Some(PublicKeyType::ECDSA(bytes)) => { + if bytes.len() != 33 { + // Producer (`generate_address_at_index`) always stores + // 33-byte compressed keys, so a length mismatch is a + // bug, not an expected drop. + tracing::warn!( + ?account_type, + ?pool_type, + index, + len = bytes.len(), + "dropping derived address: ECDSA public key is not 33 bytes" + ); + return None; + } + let mut arr = [0u8; 33]; + arr.copy_from_slice(bytes); + arr + } + Some(PublicKeyType::BLS(_)) | Some(PublicKeyType::EdDSA(_)) => { + tracing::debug!( + ?account_type, + ?pool_type, + index, + "dropping non-ECDSA derived address from event projection \ + (event field is 33-byte compressed ECDSA only)" + ); + return None; + } + }; + Some(Self { + account_type, + pool_type, + derivation_index: index, + address: derived.info.address, + public_key, + }) + } +} + +/// Project an iterator of [`DerivedAddressInfo`] entries into a +/// deduplicated [`DerivedAddress`] vec. +/// +/// Dedup keys on `(account_type, pool_type, derivation_index)` so that two +/// records in the same block both pushing the same gap-limit boundary +/// collapse to a single entry. Non-ECDSA pools are silently dropped (see +/// [`DerivedAddress::from_info`]). +pub(crate) fn project_derived_addresses(infos: I) -> Vec +where + I: IntoIterator, +{ + let mut out: Vec = Vec::new(); + let mut seen: std::collections::HashSet<(AccountType, AddressPoolType, u32)> = + std::collections::HashSet::new(); + for info in infos { + let key = (info.account_type, info.pool_type, info.info.index); + if !seen.insert(key) { + continue; + } + if let Some(d) = DerivedAddress::from_info(info) { + out.push(d); + } + } + out +} + /// Diff `current` against `prior` and return only the entries whose /// balance changed (including ones missing from `prior`). Intended for /// pairing two snapshots taken via @@ -74,6 +195,15 @@ pub enum WalletEvent { /// balance was unchanged are omitted to keep the payload small /// (most transactions touch only 1–2 accounts). account_balances: BTreeMap, + /// Addresses derived as a side effect of gap-limit maintenance + /// while processing this transaction, attributed to the same + /// account as `record` (a tx that pays into multiple accounts + /// of the same wallet emits one event per record, each scoped + /// to its own account's derivations). Empty in the common case. + /// Persisters that mirror the address pool to disk should write + /// these rows transactionally with `record` so UTXOs landing on + /// them retain a parent address row. + addresses_derived: Vec, }, /// An InstantSend lock was applied to a previously-seen off-chain /// wallet-relevant transaction. @@ -116,6 +246,13 @@ pub enum WalletEvent { /// account's full balance after the change — not a delta. Accounts /// whose balance was unchanged are omitted. account_balances: BTreeMap, + /// Addresses derived as a side effect of gap-limit maintenance + /// across every record in the block, deduplicated by + /// `(account_type, pool_type, derivation_index)`. Empty in the + /// common case. Persisters should write these rows + /// transactionally with the inserted/updated records so UTXOs + /// landing on them retain a parent address row. + addresses_derived: Vec, }, /// The wallet's scan cursor advanced because the filter pipeline /// committed a batch covering blocks up to `height`. No records or @@ -159,14 +296,16 @@ impl WalletEvent { record, balance, account_balances, + addresses_derived, .. } => { format!( - "TransactionDetected(txid={}, context={}, balance={}, account_balances={})", + "TransactionDetected(txid={}, context={}, balance={}, account_balances={}, derived={})", record.txid, record.context, balance, format_account_balances(account_balances), + addresses_derived.len(), ) } WalletEvent::TransactionInstantLocked { @@ -189,16 +328,18 @@ impl WalletEvent { matured, balance, account_balances, + addresses_derived, .. } => { format!( - "BlockProcessed(height={}, inserted={}, updated={}, matured={}, balance={}, account_balances={})", + "BlockProcessed(height={}, inserted={}, updated={}, matured={}, balance={}, account_balances={}, derived={})", height, inserted.len(), updated.len(), matured.len(), balance, format_account_balances(account_balances), + addresses_derived.len(), ) } WalletEvent::SyncHeightAdvanced { @@ -210,3 +351,148 @@ impl WalletEvent { } } } + +#[cfg(test)] +mod project_derived_addresses_tests { + use super::*; + use key_wallet::account::StandardAccountType; + use key_wallet::bip32::{ChildNumber, DerivationPath}; + use key_wallet::managed_account::address_pool::AddressInfo; + use std::collections::BTreeMap; + + /// Compressed encoding of the secp256k1 generator point — a known + /// on-curve 33-byte value `dashcore::PublicKey::from_slice` accepts. + /// Tests don't care which key it is; they care that projection + /// preserves the bytes round-trip. + const TEST_PUBKEY_G: [u8; 33] = [ + 0x02, 0x79, 0xbe, 0x66, 0x7e, 0xf9, 0xdc, 0xbb, 0xac, 0x55, 0xa0, 0x62, 0x95, 0xce, 0x87, + 0x0b, 0x07, 0x02, 0x9b, 0xfc, 0xdb, 0x2d, 0xce, 0x28, 0xd9, 0x59, 0xf2, 0x81, 0x5b, 0x16, + 0xf8, 0x17, 0x98, + ]; + + /// Build a stub `DerivedAddressInfo` for unit-testing the projection + /// without spinning up a wallet. `tag` differentiates the + /// `public_key` so "first-seen wins" can be observed across + /// duplicates; pass `None` to use the default valid key. + fn make_derived( + account_type: AccountType, + pool_type: AddressPoolType, + index: u32, + tag: Option, + ) -> DerivedAddressInfo { + // Take a real compressed pubkey and tweak only the `public_key` + // bytes via `tag` (which the projection passes through verbatim). + // We don't actually re-derive the address from the tagged bytes — + // tests don't depend on address↔pubkey consistency, only on + // dedup keys and pubkey round-trip. + let pubkey = + dashcore::PublicKey::from_slice(&TEST_PUBKEY_G).expect("generator point is valid"); + let address = dashcore::Address::p2pkh(&pubkey, key_wallet::Network::Testnet); + let script_pubkey = address.script_pubkey(); + let path = DerivationPath::from(vec![ + ChildNumber::from_normal_idx(0).unwrap(), + ChildNumber::from_normal_idx(index).unwrap(), + ]); + let mut pubkey_bytes = TEST_PUBKEY_G.to_vec(); + if let Some(t) = tag { + // Tag a non-prefix byte so `[u8; 33]` round-trip is observable + // without affecting the leading `0x02` compressed marker. + pubkey_bytes[32] = t; + } + DerivedAddressInfo { + account_type, + pool_type, + info: AddressInfo { + address, + script_pubkey, + public_key: Some(PublicKeyType::ECDSA(pubkey_bytes)), + index, + path, + used: false, + generated_at: 0, + used_at: None, + tx_count: 0, + total_received: 0, + total_sent: 0, + balance: 0, + label: None, + metadata: BTreeMap::new(), + }, + } + } + + fn standard_account_0() -> AccountType { + AccountType::Standard { + index: 0, + standard_account_type: StandardAccountType::BIP44Account, + } + } + + /// Two `DerivedAddressInfo` entries with the same + /// `(account_type, pool_type, index)` must collapse to one + /// `DerivedAddress`. First-seen wins, so the surviving entry's + /// `public_key` matches the first input — this guards against a + /// regression that swaps to last-wins or drops the dedup entirely. + #[test] + fn project_dedups_overlapping_keys() { + let acct = standard_account_0(); + let first = make_derived(acct, AddressPoolType::External, 5, Some(0xaa)); + let first_bytes = match first.info.public_key.as_ref().expect("ECDSA pubkey") { + PublicKeyType::ECDSA(b) => b.clone(), + _ => unreachable!(), + }; + let second = make_derived(acct, AddressPoolType::External, 5, Some(0xbb)); + + let projected = project_derived_addresses(vec![first, second]); + assert_eq!(projected.len(), 1, "duplicate (account, pool, index) must dedup to one entry"); + assert_eq!(projected[0].account_type, acct); + assert_eq!(projected[0].pool_type, AddressPoolType::External); + assert_eq!(projected[0].derivation_index, 5); + // First-seen wins: surviving pubkey matches `first`, not `second`. + assert_eq!(&projected[0].public_key[..], first_bytes.as_slice()); + assert_eq!(projected[0].public_key[32], 0xaa); + } + + /// Different indices on the same pool must NOT dedup. + #[test] + fn project_keeps_distinct_indices_on_same_pool() { + let acct = standard_account_0(); + let infos = vec![ + make_derived(acct, AddressPoolType::External, 5, None), + make_derived(acct, AddressPoolType::External, 6, None), + make_derived(acct, AddressPoolType::External, 7, None), + ]; + let projected = project_derived_addresses(infos); + assert_eq!(projected.len(), 3); + let mut indices: Vec = projected.iter().map(|d| d.derivation_index).collect(); + indices.sort_unstable(); + assert_eq!(indices, vec![5, 6, 7]); + } + + /// Same index, different pool types must NOT dedup — the dedup key + /// includes `pool_type`. + #[test] + fn project_keeps_same_index_across_different_pools() { + let acct = standard_account_0(); + let projected = project_derived_addresses(vec![ + make_derived(acct, AddressPoolType::External, 5, None), + make_derived(acct, AddressPoolType::Internal, 5, None), + ]); + assert_eq!(projected.len(), 2); + let pools: std::collections::BTreeSet = + projected.iter().map(|d| d.pool_type).collect(); + assert!(pools.contains(&AddressPoolType::External)); + assert!(pools.contains(&AddressPoolType::Internal)); + } + + /// `from_info` (and therefore `project_derived_addresses`) must drop + /// entries that don't carry an ECDSA pubkey. A surviving result of + /// length 0 is the contract — no panic. + #[test] + fn project_drops_entry_with_missing_pubkey() { + let mut info = make_derived(standard_account_0(), AddressPoolType::External, 5, None); + info.info.public_key = None; + let projected = project_derived_addresses(vec![info]); + assert!(projected.is_empty()); + } +} diff --git a/key-wallet-manager/src/lib.rs b/key-wallet-manager/src/lib.rs index f607b6d65..e2db38420 100644 --- a/key-wallet-manager/src/lib.rs +++ b/key-wallet-manager/src/lib.rs @@ -20,7 +20,7 @@ mod process_block; mod wallet_interface; pub use error::WalletError; -pub use events::WalletEvent; +pub use events::{DerivedAddress, WalletEvent}; pub use matching::{check_compact_filters_for_addresses, FilterMatchKey}; pub use wallet_interface::{BlockProcessingResult, MempoolTransactionResult, WalletInterface}; @@ -29,7 +29,7 @@ use dashcore::prelude::CoreBlockHeight; use key_wallet::account::AccountCollection; use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; use key_wallet::managed_account::transaction_record::TransactionRecord; -use key_wallet::transaction_checking::TransactionContext; +use key_wallet::transaction_checking::{DerivedAddressInfo, 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; @@ -74,9 +74,13 @@ pub struct CheckTransactionsResult { pub affected_wallets: Vec, /// Set to false if the transaction was already stored and is being re-processed (e.g., during rescan) pub is_new_transaction: bool, - /// New addresses generated during gap limit maintenance, attributed to the - /// wallet that produced them. - pub new_addresses: BTreeMap>, + /// Addresses derived during gap-limit maintenance, attributed to the + /// wallet that produced them. Each entry carries the originating + /// account type, pool type, and full + /// [`AddressInfo`](key_wallet::managed_account::address_pool::AddressInfo) + /// so downstream emitters can attribute the derivation precisely without + /// re-deriving. + pub new_addresses: BTreeMap>, /// Total value received across all wallets pub total_received: u64, /// Total value sent across all wallets @@ -91,10 +95,18 @@ pub struct CheckTransactionsResult { } impl CheckTransactionsResult { - /// Iterate over every newly generated address regardless of wallet attribution. - pub(crate) fn all_new_addresses(&self) -> impl Iterator { + /// Iterate over every newly derived [`DerivedAddressInfo`] regardless of + /// wallet attribution. + pub(crate) fn all_new_address_infos(&self) -> impl Iterator { self.new_addresses.values().flatten() } + + /// Iterate over every newly derived address regardless of wallet + /// attribution. The richer [`DerivedAddressInfo`] is available via + /// [`Self::all_new_address_infos`]. + pub(crate) fn all_new_addresses(&self) -> impl Iterator { + self.all_new_address_infos().map(|d| &d.info.address) + } } /// High-level wallet manager that manages multiple wallets diff --git a/key-wallet-manager/src/process_block.rs b/key-wallet-manager/src/process_block.rs index cf1632fc3..f4e818984 100644 --- a/key-wallet-manager/src/process_block.rs +++ b/key-wallet-manager/src/process_block.rs @@ -1,4 +1,4 @@ -use crate::events::diff_account_balances; +use crate::events::{diff_account_balances, project_derived_addresses, DerivedAddress}; use crate::wallet_interface::{BlockProcessingResult, MempoolTransactionResult, WalletInterface}; use crate::{WalletEvent, WalletId, WalletManager}; use async_trait::async_trait; @@ -8,7 +8,7 @@ use dashcore::prelude::CoreBlockHeight; use dashcore::{Address, Block, Transaction}; use key_wallet::account::AccountType; use key_wallet::managed_account::transaction_record::TransactionRecord; -use key_wallet::transaction_checking::{BlockInfo, TransactionContext}; +use key_wallet::transaction_checking::{BlockInfo, DerivedAddressInfo, TransactionContext}; use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; use key_wallet::WalletCoreBalance; use std::collections::{BTreeMap, BTreeSet}; @@ -30,6 +30,7 @@ impl WalletInterface for WalletM let mut per_wallet_inserted: BTreeMap> = BTreeMap::new(); let mut per_wallet_updated: BTreeMap> = BTreeMap::new(); + let mut per_wallet_derived: BTreeMap> = BTreeMap::new(); for tx in &block.txdata { let context = TransactionContext::InBlock(info); @@ -44,8 +45,10 @@ impl WalletInterface for WalletM } } - for (wallet_id, addrs) in check_result.new_addresses { - result.new_addresses.entry(wallet_id).or_default().extend(addrs); + for (wallet_id, derived) in check_result.new_addresses { + let addresses = derived.iter().map(|d| d.info.address.clone()).collect::>(); + result.new_addresses.entry(wallet_id).or_default().extend(addresses); + per_wallet_derived.entry(wallet_id).or_default().extend(derived); } for (wallet_id, records) in check_result.per_wallet_new_records { per_wallet_inserted.entry(wallet_id).or_default().extend(records); @@ -55,7 +58,13 @@ impl WalletInterface for WalletM } } - self.finalize_block_advance(height, wallets, per_wallet_inserted, per_wallet_updated); + self.finalize_block_advance( + height, + wallets, + per_wallet_inserted, + per_wallet_updated, + per_wallet_derived, + ); result } @@ -104,6 +113,7 @@ impl WalletInterface for WalletM let per_wallet_new_records = std::mem::take(&mut check_result.per_wallet_new_records); let per_wallet_updated_records = std::mem::take(&mut check_result.per_wallet_updated_records); + let mut per_wallet_derived = std::mem::take(&mut check_result.new_addresses); for (wallet_id, records) in per_wallet_new_records { let Some(info) = self.wallet_infos.get(&wallet_id) else { @@ -112,15 +122,39 @@ impl WalletInterface for WalletM let balance = info.balance(); let account_balances = per_wallet_account_diff.get(&wallet_id).cloned().unwrap_or_default(); + // Attribute derivations to the record whose owning account + // produced them. A single mempool tx can pay into more than + // one account in the same wallet; each affected account ran + // its own gap-limit maintenance, and `DerivedAddressInfo` + // already carries the originating `account_type`. Filter by + // record so persisters scoping by `record.account_type` get + // the correct rows. + let mut derived_for_wallet = per_wallet_derived.remove(&wallet_id).unwrap_or_default(); for record in records { + let record_account = record.account_type; + let (for_record, rest): (Vec<_>, Vec<_>) = + derived_for_wallet.into_iter().partition(|d| d.account_type == record_account); + derived_for_wallet = rest; let event = WalletEvent::TransactionDetected { wallet_id, record: Box::new(record), balance, account_balances: account_balances.clone(), + addresses_derived: project_derived_addresses(for_record), }; let _ = self.event_sender.send(event); } + // If any derivations were left unattributed (records vector + // didn't cover every account that derived), log so the + // mismatch is debuggable rather than silently lost. + if !derived_for_wallet.is_empty() { + tracing::warn!( + wallet_id = ?wallet_id, + leftover = derived_for_wallet.len(), + "mempool tx produced gap-limit derivations not covered by any \ + emitted TransactionDetected record; ignoring" + ); + } } if let Some(lock) = instant_lock { @@ -220,7 +254,13 @@ impl WalletInterface for WalletM height: CoreBlockHeight, ) { let wallets = BTreeSet::from([*wallet_id]); - self.finalize_block_advance(height, &wallets, BTreeMap::new(), BTreeMap::new()); + self.finalize_block_advance( + height, + &wallets, + BTreeMap::new(), + BTreeMap::new(), + BTreeMap::new(), + ); } fn subscribe_events(&self) -> broadcast::Receiver { @@ -311,6 +351,7 @@ impl WalletManager { wallets: &BTreeSet, mut per_wallet_inserted: BTreeMap>, mut per_wallet_updated: BTreeMap>, + mut per_wallet_derived: BTreeMap>, ) { if wallets.is_empty() { return; @@ -372,8 +413,15 @@ impl WalletManager { let balance_changed = snapshot.get(wallet_id).copied() != Some(new_balance); let prior = prior_account_balances.remove(wallet_id).unwrap_or_default(); let account_balances = diff_account_balances(&prior, &info.account_balances()); - - if !inserted.is_empty() || !updated.is_empty() || !matured.is_empty() || balance_changed + let derived_for_wallet = per_wallet_derived.remove(wallet_id).unwrap_or_default(); + let addresses_derived: Vec = + project_derived_addresses(derived_for_wallet); + + if !inserted.is_empty() + || !updated.is_empty() + || !matured.is_empty() + || !addresses_derived.is_empty() + || balance_changed { let event = WalletEvent::BlockProcessed { wallet_id: *wallet_id, @@ -383,6 +431,7 @@ impl WalletManager { matured, balance: new_balance, account_balances, + addresses_derived, }; let _ = self.event_sender.send(event); } diff --git a/key-wallet/src/managed_account/address_pool.rs b/key-wallet/src/managed_account/address_pool.rs index 36fd1edf6..ab9a127f6 100644 --- a/key-wallet/src/managed_account/address_pool.rs +++ b/key-wallet/src/managed_account/address_pool.rs @@ -31,7 +31,7 @@ pub enum PublicKeyType { } /// Type of address pool (external, internal, or absent/single-pool) -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] #[cfg_attr(feature = "bincode", derive(Encode, Decode))] pub enum AddressPoolType { @@ -877,8 +877,14 @@ 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 the freshly generated [`AddressInfo`] entries (in derivation + /// order). Returning the rich [`AddressInfo`] rather than just + /// [`Address`] lets callers — in particular the wallet-manager event + /// emission seam — surface the full derivation context (index, path, + /// public key) for downstream persisters without re-deriving it. + pub fn maintain_gap_limit(&mut self, key_source: &KeySource) -> Result> { let target = match self.highest_used { None => self.gap_limit - 1, Some(highest) => highest + self.gap_limit, @@ -887,8 +893,20 @@ impl AddressPool { let mut new_addresses = Vec::new(); while self.highest_generated.unwrap_or(0) < target { let next_index = self.highest_generated.map(|h| h + 1).unwrap_or(0); - let address = self.generate_address_at_index(next_index, key_source, true)?; - new_addresses.push(address); + self.generate_address_at_index(next_index, key_source, true)?; + // `generate_address_at_index` with `add_to_state = true` always + // inserts at `next_index`. Asserting the invariant explicitly + // here turns a regression that breaks it (e.g. a refactor that + // hits the early-return branch on a re-derivation) into a loud + // panic instead of an infinite loop on the outer `while`. + let info = self.addresses.get(&next_index).cloned().unwrap_or_else(|| { + panic!( + "maintain_gap_limit: generate_address_at_index({}) succeeded but \ + the entry was not stored; pool invariant broken", + next_index + ) + }); + new_addresses.push(info); } Ok(new_addresses) @@ -1229,6 +1247,7 @@ mod tests { // Should generate exactly 1 address to maintain gap_limit unused after index 0 let new_addresses = pool.maintain_gap_limit(&key_source).unwrap(); assert_eq!(new_addresses.len(), 1); + assert_eq!(new_addresses[0].index, gap_limit); assert_eq!(pool.highest_generated, Some(gap_limit)); assert_eq!(pool.addresses.len(), gap_limit as usize + 1); @@ -1239,6 +1258,8 @@ mod tests { // Should generate exactly 2 more addresses let new_addresses = pool.maintain_gap_limit(&key_source).unwrap(); assert_eq!(new_addresses.len(), 2); + assert_eq!(new_addresses[0].index, gap_limit + 1); + assert_eq!(new_addresses[1].index, gap_limit + 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 2b825f526..48c8ba93f 100644 --- a/key-wallet/src/managed_account/managed_platform_account.rs +++ b/key-wallet/src/managed_account/managed_platform_account.rs @@ -11,7 +11,7 @@ use std::collections::BTreeMap; -use super::address_pool::{AddressPool, KeySource}; +use super::address_pool::{AddressInfo, AddressPool, KeySource}; use super::platform_address::PlatformP2PKHAddress; use crate::error::{Error, Result}; use crate::Network; @@ -276,8 +276,8 @@ impl ManagedPlatformAccount { /// Maintain the gap limit for the address pool /// /// This generates new addresses if needed to maintain the gap limit. - /// Returns the newly generated addresses. - pub fn maintain_gap_limit(&mut self, key_source: &KeySource) -> Result> { + /// Returns the newly generated address info entries (in derivation order). + pub fn maintain_gap_limit(&mut self, key_source: &KeySource) -> Result> { self.addresses .maintain_gap_limit(key_source) .map_err(|e| Error::InvalidParameter(format!("Failed to maintain gap limit: {}", e))) diff --git a/key-wallet/src/transaction_checking/account_checker.rs b/key-wallet/src/transaction_checking/account_checker.rs index 36586fac5..a015f683c 100644 --- a/key-wallet/src/transaction_checking/account_checker.rs +++ b/key-wallet/src/transaction_checking/account_checker.rs @@ -6,8 +6,8 @@ use std::collections::BTreeMap; use super::transaction_router::AccountTypeToCheck; -use crate::account::{ManagedAccountCollection, ManagedCoreFundsAccount}; -use crate::managed_account::address_pool::{AddressInfo, PublicKeyType}; +use crate::account::{AccountType, ManagedAccountCollection, ManagedCoreFundsAccount}; +use crate::managed_account::address_pool::{AddressInfo, AddressPoolType, PublicKeyType}; use crate::managed_account::managed_account_trait::ManagedAccountTrait; use crate::managed_account::managed_account_type::ManagedAccountType; use crate::managed_account::transaction_record::TransactionRecord; @@ -28,6 +28,23 @@ pub enum AddressClassification { Other, } +/// A freshly-derived address produced as a side effect of gap-limit +/// maintenance during transaction processing. Carries the originating +/// [`AccountType`] and [`AddressPoolType`] alongside the rich +/// [`AddressInfo`] so downstream consumers (e.g. the wallet-manager +/// event seam) can build a fully self-describing event payload without +/// re-deriving from the wallet. +#[derive(Debug, Clone)] +pub struct DerivedAddressInfo { + /// The account that derived this address. + pub account_type: AccountType, + /// Which pool of the account the address belongs to (External / + /// Internal / Absent / AbsentHardened). + pub pool_type: AddressPoolType, + /// The full address info — derivation index, path, public key, etc. + pub info: AddressInfo, +} + /// Result of checking a transaction against accounts #[derive(Debug, Clone)] pub struct TransactionCheckResult { @@ -45,12 +62,14 @@ pub struct TransactionCheckResult { pub total_sent: u64, /// Total value received for Platform credit conversion pub total_received_for_credit_conversion: u64, - /// New addresses generated during gap limit maintenance - pub new_addresses: Vec
, + /// Addresses derived as a side effect of gap-limit maintenance during + /// this check. Each entry carries the originating account type, pool + /// type, and full [`AddressInfo`] so downstream emitters can attribute + /// the derivation precisely without re-deriving. + pub new_addresses: Vec, /// Transaction records created for new transactions. Each record carries - /// its owning [`AccountType`](crate::account::AccountType) on - /// `record.account_type`, so consumers can recover it without an external - /// pairing. + /// its owning [`AccountType`] on `record.account_type`, so consumers can + /// recover it without an external pairing. pub new_records: Vec, /// Transaction records updated by this check (confirmation or IS-lock /// applied to a previously stored record). Each record carries its owning diff --git a/key-wallet/src/transaction_checking/mod.rs b/key-wallet/src/transaction_checking/mod.rs index 964fc3656..0ccb2048d 100644 --- a/key-wallet/src/transaction_checking/mod.rs +++ b/key-wallet/src/transaction_checking/mod.rs @@ -10,7 +10,9 @@ pub mod transaction_context; pub mod transaction_router; pub mod wallet_checker; -pub use account_checker::{AccountMatch, AddressClassification, TransactionCheckResult}; +pub use account_checker::{ + AccountMatch, AddressClassification, DerivedAddressInfo, TransactionCheckResult, +}; pub use platform_checker::WalletPlatformChecker; pub use transaction_context::{BlockInfo, TransactionContext}; pub use transaction_router::{PlatformAccountConversionError, TransactionRouter, TransactionType}; diff --git a/key-wallet/src/transaction_checking/wallet_checker.rs b/key-wallet/src/transaction_checking/wallet_checker.rs index d5d799a0f..f2856d818 100644 --- a/key-wallet/src/transaction_checking/wallet_checker.rs +++ b/key-wallet/src/transaction_checking/wallet_checker.rs @@ -177,13 +177,21 @@ impl WalletTransactionChecker for ManagedWalletInfo { let key_source = KeySource::Public(xpub); let rev_before = result.new_addresses.len(); + let owning_account_type = account.managed_account_type().to_account_type(); for pool in account.managed_account_type_mut().address_pools_mut() { + let pool_type = pool.pool_type; match pool.maintain_gap_limit(&key_source) { - Ok(addrs) => result.new_addresses.extend(addrs), + Ok(infos) => result.new_addresses.extend(infos.into_iter().map(|info| { + super::account_checker::DerivedAddressInfo { + account_type: owning_account_type, + pool_type, + info, + } + })), Err(e) => { tracing::error!( account_index = ?account_match.account_type_match.account_index(), - pool_type = ?pool.pool_type, + pool_type = ?pool_type, error = %e, "Failed to maintain gap limit for address pool" );