Skip to content

Commit 871ce24

Browse files
feat(key-wallet-manager): carry addresses_derived on TransactionDetected / BlockProcessed (#725)
* feat(key-wallet-manager): carry addresses_derived on TransactionDetected / BlockProcessed Address-pool extensions triggered by gap-limit maintenance during tx / block processing now ride along on the existing event the persister already consumes, rather than living silently in Rust memory. UTXOs landing on freshly derived addresses keep their parent CoreAddress row in downstream stores. `AddressPool::maintain_gap_limit` returns `Vec<AddressInfo>` (was `Vec<Address>`); the wallet checker tags each new entry with the owning account type and pool type via a new `DerivedAddressInfo`, and the manager projects that into `DerivedAddress` (account_type, pool_type, derivation_index, derivation_path, address, public_key) on the event. Block emission de-duplicates by (account_type, pool_type, index) so two records pushing the same boundary collapse to one entry. Non-ECDSA pools (BLS / EdDSA) are silently skipped during projection. IS-lock and sync-height events are unchanged — those paths don't extend the pool. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(docs): repair rustdoc intra-doc links `AccountType` is now imported in account_checker.rs, which makes the explicit `(crate::account::AccountType)` link target redundant under `-D warnings`. And `[AddressInfo]` in key-wallet-manager/src/lib.rs needs the fully-qualified path since the type isn't imported into the crate root. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(key-wallet-manager): drop redundant derivation_path from DerivedAddress `account_type` (which carries Dashpay identity ids, IdentityTopUp's registration_index, and the like), `pool_type`, and `derivation_index` fully determine the path. Consumers that need a rendered path can recompute it deterministically rather than carrying a redundant String on every event. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review(key-wallet-manager): per-record derivations, observability, real dedup test Address review feedback on PR #725: - `DerivedAddress::from_info` is now `pub(crate)` since the only caller is the in-crate `project_derived_addresses` helper. Keeps the compatibility boundary at `WalletEvent` / `DerivedAddress` and avoids re-exporting `key_wallet::transaction_checking::DerivedAddressInfo` through this crate's public API. - Drops in `from_info` now log via `tracing` — `warn!` for the unreachable-by-construction "ECDSA but not 33 bytes" arm (which signals a bug if ever hit), `warn!` for "no public key", and `debug!` for the BLS / EdDSA fall-through (expected if a future change wires gap-limit extension into one of those pools, currently unreachable). - Mempool path now attributes derivations per-record via partition on `record.account_type == DerivedAddressInfo.account_type`. A tx that pays into multiple accounts of the same wallet now ships each record with its own account's derivations — consistent with the field's documented "transactional with `record`" semantics. Doc tightened accordingly. - `maintain_gap_limit` previously silently produced a shorter `Vec` if the just-inserted entry wasn't found; now it panics with an explicit invariant message, turning a regression that breaks the post-insert invariant into a loud failure rather than a silent infinite loop on the outer `while`. - New unit tests for `project_derived_addresses` directly exercise the dedup branch (overlapping `(account, pool, index)` → first-wins), distinct indices, distinct pools, and the missing-pubkey drop. The existing block-level test couldn't reach the dedup branch since `maintain_gap_limit` is idempotent on a second call. - The "highest external" event test now pins each emitted `(address, public_key)` against the wallet's own `AddressInfo` for the same `(account, pool, index)`. Catches regressions in `from_info` that would silently corrupt persister rows while keeping the count-and-index assertions green. - `dash-spv-ffi` dispatch arms tagged with TODOs documenting that `addresses_derived` is dropped at the FFI seam pending a paired callback. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(dash-spv-ffi): expose addresses_derived on Wallet event callbacks Closes the FFI-side TODOs left by PR #725: the Rust event already carries `Vec<DerivedAddress>`, but the C callback signature had no slot for it, so the field was dropped at the dispatch seam — defeating the persistence contract for FFI consumers (Swift / mobile persisters mirroring the address pool to disk). - New `FFIDerivedAddress { account_type, pool_type, derivation_index, address, public_key[33] }` and `FFIDerivedAddressPoolType` (4-variant, mirrors `key_wallet::AddressPoolType` 1:1; kept distinct from the existing `FFIAddressPoolType` which collapses Absent / AbsentHardened). - `OnTransactionDetectedCallback` and `OnWalletBlockProcessedCallback` signatures grow `addresses_derived: *const FFIDerivedAddress, addresses_derived_count: u32`. The C ABI is broken — every consumer has to update its callback signature; this is intentional, since the alternative (a paired auxiliary callback) would let consumers persist records without the matching address rows and reintroduce the orphan-`CoreAddress`-row failure mode the field is designed to prevent. - Memory: `FFIDerivedAddress` owns the `address` C string (freed on drop via `CString::from_raw`) and the `FFIAccountType` (which has its own Drop for the optional Dashpay identity ids). The `Vec` lives on the Rust stack across the callback and is dropped after. - `dash-spv-ffi/src/bin/ffi_cli.rs` and the dashd integration test callbacks updated for the new signatures (counts logged in the CLI; arrays are not retained in tests). - cbindgen header regenerates correctly (verified via `cargo build`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e8e8283 commit 871ce24

12 files changed

Lines changed: 955 additions & 37 deletions

File tree

dash-spv-ffi/src/bin/ffi_cli.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,12 +172,15 @@ fn read_balance(balance: *const FFIBalance) -> FFIBalance {
172172
unsafe { *balance }
173173
}
174174

175+
#[allow(clippy::too_many_arguments)]
175176
extern "C" fn on_transaction_detected(
176177
wallet_id: *const c_char,
177178
record: *const FFITransactionRecord,
178179
balance: *const FFIBalance,
179180
_account_balances: *const dash_spv_ffi::FFIAccountBalance,
180181
account_balances_count: u32,
182+
_addresses_derived: *const dash_spv_ffi::FFIDerivedAddress,
183+
addresses_derived_count: u32,
181184
_user_data: *mut c_void,
182185
) {
183186
let wallet_short = short_wallet(wallet_id);
@@ -189,7 +192,7 @@ extern "C" fn on_transaction_detected(
189192
let b = read_balance(balance);
190193
let txid_hex = hex::encode(r.txid);
191194
println!(
192-
"[Wallet] TX detected: wallet={}..., txid={}, account_kind={:?}, account_index={}, amount={} duffs, balance[confirmed={}, unconfirmed={}], changed_accounts={}",
195+
"[Wallet] TX detected: wallet={}..., txid={}, account_kind={:?}, account_index={}, amount={} duffs, balance[confirmed={}, unconfirmed={}], changed_accounts={}, derived={}",
193196
wallet_short,
194197
txid_hex,
195198
r.account_type.kind,
@@ -198,6 +201,7 @@ extern "C" fn on_transaction_detected(
198201
b.confirmed,
199202
b.unconfirmed,
200203
account_balances_count,
204+
addresses_derived_count,
201205
);
202206
}
203207

@@ -243,12 +247,14 @@ extern "C" fn on_wallet_block_processed(
243247
balance: *const FFIBalance,
244248
_account_balances: *const dash_spv_ffi::FFIAccountBalance,
245249
account_balances_count: u32,
250+
_addresses_derived: *const dash_spv_ffi::FFIDerivedAddress,
251+
addresses_derived_count: u32,
246252
_user_data: *mut c_void,
247253
) {
248254
let wallet_short = short_wallet(wallet_id);
249255
let b = read_balance(balance);
250256
println!(
251-
"[Wallet] Block processed: wallet={}..., height={}, inserted={}, updated={}, matured={}, balance[confirmed={}, unconfirmed={}, immature={}, locked={}], changed_accounts={}",
257+
"[Wallet] Block processed: wallet={}..., height={}, inserted={}, updated={}, matured={}, balance[confirmed={}, unconfirmed={}, immature={}, locked={}], changed_accounts={}, derived={}",
252258
wallet_short,
253259
height,
254260
inserted_count,
@@ -259,6 +265,7 @@ extern "C" fn on_wallet_block_processed(
259265
b.immature,
260266
b.locked,
261267
account_balances_count,
268+
addresses_derived_count,
262269
);
263270
}
264271

dash-spv-ffi/src/callbacks.rs

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,106 @@ impl FFIAccountBalance {
567567
}
568568
}
569569

570+
// ============================================================================
571+
// FFIDerivedAddress - One address derived during gap-limit maintenance
572+
// ============================================================================
573+
574+
/// Pool the derived address belongs to.
575+
///
576+
/// Mirrors `key_wallet::managed_account::address_pool::AddressPoolType`
577+
/// 1:1 — kept distinct from the existing `FFIAddressPoolType` (which
578+
/// collapses Absent / AbsentHardened into a single `Single` variant) so
579+
/// event consumers can distinguish hardened single-pool variants
580+
/// (Provider operator keys, etc.) from non-hardened ones.
581+
#[repr(C)]
582+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
583+
pub enum FFIDerivedAddressPoolType {
584+
External = 0,
585+
Internal = 1,
586+
Absent = 2,
587+
AbsentHardened = 3,
588+
}
589+
590+
impl From<key_wallet::managed_account::address_pool::AddressPoolType>
591+
for FFIDerivedAddressPoolType
592+
{
593+
fn from(t: key_wallet::managed_account::address_pool::AddressPoolType) -> Self {
594+
use key_wallet::managed_account::address_pool::AddressPoolType as P;
595+
match t {
596+
P::External => FFIDerivedAddressPoolType::External,
597+
P::Internal => FFIDerivedAddressPoolType::Internal,
598+
P::Absent => FFIDerivedAddressPoolType::Absent,
599+
P::AbsentHardened => FFIDerivedAddressPoolType::AbsentHardened,
600+
}
601+
}
602+
}
603+
604+
/// One address derived as a side effect of gap-limit maintenance during
605+
/// transaction or block processing.
606+
///
607+
/// Wallet events deliver an array of these so persisters can mirror the
608+
/// on-disk address pool transactionally with the tx/block records that
609+
/// triggered the derivation. Without this, UTXOs landing on freshly
610+
/// derived addresses orphan their parent address row at the persister.
611+
///
612+
/// `account_type` follows the same memory rules as on
613+
/// [`FFIAccountBalance`]: the embedded `identity_user` / `identity_friend`
614+
/// pointers are owned by the `FFIAccountType` and freed when the array is
615+
/// dropped after the callback returns. `address` is a heap-allocated
616+
/// null-terminated UTF-8 string, owned by this struct and freed on drop.
617+
/// Consumers that need to retain the data past the callback must copy
618+
/// every owning field — not just retain pointers.
619+
#[repr(C)]
620+
pub struct FFIDerivedAddress {
621+
/// Owning-account descriptor (discriminant + indices + identity ids).
622+
pub account_type: FFIAccountType,
623+
/// Pool within the account that derived this address.
624+
pub pool_type: FFIDerivedAddressPoolType,
625+
/// Derivation index within the pool. Combined with `account_type`
626+
/// and `pool_type`, this fully determines the derivation path —
627+
/// consumers that need a rendered path can recompute it
628+
/// deterministically.
629+
pub derivation_index: u32,
630+
/// Heap-allocated null-terminated UTF-8 string. Owned by this
631+
/// struct; freed when the struct is dropped.
632+
pub address: *mut c_char,
633+
/// 33-byte compressed ECDSA public key (inline, no allocation).
634+
pub public_key: [u8; 33],
635+
}
636+
637+
impl FFIDerivedAddress {
638+
fn from_slice(addresses: &[key_wallet_manager::DerivedAddress]) -> Vec<Self> {
639+
addresses
640+
.iter()
641+
.map(|d| {
642+
let address_str = d.address.to_string();
643+
let c_address = CString::new(address_str).unwrap_or_else(|_| CString::default());
644+
FFIDerivedAddress {
645+
account_type: FFIAccountType::from(&d.account_type),
646+
pool_type: FFIDerivedAddressPoolType::from(d.pool_type),
647+
derivation_index: d.derivation_index,
648+
address: c_address.into_raw(),
649+
public_key: d.public_key,
650+
}
651+
})
652+
.collect()
653+
}
654+
}
655+
656+
impl Drop for FFIDerivedAddress {
657+
fn drop(&mut self) {
658+
if !self.address.is_null() {
659+
// SAFETY: `address` was constructed via `CString::into_raw` in
660+
// `FFIDerivedAddress::from_slice`, so reclaiming via
661+
// `CString::from_raw` is the matching free.
662+
let _ = unsafe { CString::from_raw(self.address) };
663+
self.address = std::ptr::null_mut();
664+
}
665+
// `account_type` has its own Drop impl that frees the
666+
// identity_user / identity_friend allocations when applicable.
667+
}
668+
}
669+
570670
// ============================================================================
571671
// FFIWalletEventCallbacks - One callback per WalletEvent variant
572672
// ============================================================================
@@ -584,13 +684,22 @@ impl FFIAccountBalance {
584684
/// entries for a normal transaction); accounts whose balance is unchanged
585685
/// are omitted. The array is null with a zero count when no per-account
586686
/// balance changed.
687+
///
688+
/// `addresses_derived` is an array of size `addresses_derived_count` of
689+
/// addresses derived as a side effect of gap-limit maintenance while
690+
/// processing this transaction, attributed to the same account as
691+
/// `record`. Empty in the common case (null pointer with zero count).
692+
/// Persisters should write these rows transactionally with `record` so
693+
/// UTXOs landing on freshly-derived addresses retain a parent row.
587694
pub type OnTransactionDetectedCallback = Option<
588695
extern "C" fn(
589696
wallet_id: *const c_char,
590697
record: *const FFITransactionRecord,
591698
balance: *const FFIBalance,
592699
account_balances: *const FFIAccountBalance,
593700
account_balances_count: u32,
701+
addresses_derived: *const FFIDerivedAddress,
702+
addresses_derived_count: u32,
594703
user_data: *mut c_void,
595704
),
596705
>;
@@ -629,6 +738,13 @@ pub type OnTransactionInstantLockedCallback = Option<
629738
/// balance *after* the block was processed. `account_balances` follows the
630739
/// same contract as on [`OnTransactionDetectedCallback`].
631740
///
741+
/// `addresses_derived` is an array of size `addresses_derived_count` of
742+
/// addresses derived as a side effect of gap-limit maintenance across
743+
/// every record in the block, deduplicated by
744+
/// `(account_type, pool_type, derivation_index)`. Empty in the common
745+
/// case (null pointer with zero count). Persisters should write these
746+
/// rows transactionally with the inserted/updated records.
747+
///
632748
/// All array pointers and their contents are borrowed and only valid for the
633749
/// duration of the callback.
634750
pub type OnWalletBlockProcessedCallback = Option<
@@ -644,6 +760,8 @@ pub type OnWalletBlockProcessedCallback = Option<
644760
balance: *const FFIBalance,
645761
account_balances: *const FFIAccountBalance,
646762
account_balances_count: u32,
763+
addresses_derived: *const FFIDerivedAddress,
764+
addresses_derived_count: u32,
647765
user_data: *mut c_void,
648766
),
649767
>;
@@ -784,29 +902,39 @@ impl FFIWalletEventCallbacks {
784902
record,
785903
balance,
786904
account_balances,
905+
addresses_derived,
787906
} => {
788907
if let Some(cb) = self.on_transaction_detected {
789908
let wallet_id_hex = hex::encode(wallet_id);
790909
let c_wallet_id = CString::new(wallet_id_hex).unwrap_or_default();
791910
let ffi_record = FFITransactionRecord::from(record.as_ref());
792911
let ffi_balance = FFIBalance::from(*balance);
793912
let ffi_account_balances = FFIAccountBalance::from_map(account_balances);
913+
let ffi_addresses_derived = FFIDerivedAddress::from_slice(addresses_derived);
794914
let account_balances_ptr = if ffi_account_balances.is_empty() {
795915
ptr::null()
796916
} else {
797917
ffi_account_balances.as_ptr()
798918
};
919+
let addresses_derived_ptr = if ffi_addresses_derived.is_empty() {
920+
ptr::null()
921+
} else {
922+
ffi_addresses_derived.as_ptr()
923+
};
799924

800925
cb(
801926
c_wallet_id.as_ptr(),
802927
&ffi_record as *const FFITransactionRecord,
803928
&ffi_balance as *const FFIBalance,
804929
account_balances_ptr,
805930
ffi_account_balances.len() as u32,
931+
addresses_derived_ptr,
932+
ffi_addresses_derived.len() as u32,
806933
self.user_data,
807934
);
808935

809936
drop(ffi_account_balances);
937+
drop(ffi_addresses_derived);
810938
}
811939
}
812940
WalletEvent::TransactionInstantLocked {
@@ -851,6 +979,7 @@ impl FFIWalletEventCallbacks {
851979
matured,
852980
balance,
853981
account_balances,
982+
addresses_derived,
854983
} => {
855984
if let Some(cb) = self.on_block_processed {
856985
let wallet_id_hex = hex::encode(wallet_id);
@@ -863,6 +992,7 @@ impl FFIWalletEventCallbacks {
863992
matured.iter().map(FFITransactionRecord::from).collect();
864993
let ffi_balance = FFIBalance::from(*balance);
865994
let ffi_account_balances = FFIAccountBalance::from_map(account_balances);
995+
let ffi_addresses_derived = FFIDerivedAddress::from_slice(addresses_derived);
866996

867997
// Pass a null pointer when an array is empty so C/Swift
868998
// consumers that null-check before reading don't see a
@@ -887,6 +1017,11 @@ impl FFIWalletEventCallbacks {
8871017
} else {
8881018
ffi_account_balances.as_ptr()
8891019
};
1020+
let addresses_derived_ptr = if ffi_addresses_derived.is_empty() {
1021+
ptr::null()
1022+
} else {
1023+
ffi_addresses_derived.as_ptr()
1024+
};
8901025

8911026
cb(
8921027
c_wallet_id.as_ptr(),
@@ -900,13 +1035,16 @@ impl FFIWalletEventCallbacks {
9001035
&ffi_balance as *const FFIBalance,
9011036
account_balances_ptr,
9021037
ffi_account_balances.len() as u32,
1038+
addresses_derived_ptr,
1039+
ffi_addresses_derived.len() as u32,
9031040
self.user_data,
9041041
);
9051042

9061043
drop(ffi_inserted);
9071044
drop(ffi_updated);
9081045
drop(ffi_matured);
9091046
drop(ffi_account_balances);
1047+
drop(ffi_addresses_derived);
9101048
}
9111049
}
9121050
WalletEvent::SyncHeightAdvanced {

dash-spv-ffi/tests/dashd_sync/callbacks.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,8 @@ extern "C" fn on_transaction_detected(
421421
balance: *const FFIBalance,
422422
account_balances: *const FFIAccountBalance,
423423
account_balances_count: u32,
424+
_addresses_derived: *const dash_spv_ffi::FFIDerivedAddress,
425+
_addresses_derived_count: u32,
424426
user_data: *mut c_void,
425427
) {
426428
let Some(tracker) = (unsafe { tracker_from(user_data) }) else {
@@ -493,6 +495,8 @@ extern "C" fn on_wallet_block_processed(
493495
balance: *const FFIBalance,
494496
account_balances: *const FFIAccountBalance,
495497
account_balances_count: u32,
498+
_addresses_derived: *const dash_spv_ffi::FFIDerivedAddress,
499+
_addresses_derived_count: u32,
496500
user_data: *mut c_void,
497501
) {
498502
let Some(tracker) = (unsafe { tracker_from(user_data) }) else {

0 commit comments

Comments
 (0)