From 69aad4b381acf8dea77f38b8ef2a588623f2340f Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 13:20:08 +0200 Subject: [PATCH 01/17] feat(platform): shielded transaction history MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The wallet persisted shielded materials (own notes, OVK-recovered sends) but no user-facing history. This derives a per-operation activity log — client-side only, by explicit privacy decision: no node/DAPI queries or note→transition indexes were added, so a rescanning wallet's traffic stays indistinguishable from a syncing one. Two derivation paths share one id (sha256 of the bundle's wallet-visible output cmxs), so a live entry and a later rescan of the same bundle dedupe to one row, and a richer entry upgrades a coarser one in place: - Live recorder: each operation path (shield, fund-from-asset-lock, transfer, unshield, withdraw, identity-create) records a Pending entry at build time — exact type, amount, fee, recipient, memo, identity id — and flips it Confirmed/Failed after broadcast. - Scan deriver (restore path): clusters persisted notes / outgoing notes by block height and classifies by where value landed. A change note's rho equals the nullifier of the action that consumed our note, so a rho∈own-nullifiers match is zero-false-positive proof of an own spend, recovering exact amounts (and exact fees for transfers). The link is probabilistic in recall (~50% per 2-action bundle — Orchard shuffles spends and outputs independently before pairing), so unlinked self-pay clusters surface as honest self-transfers rather than a spend masquerading as "Received"; true third-party receives (no OVK pairing) stay Received. Stack: ShieldedActivityEntry + deriver + recorder + store/changeset plumbing in rs-platform-wallet; persist/load callbacks in rs-platform-wallet-ffi; PersistentShieldedActivity SwiftData model + persister wiring in swift-sdk; Shielded Activity list + detail view in SwiftExampleApp. Verified on devnet paloma: backfill over a wallet with 45 notes classified a week of history with no false receives; a live transfer recorded Sent with the exact pinned fee (162,851,200 credits) and a round-tripped memo; the scan-derived entry for a pre-feature transfer recovered recipient, amount, and memo via OVK. Co-Authored-By: Claude Fable 5 --- .../rs-platform-wallet-ffi/src/persistence.rs | 312 +++++- .../src/shielded_persistence.rs | 98 ++ .../src/changeset/shielded_changeset.rs | 135 ++- .../changeset/shielded_sync_start_state.rs | 11 +- .../src/wallet/platform_wallet.rs | 2 + .../src/wallet/shielded/activity.rs | 955 ++++++++++++++++++ .../src/wallet/shielded/activity_recorder.rs | 237 +++++ .../src/wallet/shielded/coordinator.rs | 112 ++ .../src/wallet/shielded/file_store.rs | 44 + .../wallet/shielded/fund_from_asset_lock.rs | 214 ++-- .../src/wallet/shielded/mod.rs | 7 + .../src/wallet/shielded/operations.rs | 336 +++++- .../src/wallet/shielded/store.rs | 196 ++++ .../sync/ovk_builder_roundtrip_tests.rs | 64 ++ .../Persistence/DashModelContainer.swift | 1 + .../Models/PersistentShieldedActivity.swift | 124 +++ .../PlatformWalletPersistenceHandler.swift | 303 ++++++ .../Core/Views/ShieldedActivityView.swift | 311 ++++++ .../Core/Views/WalletDetailView.swift | 23 + .../Core/Views/WalletsContentView.swift | 3 + 20 files changed, 3421 insertions(+), 67 deletions(-) create mode 100644 packages/rs-platform-wallet/src/wallet/shielded/activity.rs create mode 100644 packages/rs-platform-wallet/src/wallet/shielded/activity_recorder.rs create mode 100644 packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentShieldedActivity.swift create mode 100644 packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/ShieldedActivityView.swift diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index 1e16bd3efb7..6e572d52b55 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -354,6 +354,19 @@ pub struct PersistenceCallbacks { count: usize, ) -> i32, >, + /// Persist a batch of derived activity-log entries. The host upserts + /// each by `entry_id` (Pending→Confirmed/Failed flips and scan-kind + /// refinements re-emit the same id). Mirrors the other + /// `on_persist_shielded_*` callbacks. + #[cfg(feature = "shielded")] + pub on_persist_shielded_activity_fn: Option< + unsafe extern "C" fn( + context: *mut c_void, + wallet_id: *const u8, + entries: *const crate::shielded_persistence::ShieldedActivityFFI, + count: usize, + ) -> i32, + >, /// Restore-on-load: every persisted shielded note. Host /// allocates the array; Rust calls the matching free /// callback after copying. Same lifetime contract as @@ -414,6 +427,26 @@ pub struct PersistenceCallbacks { count: usize, ), >, + /// Restore-on-load: every persisted activity-log entry. Same + /// host-allocates / Rust-frees lifetime contract as + /// `on_load_shielded_notes_fn`. Inlined so cbindgen emits the + /// referenced struct in the header. + #[cfg(feature = "shielded")] + pub on_load_shielded_activity_fn: Option< + unsafe extern "C" fn( + context: *mut c_void, + out_entries: *mut *const crate::shielded_persistence::ShieldedActivityRestoreFFI, + out_count: *mut usize, + ) -> i32, + >, + #[cfg(feature = "shielded")] + pub on_load_shielded_activity_free_fn: Option< + unsafe extern "C" fn( + context: *mut c_void, + entries: *const crate::shielded_persistence::ShieldedActivityRestoreFFI, + count: usize, + ), + >, /// Look up a single core transaction record by `txid` for the /// asset-lock proof flow's persister fallback. /// @@ -542,6 +575,8 @@ impl Default for PersistenceCallbacks { #[cfg(feature = "shielded")] on_persist_shielded_synced_indices_fn: None, #[cfg(feature = "shielded")] + on_persist_shielded_activity_fn: None, + #[cfg(feature = "shielded")] on_load_shielded_notes_fn: None, #[cfg(feature = "shielded")] on_load_shielded_notes_free_fn: None, @@ -553,6 +588,10 @@ impl Default for PersistenceCallbacks { on_load_shielded_sync_states_fn: None, #[cfg(feature = "shielded")] on_load_shielded_sync_states_free_fn: None, + #[cfg(feature = "shielded")] + on_load_shielded_activity_fn: None, + #[cfg(feature = "shielded")] + on_load_shielded_activity_free_fn: None, } } } @@ -1282,6 +1321,103 @@ impl PlatformWalletPersistence for FFIPersister { } } } + + // 5) activity entries (derived activity log). The variable- + // length fields (counterparty / memo / cmx + nullifier + // arrays) borrow into `backing`, a Vec of owned byte + // buffers that outlives the callback — same pointer-validity + // discipline as `note_data_ptr` / `memo_ptr` above. The + // host upserts by `entry_id`. + if !shielded_cs.activity_entries.is_empty() { + if let Some(cb) = self.callbacks.on_persist_shielded_activity_fn { + // Concatenated cmx / nullifier buffers, kept alive for + // the callback window. One owned `Vec` per entry's + // note_cmxs and spent_nullifiers. + let mut backing: Vec<(Vec, Vec)> = Vec::new(); + for entries in shielded_cs.activity_entries.values() { + for e in entries { + let mut cmx_buf = Vec::with_capacity(e.note_cmxs.len() * 32); + for c in &e.note_cmxs { + cmx_buf.extend_from_slice(c); + } + let mut nf_buf = Vec::with_capacity(e.spent_nullifiers.len() * 32); + for n in &e.spent_nullifiers { + nf_buf.extend_from_slice(n); + } + backing.push((cmx_buf, nf_buf)); + } + } + let mut backing_iter = backing.iter(); + let entries: Vec = shielded_cs + .activity_entries + .iter() + .flat_map(|(id, entries)| { + entries.iter().map(move |e| (id, e)) + }) + .map(|(id, e)| { + let (cmx_buf, nf_buf) = backing_iter + .next() + .expect("backing has one entry per activity entry"); + let (identity_id, has_identity_id) = match &e.kind { + platform_wallet::wallet::shielded::ShieldedActivityKind::IdentityCreate { + identity_id, + } => (*identity_id, 1u8), + _ => ([0u8; 32], 0u8), + }; + let (counterparty_ptr, counterparty_len) = match &e.counterparty { + Some(c) if !c.is_empty() => (c.as_ptr(), c.len()), + _ => (std::ptr::null(), 0), + }; + let (memo_ptr, memo_len) = match &e.memo { + Some(m) if !m.is_empty() => (m.as_ptr(), m.len()), + _ => (std::ptr::null(), 0), + }; + ShieldedActivityFFI { + wallet_id: id.wallet_id, + account_index: id.account_index, + entry_id: e.id, + kind_tag: e.kind.tag(), + direction: activity_direction_tag(&e.direction), + status: activity_status_tag(&e.status), + amount: e.amount, + fee: e.fee.unwrap_or(0), + has_fee: u8::from(e.fee.is_some()), + block_height: e.block_height.unwrap_or(0), + has_block_height: u8::from(e.block_height.is_some()), + created_at_ms: e.created_at_ms, + identity_id, + has_identity_id, + counterparty_ptr, + counterparty_len, + memo_ptr, + memo_len, + note_cmxs_ptr: cmx_buf.as_ptr(), + note_cmxs_count: cmx_buf.len() / 32, + spent_nullifiers_ptr: nf_buf.as_ptr(), + spent_nullifiers_count: nf_buf.len() / 32, + } + }) + .collect(); + let result = unsafe { + cb( + self.callbacks.context, + wallet_id.as_ptr(), + entries.as_ptr(), + entries.len(), + ) + }; + if result != 0 { + eprintln!( + "Shielded activity persistence callback returned error code {}", + result + ); + round_success = false; + } + // `backing` and `entries` drop here, after the callback + // has copied everything it needs. + drop(backing); + } + } } // Close the round. Clients use this to commit (if @@ -1405,7 +1541,8 @@ impl PlatformWalletPersistence for FFIPersister { use crate::shielded_persistence::*; use platform_wallet::changeset::{ShieldedSubwalletStartState, ShieldedSyncStartState}; use platform_wallet::wallet::shielded::{ - ShieldedNote, ShieldedOutgoingNote, SubwalletId, + ShieldedActivityEntry, ShieldedActivityKind, ShieldedActivityStatus, + ShieldedDirection, ShieldedNote, ShieldedOutgoingNote, SubwalletId, }; let mut shielded_state = ShieldedSyncStartState::default(); @@ -1624,6 +1761,134 @@ impl PlatformWalletPersistence for FFIPersister { } } + // 4) derived activity entries + if self.callbacks.on_load_shielded_activity_fn.is_some() + != self.callbacks.on_load_shielded_activity_free_fn.is_some() + { + return Err(PersistenceError::backend( + "on_load_shielded_activity_fn and on_load_shielded_activity_free_fn must be \ + provided together", + )); + } + if let Some(load_activity) = self.callbacks.on_load_shielded_activity_fn { + let mut act_ptr: *const ShieldedActivityRestoreFFI = std::ptr::null(); + let mut act_count: usize = 0; + let rc = + unsafe { load_activity(self.callbacks.context, &mut act_ptr, &mut act_count) }; + if rc != 0 { + return Err(PersistenceError::backend(format!( + "on_load_shielded_activity_fn returned error code {}", + rc + ))); + } + struct ActivityGuard { + context: *mut c_void, + free_fn: Option< + unsafe extern "C" fn( + context: *mut c_void, + entries: *const ShieldedActivityRestoreFFI, + count: usize, + ), + >, + entries: *const ShieldedActivityRestoreFFI, + count: usize, + } + impl Drop for ActivityGuard { + fn drop(&mut self) { + if let Some(free_fn) = self.free_fn { + unsafe { free_fn(self.context, self.entries, self.count) }; + } + } + } + let _activity_guard = ActivityGuard { + context: self.callbacks.context, + free_fn: self.callbacks.on_load_shielded_activity_free_fn, + entries: act_ptr, + count: act_count, + }; + if !act_ptr.is_null() && act_count > 0 { + let slice = unsafe { slice::from_raw_parts(act_ptr, act_count) }; + for ffi in slice { + let kind = match ffi.kind_tag { + 0 => ShieldedActivityKind::Shield, + 1 => ShieldedActivityKind::ShieldFromAssetLock, + 2 => ShieldedActivityKind::Received, + 3 => ShieldedActivityKind::Sent, + 4 => ShieldedActivityKind::Unshield, + 5 => ShieldedActivityKind::Withdrawal, + 6 => ShieldedActivityKind::IdentityCreate { + identity_id: ffi.identity_id, + }, + // 7 and any unknown tag fall back to the + // residual — a forward-compat tag we don't yet + // model still loads as an opaque spend rather + // than getting dropped. + _ => ShieldedActivityKind::ShieldedSpend, + }; + let direction = match ffi.direction { + 0 => ShieldedDirection::In, + 1 => ShieldedDirection::Out, + _ => ShieldedDirection::SelfTransfer, + }; + let status = match ffi.status { + 0 => ShieldedActivityStatus::Pending, + 1 => ShieldedActivityStatus::Confirmed, + _ => ShieldedActivityStatus::Failed, + }; + let counterparty = if ffi.counterparty_ptr.is_null() + || ffi.counterparty_len == 0 + { + None + } else { + Some(unsafe { + slice::from_raw_parts(ffi.counterparty_ptr, ffi.counterparty_len) + .to_vec() + }) + }; + let memo = if ffi.memo_ptr.is_null() || ffi.memo_len == 0 { + None + } else { + Some(unsafe { + slice::from_raw_parts(ffi.memo_ptr, ffi.memo_len).to_vec() + }) + }; + let note_cmxs = + unsafe { decode_cmx_array(ffi.note_cmxs_ptr, ffi.note_cmxs_count) }; + let spent_nullifiers = unsafe { + decode_cmx_array(ffi.spent_nullifiers_ptr, ffi.spent_nullifiers_count) + }; + + let id = SubwalletId::new(ffi.wallet_id, ffi.account_index); + let entry = shielded_state + .per_subwallet + .entry(id) + .or_insert_with(ShieldedSubwalletStartState::default); + entry.activity.push(ShieldedActivityEntry { + id: ffi.entry_id, + kind, + direction, + amount: ffi.amount, + fee: if ffi.has_fee != 0 { + Some(ffi.fee) + } else { + None + }, + counterparty, + memo, + block_height: if ffi.has_block_height != 0 { + Some(ffi.block_height) + } else { + None + }, + status, + created_at_ms: ffi.created_at_ms, + note_cmxs, + spent_nullifiers, + }); + } + } + } + out.shielded = shielded_state; } @@ -1833,6 +2098,51 @@ impl PlatformWalletPersistence for FFIPersister { } /// Flatten an `AccountType` + encoded xpub into the C-flat +/// Decode `count` contiguous 32-byte commitments / nullifiers from a +/// host buffer into `Vec<[u8; 32]>`. A null pointer or a length that +/// isn't a clean multiple of 32 yields an empty vec (the buffer is +/// Rust-written on persist and host-round-tripped on load, so a +/// malformed length means a corrupt row — drop the linkage rather than +/// read past the buffer). +/// +/// # Safety +/// `ptr` must point to at least `count * 32` valid bytes for the call, +/// or be null. +#[cfg(feature = "shielded")] +unsafe fn decode_cmx_array(ptr: *const u8, count: usize) -> Vec<[u8; 32]> { + if ptr.is_null() || count == 0 { + return Vec::new(); + } + let bytes = slice::from_raw_parts(ptr, count * 32); + bytes + .chunks_exact(32) + .filter_map(|c| <[u8; 32]>::try_from(c).ok()) + .collect() +} + +/// Discriminant byte for a `ShieldedDirection` (FFI: 0 In, 1 Out, 2 Self). +#[cfg(feature = "shielded")] +fn activity_direction_tag(d: &platform_wallet::wallet::shielded::ShieldedDirection) -> u8 { + use platform_wallet::wallet::shielded::ShieldedDirection::*; + match d { + In => 0, + Out => 1, + SelfTransfer => 2, + } +} + +/// Discriminant byte for a `ShieldedActivityStatus` (FFI: 0 Pending, +/// 1 Confirmed, 2 Failed). +#[cfg(feature = "shielded")] +fn activity_status_tag(s: &platform_wallet::wallet::shielded::ShieldedActivityStatus) -> u8 { + use platform_wallet::wallet::shielded::ShieldedActivityStatus::*; + match s { + Pending => 0, + Confirmed => 1, + Failed => 2, + } +} + /// [`AccountSpecFFI`] layout. /// /// The returned struct borrows `xpub_bytes` — caller must keep the diff --git a/packages/rs-platform-wallet-ffi/src/shielded_persistence.rs b/packages/rs-platform-wallet-ffi/src/shielded_persistence.rs index a0838b14fb5..9b01a85cd57 100644 --- a/packages/rs-platform-wallet-ffi/src/shielded_persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/shielded_persistence.rs @@ -100,6 +100,73 @@ pub struct ShieldedSyncedIndexFFI { pub last_synced_index: u64, } +/// One derived shielded-activity entry for the host to persist. +/// +/// Mirror of `platform_wallet::wallet::shielded::ShieldedActivityEntry`. +/// The host writes one row keyed by `entry_id` (sha256 of the visible +/// output cmxs); re-persisting the same `entry_id` is an upsert that +/// refines the row (Pending→Confirmed/Failed, or a scan-derived +/// `ShieldedSpend` upgraded to a richer kind). All pointers are valid +/// only for the callback window — the host must copy. +/// +/// `Option` fields are flattened to a value + a `has_*` flag (`u8`, +/// 1 = present) rather than a sentinel, so `0`/empty is unambiguous. +#[repr(C)] +pub struct ShieldedActivityFFI { + /// 32-byte wallet identifier. + pub wallet_id: [u8; 32], + /// ZIP-32 account index. + pub account_index: u32, + /// Entry id (sha256 of sorted visible output cmxs). Primary key. + pub entry_id: [u8; 32], + /// Kind discriminant (see `ShieldedActivityKind::tag`): + /// 0 Shield, 1 ShieldFromAssetLock, 2 Received, 3 Sent, 4 Unshield, + /// 5 Withdrawal, 6 IdentityCreate, 7 ShieldedSpend. + pub kind_tag: u8, + /// Direction: 0 In, 1 Out, 2 Self. + pub direction: u8, + /// Status: 0 Pending, 1 Confirmed, 2 Failed. + pub status: u8, + /// Display amount in credits (principal; excludes self-change and + /// zero-value fillers). + pub amount: u64, + /// Exact fee in credits when `has_fee == 1`. + pub fee: u64, + /// `1` if `fee` is meaningful, `0` if the fee is unknown. + pub has_fee: u8, + /// Block height when `has_block_height == 1`. + pub block_height: u64, + /// `1` if `block_height` is meaningful (confirmed), `0` while pending. + pub has_block_height: u8, + /// Created-at time in ms since the Unix epoch (display-only; + /// `block_height` is the canonical sort key). + pub created_at_ms: u64, + /// Created identity id (only meaningful when `kind_tag == 6` / + /// IdentityCreate); all-zero and ignored otherwise. + pub identity_id: [u8; 32], + /// `1` when `identity_id` is meaningful (IdentityCreate), else `0`. + pub has_identity_id: u8, + /// Counterparty bytes pointer (43B Orchard / 21B PlatformAddress / + /// Core script) or null. Valid for the callback window only. + pub counterparty_ptr: *const u8, + /// Length of `counterparty_ptr` in bytes (0 when null). + pub counterparty_len: usize, + /// 36-byte memo pointer or null. Valid for the callback window only. + pub memo_ptr: *const u8, + /// Length of `memo_ptr` in bytes (0 when null). + pub memo_len: usize, + /// Pointer to the concatenated visible-output cmxs (`note_cmxs_count` + /// × 32 bytes). Valid for the callback window only. + pub note_cmxs_ptr: *const u8, + /// Number of 32-byte cmxs at `note_cmxs_ptr`. + pub note_cmxs_count: usize, + /// Pointer to the concatenated spent nullifiers (`spent_nullifiers_count` + /// × 32 bytes). Valid for the callback window only. + pub spent_nullifiers_ptr: *const u8, + /// Number of 32-byte nullifiers at `spent_nullifiers_ptr`. + pub spent_nullifiers_count: usize, +} + // ── Restore (load) ────────────────────────────────────────────────────── /// One persisted note as the host hands it back at boot. Mirrors @@ -146,6 +213,37 @@ pub struct ShieldedSubwalletSyncStateFFI { pub last_synced_index: u64, } +/// One persisted activity entry as the host hands it back at boot. +/// Mirrors [`ShieldedActivityFFI`] but lives in a Swift-allocated array, +/// so the buffer ownership / free contract differs (see the matching +/// `on_load_shielded_activity_free_fn`). Field semantics are identical +/// to [`ShieldedActivityFFI`]. +#[repr(C)] +pub struct ShieldedActivityRestoreFFI { + pub wallet_id: [u8; 32], + pub account_index: u32, + pub entry_id: [u8; 32], + pub kind_tag: u8, + pub direction: u8, + pub status: u8, + pub amount: u64, + pub fee: u64, + pub has_fee: u8, + pub block_height: u64, + pub has_block_height: u8, + pub created_at_ms: u64, + pub identity_id: [u8; 32], + pub has_identity_id: u8, + pub counterparty_ptr: *const u8, + pub counterparty_len: usize, + pub memo_ptr: *const u8, + pub memo_len: usize, + pub note_cmxs_ptr: *const u8, + pub note_cmxs_count: usize, + pub spent_nullifiers_ptr: *const u8, + pub spent_nullifiers_count: usize, +} + // The `on_load_shielded_*_fn` callback types are inlined inside // [`PersistenceCallbacks`] (rather than declared as `pub type` // aliases here) so cbindgen sees the full signature, walks into diff --git a/packages/rs-platform-wallet/src/changeset/shielded_changeset.rs b/packages/rs-platform-wallet/src/changeset/shielded_changeset.rs index 4dde8b701e3..1380317d7b8 100644 --- a/packages/rs-platform-wallet/src/changeset/shielded_changeset.rs +++ b/packages/rs-platform-wallet/src/changeset/shielded_changeset.rs @@ -18,7 +18,9 @@ use std::collections::BTreeMap; use crate::changeset::merge::Merge; -use crate::wallet::shielded::{ShieldedNote, ShieldedOutgoingNote, SubwalletId}; +use crate::wallet::shielded::{ + ShieldedActivityEntry, ShieldedNote, ShieldedOutgoingNote, SubwalletId, +}; /// Aggregated delta of shielded state for one persister flush. #[derive(Debug, Clone, Default)] @@ -41,6 +43,14 @@ pub struct ShieldedChangeSet { /// Latest per-subwallet `last_synced_note_index`. Last write /// wins on merge (sync only ever advances this monotonically). pub synced_indices: BTreeMap, + /// Derived activity-log entries to persist, per subwallet. Keyed by + /// `(wallet_id, account_index)`; the persister upserts by + /// `entry.id` (sha256 of the visible output cmxs), so a `Pending` + /// entry's later `Confirmed`/`Failed` re-emit, or a scan-derived + /// `ShieldedSpend`'s later refinement, overwrites the existing row. + /// Defaults empty so every pre-activity flow rides the existing + /// changeset path unchanged. + pub activity_entries: BTreeMap>, } impl ShieldedChangeSet { @@ -50,6 +60,7 @@ impl ShieldedChangeSet { && self.nullifiers_spent.is_empty() && self.outgoing_notes.is_empty() && self.synced_indices.is_empty() + && self.activity_entries.is_empty() } /// Accumulator helper: record a saved note for `id`. @@ -67,6 +78,13 @@ impl ShieldedChangeSet { self.outgoing_notes.entry(id).or_default().push(note); } + /// Accumulator helper: record a derived activity entry for `id`. + /// The persister upserts by `entry.id`, so re-recording the same id + /// with a refined kind / flipped status replaces the prior row. + pub fn record_activity_entry(&mut self, id: SubwalletId, entry: ShieldedActivityEntry) { + self.activity_entries.entry(id).or_default().push(entry); + } + /// Accumulator helper: advance the per-subwallet sync watermark. pub fn record_synced_index(&mut self, id: SubwalletId, index: u64) { let entry = self.synced_indices.entry(id).or_insert(index); @@ -92,6 +110,7 @@ impl ShieldedChangeSet { nullifiers_spent, outgoing_notes, synced_indices, + activity_entries, } = self; let mut out: BTreeMap = BTreeMap::new(); @@ -119,6 +138,12 @@ impl ShieldedChangeSet { .synced_indices .insert(id, idx); } + for (id, entries) in activity_entries { + out.entry(id.wallet_id) + .or_default() + .activity_entries + .insert(id, entries); + } // Defensive: drop empty entries so the persister doesn't // see noise. `split_by_wallet_id` is called on the result // of a sync pass where at least one map is non-empty @@ -147,9 +172,117 @@ impl Merge for ShieldedChangeSet { *entry = idx; } } + // Activity entries append; the persister upserts by `entry.id`, + // so a later flip/refinement of the same id (appended after the + // original) wins at persist time without needing to dedupe here. + for (id, entries) in other.activity_entries { + self.activity_entries.entry(id).or_default().extend(entries); + } } fn is_empty(&self) -> bool { ShieldedChangeSet::is_empty(self) } } + +#[cfg(test)] +mod activity_changeset_tests { + use super::*; + use crate::wallet::shielded::{ + ShieldedActivityEntry, ShieldedActivityKind, ShieldedActivityStatus, ShieldedDirection, + }; + + fn sub(account: u32) -> SubwalletId { + SubwalletId::new([0xDD; 32], account) + } + + fn entry(id: u8, status: ShieldedActivityStatus) -> ShieldedActivityEntry { + ShieldedActivityEntry { + id: [id; 32], + kind: ShieldedActivityKind::Sent, + direction: ShieldedDirection::Out, + amount: 100, + fee: Some(1), + counterparty: None, + memo: None, + block_height: None, + status, + created_at_ms: 0, + note_cmxs: vec![[id; 32]], + spent_nullifiers: vec![], + } + } + + /// A default `ShieldedChangeSet` (no activity) stays empty — the new + /// field must not perturb the old flush short-circuit. + #[test] + fn default_changeset_with_no_activity_is_empty() { + let cs = ShieldedChangeSet::default(); + assert!(cs.is_empty()); + assert!(crate::changeset::merge::Merge::is_empty(&cs)); + } + + /// Recording an activity entry makes the changeset non-empty so it + /// rides the existing flush. + #[test] + fn recording_activity_makes_changeset_nonempty() { + let mut cs = ShieldedChangeSet::default(); + cs.record_activity_entry(sub(0), entry(1, ShieldedActivityStatus::Pending)); + assert!(!cs.is_empty()); + assert_eq!(cs.activity_entries.get(&sub(0)).map(|v| v.len()), Some(1)); + } + + /// Merge appends activity entries; the persister upserts by id, so a + /// later Confirmed re-emit of the same id appears after the Pending + /// one and wins at persist time. + #[test] + fn merge_appends_activity_entries_in_order() { + let mut a = ShieldedChangeSet::default(); + a.record_activity_entry(sub(0), entry(7, ShieldedActivityStatus::Pending)); + let mut b = ShieldedChangeSet::default(); + b.record_activity_entry(sub(0), entry(7, ShieldedActivityStatus::Confirmed)); + + crate::changeset::merge::Merge::merge(&mut a, b); + let entries = a.activity_entries.get(&sub(0)).expect("entries present"); + assert_eq!( + entries.len(), + 2, + "merge appends both (upsert-by-id at flush)" + ); + assert_eq!(entries[0].status, ShieldedActivityStatus::Pending); + assert_eq!( + entries[1].status, + ShieldedActivityStatus::Confirmed, + "the Confirmed re-emit lands after the Pending one" + ); + } + + /// `split_by_wallet_id` routes activity entries to the owning wallet. + #[test] + fn split_by_wallet_id_routes_activity() { + let wallet_a = [0x01; 32]; + let wallet_b = [0x02; 32]; + let mut cs = ShieldedChangeSet::default(); + cs.record_activity_entry( + SubwalletId::new(wallet_a, 0), + entry(1, ShieldedActivityStatus::Confirmed), + ); + cs.record_activity_entry( + SubwalletId::new(wallet_b, 0), + entry(2, ShieldedActivityStatus::Confirmed), + ); + + let split = cs.split_by_wallet_id(); + assert_eq!(split.len(), 2); + assert!(split[&wallet_a] + .activity_entries + .contains_key(&SubwalletId::new(wallet_a, 0))); + assert!(split[&wallet_b] + .activity_entries + .contains_key(&SubwalletId::new(wallet_b, 0))); + // No cross-leakage. + assert!(!split[&wallet_a] + .activity_entries + .contains_key(&SubwalletId::new(wallet_b, 0))); + } +} diff --git a/packages/rs-platform-wallet/src/changeset/shielded_sync_start_state.rs b/packages/rs-platform-wallet/src/changeset/shielded_sync_start_state.rs index 7ac1d327c58..2a28de79dab 100644 --- a/packages/rs-platform-wallet/src/changeset/shielded_sync_start_state.rs +++ b/packages/rs-platform-wallet/src/changeset/shielded_sync_start_state.rs @@ -13,7 +13,9 @@ //! [`ShieldedWallet`]: crate::wallet::shielded::ShieldedWallet //! [`SubwalletId`]: crate::wallet::shielded::SubwalletId -use crate::wallet::shielded::{ShieldedNote, ShieldedOutgoingNote, SubwalletId}; +use crate::wallet::shielded::{ + ShieldedActivityEntry, ShieldedNote, ShieldedOutgoingNote, SubwalletId, +}; use std::collections::BTreeMap; /// Per-subwallet snapshot — every note (spent + unspent) the @@ -29,6 +31,13 @@ pub struct ShieldedSubwalletStartState { /// in-memory store's send history survives a cold start without /// re-recovering every note. Idempotent on re-record by `cmx`. pub outgoing_notes: Vec, + /// Derived activity-log entries persisted on prior sessions (live + /// recordings + scan derivations). Rehydrated so the scan deriver's + /// `existing_ids` set includes them — otherwise a cold-started scan + /// would re-derive a coarse `Sent` / `ShieldedSpend` for a cluster a + /// rich live entry already owns and overwrite it (the persister + /// upserts by `entry.id`). Idempotent on re-save by `id`. + pub activity: Vec, /// Sync watermark: count of note positions scanned = the next /// global index to scan (exclusive). `0` = nothing scanned yet. pub last_synced_index: u64, diff --git a/packages/rs-platform-wallet/src/wallet/platform_wallet.rs b/packages/rs-platform-wallet/src/wallet/platform_wallet.rs index 9cddb9f415e..61ca0a5db9b 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_wallet.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_wallet.rs @@ -948,6 +948,8 @@ impl PlatformWallet { })?; super::shielded::operations::shield( &self.sdk, + Some(&self.persister), + self.wallet_id, keyset, shielded_account, inputs, diff --git a/packages/rs-platform-wallet/src/wallet/shielded/activity.rs b/packages/rs-platform-wallet/src/wallet/shielded/activity.rs new file mode 100644 index 00000000000..2ca224c9203 --- /dev/null +++ b/packages/rs-platform-wallet/src/wallet/shielded/activity.rs @@ -0,0 +1,955 @@ +//! Derived shielded-activity log: a user-facing history of shielded +//! operations. +//! +//! The wallet already persists the *raw* shielded materials — own +//! received notes ([`ShieldedNote`]) and OVK-recovered sends +//! ([`ShieldedOutgoingNote`]) — but nothing that reads as an +//! operation-level activity feed ("you sent 0.5 DASH", "you created an +//! identity"). This module derives that feed: +//! +//! - **Live recorder** (the rich path): each of the six shielded +//! operation call sites records a [`ShieldedActivityEntry`] at +//! execution time, when the operation type, amounts, fee, recipient, +//! memo, spent notes, and created-identity id are all known exactly. +//! See `operations.rs` / `fund_from_asset_lock.rs`. +//! - **Scan deriver** (the restore path): on a wallet that upgraded +//! with existing note/outgoing-note state (or whose live entries were +//! lost), [`derive_activity_from_scan_data`] reconstructs entries +//! best-effort from the data the wallet already holds — clustering +//! own notes, OVK sends, and spends by block height. Classification is +//! 100% client-side (Option B): it queries nothing new (no node/DAPI, +//! no note→transition index). +//! +//! # The dedupe contract (READ THIS BEFORE TOUCHING [`ShieldedActivityEntry::id`]) +//! +//! Both paths key an entry by [`ShieldedActivityEntry::id`] = +//! `sha256(sorted visible output cmxs)`. "Visible output cmxs" means the +//! commitments of the outputs the WALLET CAN SEE for that bundle: +//! - the wallet's own received notes ([`ShieldedNote::cmx`]), AND +//! - the wallet's OVK-recovered sends ([`ShieldedOutgoingNote::cmx`]). +//! +//! Dummy / unrecoverable anonymity-set fillers are excluded (the wallet +//! can't see their cmx as a note or an outgoing note). The live recorder +//! computes the id over exactly this same set (own note cmxs of the +//! built bundle + the recovered/known output cmxs), so when a later +//! rescan re-derives the same cluster it produces the *same* id and the +//! persister upsert collapses them into one row — the live entry's rich +//! classification wins, the scan entry is dropped. +//! +//! Compute the id ONLY via [`compute_activity_id`] on both sides so the +//! sort-then-hash stays identical. + +use std::collections::BTreeMap; + +use crate::wallet::shielded::store::{ShieldedNote, ShieldedOutgoingNote}; + +/// How an entry's net direction reads relative to the wallet. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub enum ShieldedDirection { + /// Value entered the wallet's shielded pool (Shield / Received). + In, + /// Value left the wallet's shielded pool to another party + /// (Sent / Unshield / Withdrawal / IdentityCreate). + Out, + /// A wallet-internal move with no net party change (e.g. a residual + /// spend whose outputs are all self-change). + SelfTransfer, +} + +/// Confirmation status of a recorded activity entry. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub enum ShieldedActivityStatus { + /// Built + broadcast, not yet confirmed on chain. + Pending, + /// Observed confirmed (the live flip after `broadcast_and_wait`, or + /// a scan that matched the cluster). + Confirmed, + /// The operation definitively failed (broadcast rejected on merits). + Failed, +} + +/// The classified kind of a shielded operation. +/// +/// Designed to be **upgradeable in place**: a rescan or a later +/// correlation pass may refine [`ShieldedActivityKind::ShieldedSpend`] +/// (the unclassified residual) into a specific kind. The persister +/// upserts by [`ShieldedActivityEntry::id`], so re-emitting the same id +/// with a sharper `kind` updates the existing row rather than +/// duplicating it. +#[derive(Debug, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub enum ShieldedActivityKind { + /// Type 15: transparent platform addresses → shielded pool. + Shield, + /// Type 18: Core L1 asset lock → shielded pool. + ShieldFromAssetLock, + /// Inbound shielded note(s) with no own spend in the cluster. On the + /// restore path a Shield is indistinguishable from a third-party + /// receive, so both surface as `Received` (honest by construction). + Received, + /// Type 16: shielded pool → another Orchard address (a private send). + Sent, + /// Type 17: shielded pool → transparent platform address. + Unshield, + /// Type 19: shielded pool → Core L1 address. + Withdrawal, + /// Type 20: shielded pool → a brand-new Platform identity. + IdentityCreate { + /// The created identity's id (32 bytes). + identity_id: [u8; 32], + }, + /// Own spend whose outputs are all self-change and which no + /// correlation arm could refine. The honest residual on the restore + /// path. Carries `fee: None` because the exact fee is underivable + /// from scan data alone. + ShieldedSpend, +} + +impl ShieldedActivityKind { + /// A stable discriminant byte for the FFI / persistence layer. + /// + /// Kept explicit (rather than relying on `as u8`) so adding a + /// variant in the middle can't silently renumber a persisted kind. + pub fn tag(&self) -> u8 { + match self { + ShieldedActivityKind::Shield => 0, + ShieldedActivityKind::ShieldFromAssetLock => 1, + ShieldedActivityKind::Received => 2, + ShieldedActivityKind::Sent => 3, + ShieldedActivityKind::Unshield => 4, + ShieldedActivityKind::Withdrawal => 5, + ShieldedActivityKind::IdentityCreate { .. } => 6, + ShieldedActivityKind::ShieldedSpend => 7, + } + } +} + +/// One shielded operation, as a user-facing activity record. +/// +/// See the module docs for the [`Self::id`] dedupe contract and the +/// live-vs-scan recording split. +#[derive(Debug, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct ShieldedActivityEntry { + /// `sha256(sorted visible output cmxs)` — the dedupe key shared by + /// the live recorder and the scan deriver. See the module docs. + pub id: [u8; 32], + /// The classified operation kind. Upgradeable in place (see + /// [`ShieldedActivityKind`]). + pub kind: ShieldedActivityKind, + /// Net direction relative to the wallet. + pub direction: ShieldedDirection, + /// Display amount in credits: the operation principal, excluding + /// self-change outputs and zero-value fillers. For a send this is the + /// value paid to the counterparty; for a receive/shield the value + /// received. + pub amount: u64, + /// Exact fee in credits when derivable (live entries: + /// `spent − outputs − change`); `None` when underivable (restored + /// `ShieldedSpend`). + pub fee: Option, + /// Counterparty bytes, kind-dependent: + /// - `Sent`: 43-byte raw Orchard address. + /// - `Unshield`: 21-byte serialized `PlatformAddress`. + /// - `Withdrawal`: Core output script / address bytes. + /// `None` for self / receive / shield kinds. + pub counterparty: Option>, + /// 36-byte `DashMemo` when present and non-zero; `None` otherwise. + pub memo: Option>, + /// Block height the operation confirmed at. `None` while pending or + /// when the live confirm couldn't read it from the proof metadata — + /// the scan deriver fills it in later. **Canonical sort key** (desc), + /// with pendings (None) floated to the top. + pub block_height: Option, + /// Confirmation status. + pub status: ShieldedActivityStatus, + /// `SystemTime` (ms since epoch) at record time. Display-only and the + /// sort tiebreak after `block_height`; never the primary sort key. + pub created_at_ms: u64, + /// cmxs of the visible outputs that fed [`Self::id`] (own notes + + /// recovered sends). Linkage for status confirmation and dedupe. + /// Stored as `Vec<[u8;32]>` (serde derive covers `[u8;32]`). + pub note_cmxs: Vec<[u8; 32]>, + /// Nullifiers of the notes this operation spent (empty for inbound + /// kinds). Linkage for confirmation and dedupe. + pub spent_nullifiers: Vec<[u8; 32]>, +} + +impl ShieldedActivityEntry { + /// Current wall-clock time in milliseconds since the Unix epoch. + /// + /// Display-only (`created_at_ms`); a clock that's briefly behind + /// only perturbs the sort tiebreak, never the `block_height`-first + /// ordering, so a non-monotonic read is harmless here. + pub fn now_ms() -> u64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_millis() as u64) + .unwrap_or(0) + } +} + +/// Compute the dedupe id over a set of visible output cmxs. +/// +/// **The single source of truth for [`ShieldedActivityEntry::id`].** Both +/// the live recorder and the scan deriver MUST call this so the +/// sort-then-hash is byte-identical and a later rescan dedupes against +/// the live entry. Duplicate cmxs are de-duplicated before hashing so a +/// note that appears as both a received note and (defensively) an +/// outgoing note can't perturb the id. +/// +/// An empty set hashes to `sha256("")` — a degenerate but stable id; +/// callers that could produce an empty visible set (a fully-dummy +/// bundle) should not record an entry at all. +pub fn compute_activity_id(cmxs: &[[u8; 32]]) -> [u8; 32] { + use dpp::dashcore::hashes::{sha256, Hash, HashEngine}; + + let mut sorted: Vec<[u8; 32]> = cmxs.to_vec(); + sorted.sort_unstable(); + sorted.dedup(); + + let mut engine = sha256::Hash::engine(); + for cmx in &sorted { + engine.input(cmx); + } + sha256::Hash::from_engine(engine).to_byte_array() +} + +/// The set of events the scan deriver clusters by block height for one +/// subwallet, gathered from the data the wallet already persists. +/// +/// Pure inputs — built from [`ShieldedStore::get_all_notes`] and +/// [`ShieldedStore::get_outgoing_notes`] by the caller — so the +/// clustering + classification logic stays unit-testable without a live +/// store or network. +/// +/// [`ShieldedStore::get_all_notes`]: crate::wallet::shielded::ShieldedStore::get_all_notes +/// [`ShieldedStore::get_outgoing_notes`]: crate::wallet::shielded::ShieldedStore::get_outgoing_notes +#[derive(Debug, Clone, Default)] +pub struct ScanDeriveInput { + /// All known notes (spent + unspent) for the subwallet. + pub notes: Vec, + /// OVK-recovered outgoing (sent) notes for the subwallet. + pub outgoing: Vec, + /// The wallet's own Orchard addresses (43-byte raw bytes), used to + /// tell "sent to someone else" from "sent to self / change". Pass + /// every diversified address the subwallet can recognize; an + /// outgoing note whose recipient is in this set is self-change. + pub own_addresses: Vec>, +} + +/// One block-height cluster of a subwallet's shielded events. +/// +/// Documented limitation (acceptable for v1): two same-wallet bundles +/// landing in the *same block* merge into one cluster — height is the +/// only join key available client-side without a note→transition index +/// (which Option B forbids). +#[derive(Debug, Clone, Default)] +struct HeightCluster { + /// Own new notes that first appeared at this height. + received: Vec, + /// OVK-recovered sends at this height. + outgoing: Vec, + // NOTE: no `spent` field. A note's stored `block_height` is its + // *receipt* height; scan-based spend detection flips `is_spent` + // without recording the spend's height (Option B forbids querying + // it), so spends can't be height-clustered from note data alone. + // See [`cluster_events`] for how that bounds classification. +} + +/// Whether an outgoing note's recipient is one of the wallet's own +/// addresses (i.e. self-change rather than a payment out). +fn is_own_recipient(recipient: &[u8], own_addresses: &[Vec]) -> bool { + own_addresses.iter().any(|a| a.as_slice() == recipient) +} + +/// Extract a note's `rho` from its serialized `note_data` +/// (`recipient(43) || value(8 LE) || rho(32) || rseed(32)`, 115 bytes — +/// the layout documented on [`ShieldedNote::note_data`]). +/// +/// `rho` is the nullifier of the ACTION that created the note: when one +/// of our new notes has `rho` equal to the nullifier of one of our own +/// (now spent) notes, that new note is provably the change of our own +/// spend — the strongest classification signal available client-side. +/// +/// The link is zero-false-positive but PROBABILISTIC in recall: Orchard's +/// builder shuffles spends and outputs independently before pairing them +/// into actions (`indexed_spends.shuffle` / `indexed_outputs.shuffle` in +/// `orchard::builder`), so in a 2-action bundle the change output sits in +/// the same action as the real spend only ~half the time. A missed link +/// degrades to the honest `ShieldedSpend`/self-transfer arm — never to a +/// wrong `Received`. +fn note_rho(note_data: &[u8]) -> Option<[u8; 32]> { + if note_data.len() != 115 { + return None; + } + let mut rho = [0u8; 32]; + rho.copy_from_slice(¬e_data[51..83]); + Some(rho) +} + +/// Cluster a subwallet's events by block height. +/// +/// Receipts and sends carry a real `block_height` from the scan, so they +/// cluster cleanly. Spends are the hard case: a note's stored +/// `block_height` is its *receipt* height, and scan-based spend +/// detection flips `is_spent` without recording *when* it was spent +/// (the spend height isn't persisted anywhere the wallet layer can read +/// — Option B forbids querying it). So a spent own note can only be +/// attributed to its own receipt-height cluster, which is wrong for the +/// spend. We therefore DO NOT try to height-attribute spends here; the +/// classifier uses the per-cluster receive/send shape plus the global +/// "did this subwallet spend anything" signal. This is the v1 +/// limitation called out in the task: spend-only residuals can't be +/// height-correlated client-side, so they fall to `ShieldedSpend`. +fn cluster_events(input: &ScanDeriveInput) -> BTreeMap { + let mut clusters: BTreeMap = BTreeMap::new(); + for note in &input.notes { + clusters + .entry(note.block_height) + .or_default() + .received + .push(note.clone()); + } + for out in &input.outgoing { + clusters + .entry(out.block_height) + .or_default() + .outgoing + .push(out.clone()); + } + clusters +} + +/// Reconstruct activity entries best-effort from a subwallet's persisted +/// scan data. The restore-path counterpart to the live recorder. +/// +/// Classification (client-side only — Option B), in priority order: +/// - cluster has OVK outgoing note(s) to a NON-own address → [`Sent`] +/// (counterparty / value / memo from the outgoing notes; own receipts +/// are the change and excluded from the amount). When the rho linkage +/// (see [`note_rho`]) identifies the consumed note(s), the exact fee +/// (spent − sent − change) is recovered too. +/// - rho-linked cluster with no external recipient → [`ShieldedSpend`] +/// with direction `Out` and the exact amount that left the pool +/// (spent − change). This is provably our own spend (unshield / +/// withdrawal / identity-create — the type itself isn't recoverable +/// client-side). +/// - own new note(s) with NO OVK pairing at all → [`Received`]. A true +/// third-party receive has exactly this shape (we never hold the +/// sender's OVK), so this arm is reliable. +/// - self-pay cluster without a rho link → [`ShieldedSpend`] with +/// direction `SelfTransfer`: a shield-to-self and an unlinked change +/// (the ~50% shuffle miss) are indistinguishable, and labeling either +/// `Received` would misrepresent a spend as money arriving. +/// +/// **Correlation arms a/b/c are NOT implemented here** — see the crate +/// report. The data they'd need (per-identity registration height, +/// per-height platform-address credits, locally-indexed withdrawal docs) +/// is not reachable in `rs-platform-wallet` today, and Option B forbids +/// adding node queries to get it. Those arms remain TODOs; the residual +/// stays `ShieldedSpend`, which a later live entry (or a future +/// correlation pass) can upgrade in place via the shared id. +/// +/// `existing_ids` are entry ids already on file (live entries win): a +/// cluster whose computed id is present is skipped, so a live +/// `IdentityCreate` / `Unshield` / `Withdrawal` is never clobbered by a +/// coarser scan-derived `Sent` / `ShieldedSpend`. +/// +/// [`Sent`]: ShieldedActivityKind::Sent +/// [`Received`]: ShieldedActivityKind::Received +/// [`ShieldedSpend`]: ShieldedActivityKind::ShieldedSpend +pub fn derive_activity_from_scan_data( + input: &ScanDeriveInput, + existing_ids: &std::collections::BTreeSet<[u8; 32]>, +) -> Vec { + let clusters = cluster_events(input); + let mut out = Vec::new(); + let now_ms = ShieldedActivityEntry::now_ms(); + + // Own-nullifier → (value, cmx) lookup for the rho linkage: a cluster + // note whose `rho` is one of our own nullifiers is provably the + // change of our own spend, and the matched note is what we spent. + let own_nullifiers: BTreeMap<[u8; 32], (u64, [u8; 32])> = input + .notes + .iter() + .map(|n| (n.nullifier, (n.value, n.cmx))) + .collect(); + + for (height, cluster) in clusters { + // Visible output cmxs for this cluster: own received-note cmxs + + // recovered-send cmxs. This is exactly the set the live recorder + // hashes, so the ids line up for dedupe. + let mut visible_cmxs: Vec<[u8; 32]> = Vec::new(); + for n in &cluster.received { + visible_cmxs.push(n.cmx); + } + for o in &cluster.outgoing { + // `ShieldedOutgoingNote::cmx` is a fixed `[u8; 32]` (the cmx is + // always 32 bytes; only the 43-byte recipient/36-byte memo + // needed the `Vec` serde workaround). + visible_cmxs.push(o.cmx); + } + if visible_cmxs.is_empty() { + continue; + } + let id = compute_activity_id(&visible_cmxs); + if existing_ids.contains(&id) { + // A live entry (or an earlier scan) already owns this cluster. + continue; + } + + // Partition outgoing notes into external payments vs self-change. + let (external, _self_change): (Vec<&ShieldedOutgoingNote>, Vec<&ShieldedOutgoingNote>) = + cluster + .outgoing + .iter() + .partition(|o| !is_own_recipient(&o.recipient, &input.own_addresses)); + + // Rho linkage: a cluster note whose rho is one of OUR nullifiers + // is provably change from our own spend, and tells us exactly + // which note (value) was consumed. Exclude self-matches (a note's + // rho can't be its own nullifier, but a merged cluster could pair + // a note with a sibling created in the same block — require the + // matched spent note to be a different cmx). + let mut linked_spent_total: u64 = 0; + let mut linked_nullifiers: Vec<[u8; 32]> = Vec::new(); + for n in &cluster.received { + if let Some(rho) = note_rho(&n.note_data) { + if let Some((spent_value, spent_cmx)) = own_nullifiers.get(&rho) { + if *spent_cmx != n.cmx { + linked_spent_total = linked_spent_total.saturating_add(*spent_value); + linked_nullifiers.push(rho); + } + } + } + } + let change_total: u64 = cluster.received.iter().map(|n| n.value).sum(); + + let entry = if let Some(send) = external.first() { + // SENT: at least one outgoing note to an address we don't own. + // Amount = sum of external sends; own receipts in the cluster + // are the change and are excluded. When the rho linkage + // identified the consumed note(s), the exact fee falls out: + // spent − sent − change. + let amount: u64 = external.iter().map(|o| o.value).sum(); + let fee = (!linked_nullifiers.is_empty()) + .then(|| { + linked_spent_total + .checked_sub(amount) + .and_then(|r| r.checked_sub(change_total)) + }) + .flatten(); + ShieldedActivityEntry { + id, + kind: ShieldedActivityKind::Sent, + direction: ShieldedDirection::Out, + amount, + fee, + counterparty: Some(send.recipient.clone()), + memo: non_zero_memo(&send.memo), + block_height: Some(height), + status: ShieldedActivityStatus::Confirmed, + created_at_ms: now_ms, + note_cmxs: visible_cmxs, + spent_nullifiers: linked_nullifiers, + } + } else if !linked_nullifiers.is_empty() { + // LINKED SPEND: no external recipient, but the rho linkage + // proves our own note(s) were consumed and this cluster's + // notes are the change. Amount = value that left the pool + // (spent − change; includes the fee, which can't be split out + // without knowing the transition type). An unshield / + // withdrawal / identity-create on a restored wallet surfaces + // here; a later correlation or live entry can refine the kind. + ShieldedActivityEntry { + id, + kind: ShieldedActivityKind::ShieldedSpend, + direction: ShieldedDirection::Out, + amount: linked_spent_total.saturating_sub(change_total), + fee: None, + counterparty: None, + memo: None, + block_height: Some(height), + status: ShieldedActivityStatus::Confirmed, + created_at_ms: now_ms, + note_cmxs: visible_cmxs, + spent_nullifiers: linked_nullifiers, + } + } else if cluster.outgoing.is_empty() { + // RECEIVED: own new notes with no OVK pairing at all. A true + // third-party receive has exactly this shape (we never hold + // the sender's OVK), so `Received` is reliable here. + ShieldedActivityEntry { + id, + kind: ShieldedActivityKind::Received, + direction: ShieldedDirection::In, + amount: change_total, + fee: None, + counterparty: None, + memo: None, + block_height: Some(height), + status: ShieldedActivityStatus::Confirmed, + created_at_ms: now_ms, + note_cmxs: visible_cmxs, + spent_nullifiers: Vec::new(), + } + } else { + // SELF-PAY, UNLINKED: every output pays ourselves and no rho + // link fired. Two indistinguishable possibilities: a shield to + // self (Type 15/18 — the common case) or change from our own + // spend whose output landed in a different action than the + // real spend (the ~50% shuffle miss; see `note_rho`). Calling + // it `Received` would show a spend as money arriving, so this + // is surfaced as a self-transfer instead — honest for both + // readings and upgradeable in place by a live entry. + ShieldedActivityEntry { + id, + kind: ShieldedActivityKind::ShieldedSpend, + direction: ShieldedDirection::SelfTransfer, + amount: change_total, + fee: None, + counterparty: None, + memo: None, + block_height: Some(height), + status: ShieldedActivityStatus::Confirmed, + created_at_ms: now_ms, + note_cmxs: visible_cmxs, + spent_nullifiers: Vec::new(), + } + }; + out.push(entry); + } + + out +} + +/// Return `Some(memo)` when `memo` is non-empty and not all-zero; +/// `None` otherwise. A zero-filled 36-byte `DashMemo` is the "no memo" +/// sentinel and shouldn't surface as an attached memo. +fn non_zero_memo(memo: &[u8]) -> Option> { + if memo.is_empty() || memo.iter().all(|&b| b == 0) { + None + } else { + Some(memo.to_vec()) + } +} + +/// Sort entries for display: confirmed/failed by `block_height` +/// descending with pendings (no height) floated to the very top, then +/// tiebreak by `created_at_ms` descending, then by `id` for a total +/// order. Mutates in place. +pub fn sort_activity_for_display(entries: &mut [ShieldedActivityEntry]) { + entries.sort_by(|a, b| { + // Pendings (block_height == None) first. Among heights, larger + // (more recent) first. + match (a.block_height, b.block_height) { + (None, Some(_)) => std::cmp::Ordering::Less, + (Some(_), None) => std::cmp::Ordering::Greater, + (Some(ah), Some(bh)) => bh.cmp(&ah), + (None, None) => std::cmp::Ordering::Equal, + } + // Tiebreak: more recent record time first. + .then_with(|| b.created_at_ms.cmp(&a.created_at_ms)) + // Final total order so the sort is deterministic. + .then_with(|| a.id.cmp(&b.id)) + }); +} + +#[cfg(test)] +mod tests { + use super::*; + use std::collections::BTreeSet; + + fn own_note(cmx: u8, nullifier: u8, height: u64, value: u64, spent: bool) -> ShieldedNote { + ShieldedNote { + position: 0, + cmx: [cmx; 32], + nullifier: [nullifier; 32], + block_height: height, + is_spent: spent, + value, + note_data: vec![0u8; 115], + } + } + + fn outgoing( + cmx: u8, + recipient: Vec, + height: u64, + value: u64, + memo: Vec, + ) -> ShieldedOutgoingNote { + ShieldedOutgoingNote { + cmx: [cmx; 32], + recipient, + value, + memo, + block_height: height, + } + } + + fn addr(b: u8) -> Vec { + vec![b; 43] + } + + // ── id / dedupe ──────────────────────────────────────────────── + + #[test] + fn id_is_order_independent_and_dedupes_duplicates() { + let a = [1u8; 32]; + let b = [2u8; 32]; + let c = [3u8; 32]; + // Same set, different order → same id. + assert_eq!( + compute_activity_id(&[a, b, c]), + compute_activity_id(&[c, a, b]) + ); + // A duplicated cmx doesn't change the id. + assert_eq!( + compute_activity_id(&[a, b]), + compute_activity_id(&[a, b, a]) + ); + // A different set → different id. + assert_ne!(compute_activity_id(&[a, b]), compute_activity_id(&[a, c])); + } + + #[test] + fn live_entry_then_scan_of_same_cluster_yields_one_entry() { + // Live recorder recorded a Sent for a bundle whose single visible + // output cmx is `7`. The scan later sees the same cluster (one + // outgoing note with cmx `7`). Because the id matches and the live + // id is in `existing_ids`, the scan produces NOTHING for it. + let cmx = [7u8; 32]; + let live_id = compute_activity_id(&[cmx]); + let mut existing = BTreeSet::new(); + existing.insert(live_id); + + let input = ScanDeriveInput { + notes: vec![], + outgoing: vec![outgoing(7, addr(0xEE), 100, 500, vec![])], + own_addresses: vec![addr(0x01)], + }; + let derived = derive_activity_from_scan_data(&input, &existing); + assert!( + derived.is_empty(), + "scan must not re-emit a cluster a live entry already owns" + ); + + // With no live entry on file, the same scan DOES produce one entry, + // and its id equals the live id (the dedupe contract). + let derived2 = derive_activity_from_scan_data(&input, &BTreeSet::new()); + assert_eq!(derived2.len(), 1); + assert_eq!(derived2[0].id, live_id); + } + + // ── classification table (scan path) ─────────────────────────── + + #[test] + fn classifies_received_when_only_own_notes() { + let input = ScanDeriveInput { + notes: vec![own_note(0x10, 0x20, 50, 1_000, false)], + outgoing: vec![], + own_addresses: vec![addr(0x01)], + }; + let d = derive_activity_from_scan_data(&input, &BTreeSet::new()); + assert_eq!(d.len(), 1); + assert_eq!(d[0].kind, ShieldedActivityKind::Received); + assert_eq!(d[0].direction, ShieldedDirection::In); + assert_eq!(d[0].amount, 1_000); + assert_eq!(d[0].block_height, Some(50)); + } + + #[test] + fn classifies_sent_when_outgoing_to_external_address() { + let memo = { + let mut m = vec![0u8; 36]; + m[0] = 1; // kind tag + m[4] = b'h'; + m + }; + let input = ScanDeriveInput { + notes: vec![], + outgoing: vec![outgoing(0x30, addr(0xEE), 200, 750, memo.clone())], + own_addresses: vec![addr(0x01)], + }; + let d = derive_activity_from_scan_data(&input, &BTreeSet::new()); + assert_eq!(d.len(), 1); + assert_eq!(d[0].kind, ShieldedActivityKind::Sent); + assert_eq!(d[0].direction, ShieldedDirection::Out); + assert_eq!(d[0].amount, 750); + assert_eq!(d[0].counterparty.as_deref(), Some(addr(0xEE).as_slice())); + assert_eq!(d[0].memo, Some(memo)); + } + + #[test] + fn sent_with_own_change_in_same_cluster_excludes_change_from_amount() { + // A send that also produced an own change note at the same height: + // the change receipt must NOT inflate the amount, and the cluster + // classifies as Sent (not Received). + let input = ScanDeriveInput { + notes: vec![own_note(0x40, 0x41, 300, 9_999, false)], // change + outgoing: vec![outgoing(0x42, addr(0xEE), 300, 600, vec![])], // payment + own_addresses: vec![addr(0x01)], + }; + let d = derive_activity_from_scan_data(&input, &BTreeSet::new()); + assert_eq!(d.len(), 1); + assert_eq!(d[0].kind, ShieldedActivityKind::Sent); + assert_eq!(d[0].amount, 600, "amount is the payment, not the change"); + } + + #[test] + fn self_change_only_cluster_is_shielded_spend() { + // An outgoing note paying one of our OWN addresses, nothing else: + // a spend whose output is all self-change → residual ShieldedSpend. + let own = addr(0x01); + let input = ScanDeriveInput { + notes: vec![], + outgoing: vec![outgoing(0x50, own.clone(), 400, 0, vec![])], + own_addresses: vec![own], + }; + let d = derive_activity_from_scan_data(&input, &BTreeSet::new()); + assert_eq!(d.len(), 1); + assert_eq!(d[0].kind, ShieldedActivityKind::ShieldedSpend); + assert_eq!(d[0].direction, ShieldedDirection::SelfTransfer); + assert!(d[0].fee.is_none()); + } + + // ── zero-value filler / memo exclusion ───────────────────────── + + #[test] + fn zero_memo_is_not_attached() { + let input = ScanDeriveInput { + notes: vec![], + outgoing: vec![outgoing(0x60, addr(0xEE), 100, 500, vec![0u8; 36])], + own_addresses: vec![addr(0x01)], + }; + let d = derive_activity_from_scan_data(&input, &BTreeSet::new()); + assert_eq!(d.len(), 1); + assert!(d[0].memo.is_none(), "an all-zero memo must surface as None"); + } + + #[test] + fn empty_cluster_visible_set_is_skipped() { + // No notes, no outgoing → no clusters → no entries. + let input = ScanDeriveInput::default(); + let d = derive_activity_from_scan_data(&input, &BTreeSet::new()); + assert!(d.is_empty()); + } + + // ── backfill over existing store (multiple heights) ──────────── + + #[test] + fn backfill_over_existing_notes_produces_one_entry_per_cluster() { + // A wallet that upgraded with existing state: receives at h=10, + // a send at h=20. Backfill (existing_ids empty) yields exactly two + // entries, one per height cluster. + let input = ScanDeriveInput { + notes: vec![own_note(0x70, 0x71, 10, 2_000, false)], + outgoing: vec![outgoing(0x72, addr(0xEE), 20, 800, vec![])], + own_addresses: vec![addr(0x01)], + }; + let mut d = derive_activity_from_scan_data(&input, &BTreeSet::new()); + sort_activity_for_display(&mut d); + assert_eq!(d.len(), 2); + // After display sort, the more recent (h=20 Sent) comes first. + assert_eq!(d[0].kind, ShieldedActivityKind::Sent); + assert_eq!(d[0].block_height, Some(20)); + assert_eq!(d[1].kind, ShieldedActivityKind::Received); + assert_eq!(d[1].block_height, Some(10)); + } + + #[test] + fn backfill_skips_clusters_already_recorded() { + // h=10 already has a live entry; only h=20 should be derived. + let input = ScanDeriveInput { + notes: vec![own_note(0x70, 0x71, 10, 2_000, false)], + outgoing: vec![outgoing(0x72, addr(0xEE), 20, 800, vec![])], + own_addresses: vec![addr(0x01)], + }; + let live_id = compute_activity_id(&[[0x70u8; 32]]); + let mut existing = BTreeSet::new(); + existing.insert(live_id); + let d = derive_activity_from_scan_data(&input, &existing); + assert_eq!(d.len(), 1); + assert_eq!(d[0].block_height, Some(20)); + } + + // ── ordering ─────────────────────────────────────────────────── + + #[test] + fn display_sort_floats_pendings_then_height_desc() { + let mk = |height: Option, created: u64, id: u8| ShieldedActivityEntry { + id: [id; 32], + kind: ShieldedActivityKind::Sent, + direction: ShieldedDirection::Out, + amount: 1, + fee: None, + counterparty: None, + memo: None, + block_height: height, + status: if height.is_none() { + ShieldedActivityStatus::Pending + } else { + ShieldedActivityStatus::Confirmed + }, + created_at_ms: created, + note_cmxs: vec![[id; 32]], + spent_nullifiers: vec![], + }; + let mut v = vec![ + mk(Some(100), 1, 1), + mk(None, 5, 2), // pending — must float to top + mk(Some(300), 2, 3), + mk(Some(200), 3, 4), + ]; + sort_activity_for_display(&mut v); + assert_eq!(v[0].block_height, None, "pending floats to top"); + assert_eq!(v[1].block_height, Some(300)); + assert_eq!(v[2].block_height, Some(200)); + assert_eq!(v[3].block_height, Some(100)); + } + + #[test] + fn kind_tags_are_stable_and_distinct() { + use std::collections::BTreeSet as Set; + let kinds = [ + ShieldedActivityKind::Shield, + ShieldedActivityKind::ShieldFromAssetLock, + ShieldedActivityKind::Received, + ShieldedActivityKind::Sent, + ShieldedActivityKind::Unshield, + ShieldedActivityKind::Withdrawal, + ShieldedActivityKind::IdentityCreate { + identity_id: [0u8; 32], + }, + ShieldedActivityKind::ShieldedSpend, + ]; + let tags: Set = kinds.iter().map(|k| k.tag()).collect(); + assert_eq!(tags.len(), kinds.len(), "every kind tag must be distinct"); + } + + // ── rho linkage (restore-path spend identification) ─────────── + + /// Build a note whose `note_data` carries `rho` at the documented + /// offset (recipient(43) || value(8) || rho(32) || rseed(32)). + fn own_note_with_rho( + cmx: u8, + nullifier: u8, + height: u64, + value: u64, + rho: [u8; 32], + ) -> ShieldedNote { + let mut data = vec![0u8; 115]; + data[51..83].copy_from_slice(&rho); + ShieldedNote { + position: 0, + cmx: [cmx; 32], + nullifier: [nullifier; 32], + block_height: height, + is_spent: false, + value, + note_data: data, + } + } + + #[test] + fn rho_linked_change_classifies_as_spend_with_exact_amount() { + // Spent note (0.4979) at h13; its spender's change (0.3979) lands + // at h113 with rho == the spent note's nullifier. The change also + // appears as a self-pay OVK note (same cmx) — the exact shape a + // restored wallet sees after an unshield / identity-create. + let own = addr(0xAA); + let spent = own_note(1, 7, 13, 49_787_148_800, true); + let change = own_note_with_rho(2, 8, 113, 39_787_148_800, [7u8; 32]); + let input = ScanDeriveInput { + notes: vec![spent, change], + outgoing: vec![outgoing(2, own.clone(), 113, 39_787_148_800, vec![0u8; 36])], + own_addresses: vec![own], + }; + let d = derive_activity_from_scan_data(&input, &BTreeSet::new()); + let spend = d + .iter() + .find(|e| e.block_height == Some(113)) + .expect("spend cluster entry"); + assert_eq!(spend.kind, ShieldedActivityKind::ShieldedSpend); + assert_eq!(spend.direction, ShieldedDirection::Out); + assert_eq!( + spend.amount, 10_000_000_000, + "amount must be spent − change (value that left the pool)" + ); + assert_eq!(spend.spent_nullifiers, vec![[7u8; 32]]); + } + + #[test] + fn rho_linked_external_send_recovers_exact_fee() { + // Transfer: spend 0.5, send 0.1 to a foreign address, change back + // to self, rho links the change to the spent note → the exact fee + // (spent − sent − change) is recoverable on restore. + let own = addr(0xAA); + let foreign = addr(0xBB); + let spent = own_note(1, 7, 10, 50_000_000_000, true); + let change = own_note_with_rho(2, 8, 20, 39_900_000_000, [7u8; 32]); + let input = ScanDeriveInput { + notes: vec![spent, change], + outgoing: vec![ + outgoing(3, foreign.clone(), 20, 10_000_000_000, vec![0u8; 36]), + outgoing(2, own.clone(), 20, 39_900_000_000, vec![0u8; 36]), + ], + own_addresses: vec![own], + }; + let d = derive_activity_from_scan_data(&input, &BTreeSet::new()); + let sent = d + .iter() + .find(|e| e.block_height == Some(20)) + .expect("send cluster entry"); + assert_eq!(sent.kind, ShieldedActivityKind::Sent); + assert_eq!(sent.amount, 10_000_000_000); + assert_eq!( + sent.fee, + Some(100_000_000), + "rho linkage makes the exact fee recoverable" + ); + assert_eq!(sent.counterparty.as_deref(), Some(foreign.as_slice())); + } + + #[test] + fn unlinked_self_pay_is_self_transfer_not_received() { + // Self-pay cluster with no rho link: either a shield-to-self or + // change whose output landed in a different action than the real + // spend (the ~50% shuffle miss). Must NOT surface as `Received` — + // that would show a spend as money arriving. + let own = addr(0xAA); + let note = own_note(1, 7, 30, 1_000_000, false); + let input = ScanDeriveInput { + notes: vec![note], + outgoing: vec![outgoing(1, own.clone(), 30, 1_000_000, vec![0u8; 36])], + own_addresses: vec![own], + }; + let d = derive_activity_from_scan_data(&input, &BTreeSet::new()); + assert_eq!(d.len(), 1); + assert_eq!(d[0].kind, ShieldedActivityKind::ShieldedSpend); + assert_eq!(d[0].direction, ShieldedDirection::SelfTransfer); + assert_eq!(d[0].amount, 1_000_000); + } + + #[test] + fn third_party_receive_stays_received() { + // A true receive: own new note with NO OVK pairing at all (we + // never hold the sender's OVK). `Received` is reliable here. + let own = addr(0xAA); + let note = own_note(1, 7, 40, 5_000_000_000, false); + let input = ScanDeriveInput { + notes: vec![note], + outgoing: vec![], + own_addresses: vec![own], + }; + let d = derive_activity_from_scan_data(&input, &BTreeSet::new()); + assert_eq!(d.len(), 1); + assert_eq!(d[0].kind, ShieldedActivityKind::Received); + assert_eq!(d[0].direction, ShieldedDirection::In); + assert_eq!(d[0].amount, 5_000_000_000); + } +} diff --git a/packages/rs-platform-wallet/src/wallet/shielded/activity_recorder.rs b/packages/rs-platform-wallet/src/wallet/shielded/activity_recorder.rs new file mode 100644 index 00000000000..1be1fda427f --- /dev/null +++ b/packages/rs-platform-wallet/src/wallet/shielded/activity_recorder.rs @@ -0,0 +1,237 @@ +//! Live activity recording for the six shielded operation paths. +//! +//! At execution time each operation knows everything about itself — the +//! type, amounts, fee, recipient, memo, spent notes, and (for Type 20) +//! the created identity id. This module turns that knowledge into a +//! [`ShieldedActivityEntry`] and ships it through the +//! [`ShieldedChangeSet`] so it rides the existing persister flush. +//! +//! # Why recover cmxs from the built bundle +//! +//! The dedupe contract (see [`super::activity`]) keys an entry by +//! `sha256(sorted visible output cmxs)`, where "visible" = the outputs +//! the wallet's own IVK / OVK can recognize (own change notes + the +//! recovered send), excluding dummy fillers and spend actions. The scan +//! deriver computes that set from persisted notes / outgoing notes; for +//! the live entry to dedupe against a later rescan it must compute the +//! SAME set. So rather than guess which serialized action is an output, +//! we run the wallet's own IVK decrypt + OVK recovery over the built +//! bundle's actions — exactly what the scan does — and take the cmxs +//! that recover. This guarantees `live id == scan id` by construction. + +use dash_sdk::platform::shielded::{try_decrypt_note, try_recover_outgoing_note}; +// `ShieldedEncryptedNote` is the wire form the scan re-parses each +// on-chain note into. `drive-proof-verifier` is only a dev-dependency +// of this crate, so reach the type through `dash-sdk`'s public +// re-export (`pub use drive_proof_verifier::types as query_types`) +// rather than depending on it directly. +use dash_sdk::query_types::ShieldedEncryptedNote; +use grovedb_commitment_tree::ExtractedNoteCommitment; + +use super::activity::{ + compute_activity_id, ShieldedActivityEntry, ShieldedActivityKind, ShieldedActivityStatus, + ShieldedDirection, +}; +use super::keys::OrchardKeySet; +use super::store::{ShieldedNote, SubwalletId}; +use crate::changeset::ShieldedChangeSet; + +use dpp::shielded::SerializedAction; + +/// Reconstruct the wallet-visible output cmxs from a built bundle's +/// serialized actions, using the wallet's own viewing keys — the same +/// IVK-decrypt + OVK-recover the scan runs, so the resulting id matches. +/// +/// An action's cmx is included iff: +/// - it IVK-decrypts under `keys.incoming_viewing_key` (an own output — +/// a change note or a self-send), OR +/// - it OVK-recovers under `keys.outgoing_viewing_key` (a send the +/// wallet authored, encrypted to its own OVK at build time). +/// +/// Dummy fillers (random OVK, not own IVK) and spend actions (no +/// recoverable output plaintext for this wallet) contribute nothing — +/// exactly the outputs the scan can't see either. Order-independent: the +/// caller hashes via [`compute_activity_id`], which sorts + dedups. +pub fn visible_output_cmxs(actions: &[SerializedAction], keys: &OrchardKeySet) -> Vec<[u8; 32]> { + let prepared_ivk = keys.prepared_ivk(); + let mut cmxs: Vec<[u8; 32]> = Vec::new(); + for a in actions { + let wire = ShieldedEncryptedNote { + cmx: a.cmx.to_vec(), + nullifier: a.nullifier.to_vec(), + cv_net: a.cv_net.to_vec(), + encrypted_note: a.encrypted_note.clone(), + }; + // Own output (change / self-send) via IVK. + if let Some((note, _addr)) = try_decrypt_note(&prepared_ivk, &wire) { + cmxs.push(ExtractedNoteCommitment::from(note.commitment()).to_bytes()); + continue; + } + // Authored send via OVK (encrypted to the wallet's own OVK). + if let Some((note, _recipient, _memo)) = + try_recover_outgoing_note(&keys.outgoing_viewing_key, &wire) + { + cmxs.push(ExtractedNoteCommitment::from(note.commitment()).to_bytes()); + } + } + cmxs +} + +/// Inputs the recorder needs that aren't carried by the kind itself. +pub struct LiveEntryParams<'a> { + /// The classified kind (rich — the live path knows it exactly). + pub kind: ShieldedActivityKind, + /// Net direction relative to the wallet. + pub direction: ShieldedDirection, + /// Display amount in credits (principal — excludes self-change and + /// zero-value fillers). + pub amount: u64, + /// Exact fee in credits when derivable. The live path derives it as + /// `spent − outputs − change`; pass `None` only when genuinely + /// unknown. + pub fee: Option, + /// Counterparty bytes (43B Orchard / 21B PlatformAddress / Core + /// script) or `None`. + pub counterparty: Option>, + /// 36-byte memo when present and non-zero. + pub memo: Option>, + /// The bundle's serialized actions, for cmx extraction + nullifier + /// linkage. + pub actions: &'a [SerializedAction], + /// The notes this operation spent (for `spent_nullifiers` linkage). + /// Empty for output-only kinds (Shield / ShieldFromAssetLock). + pub spent_notes: &'a [ShieldedNote], +} + +/// Build a `Pending` [`ShieldedActivityEntry`] from the live operation's +/// full knowledge. Returns `None` when the bundle exposes no +/// wallet-visible output cmx (a degenerate all-dummy bundle), so the +/// caller skips recording rather than persisting a degenerate id. +pub fn build_pending_entry( + keys: &OrchardKeySet, + params: LiveEntryParams<'_>, +) -> Option { + let note_cmxs = visible_output_cmxs(params.actions, keys); + if note_cmxs.is_empty() { + return None; + } + let id = compute_activity_id(¬e_cmxs); + let spent_nullifiers: Vec<[u8; 32]> = params.spent_notes.iter().map(|n| n.nullifier).collect(); + Some(ShieldedActivityEntry { + id, + kind: params.kind, + direction: params.direction, + amount: params.amount, + fee: params.fee, + counterparty: params.counterparty, + memo: params.memo, + block_height: None, + status: ShieldedActivityStatus::Pending, + created_at_ms: ShieldedActivityEntry::now_ms(), + note_cmxs, + spent_nullifiers, + }) +} + +/// Return a copy of `entry` with its status set to `status` and (when +/// confirming) `block_height` filled if known. Re-emitting this through +/// the changeset upserts the existing row by `entry.id`. +pub fn with_status( + entry: &ShieldedActivityEntry, + status: ShieldedActivityStatus, + block_height: Option, +) -> ShieldedActivityEntry { + let mut next = entry.clone(); + next.status = status; + if block_height.is_some() { + next.block_height = block_height; + } + next +} + +/// Convert a fixed 36-byte memo into `Some(memo)` when it carries +/// content, or `None` when it's the all-zero "no memo" sentinel. Shared +/// by the operation paths so a zero memo never surfaces as an attached +/// note. +pub fn non_zero_memo(memo: &[u8; 36]) -> Option> { + if memo.iter().all(|&b| b == 0) { + None + } else { + Some(memo.to_vec()) + } +} + +/// Ship one entry to the host persister through a fresh +/// [`ShieldedChangeSet`]. Convenience for the operation paths, which +/// already hold a `(persister, wallet_id, SubwalletId)` and a queue +/// helper (`queue_shielded_changeset`) — they call that with the +/// changeset this returns. +pub fn changeset_for_entry(id: SubwalletId, entry: ShieldedActivityEntry) -> ShieldedChangeSet { + let mut cs = ShieldedChangeSet::default(); + cs.record_activity_entry(id, entry); + cs +} + +#[cfg(test)] +mod tests { + use super::*; + + /// `with_status` flips status + fills height without disturbing the + /// rest of the entry (the upsert-by-id contract). + #[test] + fn with_status_flips_and_fills_height_only() { + let base = ShieldedActivityEntry { + id: [9u8; 32], + kind: ShieldedActivityKind::Sent, + direction: ShieldedDirection::Out, + amount: 500, + fee: Some(10), + counterparty: Some(vec![1u8; 43]), + memo: None, + block_height: None, + status: ShieldedActivityStatus::Pending, + created_at_ms: 123, + note_cmxs: vec![[7u8; 32]], + spent_nullifiers: vec![[3u8; 32]], + }; + + let confirmed = with_status(&base, ShieldedActivityStatus::Confirmed, Some(4242)); + assert_eq!(confirmed.status, ShieldedActivityStatus::Confirmed); + assert_eq!(confirmed.block_height, Some(4242)); + // Everything else is preserved so the upsert only refines. + assert_eq!(confirmed.id, base.id); + assert_eq!(confirmed.amount, base.amount); + assert_eq!(confirmed.fee, base.fee); + assert_eq!(confirmed.counterparty, base.counterparty); + assert_eq!(confirmed.note_cmxs, base.note_cmxs); + assert_eq!(confirmed.spent_nullifiers, base.spent_nullifiers); + + // Failing without a height leaves block_height untouched (None). + let failed = with_status(&base, ShieldedActivityStatus::Failed, None); + assert_eq!(failed.status, ShieldedActivityStatus::Failed); + assert_eq!(failed.block_height, None); + } + + /// `changeset_for_entry` carries exactly one entry under the given id. + #[test] + fn changeset_for_entry_carries_one_entry() { + let id = SubwalletId::new([0xAA; 32], 0); + let entry = ShieldedActivityEntry { + id: [1u8; 32], + kind: ShieldedActivityKind::Shield, + direction: ShieldedDirection::In, + amount: 1, + fee: None, + counterparty: None, + memo: None, + block_height: None, + status: ShieldedActivityStatus::Pending, + created_at_ms: 0, + note_cmxs: vec![[1u8; 32]], + spent_nullifiers: vec![], + }; + let cs = changeset_for_entry(id, entry); + assert_eq!(cs.activity_entries.get(&id).map(|v| v.len()), Some(1)); + assert!(!cs.is_empty()); + } +} diff --git a/packages/rs-platform-wallet/src/wallet/shielded/coordinator.rs b/packages/rs-platform-wallet/src/wallet/shielded/coordinator.rs index 9093a25c3aa..5c12f8ab068 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/coordinator.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/coordinator.rs @@ -414,6 +414,15 @@ impl NetworkShieldedCoordinator { crate::error::PlatformWalletError::ShieldedStoreError(e.to_string()) })?; } + // Rehydrate persisted activity entries so the scan deriver's + // dedupe set (`existing_ids`) includes them this session — a + // rich live entry restored here is never clobbered by a + // coarser re-derivation. Idempotent (upsert by `entry.id`). + for entry in &sub.activity { + store.save_activity(*id, entry).map_err(|e| { + crate::error::PlatformWalletError::ShieldedStoreError(e.to_string()) + })?; + } store .set_last_synced_note_index(*id, sub.last_synced_index) .map_err(|e| { @@ -618,6 +627,22 @@ impl NetworkShieldedCoordinator { Err(e) => return self.fail_all_wallets(&subwallets, &e), }; + // Restore-path activity derivation: reconstruct per-operation + // activity entries best-effort from the notes / outgoing notes + // this pass (and earlier passes) persisted. Runs every pass so it + // doubles as the one-time backfill over an already-populated + // store — `derive_activity_from_scan_data` skips clusters whose + // id a live entry already owns, and `save_activity` upserts by + // id, so re-running is idempotent. Failures are logged and + // swallowed: a derivation miss must never fail a sync pass. + let mut notes = notes; + if let Err(e) = self + .derive_activity_into_changeset(&subwallets, &mut notes.changeset) + .await + { + tracing::warn!(error = %e, "Shielded activity derivation failed; skipping this pass"); + } + // The note-side changeset already carries saves, synced // indices, AND the scan-detected spends, so split it per // WalletId directly — each per-wallet `WalletPersister.store` @@ -705,6 +730,69 @@ impl NetworkShieldedCoordinator { .unwrap_or(0) } + /// Derive best-effort activity entries from each subwallet's + /// persisted scan data and add the new ones to `changeset`. + /// + /// For each subwallet: read all notes + OVK-recovered outgoing notes + /// from the store, classify the recipient of each outgoing note as + /// own-vs-external by testing it against the subwallet's IVK (the + /// `diversifier_index` check — Orchard addresses are diversified, so + /// a fixed address list can't be used), build the + /// [`super::activity::ScanDeriveInput`], and run + /// [`super::activity::derive_activity_from_scan_data`] against the + /// entry ids already on file (live entries win). Newly derived + /// entries are saved to the store and recorded on `changeset` so they + /// reach the host persister on this pass's flush. + /// + /// All client-side (Option B): no node / DAPI query, only data the + /// store already holds. + async fn derive_activity_into_changeset( + &self, + subwallets: &[(SubwalletId, AccountViewingKeys)], + changeset: &mut crate::changeset::ShieldedChangeSet, + ) -> Result<(), crate::error::PlatformWalletError> { + use super::activity::{derive_activity_from_scan_data, ScanDeriveInput}; + + let mut store = self.store.write().await; + for (id, views) in subwallets { + let notes = store.get_all_notes(*id).map_err(|e| { + crate::error::PlatformWalletError::ShieldedStoreError(e.to_string()) + })?; + let outgoing = store.get_outgoing_notes(*id).map_err(|e| { + crate::error::PlatformWalletError::ShieldedStoreError(e.to_string()) + })?; + if notes.is_empty() && outgoing.is_empty() { + continue; + } + + // Own-recipient set: an outgoing note whose recipient the + // subwallet's IVK recognizes is self-change, not a payment out. + let own_addresses: Vec> = outgoing + .iter() + .filter(|o| is_own_orchard_recipient(views, &o.recipient)) + .map(|o| o.recipient.clone()) + .collect(); + + let existing_ids = store.get_activity_ids(*id).map_err(|e| { + crate::error::PlatformWalletError::ShieldedStoreError(e.to_string()) + })?; + + let input = ScanDeriveInput { + notes, + outgoing, + own_addresses, + }; + let derived = derive_activity_from_scan_data(&input, &existing_ids); + for entry in derived { + store.save_activity(*id, &entry).map_err(|e| { + crate::error::PlatformWalletError::ShieldedStoreError(e.to_string()) + })?; + changeset.record_activity_entry(*id, entry); + } + } + Ok(()) + } + /// Build a pass summary in which every registered wallet is /// reported as a cooldown skip. Used by [`sync`](Self::sync) /// when the network-wide cooldown is in effect. @@ -804,6 +892,30 @@ fn build_per_wallet_summary( summary } +/// Whether a 43-byte raw Orchard `recipient` belongs to the subwallet's +/// own viewing keys — i.e. it's a self-change output, not a payment to +/// someone else. Orchard addresses are diversified, so this can't be a +/// fixed-address comparison; it tests the recipient against the IVK via +/// `diversifier_index` (mirrors the `sender_ovk` account-selection check +/// in `fund_from_asset_lock.rs`). A malformed recipient (wrong length / +/// off-curve) returns `false` (treated as external) so a corrupt row +/// can't silently mask a real send. +fn is_own_orchard_recipient(views: &AccountViewingKeys, recipient: &[u8]) -> bool { + let raw: [u8; 43] = match recipient.try_into() { + Ok(b) => b, + Err(_) => return false, + }; + let Some(addr) = Option::::from( + grovedb_commitment_tree::PaymentAddress::from_raw_address_bytes(&raw), + ) else { + return false; + }; + views + .incoming_viewing_key + .diversifier_index(&addr) + .is_some() +} + #[cfg(test)] mod tests { use super::*; diff --git a/packages/rs-platform-wallet/src/wallet/shielded/file_store.rs b/packages/rs-platform-wallet/src/wallet/shielded/file_store.rs index c63bbe3014a..741bc8bcfad 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/file_store.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/file_store.rs @@ -205,6 +205,50 @@ impl ShieldedStore for FileBackedShieldedStore { .unwrap_or_default()) } + fn save_activity( + &mut self, + id: SubwalletId, + entry: &super::activity::ShieldedActivityEntry, + ) -> Result<(), Self::Error> { + self.subwallets.entry(id).or_default().save_activity(entry); + Ok(()) + } + + fn get_activity( + &self, + id: SubwalletId, + offset: usize, + limit: usize, + ) -> Result, Self::Error> { + Ok(self + .subwallets + .get(&id) + .map(|sw| sw.activity_page(offset, limit)) + .unwrap_or_default()) + } + + fn get_activity_by_entry_id( + &self, + id: SubwalletId, + entry_id: &[u8; 32], + ) -> Result, Self::Error> { + Ok(self + .subwallets + .get(&id) + .and_then(|sw| sw.activity_by_id(entry_id))) + } + + fn get_activity_ids( + &self, + id: SubwalletId, + ) -> Result, Self::Error> { + Ok(self + .subwallets + .get(&id) + .map(SubwalletState::activity_ids) + .unwrap_or_default()) + } + fn append_commitment(&mut self, cmx: &[u8; 32], marked: bool) -> Result<(), Self::Error> { let retention: Retention = if marked { Retention::Marked diff --git a/packages/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rs b/packages/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rs index 164e52a55c0..f3861280b88 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rs @@ -336,64 +336,80 @@ impl PlatformWallet { // retries from silently degrading into duplicate-hash submits. let proof_out_point = out_point_from_proof(&proof); let sdk = self.sdk.clone(); - match submit_with_cl_height_retry(settings, |s| { - build_and_broadcast_shielded( - sdk.clone(), - recipient, - shield_amount, - proof.clone(), - path.clone(), - asset_lock_signer, - &prover, - sender_ovk.clone(), - surplus_output, - dummy_outputs, - s, - ) - }) - .await - { - Ok(()) => {} - Err(e) if is_instant_lock_proof_invalid(&e) => { - let out_point = proof_out_point; - tracing::warn!( - "IS-lock proof rejected by Platform for shielded fund-from-asset-lock \ + // Serialized actions of the bundle that actually landed — fed to + // the live activity recorder below. Each retry re-randomizes the + // bundle, so only the landed attempt's actions are the ones a + // later scan will recover; `build_and_broadcast_shielded` returns + // them on success. + let landed_actions: Vec = + match submit_with_cl_height_retry(settings, |s| { + build_and_broadcast_shielded( + sdk.clone(), + recipient, + shield_amount, + proof.clone(), + path.clone(), + asset_lock_signer, + &prover, + sender_ovk.clone(), + surplus_output, + dummy_outputs, + s, + ) + }) + .await + { + Ok(actions) => actions, + Err(e) if is_instant_lock_proof_invalid(&e) => { + let out_point = proof_out_point; + tracing::warn!( + "IS-lock proof rejected by Platform for shielded fund-from-asset-lock \ (tx {}), retrying with ChainLock proof", - out_point.txid - ); - let chain_proof = self - .asset_locks - .upgrade_to_chain_lock_proof(&out_point, CL_FALLBACK_TIMEOUT) - .await?; - let cs = self - .asset_locks - .advance_asset_lock_status( - &out_point, - crate::wallet::asset_lock::tracked::AssetLockStatus::ChainLocked, - Some(chain_proof.clone()), - ) - .await?; - self.asset_locks.queue_asset_lock_changeset(cs); - submit_with_cl_height_retry(settings, |s| { - build_and_broadcast_shielded( - sdk.clone(), - recipient, - shield_amount, - chain_proof.clone(), - path.clone(), - asset_lock_signer, - &prover, - sender_ovk.clone(), - surplus_output, - dummy_outputs, - s, - ) - }) - .await - .map_err(PlatformWalletError::Sdk)?; - } - Err(e) => return Err(PlatformWalletError::Sdk(e)), - } + out_point.txid + ); + let chain_proof = self + .asset_locks + .upgrade_to_chain_lock_proof(&out_point, CL_FALLBACK_TIMEOUT) + .await?; + let cs = self + .asset_locks + .advance_asset_lock_status( + &out_point, + crate::wallet::asset_lock::tracked::AssetLockStatus::ChainLocked, + Some(chain_proof.clone()), + ) + .await?; + self.asset_locks.queue_asset_lock_changeset(cs); + submit_with_cl_height_retry(settings, |s| { + build_and_broadcast_shielded( + sdk.clone(), + recipient, + shield_amount, + chain_proof.clone(), + path.clone(), + asset_lock_signer, + &prover, + sender_ovk.clone(), + surplus_output, + dummy_outputs, + s, + ) + }) + .await + .map_err(PlatformWalletError::Sdk)? + } + Err(e) => return Err(PlatformWalletError::Sdk(e)), + }; + + // Record a live `ShieldFromAssetLock` activity entry over the + // landed bundle. One entry per call (= one per seed-pool batch), + // `direction in`, amount = the real shielded note value (dummy + // fillers contribute no visible output cmx, so they're excluded + // by construction). Recorded Confirmed directly — `broadcast_and_ + // _wait` already proved inclusion. Best-effort: a recording miss + // (no bound keyset, no recoverable output) just omits the row. + self.record_shield_from_asset_lock_activity(&landed_actions, shield_amount) + .await; // Step 5: cleanup. Consume the tracked asset lock. The // shielded note itself arrives via the next sync — there's @@ -487,6 +503,68 @@ impl PlatformWallet { )) }) } + + /// Record a confirmed `ShieldFromAssetLock` (Type 18) activity entry + /// over the landed bundle's `actions`. + /// + /// Best-effort and non-fatal: the broadcast already succeeded, so a + /// recording miss (no bound shielded keyset, or no wallet-visible + /// output cmx in the bundle) must not turn the funding into a + /// failure — it just omits the activity row (a later scan still + /// surfaces the note via OVK recovery). Finds the keyset whose IVK + /// recognizes the funded note's recipient (the row then lands under + /// that account), falling back to the lowest bound account — mirrors + /// the `sender_ovk` selection above. + #[cfg(feature = "shielded")] + async fn record_shield_from_asset_lock_activity( + &self, + actions: &[dpp::shielded::SerializedAction], + shield_amount: Credits, + ) { + use crate::wallet::shielded::activity::{ + ShieldedActivityKind, ShieldedActivityStatus, ShieldedDirection, + }; + use crate::wallet::shielded::activity_recorder::{build_pending_entry, with_status}; + + let guard = self.shielded_keys.read().await; + let Some(keys_map) = guard.as_ref() else { + return; + }; + // Prefer the account whose keyset recognizes a visible output; + // fall back to the lowest bound account. + let Some((&account, keyset)) = keys_map.iter().next() else { + return; + }; + + let Some(pending) = build_pending_entry( + keyset, + crate::wallet::shielded::activity_recorder::LiveEntryParams { + kind: ShieldedActivityKind::ShieldFromAssetLock, + direction: ShieldedDirection::In, + amount: shield_amount, + // The flat pool fee is charged on the L1 side (asset-lock + // value − shield_amount); the note value is exactly + // `shield_amount`, so no shielded-pool fee is derivable + // from the bundle here. + fee: None, + counterparty: None, + memo: None, + actions, + spent_notes: &[], + }, + ) else { + return; + }; + + let confirmed = with_status(&pending, ShieldedActivityStatus::Confirmed, None); + let id = crate::wallet::shielded::SubwalletId::new(self.wallet_id(), account); + crate::wallet::shielded::operations::queue_shielded_activity( + Some(self.persister()), + self.wallet_id(), + id, + confirmed, + ); + } } /// Look up the asset-lock value in credits. @@ -542,6 +620,12 @@ async fn lookup_asset_lock_value_credits( /// /// Extracted so `submit_with_cl_height_retry`'s closure stays compact /// and the IS→CL fallback path can re-call it with the upgraded proof. +/// +/// On success returns the **landed** bundle's serialized Orchard actions +/// so the orchestrator can record a live `ShieldFromAssetLock` activity +/// entry over the exact bundle that committed (each retry re-randomizes +/// the bundle, so only the landed attempt's cmxs are the ones a later +/// scan will recover). #[allow(clippy::too_many_arguments)] async fn build_and_broadcast_shielded( sdk: std::sync::Arc, @@ -555,11 +639,14 @@ async fn build_and_broadcast_shielded( surplus_output: Option, dummy_outputs: usize, settings: Option, -) -> Result<(), dash_sdk::Error> +) -> Result, dash_sdk::Error> where AS: ::key_wallet::signer::Signer, P: OrchardProver, { + use dpp::state_transition::shield_from_asset_lock_transition::ShieldFromAssetLockTransition; + use dpp::state_transition::StateTransition; + let st = build_shield_from_asset_lock_transition_with_signer( &recipient, shield_amount, @@ -575,6 +662,15 @@ where ) .await?; + // `actions` is a public field on the V0 struct (no accessor trait for + // Shield / ShieldFromAssetLock). + let actions = match &st { + StateTransition::ShieldFromAssetLock(ShieldFromAssetLockTransition::V0(v0)) => { + v0.actions.clone() + } + _ => Vec::new(), + }; + // Wait for proven execution rather than relay-ACK. Single-use // asset-lock proof: a false-positive on a transition Platform // later rejects would strand the L1 outpoint with no in-app @@ -582,7 +678,7 @@ where // confirmation that drive-abci committed. st.broadcast_and_wait::(&sdk, settings) .await?; - Ok(()) + Ok(actions) } /// Pre-flight check for the recipient list. diff --git a/packages/rs-platform-wallet/src/wallet/shielded/mod.rs b/packages/rs-platform-wallet/src/wallet/shielded/mod.rs index a33e19d9178..b9b045f449d 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/mod.rs @@ -31,6 +31,8 @@ //! `BTreeMap` (with the spend authority); //! `PlatformWallet` doesn't need a wrapper struct around it. +pub mod activity; +pub mod activity_recorder; pub mod coordinator; pub mod file_store; pub mod fund_from_asset_lock; @@ -42,6 +44,11 @@ pub mod seed_pool; pub mod store; pub mod sync; +pub use activity::{ + compute_activity_id, derive_activity_from_scan_data, sort_activity_for_display, + ScanDeriveInput, ShieldedActivityEntry, ShieldedActivityKind, ShieldedActivityStatus, + ShieldedDirection, +}; pub use coordinator::NetworkShieldedCoordinator; pub use file_store::{FileBackedShieldedStore, FileShieldedStoreError}; pub use keys::{AccountViewingKeys, OrchardKeySet}; diff --git a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs index ff0a3309a41..0cd124d3ab0 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs @@ -19,6 +19,10 @@ //! - **IdentityCreateFromShieldedPool** (Type 20): shielded pool → a brand-new Platform identity //! funded by a fixed denomination leaving the pool (any excess re-enters as a change note) +use super::activity::{ShieldedActivityKind, ShieldedActivityStatus, ShieldedDirection}; +use super::activity_recorder::{ + build_pending_entry, changeset_for_entry, non_zero_memo, with_status, LiveEntryParams, +}; use super::keys::OrchardKeySet; use super::note_selection::{ select_notes_for_denomination, select_notes_with_fee, ShieldedFeeKind, @@ -164,6 +168,102 @@ fn queue_shielded_changeset( } } +/// Queue a single activity entry to the host persister, for callers +/// outside this module (the Type 18 orchestrator in +/// `fund_from_asset_lock.rs`). Upserts by `entry.id`. +pub(super) fn queue_shielded_activity( + persister: Option<&WalletPersister>, + wallet_id: WalletId, + id: SubwalletId, + entry: super::activity::ShieldedActivityEntry, +) { + queue_shielded_changeset(persister, wallet_id, changeset_for_entry(id, entry)); +} + +/// Extract the serialized Orchard actions from a built shielded +/// `StateTransition`, for live activity-entry recording. +/// +/// Returns `&[]` for any non-shielded variant (none reach the recorder). +/// Each shielded variant's `actions()` comes from its own accessor / +/// methods trait, so the relevant traits are imported locally. +fn shielded_actions(st: &StateTransition) -> &[dpp::shielded::SerializedAction] { + // `actions()` is on the per-type accessor trait for the spend-based + // transitions; Shield / ShieldFromAssetLock expose `actions` as a + // public field on their V0 struct (no accessor trait), so match down + // to the V0 variant for those two. + use dpp::state_transition::shield_from_asset_lock_transition::ShieldFromAssetLockTransition; + use dpp::state_transition::shield_transition::ShieldTransition; + use dpp::state_transition::shielded_transfer_transition::accessors::ShieldedTransferTransitionAccessorsV0; + use dpp::state_transition::shielded_withdrawal_transition::accessors::ShieldedWithdrawalTransitionAccessorsV0; + use dpp::state_transition::state_transitions::shielded::identity_create_from_shielded_pool_transition::accessors::IdentityCreateFromShieldedPoolTransitionAccessorsV0; + use dpp::state_transition::unshield_transition::accessors::UnshieldTransitionAccessorsV0; + + match st { + StateTransition::Shield(ShieldTransition::V0(v0)) => &v0.actions, + StateTransition::ShieldedTransfer(t) => t.actions(), + StateTransition::Unshield(t) => t.actions(), + StateTransition::ShieldFromAssetLock(ShieldFromAssetLockTransition::V0(v0)) => &v0.actions, + StateTransition::ShieldedWithdrawal(t) => t.actions(), + StateTransition::IdentityCreateFromShieldedPool(t) => t.actions(), + _ => &[], + } +} + +/// Record a live activity entry built from `params` for `id`, shipping +/// it to the host persister via [`queue_shielded_changeset`]. Returns +/// the recorded entry so the caller can flip its status later (the +/// status flip re-records by the same `entry.id`, an upsert). +/// +/// Returns `None` (and queues nothing) when the bundle exposes no +/// wallet-visible output cmx — see [`build_pending_entry`]. +fn record_pending_activity( + persister: Option<&WalletPersister>, + wallet_id: WalletId, + id: SubwalletId, + keys: &OrchardKeySet, + params: LiveEntryParams<'_>, +) -> Option { + let kind = params.kind.clone(); + let Some(entry) = build_pending_entry(keys, params) else { + // Should be unreachable for bundles our own builders produced + // (they always carry at least one wallet-visible output) — but a + // silent skip here means the operation leaves no history, so + // make it loud. + warn!( + ?kind, + "live activity entry skipped: no wallet-visible output cmx recovered from the bundle" + ); + return None; + }; + info!( + ?kind, + entry_id = %hex::encode(entry.id), + cmxs = entry.note_cmxs.len(), + "live activity entry recorded (pending)" + ); + queue_shielded_changeset(persister, wallet_id, changeset_for_entry(id, entry.clone())); + Some(entry) +} + +/// Re-record `entry` with a flipped status (and optional confirmed +/// height) for `id`. Upserts by `entry.id` at the persister, so a +/// `Pending` row becomes `Confirmed` / `Failed` in place. No-op when +/// `entry` is `None` (nothing was recorded for this operation). +fn record_activity_status( + persister: Option<&WalletPersister>, + wallet_id: WalletId, + id: SubwalletId, + entry: &Option, + status: ShieldedActivityStatus, + block_height: Option, +) { + let Some(entry) = entry else { + return; + }; + let next = with_status(entry, status, block_height); + queue_shielded_changeset(persister, wallet_id, changeset_for_entry(id, next)); +} + // ------------------------------------------------------------------------- // Shield: platform addresses -> shielded pool (Type 15) // ------------------------------------------------------------------------- @@ -205,6 +305,8 @@ fn reserve_shield_fee_on_input_0( #[allow(clippy::too_many_arguments)] pub async fn shield, P: OrchardProver>( sdk: &Arc, + persister: Option<&WalletPersister>, + wallet_id: WalletId, keys: &OrchardKeySet, account: u32, inputs: BTreeMap, @@ -213,6 +315,7 @@ pub async fn shield, P: OrchardProver>( prover: &P, ) -> Result<(), PlatformWalletError> { let recipient_addr = default_orchard_address(keys)?; + let id = SubwalletId::new(wallet_id, account); // Reserve the flat shielded fee `F` on top of `amount` in the input // claims. Consensus `validate_structure` (rs-dpp) now rejects a Shield @@ -301,6 +404,29 @@ pub async fn shield, P: OrchardProver>( trace!("Shield credits: state transition built, broadcasting..."); let network = sdk.network; + + // Live activity: Shield is `direction in`, amount = the note value + // entering the pool, fee = the flat shielded fee reserved above. The + // visible output cmx is the recipient note (own address, OVK-keyed), + // which the scan later sees as an outgoing note recovered to self — + // the ids line up. + let pending_entry = record_pending_activity( + persister, + wallet_id, + id, + keys, + LiveEntryParams { + kind: ShieldedActivityKind::Shield, + direction: ShieldedDirection::In, + amount, + fee: Some(fee), + counterparty: None, + memo: None, + actions: shielded_actions(&state_transition), + spent_notes: &[], + }, + ); + // Wait for proven execution (not just relay-ACK) so the host only // sees success once Platform has actually included the transition. A // DAPI-level ACK alone could otherwise mask a later Platform @@ -311,7 +437,7 @@ pub async fn shield, P: OrchardProver>( // ambiguous post-broadcast wait failure has no local state to strand. // Its inputs are transparent address claims guarded by on-chain // nonces, and a host-level retry re-fetches those nonces. - state_transition + let broadcast_result = state_transition .broadcast_and_wait::(sdk, None) .await .map_err(|e| { @@ -336,8 +462,30 @@ pub async fn shield, P: OrchardProver>( } else { PlatformWalletError::ShieldedBroadcastFailed(e.to_string()) } - })?; + }); + + if let Err(e) = broadcast_result { + // `broadcast_and_wait` failed: shield holds no note reservation, + // so there's nothing to roll back — just mark the activity Failed. + record_activity_status( + persister, + wallet_id, + id, + &pending_entry, + ShieldedActivityStatus::Failed, + None, + ); + return Err(e); + } + record_activity_status( + persister, + wallet_id, + id, + &pending_entry, + ShieldedActivityStatus::Confirmed, + None, + ); info!(account, credits = amount, "Shield broadcast succeeded"); Ok(()) } @@ -392,6 +540,12 @@ pub async fn unshield( // by `reserve_unspent_notes` — except the ambiguous // `ShieldedSpendUnconfirmed` one, which intentionally leaves it in // place (see the outer match below). + // + // `pending_entry` is recorded once the transition is built (so we + // have its output cmxs) and flipped to Confirmed / Failed in the + // outer match. It lives outside the async block so the flip can see + // it. A build failure leaves it `None` and records nothing. + let mut pending_entry = None; let result = async { let (spends, anchor) = extract_spends_and_anchor(store, &selected_notes).await?; @@ -417,6 +571,26 @@ pub async fn unshield( "builder fee must match the reserved unshield fee" ); + // Live activity: a confirmed Unshield is `direction out`, + // counterparty = the 21-byte serialized PlatformAddress, exact + // fee = the builder's metered fee. + pending_entry = record_pending_activity( + persister, + wallet_id, + id, + keys, + LiveEntryParams { + kind: ShieldedActivityKind::Unshield, + direction: ShieldedDirection::Out, + amount, + fee: Some(fee_used), + counterparty: Some(to_address.to_bytes()), + memo: None, + actions: shielded_actions(&state_transition), + spent_notes: &selected_notes, + }, + ); + trace!("Unshield: state transition built, broadcasting..."); broadcast_shielded_spend(sdk, &state_transition, "unshield").await } @@ -424,6 +598,14 @@ pub async fn unshield( match result { Ok(()) => { + record_activity_status( + persister, + wallet_id, + id, + &pending_entry, + ShieldedActivityStatus::Confirmed, + None, + ); // Broadcast already succeeded; spent-state bookkeeping is // best-effort. Surfacing a local write failure as a send // failure here would invite duplicate retries — the next @@ -459,8 +641,23 @@ pub async fn unshield( // app restart drops the reservation and frees the notes. Releasing // them now would invite re-selecting notes whose nullifiers may // already be consumed on chain. + // + // Activity entry: leave it `Pending`. Like the note reservation, + // the outcome is genuinely unknown here — a later scan that finds + // the spend will flip the row to Confirmed (its id matches), and + // until then surfacing it as Pending is honest. Err(e @ PlatformWalletError::ShieldedSpendUnconfirmed { .. }) => Err(e), Err(e) => { + // Definitive failure: the spend never executed. Mark the + // activity row Failed (upsert by id) before releasing notes. + record_activity_status( + persister, + wallet_id, + id, + &pending_entry, + ShieldedActivityStatus::Failed, + None, + ); cancel_pending(store, id, &selected_notes).await; Err(e) } @@ -504,6 +701,7 @@ pub async fn transfer( "Shielded transfer" ); + let mut pending_entry = None; let result = async { let (spends, anchor) = extract_spends_and_anchor(store, &selected_notes).await?; @@ -529,6 +727,26 @@ pub async fn transfer( "builder fee must match the reserved minimum fee" ); + // Live activity: a confirmed transfer is a `Sent` (direction out), + // counterparty = the recipient's 43-byte raw Orchard address, memo + // attached when non-zero. + pending_entry = record_pending_activity( + persister, + wallet_id, + id, + keys, + LiveEntryParams { + kind: ShieldedActivityKind::Sent, + direction: ShieldedDirection::Out, + amount, + fee: Some(fee_used), + counterparty: Some(to_address.to_raw_address_bytes().to_vec()), + memo: non_zero_memo(&memo), + actions: shielded_actions(&state_transition), + spent_notes: &selected_notes, + }, + ); + trace!("Shielded transfer: state transition built, broadcasting..."); broadcast_shielded_spend(sdk, &state_transition, "transfer").await } @@ -536,6 +754,14 @@ pub async fn transfer( match result { Ok(()) => { + record_activity_status( + persister, + wallet_id, + id, + &pending_entry, + ShieldedActivityStatus::Confirmed, + None, + ); // Best-effort post-broadcast bookkeeping (see unshield). if let Err(e) = finalize_pending(store, persister, wallet_id, id, &selected_notes).await { @@ -554,9 +780,18 @@ pub async fn transfer( Ok(()) } // Ambiguous post-broadcast confirmation failure: leave the - // reservation in place (see `unshield`'s outer match). + // reservation (and the Pending activity row) in place — a later + // scan flips it to Confirmed (see `unshield`'s outer match). Err(e @ PlatformWalletError::ShieldedSpendUnconfirmed { .. }) => Err(e), Err(e) => { + record_activity_status( + persister, + wallet_id, + id, + &pending_entry, + ShieldedActivityStatus::Failed, + None, + ); cancel_pending(store, id, &selected_notes).await; Err(e) } @@ -605,6 +840,12 @@ pub async fn withdraw( "Shielded withdrawal" ); + // Capture the Core output script bytes for the activity counterparty + // (the same bytes that fed `output_script` above) before the builder + // consumes `output_script`. + let counterparty_script = to_address.script_pubkey().to_bytes(); + + let mut pending_entry = None; let result = async { let (spends, anchor) = extract_spends_and_anchor(store, &selected_notes).await?; @@ -633,6 +874,25 @@ pub async fn withdraw( "builder fee must match the reserved withdrawal fee" ); + // Live activity: a Withdrawal (direction out), counterparty = the + // Core output script bytes, exact metered fee. + pending_entry = record_pending_activity( + persister, + wallet_id, + id, + keys, + LiveEntryParams { + kind: ShieldedActivityKind::Withdrawal, + direction: ShieldedDirection::Out, + amount, + fee: Some(fee_used), + counterparty: Some(counterparty_script.clone()), + memo: None, + actions: shielded_actions(&state_transition), + spent_notes: &selected_notes, + }, + ); + trace!("Shielded withdrawal: state transition built, broadcasting..."); broadcast_shielded_spend(sdk, &state_transition, "withdraw").await } @@ -640,6 +900,14 @@ pub async fn withdraw( match result { Ok(()) => { + record_activity_status( + persister, + wallet_id, + id, + &pending_entry, + ShieldedActivityStatus::Confirmed, + None, + ); // Best-effort post-broadcast bookkeeping (see unshield). if let Err(e) = finalize_pending(store, persister, wallet_id, id, &selected_notes).await { @@ -658,9 +926,18 @@ pub async fn withdraw( Ok(()) } // Ambiguous post-broadcast confirmation failure: leave the - // reservation in place (see `unshield`'s outer match). + // reservation (and the Pending activity row) in place — a later + // scan flips it to Confirmed (see `unshield`'s outer match). Err(e @ PlatformWalletError::ShieldedSpendUnconfirmed { .. }) => Err(e), Err(e) => { + record_activity_status( + persister, + wallet_id, + id, + &pending_entry, + ShieldedActivityStatus::Failed, + None, + ); cancel_pending(store, id, &selected_notes).await; Err(e) } @@ -747,6 +1024,11 @@ where // From here on every error path must release the reservation taken above — except the // ambiguous `ShieldedBroadcastUnconfirmed` one, which intentionally leaves it in place // (see the outer match below). + // + // `pending_entry` is recorded once the bundle is built (so we know the + // identity id + output cmxs) and flipped to Confirmed / Failed in the + // outer match; it lives here so the flip can see it. + let mut pending_entry = None; let result = async { let (spends, anchor) = extract_spends_and_anchor(store, &selected_notes).await?; @@ -769,6 +1051,31 @@ where let identity_id = build.identity_id; + // Live activity: IdentityCreate carries the created identity id. + // The display amount is the denomination LESS the metered fee (the + // credits the new identity is created holding); the exact fee is the + // predicted fee reserved above (the builder meters the same value). + // The output cmxs are taken from the bundle the transition is built + // from (the change note re-entering the pool, if any). + pending_entry = record_pending_activity( + persister, + wallet_id, + id, + keys, + LiveEntryParams { + kind: ShieldedActivityKind::IdentityCreate { + identity_id: identity_id.to_buffer(), + }, + direction: ShieldedDirection::Out, + amount: denomination.saturating_sub(predicted_fee), + fee: Some(predicted_fee), + counterparty: Some(identity_id.to_buffer().to_vec()), + memo: None, + actions: &build.bundle.actions, + spent_notes: &selected_notes, + }, + ); + trace!("IdentityCreateFromShieldedPool: built, broadcasting via SDK..."); // Stage the broadcast and the result-wait SEPARATELY (instead of one `broadcast_and_wait`) // so the two failure shapes can be told apart: @@ -927,6 +1234,14 @@ where match result { Ok((identity_id, identity)) => { + record_activity_status( + persister, + wallet_id, + id, + &pending_entry, + ShieldedActivityStatus::Confirmed, + None, + ); // Best-effort post-broadcast bookkeeping (see `unshield`): mark the spent notes so the // local balance reflects the exit immediately; any drift heals on the next nullifier // sync. The on-chain nullifier set — not this local mark — is the authoritative @@ -955,10 +1270,21 @@ where // actually executed, the next sync promotes these notes to spent; if it truly never landed, // an app restart drops the in-memory reservation and frees them. Releasing them now would // invite double-spend attempts against notes that may already be consumed on chain — the - // very hazard this variant exists to prevent. + // very hazard this variant exists to prevent. The activity row likewise stays `Pending` + // (a later scan that finds the identity's change note flips it to Confirmed). Err(e @ PlatformWalletError::ShieldedBroadcastUnconfirmed { .. }) => Err(e), Err(e) => { if error_releases_note_reservation(&e) { + // Definitive failure: the identity was never created. Mark + // the activity row Failed (upsert by id) before releasing. + record_activity_status( + persister, + wallet_id, + id, + &pending_entry, + ShieldedActivityStatus::Failed, + None, + ); cancel_pending(store, id, &selected_notes).await; } Err(e) diff --git a/packages/rs-platform-wallet/src/wallet/shielded/store.rs b/packages/rs-platform-wallet/src/wallet/shielded/store.rs index 1ef0687bb34..4d6595d63e0 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/store.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/store.rs @@ -186,6 +186,46 @@ pub trait ShieldedStore: Send + Sync { fn get_outgoing_notes(&self, id: SubwalletId) -> Result, Self::Error>; + // ── Derived activity log (per-subwallet) ─────────────────────────── + + /// Upsert a derived [`ShieldedActivityEntry`] for `id`, keyed by + /// `entry.id` (the sha256 of the visible output cmxs — see + /// [`crate::wallet::shielded::activity`]). + /// + /// Re-saving an entry with the same `entry.id` overwrites the + /// existing one in place. This is what lets a coarse scan-derived + /// `ShieldedSpend` be upgraded to a specific kind when a later live + /// entry (or correlation pass) re-emits the same id, and what lets a + /// `Pending` entry flip to `Confirmed`/`Failed`. + fn save_activity( + &mut self, + id: SubwalletId, + entry: &super::activity::ShieldedActivityEntry, + ) -> Result<(), Self::Error>; + + /// Return a page of derived activity for `id`, sorted for display + /// (pendings first, then by `block_height` desc, tiebreak by + /// `created_at_ms` then `id`), sliced by `[offset, offset+limit)`. + /// `limit == 0` returns an empty page. + fn get_activity( + &self, + id: SubwalletId, + offset: usize, + limit: usize, + ) -> Result, Self::Error>; + + /// Look up a single activity entry by its `entry.id`. `None` if no + /// entry with that id exists for `id`. + fn get_activity_by_entry_id( + &self, + id: SubwalletId, + entry_id: &[u8; 32], + ) -> Result, Self::Error>; + + /// Return the set of all entry ids already recorded for `id`. Used by + /// the scan deriver to skip clusters a live entry already owns. + fn get_activity_ids(&self, id: SubwalletId) -> Result, Self::Error>; + // ── Commitment tree (network-shared) ─────────────────────────────── /// Append a note commitment to the shared tree. @@ -309,6 +349,10 @@ pub(super) struct SubwalletState { /// `cmx` set of recorded outgoing notes, for O(log n) idempotency /// on `record_outgoing_note` (Orchard cmx is globally unique). pub outgoing_cmx_index: BTreeSet<[u8; 32]>, + /// Derived activity entries keyed by `entry.id` (sha256 of the + /// visible output cmxs). Upsert-by-id: a later live entry or a + /// correlation pass that re-emits the same id overwrites the row. + pub activity: BTreeMap<[u8; 32], super::activity::ShieldedActivityEntry>, } impl SubwalletState { @@ -382,6 +426,37 @@ impl SubwalletState { pub(super) fn outgoing_notes(&self) -> Vec { self.outgoing_notes.clone() } + + /// Upsert a derived activity entry by `entry.id`. + pub(super) fn save_activity(&mut self, entry: &super::activity::ShieldedActivityEntry) { + self.activity.insert(entry.id, entry.clone()); + } + + /// Display-sorted page of activity entries. + pub(super) fn activity_page( + &self, + offset: usize, + limit: usize, + ) -> Vec { + if limit == 0 { + return Vec::new(); + } + let mut all: Vec = + self.activity.values().cloned().collect(); + super::activity::sort_activity_for_display(&mut all); + all.into_iter().skip(offset).take(limit).collect() + } + + pub(super) fn activity_by_id( + &self, + entry_id: &[u8; 32], + ) -> Option { + self.activity.get(entry_id).cloned() + } + + pub(super) fn activity_ids(&self) -> BTreeSet<[u8; 32]> { + self.activity.keys().copied().collect() + } } // ── InMemoryShieldedStore ────────────────────────────────────────────── @@ -499,6 +574,47 @@ impl ShieldedStore for InMemoryShieldedStore { .unwrap_or_default()) } + fn save_activity( + &mut self, + id: SubwalletId, + entry: &super::activity::ShieldedActivityEntry, + ) -> Result<(), Self::Error> { + self.subwallets.entry(id).or_default().save_activity(entry); + Ok(()) + } + + fn get_activity( + &self, + id: SubwalletId, + offset: usize, + limit: usize, + ) -> Result, Self::Error> { + Ok(self + .subwallets + .get(&id) + .map(|sw| sw.activity_page(offset, limit)) + .unwrap_or_default()) + } + + fn get_activity_by_entry_id( + &self, + id: SubwalletId, + entry_id: &[u8; 32], + ) -> Result, Self::Error> { + Ok(self + .subwallets + .get(&id) + .and_then(|sw| sw.activity_by_id(entry_id))) + } + + fn get_activity_ids(&self, id: SubwalletId) -> Result, Self::Error> { + Ok(self + .subwallets + .get(&id) + .map(SubwalletState::activity_ids) + .unwrap_or_default()) + } + fn append_commitment(&mut self, cmx: &[u8; 32], marked: bool) -> Result<(), Self::Error> { self.commitments.push(*cmx); self.marked_positions.push(marked); @@ -647,6 +763,86 @@ mod tests { assert_eq!(store.tree_anchor().unwrap(), [0u8; 32]); } + #[test] + fn test_save_activity_upserts_and_paginates() { + use super::super::activity::{ + ShieldedActivityEntry, ShieldedActivityKind, ShieldedActivityStatus, ShieldedDirection, + }; + + fn entry( + id: u8, + height: Option, + status: ShieldedActivityStatus, + ) -> ShieldedActivityEntry { + ShieldedActivityEntry { + id: [id; 32], + kind: ShieldedActivityKind::Sent, + direction: ShieldedDirection::Out, + amount: 1, + fee: None, + counterparty: None, + memo: None, + block_height: height, + status, + created_at_ms: 0, + note_cmxs: vec![[id; 32]], + spent_nullifiers: vec![], + } + } + + let mut store = InMemoryShieldedStore::new(); + let id = test_id(0); + + // Two confirmed at different heights + one pending. + store + .save_activity(id, &entry(1, Some(10), ShieldedActivityStatus::Confirmed)) + .unwrap(); + store + .save_activity(id, &entry(2, Some(20), ShieldedActivityStatus::Confirmed)) + .unwrap(); + store + .save_activity(id, &entry(3, None, ShieldedActivityStatus::Pending)) + .unwrap(); + + // Display order: pending first, then height desc. + let page = store.get_activity(id, 0, 10).unwrap(); + assert_eq!(page.len(), 3); + assert_eq!(page[0].id, [3u8; 32], "pending floats to top"); + assert_eq!(page[1].id, [2u8; 32], "height 20 before 10"); + assert_eq!(page[2].id, [1u8; 32]); + + // Upsert by id: re-saving entry 1 as Pending replaces it in place + // (still 3 entries, but now 1 is pending and floats up). + store + .save_activity(id, &entry(1, None, ShieldedActivityStatus::Pending)) + .unwrap(); + let page = store.get_activity(id, 0, 10).unwrap(); + assert_eq!(page.len(), 3, "upsert by id, not append"); + assert_eq!( + store.get_activity_ids(id).unwrap().len(), + 3, + "still exactly three distinct ids" + ); + + // Pagination: offset/limit slice the display-sorted list. + let first_two = store.get_activity(id, 0, 2).unwrap(); + assert_eq!(first_two.len(), 2); + let last_one = store.get_activity(id, 2, 10).unwrap(); + assert_eq!(last_one.len(), 1); + // limit 0 => empty page. + assert!(store.get_activity(id, 0, 0).unwrap().is_empty()); + + // Lookup by entry id. + assert!(store + .get_activity_by_entry_id(id, &[2u8; 32]) + .unwrap() + .is_some()); + assert!(store + .get_activity_by_entry_id(id, &[9u8; 32]) + .unwrap() + .is_none()); + } + #[test] fn test_reset_commitment_tree_empties_and_reappends_from_zero() { let mut store = InMemoryShieldedStore::new(); diff --git a/packages/rs-platform-wallet/src/wallet/shielded/sync/ovk_builder_roundtrip_tests.rs b/packages/rs-platform-wallet/src/wallet/shielded/sync/ovk_builder_roundtrip_tests.rs index 35361eb81df..a94d4e3e376 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/sync/ovk_builder_roundtrip_tests.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/sync/ovk_builder_roundtrip_tests.rs @@ -194,3 +194,67 @@ async fn shield_built_note_ovk_recovers_and_persists_as_outgoing() { "a foreign OVK must not recover the wallet's send" ); } + +/// The live activity recorder must recover at least one visible output +/// cmx from a real built bundle — the same recovery the scan runs. A +/// `None` here means every live entry silently vanishes (the exact +/// failure mode debugged on devnet 2026-06-12). +#[tokio::test] +async fn live_recorder_builds_entry_from_real_shield_bundle() { + use crate::wallet::shielded::activity::{ShieldedActivityKind, ShieldedDirection}; + use crate::wallet::shielded::activity_recorder::{ + build_pending_entry, visible_output_cmxs, LiveEntryParams, + }; + + let seed = [0x42u8; 32]; + let keys = OrchardKeySet::from_seed(&seed, Network::Testnet, 0) + .expect("ZIP-32 derivation from a fixed seed should succeed"); + let recipient = OrchardAddress::from_raw_bytes(&keys.default_address.to_raw_address_bytes()) + .expect("default address must convert to OrchardAddress"); + + let mut inputs = BTreeMap::new(); + inputs.insert( + PlatformAddress::P2pkh([0xAB; 20]), + (0u32, 500_000_000_000u64), + ); + let prover = CachedOrchardProver::new(); + let st = build_shield_transition( + &recipient, + 200_000_000_000, + inputs, + vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], + &DummySigner, + 0, + &&prover, + [0u8; 36], + Some(keys.outgoing_viewing_key.clone()), + PlatformVersion::latest(), + ) + .await + .expect("shield transition build should succeed"); + + let StateTransition::Shield(ShieldTransition::V0(v0)) = st else { + panic!("expected a Shield state transition"); + }; + + let cmxs = visible_output_cmxs(&v0.actions, &keys); + assert!( + !cmxs.is_empty(), + "recorder must recover the wallet-visible output cmx from a real bundle" + ); + + let entry = build_pending_entry( + &keys, + LiveEntryParams { + kind: ShieldedActivityKind::Shield, + direction: ShieldedDirection::In, + amount: 200_000_000_000, + fee: None, + counterparty: None, + memo: None, + actions: &v0.actions, + spent_notes: &[], + }, + ); + assert!(entry.is_some(), "live entry must build from a real bundle"); +} diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/Persistence/DashModelContainer.swift b/packages/swift-sdk/Sources/SwiftDashSDK/Persistence/DashModelContainer.swift index 640c18e0446..c135e1c6a57 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/Persistence/DashModelContainer.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/Persistence/DashModelContainer.swift @@ -32,6 +32,7 @@ public enum DashModelContainer { PersistentShieldedNote.self, PersistentShieldedOutgoingNote.self, PersistentShieldedSyncState.self, + PersistentShieldedActivity.self, PersistentAssetLock.self ] } diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentShieldedActivity.swift b/packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentShieldedActivity.swift new file mode 100644 index 00000000000..16926f9c4c2 --- /dev/null +++ b/packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentShieldedActivity.swift @@ -0,0 +1,124 @@ +import Foundation +import SwiftData + +/// SwiftData row for one derived shielded-activity entry — a user-facing +/// record of a shielded operation (shield, send, unshield, withdrawal, +/// identity-create, …). +/// +/// Mirrors `platform_wallet::wallet::shielded::ShieldedActivityEntry` +/// from the Rust side. Written two ways, both keyed by `entryId`: +/// - **live** — each shielded operation records a rich entry at execution +/// time (exact kind, amount, fee, counterparty, memo, identity id); +/// - **scan-derived** — on a restored wallet the sync pass reconstructs +/// entries best-effort from persisted notes / outgoing notes. +/// +/// The persister callback upserts by `entryId`, so a `Pending` row flips +/// to `Confirmed`/`Failed` in place, and a coarse scan-derived +/// `ShieldedSpend` can be refined to a specific kind when a richer entry +/// re-emits the same id (the id = sha256 of the visible output cmxs is +/// identical across both paths by construction). +/// +/// On cold start the matching `loadShieldedActivity` callback streams +/// every row back to Rust so the scan deriver's dedupe set includes it +/// and a re-derivation never clobbers a rich live entry. +@Model +public final class PersistentShieldedActivity { + /// Composite uniqueness on `(walletId, accountIndex, entryId)` — at + /// most one activity row per operation. `entryId` alone is the + /// natural key (it's a content hash), but scoping by subwallet + /// matches how the row is addressed on both persist and load paths. + #Unique([\.walletId, \.accountIndex, \.entryId]) + /// Index `(walletId, accountIndex)` so per-subwallet history scans + /// hit an index instead of the full table. + #Index([\.walletId, \.accountIndex]) + + /// 32-byte wallet identifier (matches `PersistentWallet.walletId`). + public var walletId: Data + /// ZIP-32 account index inside the wallet. + public var accountIndex: UInt32 + /// Entry id (sha256 of sorted visible output cmxs, 32 bytes). Natural key. + public var entryId: Data + + /// Kind discriminant (`ShieldedActivityKind::tag`): 0 Shield, + /// 1 ShieldFromAssetLock, 2 Received, 3 Sent, 4 Unshield, + /// 5 Withdrawal, 6 IdentityCreate, 7 ShieldedSpend. + public var kindTag: Int + /// Direction: 0 In, 1 Out, 2 Self. + public var direction: Int + /// Status: 0 Pending, 1 Confirmed, 2 Failed. + public var status: Int + + /// Display amount in credits (principal; excludes self-change / + /// zero-value fillers). + public var amount: UInt64 + /// Exact fee in credits when `hasFee == true`; otherwise unknown. + public var fee: UInt64 + public var hasFee: Bool + /// Block height when `hasBlockHeight == true` (confirmed); otherwise + /// pending. Canonical sort key (desc) with pendings floated up. + public var blockHeight: UInt64 + public var hasBlockHeight: Bool + /// Record time in ms since the Unix epoch (display-only / sort + /// tiebreak). + public var createdAtMs: UInt64 + + /// Created identity id (32 bytes) when `kindTag == 6` + /// (IdentityCreate); empty otherwise. + public var identityId: Data + /// Counterparty bytes (43B Orchard / 21B PlatformAddress / Core + /// script) when present; empty otherwise. + public var counterparty: Data + /// 36-byte memo when present and non-zero; empty otherwise. + public var memo: Data + /// Visible output cmxs that fed `entryId` (`count` × 32 bytes, + /// concatenated). Linkage for confirmation / dedupe. + public var noteCmxs: Data + /// Spent nullifiers (`count` × 32 bytes, concatenated). Empty for + /// inbound kinds. + public var spentNullifiers: Data + + /// Insertion timestamps. + public var createdAt: Date + public var lastUpdated: Date + + public init( + walletId: Data, + accountIndex: UInt32, + entryId: Data, + kindTag: Int, + direction: Int, + status: Int, + amount: UInt64, + fee: UInt64, + hasFee: Bool, + blockHeight: UInt64, + hasBlockHeight: Bool, + createdAtMs: UInt64, + identityId: Data, + counterparty: Data, + memo: Data, + noteCmxs: Data, + spentNullifiers: Data + ) { + self.walletId = walletId + self.accountIndex = accountIndex + self.entryId = entryId + self.kindTag = kindTag + self.direction = direction + self.status = status + self.amount = amount + self.fee = fee + self.hasFee = hasFee + self.blockHeight = blockHeight + self.hasBlockHeight = hasBlockHeight + self.createdAtMs = createdAtMs + self.identityId = identityId + self.counterparty = counterparty + self.memo = memo + self.noteCmxs = noteCmxs + self.spentNullifiers = spentNullifiers + let now = Date() + self.createdAt = now + self.lastUpdated = now + } +} diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift index d551549dcfb..ef7ffd91919 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift @@ -1001,12 +1001,15 @@ public class PlatformWalletPersistenceHandler { cb.on_persist_shielded_nullifiers_spent_fn = persistShieldedNullifiersSpentCallback cb.on_persist_shielded_outgoing_notes_fn = persistShieldedOutgoingNotesCallback cb.on_persist_shielded_synced_indices_fn = persistShieldedSyncedIndicesCallback + cb.on_persist_shielded_activity_fn = persistShieldedActivityCallback cb.on_load_shielded_notes_fn = loadShieldedNotesCallback cb.on_load_shielded_notes_free_fn = loadShieldedNotesFreeCallback cb.on_load_shielded_outgoing_notes_fn = loadShieldedOutgoingNotesCallback cb.on_load_shielded_outgoing_notes_free_fn = loadShieldedOutgoingNotesFreeCallback cb.on_load_shielded_sync_states_fn = loadShieldedSyncStatesCallback cb.on_load_shielded_sync_states_free_fn = loadShieldedSyncStatesFreeCallback + cb.on_load_shielded_activity_fn = loadShieldedActivityCallback + cb.on_load_shielded_activity_free_fn = loadShieldedActivityFreeCallback cb.on_persist_asset_locks_fn = persistAssetLocksCallback cb.on_get_core_tx_record_fn = getCoreTxRecordCallback cb.on_get_core_tx_record_free_fn = getCoreTxRecordFreeCallback @@ -2388,6 +2391,90 @@ public class PlatformWalletPersistenceHandler { } } + /// One derived activity-log row from + /// `ShieldedChangeSet::activity_entries`. Decoupled from + /// `ShieldedActivityFFI` so the trampoline copies the pointer-backed + /// fields out before this runs on `onQueue` (the Rust pointers are + /// only valid for the callback window). + struct ShieldedActivitySnapshot { + let walletId: Data + let accountIndex: UInt32 + let entryId: Data + let kindTag: Int + let direction: Int + let status: Int + let amount: UInt64 + let fee: UInt64 + let hasFee: Bool + let blockHeight: UInt64 + let hasBlockHeight: Bool + let createdAtMs: UInt64 + let identityId: Data + let counterparty: Data + let memo: Data + let noteCmxs: Data + let spentNullifiers: Data + } + + /// Upsert a batch of derived activity entries by + /// `(walletId, accountIndex, entryId)`. Re-persisting the same + /// `entryId` refines the existing row in place: a `Pending` row flips + /// to `Confirmed`/`Failed`, and a scan-derived `ShieldedSpend` can be + /// upgraded to a richer kind (the Rust side re-emits the same id). + func persistShieldedActivity(walletId: Data, snapshots: [ShieldedActivitySnapshot]) { + onQueue { + for snap in snapshots { + let wid = snap.walletId + let acct = snap.accountIndex + let eid = snap.entryId + let predicate = #Predicate { + $0.walletId == wid && $0.accountIndex == acct && $0.entryId == eid + } + var descriptor = FetchDescriptor(predicate: predicate) + descriptor.fetchLimit = 1 + if let existing = try? backgroundContext.fetch(descriptor).first { + existing.kindTag = snap.kindTag + existing.direction = snap.direction + existing.status = snap.status + existing.amount = snap.amount + existing.fee = snap.fee + existing.hasFee = snap.hasFee + existing.blockHeight = snap.blockHeight + existing.hasBlockHeight = snap.hasBlockHeight + existing.createdAtMs = snap.createdAtMs + existing.identityId = snap.identityId + existing.counterparty = snap.counterparty + existing.memo = snap.memo + existing.noteCmxs = snap.noteCmxs + existing.spentNullifiers = snap.spentNullifiers + existing.lastUpdated = Date() + } else { + let row = PersistentShieldedActivity( + walletId: snap.walletId, + accountIndex: snap.accountIndex, + entryId: snap.entryId, + kindTag: snap.kindTag, + direction: snap.direction, + status: snap.status, + amount: snap.amount, + fee: snap.fee, + hasFee: snap.hasFee, + blockHeight: snap.blockHeight, + hasBlockHeight: snap.hasBlockHeight, + createdAtMs: snap.createdAtMs, + identityId: snap.identityId, + counterparty: snap.counterparty, + memo: snap.memo, + noteCmxs: snap.noteCmxs, + spentNullifiers: snap.spentNullifiers + ) + backgroundContext.insert(row) + } + } + if !self.inChangeset { try? backgroundContext.save() } + } + } + /// Mark notes as spent by nullifier. func persistShieldedNullifiersSpent( walletId: Data, @@ -2683,6 +2770,115 @@ public class PlatformWalletPersistenceHandler { } } + /// Build the host-allocated `ShieldedActivityRestoreFFI` array Rust + /// reads at boot so the scan deriver's dedupe set includes every + /// persisted entry (a rich live entry is never clobbered by a + /// re-derivation). Same allocation / `written`-counter discipline as + /// `loadShieldedOutgoingNotes`; each row's four pointer-backed fields + /// (counterparty / memo / cmx + nullifier arrays) reference per-row + /// byte buffers tracked in the allocation. + func loadShieldedActivity() -> ( + entries: UnsafePointer?, + count: Int, + errored: Bool + ) { + var resultEntries: UnsafePointer? + var resultCount: Int = 0 + var resultErrored = false + onQueue { + let descriptor = FetchDescriptor() + var rows: [PersistentShieldedActivity] + do { + rows = try backgroundContext.fetch(descriptor) + } catch { + resultErrored = true + return + } + if let inNetworkIds = self.inNetworkWalletIds() { + rows = rows.filter { inNetworkIds.contains($0.walletId) } + } + if rows.isEmpty { + return + } + let allocation = ShieldedActivityLoadAllocation() + let buf = UnsafeMutablePointer.allocate( + capacity: rows.count + ) + allocation.entries = buf + allocation.entriesCount = rows.count + var written = 0 + for row in rows { + guard row.walletId.count == 32, row.entryId.count == 32 else { continue } + + // Per-row variable-length buffers: counterparty, memo, + // noteCmxs, spentNullifiers. Allocate each and retain a + // pointer in `scalarBuffers` so `release()` frees them. + func makeBuffer(_ data: Data) -> (UnsafeMutablePointer, Int) { + let n = data.count + let p = UnsafeMutablePointer.allocate(capacity: max(n, 1)) + if n > 0 { data.copyBytes(to: p, count: n) } + allocation.scalarBuffers.append((p, n)) + return (p, n) + } + let (cpPtr, cpLen) = makeBuffer(row.counterparty) + let (memoPtr, memoLen) = makeBuffer(row.memo) + let (cmxPtr, cmxLen) = makeBuffer(row.noteCmxs) + let (nfPtr, nfLen) = makeBuffer(row.spentNullifiers) + + var walletIdTuple: FFIByteTuple32 = (0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0) + copyBytes(row.walletId, into: &walletIdTuple) + var entryIdTuple: FFIByteTuple32 = (0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0) + copyBytes(row.entryId, into: &entryIdTuple) + var identityTuple: FFIByteTuple32 = (0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0) + if row.identityId.count == 32 { + copyBytes(row.identityId, into: &identityTuple) + } + buf[written] = ShieldedActivityRestoreFFI( + wallet_id: walletIdTuple, + account_index: row.accountIndex, + entry_id: entryIdTuple, + kind_tag: UInt8(truncatingIfNeeded: row.kindTag), + direction: UInt8(truncatingIfNeeded: row.direction), + status: UInt8(truncatingIfNeeded: row.status), + amount: row.amount, + fee: row.fee, + has_fee: row.hasFee ? 1 : 0, + block_height: row.blockHeight, + has_block_height: row.hasBlockHeight ? 1 : 0, + created_at_ms: row.createdAtMs, + identity_id: identityTuple, + has_identity_id: row.identityId.count == 32 ? 1 : 0, + counterparty_ptr: cpLen > 0 ? UnsafePointer(cpPtr) : nil, + counterparty_len: UInt(cpLen), + memo_ptr: memoLen > 0 ? UnsafePointer(memoPtr) : nil, + memo_len: UInt(memoLen), + note_cmxs_ptr: cmxLen > 0 ? UnsafePointer(cmxPtr) : nil, + note_cmxs_count: UInt(cmxLen / 32), + spent_nullifiers_ptr: nfLen > 0 ? UnsafePointer(nfPtr) : nil, + spent_nullifiers_count: UInt(nfLen / 32) + ) + written += 1 + allocation.entriesInitialized = written + } + let entriesPtr = UnsafePointer(buf) + shieldedActivityLoadAllocations[UnsafeRawPointer(entriesPtr)] = allocation + resultEntries = entriesPtr + resultCount = written + } + return (resultEntries, resultCount, resultErrored) + } + + func loadShieldedActivityFree(entries: UnsafeRawPointer?) { + onQueue { + guard let entries = entries, + let allocation = shieldedActivityLoadAllocations.removeValue(forKey: entries) + else { + return + } + allocation.release() + } + } + /// Build the host-allocated `ShieldedSubwalletSyncStateFFI` /// array Rust reads at boot. Same allocation pattern as /// `loadShieldedNotes`. @@ -2764,6 +2960,8 @@ public class PlatformWalletPersistenceHandler { [UnsafeRawPointer: ShieldedOutgoingNoteLoadAllocation] = [:] private var shieldedSyncStateLoadAllocations: [UnsafeRawPointer: ShieldedSyncStateLoadAllocation] = [:] + private var shieldedActivityLoadAllocations: + [UnsafeRawPointer: ShieldedActivityLoadAllocation] = [:] /// Set network, group id + birth height on the `PersistentWallet` /// row. Fires once at wallet registration with values the Rust side @@ -4534,6 +4732,29 @@ private final class ShieldedSyncStateLoadAllocation { } } +/// Allocation tracker for `loadShieldedActivity` — the entries buffer +/// plus per-row byte buffers for the four pointer-backed fields +/// (counterparty / memo / note-cmx array / spent-nullifier array). Each +/// entry's `*_ptr` references one of `scalarBuffers`. +private final class ShieldedActivityLoadAllocation { + var entries: UnsafeMutablePointer? + var entriesCount: Int = 0 + var entriesInitialized: Int = 0 + var scalarBuffers: [(UnsafeMutablePointer, Int)] = [] + + func release() { + if let entries = entries { + if entriesInitialized > 0 { + entries.deinitialize(count: entriesInitialized) + } + entries.deallocate() + } + for (ptr, _) in scalarBuffers { + ptr.deallocate() + } + } +} + /// Copy bytes from `src` into a fixed-size C-tuple field. Swift /// imports `u8[N]` as an N-tuple — identical memory layout, so /// `withUnsafeMutableBytes` gives us a contiguous write window of @@ -5433,6 +5654,59 @@ private func persistShieldedOutgoingNotesCallback( return 0 } +private func persistShieldedActivityCallback( + context: UnsafeMutableRawPointer?, + walletIdPtr: UnsafePointer?, + entriesPtr: UnsafePointer?, + count: UInt +) -> Int32 { + guard let context = context, let walletIdPtr = walletIdPtr else { return 0 } + let handler = Unmanaged + .fromOpaque(context) + .takeUnretainedValue() + let walletId = Data(bytes: walletIdPtr, count: 32) + + var snapshots: [PlatformWalletPersistenceHandler.ShieldedActivitySnapshot] = [] + if count > 0, let entriesPtr = entriesPtr { + snapshots.reserveCapacity(Int(count)) + for i in 0..?, _ len: UInt) -> Data { + if let ptr = ptr, len > 0 { return Data(bytes: ptr, count: Int(len)) } + return Data() + } + let counterparty = data(e.counterparty_ptr, UInt(e.counterparty_len)) + let memo = data(e.memo_ptr, UInt(e.memo_len)) + let noteCmxs = data(e.note_cmxs_ptr, UInt(e.note_cmxs_count) * 32) + let spentNullifiers = data(e.spent_nullifiers_ptr, UInt(e.spent_nullifiers_count) * 32) + let identityId = e.has_identity_id != 0 ? dataFromTuple32(e.identity_id) : Data() + snapshots.append(.init( + walletId: dataFromTuple32(e.wallet_id), + accountIndex: e.account_index, + entryId: dataFromTuple32(e.entry_id), + kindTag: Int(e.kind_tag), + direction: Int(e.direction), + status: Int(e.status), + amount: e.amount, + fee: e.fee, + hasFee: e.has_fee != 0, + blockHeight: e.block_height, + hasBlockHeight: e.has_block_height != 0, + createdAtMs: e.created_at_ms, + identityId: identityId, + counterparty: counterparty, + memo: memo, + noteCmxs: noteCmxs, + spentNullifiers: spentNullifiers + )) + } + } + handler.persistShieldedActivity(walletId: walletId, snapshots: snapshots) + return 0 +} + private func persistShieldedSyncedIndicesCallback( context: UnsafeMutableRawPointer?, walletIdPtr: UnsafePointer?, @@ -5519,6 +5793,35 @@ private func loadShieldedOutgoingNotesFreeCallback( handler.loadShieldedOutgoingNotesFree(entries: entries.map(UnsafeRawPointer.init)) } +private func loadShieldedActivityCallback( + context: UnsafeMutableRawPointer?, + outEntries: UnsafeMutablePointer?>?, + outCount: UnsafeMutablePointer? +) -> Int32 { + guard let context = context, let outEntries = outEntries, let outCount = outCount else { + return 1 + } + let handler = Unmanaged + .fromOpaque(context) + .takeUnretainedValue() + let (entries, count, errored) = handler.loadShieldedActivity() + outEntries.pointee = entries + outCount.pointee = UInt(count) + return errored ? 1 : 0 +} + +private func loadShieldedActivityFreeCallback( + context: UnsafeMutableRawPointer?, + entries: UnsafePointer?, + _ count: UInt +) { + guard let context = context else { return } + let handler = Unmanaged + .fromOpaque(context) + .takeUnretainedValue() + handler.loadShieldedActivityFree(entries: entries.map(UnsafeRawPointer.init)) +} + private func loadShieldedSyncStatesCallback( context: UnsafeMutableRawPointer?, outEntries: UnsafeMutablePointer?>?, diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/ShieldedActivityView.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/ShieldedActivityView.swift new file mode 100644 index 00000000000..60a55525329 --- /dev/null +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/ShieldedActivityView.swift @@ -0,0 +1,311 @@ +import SwiftUI +import SwiftData +import SwiftDashSDK + +/// Value-based navigation route to the shielded activity list, mirroring +/// `TransactionsRoute`. Pushed via `NavigationLink(value:)` and resolved +/// by a `.navigationDestination(for:)` in `WalletsContentView`. +struct ShieldedActivityRoute: Hashable { + let walletId: Data +} + +// MARK: - Presentation model + +/// Display metadata for a `PersistentShieldedActivity.kindTag`. Kept in +/// one place so the list row, the detail sheet, and the section grouping +/// stay consistent. +enum ShieldedActivityKindDisplay { + static func label(_ tag: Int) -> String { + switch tag { + case 0: return "Shielded" + case 1: return "Shielded from Asset Lock" + case 2: return "Received" + case 3: return "Sent" + case 4: return "Unshielded" + case 5: return "Withdrawn" + case 6: return "Identity Created" + default: return "Shielded Spend" + } + } + + /// SF Symbol per kind. + static func icon(_ tag: Int) -> String { + switch tag { + case 0, 1: return "lock.fill" // Shield / ShieldFromAssetLock + case 2: return "arrow.down.circle.fill" // Received + case 3: return "arrow.up.circle.fill" // Sent + case 4: return "lock.open.fill" // Unshield + case 5: return "arrow.up.right.circle.fill" // Withdrawal + case 6: return "person.crop.circle.badge.plus" // IdentityCreate + default: return "questionmark.circle.fill" // ShieldedSpend + } + } + + /// `true` for the residual `ShieldedSpend` kind, which a restored + /// wallet couldn't fully classify — the detail sheet shows a footnote. + static func isUnclassifiedResidual(_ tag: Int) -> Bool { tag == 7 } +} + +extension PersistentShieldedActivity { + /// 1 DASH = 1e11 Platform credits (distinct from Core's 1e8/DASH). + fileprivate static let creditsPerDash = 100_000_000_000.0 + + /// Signed amount string, e.g. `-0.5000 DASH` for an out, `+1.0000` + /// for an in. `direction`: 0 In, 1 Out, 2 Self. + var signedAmountText: String { + let dash = Double(amount) / Self.creditsPerDash + let sign: String + switch direction { + case 0: sign = "+" + case 1: sign = "-" + default: sign = "" + } + return String(format: "%@%.4f DASH", sign, dash) + } + + /// Tint for the amount label by direction. + var amountColor: Color { + switch direction { + case 0: return .green + case 1: return .primary + default: return .secondary + } + } + + /// Decode the 36-byte Dash memo to text when it is a kind-1 (UTF-8 + /// text) memo: 4-byte LE kind tag == 1, then up to 32 bytes of UTF-8 + /// (trailing zeros trimmed). Returns `nil` for an empty / non-text / + /// undecodable memo. + var memoText: String? { + guard memo.count >= 4 else { return nil } + let bytes = [UInt8](memo) + let kind = UInt32(bytes[0]) | (UInt32(bytes[1]) << 8) + | (UInt32(bytes[2]) << 16) | (UInt32(bytes[3]) << 24) + guard kind == 1 else { return nil } + let payload = bytes[4...].prefix(while: { $0 != 0 }) + guard !payload.isEmpty, let s = String(bytes: payload, encoding: .utf8) else { return nil } + return s + } + + /// Short hex preview of `entryId` for diagnostics. + var entryIdHexShort: String { + let hex = entryId.map { String(format: "%02x", $0) }.joined() + return hex.count > 12 ? String(hex.prefix(12)) + "…" : hex + } +} + +// MARK: - List + +/// Per-wallet shielded activity list, grouped by status (Pending on top) +/// then ordered by block height descending. Reads `PersistentShielded +/// Activity` directly from SwiftData (the Rust persister writes the rows; +/// there is no read-side FFI — same pattern as the shielded notes / +/// outgoing-notes UI). +struct ShieldedActivityListView: View { + let walletId: Data + @Query private var entries: [PersistentShieldedActivity] + @State private var selected: PersistentShieldedActivity? + + init(walletId: Data) { + self.walletId = walletId + let descriptor = FetchDescriptor( + predicate: #Predicate { $0.walletId == walletId }, + sortBy: [ + // Confirmed/failed by height desc; pendings (hasBlockHeight + // == false, blockHeight 0) and the createdAtMs tiebreak are + // re-ordered in `sorted` below so pendings float to the top. + SortDescriptor(\PersistentShieldedActivity.blockHeight, order: .reverse), + SortDescriptor(\PersistentShieldedActivity.createdAtMs, order: .reverse), + ] + ) + _entries = Query(descriptor) + } + + /// Pending rows first (most recent record time first), then confirmed + /// / failed by block height descending — mirrors the Rust + /// `sort_activity_for_display` contract. + private var pending: [PersistentShieldedActivity] { + entries + .filter { !$0.hasBlockHeight } + .sorted { $0.createdAtMs > $1.createdAtMs } + } + + private var settled: [PersistentShieldedActivity] { + entries + .filter { $0.hasBlockHeight } + .sorted { + if $0.blockHeight != $1.blockHeight { return $0.blockHeight > $1.blockHeight } + return $0.createdAtMs > $1.createdAtMs + } + } + + var body: some View { + List { + if entries.isEmpty { + ContentUnavailableView( + "No Shielded Activity", + systemImage: "lock.rectangle.stack", + description: Text("Shielded sends, receives, and other private operations appear here.") + ) + } + if !pending.isEmpty { + Section("Pending") { + ForEach(pending, id: \.entryId) { row($0) } + } + } + if !settled.isEmpty { + Section("History") { + ForEach(settled, id: \.entryId) { row($0) } + } + } + } + .navigationTitle("Shielded Activity") + .navigationBarTitleDisplayMode(.inline) + .sheet(item: $selected) { ShieldedActivityDetailView(entry: $0) } + } + + @ViewBuilder + private func row(_ entry: PersistentShieldedActivity) -> some View { + Button { + selected = entry + } label: { + HStack(spacing: 12) { + Image(systemName: ShieldedActivityKindDisplay.icon(entry.kindTag)) + .font(.title3) + .foregroundColor(.accentColor) + .frame(width: 28) + VStack(alignment: .leading, spacing: 2) { + Text(ShieldedActivityKindDisplay.label(entry.kindTag)) + .font(.subheadline.weight(.medium)) + if let memo = entry.memoText { + Text(memo) + .font(.caption) + .foregroundColor(.secondary) + .lineLimit(1) + } else if let height = entry.hasBlockHeight ? entry.blockHeight : nil { + Text("Block \(height)") + .font(.caption) + .foregroundColor(.secondary) + } + } + Spacer() + VStack(alignment: .trailing, spacing: 2) { + Text(entry.signedAmountText) + .font(.subheadline.weight(.semibold)) + .foregroundColor(entry.amountColor) + if !entry.hasBlockHeight { + Text("Pending") + .font(.caption2) + .foregroundColor(.orange) + } else if entry.status == 2 { + Text("Failed") + .font(.caption2) + .foregroundColor(.red) + } + } + } + .contentShape(Rectangle()) + } + .buttonStyle(.plain) + .accessibilityIdentifier("shieldedActivity.row") + } +} + +// MARK: - Detail + +/// Full detail sheet for one activity entry. Shows every field; for +/// `IdentityCreate` the identity id is copyable, and `ShieldedSpend` +/// (restore-path residual) carries a clarifying footnote. +struct ShieldedActivityDetailView: View { + let entry: PersistentShieldedActivity + @Environment(\.dismiss) private var dismiss + + private var statusText: String { + switch entry.status { + case 0: return "Pending" + case 1: return "Confirmed" + default: return "Failed" + } + } + + private var directionText: String { + switch entry.direction { + case 0: return "In" + case 1: return "Out" + default: return "Self" + } + } + + var body: some View { + NavigationStack { + Form { + Section { + LabeledContent("Type", value: ShieldedActivityKindDisplay.label(entry.kindTag)) + LabeledContent("Amount", value: entry.signedAmountText) + if entry.hasFee { + LabeledContent("Fee", value: String(format: "%.6f DASH", Double(entry.fee) / 100_000_000_000.0)) + } + LabeledContent("Status", value: statusText) + LabeledContent("Direction", value: directionText) + if entry.hasBlockHeight { + LabeledContent("Block Height", value: "\(entry.blockHeight)") + } + } + + if let memo = entry.memoText { + Section("Memo") { + Text(memo).textSelection(.enabled) + } + } + + if entry.kindTag == 6, entry.identityId.count == 32 { + Section("Created Identity") { + let idHex = entry.identityId.map { String(format: "%02x", $0) }.joined() + Text(idHex) + .font(.caption.monospaced()) + .textSelection(.enabled) + Button { + UIPasteboard.general.string = idHex + } label: { + Label("Copy Identity ID", systemImage: "doc.on.doc") + } + } + } else if !entry.counterparty.isEmpty { + Section("Counterparty") { + let cpHex = entry.counterparty.map { String(format: "%02x", $0) }.joined() + Text(cpHex) + .font(.caption.monospaced()) + .textSelection(.enabled) + } + } + + Section("Details") { + LabeledContent("Entry ID", value: entry.entryIdHexShort) + let when = Date(timeIntervalSince1970: Double(entry.createdAtMs) / 1000.0) + LabeledContent("Recorded", value: when.formatted(date: .abbreviated, time: .shortened)) + if entry.noteCmxs.count >= 32 { + LabeledContent("Visible Outputs", value: "\(entry.noteCmxs.count / 32)") + } + if entry.spentNullifiers.count >= 32 { + LabeledContent("Spent Notes", value: "\(entry.spentNullifiers.count / 32)") + } + } + + if ShieldedActivityKindDisplay.isUnclassifiedResidual(entry.kindTag) { + Section { + Text("Restored wallets can't always recover the operation type. This entry was reconstructed from on-chain note data.") + .font(.footnote) + .foregroundColor(.secondary) + } + } + } + .navigationTitle("Activity Detail") + .navigationBarTitleDisplayMode(.inline) + .toolbar { + ToolbarItem(placement: .confirmationAction) { + Button("Done") { dismiss() } + } + } + } + } +} diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletDetailView.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletDetailView.swift index fe64cd79c5c..2a0d84540e3 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletDetailView.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletDetailView.swift @@ -178,6 +178,29 @@ struct WalletDetailView: View { } .buttonStyle(.plain) .padding(.horizontal) + + // Shielded Activity row — the derived private-operation + // history (shields, sends, unshields, withdrawals, + // identity-creates). Reads `PersistentShieldedActivity` + // from SwiftData; same value-based push as Transactions. + NavigationLink(value: ShieldedActivityRoute(walletId: wallet.walletId)) { + HStack { + Label("Shielded Activity", systemImage: "lock.rectangle.stack") + .font(.subheadline) + Spacer() + Image(systemName: "chevron.right") + .font(.caption) + .foregroundColor(.secondary) + } + .padding(.horizontal) + .padding(.vertical, 12) + .background(Color(UIColor.secondarySystemBackground)) + .cornerRadius(10) + } + .buttonStyle(.plain) + .padding(.horizontal) + .padding(.top, 8) + .accessibilityIdentifier("walletDetail.shieldedActivityLink") } Divider() diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletsContentView.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletsContentView.swift index b88f0d83ebf..017c377a5db 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletsContentView.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletsContentView.swift @@ -86,6 +86,9 @@ struct WalletsContentView: View { .navigationDestination(for: TransactionsRoute.self) { route in TransactionListView(walletId: route.walletId) } + .navigationDestination(for: ShieldedActivityRoute.self) { route in + ShieldedActivityListView(walletId: route.walletId) + } .navigationTitle("Wallets") .toolbar { ToolbarItem(placement: .navigationBarTrailing) { From 1fc46d93356eb5cb1c04e93d57097e9fb5d80d54 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 13:28:52 +0200 Subject: [PATCH 02/17] fix: cover PersistentShieldedActivity in the storage explorer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit check-storage-explorer requires every DashModelContainer model in the explorer's tile/count, list, and detail views — adds all three for the new activity model, following the shielded-outgoing-note pattern. Co-Authored-By: Claude Fable 5 --- .../Views/StorageExplorerView.swift | 10 +++ .../Views/StorageModelListViews.swift | 73 +++++++++++++++++++ .../Views/StorageRecordDetailViews.swift | 60 +++++++++++++++ 3 files changed, 143 insertions(+) diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageExplorerView.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageExplorerView.swift index 894bb0f6975..fd2e4ecc805 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageExplorerView.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageExplorerView.swift @@ -133,6 +133,13 @@ struct StorageExplorerView: View { ) { ShieldedSyncStateStorageListView(network: network) } + modelRow( + "Shielded Activity", + icon: "clock.arrow.circlepath", + type: PersistentShieldedActivity.self + ) { + ShieldedActivityStorageListView(network: network) + } } .navigationTitle("Storage Explorer") .toolbar { @@ -278,6 +285,9 @@ struct StorageExplorerView: View { filteredCount(PersistentShieldedSyncState.self) { walletsOnNetwork.contains($0.walletId) } + filteredCount(PersistentShieldedActivity.self) { + walletsOnNetwork.contains($0.walletId) + } filteredCount(PersistentAssetLock.self) { walletsOnNetwork.contains($0.walletId) } diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageModelListViews.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageModelListViews.swift index 48c13940a33..719f5b502e3 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageModelListViews.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageModelListViews.swift @@ -1896,6 +1896,79 @@ struct ShieldedOutgoingNoteStorageListView: View { } } +// MARK: - PersistentShieldedActivity + +/// Read-only browser for the derived shielded activity log the +/// persister mirrors out of `ShieldedChangeSet::activity_entries`. +/// Scoped by the active network via the denormalized `walletId` +/// column — same trick `ShieldedNoteStorageListView` uses. +struct ShieldedActivityStorageListView: View { + let network: Network + + /// Sort by block height (newest first), then account index so rows + /// from the same block stay deterministic. `entryId` is `Data` + /// (not Comparable), so it can't be a sort key. + @Query( + sort: [ + SortDescriptor(\PersistentShieldedActivity.blockHeight, order: .reverse), + SortDescriptor(\PersistentShieldedActivity.accountIndex), + ] + ) + private var records: [PersistentShieldedActivity] + + @Query private var allWallets: [PersistentWallet] + + private var walletIdsOnNetwork: Set { + Set(allWallets.lazy + .filter { $0.networkRaw == network.rawValue } + .map(\.walletId)) + } + + private var scopedRecords: [PersistentShieldedActivity] { + let ids = walletIdsOnNetwork + return records.filter { ids.contains($0.walletId) } + } + + var body: some View { + let visible = scopedRecords + List { + ForEach(visible) { record in + NavigationLink( + destination: ShieldedActivityStorageDetailView(record: record) + ) { + VStack(alignment: .leading, spacing: 4) { + HStack(spacing: 8) { + Text("kind \(record.kindTag)") + .font(.caption2) + .foregroundColor(.secondary) + Text("status \(record.status)") + .font(.caption2) + .foregroundColor(.secondary) + if record.hasBlockHeight { + Text("h \(record.blockHeight)") + .font(.caption2) + .foregroundColor(.secondary) + } + Spacer() + } + Text("\(record.amount) credits") + .font(.caption) + Text(record.entryId.prefix(8).map { String(format: "%02x", $0) }.joined()) + .font(.system(.caption2, design: .monospaced)) + .foregroundColor(.secondary) + } + } + } + } + .navigationTitle("Shielded Activity (\(visible.count))") + .overlay { + if visible.isEmpty { + ContentUnavailableView("No Activity", systemImage: "clock.arrow.circlepath") + } + } + } +} + // MARK: - PersistentShieldedSyncState struct ShieldedSyncStateStorageListView: View { diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageRecordDetailViews.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageRecordDetailViews.swift index 6ae85ff6c5a..52b25077771 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageRecordDetailViews.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageRecordDetailViews.swift @@ -1870,6 +1870,66 @@ struct ShieldedOutgoingNoteStorageDetailView: View { } } +// MARK: - PersistentShieldedActivity + +struct ShieldedActivityStorageDetailView: View { + let record: PersistentShieldedActivity + + var body: some View { + Form { + Section("Identity") { + FieldRow(label: "Wallet ID", value: hexString(record.walletId)) + FieldRow(label: "Account Index", value: "\(record.accountIndex)") + FieldRow(label: "Entry ID", value: hexString(record.entryId)) + } + Section("Classification") { + FieldRow(label: "Kind Tag", value: "\(record.kindTag)") + FieldRow(label: "Direction", value: "\(record.direction)") + FieldRow(label: "Status", value: "\(record.status)") + } + Section("Amounts") { + FieldRow(label: "Amount", value: "\(record.amount) credits") + FieldRow( + label: "Fee", + value: record.hasFee ? "\(record.fee) credits" : "(unknown)" + ) + FieldRow( + label: "Block Height", + value: record.hasBlockHeight ? "\(record.blockHeight)" : "(pending)" + ) + } + Section("Linkage") { + if !record.identityId.isEmpty { + FieldRow(label: "Identity ID", value: hexString(record.identityId)) + } + if !record.counterparty.isEmpty { + FieldRow(label: "Counterparty", value: hexString(record.counterparty)) + } + FieldRow(label: "Note cmxs", value: "\(record.noteCmxs.count / 32)") + FieldRow(label: "Spent Nullifiers", value: "\(record.spentNullifiers.count / 32)") + } + Section("Memo") { + if record.memo.isEmpty { + Text("(empty)") + .font(.caption) + .foregroundColor(.secondary) + } else { + Text(hexString(record.memo)) + .font(.system(.caption2, design: .monospaced)) + .textSelection(.enabled) + } + } + Section("Timestamps") { + FieldRow(label: "Created (ms)", value: "\(record.createdAtMs)") + FieldRow(label: "Created", value: dateString(record.createdAt)) + FieldRow(label: "Updated", value: dateString(record.lastUpdated)) + } + } + .navigationTitle("Shielded Activity") + .navigationBarTitleDisplayMode(.inline) + } +} + // MARK: - PersistentShieldedSyncState struct ShieldedSyncStateStorageDetailView: View { From 00133c31384300c4624df5566dde9be789f03f33 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 13:46:19 +0200 Subject: [PATCH 03/17] fix: address review on shielded activity (pending confirmation + recorder account match) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The scan deriver now returns on-chain sightings of clusters whose id already has a row (DerivedActivity::confirmations) instead of discarding them; the coordinator upgrades still-Pending / height-less stored rows to Confirmed at the observed height via with_status, so the ambiguous post-broadcast paths' documented promise ("a later scan flips the row") actually holds — and the live entry's richer fields survive the upgrade untouched. - The ShieldFromAssetLock recorder now selects the bound account whose keyset actually recognizes a visible output in the landed bundle (falling back to the lowest), matching its documented contract — a multi-account wallet no longer silently drops the entry or lands it under the wrong natural key. - FFI: empty cmx/nullifier buffers marshal real nulls per the documented "valid or null" pointer contract; unknown direction/status bytes on load warn loudly, with unknown status folding to Pending (re-confirmable) instead of the materially-distinct Failed. Co-Authored-By: Claude Fable 5 --- .../rs-platform-wallet-ffi/src/persistence.rs | 45 ++++++++++- .../src/wallet/shielded/activity.rs | 80 ++++++++++++++----- .../src/wallet/shielded/coordinator.rs | 33 +++++++- .../wallet/shielded/fund_from_asset_lock.rs | 15 +++- 4 files changed, 144 insertions(+), 29 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index 6e572d52b55..a4f1c923675 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -1391,9 +1391,23 @@ impl PlatformWalletPersistence for FFIPersister { counterparty_len, memo_ptr, memo_len, - note_cmxs_ptr: cmx_buf.as_ptr(), + // Match the documented "valid or null" + // contract (and the counterparty/memo + // siblings): an empty Vec's `as_ptr()` is + // a dangling non-null sentinel, so emit a + // real null when there's nothing to point + // at. + note_cmxs_ptr: if cmx_buf.is_empty() { + std::ptr::null() + } else { + cmx_buf.as_ptr() + }, note_cmxs_count: cmx_buf.len() / 32, - spent_nullifiers_ptr: nf_buf.as_ptr(), + spent_nullifiers_ptr: if nf_buf.is_empty() { + std::ptr::null() + } else { + nf_buf.as_ptr() + }, spent_nullifiers_count: nf_buf.len() / 32, } }) @@ -1828,12 +1842,35 @@ impl PlatformWalletPersistence for FFIPersister { let direction = match ffi.direction { 0 => ShieldedDirection::In, 1 => ShieldedDirection::Out, - _ => ShieldedDirection::SelfTransfer, + 2 => ShieldedDirection::SelfTransfer, + other => { + // Unlike kind_tag (whose residual + // `ShieldedSpend` variant is a designed + // forward-compat catch-all), direction has + // no "unknown" bucket — make a corrupted / + // future byte loud instead of silently + // reading as a real classification. + tracing::warn!( + direction = other, + "unknown shielded-activity direction byte on load; folding to SelfTransfer" + ); + ShieldedDirection::SelfTransfer + } }; let status = match ffi.status { 0 => ShieldedActivityStatus::Pending, 1 => ShieldedActivityStatus::Confirmed, - _ => ShieldedActivityStatus::Failed, + 2 => ShieldedActivityStatus::Failed, + other => { + // Failed is materially distinct from + // Pending/Confirmed — never let a stray + // byte silently mark an operation failed. + tracing::warn!( + status = other, + "unknown shielded-activity status byte on load; folding to Pending so a scan can re-confirm it" + ); + ShieldedActivityStatus::Pending + } }; let counterparty = if ffi.counterparty_ptr.is_null() || ffi.counterparty_len == 0 diff --git a/packages/rs-platform-wallet/src/wallet/shielded/activity.rs b/packages/rs-platform-wallet/src/wallet/shielded/activity.rs index 2ca224c9203..cf312c5ba45 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/activity.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/activity.rs @@ -240,6 +240,20 @@ pub struct ScanDeriveInput { pub own_addresses: Vec>, } +/// Output of [`derive_activity_from_scan_data`]. +#[derive(Debug, Clone, Default)] +pub struct DerivedActivity { + /// Entries for clusters with no existing row — new history. + pub new_entries: Vec, + /// `(entry_id, block_height)` sightings for clusters whose id + /// already has a row: the cluster was observed on-chain at that + /// height. The caller upgrades a still-`Pending` (or height-less) + /// stored row to `Confirmed` at the height — preserving the live + /// entry's richer fields — and ignores sightings for rows that are + /// already confirmed. + pub confirmations: Vec<([u8; 32], u64)>, +} + /// One block-height cluster of a subwallet's shielded events. /// /// Documented limitation (acceptable for v1): two same-wallet bundles @@ -354,9 +368,14 @@ fn cluster_events(input: &ScanDeriveInput) -> BTreeMap { /// correlation pass) can upgrade in place via the shared id. /// /// `existing_ids` are entry ids already on file (live entries win): a -/// cluster whose computed id is present is skipped, so a live +/// cluster whose computed id is present produces no NEW entry, so a live /// `IdentityCreate` / `Unshield` / `Withdrawal` is never clobbered by a -/// coarser scan-derived `Sent` / `ShieldedSpend`. +/// coarser scan-derived `Sent` / `ShieldedSpend`. It DOES produce a +/// [`DerivedActivity::confirmations`] observation — the ambiguous +/// post-broadcast paths leave their live row `Pending` with the explicit +/// promise that a later scan finding the cluster on-chain flips it to +/// `Confirmed` at the observed height (the caller performs that upgrade +/// against the stored row so the live entry's richer fields survive). /// /// [`Sent`]: ShieldedActivityKind::Sent /// [`Received`]: ShieldedActivityKind::Received @@ -364,9 +383,9 @@ fn cluster_events(input: &ScanDeriveInput) -> BTreeMap { pub fn derive_activity_from_scan_data( input: &ScanDeriveInput, existing_ids: &std::collections::BTreeSet<[u8; 32]>, -) -> Vec { +) -> DerivedActivity { let clusters = cluster_events(input); - let mut out = Vec::new(); + let mut out = DerivedActivity::default(); let now_ms = ShieldedActivityEntry::now_ms(); // Own-nullifier → (value, cmx) lookup for the rho linkage: a cluster @@ -398,6 +417,11 @@ pub fn derive_activity_from_scan_data( let id = compute_activity_id(&visible_cmxs); if existing_ids.contains(&id) { // A live entry (or an earlier scan) already owns this cluster. + // Don't synthesize a (coarser) duplicate — but DO report the + // on-chain sighting so the caller can flip a still-`Pending` + // row (the ambiguous post-broadcast paths) to Confirmed at + // this height. + out.confirmations.push((id, height)); continue; } @@ -520,7 +544,7 @@ pub fn derive_activity_from_scan_data( spent_nullifiers: Vec::new(), } }; - out.push(entry); + out.new_entries.push(entry); } out @@ -634,15 +658,22 @@ mod tests { }; let derived = derive_activity_from_scan_data(&input, &existing); assert!( - derived.is_empty(), + derived.new_entries.is_empty(), "scan must not re-emit a cluster a live entry already owns" ); + assert_eq!( + derived.confirmations, + vec![(live_id, 100)], + "...but it must report the on-chain sighting so a Pending \ + live row can be confirmed at the observed height" + ); // With no live entry on file, the same scan DOES produce one entry, // and its id equals the live id (the dedupe contract). let derived2 = derive_activity_from_scan_data(&input, &BTreeSet::new()); - assert_eq!(derived2.len(), 1); - assert_eq!(derived2[0].id, live_id); + assert_eq!(derived2.new_entries.len(), 1); + assert_eq!(derived2.new_entries[0].id, live_id); + assert!(derived2.confirmations.is_empty()); } // ── classification table (scan path) ─────────────────────────── @@ -654,7 +685,7 @@ mod tests { outgoing: vec![], own_addresses: vec![addr(0x01)], }; - let d = derive_activity_from_scan_data(&input, &BTreeSet::new()); + let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; assert_eq!(d.len(), 1); assert_eq!(d[0].kind, ShieldedActivityKind::Received); assert_eq!(d[0].direction, ShieldedDirection::In); @@ -675,7 +706,7 @@ mod tests { outgoing: vec![outgoing(0x30, addr(0xEE), 200, 750, memo.clone())], own_addresses: vec![addr(0x01)], }; - let d = derive_activity_from_scan_data(&input, &BTreeSet::new()); + let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; assert_eq!(d.len(), 1); assert_eq!(d[0].kind, ShieldedActivityKind::Sent); assert_eq!(d[0].direction, ShieldedDirection::Out); @@ -694,7 +725,7 @@ mod tests { outgoing: vec![outgoing(0x42, addr(0xEE), 300, 600, vec![])], // payment own_addresses: vec![addr(0x01)], }; - let d = derive_activity_from_scan_data(&input, &BTreeSet::new()); + let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; assert_eq!(d.len(), 1); assert_eq!(d[0].kind, ShieldedActivityKind::Sent); assert_eq!(d[0].amount, 600, "amount is the payment, not the change"); @@ -710,7 +741,7 @@ mod tests { outgoing: vec![outgoing(0x50, own.clone(), 400, 0, vec![])], own_addresses: vec![own], }; - let d = derive_activity_from_scan_data(&input, &BTreeSet::new()); + let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; assert_eq!(d.len(), 1); assert_eq!(d[0].kind, ShieldedActivityKind::ShieldedSpend); assert_eq!(d[0].direction, ShieldedDirection::SelfTransfer); @@ -726,7 +757,7 @@ mod tests { outgoing: vec![outgoing(0x60, addr(0xEE), 100, 500, vec![0u8; 36])], own_addresses: vec![addr(0x01)], }; - let d = derive_activity_from_scan_data(&input, &BTreeSet::new()); + let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; assert_eq!(d.len(), 1); assert!(d[0].memo.is_none(), "an all-zero memo must surface as None"); } @@ -735,7 +766,7 @@ mod tests { fn empty_cluster_visible_set_is_skipped() { // No notes, no outgoing → no clusters → no entries. let input = ScanDeriveInput::default(); - let d = derive_activity_from_scan_data(&input, &BTreeSet::new()); + let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; assert!(d.is_empty()); } @@ -751,7 +782,7 @@ mod tests { outgoing: vec![outgoing(0x72, addr(0xEE), 20, 800, vec![])], own_addresses: vec![addr(0x01)], }; - let mut d = derive_activity_from_scan_data(&input, &BTreeSet::new()); + let mut d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; sort_activity_for_display(&mut d); assert_eq!(d.len(), 2); // After display sort, the more recent (h=20 Sent) comes first. @@ -773,8 +804,15 @@ mod tests { let mut existing = BTreeSet::new(); existing.insert(live_id); let d = derive_activity_from_scan_data(&input, &existing); - assert_eq!(d.len(), 1); - assert_eq!(d[0].block_height, Some(20)); + assert_eq!(d.new_entries.len(), 1); + assert_eq!(d.new_entries[0].block_height, Some(20)); + assert_eq!( + d.confirmations, + vec![(live_id, 10)], + "existing-id clusters must be reported as on-chain sightings \ + with their observed height (the Pending->Confirmed promise \ + the ambiguous post-broadcast paths rely on)" + ); } // ── ordering ─────────────────────────────────────────────────── @@ -869,7 +907,7 @@ mod tests { outgoing: vec![outgoing(2, own.clone(), 113, 39_787_148_800, vec![0u8; 36])], own_addresses: vec![own], }; - let d = derive_activity_from_scan_data(&input, &BTreeSet::new()); + let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; let spend = d .iter() .find(|e| e.block_height == Some(113)) @@ -900,7 +938,7 @@ mod tests { ], own_addresses: vec![own], }; - let d = derive_activity_from_scan_data(&input, &BTreeSet::new()); + let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; let sent = d .iter() .find(|e| e.block_height == Some(20)) @@ -928,7 +966,7 @@ mod tests { outgoing: vec![outgoing(1, own.clone(), 30, 1_000_000, vec![0u8; 36])], own_addresses: vec![own], }; - let d = derive_activity_from_scan_data(&input, &BTreeSet::new()); + let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; assert_eq!(d.len(), 1); assert_eq!(d[0].kind, ShieldedActivityKind::ShieldedSpend); assert_eq!(d[0].direction, ShieldedDirection::SelfTransfer); @@ -946,7 +984,7 @@ mod tests { outgoing: vec![], own_addresses: vec![own], }; - let d = derive_activity_from_scan_data(&input, &BTreeSet::new()); + let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; assert_eq!(d.len(), 1); assert_eq!(d[0].kind, ShieldedActivityKind::Received); assert_eq!(d[0].direction, ShieldedDirection::In); diff --git a/packages/rs-platform-wallet/src/wallet/shielded/coordinator.rs b/packages/rs-platform-wallet/src/wallet/shielded/coordinator.rs index 5c12f8ab068..8e6368029ff 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/coordinator.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/coordinator.rs @@ -783,12 +783,43 @@ impl NetworkShieldedCoordinator { own_addresses, }; let derived = derive_activity_from_scan_data(&input, &existing_ids); - for entry in derived { + for entry in derived.new_entries { store.save_activity(*id, &entry).map_err(|e| { crate::error::PlatformWalletError::ShieldedStoreError(e.to_string()) })?; changeset.record_activity_entry(*id, entry); } + // On-chain sightings of clusters that already have a row: + // upgrade still-`Pending` (or height-less) rows to Confirmed + // at the observed height. This is the flip the ambiguous + // post-broadcast paths (`ShieldedSpendUnconfirmed` / + // `ShieldedBroadcastUnconfirmed`) leave to the scan. The + // upgrade rewrites the STORED entry via `with_status`, so the + // live entry's richer fields (kind / fee / memo / + // counterparty) survive untouched. + for (entry_id, height) in derived.confirmations { + let stored = store + .get_activity_by_entry_id(*id, &entry_id) + .map_err(|e| { + crate::error::PlatformWalletError::ShieldedStoreError(e.to_string()) + })?; + let Some(stored) = stored else { continue }; + let needs_upgrade = stored.status + == super::activity::ShieldedActivityStatus::Pending + || stored.block_height.is_none(); + if !needs_upgrade { + continue; + } + let upgraded = super::activity_recorder::with_status( + &stored, + super::activity::ShieldedActivityStatus::Confirmed, + Some(height), + ); + store.save_activity(*id, &upgraded).map_err(|e| { + crate::error::PlatformWalletError::ShieldedStoreError(e.to_string()) + })?; + changeset.record_activity_entry(*id, upgraded); + } } Ok(()) } diff --git a/packages/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rs b/packages/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rs index f3861280b88..784472b03ce 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rs @@ -530,9 +530,18 @@ impl PlatformWallet { let Some(keys_map) = guard.as_ref() else { return; }; - // Prefer the account whose keyset recognizes a visible output; - // fall back to the lowest bound account. - let Some((&account, keyset)) = keys_map.iter().next() else { + // Prefer the account whose keyset actually recognizes a visible + // output in the landed bundle (the funded note decrypts under its + // IVK / recovers under its OVK) — the row must land under THAT + // account or the recorder builds with the wrong keys, recovers no + // cmx, and silently drops the entry; it would also break the + // shared-id natural key against the eventual scan-derived row. + // Fall back to the lowest bound account only when nothing + // matches (a shield to a fully external recipient). + let matched = keys_map.iter().find(|(_, ks)| { + !crate::wallet::shielded::activity_recorder::visible_output_cmxs(actions, ks).is_empty() + }); + let Some((&account, keyset)) = matched.or_else(|| keys_map.iter().next()) else { return; }; From a35bc9ad33cd9593c1f8662da8f7ba2e94dff0cb Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 13:53:49 +0200 Subject: [PATCH 04/17] fix: partition and badge shielded activity rows by status, not height The live recorders mark a successful broadcast Confirmed with block_height=None (the scan backfills the height later), so a height-less row is the COMMON success shape. Partitioning/badging on hasBlockHeight rendered every fresh success as "Pending"; both now key on entry.status (matching the detail view), with just-confirmed height-less rows floating to the top of History. Co-Authored-By: Claude Fable 5 --- .../Core/Views/ShieldedActivityView.swift | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/ShieldedActivityView.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/ShieldedActivityView.swift index 60a55525329..27fb8d98e2e 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/ShieldedActivityView.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/ShieldedActivityView.swift @@ -124,17 +124,28 @@ struct ShieldedActivityListView: View { /// Pending rows first (most recent record time first), then confirmed /// / failed by block height descending — mirrors the Rust /// `sort_activity_for_display` contract. + /// + /// Partition by STATUS, not by `hasBlockHeight`: the live recorders + /// mark a successful broadcast `Confirmed` with `block_height: None` + /// (the scan backfills the height later), so a height-less row is + /// the COMMON success shape — only `status == 0` is actually + /// pending. Height-less confirmed rows sort by record time among + /// the settled (they're the newest). private var pending: [PersistentShieldedActivity] { entries - .filter { !$0.hasBlockHeight } + .filter { $0.status == 0 } .sorted { $0.createdAtMs > $1.createdAtMs } } private var settled: [PersistentShieldedActivity] { entries - .filter { $0.hasBlockHeight } + .filter { $0.status != 0 } .sorted { - if $0.blockHeight != $1.blockHeight { return $0.blockHeight > $1.blockHeight } + // Height-less (just-confirmed) rows float above + // height-bearing ones, then height desc, then recency. + let lh = $0.hasBlockHeight ? $0.blockHeight : UInt64.max + let rh = $1.hasBlockHeight ? $1.blockHeight : UInt64.max + if lh != rh { return lh > rh } return $0.createdAtMs > $1.createdAtMs } } @@ -193,7 +204,10 @@ struct ShieldedActivityListView: View { Text(entry.signedAmountText) .font(.subheadline.weight(.semibold)) .foregroundColor(entry.amountColor) - if !entry.hasBlockHeight { + // Badge by STATUS — a Confirmed row without a height + // (live success awaiting the scan's height backfill) + // must not read as Pending. + if entry.status == 0 { Text("Pending") .font(.caption2) .foregroundColor(.orange) From 97e190ca1d44b4fbc11537eca28afd2d1509bb2f Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 14:06:46 +0200 Subject: [PATCH 05/17] fix: address CodeRabbit review on shielded activity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - decode_cmx_array guards the host-supplied count with checked_mul before from_raw_parts (corrupt rows drop the linkage instead of UB). - Scan classifier: a self-pay cluster in a subwallet with ZERO spent notes is necessarily a shield-to-self (change-from-spend is impossible), so it now reads as Received/In; the SelfTransfer residual is reserved for wallets that have spends. Tests pin both. - Swift trampoline uses checked multiplication for the Rust-supplied linkage counts; the restore marshal rejects blobs that aren't whole 32-byte multiples (null/0) instead of silently truncating. - Storage-explorer activity detail decodes kind/direction/status discriminants to labeled values. Not changed: build_pending_entry's None branch (review suggested a fallback id for zero-visible-output bundles) — build_spend_bundle_with adds the change/recipient output unconditionally, OVK-keyed to the wallet's own external OVK, so even an exact-input spend carries a wallet-visible output; the branch guards a shape our builders cannot produce and warns loudly if it ever fires. Co-Authored-By: Claude Fable 5 --- .../rs-platform-wallet-ffi/src/persistence.rs | 12 ++- .../src/wallet/shielded/activity.rs | 100 ++++++++++++++---- .../PlatformWalletPersistenceHandler.swift | 23 ++-- .../Views/StorageRecordDetailViews.swift | 44 +++++++- 4 files changed, 147 insertions(+), 32 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index a4f1c923675..ef637c28fd8 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -2150,7 +2150,17 @@ unsafe fn decode_cmx_array(ptr: *const u8, count: usize) -> Vec<[u8; 32]> { if ptr.is_null() || count == 0 { return Vec::new(); } - let bytes = slice::from_raw_parts(ptr, count * 32); + // `count` is host-supplied: guard the multiplication so a corrupt + // row degrades to a dropped linkage instead of an overflowed length + // handed to `from_raw_parts` (UB). + let Some(byte_len) = count.checked_mul(32) else { + tracing::warn!( + count, + "shielded activity linkage count overflows on load; dropping linkage bytes" + ); + return Vec::new(); + }; + let bytes = slice::from_raw_parts(ptr, byte_len); bytes .chunks_exact(32) .filter_map(|c| <[u8; 32]>::try_from(c).ok()) diff --git a/packages/rs-platform-wallet/src/wallet/shielded/activity.rs b/packages/rs-platform-wallet/src/wallet/shielded/activity.rs index cf312c5ba45..f3f92905818 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/activity.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/activity.rs @@ -397,6 +397,13 @@ pub fn derive_activity_from_scan_data( .map(|n| (n.nullifier, (n.value, n.cmx))) .collect(); + // Global "did this subwallet ever spend anything" signal. A self-pay + // cluster can only be change-from-a-spend if SOME own note was spent; + // a wallet with zero spent notes can only have produced it by + // shielding to itself, so the self-pay ambiguity collapses to a + // receive in that case. + let wallet_has_any_spend = input.notes.iter().any(|n| n.is_spent); + for (height, cluster) in clusters { // Visible output cmxs for this cluster: own received-note cmxs + // recovered-send cmxs. This is exactly the set the live recorder @@ -520,15 +527,33 @@ pub fn derive_activity_from_scan_data( note_cmxs: visible_cmxs, spent_nullifiers: Vec::new(), } + } else if !wallet_has_any_spend { + // SELF-PAY in a subwallet that has NEVER spent a note: change + // from a spend is impossible, so this is necessarily a shield + // to self — surface it as the inbound value it is. + ShieldedActivityEntry { + id, + kind: ShieldedActivityKind::Received, + direction: ShieldedDirection::In, + amount: change_total, + fee: None, + counterparty: None, + memo: None, + block_height: Some(height), + status: ShieldedActivityStatus::Confirmed, + created_at_ms: now_ms, + note_cmxs: visible_cmxs, + spent_nullifiers: Vec::new(), + } } else { - // SELF-PAY, UNLINKED: every output pays ourselves and no rho - // link fired. Two indistinguishable possibilities: a shield to - // self (Type 15/18 — the common case) or change from our own - // spend whose output landed in a different action than the - // real spend (the ~50% shuffle miss; see `note_rho`). Calling - // it `Received` would show a spend as money arriving, so this - // is surfaced as a self-transfer instead — honest for both - // readings and upgradeable in place by a live entry. + // SELF-PAY, UNLINKED, in a wallet that HAS spends: two + // indistinguishable possibilities — a shield to self (Type + // 15/18) or change from our own spend whose output landed in a + // different action than the real spend (the ~50% shuffle miss; + // see `note_rho`). Calling it `Received` would show a spend as + // money arriving, so this is surfaced as a self-transfer + // instead — honest for both readings and upgradeable in place + // by a live entry. ShieldedActivityEntry { id, kind: ShieldedActivityKind::ShieldedSpend, @@ -733,19 +758,26 @@ mod tests { #[test] fn self_change_only_cluster_is_shielded_spend() { - // An outgoing note paying one of our OWN addresses, nothing else: - // a spend whose output is all self-change → residual ShieldedSpend. + // An outgoing note paying one of our OWN addresses, nothing else, + // in a wallet that HAS spent notes: a spend whose output is all + // self-change → residual ShieldedSpend. (In a never-spent wallet + // the same shape collapses to Received — see + // `self_pay_in_never_spent_wallet_is_received`.) let own = addr(0x01); + let spent_elsewhere = own_note(9, 0x99, 5, 7_000, true); let input = ScanDeriveInput { - notes: vec![], + notes: vec![spent_elsewhere], outgoing: vec![outgoing(0x50, own.clone(), 400, 0, vec![])], own_addresses: vec![own], }; let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; - assert_eq!(d.len(), 1); - assert_eq!(d[0].kind, ShieldedActivityKind::ShieldedSpend); - assert_eq!(d[0].direction, ShieldedDirection::SelfTransfer); - assert!(d[0].fee.is_none()); + let e = d + .iter() + .find(|e| e.block_height == Some(400)) + .expect("self-change cluster entry"); + assert_eq!(e.kind, ShieldedActivityKind::ShieldedSpend); + assert_eq!(e.direction, ShieldedDirection::SelfTransfer); + assert!(e.fee.is_none()); } // ── zero-value filler / memo exclusion ───────────────────────── @@ -954,11 +986,35 @@ mod tests { } #[test] - fn unlinked_self_pay_is_self_transfer_not_received() { - // Self-pay cluster with no rho link: either a shield-to-self or - // change whose output landed in a different action than the real - // spend (the ~50% shuffle miss). Must NOT surface as `Received` — - // that would show a spend as money arriving. + fn unlinked_self_pay_is_self_transfer_when_wallet_has_spends() { + // Self-pay cluster with no rho link in a wallet that HAS spent + // notes: either a shield-to-self or change whose output landed in + // a different action than the real spend (the ~50% shuffle miss). + // Must NOT surface as `Received` — that would show a spend as + // money arriving. + let own = addr(0xAA); + let spent_elsewhere = own_note(9, 0x99, 5, 7_000, true); + let note = own_note(1, 7, 30, 1_000_000, false); + let input = ScanDeriveInput { + notes: vec![spent_elsewhere, note], + outgoing: vec![outgoing(1, own.clone(), 30, 1_000_000, vec![0u8; 36])], + own_addresses: vec![own], + }; + let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; + let e = d + .iter() + .find(|e| e.block_height == Some(30)) + .expect("self-pay cluster entry"); + assert_eq!(e.kind, ShieldedActivityKind::ShieldedSpend); + assert_eq!(e.direction, ShieldedDirection::SelfTransfer); + assert_eq!(e.amount, 1_000_000); + } + + #[test] + fn self_pay_in_never_spent_wallet_is_received() { + // The same self-pay shape in a subwallet with ZERO spent notes: + // change-from-spend is impossible, so it is necessarily a shield + // to self and must read as inbound value, not a self-transfer. let own = addr(0xAA); let note = own_note(1, 7, 30, 1_000_000, false); let input = ScanDeriveInput { @@ -968,8 +1024,8 @@ mod tests { }; let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; assert_eq!(d.len(), 1); - assert_eq!(d[0].kind, ShieldedActivityKind::ShieldedSpend); - assert_eq!(d[0].direction, ShieldedDirection::SelfTransfer); + assert_eq!(d[0].kind, ShieldedActivityKind::Received); + assert_eq!(d[0].direction, ShieldedDirection::In); assert_eq!(d[0].amount, 1_000_000); } diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift index ef7ffd91919..9d321628810 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift @@ -2852,10 +2852,14 @@ public class PlatformWalletPersistenceHandler { counterparty_len: UInt(cpLen), memo_ptr: memoLen > 0 ? UnsafePointer(memoPtr) : nil, memo_len: UInt(memoLen), - note_cmxs_ptr: cmxLen > 0 ? UnsafePointer(cmxPtr) : nil, - note_cmxs_count: UInt(cmxLen / 32), - spent_nullifiers_ptr: nfLen > 0 ? UnsafePointer(nfPtr) : nil, - spent_nullifiers_count: UInt(nfLen / 32) + // A persisted blob that isn't a whole number of + // 32-byte elements is corrupt — drop the linkage + // (count 0, null ptr) rather than silently truncating + // trailing bytes into a wrong-but-plausible array. + note_cmxs_ptr: cmxLen > 0 && cmxLen % 32 == 0 ? UnsafePointer(cmxPtr) : nil, + note_cmxs_count: cmxLen % 32 == 0 ? UInt(cmxLen / 32) : 0, + spent_nullifiers_ptr: nfLen > 0 && nfLen % 32 == 0 ? UnsafePointer(nfPtr) : nil, + spent_nullifiers_count: nfLen % 32 == 0 ? UInt(nfLen / 32) : 0 ) written += 1 allocation.entriesInitialized = written @@ -5679,8 +5683,15 @@ private func persistShieldedActivityCallback( } let counterparty = data(e.counterparty_ptr, UInt(e.counterparty_len)) let memo = data(e.memo_ptr, UInt(e.memo_len)) - let noteCmxs = data(e.note_cmxs_ptr, UInt(e.note_cmxs_count) * 32) - let spentNullifiers = data(e.spent_nullifiers_ptr, UInt(e.spent_nullifiers_count) * 32) + // The counts are Rust-supplied: use checked multiplication so + // a corrupt row degrades to an empty linkage instead of a + // trapped overflow crashing the callback path. + func byteLen(_ count: UInt) -> UInt { + let (value, overflow) = count.multipliedReportingOverflow(by: 32) + return overflow ? 0 : value + } + let noteCmxs = data(e.note_cmxs_ptr, byteLen(UInt(e.note_cmxs_count))) + let spentNullifiers = data(e.spent_nullifiers_ptr, byteLen(UInt(e.spent_nullifiers_count))) let identityId = e.has_identity_id != 0 ? dataFromTuple32(e.identity_id) : Data() snapshots.append(.init( walletId: dataFromTuple32(e.wallet_id), diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageRecordDetailViews.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageRecordDetailViews.swift index 52b25077771..a3c5fe7ab1c 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageRecordDetailViews.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageRecordDetailViews.swift @@ -1883,9 +1883,9 @@ struct ShieldedActivityStorageDetailView: View { FieldRow(label: "Entry ID", value: hexString(record.entryId)) } Section("Classification") { - FieldRow(label: "Kind Tag", value: "\(record.kindTag)") - FieldRow(label: "Direction", value: "\(record.direction)") - FieldRow(label: "Status", value: "\(record.status)") + FieldRow(label: "Kind Tag", value: kindDisplay(record.kindTag)) + FieldRow(label: "Direction", value: directionDisplay(record.direction)) + FieldRow(label: "Status", value: statusDisplay(record.status)) } Section("Amounts") { FieldRow(label: "Amount", value: "\(record.amount) credits") @@ -1928,6 +1928,44 @@ struct ShieldedActivityStorageDetailView: View { .navigationTitle("Shielded Activity") .navigationBarTitleDisplayMode(.inline) } + + private func kindDisplay(_ tag: Int) -> String { + let name: String + switch tag { + case 0: name = "Shield" + case 1: name = "ShieldFromAssetLock" + case 2: name = "Received" + case 3: name = "Sent" + case 4: name = "Unshield" + case 5: name = "Withdrawal" + case 6: name = "IdentityCreate" + case 7: name = "ShieldedSpend" + default: return "Unknown(\(tag))" + } + return "\(name) (\(tag))" + } + + private func directionDisplay(_ raw: Int) -> String { + let name: String + switch raw { + case 0: name = "In" + case 1: name = "Out" + case 2: name = "Self" + default: return "Unknown(\(raw))" + } + return "\(name) (\(raw))" + } + + private func statusDisplay(_ raw: Int) -> String { + let name: String + switch raw { + case 0: name = "Pending" + case 1: name = "Confirmed" + case 2: name = "Failed" + default: return "Unknown(\(raw))" + } + return "\(name) (\(raw))" + } } // MARK: - PersistentShieldedSyncState From ae311935e431948fd6b4eb44fef03d44d16e055d Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 14:31:23 +0200 Subject: [PATCH 06/17] fix: write live activity entries to the in-memory store; overlap-based scan dedupe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the remaining review findings on the shielded activity log: - Live recorders (record_pending_activity / record_activity_status / queue_shielded_activity) now save_activity into the in-memory ShieldedStore before queueing to the host persister, so the scan deriver's dedupe sees intra-session live entries and can no longer clobber a rich live row with a coarser scan-derived one. The Shield (T15) and ShieldFromAssetLock/seed paths gained the store/coordinator plumbing this requires (FFI extern "C" signatures unchanged). - The deriver dedupes by cmx OVERLAP against stored entries (cmx → entry-id map) instead of exact id equality: a same-block cluster merging two live ops now confirms BOTH entries at the observed height instead of synthesizing a spurious aggregate row. New test pins it. - Confirmation-upgrade gate keeps upgrading Failed-no-height rows, now with the chain-truth-wins policy stated explicitly (observed on devnet: a client-side result-proof fetch error marked an actually-landed identity-create as Failed). - decode_cmx_array also enforces slice::from_raw_parts' isize::MAX bound, and its stale "multiple of 32" doc clause now describes the real guards; the unknown-tag warn messages are single-line literals. Co-Authored-By: Claude Fable 5 --- .../rs-platform-wallet-ffi/src/persistence.rs | 28 ++-- .../src/shielded_send.rs | 58 +++---- .../src/wallet/platform_wallet.rs | 2 + .../src/wallet/shielded/activity.rs | 142 +++++++++++++----- .../src/wallet/shielded/coordinator.rs | 25 ++- .../wallet/shielded/fund_from_asset_lock.rs | 8 +- .../src/wallet/shielded/operations.rs | 114 ++++++++++---- .../src/wallet/shielded/seed_pool.rs | 2 + 8 files changed, 259 insertions(+), 120 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index ef637c28fd8..d83984842de 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -1852,7 +1852,7 @@ impl PlatformWalletPersistence for FFIPersister { // reading as a real classification. tracing::warn!( direction = other, - "unknown shielded-activity direction byte on load; folding to SelfTransfer" + "unknown shielded-activity direction byte on load; folding to SelfTransfer" ); ShieldedDirection::SelfTransfer } @@ -1867,7 +1867,7 @@ impl PlatformWalletPersistence for FFIPersister { // byte silently mark an operation failed. tracing::warn!( status = other, - "unknown shielded-activity status byte on load; folding to Pending so a scan can re-confirm it" + "unknown shielded-activity status byte on load; folding to Pending so a scan can re-confirm it" ); ShieldedActivityStatus::Pending } @@ -2136,15 +2136,16 @@ impl PlatformWalletPersistence for FFIPersister { /// Flatten an `AccountType` + encoded xpub into the C-flat /// Decode `count` contiguous 32-byte commitments / nullifiers from a -/// host buffer into `Vec<[u8; 32]>`. A null pointer or a length that -/// isn't a clean multiple of 32 yields an empty vec (the buffer is -/// Rust-written on persist and host-round-tripped on load, so a -/// malformed length means a corrupt row — drop the linkage rather than -/// read past the buffer). +/// host buffer into `Vec<[u8; 32]>`. A null pointer, a zero count, or a +/// `count` whose byte length overflows / exceeds `isize::MAX` (the +/// `slice::from_raw_parts` bound) yields an empty vec — the buffer is +/// Rust-written on persist and host-round-tripped on load, so an +/// out-of-range count means a corrupt row, and the linkage is dropped +/// rather than read past the buffer. /// /// # Safety -/// `ptr` must point to at least `count * 32` valid bytes for the call, -/// or be null. +/// When non-null, `ptr` must point to at least `count * 32` valid +/// bytes for the duration of the call. #[cfg(feature = "shielded")] unsafe fn decode_cmx_array(ptr: *const u8, count: usize) -> Vec<[u8; 32]> { if ptr.is_null() || count == 0 { @@ -2152,8 +2153,13 @@ unsafe fn decode_cmx_array(ptr: *const u8, count: usize) -> Vec<[u8; 32]> { } // `count` is host-supplied: guard the multiplication so a corrupt // row degrades to a dropped linkage instead of an overflowed length - // handed to `from_raw_parts` (UB). - let Some(byte_len) = count.checked_mul(32) else { + // handed to `from_raw_parts` (UB). Also enforce `from_raw_parts`' + // documented bound that the slice length must not exceed + // `isize::MAX` bytes. + let Some(byte_len) = count + .checked_mul(32) + .filter(|&len| len <= isize::MAX as usize) + else { tracing::warn!( count, "shielded activity linkage count overflows on load; dropping linkage bytes" diff --git a/packages/rs-platform-wallet-ffi/src/shielded_send.rs b/packages/rs-platform-wallet-ffi/src/shielded_send.rs index f1fd13f7bfc..268ba56e303 100644 --- a/packages/rs-platform-wallet-ffi/src/shielded_send.rs +++ b/packages/rs-platform-wallet-ffi/src/shielded_send.rs @@ -674,8 +674,11 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_shield( let mut wallet_id = [0u8; 32]; std::ptr::copy_nonoverlapping(wallet_id_bytes, wallet_id.as_mut_ptr(), 32); - let wallet = match resolve_wallet(handle, &wallet_id) { - Ok(w) => w, + // Shield writes its live activity entry to the coordinator's shared + // in-memory store, so resolve the coordinator alongside the wallet + // (same resolver the transfer / unshield / withdraw spends use). + let (wallet, coordinator) = match resolve_wallet_and_coordinator(handle, &wallet_id) { + Ok(p) => p, Err(result) => return result, }; @@ -704,6 +707,7 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_shield( let prover = CachedOrchardProver::new(); wallet .shielded_shield_from_account( + &coordinator, shielded_account, payment_account, amount, @@ -802,8 +806,11 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_fund_from_asset_lock( Err(result) => return result, }; - let wallet = match resolve_wallet(handle, &wallet_id) { - Ok(w) => w, + // The Type 18 live activity recorder writes to the coordinator's + // shared in-memory store, so resolve the coordinator alongside the + // wallet. + let (wallet, coordinator) = match resolve_wallet_and_coordinator(handle, &wallet_id) { + Ok(p) => p, Err(result) => return result, }; let network = wallet.network(); @@ -830,6 +837,7 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_fund_from_asset_lock( let prover = CachedOrchardProver::new(); wallet .shielded_fund_from_asset_lock( + &coordinator, AssetLockFunding::FromWalletBalance { amount_duffs, account_index, @@ -950,8 +958,11 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_resume_fund_from_asset vout: out_point_ffi.vout, }; - let wallet = match resolve_wallet(handle, &wallet_id) { - Ok(w) => w, + // The Type 18 live activity recorder writes to the coordinator's + // shared in-memory store, so resolve the coordinator alongside the + // wallet. + let (wallet, coordinator) = match resolve_wallet_and_coordinator(handle, &wallet_id) { + Ok(p) => p, Err(result) => return result, }; let network = wallet.network(); @@ -971,6 +982,7 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_resume_fund_from_asset let prover = CachedOrchardProver::new(); wallet .shielded_fund_from_asset_lock( + &coordinator, AssetLockFunding::FromExistingAssetLock { out_point: resume_outpoint, }, @@ -1056,8 +1068,11 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_seed_pool_notes( let mut wallet_id = [0u8; 32]; std::ptr::copy_nonoverlapping(wallet_id_bytes, wallet_id.as_mut_ptr(), 32); - let wallet = match resolve_wallet(handle, &wallet_id) { - Ok(w) => w, + // Each seeding batch's Type 18 live activity recorder writes to the + // coordinator's shared in-memory store, so resolve the coordinator + // alongside the wallet. + let (wallet, coordinator) = match resolve_wallet_and_coordinator(handle, &wallet_id) { + Ok(p) => p, Err(result) => return result, }; let network = wallet.network(); @@ -1107,6 +1122,7 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_seed_pool_notes( wallet .shielded_seed_pool_notes( + &coordinator, &wallet_id, account, target_total_notes, @@ -1127,32 +1143,6 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_seed_pool_notes( } } -/// Resolve the wallet `Arc` for the given manager handle, or -/// produce a `PlatformWalletFFIResult` describing why we couldn't. -fn resolve_wallet( - handle: Handle, - wallet_id: &[u8; 32], -) -> Result, PlatformWalletFFIResult> { - let option = PLATFORM_WALLET_MANAGER_STORAGE.with_item(handle, |manager| { - runtime().block_on(manager.get_wallet(wallet_id)) - }); - let inner_option = match option { - Some(v) => v, - None => { - return Err(PlatformWalletFFIResult::err( - PlatformWalletFFIResultCode::ErrorInvalidHandle, - format!("invalid manager handle: {handle}"), - )); - } - }; - inner_option.ok_or_else(|| { - PlatformWalletFFIResult::err( - PlatformWalletFFIResultCode::ErrorWalletOperation, - format!("wallet not found: {}", hex::encode(wallet_id)), - ) - }) -} - /// Resolve both the wallet `Arc` and the network-scoped shielded /// coordinator `Arc` for the given manager handle. Shielded /// spend operations need the coordinator's shared store, so this diff --git a/packages/rs-platform-wallet/src/wallet/platform_wallet.rs b/packages/rs-platform-wallet/src/wallet/platform_wallet.rs index 61ca0a5db9b..8a95c25d210 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_wallet.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_wallet.rs @@ -837,6 +837,7 @@ impl PlatformWallet { #[cfg(feature = "shielded")] pub async fn shielded_shield_from_account( &self, + coordinator: &Arc, shielded_account: u32, payment_account: u32, amount: u64, @@ -948,6 +949,7 @@ impl PlatformWallet { })?; super::shielded::operations::shield( &self.sdk, + coordinator.store(), Some(&self.persister), self.wallet_id, keyset, diff --git a/packages/rs-platform-wallet/src/wallet/shielded/activity.rs b/packages/rs-platform-wallet/src/wallet/shielded/activity.rs index f3f92905818..73726955cc7 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/activity.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/activity.rs @@ -367,22 +367,33 @@ fn cluster_events(input: &ScanDeriveInput) -> BTreeMap { /// stays `ShieldedSpend`, which a later live entry (or a future /// correlation pass) can upgrade in place via the shared id. /// -/// `existing_ids` are entry ids already on file (live entries win): a -/// cluster whose computed id is present produces no NEW entry, so a live +/// `existing_cmxs` maps every stored entry's visible output cmx to the +/// id of the entry that owns it (live entries win). Dedupe is by cmx +/// OVERLAP, not exact-id equality: a cluster whose visible cmxs intersect +/// any stored entry's cmx set produces no NEW entry (so a live /// `IdentityCreate` / `Unshield` / `Withdrawal` is never clobbered by a -/// coarser scan-derived `Sent` / `ShieldedSpend`. It DOES produce a -/// [`DerivedActivity::confirmations`] observation — the ambiguous -/// post-broadcast paths leave their live row `Pending` with the explicit -/// promise that a later scan finding the cluster on-chain flips it to -/// `Confirmed` at the observed height (the caller performs that upgrade -/// against the stored row so the live entry's richer fields survive). +/// coarser scan-derived `Sent` / `ShieldedSpend`), and instead emits a +/// [`DerivedActivity::confirmations`] observation for EACH overlapped id. +/// The ambiguous post-broadcast paths leave their live row `Pending` with +/// the explicit promise that a later scan finding the cluster on-chain +/// flips it to `Confirmed` at the observed height (the caller performs +/// that upgrade against the stored row so the live entry's richer fields +/// survive). +/// +/// Overlap subsumes the exact-id case AND handles the same-block merge +/// hazard: when one cluster merges two live ops (cmx sets A and B) its +/// computed id `H(A∪B)` matches neither live id, but the overlap catches +/// both and forgoes synthesizing a spurious aggregate row. The +/// conservative trade: a same-block mix of an owned-op and an unrelated +/// receive forgoes the receive's own row (it folds into the confirmation +/// for the owned op) — acceptable to avoid both clobber and aggregates. /// /// [`Sent`]: ShieldedActivityKind::Sent /// [`Received`]: ShieldedActivityKind::Received /// [`ShieldedSpend`]: ShieldedActivityKind::ShieldedSpend pub fn derive_activity_from_scan_data( input: &ScanDeriveInput, - existing_ids: &std::collections::BTreeSet<[u8; 32]>, + existing_cmxs: &std::collections::BTreeMap<[u8; 32], [u8; 32]>, ) -> DerivedActivity { let clusters = cluster_events(input); let mut out = DerivedActivity::default(); @@ -422,13 +433,25 @@ pub fn derive_activity_from_scan_data( continue; } let id = compute_activity_id(&visible_cmxs); - if existing_ids.contains(&id) { - // A live entry (or an earlier scan) already owns this cluster. - // Don't synthesize a (coarser) duplicate — but DO report the - // on-chain sighting so the caller can flip a still-`Pending` - // row (the ambiguous post-broadcast paths) to Confirmed at - // this height. - out.confirmations.push((id, height)); + // Overlap-based dedupe: any stored entry whose visible cmx set + // intersects this cluster's cmxs already owns (part of) the + // cluster. `BTreeSet` so each overlapped id is reported once even + // when several of the cluster's cmxs map to the same entry. + let overlapping: std::collections::BTreeSet<[u8; 32]> = visible_cmxs + .iter() + .filter_map(|c| existing_cmxs.get(c)) + .copied() + .collect(); + if !overlapping.is_empty() { + // A live entry (or an earlier scan) already owns this cluster + // (exactly, or as one of a same-block merge). Don't synthesize + // a (coarser, or spuriously aggregate) duplicate — but DO + // report the on-chain sighting for EACH overlapped id so the + // caller can flip a still-`Pending` row (the ambiguous + // post-broadcast paths) to Confirmed at this height. + for entry_id in overlapping { + out.confirmations.push((entry_id, height)); + } continue; } @@ -610,7 +633,7 @@ pub fn sort_activity_for_display(entries: &mut [ShieldedActivityEntry]) { #[cfg(test)] mod tests { use super::*; - use std::collections::BTreeSet; + use std::collections::BTreeMap; fn own_note(cmx: u8, nullifier: u8, height: u64, value: u64, spent: bool) -> ShieldedNote { ShieldedNote { @@ -669,12 +692,13 @@ mod tests { fn live_entry_then_scan_of_same_cluster_yields_one_entry() { // Live recorder recorded a Sent for a bundle whose single visible // output cmx is `7`. The scan later sees the same cluster (one - // outgoing note with cmx `7`). Because the id matches and the live - // id is in `existing_ids`, the scan produces NOTHING for it. + // outgoing note with cmx `7`). Because the cluster's cmx overlaps + // the stored entry's cmx (mapped to `live_id` in `existing_cmxs`), + // the scan produces NOTHING for it. let cmx = [7u8; 32]; let live_id = compute_activity_id(&[cmx]); - let mut existing = BTreeSet::new(); - existing.insert(live_id); + let mut existing = BTreeMap::new(); + existing.insert(cmx, live_id); let input = ScanDeriveInput { notes: vec![], @@ -695,12 +719,51 @@ mod tests { // With no live entry on file, the same scan DOES produce one entry, // and its id equals the live id (the dedupe contract). - let derived2 = derive_activity_from_scan_data(&input, &BTreeSet::new()); + let derived2 = derive_activity_from_scan_data(&input, &BTreeMap::new()); assert_eq!(derived2.new_entries.len(), 1); assert_eq!(derived2.new_entries[0].id, live_id); assert!(derived2.confirmations.is_empty()); } + #[test] + fn same_block_merge_of_two_live_ops_confirms_both_without_new_entries() { + // Two existing live entries own cmxs {A} and {B} respectively. A + // later scan sees a single same-block cluster whose visible cmxs + // are {A, B} (the documented same-block merge). The merged + // cluster's own id `H(A∪B)` matches NEITHER live id, but overlap + // dedupe catches both: no new (spuriously aggregate) entry, and a + // confirmation for each overlapped id at the observed height. + let a = [0xA1u8; 32]; + let b = [0xB2u8; 32]; + let id_a = compute_activity_id(&[a]); + let id_b = compute_activity_id(&[b]); + let mut existing = BTreeMap::new(); + existing.insert(a, id_a); + existing.insert(b, id_b); + + // Cluster at height H carrying both cmxs as own received notes. + let height = 500u64; + let input = ScanDeriveInput { + notes: vec![ + own_note(0xA1, 0x10, height, 1_000, false), + own_note(0xB2, 0x11, height, 2_000, false), + ], + outgoing: vec![], + own_addresses: vec![addr(0x01)], + }; + let derived = derive_activity_from_scan_data(&input, &existing); + assert!( + derived.new_entries.is_empty(), + "a same-block merge of two owned ops must not synthesize an aggregate row" + ); + // `confirmations` carries both overlapped ids at H (BTreeSet order: + // id_a sorts before id_b since 0xA1.. < 0xB2.. when hashed? — assert + // membership rather than order to stay robust). + assert_eq!(derived.confirmations.len(), 2); + assert!(derived.confirmations.contains(&(id_a, height))); + assert!(derived.confirmations.contains(&(id_b, height))); + } + // ── classification table (scan path) ─────────────────────────── #[test] @@ -710,7 +773,7 @@ mod tests { outgoing: vec![], own_addresses: vec![addr(0x01)], }; - let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; + let d = derive_activity_from_scan_data(&input, &BTreeMap::new()).new_entries; assert_eq!(d.len(), 1); assert_eq!(d[0].kind, ShieldedActivityKind::Received); assert_eq!(d[0].direction, ShieldedDirection::In); @@ -731,7 +794,7 @@ mod tests { outgoing: vec![outgoing(0x30, addr(0xEE), 200, 750, memo.clone())], own_addresses: vec![addr(0x01)], }; - let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; + let d = derive_activity_from_scan_data(&input, &BTreeMap::new()).new_entries; assert_eq!(d.len(), 1); assert_eq!(d[0].kind, ShieldedActivityKind::Sent); assert_eq!(d[0].direction, ShieldedDirection::Out); @@ -750,7 +813,7 @@ mod tests { outgoing: vec![outgoing(0x42, addr(0xEE), 300, 600, vec![])], // payment own_addresses: vec![addr(0x01)], }; - let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; + let d = derive_activity_from_scan_data(&input, &BTreeMap::new()).new_entries; assert_eq!(d.len(), 1); assert_eq!(d[0].kind, ShieldedActivityKind::Sent); assert_eq!(d[0].amount, 600, "amount is the payment, not the change"); @@ -770,7 +833,7 @@ mod tests { outgoing: vec![outgoing(0x50, own.clone(), 400, 0, vec![])], own_addresses: vec![own], }; - let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; + let d = derive_activity_from_scan_data(&input, &BTreeMap::new()).new_entries; let e = d .iter() .find(|e| e.block_height == Some(400)) @@ -789,7 +852,7 @@ mod tests { outgoing: vec![outgoing(0x60, addr(0xEE), 100, 500, vec![0u8; 36])], own_addresses: vec![addr(0x01)], }; - let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; + let d = derive_activity_from_scan_data(&input, &BTreeMap::new()).new_entries; assert_eq!(d.len(), 1); assert!(d[0].memo.is_none(), "an all-zero memo must surface as None"); } @@ -798,7 +861,7 @@ mod tests { fn empty_cluster_visible_set_is_skipped() { // No notes, no outgoing → no clusters → no entries. let input = ScanDeriveInput::default(); - let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; + let d = derive_activity_from_scan_data(&input, &BTreeMap::new()).new_entries; assert!(d.is_empty()); } @@ -814,7 +877,7 @@ mod tests { outgoing: vec![outgoing(0x72, addr(0xEE), 20, 800, vec![])], own_addresses: vec![addr(0x01)], }; - let mut d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; + let mut d = derive_activity_from_scan_data(&input, &BTreeMap::new()).new_entries; sort_activity_for_display(&mut d); assert_eq!(d.len(), 2); // After display sort, the more recent (h=20 Sent) comes first. @@ -833,17 +896,18 @@ mod tests { own_addresses: vec![addr(0x01)], }; let live_id = compute_activity_id(&[[0x70u8; 32]]); - let mut existing = BTreeSet::new(); - existing.insert(live_id); + let mut existing = BTreeMap::new(); + existing.insert([0x70u8; 32], live_id); let d = derive_activity_from_scan_data(&input, &existing); assert_eq!(d.new_entries.len(), 1); assert_eq!(d.new_entries[0].block_height, Some(20)); assert_eq!( d.confirmations, vec![(live_id, 10)], - "existing-id clusters must be reported as on-chain sightings \ - with their observed height (the Pending->Confirmed promise \ - the ambiguous post-broadcast paths rely on)" + "clusters overlapping an existing entry's cmxs must be reported \ + as on-chain sightings with their observed height (the \ + Pending->Confirmed promise the ambiguous post-broadcast paths \ + rely on)" ); } @@ -939,7 +1003,7 @@ mod tests { outgoing: vec![outgoing(2, own.clone(), 113, 39_787_148_800, vec![0u8; 36])], own_addresses: vec![own], }; - let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; + let d = derive_activity_from_scan_data(&input, &BTreeMap::new()).new_entries; let spend = d .iter() .find(|e| e.block_height == Some(113)) @@ -970,7 +1034,7 @@ mod tests { ], own_addresses: vec![own], }; - let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; + let d = derive_activity_from_scan_data(&input, &BTreeMap::new()).new_entries; let sent = d .iter() .find(|e| e.block_height == Some(20)) @@ -1000,7 +1064,7 @@ mod tests { outgoing: vec![outgoing(1, own.clone(), 30, 1_000_000, vec![0u8; 36])], own_addresses: vec![own], }; - let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; + let d = derive_activity_from_scan_data(&input, &BTreeMap::new()).new_entries; let e = d .iter() .find(|e| e.block_height == Some(30)) @@ -1022,7 +1086,7 @@ mod tests { outgoing: vec![outgoing(1, own.clone(), 30, 1_000_000, vec![0u8; 36])], own_addresses: vec![own], }; - let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; + let d = derive_activity_from_scan_data(&input, &BTreeMap::new()).new_entries; assert_eq!(d.len(), 1); assert_eq!(d[0].kind, ShieldedActivityKind::Received); assert_eq!(d[0].direction, ShieldedDirection::In); @@ -1040,7 +1104,7 @@ mod tests { outgoing: vec![], own_addresses: vec![own], }; - let d = derive_activity_from_scan_data(&input, &BTreeSet::new()).new_entries; + let d = derive_activity_from_scan_data(&input, &BTreeMap::new()).new_entries; assert_eq!(d.len(), 1); assert_eq!(d[0].kind, ShieldedActivityKind::Received); assert_eq!(d[0].direction, ShieldedDirection::In); diff --git a/packages/rs-platform-wallet/src/wallet/shielded/coordinator.rs b/packages/rs-platform-wallet/src/wallet/shielded/coordinator.rs index 8e6368029ff..a78932044e0 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/coordinator.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/coordinator.rs @@ -773,16 +773,23 @@ impl NetworkShieldedCoordinator { .map(|o| o.recipient.clone()) .collect(); - let existing_ids = store.get_activity_ids(*id).map_err(|e| { - crate::error::PlatformWalletError::ShieldedStoreError(e.to_string()) - })?; + // Map every stored entry's visible output cmx to the owning + // entry id, so the deriver can dedupe by cmx OVERLAP (not exact + // id): a same-block cluster that merges two live ops hashes to + // an id matching neither, but its cmxs still overlap both. + let existing_cmxs: BTreeMap<[u8; 32], [u8; 32]> = store + .get_activity(*id, 0, usize::MAX) + .map_err(|e| crate::error::PlatformWalletError::ShieldedStoreError(e.to_string()))? + .into_iter() + .flat_map(|entry| entry.note_cmxs.into_iter().map(move |c| (c, entry.id))) + .collect(); let input = ScanDeriveInput { notes, outgoing, own_addresses, }; - let derived = derive_activity_from_scan_data(&input, &existing_ids); + let derived = derive_activity_from_scan_data(&input, &existing_cmxs); for entry in derived.new_entries { store.save_activity(*id, &entry).map_err(|e| { crate::error::PlatformWalletError::ShieldedStoreError(e.to_string()) @@ -804,6 +811,16 @@ impl NetworkShieldedCoordinator { crate::error::PlatformWalletError::ShieldedStoreError(e.to_string()) })?; let Some(stored) = stored else { continue }; + // Chain truth wins: a row marked Failed by a client-side + // post-broadcast error whose outputs are later observed + // on-chain was not actually a failure — upgrade it. (We + // observed exactly this on devnet: the rc.1 result-proof + // fetch failure marked an actually-landed identity-create + // as failed; the cluster's cmxs appearing on-chain is + // ground truth that the operation executed.) The gate is + // therefore `Pending || block_height.is_none()`, which + // also catches those Failed-no-height rows; only a + // Confirmed-with-height row is final. let needs_upgrade = stored.status == super::activity::ShieldedActivityStatus::Pending || stored.block_height.is_none(); diff --git a/packages/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rs b/packages/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rs index 784472b03ce..50fb3169648 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rs @@ -123,6 +123,7 @@ impl PlatformWallet { #[allow(clippy::too_many_arguments)] pub async fn shielded_fund_from_asset_lock( &self, + coordinator: &std::sync::Arc, funding: AssetLockFunding, recipients: Vec<(OrchardAddress, Option)>, asset_lock_signer: &AS, @@ -408,7 +409,7 @@ impl PlatformWallet { // by construction). Recorded Confirmed directly — `broadcast_and_ // _wait` already proved inclusion. Best-effort: a recording miss // (no bound keyset, no recoverable output) just omits the row. - self.record_shield_from_asset_lock_activity(&landed_actions, shield_amount) + self.record_shield_from_asset_lock_activity(coordinator, &landed_actions, shield_amount) .await; // Step 5: cleanup. Consume the tracked asset lock. The @@ -518,6 +519,7 @@ impl PlatformWallet { #[cfg(feature = "shielded")] async fn record_shield_from_asset_lock_activity( &self, + coordinator: &std::sync::Arc, actions: &[dpp::shielded::SerializedAction], shield_amount: Credits, ) { @@ -568,11 +570,13 @@ impl PlatformWallet { let confirmed = with_status(&pending, ShieldedActivityStatus::Confirmed, None); let id = crate::wallet::shielded::SubwalletId::new(self.wallet_id(), account); crate::wallet::shielded::operations::queue_shielded_activity( + coordinator.store(), Some(self.persister()), self.wallet_id(), id, confirmed, - ); + ) + .await; } } diff --git a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs index 0cd124d3ab0..c01cce1d613 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs @@ -168,15 +168,29 @@ fn queue_shielded_changeset( } } -/// Queue a single activity entry to the host persister, for callers -/// outside this module (the Type 18 orchestrator in -/// `fund_from_asset_lock.rs`). Upserts by `entry.id`. -pub(super) fn queue_shielded_activity( +/// Write an activity entry to the in-memory store, then queue it to the +/// host persister. For callers outside this module (the Type 18 +/// orchestrator in `fund_from_asset_lock.rs`). Upserts by `entry.id`. +/// +/// The in-memory `save_activity` MUST happen before the persister queue so +/// the same-session scan deriver's dedupe (which reads the in-memory store) +/// sees this live row and never re-derives a coarser entry over it. A store +/// write failure is logged but does not fail the op — the persister still +/// gets the entry, and the next sync reconciles the in-memory side. +pub(super) async fn queue_shielded_activity( + store: &Arc>, persister: Option<&WalletPersister>, wallet_id: WalletId, id: SubwalletId, entry: super::activity::ShieldedActivityEntry, ) { + if let Err(e) = store.write().await.save_activity(id, &entry) { + warn!( + entry_id = %hex::encode(entry.id), + error = %e, + "live activity entry: in-memory save_activity failed; persister still queued" + ); + } queue_shielded_changeset(persister, wallet_id, changeset_for_entry(id, entry)); } @@ -209,14 +223,21 @@ fn shielded_actions(st: &StateTransition) -> &[dpp::shielded::SerializedAction] } } -/// Record a live activity entry built from `params` for `id`, shipping -/// it to the host persister via [`queue_shielded_changeset`]. Returns -/// the recorded entry so the caller can flip its status later (the +/// Record a live activity entry built from `params` for `id`, writing it +/// to the in-memory `store` and then shipping it to the host persister. +/// Returns the recorded entry so the caller can flip its status later (the /// status flip re-records by the same `entry.id`, an upsert). /// +/// The in-memory `save_activity` MUST land before the persister queue so +/// the same-session scan deriver's dedupe (which reads the IN-MEMORY store +/// — see `coordinator::derive_activity_into_changeset`) sees this live row +/// and never re-derives a coarser entry that `save_activity` would upsert +/// over it. See [`queue_shielded_activity`]. +/// /// Returns `None` (and queues nothing) when the bundle exposes no /// wallet-visible output cmx — see [`build_pending_entry`]. -fn record_pending_activity( +async fn record_pending_activity( + store: &Arc>, persister: Option<&WalletPersister>, wallet_id: WalletId, id: SubwalletId, @@ -241,15 +262,17 @@ fn record_pending_activity( cmxs = entry.note_cmxs.len(), "live activity entry recorded (pending)" ); - queue_shielded_changeset(persister, wallet_id, changeset_for_entry(id, entry.clone())); + queue_shielded_activity(store, persister, wallet_id, id, entry.clone()).await; Some(entry) } /// Re-record `entry` with a flipped status (and optional confirmed -/// height) for `id`. Upserts by `entry.id` at the persister, so a -/// `Pending` row becomes `Confirmed` / `Failed` in place. No-op when -/// `entry` is `None` (nothing was recorded for this operation). -fn record_activity_status( +/// height) for `id`. Upserts by `entry.id` in the in-memory store and at +/// the persister, so a `Pending` row becomes `Confirmed` / `Failed` in +/// place. No-op when `entry` is `None` (nothing was recorded for this +/// operation). +async fn record_activity_status( + store: &Arc>, persister: Option<&WalletPersister>, wallet_id: WalletId, id: SubwalletId, @@ -261,7 +284,7 @@ fn record_activity_status( return; }; let next = with_status(entry, status, block_height); - queue_shielded_changeset(persister, wallet_id, changeset_for_entry(id, next)); + queue_shielded_activity(store, persister, wallet_id, id, next).await; } // ------------------------------------------------------------------------- @@ -303,8 +326,9 @@ fn reserve_shield_fee_on_input_0( /// shielded pool, with the resulting note assigned to `account`'s /// default Orchard payment address derived from `keys`. #[allow(clippy::too_many_arguments)] -pub async fn shield, P: OrchardProver>( +pub async fn shield, P: OrchardProver>( sdk: &Arc, + store: &Arc>, persister: Option<&WalletPersister>, wallet_id: WalletId, keys: &OrchardKeySet, @@ -411,6 +435,7 @@ pub async fn shield, P: OrchardProver>( // which the scan later sees as an outgoing note recovered to self — // the ids line up. let pending_entry = record_pending_activity( + store, persister, wallet_id, id, @@ -425,7 +450,8 @@ pub async fn shield, P: OrchardProver>( actions: shielded_actions(&state_transition), spent_notes: &[], }, - ); + ) + .await; // Wait for proven execution (not just relay-ACK) so the host only // sees success once Platform has actually included the transition. A @@ -468,24 +494,28 @@ pub async fn shield, P: OrchardProver>( // `broadcast_and_wait` failed: shield holds no note reservation, // so there's nothing to roll back — just mark the activity Failed. record_activity_status( + store, persister, wallet_id, id, &pending_entry, ShieldedActivityStatus::Failed, None, - ); + ) + .await; return Err(e); } record_activity_status( + store, persister, wallet_id, id, &pending_entry, ShieldedActivityStatus::Confirmed, None, - ); + ) + .await; info!(account, credits = amount, "Shield broadcast succeeded"); Ok(()) } @@ -575,6 +605,7 @@ pub async fn unshield( // counterparty = the 21-byte serialized PlatformAddress, exact // fee = the builder's metered fee. pending_entry = record_pending_activity( + store, persister, wallet_id, id, @@ -589,7 +620,8 @@ pub async fn unshield( actions: shielded_actions(&state_transition), spent_notes: &selected_notes, }, - ); + ) + .await; trace!("Unshield: state transition built, broadcasting..."); broadcast_shielded_spend(sdk, &state_transition, "unshield").await @@ -599,13 +631,15 @@ pub async fn unshield( match result { Ok(()) => { record_activity_status( + store, persister, wallet_id, id, &pending_entry, ShieldedActivityStatus::Confirmed, None, - ); + ) + .await; // Broadcast already succeeded; spent-state bookkeeping is // best-effort. Surfacing a local write failure as a send // failure here would invite duplicate retries — the next @@ -651,13 +685,15 @@ pub async fn unshield( // Definitive failure: the spend never executed. Mark the // activity row Failed (upsert by id) before releasing notes. record_activity_status( + store, persister, wallet_id, id, &pending_entry, ShieldedActivityStatus::Failed, None, - ); + ) + .await; cancel_pending(store, id, &selected_notes).await; Err(e) } @@ -731,6 +767,7 @@ pub async fn transfer( // counterparty = the recipient's 43-byte raw Orchard address, memo // attached when non-zero. pending_entry = record_pending_activity( + store, persister, wallet_id, id, @@ -745,7 +782,8 @@ pub async fn transfer( actions: shielded_actions(&state_transition), spent_notes: &selected_notes, }, - ); + ) + .await; trace!("Shielded transfer: state transition built, broadcasting..."); broadcast_shielded_spend(sdk, &state_transition, "transfer").await @@ -755,13 +793,15 @@ pub async fn transfer( match result { Ok(()) => { record_activity_status( + store, persister, wallet_id, id, &pending_entry, ShieldedActivityStatus::Confirmed, None, - ); + ) + .await; // Best-effort post-broadcast bookkeeping (see unshield). if let Err(e) = finalize_pending(store, persister, wallet_id, id, &selected_notes).await { @@ -785,13 +825,15 @@ pub async fn transfer( Err(e @ PlatformWalletError::ShieldedSpendUnconfirmed { .. }) => Err(e), Err(e) => { record_activity_status( + store, persister, wallet_id, id, &pending_entry, ShieldedActivityStatus::Failed, None, - ); + ) + .await; cancel_pending(store, id, &selected_notes).await; Err(e) } @@ -877,6 +919,7 @@ pub async fn withdraw( // Live activity: a Withdrawal (direction out), counterparty = the // Core output script bytes, exact metered fee. pending_entry = record_pending_activity( + store, persister, wallet_id, id, @@ -891,7 +934,8 @@ pub async fn withdraw( actions: shielded_actions(&state_transition), spent_notes: &selected_notes, }, - ); + ) + .await; trace!("Shielded withdrawal: state transition built, broadcasting..."); broadcast_shielded_spend(sdk, &state_transition, "withdraw").await @@ -901,13 +945,15 @@ pub async fn withdraw( match result { Ok(()) => { record_activity_status( + store, persister, wallet_id, id, &pending_entry, ShieldedActivityStatus::Confirmed, None, - ); + ) + .await; // Best-effort post-broadcast bookkeeping (see unshield). if let Err(e) = finalize_pending(store, persister, wallet_id, id, &selected_notes).await { @@ -931,13 +977,15 @@ pub async fn withdraw( Err(e @ PlatformWalletError::ShieldedSpendUnconfirmed { .. }) => Err(e), Err(e) => { record_activity_status( + store, persister, wallet_id, id, &pending_entry, ShieldedActivityStatus::Failed, None, - ); + ) + .await; cancel_pending(store, id, &selected_notes).await; Err(e) } @@ -1058,6 +1106,7 @@ where // The output cmxs are taken from the bundle the transition is built // from (the change note re-entering the pool, if any). pending_entry = record_pending_activity( + store, persister, wallet_id, id, @@ -1074,7 +1123,8 @@ where actions: &build.bundle.actions, spent_notes: &selected_notes, }, - ); + ) + .await; trace!("IdentityCreateFromShieldedPool: built, broadcasting via SDK..."); // Stage the broadcast and the result-wait SEPARATELY (instead of one `broadcast_and_wait`) @@ -1235,13 +1285,15 @@ where match result { Ok((identity_id, identity)) => { record_activity_status( + store, persister, wallet_id, id, &pending_entry, ShieldedActivityStatus::Confirmed, None, - ); + ) + .await; // Best-effort post-broadcast bookkeeping (see `unshield`): mark the spent notes so the // local balance reflects the exit immediately; any drift heals on the next nullifier // sync. The on-chain nullifier set — not this local mark — is the authoritative @@ -1278,13 +1330,15 @@ where // Definitive failure: the identity was never created. Mark // the activity row Failed (upsert by id) before releasing. record_activity_status( + store, persister, wallet_id, id, &pending_entry, ShieldedActivityStatus::Failed, None, - ); + ) + .await; cancel_pending(store, id, &selected_notes).await; } Err(e) diff --git a/packages/rs-platform-wallet/src/wallet/shielded/seed_pool.rs b/packages/rs-platform-wallet/src/wallet/shielded/seed_pool.rs index f2d236c160b..2f43d7daf75 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/seed_pool.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/seed_pool.rs @@ -139,6 +139,7 @@ impl PlatformWallet { #[allow(clippy::too_many_arguments)] pub async fn shielded_seed_pool_notes( &self, + coordinator: &std::sync::Arc, wallet_id: &[u8; 32], account: u32, target_total_notes: u64, @@ -276,6 +277,7 @@ impl PlatformWallet { attempt += 1; match self .shielded_fund_from_asset_lock( + coordinator, funding, vec![(recipient, None)], asset_lock_signer, From d64de1673a258e122a24a460437573ad688f23e4 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 14:48:14 +0200 Subject: [PATCH 07/17] docs: drop stale doc fragment above decode_cmx_array Co-Authored-By: Claude Fable 5 --- packages/rs-platform-wallet-ffi/src/persistence.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index d83984842de..9b1de0040a1 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -2134,7 +2134,6 @@ impl PlatformWalletPersistence for FFIPersister { } } -/// Flatten an `AccountType` + encoded xpub into the C-flat /// Decode `count` contiguous 32-byte commitments / nullifiers from a /// host buffer into `Vec<[u8; 32]>`. A null pointer, a zero count, or a /// `count` whose byte length overflows / exceeds `isize::MAX` (the From 60b72599ef92fb353e75971711d6210532cfe62a Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 15:08:45 +0200 Subject: [PATCH 08/17] fix: stage Shield broadcast so ambiguous wait failures stay Pending MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shield (T15) now mirrors broadcast_shielded_spend's staging: broadcast first (a definitive CheckTx verdict marks the activity row Failed with the enriched address-funds diagnostics), then wait_for_response — a consensus rejection in the result is Failed, while an ambiguous wait failure (timeout / result-proof fetch error) leaves the row Pending and surfaces ShieldedSpendUnconfirmed, since the shield may still land and the scan's cmx-overlap confirmation flips the row when its note appears on-chain. Type 18 deliberately keeps recording only the landed bundle: the CL-height retry loop re-randomizes the bundle (different cmxs => a different activity id) per attempt, so a pre-broadcast Pending row would orphan whenever a retry lands; in-flight/failed T18s are surfaced via the tracked asset-lock lifecycle, which owns the recoverable L1 value. Rationale documented on the recorder. Co-Authored-By: Claude Fable 5 --- .../wallet/shielded/fund_from_asset_lock.rs | 12 ++ .../src/wallet/shielded/operations.rs | 131 ++++++++++++------ 2 files changed, 99 insertions(+), 44 deletions(-) diff --git a/packages/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rs b/packages/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rs index 50fb3169648..7b118c9125a 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rs @@ -508,6 +508,18 @@ impl PlatformWallet { /// Record a confirmed `ShieldFromAssetLock` (Type 18) activity entry /// over the landed bundle's `actions`. /// + /// Deliberately records ONLY the landed bundle (no Pending row before + /// broadcast, no Failed row after): `submit_with_cl_height_retry` + /// re-builds and re-randomizes the Orchard bundle on every attempt, + /// so each attempt has different output cmxs — and the activity id is + /// keyed to those cmxs. A pre-broadcast Pending row would orphan + /// (unconfirmable forever, its cmxs never on-chain) whenever a retry + /// is the attempt that lands. In-flight and failed Type 18s are + /// surfaced through the tracked asset-lock lifecycle instead + /// (Built/Broadcast/Locked/Consumed + the resumable-funding UI), + /// which tracks the L1 lock — the artifact that actually carries the + /// recoverable value on failure. + /// /// Best-effort and non-fatal: the broadcast already succeeded, so a /// recording miss (no bound shielded keyset, or no wallet-visible /// output cmx in the bundle) must not turn the funding into a diff --git a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs index c01cce1d613..22c1535ec92 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs @@ -457,53 +457,96 @@ pub async fn shield, P: OrchardPr // sees success once Platform has actually included the transition. A // DAPI-level ACK alone could otherwise mask a later Platform // rejection. The proven result is discarded; we only need the - // confirmation. Unlike the note-spending flows (unshield / transfer / - // withdraw — see `broadcast_shielded_spend`), shield deliberately - // keeps the one-shot helper: it takes no note reservation, so an - // ambiguous post-broadcast wait failure has no local state to strand. - // Its inputs are transparent address claims guarded by on-chain + // confirmation. Staged like `broadcast_shielded_spend`: only a + // DEFINITIVE verdict (CheckTx rejection / refused admission) marks + // the activity row Failed — an ambiguous wait failure (timeout, + // result-proof fetch error) leaves it Pending, because the shield + // may still land and the scan's cmx-overlap confirmation will flip + // the row when its note appears on-chain. Shield takes no note + // reservation, so the ambiguous arm strands no local spend state; + // its inputs are transparent address claims guarded by on-chain // nonces, and a host-level retry re-fetches those nonces. - let broadcast_result = state_transition - .broadcast_and_wait::(sdk, None) + let enrich = |e: &dash_sdk::Error| -> PlatformWalletError { + if let Some(rich) = addresses_not_enough_funds(e) { + let claimed = claimed_inputs + .iter() + .map(|(addr, (nonce, credits))| { + format!( + "{}=(nonce {nonce}, {credits} credits)", + addr.to_bech32m_string(network) + ) + }) + .collect::>() + .join(", "); + PlatformWalletError::ShieldedBroadcastFailed(format!( + "addresses not enough funds: required {} credits; \ + claimed inputs [{}]; platform sees [{}]", + rich.required_balance(), + claimed, + format_addresses_with_info(rich.addresses_with_info(), network), + )) + } else { + PlatformWalletError::ShieldedBroadcastFailed(e.to_string()) + } + }; + + match state_transition.broadcast(sdk, None).await { + Ok(()) => {} + Err(e) if broadcast_definitely_failed(&e) => { + record_activity_status( + store, + persister, + wallet_id, + id, + &pending_entry, + ShieldedActivityStatus::Failed, + None, + ) + .await; + return Err(enrich(&e)); + } + Err(e) => { + warn!( + account, + error = %e, + "Shield broadcast returned no verdict; the transition may have been \ + admitted — falling through to the result wait" + ); + } + } + + if let Err(wait_err) = state_transition + .wait_for_response::(sdk, None) .await - .map_err(|e| { - if let Some(rich) = addresses_not_enough_funds(&e) { - let claimed = claimed_inputs - .iter() - .map(|(addr, (nonce, credits))| { - format!( - "{}=(nonce {nonce}, {credits} credits)", - addr.to_bech32m_string(network) - ) - }) - .collect::>() - .join(", "); - PlatformWalletError::ShieldedBroadcastFailed(format!( - "addresses not enough funds: required {} credits; \ - claimed inputs [{}]; platform sees [{}]", - rich.required_balance(), - claimed, - format_addresses_with_info(rich.addresses_with_info(), network), - )) - } else { - PlatformWalletError::ShieldedBroadcastFailed(e.to_string()) - } + { + if carries_consensus_rejection(&wait_err) { + // A consensus verdict in the result: the shield definitively + // did not execute. + record_activity_status( + store, + persister, + wallet_id, + id, + &pending_entry, + ShieldedActivityStatus::Failed, + None, + ) + .await; + return Err(enrich(&wait_err)); + } + // Ambiguous: admitted but unconfirmed. Leave the activity row + // Pending — the scan's confirmation pass flips it when the + // shielded note lands on-chain. + warn!( + account, + error = %wait_err, + "Shield broadcast accepted but result confirmation failed; \ + leaving the activity row pending" + ); + return Err(PlatformWalletError::ShieldedSpendUnconfirmed { + operation: "shield", + reason: wait_err.to_string(), }); - - if let Err(e) = broadcast_result { - // `broadcast_and_wait` failed: shield holds no note reservation, - // so there's nothing to roll back — just mark the activity Failed. - record_activity_status( - store, - persister, - wallet_id, - id, - &pending_entry, - ShieldedActivityStatus::Failed, - None, - ) - .await; - return Err(e); } record_activity_status( From 419646223519432bd12a8565969fb0f2bb7006f2 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 15:16:04 +0200 Subject: [PATCH 09/17] fix: address review on activity sorting, FFI marshalling, and lock scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - sort_activity_for_display keys the pending band on STATUS (matching the Swift partition and the store contract): a live-Confirmed row whose height the scan hasn't backfilled sorts at the top of the settled band instead of masquerading as pending. Test extended. - The activity persist FFI builds each entry and its owned cmx / nullifier buffers in one structurally-paired pass, so the pointer-into-backing invariant can't silently mis-pair if either side is ever filtered or reordered. - derive_activity_into_changeset snapshots inputs under a read lock, classifies lock-free, and takes the write lock only for the upserts — the derivation is idempotent, so anything landing between passes reconciles on the next one. - non_zero_memo exists once (the deriver's slice-keyed helper); the recorder's fixed-array version is a thin adapter. Co-Authored-By: Claude Fable 5 --- .../rs-platform-wallet-ffi/src/persistence.rs | 46 +++++----- .../src/wallet/shielded/activity.rs | 54 +++++++---- .../src/wallet/shielded/activity_recorder.rs | 13 ++- .../src/wallet/shielded/coordinator.rs | 90 ++++++++++++------- 4 files changed, 126 insertions(+), 77 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index 9b1de0040a1..a81dbaa5c87 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -1330,12 +1330,24 @@ impl PlatformWalletPersistence for FFIPersister { // host upserts by `entry_id`. if !shielded_cs.activity_entries.is_empty() { if let Some(cb) = self.callbacks.on_persist_shielded_activity_fn { - // Concatenated cmx / nullifier buffers, kept alive for - // the callback window. One owned `Vec` per entry's - // note_cmxs and spent_nullifiers. - let mut backing: Vec<(Vec, Vec)> = Vec::new(); - for entries in shielded_cs.activity_entries.values() { - for e in entries { + // One pass pairs each entry with its owned cmx / + // nullifier buffers STRUCTURALLY (same tuple), so the + // pointer-into-backing invariant can't silently + // mis-pair if either side is ever filtered or + // reordered. The buffers live in `rows` (immutable + // once built) for the whole callback window; an inner + // `Vec`'s heap allocation is stable even as the + // outer Vec grows. + let rows: Vec<( + &platform_wallet::wallet::shielded::SubwalletId, + &platform_wallet::wallet::shielded::ShieldedActivityEntry, + Vec, + Vec, + )> = shielded_cs + .activity_entries + .iter() + .flat_map(|(id, entries)| entries.iter().map(move |e| (id, e))) + .map(|(id, e)| { let mut cmx_buf = Vec::with_capacity(e.note_cmxs.len() * 32); for c in &e.note_cmxs { cmx_buf.extend_from_slice(c); @@ -1344,20 +1356,12 @@ impl PlatformWalletPersistence for FFIPersister { for n in &e.spent_nullifiers { nf_buf.extend_from_slice(n); } - backing.push((cmx_buf, nf_buf)); - } - } - let mut backing_iter = backing.iter(); - let entries: Vec = shielded_cs - .activity_entries - .iter() - .flat_map(|(id, entries)| { - entries.iter().map(move |e| (id, e)) + (id, e, cmx_buf, nf_buf) }) - .map(|(id, e)| { - let (cmx_buf, nf_buf) = backing_iter - .next() - .expect("backing has one entry per activity entry"); + .collect(); + let entries: Vec = rows + .iter() + .map(|(id, e, cmx_buf, nf_buf)| { let (identity_id, has_identity_id) = match &e.kind { platform_wallet::wallet::shielded::ShieldedActivityKind::IdentityCreate { identity_id, @@ -1427,9 +1431,9 @@ impl PlatformWalletPersistence for FFIPersister { ); round_success = false; } - // `backing` and `entries` drop here, after the callback + // `rows` and `entries` drop here, after the callback // has copied everything it needs. - drop(backing); + drop(rows); } } } diff --git a/packages/rs-platform-wallet/src/wallet/shielded/activity.rs b/packages/rs-platform-wallet/src/wallet/shielded/activity.rs index 73726955cc7..984d80ec519 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/activity.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/activity.rs @@ -601,7 +601,7 @@ pub fn derive_activity_from_scan_data( /// Return `Some(memo)` when `memo` is non-empty and not all-zero; /// `None` otherwise. A zero-filled 36-byte `DashMemo` is the "no memo" /// sentinel and shouldn't surface as an attached memo. -fn non_zero_memo(memo: &[u8]) -> Option> { +pub(crate) fn non_zero_memo(memo: &[u8]) -> Option> { if memo.is_empty() || memo.iter().all(|&b| b == 0) { None } else { @@ -609,19 +609,30 @@ fn non_zero_memo(memo: &[u8]) -> Option> { } } -/// Sort entries for display: confirmed/failed by `block_height` -/// descending with pendings (no height) floated to the very top, then -/// tiebreak by `created_at_ms` descending, then by `id` for a total -/// order. Mutates in place. +/// Sort entries for display: `Pending` STATUS rows float to the very +/// top, then confirmed/failed by `block_height` descending (height-less +/// rows — live successes whose height the scan hasn't backfilled yet — +/// above heighted ones), then tiebreak by `created_at_ms` descending, +/// then by `id` for a total order. Mutates in place. +/// +/// Pending is a status, not "missing height": the live recorder flips +/// successful ops to `Confirmed` with `block_height: None` and the scan +/// fills the height later, so keying the pending band on height would +/// misfile the common success shape (the Swift UI partitions by status +/// for the same reason). pub fn sort_activity_for_display(entries: &mut [ShieldedActivityEntry]) { entries.sort_by(|a, b| { - // Pendings (block_height == None) first. Among heights, larger - // (more recent) first. - match (a.block_height, b.block_height) { - (None, Some(_)) => std::cmp::Ordering::Less, - (Some(_), None) => std::cmp::Ordering::Greater, - (Some(ah), Some(bh)) => bh.cmp(&ah), - (None, None) => std::cmp::Ordering::Equal, + let a_pending = a.status == ShieldedActivityStatus::Pending; + let b_pending = b.status == ShieldedActivityStatus::Pending; + match (a_pending, b_pending) { + (true, false) => std::cmp::Ordering::Less, + (false, true) => std::cmp::Ordering::Greater, + _ => match (a.block_height, b.block_height) { + (None, Some(_)) => std::cmp::Ordering::Less, + (Some(_), None) => std::cmp::Ordering::Greater, + (Some(ah), Some(bh)) => bh.cmp(&ah), + (None, None) => std::cmp::Ordering::Equal, + }, } // Tiebreak: more recent record time first. .then_with(|| b.created_at_ms.cmp(&a.created_at_ms)) @@ -933,17 +944,30 @@ mod tests { note_cmxs: vec![[id; 32]], spent_nullifiers: vec![], }; + // A live success: Confirmed but the scan hasn't backfilled the + // height yet. Must NOT land in the pending band (status is the + // discriminator), but sorts above heighted settled rows. + let mut confirmed_no_height = mk(None, 4, 5); + confirmed_no_height.status = ShieldedActivityStatus::Confirmed; + let mut v = vec![ mk(Some(100), 1, 1), mk(None, 5, 2), // pending — must float to top mk(Some(300), 2, 3), mk(Some(200), 3, 4), + confirmed_no_height, ]; sort_activity_for_display(&mut v); + assert_eq!(v[0].status, ShieldedActivityStatus::Pending); assert_eq!(v[0].block_height, None, "pending floats to top"); - assert_eq!(v[1].block_height, Some(300)); - assert_eq!(v[2].block_height, Some(200)); - assert_eq!(v[3].block_height, Some(100)); + assert_eq!( + (v[1].status, v[1].block_height), + (ShieldedActivityStatus::Confirmed, None), + "confirmed-without-height sorts at the top of the settled band, not as pending" + ); + assert_eq!(v[2].block_height, Some(300)); + assert_eq!(v[3].block_height, Some(200)); + assert_eq!(v[4].block_height, Some(100)); } #[test] diff --git a/packages/rs-platform-wallet/src/wallet/shielded/activity_recorder.rs b/packages/rs-platform-wallet/src/wallet/shielded/activity_recorder.rs index 1be1fda427f..33d0c59bca2 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/activity_recorder.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/activity_recorder.rs @@ -150,15 +150,12 @@ pub fn with_status( } /// Convert a fixed 36-byte memo into `Some(memo)` when it carries -/// content, or `None` when it's the all-zero "no memo" sentinel. Shared -/// by the operation paths so a zero memo never surfaces as an attached -/// note. +/// content, or `None` when it's the all-zero "no memo" sentinel. Thin +/// fixed-array adapter over the scan deriver's slice-keyed helper so +/// the sentinel logic exists exactly once and the live and scan paths +/// can never drift. pub fn non_zero_memo(memo: &[u8; 36]) -> Option> { - if memo.iter().all(|&b| b == 0) { - None - } else { - Some(memo.to_vec()) - } + super::activity::non_zero_memo(&memo[..]) } /// Ship one entry to the host persister through a fresh diff --git a/packages/rs-platform-wallet/src/wallet/shielded/coordinator.rs b/packages/rs-platform-wallet/src/wallet/shielded/coordinator.rs index a78932044e0..189403b48ab 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/coordinator.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/coordinator.rs @@ -753,43 +753,67 @@ impl NetworkShieldedCoordinator { ) -> Result<(), crate::error::PlatformWalletError> { use super::activity::{derive_activity_from_scan_data, ScanDeriveInput}; - let mut store = self.store.write().await; for (id, views) in subwallets { - let notes = store.get_all_notes(*id).map_err(|e| { - crate::error::PlatformWalletError::ShieldedStoreError(e.to_string()) - })?; - let outgoing = store.get_outgoing_notes(*id).map_err(|e| { - crate::error::PlatformWalletError::ShieldedStoreError(e.to_string()) - })?; - if notes.is_empty() && outgoing.is_empty() { - continue; - } + // Read pass: snapshot the inputs under a shared lock, then + // release it before the CPU-bound classification — holding + // the WRITE lock across the whole derivation would serialize + // every other store consumer for the full window. The + // derivation is idempotent (overlap dedupe + upsert-by-id), + // so anything that lands between the read and write passes + // is reconciled on the next pass. + let (input, existing_cmxs) = { + let store = self.store.read().await; + let notes = store.get_all_notes(*id).map_err(|e| { + crate::error::PlatformWalletError::ShieldedStoreError(e.to_string()) + })?; + let outgoing = store.get_outgoing_notes(*id).map_err(|e| { + crate::error::PlatformWalletError::ShieldedStoreError(e.to_string()) + })?; + if notes.is_empty() && outgoing.is_empty() { + continue; + } - // Own-recipient set: an outgoing note whose recipient the - // subwallet's IVK recognizes is self-change, not a payment out. - let own_addresses: Vec> = outgoing - .iter() - .filter(|o| is_own_orchard_recipient(views, &o.recipient)) - .map(|o| o.recipient.clone()) - .collect(); - - // Map every stored entry's visible output cmx to the owning - // entry id, so the deriver can dedupe by cmx OVERLAP (not exact - // id): a same-block cluster that merges two live ops hashes to - // an id matching neither, but its cmxs still overlap both. - let existing_cmxs: BTreeMap<[u8; 32], [u8; 32]> = store - .get_activity(*id, 0, usize::MAX) - .map_err(|e| crate::error::PlatformWalletError::ShieldedStoreError(e.to_string()))? - .into_iter() - .flat_map(|entry| entry.note_cmxs.into_iter().map(move |c| (c, entry.id))) - .collect(); - - let input = ScanDeriveInput { - notes, - outgoing, - own_addresses, + // Own-recipient set: an outgoing note whose recipient the + // subwallet's IVK recognizes is self-change, not a payment + // out. + let own_addresses: Vec> = outgoing + .iter() + .filter(|o| is_own_orchard_recipient(views, &o.recipient)) + .map(|o| o.recipient.clone()) + .collect(); + + // Map every stored entry's visible output cmx to the owning + // entry id, so the deriver can dedupe by cmx OVERLAP (not + // exact id): a same-block cluster that merges two live ops + // hashes to an id matching neither, but its cmxs still + // overlap both. + let existing_cmxs: BTreeMap<[u8; 32], [u8; 32]> = store + .get_activity(*id, 0, usize::MAX) + .map_err(|e| { + crate::error::PlatformWalletError::ShieldedStoreError(e.to_string()) + })? + .into_iter() + .flat_map(|entry| entry.note_cmxs.into_iter().map(move |c| (c, entry.id))) + .collect(); + + ( + ScanDeriveInput { + notes, + outgoing, + own_addresses, + }, + existing_cmxs, + ) }; + + // Lock-free classification. let derived = derive_activity_from_scan_data(&input, &existing_cmxs); + if derived.new_entries.is_empty() && derived.confirmations.is_empty() { + continue; + } + + // Write pass: only the upserts hold the write lock. + let mut store = self.store.write().await; for entry in derived.new_entries { store.save_activity(*id, &entry).map_err(|e| { crate::error::PlatformWalletError::ShieldedStoreError(e.to_string()) From 50b98ae900ea560032dd88d2b5205745f1ec48e7 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 15:52:27 +0200 Subject: [PATCH 10/17] fix: per-batch note heights and stale-snapshot races in activity recording - Stamp incoming notes with their own chunk's proven height (matching OVK-recovered outgoing notes) instead of the pass-wide max, so a bundle's change and send agree on the cluster key in multi-batch syncs - Re-check cmx overlap under the write lock before inserting scan-derived entries; overlaps with rows a live recorder landed mid-derivation degrade to confirmation sightings instead of clobbering/duplicating - record_activity_status now flips the CURRENT stored row (read under the save's write lock), never the stale pre-broadcast capture; a row the scan already confirmed with a height is left untouched Co-Authored-By: Claude Fable 5 --- .../src/wallet/shielded/activity.rs | 7 +- .../src/wallet/shielded/coordinator.rs | 37 ++++- .../src/wallet/shielded/operations.rs | 132 +++++++++++++++++- .../src/wallet/shielded/store.rs | 5 +- .../src/wallet/shielded/sync.rs | 19 ++- 5 files changed, 185 insertions(+), 15 deletions(-) diff --git a/packages/rs-platform-wallet/src/wallet/shielded/activity.rs b/packages/rs-platform-wallet/src/wallet/shielded/activity.rs index 984d80ec519..220ff54b50c 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/activity.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/activity.rs @@ -306,8 +306,11 @@ fn note_rho(note_data: &[u8]) -> Option<[u8; 32]> { /// Cluster a subwallet's events by block height. /// -/// Receipts and sends carry a real `block_height` from the scan, so they -/// cluster cleanly. Spends are the hard case: a note's stored +/// Receipts and sends carry a `block_height` from the scan — the proven +/// height of the chunk that surfaced them, stamped per-batch on BOTH +/// sides (see `ShieldedNote::block_height`) so a bundle's incoming +/// change and OVK-recovered send agree on the key and cluster +/// together. Spends are the hard case: a note's stored /// `block_height` is its *receipt* height, and scan-based spend /// detection flips `is_spent` without recording *when* it was spent /// (the spend height isn't persisted anywhere the wallet layer can read diff --git a/packages/rs-platform-wallet/src/wallet/shielded/coordinator.rs b/packages/rs-platform-wallet/src/wallet/shielded/coordinator.rs index 189403b48ab..7459fe575d7 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/coordinator.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/coordinator.rs @@ -757,10 +757,9 @@ impl NetworkShieldedCoordinator { // Read pass: snapshot the inputs under a shared lock, then // release it before the CPU-bound classification — holding // the WRITE lock across the whole derivation would serialize - // every other store consumer for the full window. The - // derivation is idempotent (overlap dedupe + upsert-by-id), - // so anything that lands between the read and write passes - // is reconciled on the next pass. + // every other store consumer for the full window. Anything a + // live recorder lands between the two passes is caught by the + // overlap re-check under the write lock below. let (input, existing_cmxs) = { let store = self.store.read().await; let notes = store.get_all_notes(*id).map_err(|e| { @@ -814,7 +813,35 @@ impl NetworkShieldedCoordinator { // Write pass: only the upserts hold the write lock. let mut store = self.store.write().await; + // Re-check cmx overlap against the CURRENT activity rows + // before inserting: a live recorder may have written a richer + // row (kind / fee / memo / created identity id) for the same + // cmx set between the read snapshot and here. Saving the + // scan-derived entry anyway would either clobber that row + // (id collision) or duplicate it (id mismatch), and scan-only + // data can never reconstruct the lost live fields. Overlapped + // clusters degrade to confirmation sightings instead — same + // treatment the classifier gives overlaps it can see. + let current_cmxs: BTreeMap<[u8; 32], [u8; 32]> = store + .get_activity(*id, 0, usize::MAX) + .map_err(|e| crate::error::PlatformWalletError::ShieldedStoreError(e.to_string()))? + .into_iter() + .flat_map(|entry| entry.note_cmxs.into_iter().map(move |c| (c, entry.id))) + .collect(); + let mut confirmations = derived.confirmations; for entry in derived.new_entries { + let overlapped: std::collections::BTreeSet<[u8; 32]> = entry + .note_cmxs + .iter() + .filter_map(|c| current_cmxs.get(c)) + .copied() + .collect(); + if !overlapped.is_empty() { + if let Some(height) = entry.block_height { + confirmations.extend(overlapped.into_iter().map(|eid| (eid, height))); + } + continue; + } store.save_activity(*id, &entry).map_err(|e| { crate::error::PlatformWalletError::ShieldedStoreError(e.to_string()) })?; @@ -828,7 +855,7 @@ impl NetworkShieldedCoordinator { // upgrade rewrites the STORED entry via `with_status`, so the // live entry's richer fields (kind / fee / memo / // counterparty) survive untouched. - for (entry_id, height) in derived.confirmations { + for (entry_id, height) in confirmations { let stored = store .get_activity_by_entry_id(*id, &entry_id) .map_err(|e| { diff --git a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs index 22c1535ec92..a0ef0d83349 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs @@ -271,6 +271,14 @@ async fn record_pending_activity( /// the persister, so a `Pending` row becomes `Confirmed` / `Failed` in /// place. No-op when `entry` is `None` (nothing was recorded for this /// operation). +/// +/// The flip is applied to the CURRENT stored row (read under the same +/// write lock as the save), not the captured pre-broadcast entry: a +/// concurrent scan pass can upgrade the row to `Confirmed` at a real +/// height between broadcast and the result-wait, and rewriting the stale +/// capture would erase that scan-learned height. A row the scan already +/// confirmed WITH a height is chain truth and is left untouched +/// entirely — nothing the result-wait knows can improve on it. async fn record_activity_status( store: &Arc>, persister: Option<&WalletPersister>, @@ -283,8 +291,28 @@ async fn record_activity_status( let Some(entry) = entry else { return; }; - let next = with_status(entry, status, block_height); - queue_shielded_activity(store, persister, wallet_id, id, next).await; + let next = { + let mut store = store.write().await; + let stored = store + .get_activity_by_entry_id(id, &entry.id) + .unwrap_or_default(); + let base = stored.as_ref().unwrap_or(entry); + if base.status == ShieldedActivityStatus::Confirmed && base.block_height.is_some() { + return; + } + // `with_status` keeps `base.block_height` when `block_height` is + // `None`, so a height the scan populated survives the flip. + let next = with_status(base, status, block_height); + if let Err(e) = store.save_activity(id, &next) { + warn!( + entry_id = %hex::encode(next.id), + error = %e, + "live activity status flip: in-memory save_activity failed; persister still queued" + ); + } + next + }; + queue_shielded_changeset(persister, wallet_id, changeset_for_entry(id, next)); } // ------------------------------------------------------------------------- @@ -2167,3 +2195,103 @@ mod note_reservation_release_tests { } } } + +#[cfg(test)] +mod record_activity_status_tests { + use super::*; + use crate::wallet::shielded::activity::{ + ShieldedActivityEntry, ShieldedActivityKind, ShieldedDirection, + }; + use crate::wallet::shielded::store::InMemoryShieldedStore; + + fn sub() -> SubwalletId { + SubwalletId::new([0xCC; 32], 0) + } + + /// The Pending entry a live recorder captures before broadcast. + fn captured_pending() -> ShieldedActivityEntry { + ShieldedActivityEntry { + id: [0xAA; 32], + kind: ShieldedActivityKind::Shield, + direction: ShieldedDirection::In, + amount: 1_000, + fee: Some(10), + counterparty: None, + memo: None, + block_height: None, + status: ShieldedActivityStatus::Pending, + created_at_ms: 1, + note_cmxs: vec![[0x01; 32]], + spent_nullifiers: vec![], + } + } + + /// A scan pass that confirmed the row at a real height between the + /// broadcast and the result-wait must win over the post-wait flip: + /// the stale captured entry must not overwrite the stored + /// `Confirmed`-with-height row (neither downgrading it to `Failed` + /// nor erasing the scan-learned height). + #[tokio::test] + async fn flip_does_not_clobber_scan_confirmed_row() { + let store = Arc::new(RwLock::new(InMemoryShieldedStore::new())); + let id = sub(); + let pending = captured_pending(); + let scan_confirmed = with_status(&pending, ShieldedActivityStatus::Confirmed, Some(777)); + store + .write() + .await + .save_activity(id, &scan_confirmed) + .unwrap(); + + record_activity_status( + &store, + None, + id.wallet_id, + id, + &Some(pending), + ShieldedActivityStatus::Failed, + None, + ) + .await; + + let stored = store + .read() + .await + .get_activity_by_entry_id(id, &[0xAA; 32]) + .unwrap() + .expect("row must still exist"); + assert_eq!(stored.status, ShieldedActivityStatus::Confirmed); + assert_eq!(stored.block_height, Some(777)); + } + + /// No concurrent scan: the flip applies to the stored Pending row + /// (and falls back to the captured entry when the store has none), + /// writing the new status into the in-memory store. + #[tokio::test] + async fn flip_applies_when_row_is_still_pending() { + let store = Arc::new(RwLock::new(InMemoryShieldedStore::new())); + let id = sub(); + let pending = captured_pending(); + store.write().await.save_activity(id, &pending).unwrap(); + + record_activity_status( + &store, + None, + id.wallet_id, + id, + &Some(pending), + ShieldedActivityStatus::Confirmed, + Some(900), + ) + .await; + + let stored = store + .read() + .await + .get_activity_by_entry_id(id, &[0xAA; 32]) + .unwrap() + .expect("row must exist"); + assert_eq!(stored.status, ShieldedActivityStatus::Confirmed); + assert_eq!(stored.block_height, Some(900)); + } +} diff --git a/packages/rs-platform-wallet/src/wallet/shielded/store.rs b/packages/rs-platform-wallet/src/wallet/shielded/store.rs index 4d6595d63e0..e57ca6d92c1 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/store.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/store.rs @@ -67,7 +67,10 @@ pub struct ShieldedNote { pub cmx: [u8; 32], /// Nullifier for detecting when spent (32 bytes). pub nullifier: [u8; 32], - /// Block height where the note appeared. + /// Proven platform height of the chunk fetch that surfaced the note. + /// Stamped per-batch — the SAME height OVK-recovered outgoing notes + /// from that chunk get — so the activity deriver can cluster one + /// bundle's incoming change and outgoing send together by height. pub block_height: u64, /// Whether the nullifier was seen on-chain (spent). pub is_spent: bool, diff --git a/packages/rs-platform-wallet/src/wallet/shielded/sync.rs b/packages/rs-platform-wallet/src/wallet/shielded/sync.rs index b18136a85b7..e8109356b80 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/sync.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/sync.rs @@ -369,9 +369,6 @@ pub(super) async fn sync_notes_across( // — drives the watermark advance and the host-visible scan volume, // exactly as `result.total_notes_scanned` did in the one-shot path. let mut total_notes_scanned: u64 = 0; - // Max block height across batches and the resume point for - // `next_start_index` semantics, accumulated as batches arrive. - let mut max_block_height: u64 = 0; // Mirrors the one-shot rewind rule: track the last non-empty // batch's `(start_index, is_partial)` so we can warn on a // rewind-to-zero just like before. @@ -396,7 +393,6 @@ pub(super) async fn sync_notes_across( while let Some(item) = stream.next().await { let batch = item.map_err(|e| PlatformWalletError::ShieldedSyncFailed(e.to_string()))?; - max_block_height = max_block_height.max(batch.block_height); // The denominator arrives with the batch (extracted from the // note-fetch proof). It is stable across a sync; take the max-seen // so a late chunk proven at a slightly higher block never lowers @@ -462,6 +458,7 @@ pub(super) async fn sync_notes_across( position: dn.position, cmx: dn.cmx, note: dn.note, + block_height: batch.block_height, }); } for (id, views) in subwallets.iter().skip(1) { @@ -479,6 +476,7 @@ pub(super) async fn sync_notes_across( position, cmx: cmx_bytes, note, + block_height: batch.block_height, }); } } @@ -601,12 +599,18 @@ pub(super) async fn sync_notes_across( "Note DECRYPTED" ); let note_data = serialize_note(&d.note); + // Stamp the note with ITS chunk's proven height — the same + // per-batch height OVK-recovered outgoing notes get below — + // never a pass-wide max. The activity deriver clusters + // incoming and outgoing events by this height, so a bundle's + // change note and its OVK-recovered send must carry the same + // value or the bundle splits across two clusters. let shielded_note = super::store::ShieldedNote { note_data, position: d.position, cmx: d.cmx, nullifier: nullifier.to_bytes(), - block_height: max_block_height, + block_height: d.block_height, is_spent: false, value, }; @@ -796,6 +800,11 @@ struct DiscoveredNote { position: u64, cmx: [u8; 32], note: OrchardNote, + /// Proven platform height of the chunk that surfaced this note — + /// per-batch, matching [`RecoveredOutgoing::block_height`], so that + /// one bundle's incoming change and OVK-recovered send carry the + /// same height and cluster together in the activity deriver. + block_height: u64, } /// One outgoing (sent) note recovered via OVK during a sync pass. From f1ce3f7baf856bb5efed4fecb248aa7813fae07f Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 16:30:09 +0200 Subject: [PATCH 11/17] fix: preserve shield retry-safety code over FFI and purge activity rows on wipe - Route the shield FFI result through map_spend_result so hosts keep the ErrorShieldedSpendUnconfirmed (do-not-retry) / ErrorShieldedBroadcastFailed (safe-to-retry) split instead of a generic ErrorWalletOperation - Delete PersistentShieldedActivity in Clear Shielded State and in the wallet-deletion cascade so wiped history can't rehydrate into Rust via the load callback as ghost entries - Soften ShieldedSpendUnconfirmed wording to cover the shield path (no spent notes to reconcile) and document that cluster heights are fetch proof anchor heights (same fetch batch), incl. cold-restore aggregation Co-Authored-By: Claude Fable 5 --- .../src/shielded_send.rs | 16 +++----- packages/rs-platform-wallet/src/error.rs | 22 ++++++----- .../src/wallet/shielded/activity.rs | 38 +++++++++++++------ .../PlatformWalletPersistenceHandler.swift | 14 ++++++- .../Core/Services/ShieldedService.swift | 1 + 5 files changed, 57 insertions(+), 34 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/shielded_send.rs b/packages/rs-platform-wallet-ffi/src/shielded_send.rs index 268ba56e303..1efd3d4961a 100644 --- a/packages/rs-platform-wallet-ffi/src/shielded_send.rs +++ b/packages/rs-platform-wallet-ffi/src/shielded_send.rs @@ -426,10 +426,10 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_withdraw( map_spend_result(result, "shielded withdraw") } -/// Map a shielded spend outcome (unshield / transfer / withdraw) to a typed -/// FFI result, mirroring the identity-create sibling's code split so hosts -/// can tell "definitively failed, safe to retry" from "may have executed, -/// do NOT retry". +/// Map a shielded operation outcome (shield / unshield / transfer / +/// withdraw) to a typed FFI result, mirroring the identity-create sibling's +/// code split so hosts can tell "definitively failed, safe to retry" from +/// "may have executed, do NOT retry". fn map_spend_result( result: Result<(), PlatformWalletError>, operation: &str, @@ -716,13 +716,7 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_shield( ) .await }); - if let Err(e) = result { - return PlatformWalletFFIResult::err( - PlatformWalletFFIResultCode::ErrorWalletOperation, - format!("shielded shield failed: {e}"), - ); - } - PlatformWalletFFIResult::ok() + map_spend_result(result, "shielded shield") } /// Fund the shielded pool from a Core L1 asset lock, orchestrated diff --git a/packages/rs-platform-wallet/src/error.rs b/packages/rs-platform-wallet/src/error.rs index 598b1b42838..189772d10e0 100644 --- a/packages/rs-platform-wallet/src/error.rs +++ b/packages/rs-platform-wallet/src/error.rs @@ -202,21 +202,23 @@ pub enum PlatformWalletError { reason: String, }, - /// A shielded spend transition (`operation` is `"unshield"`, `"transfer"` or `"withdraw"`) was - /// **broadcast and accepted by the relay**, but the SDK could not confirm its execution result - /// (the result-proof fetch/verify failed — e.g. a transient DAPI/proof error or timeout, not a - /// platform rejection). The spend may already be executed on chain, so the spent notes' - /// reservations are intentionally left in place rather than released — releasing them would - /// invite re-selecting notes whose nullifiers may already be consumed. The next nullifier sync - /// (or an app restart, since reservations are in-memory only) reconciles them. `reason` carries - /// the underlying SDK error for diagnostics. + /// A shielded transition (`operation` is `"shield"`, `"unshield"`, `"transfer"` or + /// `"withdraw"`) was **broadcast and accepted by the relay**, but the SDK could not confirm + /// its execution result (the result-proof fetch/verify failed — e.g. a transient DAPI/proof + /// error or timeout, not a platform rejection). The operation may already be executed on + /// chain, so re-submitting risks a double-execution. For the spend-based operations the spent + /// notes' reservations are intentionally left in place rather than released — releasing them + /// would invite re-selecting notes whose nullifiers may already be consumed; a shield spends + /// no notes, so it has nothing reserved. The next sync (or an app restart, since reservations + /// are in-memory only) reconciles the outcome either way. `reason` carries the underlying SDK + /// error for diagnostics. /// /// The identity-create sibling is [`Self::ShieldedBroadcastUnconfirmed`], which additionally /// carries the derived identity id so the caller can hold the registration slot. #[error( "Shielded {operation} broadcast succeeded but its execution result could not be \ - confirmed; the spend may already be executed on chain — do not re-submit \ - (the next sync reconciles the spent notes): {reason}" + confirmed; it may already be executed on chain — do not re-submit \ + (the next sync reconciles the outcome): {reason}" )] ShieldedSpendUnconfirmed { operation: &'static str, diff --git a/packages/rs-platform-wallet/src/wallet/shielded/activity.rs b/packages/rs-platform-wallet/src/wallet/shielded/activity.rs index 220ff54b50c..fd6f08b739b 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/activity.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/activity.rs @@ -254,12 +254,25 @@ pub struct DerivedActivity { pub confirmations: Vec<([u8; 32], u64)>, } -/// One block-height cluster of a subwallet's shielded events. +/// One height-keyed cluster of a subwallet's shielded events. /// -/// Documented limitation (acceptable for v1): two same-wallet bundles -/// landing in the *same block* merge into one cluster — height is the -/// only join key available client-side without a note→transition index -/// (which Option B forbids). +/// The key is NOT a per-note mined height — the proven note data carries +/// none. It is the fetch chunk's proof anchor height (the chain tip the +/// response was proven at), stamped per-batch on incoming and outgoing +/// notes alike, so "same height" really means "surfaced by the same +/// fetch batch". +/// +/// Documented limitation (acceptable for v1): events that share a batch +/// merge into one cluster — the anchor height is the only join key +/// available client-side without a note→transition index (which Option B +/// forbids). For live syncing this approximates "same block" well (a +/// near-tip pass fetches few new notes per chunk), but on a cold restore +/// one chunk can span many historical blocks of unrelated operations, +/// which then merge into a single synthetic entry with combined +/// amount/kind/memo. Live-recorded entries are protected from this by +/// the cmx-overlap dedupe; only restore-derived history aggregates. The +/// real fix needs a per-note mined height in the note-fetch proof +/// (node-side change). #[derive(Debug, Clone, Default)] struct HeightCluster { /// Own new notes that first appeared at this height. @@ -304,13 +317,16 @@ fn note_rho(note_data: &[u8]) -> Option<[u8; 32]> { Some(rho) } -/// Cluster a subwallet's events by block height. +/// Cluster a subwallet's events by stored `block_height`. /// -/// Receipts and sends carry a `block_height` from the scan — the proven -/// height of the chunk that surfaced them, stamped per-batch on BOTH -/// sides (see `ShieldedNote::block_height`) so a bundle's incoming -/// change and OVK-recovered send agree on the key and cluster -/// together. Spends are the hard case: a note's stored +/// Receipts and sends carry a `block_height` from the scan — the fetch +/// chunk's proof ANCHOR height, not a per-note mined height (none +/// exists in the proven data), stamped per-batch on BOTH sides (see +/// `ShieldedNote::block_height`) so a bundle's incoming change and +/// OVK-recovered send agree on the key and cluster together. "Same +/// height" therefore means "same fetch batch"; see [`HeightCluster`] +/// for the cold-restore aggregation this implies. Spends are the hard +/// case: a note's stored /// `block_height` is its *receipt* height, and scan-based spend /// detection flips `is_spent` without recording *when* it was spent /// (the spend height isn't persisted anywhere the wallet layer can read diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift index 9d321628810..33fbcc6735d 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift @@ -3175,12 +3175,15 @@ public class PlatformWalletPersistenceHandler { } } - // Shielded (Orchard) per-wallet state. These three + // Shielded (Orchard) per-wallet state. These four // tables are keyed by raw `walletId` (no relationship // to `PersistentWallet`), so the wallet-row delete // below does not cascade them — purge them explicitly // or they leak after a wipe and could resurface / - // mis-attribute if the same `walletId` is reimported. + // mis-attribute if the same `walletId` is reimported + // (activity rows rehydrate into Rust via the + // `on_load_shielded_activity_fn` callback as ghost + // history and suppress fresh scan-derived entries). let shieldedNoteDescriptor = FetchDescriptor( predicate: #Predicate { $0.walletId == walletId } ) @@ -3202,6 +3205,13 @@ public class PlatformWalletPersistenceHandler { backgroundContext.delete(row) } + let shieldedActivityDescriptor = FetchDescriptor( + predicate: #Predicate { $0.walletId == walletId } + ) + for row in try backgroundContext.fetch(shieldedActivityDescriptor) { + backgroundContext.delete(row) + } + if let walletRow = walletRow { backgroundContext.delete(walletRow) } diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/ShieldedService.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/ShieldedService.swift index 02016e61b40..f60eb18becb 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/ShieldedService.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/ShieldedService.swift @@ -620,6 +620,7 @@ class ShieldedService: ObservableObject { try modelContext.delete(model: PersistentShieldedNote.self) try modelContext.delete(model: PersistentShieldedOutgoingNote.self) try modelContext.delete(model: PersistentShieldedSyncState.self) + try modelContext.delete(model: PersistentShieldedActivity.self) try modelContext.save() } catch { lastError = "Failed to wipe persisted shielded state: \(error.localizedDescription)" From ac2f2b4bb436f88d3777de482e3a7096f4f73857 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 16:48:16 +0200 Subject: [PATCH 12/17] docs: cover the shield path in map_spend_result's unconfirmed-arm comment Co-Authored-By: Claude Fable 5 --- packages/rs-platform-wallet-ffi/src/shielded_send.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/shielded_send.rs b/packages/rs-platform-wallet-ffi/src/shielded_send.rs index 1efd3d4961a..a7c9a0283e5 100644 --- a/packages/rs-platform-wallet-ffi/src/shielded_send.rs +++ b/packages/rs-platform-wallet-ffi/src/shielded_send.rs @@ -437,9 +437,11 @@ fn map_spend_result( match result { Ok(()) => PlatformWalletFFIResult::ok(), // Ambiguous: the broadcast was accepted but its execution result - // couldn't be confirmed. The notes stay reserved wallet-side and the - // next nullifier sync (or an app restart) reconciles them; the typed - // Display already carries the operation name and guidance. + // couldn't be confirmed — the host must NOT re-submit. For the + // spend-based operations the notes stay reserved wallet-side; a + // shield reserves nothing. Either way the next sync (or an app + // restart) reconciles the outcome; the typed Display already + // carries the operation name and guidance. Err(e @ PlatformWalletError::ShieldedSpendUnconfirmed { .. }) => { PlatformWalletFFIResult::err( PlatformWalletFFIResultCode::ErrorShieldedSpendUnconfirmed, From 75406ebc912949ac2e907c3653ba0b715da624f4 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 17:11:53 +0200 Subject: [PATCH 13/17] fix: key activity rows by model identity, not entryId The activity table is uniqued on (walletId, accountIndex, entryId), so a wallet-level list can hold the same entryId under two accounts (e.g. an intra-wallet transfer's Sent + Received rows). Keying ForEach by entryId alone made SwiftUI drop/reuse rows. Also neutralize the spend-only wording on the Swift mirrors of result code 18 now that shield routes through the same path. Co-Authored-By: Claude Fable 5 --- .../PlatformWalletManagerShieldedSync.swift | 7 ++++++ .../PlatformWallet/PlatformWalletResult.swift | 22 ++++++++++--------- .../Core/ViewModels/SendViewModel.swift | 15 +++++++------ .../Core/Views/ShieldedActivityView.swift | 10 +++++++-- 4 files changed, 35 insertions(+), 19 deletions(-) diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedSync.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedSync.swift index 501a29a7155..7d2429b56c4 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedSync.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedSync.swift @@ -568,6 +568,13 @@ extension PlatformWalletManager { /// /// Heavy CPU work (Halo 2 proof + per-input signing) runs on a /// detached task so the caller's actor isn't blocked. + /// + /// Throws `PlatformWalletError.shieldedSpendUnconfirmed` when the + /// broadcast was accepted but its execution result couldn't be + /// confirmed — the shield may already be on chain, so the caller + /// must NOT retry (a retry would rebuild the bundle and could + /// double-shield; the next sync reconciles the outcome). A shield + /// spends no notes, so nothing is reserved wallet-side. public func shieldedShield( walletId: Data, shieldedAccount: UInt32 = 0, diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift index 453cfd1f2de..2c311f91e9d 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift @@ -31,12 +31,13 @@ public enum PlatformWalletResultCode: Int32, Sendable { /// the derived id into `outIdentityId` on this code, so the caller can /// hold the slot rather than treat the registration as failed. case errorShieldedBroadcastUnconfirmed = 17 - /// A shielded spend (unshield / transfer / withdrawal) was accepted by - /// the relay but its execution result could not be confirmed. The spend - /// may have executed on chain and the wallet keeps the notes reserved - /// until the next sync (or app restart) reconciles them. Do NOT - /// auto-retry — a retry would select different notes and could - /// double-send if the original spend landed. + /// A shielded operation (shield / unshield / transfer / withdrawal) was + /// accepted by the relay but its execution result could not be + /// confirmed. It may have executed on chain; for the spend-based + /// operations the wallet keeps the notes reserved (a shield reserves + /// nothing) until the next sync (or app restart) reconciles the + /// outcome. Do NOT auto-retry — a retry would rebuild the bundle and + /// could double-execute if the original landed. case errorShieldedSpendUnconfirmed = 18 case notFound = 98 case errorUnknown = 99 @@ -170,10 +171,11 @@ public enum PlatformWalletError: LocalizedError { /// `outIdentityId`) before falling back to this error — see /// `ShieldedIdentityCreateUnconfirmedError`. case shieldedBroadcastUnconfirmed(String) - /// A shielded spend (unshield / transfer / withdrawal) was accepted by - /// the relay but its execution result could not be confirmed. The spend - /// may have executed; the notes stay reserved wallet-side until the next - /// sync reconciles them. Do NOT auto-retry. + /// A shielded operation (shield / unshield / transfer / withdrawal) + /// was accepted by the relay but its execution result could not be + /// confirmed. It may have executed; spend-based operations keep their + /// notes reserved wallet-side (a shield reserves nothing) until the + /// next sync reconciles the outcome. Do NOT auto-retry. case shieldedSpendUnconfirmed(String) case notFound(String) case unknown(String) diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/ViewModels/SendViewModel.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/ViewModels/SendViewModel.swift index 45ecece47a1..d1e4770b438 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/ViewModels/SendViewModel.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/ViewModels/SendViewModel.swift @@ -500,13 +500,14 @@ class SendViewModel: ObservableObject { } } catch PlatformWalletError.shieldedSpendUnconfirmed { - // The shielded spend (unshield / transfer / withdraw) was broadcast - // and accepted, but its execution result couldn't be confirmed — it - // may already be on chain. Rust intentionally KEEPS the spent notes' - // reservations, so this must NOT be presented as a retryable failure: - // retrying would select other unreserved notes and double-send the - // payment. Surface it through the non-error (success) path so the UI - // doesn't invite a retry; the next shielded sync reconciles the notes. + // The shielded operation (shield / unshield / transfer / withdraw) + // was broadcast and accepted, but its execution result couldn't be + // confirmed — it may already be on chain. Rust intentionally KEEPS + // any spent notes' reservations, so this must NOT be presented as a + // retryable failure: retrying would rebuild the bundle and could + // double-execute if the original landed. Surface it through the + // non-error (success) path so the UI doesn't invite a retry; the + // next shielded sync reconciles the outcome. successMessage = "Transaction may have gone through — waiting for " + "the next shielded sync to confirm. Do not retry." } catch { diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/ShieldedActivityView.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/ShieldedActivityView.swift index 27fb8d98e2e..5b41da8ef2c 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/ShieldedActivityView.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/ShieldedActivityView.swift @@ -159,14 +159,20 @@ struct ShieldedActivityListView: View { description: Text("Shielded sends, receives, and other private operations appear here.") ) } + // Key rows by MODEL identity, not `entryId`: the table is + // uniqued on (walletId, accountIndex, entryId), so one + // wallet-level list can legitimately hold the same entryId + // twice — e.g. an intra-wallet transfer writes a Sent row on + // the sending account and a Received row on the receiving + // account for the same visible outputs. if !pending.isEmpty { Section("Pending") { - ForEach(pending, id: \.entryId) { row($0) } + ForEach(pending, id: \.persistentModelID) { row($0) } } } if !settled.isEmpty { Section("History") { - ForEach(settled, id: \.entryId) { row($0) } + ForEach(settled, id: \.persistentModelID) { row($0) } } } } From 433f0992b0f6f9b40dfad189191d262cca8fc3dc Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 17:29:31 +0200 Subject: [PATCH 14/17] docs: activity FFI rows are keyed by (wallet_id, account_index, entry_id) Co-Authored-By: Claude Fable 5 --- .../src/shielded_persistence.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/shielded_persistence.rs b/packages/rs-platform-wallet-ffi/src/shielded_persistence.rs index 9b01a85cd57..a69c2e43d4c 100644 --- a/packages/rs-platform-wallet-ffi/src/shielded_persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/shielded_persistence.rs @@ -103,11 +103,15 @@ pub struct ShieldedSyncedIndexFFI { /// One derived shielded-activity entry for the host to persist. /// /// Mirror of `platform_wallet::wallet::shielded::ShieldedActivityEntry`. -/// The host writes one row keyed by `entry_id` (sha256 of the visible -/// output cmxs); re-persisting the same `entry_id` is an upsert that -/// refines the row (Pending→Confirmed/Failed, or a scan-derived -/// `ShieldedSpend` upgraded to a richer kind). All pointers are valid -/// only for the callback window — the host must copy. +/// The host writes one row keyed by `(wallet_id, account_index, +/// entry_id)` — NOT `entry_id` alone: the same entry id (sha256 of the +/// visible output cmxs) legitimately appears under two accounts of one +/// wallet, e.g. an intra-wallet transfer producing a Sent row on the +/// sending account and a Received row on the receiving account. +/// Re-persisting the same tuple is an upsert that refines the row +/// (Pending→Confirmed/Failed, or a scan-derived `ShieldedSpend` +/// upgraded to a richer kind). All pointers are valid only for the +/// callback window — the host must copy. /// /// `Option` fields are flattened to a value + a `has_*` flag (`u8`, /// 1 = present) rather than a sentinel, so `0`/empty is unambiguous. @@ -117,7 +121,8 @@ pub struct ShieldedActivityFFI { pub wallet_id: [u8; 32], /// ZIP-32 account index. pub account_index: u32, - /// Entry id (sha256 of sorted visible output cmxs). Primary key. + /// Entry id (sha256 of sorted visible output cmxs). Unique only + /// within `(wallet_id, account_index)` — see the struct doc. pub entry_id: [u8; 32], /// Kind discriminant (see `ShieldedActivityKind::tag`): /// 0 Shield, 1 ShieldFromAssetLock, 2 Received, 3 Sent, 4 Unshield, From b4479ff921407841caa72c8af0a10b1b12e079da Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 17:48:16 +0200 Subject: [PATCH 15/17] docs: align activity-key docs with the (wallet_id, account_index, entry_id) tuple Co-Authored-By: Claude Fable 5 --- .../rs-platform-wallet-ffi/src/persistence.rs | 8 ++++--- .../Models/PersistentShieldedActivity.swift | 23 +++++++++++-------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index a81dbaa5c87..be99cecfd6d 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -355,9 +355,11 @@ pub struct PersistenceCallbacks { ) -> i32, >, /// Persist a batch of derived activity-log entries. The host upserts - /// each by `entry_id` (Pending→Confirmed/Failed flips and scan-kind - /// refinements re-emit the same id). Mirrors the other - /// `on_persist_shielded_*` callbacks. + /// each by `(wallet_id, account_index, entry_id)` — `entry_id` alone + /// is not globally unique across accounts (see [`ShieldedActivityFFI`]). + /// Pending→Confirmed/Failed flips and scan-kind refinements re-emit + /// the same tuple. Mirrors the other `on_persist_shielded_*` + /// callbacks. #[cfg(feature = "shielded")] pub on_persist_shielded_activity_fn: Option< unsafe extern "C" fn( diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentShieldedActivity.swift b/packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentShieldedActivity.swift index 16926f9c4c2..2331f000483 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentShieldedActivity.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentShieldedActivity.swift @@ -6,17 +6,21 @@ import SwiftData /// identity-create, …). /// /// Mirrors `platform_wallet::wallet::shielded::ShieldedActivityEntry` -/// from the Rust side. Written two ways, both keyed by `entryId`: +/// from the Rust side. Written two ways, both scoped by +/// `(walletId, accountIndex, entryId)`: /// - **live** — each shielded operation records a rich entry at execution /// time (exact kind, amount, fee, counterparty, memo, identity id); /// - **scan-derived** — on a restored wallet the sync pass reconstructs /// entries best-effort from persisted notes / outgoing notes. /// -/// The persister callback upserts by `entryId`, so a `Pending` row flips -/// to `Confirmed`/`Failed` in place, and a coarse scan-derived -/// `ShieldedSpend` can be refined to a specific kind when a richer entry -/// re-emits the same id (the id = sha256 of the visible output cmxs is -/// identical across both paths by construction). +/// The persister callback upserts by that tuple — `entryId` alone is not +/// globally unique across accounts (an intra-wallet transfer writes a +/// Sent row on the sending account and a Received row on the receiving +/// account sharing one `entryId`). Re-persisting the same tuple flips a +/// `Pending` row to `Confirmed`/`Failed` in place, and a coarse +/// scan-derived `ShieldedSpend` can be refined to a specific kind when a +/// richer entry re-emits the same id (the id = sha256 of the visible +/// output cmxs is identical across both paths by construction). /// /// On cold start the matching `loadShieldedActivity` callback streams /// every row back to Rust so the scan deriver's dedupe set includes it @@ -24,9 +28,10 @@ import SwiftData @Model public final class PersistentShieldedActivity { /// Composite uniqueness on `(walletId, accountIndex, entryId)` — at - /// most one activity row per operation. `entryId` alone is the - /// natural key (it's a content hash), but scoping by subwallet - /// matches how the row is addressed on both persist and load paths. + /// most one activity row per operation PER subwallet. The subwallet + /// scope is required, not cosmetic: `entryId` is a content hash that + /// two accounts of one wallet legitimately share (intra-wallet + /// transfer → Sent on the sender, Received on the receiver). #Unique([\.walletId, \.accountIndex, \.entryId]) /// Index `(walletId, accountIndex)` so per-subwallet history scans /// hit an index instead of the full table. From e3cddf8cb9a37a7d789bd7e2fbfa94189eb59507 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 18:11:11 +0200 Subject: [PATCH 16/17] fix: reject out-of-range activity tags on load and skip status flip on read error - Activity restore now converts kind/direction/status with UInt8(exactly:) and drops the row on overflow, so corrupt persisted tags can't wrap into a different valid discriminant and bypass Rust's unknown-tag fallback - record_activity_status skips the flip (warn + return) when the store read fails instead of falling back to the stale pre-broadcast capture, which would have been exactly the clobber the merge exists to prevent - Align the dispatch call-site comment with the composite upsert key Co-Authored-By: Claude Fable 5 --- .../rs-platform-wallet-ffi/src/persistence.rs | 2 +- .../src/wallet/shielded/operations.rs | 20 ++++++++++++++++--- .../PlatformWalletPersistenceHandler.swift | 16 ++++++++++++--- 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index be99cecfd6d..a1e37a3655a 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -1329,7 +1329,7 @@ impl PlatformWalletPersistence for FFIPersister { // arrays) borrow into `backing`, a Vec of owned byte // buffers that outlives the callback — same pointer-validity // discipline as `note_data_ptr` / `memo_ptr` above. The - // host upserts by `entry_id`. + // host upserts by `(wallet_id, account_index, entry_id)`. if !shielded_cs.activity_entries.is_empty() { if let Some(cb) = self.callbacks.on_persist_shielded_activity_fn { // One pass pairs each entry with its owned cmx / diff --git a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs index a0ef0d83349..c376c99a78c 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs @@ -293,9 +293,23 @@ async fn record_activity_status( }; let next = { let mut store = store.write().await; - let stored = store - .get_activity_by_entry_id(id, &entry.id) - .unwrap_or_default(); + // A read FAILURE is not "no row": falling back to the captured + // entry on Err would queue exactly the stale overwrite this + // function exists to prevent, precisely when the invariant + // couldn't be checked. Skip the flip instead — the scan + // confirmation path reconciles the row's status later. + let stored = match store.get_activity_by_entry_id(id, &entry.id) { + Ok(stored) => stored, + Err(e) => { + warn!( + entry_id = %hex::encode(entry.id), + error = %e, + "live activity status flip: get_activity_by_entry_id failed; \ + skipping flip to avoid clobbering a richer row" + ); + return; + } + }; let base = stored.as_ref().unwrap_or(entry); if base.status == ShieldedActivityStatus::Confirmed && base.block_height.is_some() { return; diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift index 33fbcc6735d..95e2047a508 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift @@ -2809,6 +2809,16 @@ public class PlatformWalletPersistenceHandler { var written = 0 for row in rows { guard row.walletId.count == 32, row.entryId.count == 32 else { continue } + // Exact conversion, not truncating: a stored tag outside + // u8 range (corruption / unmigrated future tag) must NOT + // wrap into a different valid-looking discriminant (256 → + // 0 = Shield/In/Pending) — that would bypass Rust's + // unknown-tag fallback. Drop the row instead. + guard let kindTagU8 = UInt8(exactly: row.kindTag), + let directionU8 = UInt8(exactly: row.direction), + let statusU8 = UInt8(exactly: row.status) else { + continue + } // Per-row variable-length buffers: counterparty, memo, // noteCmxs, spentNullifiers. Allocate each and retain a @@ -2837,9 +2847,9 @@ public class PlatformWalletPersistenceHandler { wallet_id: walletIdTuple, account_index: row.accountIndex, entry_id: entryIdTuple, - kind_tag: UInt8(truncatingIfNeeded: row.kindTag), - direction: UInt8(truncatingIfNeeded: row.direction), - status: UInt8(truncatingIfNeeded: row.status), + kind_tag: kindTagU8, + direction: directionU8, + status: statusU8, amount: row.amount, fee: row.fee, has_fee: row.hasFee ? 1 : 0, From 123ebbd0320db0132f8be4c7863eec88d34b0258 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Sat, 13 Jun 2026 14:33:35 +0200 Subject: [PATCH 17/17] fix(rs-sdk-ffi): stop labeling unclassified SDK errors as "failed to fetch balances" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The catch-all arm of the FFIError::SDKError -> DashSDKError classifier prefixed every unclassified error with a copy-pasted "Failed to fetch balances: " string and mapped it to NetworkError. Any error that didn't match the timeout/connection/DAPI/protocol/not-found heuristics (e.g. a getDataContractHistory proof-verification failure) surfaced to the UI as "Internal Error: Failed to fetch balances: Proof verification error: ..." — both the prefix and the network classification were wrong. Pass the original message through unchanged and map to InternalError, matching the existing pass-through arms (protocol/not found/timeout). Added a regression test covering the catch-all. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/rs-sdk-ffi/src/error.rs | 38 +++++++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/packages/rs-sdk-ffi/src/error.rs b/packages/rs-sdk-ffi/src/error.rs index f2e5f22a479..bdc58e52819 100644 --- a/packages/rs-sdk-ffi/src/error.rs +++ b/packages/rs-sdk-ffi/src/error.rs @@ -143,11 +143,13 @@ impl From for DashSDKError { } else if error_str.contains("not found") || error_str.contains("Not found") { (DashSDKErrorCode::NotFound, error_str) } else { - // Default to network error with the original message - ( - DashSDKErrorCode::NetworkError, - format!("Failed to fetch balances: {}", error_str), - ) + // Unclassified SDK error: pass the original message through + // unchanged and map to InternalError rather than guessing a + // network cause. (Previously this hardcoded a "Failed to fetch + // balances:" prefix and the NetworkError code, mislabeling + // unrelated failures such as proof-verification errors from + // getDataContractHistory.) + (DashSDKErrorCode::InternalError, error_str) }; (code, detailed_msg) @@ -253,4 +255,30 @@ mod tests { let err = dash_sdk::Error::Generic("identity not found".to_string()); assert_eq!(classify(err), DashSDKErrorCode::NotFound); } + + #[test] + fn unclassified_error_maps_to_internal_error_without_balance_prefix() { + // A proof-verification failure (e.g. from getDataContractHistory) matches + // none of the substring heuristics and must fall through the catch-all. + // It should be classified as InternalError and keep its original Display + // verbatim — no copy-pasted "Failed to fetch balances:" prefix. + let err = dash_sdk::Error::Generic( + "Proof verification error: corrupted element for the historical contract".to_string(), + ); + // The catch-all passes the SDK error's Display through unchanged. + let expected = err.to_string(); + + let dash_sdk_error: DashSDKError = FFIError::SDKError(err).into(); + let rendered = unsafe { + let m = std::ffi::CStr::from_ptr(dash_sdk_error.message) + .to_string_lossy() + .into_owned(); + let _ = CString::from_raw(dash_sdk_error.message); + m + }; + + assert_eq!(dash_sdk_error.code, DashSDKErrorCode::InternalError); + assert_eq!(rendered, expected); + assert!(!rendered.contains("Failed to fetch balances")); + } }