From 67d5adae022e2e7e766c26f442ba62498c595f43 Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Wed, 9 Sep 2026 12:03:52 +0200 Subject: [PATCH 01/16] refactor(platform-wallet): remove transfer_with_change_address and FeeStrategyByAddress MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `transfer_with_change_address` and the whole address-keyed fee-strategy machinery (`FeeStrategyByAddress`, `FeeStrategyStepByAddress`, `FeeStrategyResolveError`) had no consumer — not in rs-platform-wallet-ffi, swift-sdk, kotlin-sdk or rs-unified-sdk-jni. The FFI entry point `platform_address_wallet_transfer` only ever calls `transfer`. Going with them: `validate_change_address`, `augment_outputs_with_change`, `checked_sum_credits`, the `ChangeBelowMinimumOutput` error variant (never mapped in ffi/error.rs), and ~480 lines of tests that covered only this path. `InputSumOverflow` stays — platform_wallet.rs still uses it. The `transfer` doc loses its "When to use this vs transfer_with_change_address" section. Co-Authored-By: Claude Opus 5 --- packages/rs-platform-wallet/src/error.rs | 13 - .../src/wallet/platform_addresses/mod.rs | 18 +- .../src/wallet/platform_addresses/transfer.rs | 890 +----------------- 3 files changed, 3 insertions(+), 918 deletions(-) diff --git a/packages/rs-platform-wallet/src/error.rs b/packages/rs-platform-wallet/src/error.rs index d3ab952c93d..0f65ea69bae 100644 --- a/packages/rs-platform-wallet/src/error.rs +++ b/packages/rs-platform-wallet/src/error.rs @@ -682,19 +682,6 @@ pub enum PlatformWalletError { min_input_amount: Credits, }, - #[error( - "change output amount {change_amount} is below the protocol per-output \ - minimum {min_output_amount}; raise the input sum or drop the change \ - address so the residual would exceed the minimum" - )] - ChangeBelowMinimumOutput { - /// `Σ inputs − Σ user_outputs` — the residual that would have been - /// routed to the change output. - change_amount: Credits, - /// Per-output minimum from the active platform version. - min_output_amount: Credits, - }, - #[error("input sum overflow: caller-supplied input balances exceed u64::MAX")] InputSumOverflow, diff --git a/packages/rs-platform-wallet/src/wallet/platform_addresses/mod.rs b/packages/rs-platform-wallet/src/wallet/platform_addresses/mod.rs index 786c5c1ae56..4f427087eb4 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_addresses/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_addresses/mod.rs @@ -18,11 +18,7 @@ mod withdrawal; /// Saturating sum over `Credits` (== `u64`) — total credit supply is far /// below `u64::MAX`, so saturation is unreachable in practice but the policy -/// keeps debug-build panics off the table. Use this only for sums over -/// wallet-derived balances; for caller-supplied input maps prefer -/// [`checked_sum_credits`] so a bogus FFI input is reported as -/// [`crate::PlatformWalletError::InputSumOverflow`] rather than silently -/// saturating to `u64::MAX`. +/// keeps debug-build panics off the table. pub(crate) fn saturating_sum_credits(iter: I) -> Credits where I: IntoIterator, @@ -30,18 +26,6 @@ where iter.into_iter().fold(0u64, Credits::saturating_add) } -/// Checked sum over `Credits` for caller-supplied input maps. Returns -/// [`crate::PlatformWalletError::InputSumOverflow`] on overflow so a -/// bogus FFI caller cannot trigger a silent saturation downstream. -pub(crate) fn checked_sum_credits(iter: I) -> Result -where - I: IntoIterator, -{ - iter.into_iter() - .try_fold(0u64, |acc, c| acc.checked_add(c)) - .ok_or(crate::PlatformWalletError::InputSumOverflow) -} - pub use provider::{ PerAccountPlatformAddressState, PerWalletPlatformAddressState, PlatformAddressTag, }; diff --git a/packages/rs-platform-wallet/src/wallet/platform_addresses/transfer.rs b/packages/rs-platform-wallet/src/wallet/platform_addresses/transfer.rs index 3d5fb91113d..7f2d1bd1466 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_addresses/transfer.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_addresses/transfer.rs @@ -13,125 +13,8 @@ use dash_sdk::platform::transition::transfer_address_funds::TransferAddressFunds use dash_sdk::platform::FetchMany; use dash_sdk::query_types::AddressInfo; +use super::saturating_sum_credits; pub use super::InputSelection; -use super::{checked_sum_credits, saturating_sum_credits}; - -/// Address-keyed step in a fee strategy. Resolves to an -/// [`AddressFundsFeeStrategyStep`] by looking up the named address in the -/// final inputs / outputs maps that the signer will see. -#[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub enum FeeStrategyStepByAddress { - /// Deduct fee from the named input address's remaining balance. - DeductFromInputAddress(PlatformAddress), - /// Reduce the named output address's credited amount by the fee. - ReduceOutputAddress(PlatformAddress), -} - -/// Address-keyed analogue of [`AddressFundsFeeStrategy`]. -/// -/// Used by [`PlatformAddressWallet::transfer_with_change_address`] so -/// callers identify the fee-bearing row by address rather than by index -/// into the post-canonicalisation `BTreeMap`. Lowered to the consensus -/// [`AddressFundsFeeStrategy`] inside the wrapper, AFTER augmentation, -/// when the final outputs map is known. -#[derive(Debug, Clone, PartialEq, Eq, Default)] -pub struct FeeStrategyByAddress(pub Vec); - -/// Errors specific to lowering [`FeeStrategyByAddress`] to -/// [`AddressFundsFeeStrategy`]. Folds into -/// [`PlatformWalletError::AddressOperation`]. -#[derive(Debug, thiserror::Error)] -pub enum FeeStrategyResolveError { - #[error("DeductFromInputAddress: address {0:?} not present in inputs map")] - InputAddressNotFound(PlatformAddress), - #[error("ReduceOutputAddress: address {0:?} not present in outputs map")] - OutputAddressNotFound(PlatformAddress), - #[error("{kind} index {index} exceeds u16::MAX (BTreeMap holds > 65535 entries)")] - IndexOverflow { kind: &'static str, index: usize }, -} - -// INTENTIONAL(CMT-004): FeeStrategyResolveError variants flatten into -// PlatformWalletError::AddressOperation(String) rather than being promoted -// to typed PlatformWalletError variants. Rationale: fee-strategy resolution -// errors are caller-input errors (caller named an address not in the -// inputs/outputs map they themselves supplied) distinct from wallet-state -// errors like ChangeBelowMinimumOutput / InputSumOverflow / -// OnlyOutputAddressesFunded. Keeping the wallet error enum lean by -// flattening caller-input mistakes into a string is the deliberate trade-off. -impl From for PlatformWalletError { - fn from(e: FeeStrategyResolveError) -> Self { - PlatformWalletError::AddressOperation(e.to_string()) - } -} - -impl FeeStrategyByAddress { - pub fn new() -> Self { - Self(Vec::new()) - } - - /// Convenience: target an input address for fee deduction. - pub fn deduct_from_input(addr: PlatformAddress) -> Self { - Self(vec![FeeStrategyStepByAddress::DeductFromInputAddress(addr)]) - } - - /// Convenience: target an output address (typically the change output) - /// for fee reduction. - pub fn reduce_output(addr: PlatformAddress) -> Self { - Self(vec![FeeStrategyStepByAddress::ReduceOutputAddress(addr)]) - } - - /// Lower to consensus-indexed [`AddressFundsFeeStrategy`]. - /// - /// `inputs` and `outputs` MUST be the FINAL maps the signer will use — - /// i.e. for outputs, the post-`augment_outputs_with_change` map - /// containing the change row when applicable. Calling this before - /// augmentation will produce incorrect indexes. - pub fn to_indexed( - &self, - inputs: &BTreeMap, - outputs: &BTreeMap, - ) -> Result { - self.0 - .iter() - .map(|step| step.to_indexed(inputs, outputs)) - .collect() - } -} - -impl FeeStrategyStepByAddress { - pub fn to_indexed( - &self, - inputs: &BTreeMap, - outputs: &BTreeMap, - ) -> Result { - match self { - FeeStrategyStepByAddress::DeductFromInputAddress(addr) => { - let pos = inputs - .keys() - .position(|k| k == addr) - .ok_or(FeeStrategyResolveError::InputAddressNotFound(*addr))?; - let idx = - u16::try_from(pos).map_err(|_| FeeStrategyResolveError::IndexOverflow { - kind: "input", - index: pos, - })?; - Ok(AddressFundsFeeStrategyStep::DeductFromInput(idx)) - } - FeeStrategyStepByAddress::ReduceOutputAddress(addr) => { - let pos = outputs - .keys() - .position(|k| k == addr) - .ok_or(FeeStrategyResolveError::OutputAddressNotFound(*addr))?; - let idx = - u16::try_from(pos).map_err(|_| FeeStrategyResolveError::IndexOverflow { - kind: "output", - index: pos, - })?; - Ok(AddressFundsFeeStrategyStep::ReduceOutput(idx)) - } - } - } -} impl PlatformAddressWallet { /// Transfer credits between platform addresses. @@ -148,21 +31,6 @@ impl PlatformAddressWallet { /// [`PlatformAddress`]es; the wallet itself holds no key material — /// callers supply a seed-backed, hardware, or FFI-trampoline signer. /// - /// # When to use this vs `transfer_with_change_address` - /// - /// - `transfer()` — simple entry point with an **indexed** - /// [`AddressFundsFeeStrategy`]. Supports all three [`InputSelection`] - /// variants. Use when you can supply fee-strategy indices directly - /// against the canonical lex-sorted outputs map, or when you want - /// auto-selection. - /// - [`Self::transfer_with_change_address`] — wrapper that takes an - /// **address-keyed** [`FeeStrategyByAddress`] and optionally routes - /// surplus from explicit inputs to a change output. Use when you want - /// to identify the fee-bearing row by address (the wrapper resolves to - /// indices after change-augmentation) and/or want the wallet to compute - /// the change amount from over-funded explicit inputs. Rejects - /// [`InputSelection::Auto`]. - /// /// # Invariant: `Σ inputs == Σ outputs` /// /// The protocol enforces strict equality. Responsibility for satisfying @@ -173,9 +41,7 @@ impl PlatformAddressWallet { /// under Auto: `[DeductFromInput(0)]` and `[ReduceOutput(0)]` only. /// - [`InputSelection::Explicit`] / [`InputSelection::ExplicitWithNonces`]: /// **caller** must construct the inputs map such that the sum equals - /// `Σ outputs`. For automatic surplus routing to a change output, use - /// [`Self::transfer_with_change_address`] with - /// `output_change_address: Some(_)`. + /// `Σ outputs`. /// /// # Output order semantics /// @@ -285,178 +151,6 @@ impl PlatformAddressWallet { .await) } - /// Transfer credits with an address-keyed fee strategy and an optional - /// "change address" override. - /// - /// # When to use - /// - /// Use this wrapper when you want to identify the fee-bearing row by - /// [`PlatformAddress`] and/or want the wallet to compute a change output - /// from over-funded explicit inputs. For [`InputSelection::Auto`] or - /// index-based fee strategies, use [`Self::transfer`] directly — this - /// wrapper rejects `Auto` because the auto-selector leaves no surplus - /// to route as change. - /// - /// The fee-bearing row is identified by [`PlatformAddress`] rather than - /// by index — the wrapper lowers [`FeeStrategyByAddress`] to the - /// consensus [`AddressFundsFeeStrategy`] AFTER `user_outputs` has been - /// augmented with the change row, so indexes always resolve against the - /// final lex-ordered outputs map the signer will see. This eliminates a - /// class of misrouting bugs where inserting a change address shifts the - /// post-canonicalisation index of one or more user outputs. - /// - /// When `output_change_address` is `Some(change_addr)`, the wrapper adds - /// a change output absorbing `Σ consumed − Σ user_outputs` so the - /// `Σ inputs == Σ outputs` invariant holds. When `None`, the user - /// outputs are forwarded as-is. - /// - /// Requires [`InputSelection::Explicit`] or - /// [`InputSelection::ExplicitWithNonces`] — auto-selection trims inputs - /// to a covering prefix (no residual to route as change) and - /// address-keyed lowering needs the inputs map known up front. Callers - /// who want auto-selection should use [`Self::transfer`] directly. - /// - /// Under `DeductFromInputAddress(_)` the caller MUST reserve fee - /// headroom on the targeted input (i.e. its map value must be strictly - /// below its on-chain balance by at least the estimated fee); otherwise - /// the chain rejects the transition with `fee_fully_covered = false`. - /// Under `ReduceOutputAddress(_)` callers may pass the full balances; - /// the named output absorbs the fee. - /// - /// # Errors - /// - /// [`PlatformWalletError::AddressOperation`] when: - /// - `fee_strategy` is empty, - /// - [`InputSelection::Auto`] is supplied, - /// - `change_addr` collides with `user_outputs` or inputs, - /// - `Σ inputs ≤ Σ user_outputs` (no surplus for change), - /// - a fee-strategy step names an address not present in the resolved - /// inputs / outputs maps. - #[allow(clippy::too_many_arguments)] // mirrors `transfer` plus the change-address override. - pub async fn transfer_with_change_address + Send + Sync>( - &self, - account_index: u32, - input_selection: InputSelection, - user_outputs: BTreeMap, - output_change_address: Option, - fee_strategy: FeeStrategyByAddress, - platform_version: Option<&PlatformVersion>, - address_signer: &S, - ) -> Result { - if fee_strategy.0.is_empty() { - return Err(PlatformWalletError::AddressOperation( - "fee_strategy must contain at least one step".to_string(), - )); - } - - // Auto is incompatible with this wrapper: the auto-selector trims - // inputs to satisfy `Σ inputs == Σ outputs`, leaving no residual to - // route to a change address; address-keyed fee resolution also - // needs the inputs map known up-front. - let inputs_for_resolve: BTreeMap = match &input_selection { - InputSelection::Explicit(inputs) => inputs.clone(), - InputSelection::ExplicitWithNonces(inputs) => inputs - .iter() - .map(|(addr, (_nonce, credits))| (*addr, *credits)) - .collect(), - InputSelection::Auto => { - return Err(PlatformWalletError::AddressOperation( - "transfer_with_change_address requires InputSelection::Explicit or \ - ExplicitWithNonces — the auto-selector trims inputs to a covering \ - prefix and has no concept of a residual to route to a change address; \ - address-keyed fee strategy also needs the inputs map known up-front. \ - Use `transfer` for Auto selection." - .to_string(), - )); - } - }; - - // Default to the wallet's SDK version (`self.sdk.version()`) — the - // same network-floored, protocol-version-tracking accessor that - // `transfer()` uses — rather than `LATEST_PLATFORM_VERSION`. This - // wrapper rejects `InputSelection::Auto`, so the production Auto UI - // never reaches it, but defaulting to LATEST here would still let the - // change-augmentation / fee-headroom math size version-keyed values - // (`min_output_amount`, `estimate_min_fee`) against a different - // version than the submit gate on a non-latest-pinned SDK. Defending - // in depth keeps both `transfer` entry points sizing every - // version-keyed value against the SAME version. An explicit `Some(v)` - // is honored as given. - let version = platform_version.unwrap_or_else(|| self.sdk.version()); - - let final_outputs = match output_change_address { - Some(change_addr) => { - validate_change_address(&change_addr, &user_outputs, inputs_for_resolve.keys())?; - let input_sum = checked_sum_credits(inputs_for_resolve.values().copied())?; - augment_outputs_with_change(user_outputs, change_addr, input_sum, version)? - } - None => user_outputs, - }; - - // Lower fee_strategy AFTER augmentation so indexes resolve against - // the FINAL outputs map. Lowering before would cause the - // misrouting this wrapper exists to prevent. - let indexed_fee_strategy = fee_strategy.to_indexed(&inputs_for_resolve, &final_outputs)?; - - // Replicate the Auto path's ReduceOutput fee-headroom guard so callers - // get a typed wallet-side rejection (and 3x safety-band warning) - // instead of a generic chain-time `fee_fully_covered = false`. Covers - // ALL `ReduceOutput(i)` steps since `to_indexed` can produce non-zero - // indices; mirrors the Auto path's choice to guard outputs only. - for step in indexed_fee_strategy.iter() { - if let AddressFundsFeeStrategyStep::ReduceOutput(idx) = step { - let target = final_outputs - .values() - .nth(*idx as usize) - .copied() - .unwrap_or(0); - let estimated_fee = AddressFundsTransferTransition::estimate_min_fee( - inputs_for_resolve.len(), - final_outputs.len(), - version, - ); - if target < estimated_fee { - return Err(PlatformWalletError::AddressOperation(format!( - "ReduceOutput target at index {} ({} credits) cannot absorb \ - estimated fee ({} credits); raise that output or switch to \ - DeductFromInput-based fee strategy", - idx, target, estimated_fee, - ))); - } - const REDUCE_OUTPUT_FEE_SAFETY_MULTIPLE: Credits = 3; - let safe_threshold = - estimated_fee.saturating_mul(REDUCE_OUTPUT_FEE_SAFETY_MULTIPLE); - if target < safe_threshold { - tracing::warn!( - target_index = *idx, - target_amount = target, - estimated_fee, - safety_multiple = REDUCE_OUTPUT_FEE_SAFETY_MULTIPLE, - tracking_issue = "platform#3040", - "[ReduceOutputAddress] target ({} credits) is within {}x of the \ - static estimated fee ({} credits); chain-time fee may exceed the \ - static estimate (platform#3040), risking on-chain rejection. \ - Consider raising the target output or switching to \ - DeductFromInputAddress.", - target, - REDUCE_OUTPUT_FEE_SAFETY_MULTIPLE, - estimated_fee, - ); - } - } - } - - self.transfer( - account_index, - input_selection, - final_outputs, - indexed_fee_strategy, - platform_version, - address_signer, - ) - .await - } - /// Dispatch to the strategy-specific selector. Returned map values are the /// **consumed amount per address**; protocol enforces `Σ inputs == Σ outputs`. /// Supported strategies: `[DeductFromInput(0)]`, `[ReduceOutput(0)]`. @@ -1097,79 +791,6 @@ fn select_inputs_reduce_output( Ok(selected) } -/// Reject `change_addr` collisions before the chain does: the protocol -/// errors deterministically when a transition has the same address as -/// both input and output, and silently merging into a caller-declared -/// output would mask a destination amount. Called at every -/// `transfer_with_change_address` entry that has the inputs map in scope. -fn validate_change_address<'a, I>( - change_addr: &PlatformAddress, - user_outputs: &BTreeMap, - inputs: I, -) -> Result<(), PlatformWalletError> -where - I: IntoIterator, -{ - if user_outputs.contains_key(change_addr) { - return Err(PlatformWalletError::AddressOperation(format!( - "output_change_address {change_addr:?} already appears in user_outputs; \ - refusing to silently merge a change-output amount into a caller-declared \ - output. Pick a fresh change_addr.", - ))); - } - if inputs.into_iter().any(|addr| addr == change_addr) { - return Err(PlatformWalletError::AddressOperation(format!( - "output_change_address {change_addr:?} also appears in the input map; \ - the protocol rejects transitions where the same address is both input \ - and output. Pick a fresh change_addr.", - ))); - } - Ok(()) -} - -/// Augment `user_outputs` with an explicit change output absorbing the -/// surplus `Σ inputs − Σ user_outputs`. Caller MUST invoke -/// [`validate_change_address`] first to rule out collisions; this fn -/// re-checks the user_outputs side defensively, rejects the no-surplus -/// case, and rejects residuals below the protocol per-output minimum -/// (`OutputBelowMinimumError`, code 10810). -fn augment_outputs_with_change( - mut user_outputs: BTreeMap, - change_addr: PlatformAddress, - input_sum: Credits, - platform_version: &PlatformVersion, -) -> Result, PlatformWalletError> { - if user_outputs.contains_key(&change_addr) { - return Err(PlatformWalletError::AddressOperation(format!( - "output_change_address {change_addr:?} already appears in user_outputs; \ - refusing to silently merge a change-output amount into a caller-declared \ - output. Pick a fresh change_addr.", - ))); - } - let user_output_sum: Credits = saturating_sum_credits(user_outputs.values().copied()); - if input_sum <= user_output_sum { - return Err(PlatformWalletError::AddressOperation(format!( - "output_change_address: Some(_) requires Σ inputs ({input_sum}) > \ - Σ user_outputs ({user_output_sum}); no surplus to route as change. \ - Drop output_change_address or grow the input map.", - ))); - } - let change_amount = input_sum.saturating_sub(user_output_sum); - let min_output_amount = platform_version - .dpp - .state_transitions - .address_funds - .min_output_amount; - if change_amount < min_output_amount { - return Err(PlatformWalletError::ChangeBelowMinimumOutput { - change_amount, - min_output_amount, - }); - } - user_outputs.insert(change_addr, change_amount); - Ok(user_outputs) -} - #[cfg(test)] mod auto_select_tests { use super::*; @@ -1674,135 +1295,6 @@ mod auto_select_tests { ); } - /// PA-001b: the change-address override must add exactly one extra output - /// absorbing `Σ inputs − Σ user_outputs`, leaving `Σ inputs == Σ outputs` - /// so the protocol's structural invariant still holds. - #[test] - fn augment_outputs_with_change_adds_residual_output() { - let user_target = p2pkh(0x22); - let change_addr = p2pkh(0x33); - let user_outputs = outputs_for(user_target, 5_000_000); - let pv = LATEST_PLATFORM_VERSION; - let outputs = augment_outputs_with_change(user_outputs, change_addr, 60_000_000, pv) - .expect("augment"); - assert_eq!(outputs.len(), 2); - assert_eq!(outputs.get(&user_target), Some(&5_000_000)); - assert_eq!( - outputs.get(&change_addr), - Some(&55_000_000), - "change output must absorb exactly the surplus" - ); - let output_sum: Credits = outputs.values().sum(); - assert_eq!( - output_sum, 60_000_000, - "Σ outputs must equal input sum (Σ inputs == Σ outputs invariant)" - ); - } - - /// PA-001b: the override must reject a `change_addr` that already appears - /// in the caller's user outputs to prevent a silent merge. - #[test] - fn augment_outputs_with_change_rejects_duplicate_address() { - let target = p2pkh(0x44); - let user_outputs = outputs_for(target, 5_000_000); - let pv = LATEST_PLATFORM_VERSION; - let err = augment_outputs_with_change(user_outputs, target, 60_000_000, pv) - .expect_err("change_addr equal to user output must be rejected"); - match err { - PlatformWalletError::AddressOperation(msg) => { - assert!( - msg.contains("already appears in user_outputs"), - "unexpected message: {msg}" - ); - } - other => panic!("expected AddressOperation, got {other:?}"), - } - } - - /// PA-001b: when `Σ user_outputs ≥ Σ inputs` there is no surplus to route. - /// The wrapper must reject rather than emit a zero-credit (or underflowing) - /// change output. - #[test] - fn augment_outputs_with_change_rejects_no_surplus() { - let target = p2pkh(0x55); - let change_addr = p2pkh(0x66); - let user_outputs = outputs_for(target, 60_000_000); - let pv = LATEST_PLATFORM_VERSION; - let err = augment_outputs_with_change(user_outputs, change_addr, 60_000_000, pv) - .expect_err("equal sums must be rejected: nothing to route as change"); - match err { - PlatformWalletError::AddressOperation(msg) => { - assert!(msg.contains("no surplus"), "unexpected message: {msg}"); - } - other => panic!("expected AddressOperation, got {other:?}"), - } - } - - /// QA-001: residual in the `(0, min_output_amount)` band must be rejected - /// before the chain does (`OutputBelowMinimumError`, code 10810). The - /// existing tests cover residual=0 (no-surplus) and residual=55M (well - /// above min); this fills the gap in the middle. - #[test] - fn augment_outputs_with_change_rejects_sub_minimum_residual() { - let target = p2pkh(0x77); - let change_addr = p2pkh(0x88); - let pv = LATEST_PLATFORM_VERSION; - let min_output = pv.dpp.state_transitions.address_funds.min_output_amount; - // Pick a residual strictly between 0 and min_output_amount. - let residual = min_output - 1; - let user_output_amount = 5_000_000u64; - let user_outputs = outputs_for(target, user_output_amount); - let input_sum = user_output_amount + residual; - - let err = augment_outputs_with_change(user_outputs, change_addr, input_sum, pv) - .expect_err("sub-min residual must be rejected pre-broadcast"); - match err { - PlatformWalletError::ChangeBelowMinimumOutput { - change_amount, - min_output_amount, - } => { - assert_eq!(change_amount, residual); - assert_eq!(min_output_amount, min_output); - } - other => panic!("expected ChangeBelowMinimumOutput, got {other:?}"), - } - } - - /// `validate_change_address` rejects collisions with user_outputs OR - /// the input map; otherwise it accepts. - #[test] - fn validate_change_address_rejects_both_collision_shapes() { - let target = p2pkh(0xAA); - let input = p2pkh(0xBB); - let other = p2pkh(0xCC); - let user_outputs = outputs_for(target, 5_000_000); - let input_keys = std::iter::once(&input); - - // Collision with user_outputs. - let err = validate_change_address(&target, &user_outputs, input_keys.clone()) - .expect_err("user_outputs collision"); - match err { - PlatformWalletError::AddressOperation(msg) => { - assert!(msg.contains("already appears in user_outputs"), "{msg}"); - } - other => panic!("expected AddressOperation, got {other:?}"), - } - - // Collision with inputs. - let err = validate_change_address(&input, &user_outputs, input_keys.clone()) - .expect_err("input collision"); - match err { - PlatformWalletError::AddressOperation(msg) => { - assert!(msg.contains("also appears in the input map"), "{msg}"); - } - other => panic!("expected AddressOperation, got {other:?}"), - } - - // Fresh address — accepted. - validate_change_address(&other, &user_outputs, input_keys) - .expect("fresh change_addr must validate"); - } - /// `[ReduceOutput(0)]` Phase 3 success: two-input prefix where the trim /// drops the last entry below `min_input_amount` and the donor has the /// headroom to lift it back. Verifies the shift lands both entries @@ -1910,382 +1402,4 @@ mod auto_select_tests { other => panic!("expected AddressOperation, got {other:?}"), } } - - /// Build a `PlatformAddressWallet` wired with a stub asset-lock manager - /// for tests that short-circuit before any I/O. The asset-lock manager - /// is constructed but never exercised — the rejection arms under test - /// fire before any of its methods are called. - fn build_short_circuit_wallet() -> crate::wallet::platform_addresses::PlatformAddressWallet { - use crate::broadcaster::SpvBroadcaster; - use crate::events::PlatformEventManager; - use crate::spv::SpvRuntime; - use crate::wallet::asset_lock::manager::AssetLockManager; - use crate::wallet::persister::{NoPlatformPersistence, WalletPersister}; - use crate::wallet::platform_addresses::PlatformAddressWallet; - use std::sync::Arc; - use tokio::sync::{Notify, RwLock}; - - let sdk = Arc::new(dash_sdk::SdkBuilder::new_mock().build().expect("mock sdk")); - let wallet_manager = Arc::new(RwLock::new(key_wallet_manager::WalletManager::new( - sdk.network, - ))); - let persister = WalletPersister::new([0u8; 32], Arc::new(NoPlatformPersistence)); - let event_manager = Arc::new(PlatformEventManager::new(Vec::new())); - let spv = Arc::new(SpvRuntime::new(Arc::clone(&wallet_manager), event_manager)); - let broadcaster = Arc::new(SpvBroadcaster::new(spv)); - let asset_locks = Arc::new(AssetLockManager::new( - Arc::clone(&sdk), - Arc::clone(&wallet_manager), - [0u8; 32], - Arc::new(Notify::new()), - broadcaster, - persister.clone(), - )); - PlatformAddressWallet::new(sdk, wallet_manager, [0u8; 32], asset_locks, persister) - } - - /// CMT-007: `transfer_with_change_address` must reject - /// `InputSelection::Auto` before doing any I/O. The rejection happens - /// in the wrapper's own match arm, well before any wallet-manager or - /// broadcaster method is touched. - #[tokio::test] - async fn transfer_with_change_address_rejects_auto_selection() { - let wallet = build_short_circuit_wallet(); - let signer = NullSigner; - let target = p2pkh(0x77); - let outputs: BTreeMap = outputs_for(target, 10_000_000); - let change_addr = p2pkh(0x88); - let fee_strategy = FeeStrategyByAddress::reduce_output(target); - - let err = wallet - .transfer_with_change_address( - 0, - InputSelection::Auto, - outputs, - Some(change_addr), - fee_strategy, - None, - &signer, - ) - .await - .expect_err("Auto + Some(change_addr) must error"); - match err { - PlatformWalletError::AddressOperation(msg) => { - assert!( - msg.contains("InputSelection::Explicit"), - "unexpected message: {msg}" - ); - } - other => panic!("expected AddressOperation, got {other:?}"), - } - } - - /// Smoke test for the standard `transfer_with_change_address` entry: a - /// `BTreeMap` of user outputs reaches the "Auto + Some(change_addr)" - /// rejection arm without any I/O. Pins the public-API parameter type. - #[tokio::test] - async fn transfer_with_change_address_accepts_btreemap_outputs() { - let wallet = build_short_circuit_wallet(); - let signer = NullSigner; - let target = p2pkh(0x77); - let outputs: BTreeMap = outputs_for(target, 10_000_000); - let change_addr = p2pkh(0x88); - let fee_strategy = FeeStrategyByAddress::reduce_output(target); - - let err = wallet - .transfer_with_change_address( - 0, - InputSelection::Auto, - outputs, - Some(change_addr), - fee_strategy, - None, - &signer, - ) - .await - .expect_err("Auto + Some(change_addr) must error"); - assert!(matches!(err, PlatformWalletError::AddressOperation(_))); - } - - // The `inputs ∪ outputs` translation (external-recipient filtering, - // missing-info-as-zero) now lives in the shared reconciliation seam — - // see `build_entries_drops_unresolved_and_zeroes_missing_info` in - // `provider.rs`. - - /// Signer used only by tests that exercise paths which short-circuit - /// before any signing happens. Never produces a signature. - #[derive(Debug)] - pub(super) struct NullSigner; - - #[async_trait::async_trait] - impl dpp::identity::signer::Signer for NullSigner { - async fn sign( - &self, - _key: &PlatformAddress, - _data: &[u8], - ) -> Result { - unreachable!("NullSigner used by a test path that should short-circuit before signing") - } - - async fn sign_create_witness( - &self, - _key: &PlatformAddress, - _data: &[u8], - ) -> Result { - unreachable!("NullSigner used by a test path that should short-circuit before signing") - } - - fn can_sign_with(&self, _key: &PlatformAddress) -> bool { - false - } - } - - // -- FeeStrategyByAddress::to_indexed ----------------------------------- - - /// Build a 3-row outputs map keyed by `addrs[0..3]` (assumed lex-ordered - /// by the caller). Values are arbitrary, only the keys matter for index - /// resolution. - fn three_output_map(addrs: [PlatformAddress; 3]) -> BTreeMap { - let mut m = BTreeMap::new(); - m.insert(addrs[0], 100); - m.insert(addrs[1], 200); - m.insert(addrs[2], 300); - m - } - - fn three_input_map(addrs: [PlatformAddress; 3]) -> BTreeMap { - let mut m = BTreeMap::new(); - m.insert(addrs[0], 10); - m.insert(addrs[1], 20); - m.insert(addrs[2], 30); - m - } - - #[test] - fn reduce_output_resolves_smallest_to_index_zero() { - let a = p2pkh(0x01); - let b = p2pkh(0x02); - let c = p2pkh(0x03); - let outputs = three_output_map([a, b, c]); - let inputs: BTreeMap = BTreeMap::new(); - - let indexed = FeeStrategyByAddress::reduce_output(a) - .to_indexed(&inputs, &outputs) - .expect("resolve"); - assert_eq!(indexed, vec![AddressFundsFeeStrategyStep::ReduceOutput(0)]); - } - - #[test] - fn reduce_output_resolves_middle_to_correct_index() { - let a = p2pkh(0x01); - let b = p2pkh(0x02); - let c = p2pkh(0x03); - let outputs = three_output_map([a, b, c]); - let inputs: BTreeMap = BTreeMap::new(); - - let indexed = FeeStrategyByAddress::reduce_output(b) - .to_indexed(&inputs, &outputs) - .expect("resolve"); - assert_eq!(indexed, vec![AddressFundsFeeStrategyStep::ReduceOutput(1)]); - } - - #[test] - fn reduce_output_resolves_largest_to_last_index() { - let a = p2pkh(0x01); - let b = p2pkh(0x02); - let c = p2pkh(0x03); - let outputs = three_output_map([a, b, c]); - let inputs: BTreeMap = BTreeMap::new(); - - let indexed = FeeStrategyByAddress::reduce_output(c) - .to_indexed(&inputs, &outputs) - .expect("resolve"); - assert_eq!(indexed, vec![AddressFundsFeeStrategyStep::ReduceOutput(2)]); - } - - #[test] - fn deduct_from_input_resolves_at_each_position() { - let a = p2pkh(0x10); - let b = p2pkh(0x20); - let c = p2pkh(0x30); - let inputs = three_input_map([a, b, c]); - let outputs: BTreeMap = BTreeMap::new(); - - let cases = [(a, 0u16), (b, 1u16), (c, 2u16)]; - for (addr, expected) in cases { - let indexed = FeeStrategyByAddress::deduct_from_input(addr) - .to_indexed(&inputs, &outputs) - .expect("resolve"); - assert_eq!( - indexed, - vec![AddressFundsFeeStrategyStep::DeductFromInput(expected)] - ); - } - } - - #[test] - fn multi_step_resolves_each_independently() { - let in_a = p2pkh(0x10); - let in_b = p2pkh(0x20); - let out_a = p2pkh(0x40); - let out_b = p2pkh(0x50); - let inputs = { - let mut m = BTreeMap::new(); - m.insert(in_a, 10u64); - m.insert(in_b, 20u64); - m - }; - let outputs = { - let mut m = BTreeMap::new(); - m.insert(out_a, 100u64); - m.insert(out_b, 200u64); - m - }; - - let strategy = FeeStrategyByAddress(vec![ - FeeStrategyStepByAddress::DeductFromInputAddress(in_b), - FeeStrategyStepByAddress::ReduceOutputAddress(out_b), - ]); - let indexed = strategy.to_indexed(&inputs, &outputs).expect("resolve"); - assert_eq!( - indexed, - vec![ - AddressFundsFeeStrategyStep::DeductFromInput(1), - AddressFundsFeeStrategyStep::ReduceOutput(1), - ] - ); - } - - #[test] - fn unknown_input_address_yields_input_not_found() { - let known = p2pkh(0x10); - let unknown = p2pkh(0xFF); - let mut inputs: BTreeMap = BTreeMap::new(); - inputs.insert(known, 10); - let outputs: BTreeMap = BTreeMap::new(); - - let err = FeeStrategyByAddress::deduct_from_input(unknown) - .to_indexed(&inputs, &outputs) - .expect_err("must fail"); - assert!(matches!( - err, - FeeStrategyResolveError::InputAddressNotFound(addr) if addr == unknown - )); - } - - #[test] - fn unknown_output_address_yields_output_not_found() { - let known = p2pkh(0x40); - let unknown = p2pkh(0xFF); - let inputs: BTreeMap = BTreeMap::new(); - let mut outputs: BTreeMap = BTreeMap::new(); - outputs.insert(known, 100); - - let err = FeeStrategyByAddress::reduce_output(unknown) - .to_indexed(&inputs, &outputs) - .expect_err("must fail"); - assert!(matches!( - err, - FeeStrategyResolveError::OutputAddressNotFound(addr) if addr == unknown - )); - } - - /// Regression: with `user_outputs = { 0x02, 0x03 }` and a lex-smaller - /// change address `0x01` inserted by augmentation, a fee strategy - /// targeting `0x03` must resolve to index **2**, not 1. The pre-refactor - /// indexed API silently produced index 1 (caller's pre-augmentation - /// position), misrouting the fee. The address-keyed API resolves against - /// the final outputs map and produces index 2. - #[test] - fn reduce_output_handles_non_zero_index_shift_after_augmentation() { - let change = p2pkh(0x01); - let user_a = p2pkh(0x02); - let user_b = p2pkh(0x03); - - // Final outputs as seen by the signer (post-augmentation). - let mut final_outputs: BTreeMap = BTreeMap::new(); - final_outputs.insert(change, 50); - final_outputs.insert(user_a, 100); - final_outputs.insert(user_b, 200); - - let inputs: BTreeMap = BTreeMap::new(); - - let indexed = FeeStrategyByAddress::reduce_output(user_b) - .to_indexed(&inputs, &final_outputs) - .expect("resolve"); - assert_eq!( - indexed, - vec![AddressFundsFeeStrategyStep::ReduceOutput(2)], - "user_b is at index 2 of the post-augmentation lex order, not 1" - ); - } - - /// The old wrapper rejected `[ReduceOutput(0)]` when the change address - /// was lex-smaller than every user output, because it assumed the index - /// referred to the caller's mental model rather than the final map. With - /// the address-keyed API this is now a legitimate, supported call: - /// targeting the change row by address resolves to whatever index it - /// occupies in the final outputs map. - #[test] - fn reduce_output_targeting_lex_smallest_change_address_is_allowed() { - let change = p2pkh(0x01); - let user_a = p2pkh(0x02); - let user_b = p2pkh(0x03); - - let mut final_outputs: BTreeMap = BTreeMap::new(); - final_outputs.insert(change, 50); - final_outputs.insert(user_a, 100); - final_outputs.insert(user_b, 200); - - let inputs: BTreeMap = BTreeMap::new(); - - let indexed = FeeStrategyByAddress::reduce_output(change) - .to_indexed(&inputs, &final_outputs) - .expect("resolve must succeed; old heuristic is gone"); - assert_eq!(indexed, vec![AddressFundsFeeStrategyStep::ReduceOutput(0)]); - } - - /// CMT-003: when the wrapper resolves `ReduceOutputAddress` to an output - /// row whose amount is below the static `estimated_fee`, it must reject - /// pre-broadcast with a typed `AddressOperation` error rather than letting - /// the chain refuse it with a generic `fee_fully_covered = false`. - #[tokio::test] - async fn transfer_with_change_address_rejects_sub_fee_reduce_output_target() { - let wallet = build_short_circuit_wallet(); - let signer = NullSigner; - let target = p2pkh(0x77); - let input_addr = p2pkh(0x11); - // Target output amount well below `estimate_min_fee(1, 1, _)` - // (≈6.5M credits): the guard must fire. - let outputs = outputs_for(target, 100_000); - let mut inputs: BTreeMap = BTreeMap::new(); - inputs.insert(input_addr, 100_000); - let fee_strategy = FeeStrategyByAddress::reduce_output(target); - - let err = wallet - .transfer_with_change_address( - 0, - InputSelection::Explicit(inputs), - outputs, - None, - fee_strategy, - None, - &signer, - ) - .await - .expect_err("sub-fee ReduceOutput target must be rejected"); - match err { - PlatformWalletError::AddressOperation(msg) => { - assert!( - msg.contains("cannot absorb estimated fee"), - "unexpected message: {msg}" - ); - assert!( - msg.contains("ReduceOutput target at index"), - "unexpected message: {msg}" - ); - } - other => panic!("expected AddressOperation, got {other:?}"), - } - } } From 6d179338f828c0273263832a7b4e7a9b61a1a2a3 Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Wed, 9 Sep 2026 12:06:21 +0200 Subject: [PATCH 02/16] refactor(platform-wallet): remove DashpayAddressMatch and the match_incoming_dashpay_address variants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three public variants (`match_incoming_dashpay_address`, `_blocking`, `try_match_incoming_dashpay_address`) had no call site — not from Rust, and not through the FFI, Swift or Kotlin. One of them (`_blocking`) panics when called from a tokio context. The only live consumer was `payments.rs`, reaching the private `match_in_collection` helper through an artificial `DashPayView::::` turbofish. That helper becomes a plain free function `match_receival_address` in payments.rs and returns a pair of identifiers instead of a struct — `DashpayAddressMatch`'s `address_index` field had no reader. `DashpayAddressMatch` goes along with its re-exports in types/dashpay/mod.rs, types/mod.rs and identity/mod.rs. Co-Authored-By: Claude Opus 5 --- .../src/wallet/identity/mod.rs | 6 +- .../src/wallet/identity/network/contacts.rs | 105 ------------------ .../src/wallet/identity/network/payments.rs | 54 ++++++++- .../src/wallet/identity/types/dashpay/mod.rs | 2 +- .../wallet/identity/types/dashpay/payment.rs | 19 ---- .../src/wallet/identity/types/mod.rs | 4 +- 6 files changed, 54 insertions(+), 136 deletions(-) diff --git a/packages/rs-platform-wallet/src/wallet/identity/mod.rs b/packages/rs-platform-wallet/src/wallet/identity/mod.rs index 54b7b8bb300..99e8ba106aa 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/mod.rs @@ -35,7 +35,7 @@ pub use state::{ }; pub use types::dashpay::profile::{calculate_avatar_hash, calculate_dhash_fingerprint}; pub use types::{ - ContactProfileEntry, ContactRequest, DashPayProfile, DashpayAddressMatch, DpnsNameInfo, - EstablishedContact, IdentityStatus, KeyStorage, PaymentDirection, PaymentEntry, PaymentStatus, - PrivateKeyData, ProfileUpdate, + ContactProfileEntry, ContactRequest, DashPayProfile, DpnsNameInfo, EstablishedContact, + IdentityStatus, KeyStorage, PaymentDirection, PaymentEntry, PaymentStatus, PrivateKeyData, + ProfileUpdate, }; diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs b/packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs index a19288f4128..9f92b6a4828 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs @@ -11,8 +11,6 @@ use crate::broadcaster::TransactionBroadcaster; use crate::changeset::{AccountRegistrationEntry, PlatformWalletChangeSet}; use crate::error::PlatformWalletError; use crate::wallet::identity::types::dashpay::established_contact::EstablishedContact; -use crate::wallet::identity::types::dashpay::payment::DashpayAddressMatch; -use crate::wallet::platform_wallet::PlatformWalletInfo; /// Build the persistence round for a newly registered DashPay account /// (`DashpayReceivingFunds` / `DashpayExternalAccount`): the @@ -256,109 +254,6 @@ impl DashPayView<'_, B> { Ok(()) } - - /// Match an on-chain address against this wallet's registered - /// DashPay contact receival accounts. - /// - /// Iterates every `DashpayReceivingFunds` account in this - /// wallet's [`key_wallet::managed_account::ManagedAccountCollection`] - /// and checks whether the address belongs to any of their - /// address pools. Returns the first match as a - /// [`DashpayAddressMatch`], or `None` if the address is not - /// a DashPay contact address for this wallet. - /// - /// Used by the SPV / backend task layer to classify observed - /// transaction outputs as DashPay incoming payments from a - /// specific contact. No separate reverse-lookup table is needed - /// in the UI layer: the authoritative state is already - /// tracked by `register_contact_account`, which inserts the - /// account into the wallet's `ManagedAccountCollection` so - /// key-wallet manages the address pool (derivation + gap limit - /// + used tracking). - /// - /// Only the external pool of each receival account is - /// searched: DashPay uses a single-pool account type so all - /// contact payment addresses live on that one pool. - pub async fn match_incoming_dashpay_address( - &self, - address: &dashcore::Address, - ) -> Option { - let wm = self.wallet_manager.read().await; - let info = wm.get_wallet_info(&self.wallet_id)?; - Self::match_in_collection(info, address) - } - - /// Blocking variant of [`match_incoming_dashpay_address`] for - /// sync callers (SPV transaction-processing frame loop). Uses - /// `tokio::sync::RwLock::blocking_read` — must NOT be called - /// from within a tokio async context. - pub fn match_incoming_dashpay_address_blocking( - &self, - address: &dashcore::Address, - ) -> Option { - let wm = self.wallet_manager.blocking_read(); - let info = wm.get_wallet_info(&self.wallet_id)?; - Self::match_in_collection(info, address) - } - - /// Non-blocking variant of [`match_incoming_dashpay_address`]. - /// Returns `Err(())` if the wallet-manager lock is currently - /// contended (e.g. SPV is processing a block). Returns `Ok(None)` - /// if the address does not belong to any DashPay receiving - /// account. Safe to call from any thread, including tokio runtime - /// threads, where the blocking variant would panic. - #[allow(clippy::result_unit_err)] - pub fn try_match_incoming_dashpay_address( - &self, - address: &dashcore::Address, - ) -> Result, ()> { - let wm = self.wallet_manager.try_read().map_err(|_| ())?; - let Some(info) = wm.get_wallet_info(&self.wallet_id) else { - return Ok(None); - }; - Ok(Self::match_in_collection(info, address)) - } - - /// Shared implementation that iterates - /// `info.core_wallet.accounts.dashpay_receival_accounts` and - /// checks each account's address pool for a match. - pub(super) fn match_in_collection( - info: &PlatformWalletInfo, - address: &dashcore::Address, - ) -> Option { - use key_wallet::managed_account::managed_account_type::ManagedAccountType; - - for (key, account) in &info.core_wallet.accounts.dashpay_receival_accounts { - let ManagedAccountType::DashpayReceivingFunds { - user_identity_id, - friend_identity_id, - .. - } = account.managed_account_type() - else { - // Routing invariant: dashpay_receival_accounts must - // only contain DashpayReceivingFunds. If this ever - // trips, it's a key-wallet bug. - debug_assert!( - false, - "non-DashpayReceivingFunds in dashpay_receival_accounts" - ); - continue; - }; - let Some(info) = account.get_address_info(address) else { - continue; - }; - // Sanity check — the collection key should match the - // account type's own identity ids. - debug_assert_eq!(&key.user_identity_id, user_identity_id); - debug_assert_eq!(&key.friend_identity_id, friend_identity_id); - return Some(DashpayAddressMatch { - user_identity_id: Identifier::from(*user_identity_id), - friend_identity_id: Identifier::from(*friend_identity_id), - address_index: info.index, - }); - } - None - } } // --------------------------------------------------------------------------- diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs index 4c455aba433..80f1fb3ba5a 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs @@ -735,6 +735,52 @@ impl DashPayView<'_, B> { } } +/// Classify an observed address against the wallet's registered DashPay +/// contact receival accounts. +/// +/// Iterates `info.core_wallet.accounts.dashpay_receival_accounts` and checks +/// each account's address pool. Returns the `(user_identity_id, +/// friend_identity_id)` pair of the first match, or `None` when the address +/// is not a DashPay contact address for this wallet. +/// +/// Only the external pool of each receival account is searched: DashPay uses +/// a single-pool account type, so all contact payment addresses live there. +fn match_receival_address( + info: &PlatformWalletInfo, + address: &dashcore::Address, +) -> Option<(Identifier, Identifier)> { + use key_wallet::managed_account::managed_account_type::ManagedAccountType; + + for (key, account) in &info.core_wallet.accounts.dashpay_receival_accounts { + let ManagedAccountType::DashpayReceivingFunds { + user_identity_id, + friend_identity_id, + .. + } = account.managed_account_type() + else { + // Routing invariant: dashpay_receival_accounts must only contain + // DashpayReceivingFunds. If this ever trips, it's a key-wallet bug. + debug_assert!( + false, + "non-DashpayReceivingFunds in dashpay_receival_accounts" + ); + continue; + }; + if account.get_address_info(address).is_none() { + continue; + } + // Sanity check — the collection key should match the account type's + // own identity ids. + debug_assert_eq!(&key.user_identity_id, user_identity_id); + debug_assert_eq!(&key.friend_identity_id, friend_identity_id); + return Some(( + Identifier::from(*user_identity_id), + Identifier::from(*friend_identity_id), + )); + } + None +} + /// Record `Received` [`PaymentEntry`]s for a freshly detected Core /// transaction whose outputs pay DashPay receival-account addresses. /// @@ -778,12 +824,8 @@ pub(crate) async fn record_incoming_dashpay_payments( let mut totals: BTreeMap<(Identifier, Identifier), u64> = BTreeMap::new(); for (address, value) in candidates { - if let Some(m) = - DashPayView::::match_in_collection(info, &address) - { - *totals - .entry((m.user_identity_id, m.friend_identity_id)) - .or_default() += value; + if let Some(pair) = match_receival_address(info, &address) { + *totals.entry(pair).or_default() += value; } } diff --git a/packages/rs-platform-wallet/src/wallet/identity/types/dashpay/mod.rs b/packages/rs-platform-wallet/src/wallet/identity/types/dashpay/mod.rs index 339520651d7..a63e30c39ea 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/types/dashpay/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/types/dashpay/mod.rs @@ -7,7 +7,7 @@ pub mod profile; pub use contact_request::ContactRequest; pub use established_contact::EstablishedContact; -pub use payment::{DashpayAddressMatch, PaymentDirection, PaymentEntry, PaymentStatus}; +pub use payment::{PaymentDirection, PaymentEntry, PaymentStatus}; pub use profile::{ calculate_avatar_hash, calculate_dhash_fingerprint, ContactProfileEntry, DashPayProfile, ProfileUpdate, diff --git a/packages/rs-platform-wallet/src/wallet/identity/types/dashpay/payment.rs b/packages/rs-platform-wallet/src/wallet/identity/types/dashpay/payment.rs index 976b64acad3..323d17d0b15 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/types/dashpay/payment.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/types/dashpay/payment.rs @@ -40,25 +40,6 @@ pub enum PaymentStatus { Failed, } -/// Match result from -/// [`IdentityWallet::match_incoming_dashpay_address`](crate::wallet::identity::IdentityWallet). -/// -/// Returned when an on-chain address matches one of the DashPay -/// contact receival accounts registered in this wallet's -/// [`ManagedAccountCollection`]. Lets the SPV / backend task layer -/// classify an observed transaction output as a DashPay incoming -/// payment from a specific contact, at a specific BIP44 index, -/// without needing to maintain a separate reverse-lookup table. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct DashpayAddressMatch { - /// Our (owner) identity ID — the recipient of the payment. - pub user_identity_id: Identifier, - /// The sending contact's identity ID. - pub friend_identity_id: Identifier, - /// Address index within the account's external pool. - pub address_index: u32, -} - /// A single DashPay payment entry recorded on a /// [`ManagedIdentity`](crate::wallet::identity::ManagedIdentity). /// diff --git a/packages/rs-platform-wallet/src/wallet/identity/types/mod.rs b/packages/rs-platform-wallet/src/wallet/identity/types/mod.rs index 288f88a021c..fb53ae73c89 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/types/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/types/mod.rs @@ -11,7 +11,7 @@ pub mod key_storage; pub use block_time::BlockTime; pub use dashpay::{ - ContactProfileEntry, ContactRequest, DashPayProfile, DashpayAddressMatch, EstablishedContact, - PaymentDirection, PaymentEntry, PaymentStatus, ProfileUpdate, + ContactProfileEntry, ContactRequest, DashPayProfile, EstablishedContact, PaymentDirection, + PaymentEntry, PaymentStatus, ProfileUpdate, }; pub use key_storage::{DpnsNameInfo, IdentityStatus, KeyStorage, PrivateKeyData}; From a4a85c616bb949a42f3c3dc06ce0979b8e32b343 Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Wed, 9 Sep 2026 12:09:18 +0200 Subject: [PATCH 03/16] refactor(platform-wallet): remove uncalled public DashPay API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No consumer in ffi, jni, Swift or Kotlin — verified by grepping all of packages/: - `ManagedIdentity::accept_incoming_request` — a second contact-establishment path that bypasses the persist-before-commit rule the live path enforces (`add_sent_contact_request` / `add_incoming_contact_request`). Called only by its own three tests. - `ManagedIdentity::remove_incoming_contact_request` — tests only; production rejects through `ignore_sender`. The `test_reject_contact_request` integration test moves onto `ignore_sender`. - `state/contacts.rs`: `add_established_contact`, `remove_established_contact` and `established_contact` were hiding under an `#[allow(dead_code)]` spanning the whole impl block. `established_contact_mut`, the one in use, stays. - The `EstablishedContact` setters (`set_alias`, `clear_alias`, `set_note`, `clear_note`, `hide`, `unhide`, `add_accepted_account`, `remove_accepted_account`) — production writes the fields directly via `set_contact_metadata`. Tests that still carry their weight (metadata preservation on re-establish, account_reference rotation) build their fixture by assigning fields. - `ContactRequest::is_outgoing` / `is_incoming` — own tests only. The `is_outgoing` *fields* in the FFI/JNI/Swift persistence layer are a separate thing and are left untouched. Co-Authored-By: Claude Opus 5 --- .../src/changeset/changeset.rs | 2 +- .../managed_identity/contact_requests.rs | 206 +----------------- .../state/managed_identity/contacts.rs | 24 -- .../identity/types/dashpay/contact_request.rs | 30 --- .../types/dashpay/established_contact.rs | 123 +---------- .../tests/contact_workflow_tests.rs | 60 +---- 6 files changed, 16 insertions(+), 429 deletions(-) diff --git a/packages/rs-platform-wallet/src/changeset/changeset.rs b/packages/rs-platform-wallet/src/changeset/changeset.rs index b95ff429398..2527c9cd537 100644 --- a/packages/rs-platform-wallet/src/changeset/changeset.rs +++ b/packages/rs-platform-wallet/src/changeset/changeset.rs @@ -1128,7 +1128,7 @@ pub struct ContactChangeSet { pub removed_sent: BTreeSet, /// Incoming contact requests keyed by (owner ← sender). pub incoming_requests: BTreeMap, - /// Incoming requests explicitly removed (e.g. `remove_incoming_contact_request`). + /// Incoming requests explicitly removed (e.g. by `ignore_sender`). pub removed_incoming: BTreeSet, /// Newly established contacts keyed by (owner, contact). The full /// [`EstablishedContact`] is carried so the apply path can rebuild diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/contact_requests.rs b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/contact_requests.rs index a5eb645450e..0fdbf71ff23 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/contact_requests.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/contact_requests.rs @@ -239,8 +239,8 @@ impl ManagedIdentity { // this makes the two backends consistent.) // // Guarded on an ACTUAL removal — the same `removed.is_some()` - // discipline as `remove_incoming_contact_request` / - // `remove_sent_contact_request`. Ignoring a sender who has no + // discipline as `remove_sent_contact_request`. Ignoring a sender + // who has no // pending incoming entry (e.g. an already-established contact, or // one that raced auto-establish) must not emit a tombstone: the // contacts table is one row per pair, so an unconditional @@ -657,79 +657,6 @@ impl ManagedIdentity { Ok(rekeyed_established) } - - /// Remove an incoming contact request. - /// - /// Returns the removed request (if any) and a tombstone changeset. - pub fn remove_incoming_contact_request( - &mut self, - sender_id: &Identifier, - ) -> (Option, ContactChangeSet) { - let removed = self.dashpay.incoming_contact_requests.remove(sender_id); - let mut cs = ContactChangeSet::default(); - if removed.is_some() { - cs.removed_incoming.insert(ReceivedContactRequestKey { - owner_id: self.id(), - sender_id: *sender_id, - }); - } - (removed, cs) - } - - /// Accept an incoming contact request and establish the contact. - /// - /// Returns the established contact (if both incoming and outgoing - /// requests exist) and a changeset describing the transition. Returns - /// `(None, empty)` without modifying state if either request is - /// missing. - pub fn accept_incoming_request( - &mut self, - sender_id: &Identifier, - ) -> (Option, ContactChangeSet) { - // Check both exist before removing either (prevents data loss). - if !self - .dashpay - .incoming_contact_requests - .contains_key(sender_id) - || !self.dashpay.sent_contact_requests.contains_key(sender_id) - { - return (None, ContactChangeSet::default()); - } - // Both `remove` calls are guaranteed `Some` by the pre-check above. - let incoming_request = self - .dashpay - .incoming_contact_requests - .remove(sender_id) - .expect("incoming request presence checked above"); - let outgoing_request = self - .dashpay - .sent_contact_requests - .remove(sender_id) - .expect("sent request presence checked above"); - - // Create the established contact - let contact = EstablishedContact::new(*sender_id, outgoing_request, incoming_request); - - // Add to established contacts - self.dashpay - .established_contacts - .insert(*sender_id, contact.clone()); - - // Per the ContactChangeSet auto-establishment contract, `established` - // implies the matching pending requests are dropped — no separate - // `removed_sent` / `removed_incoming` emission needed here. - let owner_id = self.id(); - let mut cs = ContactChangeSet::default(); - cs.established.insert( - SentContactRequestKey { - owner_id, - recipient_id: *sender_id, - }, - contact.clone(), - ); - - (Some(contact), cs) - } } // --- High-water sync cursors (compare-and-advance) --- @@ -993,7 +920,7 @@ mod tests { /// established row — both request blobs plus the user's /// alias/note/hidden/accepted-accounts — while memory keeps the contact /// established. Mirrors the `removed.is_some()` guard already used by - /// `remove_incoming_contact_request`. Was red against the unconditional + /// `remove_sent_contact_request`. Was red against the unconditional /// emission. #[test] fn ignore_sender_without_pending_incoming_emits_no_tombstone() { @@ -1361,123 +1288,6 @@ mod tests { assert!(cs.removed_sent.is_empty()); } - #[test] - fn test_remove_incoming_contact_request() { - let mut managed = create_test_identity([1u8; 32]); - let sender_id = Identifier::from([2u8; 32]); - let recipient_id = Identifier::from([1u8; 32]); - let p = noop_persister(); - - let request = create_contact_request(sender_id, recipient_id, 1234567890); - managed - .add_incoming_contact_request(request.clone(), &p) - .expect("setup persists"); - - assert_eq!(managed.dashpay.incoming_contact_requests.len(), 1); - - // Remove the request - let (removed, cs) = managed.remove_incoming_contact_request(&sender_id); - assert!(removed.is_some()); - assert_eq!(removed.unwrap().sender_id, sender_id); - assert!(cs.removed_incoming.contains(&ReceivedContactRequestKey { - owner_id: managed.id(), - sender_id - })); - assert_eq!(managed.dashpay.incoming_contact_requests.len(), 0); - } - - #[test] - fn test_remove_nonexistent_incoming_request() { - let mut managed = create_test_identity([1u8; 32]); - let nonexistent_id = Identifier::from([99u8; 32]); - - let (removed, cs) = managed.remove_incoming_contact_request(&nonexistent_id); - assert!(removed.is_none()); - assert!(cs.removed_incoming.is_empty()); - } - - #[test] - fn test_accept_incoming_request_success() { - let mut managed = create_test_identity([1u8; 32]); - let our_id = Identifier::from([1u8; 32]); - let contact_id = Identifier::from([2u8; 32]); - - // Add both requests without auto-establishment - let outgoing = create_contact_request(our_id, contact_id, 1234567890); - let incoming = create_contact_request(contact_id, our_id, 1234567891); - - managed - .dashpay - .sent_contact_requests - .insert(contact_id, outgoing); - managed - .dashpay - .incoming_contact_requests - .insert(contact_id, incoming); - - // Accept the incoming request - let (result, cs) = managed.accept_incoming_request(&contact_id); - assert!(result.is_some()); - - let contact = result.unwrap(); - assert_eq!(contact.contact_identity_id, contact_id); - assert!(cs.established.contains_key(&SentContactRequestKey { - owner_id: our_id, - recipient_id: contact_id - })); - // Per the auto-establishment contract, `established` implies the - // matching pending requests are dropped — no separate tombstones. - assert!(cs.removed_sent.is_empty()); - assert!(cs.removed_incoming.is_empty()); - - // Verify requests were removed and contact established - assert_eq!(managed.dashpay.sent_contact_requests.len(), 0); - assert_eq!(managed.dashpay.incoming_contact_requests.len(), 0); - assert_eq!(managed.dashpay.established_contacts.len(), 1); - assert!(managed - .dashpay - .established_contacts - .contains_key(&contact_id)); - } - - #[test] - fn test_accept_incoming_request_missing_incoming() { - let mut managed = create_test_identity([1u8; 32]); - let our_id = Identifier::from([1u8; 32]); - let contact_id = Identifier::from([2u8; 32]); - - // Only add outgoing request - let outgoing = create_contact_request(our_id, contact_id, 1234567890); - managed - .dashpay - .sent_contact_requests - .insert(contact_id, outgoing); - - // Accept should fail - no incoming request - let (result, cs) = managed.accept_incoming_request(&contact_id); - assert!(result.is_none()); - assert!(::is_empty(&cs)); - } - - #[test] - fn test_accept_incoming_request_missing_outgoing() { - let mut managed = create_test_identity([1u8; 32]); - let contact_id = Identifier::from([2u8; 32]); - let our_id = Identifier::from([1u8; 32]); - - // Only add incoming request - let incoming = create_contact_request(contact_id, our_id, 1234567891); - managed - .dashpay - .incoming_contact_requests - .insert(contact_id, incoming); - - // Accept should fail - no outgoing request - let (result, cs) = managed.accept_incoming_request(&contact_id); - assert!(result.is_none()); - assert!(::is_empty(&cs)); - } - /// Re-ingesting one's own already-tracked sent request must be a /// no-op — no phantom pending-sent row, no second changeset write. The /// sent-side guard mirrors the received-side dedup. @@ -1530,9 +1340,9 @@ mod tests { .established_contacts .get_mut(&contact_id) .unwrap(); - established.set_alias("Alice".to_string()); - established.set_note("from work".to_string()); - established.hide(); + established.alias = Some("Alice".to_string()); + established.note = Some("from work".to_string()); + established.is_hidden = true; // Recurring sweep re-ingests our own sent request for an already // established contact — must not reset metadata. @@ -1581,7 +1391,7 @@ mod tests { .established_contacts .get_mut(&contact_id) .unwrap(); - est.set_alias("Carol".to_string()); + est.alias = Some("Carol".to_string()); assert_eq!(est.outgoing_request.account_reference, 100); // Rotation #1: re-send with a bumped reference R1. @@ -1724,7 +1534,7 @@ mod tests { .established_contacts .get_mut(&contact_id) .unwrap(); - est.set_alias("Bob".to_string()); + est.alias = Some("Bob".to_string()); // Simulate a re-ingested incoming reciprocal landing while a sent // request also exists in the map (forced state). diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/contacts.rs b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/contacts.rs index eaee6934cf0..a86f6ab405f 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/contacts.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/contacts.rs @@ -4,31 +4,7 @@ use super::ManagedIdentity; use crate::EstablishedContact; use dpp::prelude::Identifier; -#[allow(dead_code)] impl ManagedIdentity { - /// Add an established contact - pub(crate) fn add_established_contact(&mut self, contact: EstablishedContact) { - self.dashpay - .established_contacts - .insert(contact.contact_identity_id, contact); - } - - /// Remove an established contact by identity ID - pub(crate) fn remove_established_contact( - &mut self, - contact_id: &Identifier, - ) -> Option { - self.dashpay.established_contacts.remove(contact_id) - } - - /// Get an established contact by identity ID - pub(crate) fn established_contact( - &self, - contact_id: &Identifier, - ) -> Option<&EstablishedContact> { - self.dashpay.established_contacts.get(contact_id) - } - /// Get a mutable established contact by identity ID. /// /// Escape hatch for per-contact sub-field mutation (channel-broken diff --git a/packages/rs-platform-wallet/src/wallet/identity/types/dashpay/contact_request.rs b/packages/rs-platform-wallet/src/wallet/identity/types/dashpay/contact_request.rs index d0b1540a3ce..9447eafeab0 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/types/dashpay/contact_request.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/types/dashpay/contact_request.rs @@ -67,16 +67,6 @@ impl ContactRequest { created_at, } } - - /// Check if this is an outgoing request for the given identity - pub fn is_outgoing(&self, identity_id: &Identifier) -> bool { - self.sender_id == *identity_id - } - - /// Check if this is an incoming request for the given identity - pub fn is_incoming(&self, identity_id: &Identifier) -> bool { - self.recipient_id == *identity_id - } } #[cfg(test)] @@ -109,24 +99,4 @@ mod tests { assert_eq!(request.core_height_created_at, 100000); assert_eq!(request.created_at, 1234567890); } - - #[test] - fn test_is_outgoing() { - let request = create_test_contact_request(); - let sender_id = Identifier::from([1u8; 32]); - let other_id = Identifier::from([3u8; 32]); - - assert!(request.is_outgoing(&sender_id)); - assert!(!request.is_outgoing(&other_id)); - } - - #[test] - fn test_is_incoming() { - let request = create_test_contact_request(); - let recipient_id = Identifier::from([2u8; 32]); - let other_id = Identifier::from([3u8; 32]); - - assert!(request.is_incoming(&recipient_id)); - assert!(!request.is_incoming(&other_id)); - } } diff --git a/packages/rs-platform-wallet/src/wallet/identity/types/dashpay/established_contact.rs b/packages/rs-platform-wallet/src/wallet/identity/types/dashpay/established_contact.rs index 62c74084b8a..f36e4a4c293 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/types/dashpay/established_contact.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/types/dashpay/established_contact.rs @@ -152,48 +152,6 @@ impl EstablishedContact { external_account_reference: None, } } - - /// Set the alias for this contact - pub fn set_alias(&mut self, alias: String) { - self.alias = Some(alias); - } - - /// Clear the alias for this contact - pub fn clear_alias(&mut self) { - self.alias = None; - } - - /// Set a note for this contact - pub fn set_note(&mut self, note: String) { - self.note = Some(note); - } - - /// Clear the note for this contact - pub fn clear_note(&mut self) { - self.note = None; - } - - /// Hide this contact from the contact list - pub fn hide(&mut self) { - self.is_hidden = true; - } - - /// Unhide this contact - pub fn unhide(&mut self) { - self.is_hidden = false; - } - - /// Add an accepted account reference - pub fn add_accepted_account(&mut self, account_reference: u32) { - if !self.accepted_accounts.contains(&account_reference) { - self.accepted_accounts.push(account_reference); - } - } - - /// Remove an accepted account reference - pub fn remove_accepted_account(&mut self, account_reference: u32) { - self.accepted_accounts.retain(|&a| a != account_reference); - } } #[cfg(test)] @@ -255,10 +213,10 @@ mod tests { create_test_outgoing_request(), create_test_incoming_request(), ); - existing.set_alias("Best Friend".to_string()); - existing.set_note("Met at conference".to_string()); - existing.hide(); - existing.add_accepted_account(7); + existing.alias = Some("Best Friend".to_string()); + existing.note = Some("Met at conference".to_string()); + existing.is_hidden = true; + existing.accepted_accounts.push(7); existing.payment_channel_broken = true; existing.contact_account_label = Some("Stale label".to_string()); @@ -288,77 +246,4 @@ mod tests { // carried over (it is a property of the request, not user metadata). assert_eq!(reestablished.contact_account_label, None); } - - #[test] - fn test_alias_management() { - let mut contact = EstablishedContact::new( - Identifier::from([2u8; 32]), - create_test_outgoing_request(), - create_test_incoming_request(), - ); - - contact.set_alias("Best Friend".to_string()); - assert_eq!(contact.alias, Some("Best Friend".to_string())); - - contact.clear_alias(); - assert_eq!(contact.alias, None); - } - - #[test] - fn test_note_management() { - let mut contact = EstablishedContact::new( - Identifier::from([2u8; 32]), - create_test_outgoing_request(), - create_test_incoming_request(), - ); - - contact.set_note("Met at conference".to_string()); - assert_eq!(contact.note, Some("Met at conference".to_string())); - - contact.clear_note(); - assert_eq!(contact.note, None); - } - - #[test] - fn test_hide_unhide() { - let mut contact = EstablishedContact::new( - Identifier::from([2u8; 32]), - create_test_outgoing_request(), - create_test_incoming_request(), - ); - - assert_eq!(contact.is_hidden, false); - - contact.hide(); - assert_eq!(contact.is_hidden, true); - - contact.unhide(); - assert_eq!(contact.is_hidden, false); - } - - #[test] - fn test_accepted_accounts() { - let mut contact = EstablishedContact::new( - Identifier::from([2u8; 32]), - create_test_outgoing_request(), - create_test_incoming_request(), - ); - - // Add accounts - contact.add_accepted_account(1); - contact.add_accepted_account(2); - assert_eq!(contact.accepted_accounts.len(), 2); - assert!(contact.accepted_accounts.contains(&1)); - assert!(contact.accepted_accounts.contains(&2)); - - // Adding duplicate should not increase count - contact.add_accepted_account(1); - assert_eq!(contact.accepted_accounts.len(), 2); - - // Remove account - contact.remove_accepted_account(1); - assert_eq!(contact.accepted_accounts.len(), 1); - assert!(!contact.accepted_accounts.contains(&1)); - assert!(contact.accepted_accounts.contains(&2)); - } } diff --git a/packages/rs-platform-wallet/tests/contact_workflow_tests.rs b/packages/rs-platform-wallet/tests/contact_workflow_tests.rs index aef3d0fbc6b..d4ea15c7f43 100644 --- a/packages/rs-platform-wallet/tests/contact_workflow_tests.rs +++ b/packages/rs-platform-wallet/tests/contact_workflow_tests.rs @@ -279,60 +279,6 @@ fn test_multiple_contact_requests_workflow() { assert_eq!(managed_main.dashpay().established_contacts().len(), 2); } -#[test] -fn test_contact_alias_and_metadata() { - // Test setting alias, notes, and other metadata on established contacts - - let identity_a = create_test_identity([1u8; 32]); - let identity_b = create_test_identity([2u8; 32]); - - let id_a = identity_a.id(); - let id_b = identity_b.id(); - - let mut managed_a = ManagedIdentity::new(identity_a, 0); - - // Establish contact - let request_a_to_b = create_contact_request(id_a, id_b, 0, 1000); - let request_b_to_a = create_contact_request(id_b, id_a, 0, 1001); - - managed_a - .add_sent_contact_request(request_a_to_b, &noop_persister()) - .expect("setup persists"); - managed_a - .add_incoming_contact_request(request_b_to_a, &noop_persister()) - .expect("setup persists"); - - // Contact should be established - assert_eq!(managed_a.dashpay().established_contacts().len(), 1); - - // Get mutable reference to contact and modify metadata - let contact = managed_a.established_contact_mut(&id_b).unwrap(); - - // Set alias - contact.set_alias("Best Friend".to_string()); - assert_eq!(contact.alias, Some("Best Friend".to_string())); - - // Set note - contact.set_note("Met at DevCon 2024".to_string()); - assert_eq!(contact.note, Some("Met at DevCon 2024".to_string())); - - // Test hiding/unhiding - assert!(!contact.is_hidden); - contact.hide(); - assert!(contact.is_hidden); - contact.unhide(); - assert!(!contact.is_hidden); - - // Test account management - contact.add_accepted_account(1); - contact.add_accepted_account(2); - assert_eq!(contact.accepted_accounts.len(), 2); - - contact.remove_accepted_account(1); - assert_eq!(contact.accepted_accounts.len(), 1); - assert!(contact.accepted_accounts.contains(&2)); -} - #[test] fn test_reject_contact_request() { // Test rejecting/removing contact requests @@ -355,9 +301,9 @@ fn test_reject_contact_request() { assert_eq!(managed_a.dashpay().incoming_contact_requests().len(), 1); - // Reject by removing the request - let (removed, _cs) = managed_a.remove_incoming_contact_request(&id_b); - assert!(removed.is_some()); + // Reject by ignoring the sender — the path production actually uses. + let cs = managed_a.ignore_sender(&id_b); + assert!(cs.removed_incoming.len() == 1); assert_eq!(managed_a.dashpay().incoming_contact_requests().len(), 0); } From 17a4591cd0aad2f4688bec2ef63a52202af3e380 Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Wed, 9 Sep 2026 12:12:42 +0200 Subject: [PATCH 04/16] refactor(platform-wallet): remove PrivateKeyData/KeyStorage and a vestigial re-export shim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related cleanups in the same file. 1. `PrivateKeyData` and `KeyStorage` were never constructed or read — `PrivateKeyData::` returns zero hits across all of packages/, including rs-platform-wallet-ffi. The doc claimed "the IdentityKeysChangeSet apply path constructs one per replay, the FFI key-preview path uses one internally"; `apply_identity_key_entry` touches neither. `key_storage.rs` is renamed to `identity_status.rs` — it now holds only `IdentityStatus` and `DpnsNameInfo`. 2. `state::managed_identity` re-exported `block_time` and `key_storage` under the old path so "external users can still reach them" — a comment describing a past move, not the current layout. Ten sites in the tree (including rs-platform-wallet-ffi/src/memory_explorer.rs) reached the types through the shim instead of `crate::wallet::identity::types::*`. All retargeted, shim removed. Import-path changes only — no C ABI impact. Co-Authored-By: Claude Opus 5 --- .../src/memory_explorer.rs | 2 +- .../src/changeset/changeset.rs | 8 +-- packages/rs-platform-wallet/src/lib.rs | 4 +- .../rs-platform-wallet/src/wallet/apply.rs | 4 +- .../src/wallet/identity/mod.rs | 6 +- .../src/wallet/identity/network/discovery.rs | 4 +- .../src/wallet/identity/network/dpns.rs | 2 +- .../identity/network/dpns_marketplace.rs | 2 +- .../src/wallet/identity/network/loading.rs | 8 +-- .../state/managed_identity/identity_ops.rs | 2 +- .../identity/state/managed_identity/mod.rs | 11 +--- .../identity/state/managed_identity/sync.rs | 2 +- .../wallet/identity/state/manager/apply.rs | 2 +- .../src/wallet/identity/state/mod.rs | 2 +- .../wallet/identity/types/identity_status.rs | 21 +++++++ .../src/wallet/identity/types/key_storage.rs | 56 ------------------- .../src/wallet/identity/types/mod.rs | 4 +- 17 files changed, 49 insertions(+), 91 deletions(-) create mode 100644 packages/rs-platform-wallet/src/wallet/identity/types/identity_status.rs delete mode 100644 packages/rs-platform-wallet/src/wallet/identity/types/key_storage.rs diff --git a/packages/rs-platform-wallet-ffi/src/memory_explorer.rs b/packages/rs-platform-wallet-ffi/src/memory_explorer.rs index 1650e9dcf70..0f02b5249a6 100644 --- a/packages/rs-platform-wallet-ffi/src/memory_explorer.rs +++ b/packages/rs-platform-wallet-ffi/src/memory_explorer.rs @@ -26,7 +26,7 @@ use crate::handle::*; use crate::types::*; use crate::{check_ptr, unwrap_option_or_return}; use dpp::identity::accessors::IdentityGettersV0; -use platform_wallet::wallet::identity::state::managed_identity::IdentityStatus; +use platform_wallet::IdentityStatus; /// Per-wallet snapshot returned by [`platform_wallet_get_in_memory_summary`]. #[repr(C)] diff --git a/packages/rs-platform-wallet/src/changeset/changeset.rs b/packages/rs-platform-wallet/src/changeset/changeset.rs index 2527c9cd537..40fef58dcfd 100644 --- a/packages/rs-platform-wallet/src/changeset/changeset.rs +++ b/packages/rs-platform-wallet/src/changeset/changeset.rs @@ -47,9 +47,9 @@ use key_wallet::wallet::managed_wallet_info::asset_lock_builder::AssetLockFundin use crate::wallet::asset_lock::tracked::AssetLockStatus; use crate::changeset::merge::Merge; -use crate::wallet::identity::state::managed_identity::{ - BlockTime, DpnsNameInfo, IdentityStatus, ManagedIdentity, -}; +use crate::wallet::identity::state::managed_identity::ManagedIdentity; +use crate::wallet::identity::types::block_time::BlockTime; +use crate::wallet::identity::types::identity_status::{DpnsNameInfo, IdentityStatus}; use crate::wallet::identity::{ ContactProfileEntry, ContactRequest, DashPayProfile, EstablishedContact, PaymentEntry, }; @@ -749,7 +749,7 @@ impl Merge for CoreChangeSet { /// /// Carries the per-identity scalars (id / balance / revision + wallet /// metadata) but NOT the DPP `public_keys` map or the private -/// `KeyStorage`. Keys live in the sibling [`IdentityKeysChangeSet`] +/// key storage. Keys live in the sibling [`IdentityKeysChangeSet`] /// keyed by `(identity_id, key_id)` so that a simple scalar mutation /// (e.g. a balance refresh) serializes only the scalar fields without /// re-serializing every public-key byte and private-key data blob. diff --git a/packages/rs-platform-wallet/src/lib.rs b/packages/rs-platform-wallet/src/lib.rs index 2fedc74a1a8..6c693eac50e 100644 --- a/packages/rs-platform-wallet/src/lib.rs +++ b/packages/rs-platform-wallet/src/lib.rs @@ -83,8 +83,8 @@ pub use wallet::identity::{ derive_contact_payment_addresses, derive_contact_xpub, pubkey_binds_expected_key_data, unmask_account_reference, BlockTime, ContactProfileEntry, ContactRequest, ContactXpubData, DashPayProfile, DashPayState, DpnsNameInfo, EstablishedContact, IdentityLocation, - IdentityManager, IdentityStatus, KeyStorage, ManagedIdentity, PrivateKeyData, ProfileUpdate, - RegistrationIndex, DEFAULT_CONTACT_GAP_LIMIT, + IdentityManager, IdentityStatus, ManagedIdentity, ProfileUpdate, RegistrationIndex, + DEFAULT_CONTACT_GAP_LIMIT, }; pub use wallet::masternode_withdrawal::{ MasternodeWithdrawalKey, MasternodeWithdrawalKeys, MasternodeWithdrawalRequest, diff --git a/packages/rs-platform-wallet/src/wallet/apply.rs b/packages/rs-platform-wallet/src/wallet/apply.rs index abd89c534ec..02d0178b400 100644 --- a/packages/rs-platform-wallet/src/wallet/apply.rs +++ b/packages/rs-platform-wallet/src/wallet/apply.rs @@ -937,7 +937,7 @@ mod tests { #[test] fn round_trip_dpns_name() { - use crate::wallet::identity::state::managed_identity::DpnsNameInfo; + use crate::wallet::identity::types::identity_status::DpnsNameInfo; let wallet_a = build_test_wallet(); let mut info_a = empty_info(&wallet_a); @@ -991,7 +991,7 @@ mod tests { #[test] fn round_trip_block_time_updates() { - use crate::wallet::identity::state::managed_identity::BlockTime; + use crate::wallet::identity::types::block_time::BlockTime; let wallet_a = build_test_wallet(); let mut info_a = empty_info(&wallet_a); diff --git a/packages/rs-platform-wallet/src/wallet/identity/mod.rs b/packages/rs-platform-wallet/src/wallet/identity/mod.rs index 99e8ba106aa..fa27162a78a 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/mod.rs @@ -31,11 +31,11 @@ pub use crypto::{ }; pub use network::{DashPayView, IdentityWallet}; pub use state::{ - BlockTime, DashPayState, IdentityLocation, IdentityManager, ManagedIdentity, RegistrationIndex, + DashPayState, IdentityLocation, IdentityManager, ManagedIdentity, RegistrationIndex, }; +pub use types::block_time::BlockTime; pub use types::dashpay::profile::{calculate_avatar_hash, calculate_dhash_fingerprint}; pub use types::{ ContactProfileEntry, ContactRequest, DashPayProfile, DpnsNameInfo, EstablishedContact, - IdentityStatus, KeyStorage, PaymentDirection, PaymentEntry, PaymentStatus, PrivateKeyData, - ProfileUpdate, + IdentityStatus, PaymentDirection, PaymentEntry, PaymentStatus, ProfileUpdate, }; diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/discovery.rs b/packages/rs-platform-wallet/src/wallet/identity/network/discovery.rs index 5d6939babcb..eb85bdf3902 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/discovery.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/discovery.rs @@ -335,8 +335,8 @@ impl IdentityWallet { enrichment_deadline: Option, ) -> Result, PlatformWalletError> { use super::identity_handle::{derive_identity_auth_key_hash_from_master, MASTER_KEY_INDEX}; - use crate::wallet::identity::state::managed_identity::key_storage::DpnsNameInfo; - use crate::wallet::identity::state::managed_identity::key_storage::IdentityStatus; + use crate::wallet::identity::types::identity_status::DpnsNameInfo; + use crate::wallet::identity::types::identity_status::IdentityStatus; use dash_sdk::platform::types::identity::PublicKeyHash; use dash_sdk::platform::Fetch; diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/dpns.rs b/packages/rs-platform-wallet/src/wallet/identity/network/dpns.rs index 86f0b809bb2..85144a48ed1 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/dpns.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/dpns.rs @@ -17,7 +17,7 @@ use dpp::address_funds::AddressWitness; use dpp::platform_value::BinaryData; use crate::error::PlatformWalletError; -use crate::wallet::identity::types::key_storage::DpnsNameInfo; +use crate::wallet::identity::types::identity_status::DpnsNameInfo; use super::*; diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs b/packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs index 603aff74b8c..3cb874b4bee 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs @@ -52,7 +52,7 @@ use crate::changeset::{ DpnsNameSaleStatus, DpnsNameStateChangeSet, DpnsNameStateEntry, PersistenceError, }; use crate::error::PlatformWalletError; -use crate::wallet::identity::types::key_storage::DpnsNameInfo; +use crate::wallet::identity::types::identity_status::DpnsNameInfo; use super::document::allowed_signing_security_levels; use super::*; diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/loading.rs b/packages/rs-platform-wallet/src/wallet/identity/network/loading.rs index ecc7897ae4b..c3590407c50 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/loading.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/loading.rs @@ -166,8 +166,8 @@ impl IdentityWallet { identity_index: u32, source: LoadKeyHashSource<'_>, ) -> Result, PlatformWalletError> { - use crate::wallet::identity::state::managed_identity::key_storage::DpnsNameInfo; - use crate::wallet::identity::state::managed_identity::key_storage::IdentityStatus; + use crate::wallet::identity::types::identity_status::DpnsNameInfo; + use crate::wallet::identity::types::identity_status::IdentityStatus; use dash_sdk::platform::types::identity::PublicKeyHash; use dash_sdk::platform::Fetch; @@ -333,7 +333,7 @@ impl IdentityWallet { &self, identity_id: &Identifier, ) -> Result { - use crate::wallet::identity::state::managed_identity::key_storage::IdentityStatus; + use crate::wallet::identity::types::identity_status::IdentityStatus; use dash_sdk::platform::Fetch; // Verify identity exists in the manager. @@ -411,7 +411,7 @@ impl IdentityWallet { /// for its current DPNS usernames, and replaces the stored /// `dpns_names` list with the fresh results. pub async fn refresh_dpns_names(&self) -> Result<(), PlatformWalletError> { - use crate::wallet::identity::state::managed_identity::key_storage::DpnsNameInfo; + use crate::wallet::identity::types::identity_status::DpnsNameInfo; // Collect identity IDs so we don't hold the lock during network calls. let identity_ids: Vec = { diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/identity_ops.rs b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/identity_ops.rs index 0ecb4fe2c81..1a2ebe7d847 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/identity_ops.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/identity_ops.rs @@ -1,8 +1,8 @@ //! Core identity operations for ManagedIdentity -use super::key_storage::{DpnsNameInfo, IdentityStatus}; use super::ManagedIdentity; use crate::changeset::{IdentityChangeSet, IdentityEntry, IdentityKeyEntry, IdentityKeysChangeSet}; +use crate::wallet::identity::types::identity_status::{DpnsNameInfo, IdentityStatus}; use crate::wallet::persister::WalletPersister; use dpp::identity::accessors::IdentityGettersV0; use dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs index c1afd616375..87d5a6c10af 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs @@ -11,15 +11,8 @@ mod sync; pub use dashpay::DashPayState; -// `block_time` + `key_storage` moved to `crate::wallet::identity::types`. -// Re-export so every `impl ManagedIdentity` block below keeps working -// unchanged and external users can still reach them through the old -// `state::managed_identity::*` path. -pub use crate::wallet::identity::types::block_time::{self, BlockTime}; -pub use crate::wallet::identity::types::key_storage::{ - self, DpnsNameInfo, IdentityStatus, KeyStorage, PrivateKeyData, -}; - +use crate::wallet::identity::types::block_time::BlockTime; +use crate::wallet::identity::types::identity_status::{DpnsNameInfo, IdentityStatus}; use dpp::identity::Identity; /// A managed identity that combines an Identity with wallet-specific metadata. diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/sync.rs b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/sync.rs index 6e09b493126..66d98c1f96e 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/sync.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/sync.rs @@ -1,8 +1,8 @@ //! Synchronization and block time management for ManagedIdentity use super::ManagedIdentity; +use crate::wallet::identity::types::block_time::BlockTime; use crate::wallet::persister::WalletPersister; -use crate::BlockTime; use dpp::prelude::TimestampMillis; impl ManagedIdentity { diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/manager/apply.rs b/packages/rs-platform-wallet/src/wallet/identity/state/manager/apply.rs index 7317eb129cf..7bd66620de7 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/state/manager/apply.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/state/manager/apply.rs @@ -207,7 +207,7 @@ impl IdentityManager { mod tests { use super::*; use crate::changeset::IdentityEntry; - use crate::wallet::identity::state::managed_identity::IdentityStatus; + use crate::wallet::identity::types::identity_status::IdentityStatus; use std::collections::{BTreeMap, BTreeSet}; fn entry(id: Identifier, labels: &[&str]) -> IdentityEntry { diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/mod.rs b/packages/rs-platform-wallet/src/wallet/identity/state/mod.rs index affa70d0156..e37a78c0946 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/state/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/state/mod.rs @@ -8,7 +8,7 @@ pub mod managed_identity; pub mod manager; -pub use managed_identity::{BlockTime, DashPayState, ManagedIdentity}; +pub use managed_identity::{DashPayState, ManagedIdentity}; pub use manager::IdentityLocation; pub use manager::IdentityManager; pub use manager::RegistrationIndex; diff --git a/packages/rs-platform-wallet/src/wallet/identity/types/identity_status.rs b/packages/rs-platform-wallet/src/wallet/identity/types/identity_status.rs new file mode 100644 index 00000000000..be35101e9b3 --- /dev/null +++ b/packages/rs-platform-wallet/src/wallet/identity/types/identity_status.rs @@ -0,0 +1,21 @@ +//! Identity lifecycle status and DPNS name metadata for managed identities. + +/// Identity lifecycle status on Platform. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub enum IdentityStatus { + #[default] + Unknown, + PendingCreation, + Active, + FailedCreation, + NotFound, +} + +/// DPNS username associated with an identity. +#[derive(Debug, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct DpnsNameInfo { + pub label: String, + pub acquired_at: Option, +} diff --git a/packages/rs-platform-wallet/src/wallet/identity/types/key_storage.rs b/packages/rs-platform-wallet/src/wallet/identity/types/key_storage.rs deleted file mode 100644 index 8dee1a702b9..00000000000 --- a/packages/rs-platform-wallet/src/wallet/identity/types/key_storage.rs +++ /dev/null @@ -1,56 +0,0 @@ -//! Key storage types, identity status, and DPNS name metadata for managed identities. - -use dpp::identity::IdentityPublicKey; -use dpp::identity::KeyID; -use key_wallet::bip32::DerivationPath; -use std::collections::BTreeMap; -use zeroize::Zeroizing; - -/// How a private key is stored/resolved. -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum PrivateKeyData { - /// Raw key bytes in memory (zeroized on drop). - Clear(Zeroizing<[u8; 32]>), - /// Derive on-demand from wallet seed at this path. Carries the - /// DIP-9 `(identity_index, key_index)` pair alongside the fully - /// materialized `derivation_path` so callers that need either - /// form get it without reparsing. - AtWalletDerivationPath { - wallet_id: [u8; 32], - derivation_path: DerivationPath, - /// DIP-9 identity index. - identity_index: u32, - /// DIP-9 key index within the identity. - key_index: u32, - }, -} - -/// Identity lifecycle status on Platform. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -pub enum IdentityStatus { - #[default] - Unknown, - PendingCreation, - Active, - FailedCreation, - NotFound, -} - -/// DPNS username associated with an identity. -#[derive(Debug, Clone, PartialEq, Eq)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -pub struct DpnsNameInfo { - pub label: String, - pub acquired_at: Option, -} - -/// Private key storage mapping KeyID to public key metadata + private key data. -/// -/// Lives only in transient places — the `IdentityKeysChangeSet` apply -/// path constructs one per replay, the FFI key-preview path uses one -/// internally — but is no longer carried as a field on `ManagedIdentity`. -/// Private keys belong in the iOS Keychain on the client side; the Rust -/// side derives them on demand from the wallet seed via the DIP-9 path -/// recorded in `PrivateKeyData::AtWalletDerivationPath`. -pub type KeyStorage = BTreeMap; diff --git a/packages/rs-platform-wallet/src/wallet/identity/types/mod.rs b/packages/rs-platform-wallet/src/wallet/identity/types/mod.rs index fb53ae73c89..49ba997ce8b 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/types/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/types/mod.rs @@ -7,11 +7,11 @@ pub mod block_time; pub mod dashpay; -pub mod key_storage; +pub mod identity_status; pub use block_time::BlockTime; pub use dashpay::{ ContactProfileEntry, ContactRequest, DashPayProfile, EstablishedContact, PaymentDirection, PaymentEntry, PaymentStatus, ProfileUpdate, }; -pub use key_storage::{DpnsNameInfo, IdentityStatus, KeyStorage, PrivateKeyData}; +pub use identity_status::{DpnsNameInfo, IdentityStatus}; From 7efe35bf0a655a62ad28ad0601c2e7a904eb716c Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Wed, 9 Sep 2026 12:16:41 +0200 Subject: [PATCH 05/16] refactor(platform-wallet): remove dead broadcast paths and DapiBroadcaster MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit None of these had a call site in packages/ — not from Rust, and not through the FFI, Swift or Kotlin: - `CoreWallet::broadcast_transaction` — one hit in the entire repo, its own definition. - `CoreWallet::broadcast_transaction_releasing_reservation` and its only callee `reservations::broadcast_releasing_on_rejection`, together with two tests (`broadcast_releases_reservation_on_rejection`, `broadcast_keeps_reservation_on_ambiguous_failure`) and the `build_signed_tx` helper that existed solely for them. The finalized-handle tests already cover those properties. - `DapiBroadcaster` — 45 lines of a second trait implementation, never instantiated. `PlatformWallet.core` is `CoreWallet`, and the FFI/JNI use `SpvBroadcaster` exclusively. Eight comment sites justified the pending-spend fence design by "the DapiBroadcaster path"; rewritten to describe the shape (a broadcaster returning before mempool injection) rather than naming a type that no longer exists. - `PlatformEventManager::add_handler` — every `PlatformEventManager::new` site passes the full handler list up front. - `is_instant_lock_timeout` — a one-line `matches!` whose only consumer was its own test; production matches `FinalityTimeout` directly. The `signed_payment_registry` doc stops calling the removed method "the regular send path". Co-Authored-By: Claude Opus 5 --- .../rs-platform-wallet/src/broadcaster.rs | 67 +---- packages/rs-platform-wallet/src/error.rs | 17 -- packages/rs-platform-wallet/src/events.rs | 11 - .../src/wallet/asset_lock/manager.rs | 9 +- .../src/wallet/asset_lock/orchestration.rs | 2 +- .../src/wallet/asset_lock/sync/recovery.rs | 4 +- .../src/wallet/core/broadcast.rs | 236 +----------------- .../src/wallet/core/generation.rs | 2 +- .../src/wallet/core/spend_observer.rs | 2 +- .../wallet/identity/network/registration.rs | 47 ---- .../src/wallet/reservations.rs | 53 +--- .../src/wallet/signed_payment_registry.rs | 2 +- 12 files changed, 29 insertions(+), 423 deletions(-) diff --git a/packages/rs-platform-wallet/src/broadcaster.rs b/packages/rs-platform-wallet/src/broadcaster.rs index 2088628a1a6..42f59c44323 100644 --- a/packages/rs-platform-wallet/src/broadcaster.rs +++ b/packages/rs-platform-wallet/src/broadcaster.rs @@ -8,9 +8,11 @@ //! subset of peers while withheld from the rest, and a withheld peer //! announcing the txid back, an InstantSend lock, or a confirmation proves //! the network accepted it. Trustless; no DAPI involvement. -//! - [`DapiBroadcaster`] (fallback for wallets without an SPV runtime): -//! submission via DAPI's gRPC endpoint, with every failure conservatively -//! classified as [`BroadcastError::MaybeSent`]. +//! +//! A broadcaster whose transport returns before the transaction reaches a +//! mempool would need every failure conservatively classified as +//! [`BroadcastError::MaybeSent`]; the pending-spend fence below is written so +//! such a transport stays safe. use std::sync::Arc; use std::time::Duration; @@ -26,8 +28,7 @@ use crate::spv::SpvRuntime; /// never entered the network or its acceptance remains unknown. /// /// The classification decides whether the transaction's reserved inputs are -/// safe to release for an immediate retry (see -/// `wallet::reservations::broadcast_releasing_on_rejection`). +/// safe to release for an immediate retry. #[derive(Debug, thiserror::Error)] pub enum BroadcastError { /// The network definitively did not take the transaction — a rejection @@ -94,65 +95,13 @@ pub trait TransactionBroadcaster: Send + Sync + 'static { /// with no ceiling there is a deadlock, not a delay. /// /// The default is "always ready" — correct for any broadcaster with no - /// startup phase of its own, such as [`DapiBroadcaster`], whose gRPC - /// requests carry their own connection handling. + /// startup phase of its own, such as one whose requests carry their own + /// connection handling. async fn wait_until_ready(&self, _timeout: Duration) -> bool { true } } -/// Broadcasts transactions via Platform's DAPI gRPC endpoint. -/// -/// Used by default when no SPV runtime is available. -pub struct DapiBroadcaster { - sdk: Arc, -} - -impl DapiBroadcaster { - pub fn new(sdk: Arc) -> Self { - Self { sdk } - } -} - -#[async_trait] -impl TransactionBroadcaster for DapiBroadcaster { - async fn broadcast(&self, transaction: &Transaction) -> Result { - use dash_sdk::dapi_client::{DapiRequestExecutor, IntoInner, RequestSettings}; - use dash_sdk::dapi_grpc::core::v0::BroadcastTransactionRequest; - use dashcore::consensus; - - let tx_bytes = consensus::serialize(transaction); - - let request = BroadcastTransactionRequest { - transaction: tx_bytes, - allow_high_fees: false, - bypass_limits: false, - }; - - // Every DAPI failure is classified `MaybeSent`: `sdk.execute` retries - // across nodes internally (RequestSettings::default()), so the error - // surfaced here is only the *last* attempt's — an earlier attempt may - // have delivered the transaction even though the response was lost - // (the classic shape being a node that accepts the tx while its gRPC - // response times out, followed by a retry that fails differently). - // Distinguishing a genuinely pre-send rejection would require - // disabling the internal retries and inspecting transport errors; - // until then the conservative classification keeps reserved inputs - // safe from double-spends at the cost of holding them for the - // reservation TTL. - let _response = self - .sdk - .execute(request, RequestSettings::default()) - .await - .into_inner() - .map_err(|e| BroadcastError::MaybeSent { - reason: format!("DAPI broadcast failed: {}", e), - })?; - - Ok(transaction.txid()) - } -} - /// The SPV broadcast channel: send through P2P peers and await dash-spv's /// network-acceptance verdict (rust-dashcore#913). #[async_trait] diff --git a/packages/rs-platform-wallet/src/error.rs b/packages/rs-platform-wallet/src/error.rs index 0f65ea69bae..2dcb3b70a62 100644 --- a/packages/rs-platform-wallet/src/error.rs +++ b/packages/rs-platform-wallet/src/error.rs @@ -919,23 +919,6 @@ pub fn is_instant_lock_proof_invalid(error: &dash_sdk::Error) -> bool { ) } -/// Check whether a platform-wallet error represents a *Core-side* -/// InstantSend lock timeout (the asset-lock manager waited the full -/// timeout for an IS-lock proof and never observed one). -/// -/// Companion to [`is_instant_lock_proof_invalid`] (which detects -/// **Platform-side** rejection of an IS proof after one was obtained). -/// Both surfaces trigger the same fallback path in the registration / -/// top-up flow: upgrade the asset-lock to a ChainLock proof and retry. -/// -/// The IS-timeout shape comes from -/// [`AssetLockManager::wait_for_proof`](crate::wallet::asset_lock::manager::AssetLockManager), -/// which emits `PlatformWalletError::FinalityTimeout(Txid)` when the -/// 300-second IS deadline elapses. -pub fn is_instant_lock_timeout(error: &PlatformWalletError) -> bool { - matches!(error, PlatformWalletError::FinalityTimeout(_)) -} - /// Extract the `InvalidAssetLockProofCoreChainHeightError` (DPP /// consensus code 10506) from an SDK error if Platform rejected a /// ChainLock asset-lock proof because Platform's diff --git a/packages/rs-platform-wallet/src/events.rs b/packages/rs-platform-wallet/src/events.rs index c329c5a32f5..713e0f66388 100644 --- a/packages/rs-platform-wallet/src/events.rs +++ b/packages/rs-platform-wallet/src/events.rs @@ -107,10 +107,8 @@ pub trait PlatformEventHandler: EventHandler { /// Dispatches events to all registered [`PlatformEventHandler`]s. /// /// Passed to `DashSpvClient` as the `EventHandler` (via `Arc`). -/// Supports dynamic handler registration via [`add_handler`](Self::add_handler). /// /// Read path (every event): one atomic pointer load, then iterate. -/// Write path (add_handler): clone Vec + atomic swap — rare, not on SPV hot path. pub struct PlatformEventManager { handlers: ArcSwap>>, } @@ -123,15 +121,6 @@ impl PlatformEventManager { } } - /// Register an additional handler. Lock-free for readers. - pub fn add_handler(&self, handler: Arc) { - self.handlers.rcu(|current| { - let mut new = (**current).clone(); - new.push(handler.clone()); - new - }); - } - /// Dispatch a platform-address sync completion to every handler. /// /// Not on the SPV hot path — called once per sync pass (~15s). diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/manager.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/manager.rs index b6d223b3239..da538cf0ef6 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/manager.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/manager.rs @@ -39,12 +39,9 @@ pub struct AssetLockManager { /// Used by `wait_for_proof()` and `wait_for_chain_lock()`. pub(super) lock_notify: Arc, /// Transaction broadcaster — pluggable so the same `AssetLockManager` - /// works with different broadcast backends: - /// - /// - [`DapiBroadcaster`](crate::broadcaster::DapiBroadcaster) — gRPC via - /// Platform DAPI (default for standalone wallets without SPV). - /// - [`SpvBroadcaster`](crate::broadcaster::SpvBroadcaster) — P2P via SPV - /// peers (used when managed by `PlatformWalletManager` with SPV enabled). + /// works with different broadcast backends. Production uses + /// [`SpvBroadcaster`](crate::broadcaster::SpvBroadcaster) — P2P via SPV + /// peers. /// /// Injected at construction by `PlatformWallet::new()`. The caller /// (typically `PlatformWalletManager`) decides which implementation to use. diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/orchestration.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/orchestration.rs index 75512084f8b..4ea7a20c626 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/orchestration.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/orchestration.rs @@ -99,7 +99,7 @@ pub(crate) const RECONCILIATION_CHAIN_LOCK_TIMEOUT: Duration = Duration::from_se /// The unbounded `wait_for_proof(None)` used by the funding flows is /// justified by the transaction being *known* broadcast — finality is then /// only a matter of time. A `MaybeSent` verdict does not establish that: -/// `DapiBroadcaster` classifies every failure as `MaybeSent`, and the SPV +/// A gRPC-style broadcaster classifies every failure as `MaybeSent`, and the SPV /// broadcaster reports `Rejected` only for `NotConnected`, so a genuinely /// rejected transaction is indistinguishable from an accepted one. Waiting /// without a bound on that signal converts a ~30s broadcast failure into a diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs index e53d9f8a4b5..caa8c2ebc3a 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs @@ -1118,7 +1118,7 @@ impl AssetLockManager { // does with the identical signal. // // `MaybeSent` is however ALSO what the broadcaster reports - // for a genuinely rejected transaction: `DapiBroadcaster` + // for a genuinely rejected transaction: a gRPC-style broadcaster // classifies every failure that way by construction, and the // SPV broadcaster only reaches `Rejected` on `NotConnected`. // So the advance above cannot be read as evidence the tx is @@ -4472,7 +4472,7 @@ mod tests { /// must not hang. /// /// `MaybeSent` is the broadcaster's verdict for a genuinely rejected - /// transaction as much as for an accepted one — `DapiBroadcaster` + /// transaction as much as for an accepted one — a gRPC-style broadcaster /// classifies every failure that way, and the SPV broadcaster reaches /// `Rejected` only on `NotConnected`. So advancing to `Broadcast` and /// then waiting with `wait_for_proof(None)` — which is what the three diff --git a/packages/rs-platform-wallet/src/wallet/core/broadcast.rs b/packages/rs-platform-wallet/src/wallet/core/broadcast.rs index 1879f18346c..ef59dbc4571 100644 --- a/packages/rs-platform-wallet/src/wallet/core/broadcast.rs +++ b/packages/rs-platform-wallet/src/wallet/core/broadcast.rs @@ -1,11 +1,10 @@ use dashcore::Transaction; -use key_wallet::account::account_type::StandardAccountType; use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; use key_wallet::ReservationToken; use super::SignedCoreTransaction; use crate::broadcaster::{BroadcastError, TransactionBroadcaster}; -use crate::wallet::reservations::{broadcast_releasing_on_rejection, reservation_expired}; +use crate::wallet::reservations::reservation_expired; use crate::{CoreWallet, PlatformWalletError}; /// Outcome of [`CoreWallet::dispatch_unexpired`] — the guarded @@ -41,7 +40,7 @@ impl CoreWallet { /// await, the guard starves the pipeline and every dispatch rides the /// full acceptance timeout to an ambiguous verdict while the whole /// manager stalls behind tokio's write-preferring queue. (Same - /// lock-free shape as `broadcast_releasing_on_rejection`.) + /// lock-free shape as the rejection-release path.) /// /// What spans the await instead is the **in-broadcast pin** /// ([`WalletGeneration::pin_in_broadcast`](super::WalletGeneration::pin_in_broadcast)), @@ -63,7 +62,7 @@ impl CoreWallet { /// wallet has observed the spend", and the two differ per broadcaster: /// `SpvBroadcaster` injects into dash-spv's local mempool pipeline, so the /// inputs leave this wallet's selectable set within milliseconds; - /// `DapiBroadcaster::broadcast` only awaits `sdk.execute` and injects + /// a gRPC-style broadcast only awaits the submit call and injects /// nothing, so on that path the inputs are still selectable while the /// transaction is in flight. So: /// @@ -85,7 +84,7 @@ impl CoreWallet { /// historical catch-up: the wallet advances that height by thousands of /// blocks in seconds, and those blocks were mined BEFORE this transaction /// was submitted, so they are not evidence that it has been seen or - /// dropped. On the `DapiBroadcaster` path — which returns from + /// dropped. On a gRPC-style path — which returns from /// `sdk.execute` without injecting anything into local wallet state — the /// input then becomes reselectable while the transaction is in flight. /// @@ -263,64 +262,6 @@ impl CoreWallet { } } - /// Broadcast a signed transaction to the network. - /// - /// Transactions can be built and signed with key-wallet's - /// [`TransactionBuilder`](key_wallet::wallet::managed_wallet_info::transaction_builder::TransactionBuilder) - /// before being passed here; this method only broadcasts the - /// caller-supplied signed transaction. - /// - /// Delegates to the injected [`TransactionBroadcaster`] which may use - /// SPV (P2P) or DAPI (gRPC) depending on how the wallet was constructed. - /// - /// Returns the transaction ID on success. - /// - /// This plain form does **not** reconcile the funding account's UTXO - /// reservation on failure. Prefer - /// [`broadcast_transaction_releasing_reservation`](Self::broadcast_transaction_releasing_reservation) - /// for the build-then-broadcast send path, where a `build_signed` - /// reserved the selected inputs and a failed broadcast must release them. - pub async fn broadcast_transaction( - &self, - transaction: &Transaction, - ) -> Result { - self.broadcaster - .broadcast(transaction) - .await - .map_err(Into::into) - } - - /// Broadcast a signed transaction, reconciling the funding account's UTXO - /// reservation on failure. - /// - /// `build_signed` reserves the selected inputs in the funding account's - /// `ReservationSet` and leaves the reservation held on success (expecting - /// this broadcast). On a definitive rejection the reservation is released - /// so an immediate retry can reselect those inputs; on an ambiguous - /// failure it is kept. See - /// [`broadcast_releasing_on_rejection`](crate::wallet::reservations::broadcast_releasing_on_rejection) - /// for the full rationale. - /// - /// `account_type`/`account_index` identify the funding account handed to - /// `set_funding` when the transaction was built. - pub async fn broadcast_transaction_releasing_reservation( - &self, - account_type: StandardAccountType, - account_index: u32, - transaction: &Transaction, - ) -> Result { - broadcast_releasing_on_rejection( - self.broadcaster.as_ref(), - &self.wallet_manager, - &self.wallet_id, - account_type, - account_index, - transaction, - ) - .await - .map_err(Into::into) - } - /// Broadcast a raw signed `transaction` for the deferred-payment /// [`SignedPaymentRegistry`](crate::SignedPaymentRegistry), reconciling the /// funding reservation on failure. @@ -328,11 +269,9 @@ impl CoreWallet { /// Same policy as /// [`broadcast_finalized_transaction`](Self::broadcast_finalized_transaction): /// a definitive [`BroadcastError::Rejected`] releases the reservation for an - /// immediate rebuild; an ambiguous `MaybeSent` keeps it. Unlike the - /// `StandardAccountType`-typed - /// [`broadcast_transaction_releasing_reservation`](Self::broadcast_transaction_releasing_reservation) - /// used by the immediate send path, this takes an [`AccountTypePreference`] - /// so it ALSO reconciles a CoinJoin-funded deferred payment — one whose + /// immediate rebuild; an ambiguous `MaybeSent` keeps it. It takes an + /// [`AccountTypePreference`] rather than a `StandardAccountType`, so it + /// ALSO reconciles a CoinJoin-funded deferred payment — one whose /// `build_signed`/`finalize` reserved the selected inputs but which has no /// `StandardAccountType`, and whose reservation would otherwise stay held /// until the TTL backstop. @@ -396,17 +335,13 @@ mod tests { use super::GuardedDispatch; use dashcore::{Address as DashAddress, Network, Transaction}; use key_wallet::account::account_type::StandardAccountType; - use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; - use key_wallet::signer::Signer; - use key_wallet::wallet::managed_wallet_info::coin_selection::SelectionStrategy; use key_wallet::wallet::managed_wallet_info::transaction_builder::TransactionBuilder; use key_wallet::wallet::managed_wallet_info::transaction_building::AccountTypePreference; use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; use crate::broadcaster::TransactionBroadcaster; use crate::test_support::{ - funded_wallet_manager, AlwaysMaybeSentBroadcaster, AlwaysOkBroadcaster, - RejectFirstBroadcaster, WalletSigner, + funded_wallet_manager, AlwaysOkBroadcaster, RejectFirstBroadcaster, WalletSigner, }; use crate::wallet::core::{CoreWallet, SpendObservationHandler}; use crate::wallet::platform_wallet::WalletId; @@ -431,68 +366,6 @@ mod tests { (core, signer, outputs) } - /// Build and sign a payment the way the split send path does: `build_signed` - /// reserves the selected inputs in the funding account's `ReservationSet`, - /// leaving the reservation held for the subsequent broadcast. Mirrors the - /// FFI `core_wallet_tx_builder_*` sequence. - async fn build_signed_tx( - core: &CoreWallet, - account_type: StandardAccountType, - account_index: u32, - outputs: &[(DashAddress, u64)], - signer: &S, - ) -> Result { - let mut wm = core.wallet_manager.write().await; - let (wallet, info) = wm - .get_wallet_and_info_mut(&core.wallet_id()) - .expect("wallet present in manager"); - - let current_height = info.core_wallet.synced_height(); - - let (managed_account, account) = match account_type { - StandardAccountType::BIP44Account => ( - info.core_wallet - .accounts - .standard_bip44_accounts - .get_mut(&account_index) - .expect("bip44 managed account"), - wallet - .accounts - .standard_bip44_accounts - .get(&account_index) - .expect("bip44 account"), - ), - StandardAccountType::BIP32Account => ( - info.core_wallet - .accounts - .standard_bip32_accounts - .get_mut(&account_index) - .expect("bip32 managed account"), - wallet - .accounts - .standard_bip32_accounts - .get(&account_index) - .expect("bip32 account"), - ), - }; - - let mut builder = TransactionBuilder::new() - .set_current_height(current_height) - .set_selection_strategy(SelectionStrategy::LargestFirst) - .add_funding(managed_account, account); - for (addr, amount) in outputs { - builder = builder.add_output(addr, *amount); - } - - let (tx, _fee) = builder - .build_signed(signer, |addr| { - managed_account.address_derivation_path(&addr) - }) - .await - .map_err(|e| PlatformWalletError::TransactionBuild(e.to_string()))?; - Ok(tx) - } - /// Atomically fund + reserve + sign a `SignedCoreTransaction` the way the /// finalized-handle path (`core_wallet_tx_builder_finalize`) does, capturing /// the reservation's stamp height on the returned handle. @@ -939,7 +812,7 @@ mod tests { // height-anchored bound would only defer it. The assertion holds // regardless of the chain clock, because the fence is waiting for an // observed spend that this mock manager — which runs no mempool - // pipeline, exactly like the `DapiBroadcaster` path — never produces. + // pipeline, exactly like a gRPC-style path — never produces. let still_fenced = try_finalize_tx(&core, AccountTypePreference::BIP44, &outputs, &signer).await; expect_mid_broadcast( @@ -968,7 +841,7 @@ mod tests { /// /// The broadcaster is `AlwaysOk`: the transaction is ACCEPTED, so it is /// certainly on the wire. The manager runs no mempool pipeline, which is - /// the `DapiBroadcaster` shape — `sdk.execute` returns without injecting + /// the gRPC-style shape — the submit call returns without injecting /// anything locally — so nothing has observed the spend. #[tokio::test] async fn fence_survives_a_full_historical_catch_up_advance() { @@ -1327,7 +1200,7 @@ mod tests { /// /// This drives that exact sequence through the real send path: accept the /// transaction (`AlwaysOk`, and this manager runs no mempool pipeline — the - /// `DapiBroadcaster` shape, so nothing observes the spend), run catch-up far + /// gRPC-style shape, so nothing observes the spend), run catch-up far /// past key-wallet's reservation TTL, bring due every timeout the fence might /// carry, and build again. Under a deadline-bearing fence that second build /// SUCCEEDS and returns a second signed transaction spending the same @@ -1494,91 +1367,4 @@ mod tests { }); core.abandon_transaction(&rebuilt).await; } - - /// A pre-send broadcast rejection must release the UTXO reservation taken - /// while building the transaction, so an immediate retry can reselect those - /// inputs instead of failing with spurious insufficient funds until the TTL - /// backstop. Covers both funds-account arms of the release path. - #[tokio::test] - async fn broadcast_releases_reservation_on_rejection() { - for account_type in [ - StandardAccountType::BIP44Account, - StandardAccountType::BIP32Account, - ] { - let broadcaster = Arc::new(RejectFirstBroadcaster::new()); - let (core, signer, outputs) = funded_core_wallet(account_type, broadcaster).await; - - // First attempt: build + sign reserve the input, broadcast is rejected. - let tx = build_signed_tx(&core, account_type, 0, &outputs, &signer) - .await - .expect("first build should succeed"); - let first = core - .broadcast_transaction_releasing_reservation(account_type, 0, &tx) - .await; - assert!( - matches!(first, Err(PlatformWalletError::TransactionBroadcast(_))), - "first broadcast should surface the rejection for {account_type:?}, got {first:?}" - ); - - // Immediate retry: the build only succeeds if the failed broadcast - // released the reservation. With the leak, coin selection sees no - // spendable UTXO and the build fails. - let retry_tx = build_signed_tx(&core, account_type, 0, &outputs, &signer).await; - assert!( - retry_tx.is_ok(), - "retry build after a released reservation should succeed for \ - {account_type:?}, got {retry_tx:?}" - ); - let second = core - .broadcast_transaction_releasing_reservation( - account_type, - 0, - &retry_tx.expect("retry tx"), - ) - .await; - assert!( - second.is_ok(), - "retry broadcast should succeed for {account_type:?}, got {second:?}" - ); - } - } - - /// An *ambiguous* broadcast failure — the network may already have accepted - /// the transaction — must NOT release the reservation: retrying would risk a - /// double-spend. The reservation is kept, so an immediate retry fails at the - /// build stage (no spendable UTXO) rather than reaching broadcast again. - #[tokio::test] - async fn broadcast_keeps_reservation_on_ambiguous_failure() { - for account_type in [ - StandardAccountType::BIP44Account, - StandardAccountType::BIP32Account, - ] { - let broadcaster = Arc::new(AlwaysMaybeSentBroadcaster); - let (core, signer, outputs) = funded_core_wallet(account_type, broadcaster).await; - - let tx = build_signed_tx(&core, account_type, 0, &outputs, &signer) - .await - .expect("first build should succeed"); - let first = core - .broadcast_transaction_releasing_reservation(account_type, 0, &tx) - .await; - assert!( - matches!( - first, - Err(PlatformWalletError::TransactionBroadcastUnconfirmed(_)) - ), - "first broadcast should surface the ambiguous failure for \ - {account_type:?}, got {first:?}" - ); - - // Reservation kept: the retry cannot reselect the reserved input and - // fails while building, never reaching the broadcaster again. - let second = build_signed_tx(&core, account_type, 0, &outputs, &signer).await; - assert!( - matches!(second, Err(PlatformWalletError::TransactionBuild(_))), - "retry build must fail with the reservation kept for \ - {account_type:?}, got {second:?}" - ); - } - } } diff --git a/packages/rs-platform-wallet/src/wallet/core/generation.rs b/packages/rs-platform-wallet/src/wallet/core/generation.rs index 5224985f838..26084308198 100644 --- a/packages/rs-platform-wallet/src/wallet/core/generation.rs +++ b/packages/rs-platform-wallet/src/wallet/core/generation.rs @@ -97,7 +97,7 @@ pub struct WalletGeneration { /// The second phase exists because dispatch returning does not mean the /// wallet has observed the spend. `SpvBroadcaster` injects the transaction /// into dash-spv's local mempool pipeline, so its inputs leave this wallet's - /// selectable set within milliseconds — but `DapiBroadcaster::broadcast` only + /// selectable set within milliseconds — but a broadcaster that returns before mempool injection only /// awaits `sdk.execute` and performs no local injection at all, so both an /// accepted response and an ambiguous `MaybeSent` return with the input still /// selectable here while the transaction is in flight. Dropping the fence at diff --git a/packages/rs-platform-wallet/src/wallet/core/spend_observer.rs b/packages/rs-platform-wallet/src/wallet/core/spend_observer.rs index 8f239a22fe5..cad3365e13f 100644 --- a/packages/rs-platform-wallet/src/wallet/core/spend_observer.rs +++ b/packages/rs-platform-wallet/src/wallet/core/spend_observer.rs @@ -27,7 +27,7 @@ use crate::wallet::PlatformWallet; /// A dispatch that returns anything but a definitive pre-send rejection leaves /// its inputs fenced, because the broadcaster's return says "this may be on the /// network", not "this wallet has seen the spend" — and on the -/// `DapiBroadcaster` path the two are far apart, since `sdk.execute` injects +/// gRPC-style path the two are far apart, since the submit call injects /// nothing into local wallet state. Something has to end that fence, and /// elapsed `last_processed_height` cannot be it: catch-up advances the chain /// clock over blocks mined *before* the transaction was submitted, so an diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/registration.rs b/packages/rs-platform-wallet/src/wallet/identity/network/registration.rs index 9cdb0e59977..f6d24e428f2 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/registration.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/registration.rs @@ -580,50 +580,3 @@ impl IdentityWallet { // --------------------------------------------------------------------------- // Tests -// --------------------------------------------------------------------------- - -#[cfg(test)] -mod tests { - use super::*; - use crate::error::is_instant_lock_timeout; - use dashcore::{OutPoint, Txid}; - - /// Pins the IS-timeout discriminator: only `FinalityTimeout` - /// matches, so the IS→CL fallback arms route exactly the cases - /// we expect. Companion to `is_instant_lock_proof_invalid` - /// (which discriminates SDK errors at the Platform-rejection - /// boundary). - #[test] - fn is_instant_lock_timeout_only_matches_finality_timeout() { - let timeout = PlatformWalletError::FinalityTimeout(OutPoint { - txid: Txid::from([0u8; 32]), - vout: 0, - }); - assert!( - is_instant_lock_timeout(&timeout), - "FinalityTimeout must route to IS→CL fallback" - ); - - // Adjacent error shapes that share the asset-lock domain but - // are NOT timeouts — must NOT trigger the fallback. - let expired = PlatformWalletError::AssetLockExpired("CL not yet available".to_string()); - assert!( - !is_instant_lock_timeout(&expired), - "AssetLockExpired must NOT trigger IS→CL fallback \ - (the lock is already past the CL grace window)" - ); - - let not_cl = PlatformWalletError::AssetLockNotChainLocked("missing".to_string()); - assert!( - !is_instant_lock_timeout(¬_cl), - "AssetLockNotChainLocked must NOT trigger IS→CL fallback" - ); - - let wait_err = PlatformWalletError::AssetLockProofWait("not tracked".to_string()); - assert!( - !is_instant_lock_timeout(&wait_err), - "AssetLockProofWait must NOT trigger IS→CL fallback \ - (wallet-state mismatch is a hard failure)" - ); - } -} diff --git a/packages/rs-platform-wallet/src/wallet/reservations.rs b/packages/rs-platform-wallet/src/wallet/reservations.rs index fb4ddd7676b..a5709bdd386 100644 --- a/packages/rs-platform-wallet/src/wallet/reservations.rs +++ b/packages/rs-platform-wallet/src/wallet/reservations.rs @@ -26,13 +26,11 @@ //! `Built` row first); those call the broadcaster directly and then //! [`release_reservation_after_rejected_broadcast`]. -use dashcore::{Transaction, Txid}; -use key_wallet::account::account_type::StandardAccountType; +use dashcore::Transaction; use key_wallet::account::AccountType; use key_wallet_manager::WalletManager; use tokio::sync::RwLock; -use crate::broadcaster::{BroadcastError, TransactionBroadcaster}; use crate::wallet::platform_wallet::{PlatformWalletInfo, WalletId}; /// Maximum age, in `last_processed_height` blocks, of a held funding @@ -126,55 +124,6 @@ pub(crate) fn reservation_expired(registered_height: u32, current_height: Option } } -/// Broadcast `tx` and reconcile the funding account's UTXO reservation on -/// failure. -/// -/// On [`BroadcastError::Rejected`] — Core definitively did not accept the -/// transaction — the inputs reserved by the preceding `build_signed` are -/// released so an immediate retry can reselect them instead of failing with -/// spurious insufficient funds until the reservation-TTL backstop. On -/// [`BroadcastError::MaybeSent`] the reservation is intentionally kept: -/// releasing inputs of a transaction that may already be propagating invites -/// a double-spend on retry. -/// -/// `account_type`/`account_index` identify the funding account whose -/// `ReservationSet` holds the inputs — the same account handed to -/// `set_funding` when the transaction was built. -/// -/// Returns the still-typed [`BroadcastError`]; `?` converts it into -/// [`PlatformWalletError`](crate::PlatformWalletError) at the call sites. -pub(crate) async fn broadcast_releasing_on_rejection( - broadcaster: &B, - wallet_manager: &RwLock>, - wallet_id: &WalletId, - account_type: StandardAccountType, - account_index: u32, - tx: &Transaction, -) -> Result { - match broadcaster.broadcast(tx).await { - Ok(txid) => Ok(txid), - Err(e) => { - if matches!(e, BroadcastError::Rejected { .. }) { - release_reservation_after_rejected_broadcast( - wallet_manager, - wallet_id, - &[AccountType::Standard { - index: account_index, - standard_account_type: account_type, - }], - tx, - // The generic send path doesn't thread the build's - // reservation token yet; keep its historical - // unconditional release. - None, - ) - .await; - } - Err(e) - } - } -} - /// Release the funding accounts' UTXO reservations for `tx` after its /// broadcast came back [`BroadcastError::Rejected`]. /// diff --git a/packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs b/packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs index d3c37ead5ea..a0051392d1f 100644 --- a/packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs +++ b/packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs @@ -2,7 +2,7 @@ //! lifecycle (BIP70 / BIP270 "sign now, submit on merchant ack"). //! //! The regular send path -//! ([`CoreWallet::broadcast_transaction_releasing_reservation`](crate::CoreWallet::broadcast_transaction_releasing_reservation)) +//! ([`CoreWallet::broadcast_finalized_transaction`](crate::CoreWallet::broadcast_finalized_transaction)) //! builds, signs, and broadcasts in one uninterrupted step. BIP70-style flows //! must split that: sign now (reserving the funding UTXOs), hand the raw bytes //! to a merchant server, and broadcast **only** once the server acks — or From fc56d5b853ec62fa6fc9f00a2154938fbd8a2027 Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Wed, 9 Sep 2026 12:17:43 +0200 Subject: [PATCH 06/16] refactor(platform-wallet): remove six never-constructed PlatformWalletError variants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `WalletLocked`, `NoPrimaryIdentity`, `NoWalletsConfigured`, `DashpayReceivingAccountAlreadyExists`, `DashpayExternalAccountAlreadyExists` and `AssetLockExpired` had no constructor anywhere in packages/ — no match arm, no FFI mapping, no Swift/Kotlin mirror. `From` in rs-platform-wallet-ffi routed all six through its `_` arm, so host-visible codes do not change. Dead variants widened the public enum and implied concepts this crate does not have (a wallet lock, a "primary identity"). The two DashPay variants carried four fields nobody filled. The `key_wallet::Network` import existed only for those two. Co-Authored-By: Claude Opus 5 --- packages/rs-platform-wallet/src/error.rs | 33 ------------------------ 1 file changed, 33 deletions(-) diff --git a/packages/rs-platform-wallet/src/error.rs b/packages/rs-platform-wallet/src/error.rs index 2dcb3b70a62..cfbd50d8cd0 100644 --- a/packages/rs-platform-wallet/src/error.rs +++ b/packages/rs-platform-wallet/src/error.rs @@ -6,7 +6,6 @@ use dpp::prelude::{AddressNonce, CoreBlockHeight}; use key_wallet::account::StandardAccountType; use key_wallet::wallet::managed_wallet_info::asset_lock_builder::AssetLockFundingType; use key_wallet::wallet::managed_wallet_info::transaction_building::AccountTypePreference; -use key_wallet::Network; /// Errors that can occur in platform wallet operations #[derive(Debug, thiserror::Error)] @@ -26,9 +25,6 @@ pub enum PlatformWalletError { #[error("Identity not found: {0}")] IdentityNotFound(Identifier), - #[error("No primary identity set")] - NoPrimaryIdentity, - #[error("Invalid identity data: {0}")] InvalidIdentityData(String), @@ -76,26 +72,6 @@ pub enum PlatformWalletError { source: Box, }, - #[error( - "DashPay receiving account already exists for identity {identity} with contact {contact} on network {network:?} (account index {account_index})" - )] - DashpayReceivingAccountAlreadyExists { - identity: Identifier, - contact: Identifier, - network: Network, - account_index: u32, - }, - - #[error( - "DashPay external account already exists for identity {identity} with contact {contact} on network {network:?} (account index {account_index})" - )] - DashpayExternalAccountAlreadyExists { - identity: Identifier, - contact: Identifier, - network: Network, - account_index: u32, - }, - #[error("Asset lock transaction failed: {0}")] AssetLockTransaction(String), @@ -691,9 +667,6 @@ pub enum PlatformWalletError { #[error("Key derivation failed: {0}")] KeyDerivation(String), - #[error("Wallet is locked — unlock it before performing this operation")] - WalletLocked, - #[error( "Signer does not bind to wallet {wallet_id}: it derives a different \ BIP44 account-0 xpub (refusing to sign with the wrong seed)" @@ -746,9 +719,6 @@ pub enum PlatformWalletError { #[error("SPV is already running — stop it before starting again")] SpvAlreadyRunning, - #[error("No wallets configured — add a wallet before starting SPV")] - NoWalletsConfigured, - #[error("SPV error: {0}")] SpvError(String), @@ -764,9 +734,6 @@ pub enum PlatformWalletError { /// unproven locks share that key. FinalityTimeout(dashcore::OutPoint), - #[error("Asset lock proof expired (IS proof too old, CL not yet available): {0}")] - AssetLockExpired(String), - #[error("Asset lock transaction not chain-locked, cannot fall back to CL proof: {0}")] AssetLockNotChainLocked(String), From 0f0ca28ce838c1bf232023441c6084d245baced8 Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Wed, 9 Sep 2026 12:18:33 +0200 Subject: [PATCH 07/16] refactor(platform-wallet): remove top_up.rs and its uncalled convenience wrapper `network/top_up.rs` is 67 lines: a 10-line module doc explaining that the file "just hosts the convenience wrapper `top_up_identity`", 20 lines of argument docs, and a body forwarding to `top_up_identity_with_funding(id, AssetLockFunding::FromWalletBalance{..}, ..)`. `top_up_identity` has no call site in packages/. Both FFI entry points (`identity_top_up.rs`, `identity_registration_funded_with_signer.rs`) call `top_up_identity_with_funding` directly. Co-Authored-By: Claude Opus 5 --- .../src/wallet/identity/network/mod.rs | 1 - .../src/wallet/identity/network/top_up.rs | 67 ------------------- 2 files changed, 68 deletions(-) delete mode 100644 packages/rs-platform-wallet/src/wallet/identity/network/top_up.rs diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/mod.rs b/packages/rs-platform-wallet/src/wallet/identity/network/mod.rs index fce9972f855..aa186e3c07b 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/mod.rs @@ -29,7 +29,6 @@ mod identity_handle; mod loading; mod register_from_addresses; mod registration; -mod top_up; mod top_up_from_addresses; mod transfer; mod transfer_to_addresses; diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/top_up.rs b/packages/rs-platform-wallet/src/wallet/identity/network/top_up.rs deleted file mode 100644 index dffc04aa9ca..00000000000 --- a/packages/rs-platform-wallet/src/wallet/identity/network/top_up.rs +++ /dev/null @@ -1,67 +0,0 @@ -//! Identity top-up convenience wrapper. -//! -//! The substantive top-up logic lives next to identity registration in -//! [`super::registration`] — both flows share the same funding -//! resolution, IS→CL fallback, and asset-lock cleanup machinery, so -//! keeping them in one module avoids duplication. -//! -//! This file just hosts the convenience wrapper `top_up_identity` -//! (defaults to wallet-balance funding) that delegates to the L2 -//! orchestrator `top_up_identity_with_funding`. - -use dpp::prelude::Identifier; - -use dash_sdk::platform::transition::put_settings::PutSettings; - -use crate::error::PlatformWalletError; -use crate::wallet::asset_lock::AssetLockFunding; - -use super::*; - -impl IdentityWallet { - /// Top up an existing identity's credit balance from this wallet's - /// UTXOs. - /// - /// Convenience wrapper around - /// [`top_up_identity_with_funding`](Self::top_up_identity_with_funding) - /// for the common case (`AssetLockFunding::FromWalletBalance`). - /// - /// # Arguments - /// - /// * `identity_id` - The identifier of the identity to top up. - /// * `amount_duffs` - Amount of Dash (in duffs) to add. - /// * `account_index` - Index addressing the standard (BIP44/BIP32) - /// families of the pooled funding set. The top-up draws from the - /// union of those two accounts and every DashPay contact-receiving - /// account (which span their own indices); CoinJoin stays out of - /// the pool, since mixing it with transparent coins in one - /// transaction would undo the mixing. - /// * `asset_lock_signer` - External ECDSA signer that produces both - /// the funding-input P2PKH signatures during asset-lock build and - /// the consume-phase outer signature on the IdentityTopUp - /// transition. In Swift this is a `MnemonicResolverCoreSigner` - /// wrapping the Keychain resolver vtable. - pub async fn top_up_identity( - &self, - identity_id: &Identifier, - amount_duffs: u64, - account_index: u32, - asset_lock_signer: &AS, - settings: Option, - ) -> Result<(), PlatformWalletError> - where - AS: ::key_wallet::signer::ExtendedPubKeySigner + Send + Sync, - { - self.top_up_identity_with_funding( - identity_id, - AssetLockFunding::FromWalletBalance { - amount_duffs, - account_index, - }, - asset_lock_signer, - settings, - ) - .await?; - Ok(()) - } -} From cc92cdb820cff9c1e8fddd8e316fb9c4a01fe1ba Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Wed, 9 Sep 2026 12:18:43 +0200 Subject: [PATCH 08/16] refactor(platform-wallet): remove list_tracked_locks_blocking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Public API kept alive solely by `examples/basic_usage.rs`. The method uses `tokio::sync::RwLock::blocking_read` and — as its own doc says — panics when called from an async context, so it doubled the accessor surface with a footgun variant. The FFI (`rs-platform-wallet-ffi/src/asset_lock/manager.rs`) and Swift/Kotlin reach the async `list_tracked_locks` under `block_on`. The example already runs inside a runtime, so it moves to `.await`. Co-Authored-By: Claude Opus 5 --- packages/rs-platform-wallet/examples/basic_usage.rs | 2 +- .../src/wallet/asset_lock/manager.rs | 13 +------------ 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/packages/rs-platform-wallet/examples/basic_usage.rs b/packages/rs-platform-wallet/examples/basic_usage.rs index 21bba38472a..0844d8b78a7 100644 --- a/packages/rs-platform-wallet/examples/basic_usage.rs +++ b/packages/rs-platform-wallet/examples/basic_usage.rs @@ -107,7 +107,7 @@ async fn main() -> Result<(), Box> { // --- Asset locks --- let asset_locks = wallet.asset_locks(); - let tracked = asset_locks.list_tracked_locks_blocking(); + let tracked = asset_locks.list_tracked_locks().await; println!("Tracked asset locks: {}", tracked.len()); Ok(()) diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/manager.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/manager.rs index da538cf0ef6..65eb101ff3c 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/manager.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/manager.rs @@ -235,18 +235,7 @@ impl AssetLockManager { self.sdk.network } - /// List all tracked asset locks (blocking version for UI / synchronous contexts). - /// - /// Uses `tokio::sync::RwLock::blocking_read` — must NOT be called from - /// within a tokio async context. - pub fn list_tracked_locks_blocking(&self) -> Vec { - let wm = self.wallet_manager.blocking_read(); - wm.get_wallet_info(&self.wallet_id) - .map(|info| info.tracked_asset_locks.values().cloned().collect()) - .unwrap_or_default() - } - - /// List all tracked asset locks (async version). + /// List all tracked asset locks. pub async fn list_tracked_locks(&self) -> Vec { let wm = self.wallet_manager.read().await; wm.get_wallet_info(&self.wallet_id) From 572f2e80550b246dbf4a094abcc2f8dbb0bb50b6 Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Wed, 9 Sep 2026 12:20:25 +0200 Subject: [PATCH 09/16] refactor(platform-wallet): remove ManagedIdentity freshness helpers and BlockTime::{new,is_older_than} MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `managed_identity/sync.rs` (62 lines: `needs_balance_update`, `needs_keys_sync`, `update_keys_sync_block_time`, `update_balance_block_time`) together with `BlockTime::new` and `BlockTime::is_older_than` existed only to test each other — the only hits outside their own tests were inside a `#[test]` block in wallet/apply.rs. Production sets the fields directly (rs-platform-wallet-ffi/src/managed_identity.rs: `identity.last_updated_balance_block_time = Some(owned)`). `is_older_than` computed `(current_timestamp - self.timestamp) > max_age_millis` — a debug panic, or a wrap in release, if a stored block timestamp ran ahead of the caller's clock. If a freshness check is ever wanted, the right shape is a single `age_millis(now)` built on `saturating_sub` at the point of use. `round_trip_block_time_updates` stays — it covers changeset replay for fields that are still live — and builds its fixture with a struct literal instead of the removed setters. Co-Authored-By: Claude Opus 5 --- .../rs-platform-wallet/src/wallet/apply.rs | 21 ++--- .../identity/state/managed_identity/mod.rs | 82 ------------------- .../identity/state/managed_identity/sync.rs | 62 -------------- .../src/wallet/identity/types/block_time.rs | 51 ------------ 4 files changed, 11 insertions(+), 205 deletions(-) delete mode 100644 packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/sync.rs diff --git a/packages/rs-platform-wallet/src/wallet/apply.rs b/packages/rs-platform-wallet/src/wallet/apply.rs index 02d0178b400..23e19c241b9 100644 --- a/packages/rs-platform-wallet/src/wallet/apply.rs +++ b/packages/rs-platform-wallet/src/wallet/apply.rs @@ -1005,19 +1005,20 @@ mod tests { .expect("add"); } let id = Identifier::from([1u8; 32]); - let bt = BlockTime::new(100, 200, 1_700_000_000); + let bt = BlockTime { + height: 100, + core_height: 200, + timestamp: 1_700_000_000, + }; - // Update both timestamps on A (persists internally via noop persister). - info_a + // Set both timestamps on A the way production does — the FFI writes + // these fields directly. + let managed_a = info_a .identity_manager .managed_identity_mut(&id) - .expect("a") - .update_balance_block_time(bt, &p); - info_a - .identity_manager - .managed_identity_mut(&id) - .expect("a") - .update_keys_sync_block_time(bt, &p); + .expect("a"); + managed_a.last_updated_balance_block_time = Some(bt); + managed_a.last_synced_keys_block_time = Some(bt); // Build a single replay changeset from A's final state (both block times set). let managed = info_a.identity_manager.managed_identity(&id).expect("a"); diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs index 87d5a6c10af..80c15c2716a 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs @@ -7,7 +7,6 @@ mod contact_requests; mod contacts; mod dashpay; mod identity_ops; -mod sync; pub use dashpay::DashPayState; @@ -200,87 +199,6 @@ mod tests { assert_eq!(managed.last_synced_keys_block_time, None); } - #[test] - fn test_balance_block_time() { - let identity = create_test_identity(); - let mut managed = ManagedIdentity::new(identity, 0); - let p = noop_persister(); - - let block_time = BlockTime::new(100000, 900000, 1234567890); - managed.update_balance_block_time(block_time, &p); - - assert_eq!(managed.last_updated_balance_block_time, Some(block_time)); - assert_eq!( - managed.last_updated_balance_block_time.unwrap().height, - 100000 - ); - assert_eq!( - managed.last_updated_balance_block_time.unwrap().core_height, - 900000 - ); - assert_eq!( - managed.last_updated_balance_block_time.unwrap().timestamp, - 1234567890 - ); - } - - #[test] - fn test_keys_sync_block_time() { - let identity = create_test_identity(); - let mut managed = ManagedIdentity::new(identity, 0); - let p = noop_persister(); - - let block_time = BlockTime::new(50000, 450000, 9876543210); - managed.update_keys_sync_block_time(block_time, &p); - - assert_eq!(managed.last_synced_keys_block_time, Some(block_time)); - assert_eq!(managed.last_synced_keys_block_time.unwrap().height, 50000); - assert_eq!( - managed.last_synced_keys_block_time.unwrap().core_height, - 450000 - ); - assert_eq!( - managed.last_synced_keys_block_time.unwrap().timestamp, - 9876543210 - ); - } - - #[test] - fn test_needs_balance_update() { - let identity = create_test_identity(); - let mut managed = ManagedIdentity::new(identity, 0); - let p = noop_persister(); - - // Never updated - needs update - assert_eq!(managed.needs_balance_update(1000, 100), true); - - // Just updated - let block_time = BlockTime::new(100, 900, 1000); - managed.update_balance_block_time(block_time, &p); - assert_eq!(managed.needs_balance_update(1050, 100), false); - - // Old update - needs update - assert_eq!(managed.needs_balance_update(1200, 100), true); - } - - #[test] - fn test_needs_keys_sync() { - let identity = create_test_identity(); - let mut managed = ManagedIdentity::new(identity, 0); - let p = noop_persister(); - - // Never synced - needs sync - assert_eq!(managed.needs_keys_sync(1000, 100), true); - - // Just synced - let block_time = BlockTime::new(100, 900, 1000); - managed.update_keys_sync_block_time(block_time, &p); - assert_eq!(managed.needs_keys_sync(1050, 100), false); - - // Old sync - needs sync - assert_eq!(managed.needs_keys_sync(1200, 100), true); - } - #[test] fn test_auto_establish_on_sent_request() { let identity = create_test_identity(); diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/sync.rs b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/sync.rs deleted file mode 100644 index 66d98c1f96e..00000000000 --- a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/sync.rs +++ /dev/null @@ -1,62 +0,0 @@ -//! Synchronization and block time management for ManagedIdentity - -use super::ManagedIdentity; -use crate::wallet::identity::types::block_time::BlockTime; -use crate::wallet::persister::WalletPersister; -use dpp::prelude::TimestampMillis; - -impl ManagedIdentity { - /// Update the last balance update block time. - /// - /// Persists the resulting changeset via `persister` and returns `()`. - pub fn update_balance_block_time( - &mut self, - block_time: BlockTime, - persister: &WalletPersister, - ) { - self.last_updated_balance_block_time = Some(block_time); - let cs = self.snapshot_changeset(); - if let Err(e) = persister.store(cs.into()) { - tracing::error!("Failed to persist changeset: {}", e); - } - } - - /// Update the last keys sync block time. - /// - /// Persists the resulting changeset via `persister` and returns `()`. - pub fn update_keys_sync_block_time( - &mut self, - block_time: BlockTime, - persister: &WalletPersister, - ) { - self.last_synced_keys_block_time = Some(block_time); - let cs = self.snapshot_changeset(); - if let Err(e) = persister.store(cs.into()) { - tracing::error!("Failed to persist changeset: {}", e); - } - } - - /// Check if balance needs updating based on time elapsed - pub fn needs_balance_update( - &self, - current_timestamp: TimestampMillis, - max_age_millis: TimestampMillis, - ) -> bool { - match self.last_updated_balance_block_time { - Some(block_time) => block_time.is_older_than(current_timestamp, max_age_millis), - None => true, // Never updated - } - } - - /// Check if keys need syncing based on time elapsed - pub fn needs_keys_sync( - &self, - current_timestamp: TimestampMillis, - max_age_millis: TimestampMillis, - ) -> bool { - match self.last_synced_keys_block_time { - Some(block_time) => block_time.is_older_than(current_timestamp, max_age_millis), - None => true, // Never synced - } - } -} diff --git a/packages/rs-platform-wallet/src/wallet/identity/types/block_time.rs b/packages/rs-platform-wallet/src/wallet/identity/types/block_time.rs index b4291e5b57a..71045cc1823 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/types/block_time.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/types/block_time.rs @@ -18,54 +18,3 @@ pub struct BlockTime { /// Block timestamp in milliseconds since epoch pub timestamp: TimestampMillis, } - -impl BlockTime { - /// Create a new BlockTime - pub fn new( - height: BlockHeight, - core_height: CoreBlockHeight, - timestamp: TimestampMillis, - ) -> Self { - Self { - height, - core_height, - timestamp, - } - } - - /// Check if this block time is older than a given age in milliseconds - pub fn is_older_than(&self, current_timestamp: TimestampMillis, max_age_millis: u64) -> bool { - (current_timestamp - self.timestamp) > max_age_millis - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_block_time_creation() { - let block_time = BlockTime::new(100000, 900000, 1234567890); - - assert_eq!(block_time.height, 100000); - assert_eq!(block_time.core_height, 900000); - assert_eq!(block_time.timestamp, 1234567890); - } - - #[test] - fn test_is_older_than() { - let block_time = BlockTime::new(100000, 900000, 1000); - - // Not old enough - assert_eq!(block_time.is_older_than(1050, 100), false); - - // Old enough - assert_eq!(block_time.is_older_than(1200, 100), true); - - // Exactly at the threshold - assert_eq!(block_time.is_older_than(1100, 100), false); - - // Just over the threshold - assert_eq!(block_time.is_older_than(1101, 100), true); - } -} From a27d4a015c3808b741181a10cd238e389099262f Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Wed, 9 Sep 2026 12:22:00 +0200 Subject: [PATCH 10/16] refactor(platform-wallet): remove six uncalled IdentityWallet methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For each name, a grep across all of packages/ (Rust, Swift, Kotlin, tests, examples) returns only the definition: - `refresh_dpns_names` (loading.rs) — its wholesale-replace semantics are already covered by the existing `sync_dpns_names`. - `load_identity_by_dpns_name` (loading.rs) — the only non-definition hit is a comment in PlatformWalletPersistenceHandler.swift. - `register_name_with_signer` (dpns.rs) — returned `dash_sdk::Error` unlike every sibling; production uses `register_name_with_external_signer`. - `dpns_domain_states_for_identity` (dpns_marketplace.rs) along with its limit/pagination loop. `dpns_domain_states_page` stays — marketplace sync uses it. - `wallet_manager_read` / `wallet_manager_write` / `try_wallet_manager_write` (identity_handle.rs) — these leaked an RwLock guard outside the crate. - `derive_identity_key_bytes` (identity_handle.rs). The FFI crate calls 14 `IdentityWallet` methods and none of the above; kotlin-sdk and rs-unified-sdk-jni do not reference them at all. `refresh_identity_with_signer` from the same audit entry is KEPT — its doc explicitly names an out-of-repo consumer (dash-evo-tool's `QualifiedIdentity`). Co-Authored-By: Claude Opus 5 --- .../src/wallet/identity/network/dpns.rs | 31 ----- .../identity/network/dpns_marketplace.rs | 52 -------- .../identity/network/identity_handle.rs | 79 ------------ .../src/wallet/identity/network/loading.rs | 114 ------------------ 4 files changed, 276 deletions(-) diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/dpns.rs b/packages/rs-platform-wallet/src/wallet/identity/network/dpns.rs index 85144a48ed1..1da56ba13ac 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/dpns.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/dpns.rs @@ -2,7 +2,6 @@ use dpp::identity::accessors::IdentityGettersV0; -use dpp::identity::Identity; use dpp::identity::IdentityPublicKey; use dpp::identity::KeyType; use dpp::identity::Purpose; @@ -123,36 +122,6 @@ pub enum ContestWinner { // --------------------------------------------------------------------------- impl IdentityWallet { - /// Register a DPNS name using an externally-provided identity and signer. - /// - /// Unlike - /// [`register_name_with_external_signer`](Self::register_name_with_external_signer), - /// this method does **not** look up the identity in the internal - /// `IdentityManager`. The caller supplies the `Identity`, the - /// signing key, and a `Signer` directly. - /// - /// Returns the full domain name (e.g. "alice.dash"). - pub async fn register_name_with_signer>( - &self, - identity: Identity, - name: &str, - identity_public_key: IdentityPublicKey, - signer: S, - ) -> Result { - use dash_sdk::platform::dpns_usernames::RegisterDpnsNameInput; - - let input = RegisterDpnsNameInput { - label: name.to_string(), - identity, - identity_public_key, - signer, - preorder_callback: None, - }; - - let result = self.sdk.register_dpns_name(input).await?; - Ok(result.full_domain_name) - } - /// Register a DPNS name for an identity using an /// externally-supplied signer. /// diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs b/packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs index 3cb874b4bee..c142f416212 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs @@ -701,58 +701,6 @@ impl IdentityWallet { .next()) } - /// Fetch the domain documents associated with `identity_id` via the - /// `records.identity` index (the only identity-keyed index; the - /// protocol rewrites `records.identity` to the new owner on - /// purchase/transfer, so this stays authoritative across sales). - /// `None` drains every server page; `Some(n)` returns at most `n` - /// documents while still respecting the server's per-page limit. - pub async fn dpns_domain_states_for_identity( - &self, - identity_id: &Identifier, - limit: Option, - ) -> Result, PlatformWalletError> { - if limit == Some(0) { - return Ok(Vec::new()); - } - let contract = self.dpns_contract().await?; - let maximum = limit.map(|value| value as usize); - let mut states = Vec::new(); - let mut cursor: Option = None; - - loop { - let remaining = maximum.map(|value| value.saturating_sub(states.len())); - let page_limit = remaining - .map(|value| value.min(SYNC_QUERY_LIMIT as usize)) - .unwrap_or(SYNC_QUERY_LIMIT as usize); - if page_limit == 0 { - break; - } - - let (page, next_cursor, complete) = self - .dpns_domain_states_page( - Arc::clone(&contract), - identity_id, - cursor, - page_limit as u32, - ) - .await?; - states.extend(page); - - if complete || maximum.is_some_and(|value| states.len() >= value) { - break; - } - if cursor == next_cursor { - return Err(PlatformWalletError::InvalidIdentityData( - "DPNS identity query pagination cursor did not advance".to_string(), - )); - } - cursor = next_cursor; - } - - Ok(states) - } - /// Fetch exactly one identity-owned DPNS page. The returned cursor is /// retained by marketplace sync so one pass never drains an unbounded /// document set. diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/identity_handle.rs b/packages/rs-platform-wallet/src/wallet/identity/network/identity_handle.rs index 402b6930737..207e1717a2e 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/identity_handle.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/identity_handle.rs @@ -24,8 +24,6 @@ use std::collections::BTreeMap; use std::sync::{Arc, Mutex as StdMutex}; use dashcore::secp256k1::PublicKey; -use dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; -use dpp::identity::{IdentityPublicKey, KeyType}; use dpp::prelude::Identifier; use key_wallet::bip32::{ChildNumber, DerivationPath, ExtendedPrivKey, KeyDerivationType}; use key_wallet::dip9::{ @@ -392,83 +390,6 @@ impl IdentityWallet { ])) } - /// Derive the raw private key bytes for an identity authentication key. - /// - /// Determines the correct [`KeyDerivationType`] from the public key's - /// [`KeyType`], builds the DIP-9 derivation path, and derives the - /// private key from the wallet. - /// - /// Returns the bytes wrapped in [`Zeroizing`] so they are automatically - /// wiped from memory when the value is dropped. - pub fn derive_identity_key_bytes( - wallet: &Wallet, - network: Network, - identity_index: u32, - identity_public_key: &IdentityPublicKey, - ) -> Result, PlatformWalletError> { - let key_id = identity_public_key.id(); - let key_derivation_type = match identity_public_key.key_type() { - KeyType::ECDSA_SECP256K1 | KeyType::ECDSA_HASH160 => KeyDerivationType::ECDSA, - KeyType::BLS12_381 => KeyDerivationType::BLS, - // EdDSA uses the ECDSA derivation path; the raw bytes are - // reinterpreted as an Ed25519 seed. - KeyType::EDDSA_25519_HASH160 => KeyDerivationType::ECDSA, - KeyType::BIP13_SCRIPT_HASH => { - return Err(PlatformWalletError::InvalidIdentityData( - "BIP13_SCRIPT_HASH keys are not supported for signing".to_string(), - )); - } - }; - - let path = Self::identity_auth_derivation_path( - network, - key_derivation_type, - identity_index, - key_id, - )?; - - let secret_key = wallet.derive_private_key(&path).map_err(|e| { - PlatformWalletError::InvalidIdentityData(format!( - "Failed to derive private key for identity key {}: {}", - key_id, e - )) - })?; - - Ok(Zeroizing::new(secret_key.secret_bytes())) - } - - /// Get a read-lock handle to the shared [`WalletManager`]. - /// - /// Access wallet info via `wm.get_wallet_info(&wallet_id)` and key material - /// via `wm.get_wallet(&wallet_id)` on the returned guard. The identity - /// manager is on the wallet info: `info.identity_manager`. - pub async fn wallet_manager_read( - &self, - ) -> tokio::sync::RwLockReadGuard<'_, WalletManager> { - self.wallet_manager.read().await - } - - /// Get a write-lock handle to the shared [`WalletManager`]. - /// - /// Access wallet info via `wm.get_wallet_info_mut(&wallet_id)` on the - /// returned guard. This allows callers to mutate managed identities (e.g. - /// adding or updating identities from an external persistence layer). - pub async fn wallet_manager_write( - &self, - ) -> tokio::sync::RwLockWriteGuard<'_, WalletManager> { - self.wallet_manager.write().await - } - - /// Try to acquire a write-lock on the shared [`WalletManager`] without blocking. - /// - /// Returns `None` if the lock is currently held by another task. - /// Useful for synchronous callers that cannot await. - pub fn try_wallet_manager_write( - &self, - ) -> Option>> { - self.wallet_manager.try_write().ok() - } - /// The wallet ID for this identity wallet's underlying key material. pub fn wallet_id(&self) -> &WalletId { &self.wallet_id diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/loading.rs b/packages/rs-platform-wallet/src/wallet/identity/network/loading.rs index c3590407c50..affc73ce838 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/loading.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/loading.rs @@ -404,120 +404,6 @@ impl IdentityWallet { dash_sdk::Error::Generic(format!("Identity {} not found on Platform", identity_id)) }) } - - /// Refresh DPNS names for all identities in the manager. - /// - /// Iterates every identity in the [`IdentityManager`], queries Platform - /// for its current DPNS usernames, and replaces the stored - /// `dpns_names` list with the fresh results. - pub async fn refresh_dpns_names(&self) -> Result<(), PlatformWalletError> { - use crate::wallet::identity::types::identity_status::DpnsNameInfo; - - // Collect identity IDs so we don't hold the lock during network calls. - let identity_ids: Vec = { - let wm = self.wallet_manager.read().await; - let info = wm.get_wallet_info(&self.wallet_id).ok_or_else(|| { - crate::error::PlatformWalletError::WalletNotFound( - "Wallet info not found in wallet manager".to_string(), - ) - })?; - info.identity_manager - .all_identities() - .into_iter() - .map(|i| i.id()) - .collect() - }; - - for identity_id in identity_ids { - match self - .sdk - .get_dpns_usernames_by_identity(identity_id, None) - .await - { - Ok(usernames) => { - let mut wm = self.wallet_manager.write().await; - let info = wm.get_wallet_info_mut(&self.wallet_id).ok_or_else(|| { - crate::error::PlatformWalletError::WalletNotFound( - "Wallet info not found in wallet manager".to_string(), - ) - })?; - if let Some(managed) = info.identity_manager.managed_identity_mut(&identity_id) - { - managed.dpns_names = usernames - .into_iter() - .map(|u| DpnsNameInfo { - label: u.label, - acquired_at: None, - }) - .collect(); - } - } - Err(e) => { - tracing::warn!( - "Failed to fetch DPNS names for identity {}: {}", - identity_id, - e - ); - } - } - } - - Ok(()) - } - - /// Load an identity by resolving a DPNS name. - /// - /// Resolves the given `name` to an identity identifier via - /// [`resolve_name`](Self::resolve_name), fetches the identity from - /// Platform, and adds it to the **watched** identities collection (since - /// the wallet derivation index is unknown for externally-resolved names - /// and we cannot sign on their behalf). - /// - /// Returns the identity if the name resolves successfully, or `None` if - /// the name does not exist. - pub async fn load_identity_by_dpns_name( - &self, - name: &str, - ) -> Result, PlatformWalletError> { - use dash_sdk::platform::Fetch; - - // Resolve the DPNS name to an identity ID. - let identity_id = match self.resolve_name(name).await? { - Some(id) => id, - None => return Ok(None), - }; - - // Fetch the identity from Platform. - let identity = Identity::fetch(&self.sdk, identity_id) - .await - .map_err(|e| { - PlatformWalletError::InvalidIdentityData(format!( - "Failed to fetch identity {} for DPNS name '{}': {}", - identity_id, name, e - )) - })? - .ok_or_else(|| { - PlatformWalletError::InvalidIdentityData(format!( - "DPNS name '{}' resolved to identity {} but it was not found on Platform", - name, identity_id - )) - })?; - - // Add to the out-of-wallet bucket (observed read-only — we - // don't know the wallet index and cannot sign). - { - let mut wm = self.wallet_manager.write().await; - let info = wm.get_wallet_info_mut(&self.wallet_id).ok_or_else(|| { - crate::error::PlatformWalletError::WalletNotFound( - "Wallet info not found in wallet manager".to_string(), - ) - })?; - info.identity_manager - .add_out_of_wallet_identity(identity.clone(), &self.persister)?; - } - - Ok(Some(identity)) - } } #[cfg(test)] From 6328352baba4da842fa867c3aa715ec0c46b4914 Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Wed, 9 Sep 2026 12:24:42 +0200 Subject: [PATCH 11/16] refactor(platform-wallet): drop dead auto-accept and contact-address helpers from the public API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All of these were exported from the crate root with no consumer in the FFI, Swift, Kotlin, or in tests outside their own module. They advertised a seed-resident code path the seedless design deliberately removed. Removed outright: - `derive_contact_payment_addresses` (the batch wrapper) and its test, - `DEFAULT_CONTACT_GAP_LIMIT` and its test, - the re-exports in crypto/mod.rs, identity/mod.rs and lib.rs. Gated behind `#[cfg(test)] pub(crate)` rather than deleted, because they pin the correctness of code that is still live: - `derive_contact_payment_address` — the pin that `reconstruct_contact_xpub` yields an equivalent key (production derives contact addresses through key-wallet's `AccountType::DashpayReceivingFunds` pool), - `generate_auto_accept_proof`, `verify_auto_accept_proof`, `derive_auto_accept_private_key` — coverage for the proof scheme; production goes through `provider.export_auto_accept_private_key` + `verify_auto_accept_proof_with_pubkey`. Their own docs already said "Kept for owner-side tests / a self-check". `ContactRequestValidation::new()` from the same audit entry is KEPT — verification found 9 call sites in validation.rs, two of them on the production path (`validate_sender_key`, `validate_recipient_key`). The audit classed "self-referenced" as dead code; it is not the same thing. Co-Authored-By: Claude Opus 5 --- packages/rs-platform-wallet/src/lib.rs | 4 +- .../src/wallet/identity/crypto/auto_accept.rs | 13 +++- .../src/wallet/identity/crypto/dip14.rs | 77 +++---------------- .../src/wallet/identity/crypto/mod.rs | 4 +- .../src/wallet/identity/mod.rs | 5 +- 5 files changed, 24 insertions(+), 79 deletions(-) diff --git a/packages/rs-platform-wallet/src/lib.rs b/packages/rs-platform-wallet/src/lib.rs index 6c693eac50e..c674849beb7 100644 --- a/packages/rs-platform-wallet/src/lib.rs +++ b/packages/rs-platform-wallet/src/lib.rs @@ -79,12 +79,10 @@ pub use wallet::identity::network::{ MASTER_KEY_INDEX, }; pub use wallet::identity::{ - calculate_account_reference, derive_auto_accept_private_key, derive_contact_payment_address, - derive_contact_payment_addresses, derive_contact_xpub, pubkey_binds_expected_key_data, + calculate_account_reference, derive_contact_xpub, pubkey_binds_expected_key_data, unmask_account_reference, BlockTime, ContactProfileEntry, ContactRequest, ContactXpubData, DashPayProfile, DashPayState, DpnsNameInfo, EstablishedContact, IdentityLocation, IdentityManager, IdentityStatus, ManagedIdentity, ProfileUpdate, RegistrationIndex, - DEFAULT_CONTACT_GAP_LIMIT, }; pub use wallet::masternode_withdrawal::{ MasternodeWithdrawalKey, MasternodeWithdrawalKeys, MasternodeWithdrawalRequest, diff --git a/packages/rs-platform-wallet/src/wallet/identity/crypto/auto_accept.rs b/packages/rs-platform-wallet/src/wallet/identity/crypto/auto_accept.rs index ed20b65f11e..c31678bd94c 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/crypto/auto_accept.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/crypto/auto_accept.rs @@ -31,6 +31,7 @@ use key_wallet::bip32::{ChildNumber, DerivationPath}; use key_wallet::dip9::{ DASH_COIN_TYPE, DASH_TESTNET_COIN_TYPE, FEATURE_PURPOSE, FEATURE_PURPOSE_DASHPAY_AUTO_ACCEPT, }; +#[cfg(test)] use key_wallet::wallet::Wallet; use key_wallet::Network; @@ -101,7 +102,11 @@ pub fn auto_accept_derivation_path( } /// Derive the auto-accept private key at `m/9'/coin'/16'/timestamp'`. -pub fn derive_auto_accept_private_key( +/// +/// Test-only: production is seedless and never holds a resident `Wallet` +/// — the drain exports the key through `ContactCryptoProvider`. +#[cfg(test)] +pub(crate) fn derive_auto_accept_private_key( wallet: &Wallet, network: Network, timestamp: u32, @@ -168,7 +173,8 @@ pub fn sign_auto_accept_proof( /// /// # Returns /// A 70-byte proof: `key_type(1) + timestamp(4 BE) + sig_size(1) + signature(64)`. -pub fn generate_auto_accept_proof( +#[cfg(test)] +pub(crate) fn generate_auto_accept_proof( wallet: &Wallet, network: Network, sender_id: &Identifier, @@ -253,7 +259,8 @@ pub fn verify_auto_accept_proof_with_pubkey( /// (there is no resident `Wallet`); the drain derives the public key via the /// `ContactCryptoProvider` and calls the pubkey variant directly. Kept for /// owner-side tests. Does not check expiry. -pub fn verify_auto_accept_proof( +#[cfg(test)] +pub(crate) fn verify_auto_accept_proof( wallet: &Wallet, network: Network, proof_bytes: &[u8], diff --git a/packages/rs-platform-wallet/src/wallet/identity/crypto/dip14.rs b/packages/rs-platform-wallet/src/wallet/identity/crypto/dip14.rs index 9145403be8f..66bf95f70a9 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/crypto/dip14.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/crypto/dip14.rs @@ -26,8 +26,11 @@ //! - [DIP-14](https://github.com/dashpay/dips/blob/master/dip-0014.md) //! - [DIP-15](https://github.com/dashpay/dips/blob/master/dip-0015.md) +#[cfg(test)] use dashcore::secp256k1::Secp256k1; -use dashcore::{Address, Network, PublicKey}; +use dashcore::Network; +#[cfg(test)] +use dashcore::{Address, PublicKey}; use dpp::prelude::Identifier; use key_wallet::account::AccountType; use key_wallet::bip32::{ChildNumber, ExtendedPubKey}; @@ -205,7 +208,12 @@ pub use platform_encryption::{calculate_account_reference, unmask_account_refere /// * `contact_xpub` - The contact relationship extended public key. /// * `index` - The payment address index (non-hardened). /// * `network` - Network for address encoding. -pub fn derive_contact_payment_address( +/// +/// Test-only: production derives contact payment addresses through +/// key-wallet's `AccountType::DashpayReceivingFunds` pool. Kept as the pin +/// that [`reconstruct_contact_xpub`] yields an equivalent key. +#[cfg(test)] +pub(crate) fn derive_contact_payment_address( contact_xpub: &ExtendedPubKey, index: u32, network: Network, @@ -228,31 +236,6 @@ pub fn derive_contact_payment_address( Ok(Address::p2pkh(&pubkey, network)) } -/// Derive multiple payment addresses for a contact, starting from -/// `start_index` up to `start_index + count - 1`. -/// -/// This is a convenience wrapper around [`derive_contact_payment_address`]. -pub fn derive_contact_payment_addresses( - contact_xpub: &ExtendedPubKey, - start_index: u32, - count: u32, - network: Network, -) -> Result, PlatformWalletError> { - (start_index..start_index.saturating_add(count)) - .map(|i| derive_contact_payment_address(contact_xpub, i, network)) - .collect() -} - -// --------------------------------------------------------------------------- -// Gap limit constants -// --------------------------------------------------------------------------- - -/// Default gap limit for contact payment addresses as recommended by DIP-15. -/// -/// "We recommend a gap limit of 10 at this stage, which means to load 10 -/// addresses past the last used address." -pub const DEFAULT_CONTACT_GAP_LIMIT: u32 = 10; - // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- @@ -386,46 +369,6 @@ mod tests { assert_eq!(addr_a, addr_b, "Same index should yield same address"); } - #[test] - fn test_derive_contact_payment_addresses_batch() { - let wallet = test_wallet(Network::Testnet); - let (sender, recipient) = test_identifiers(); - - let data = derive_contact_xpub(&wallet, Network::Testnet, 0, &sender, &recipient) - .expect("derive xpub"); - - let addrs = derive_contact_payment_addresses(&data.xpub, 0, 5, Network::Testnet) - .expect("batch derive"); - - assert_eq!(addrs.len(), 5); - // All addresses should be unique. - for i in 0..addrs.len() { - for j in (i + 1)..addrs.len() { - assert_ne!( - addrs[i], addrs[j], - "Addresses at index {} and {} collide", - i, j - ); - } - } - - // Individually derived addresses should match batch results. - for (i, addr) in addrs.iter().enumerate() { - let single = derive_contact_payment_address(&data.xpub, i as u32, Network::Testnet) - .expect("single derive"); - assert_eq!( - addr, &single, - "Batch and single derivation mismatch at index {}", - i - ); - } - } - - #[test] - fn test_default_gap_limit() { - assert_eq!(DEFAULT_CONTACT_GAP_LIMIT, 10); - } - #[test] fn compact_xpub_is_69_byte_dip15_plaintext_not_107_byte_encode() { // The send path must encrypt the DIP-15 compact diff --git a/packages/rs-platform-wallet/src/wallet/identity/crypto/mod.rs b/packages/rs-platform-wallet/src/wallet/identity/crypto/mod.rs index c0a0687b44b..2b513987232 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/crypto/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/crypto/mod.rs @@ -9,14 +9,12 @@ pub mod dip14; pub mod invitation; pub mod validation; -pub use auto_accept::derive_auto_accept_private_key; pub use contact_info::{ decode_private_data, derive_contact_info_keys, encode_private_data, ContactInfoKeys, ContactInfoPrivateData, }; pub use dip14::{ - calculate_account_reference, derive_contact_payment_address, derive_contact_payment_addresses, - derive_contact_xpub, unmask_account_reference, ContactXpubData, DEFAULT_CONTACT_GAP_LIMIT, + calculate_account_reference, derive_contact_xpub, unmask_account_reference, ContactXpubData, }; pub use invitation::{ encode_invitation_uri, parse_invitation_uri, voucher_output_index, wif_network_matches, diff --git a/packages/rs-platform-wallet/src/wallet/identity/mod.rs b/packages/rs-platform-wallet/src/wallet/identity/mod.rs index fa27162a78a..d223ef3aa8d 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/mod.rs @@ -25,9 +25,8 @@ pub mod types; // latter so `lib.rs`-level re-exports keep resolving. pub use crypto::{ - calculate_account_reference, derive_auto_accept_private_key, derive_contact_payment_address, - derive_contact_payment_addresses, derive_contact_xpub, pubkey_binds_expected_key_data, - unmask_account_reference, ContactXpubData, DEFAULT_CONTACT_GAP_LIMIT, + calculate_account_reference, derive_contact_xpub, pubkey_binds_expected_key_data, + unmask_account_reference, ContactXpubData, }; pub use network::{DashPayView, IdentityWallet}; pub use state::{ From e91e1777690bb854ff0aa0ba7439ee255d7b95a7 Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Wed, 9 Sep 2026 12:26:44 +0200 Subject: [PATCH 12/16] refactor(platform-wallet): remove operations::shield and the SelectionResultOwned trait MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `operations::shield` described itself as a "self-shield front for `shield_to`, preserving the pre-recipient signature for existing callers" — there are no such callers. The only producer, platform_wallet.rs, calls `shield_to` directly; beyond that the name appeared only in test-file prose (retargeted to `shield_to`). `trait SelectionResultOwned` had one implementation and one requirement, doing `refs.into_iter().cloned().collect()` in two places. Replaced by a free function `own_selection` that says so directly. Co-Authored-By: Claude Opus 5 --- .../src/wallet/shielded/operations.rs | 57 +++++-------------- .../shielded/sync/memo_roundtrip_tests.rs | 6 +- .../sync/ovk_builder_roundtrip_tests.rs | 2 +- .../shielded/sync/shield_decrypt_tests.rs | 10 ++-- 4 files changed, 23 insertions(+), 52 deletions(-) diff --git a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs index aab60566879..1b84a81e5cc 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs @@ -509,33 +509,6 @@ fn resolve_shield_recipient( } } -/// Shield credits from transparent platform addresses into the -/// shielded pool, with the resulting note assigned to `account`'s -/// default Orchard payment address derived from `keys`. -/// -/// Self-shield front for [`shield_to`], preserving the pre-recipient -/// signature for existing callers. -#[allow(clippy::too_many_arguments)] -pub async fn shield, P: OrchardProver>( - sdk: &Arc, - store: &Arc>, - persister: Option<&WalletPersister>, - wallet_id: WalletId, - keys: &AccountViewingKeys, - account: u32, - inputs: BTreeMap, - amount: u64, - signer: &Sig, - prover: &P, -) -> Result<(), PlatformWalletError> { - shield_to( - sdk, store, persister, wallet_id, keys, account, None, inputs, amount, - [0u8; 36], // empty memo - signer, prover, - ) - .await -} - /// Shield credits from transparent platform addresses into the /// shielded pool. `recipient` selects the note's Orchard payment /// address: `None` assigns it to `account`'s default address derived @@ -2030,8 +2003,13 @@ async fn reserve_unspent_notes( let unspent = store .get_unspent_notes(id) .map_err(|e| PlatformWalletError::ShieldedStoreError(e.to_string()))?; - let (selected, total_input, exact_fee) = - select_notes_with_fee(&unspent, amount, outputs, fee_kind, sdk.version())?.into_owned(); + let (selected, total_input, exact_fee) = own_selection(select_notes_with_fee( + &unspent, + amount, + outputs, + fee_kind, + sdk.version(), + )?); for note in &selected { store .mark_pending(id, ¬e.nullifier) @@ -2058,14 +2036,13 @@ async fn reserve_unspent_notes_for_denomination( let unspent = store .get_unspent_notes(id) .map_err(|e| PlatformWalletError::ShieldedStoreError(e.to_string()))?; - let (selected, total_input, predicted_fee) = select_notes_for_denomination( + let (selected, total_input, predicted_fee) = own_selection(select_notes_for_denomination( &unspent, denomination, min_actions, num_keys, sdk.version(), - )? - .into_owned(); + )?); for note in &selected { store .mark_pending(id, ¬e.nullifier) @@ -2526,17 +2503,11 @@ fn classify_spend_wait_failure( } } -/// Helper to clone selection results out from under the store lock. -trait SelectionResultOwned { - fn into_owned(self) -> (Vec, u64, u64); -} - -impl SelectionResultOwned for (Vec<&ShieldedNote>, u64, u64) { - fn into_owned(self) -> (Vec, u64, u64) { - let (refs, total, fee) = self; - let owned: Vec = refs.into_iter().cloned().collect(); - (owned, total, fee) - } +/// Clone a selection result out from under the store lock. +fn own_selection( + (refs, total, fee): (Vec<&ShieldedNote>, u64, u64), +) -> (Vec, u64, u64) { + (refs.into_iter().cloned().collect(), total, fee) } /// Convert a `PaymentAddress` to an `OrchardAddress` for the DPP builder. diff --git a/packages/rs-platform-wallet/src/wallet/shielded/sync/memo_roundtrip_tests.rs b/packages/rs-platform-wallet/src/wallet/shielded/sync/memo_roundtrip_tests.rs index 6cdb39536f8..cab4870a676 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/sync/memo_roundtrip_tests.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/sync/memo_roundtrip_tests.rs @@ -5,7 +5,7 @@ //! Two complementary halves, mirroring the two scan-side primitives: //! //! * IVK side — build a real Type 15 Shield transition (exactly as -//! `operations::shield` does: `OrchardKeySet::from_seed` → +//! `operations::shield_to` does: `OrchardKeySet::from_seed` → //! `build_shield_transition` with the `&&prover` double-ref) carrying //! a text memo, then assert the FULL incoming decryption //! (`try_decrypt_note_with_memo`) under the recipient's IVK recovers @@ -172,7 +172,7 @@ async fn shield_memo_round_trips_through_ivk_decryption() { let keys = OrchardKeySet::from_seed(&seed, Network::Testnet, 0) .expect("ZIP-32 derivation from a fixed seed should succeed"); - // Recipient = the wallet's own default address, as `operations::shield` + // Recipient = the wallet's own default address, as `operations::shield_to` // derives via `default_orchard_address`. let recipient = OrchardAddress::from_raw_bytes(&keys.default_address.to_raw_address_bytes()) .expect("default address must convert to OrchardAddress"); @@ -186,7 +186,7 @@ async fn shield_memo_round_trips_through_ivk_decryption() { // `OrchardProver` is impl'd for `&CachedOrchardProver`, so the // builder's `&P` is a double reference — the same shape - // `operations::shield` passes. + // `operations::shield_to` passes. let prover = CachedOrchardProver::new(); let st = build_shield_transition( &recipient, 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 ffef155a4aa..ba36d4b42f0 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 @@ -4,7 +4,7 @@ //! //! This is the exact client-side pair the app exercises end-to-end: //! -//! * build side — `operations::shield` passes the account's +//! * build side — `operations::shield_to` passes the account's //! `OrchardKeySet::outgoing_viewing_key` into dpp's //! `build_shield_transition`, which keys the recipient output's //! `out_ciphertext` to it (the Zcash outgoing-transaction-history diff --git a/packages/rs-platform-wallet/src/wallet/shielded/sync/shield_decrypt_tests.rs b/packages/rs-platform-wallet/src/wallet/shielded/sync/shield_decrypt_tests.rs index fa9dafc2297..d28bc2ec13a 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/sync/shield_decrypt_tests.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/sync/shield_decrypt_tests.rs @@ -3,7 +3,7 @@ //! //! This is the exact client-side pair the app exercises end-to-end: //! -//! * build side — `operations::shield` derives the recipient from +//! * build side — `operations::shield_to` derives the recipient from //! `OrchardKeySet::default_address` and calls dpp's //! `build_shield_transition`, whose serialized actions are stored //! verbatim on-chain (`ShieldedActionNote::from(&SerializedAction)` @@ -75,7 +75,7 @@ async fn shield_built_note_is_trial_decryptable_by_own_ivk() { .expect("ZIP-32 derivation from a fixed seed should succeed"); // Recipient = the wallet's own default address — the same - // conversion `operations::shield` performs via + // conversion `operations::shield_to` performs via // `default_orchard_address`. let recipient = OrchardAddress::from_raw_bytes(&keys.default_address.to_raw_address_bytes()) .expect("default address must convert to OrchardAddress"); @@ -90,7 +90,7 @@ async fn shield_built_note_is_trial_decryptable_by_own_ivk() { // `OrchardProver` is implemented for `&CachedOrchardProver` // (the cached key lives in a static), so P = `&CachedOrchardProver` // and the builder's `&P` is a double reference — the same shape - // `shielded_shield_from_account` passes through `operations::shield`. + // `shielded_shield_from_account` passes through `operations::shield_to`. let prover = CachedOrchardProver::new(); let st = build_shield_transition( &recipient, @@ -101,7 +101,7 @@ async fn shield_built_note_is_trial_decryptable_by_own_ivk() { 0, &&prover, [0u8; 36], - // Production config (`operations::shield`): the output's + // Production config (`operations::shield_to`): the output's // out_ciphertext is keyed to the wallet's own OVK. Irrelevant to // the IVK trial-decryption under test, but kept in lockstep. Some(keys.outgoing_viewing_key.clone()), @@ -190,7 +190,7 @@ async fn shield_to_external_recipient_decrypts_for_recipient_and_recovers_for_se 0, &&prover, memo, - // Production config (`operations::shield`): OVK-keyed to the + // Production config (`operations::shield_to`): OVK-keyed to the // SENDER, so the sender's scan can recover the send. Some(sender_keys.outgoing_viewing_key.clone()), PlatformVersion::latest(), From 8703d0f326806407c1a13070a324151cf5994740 Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Wed, 9 Sep 2026 12:28:39 +0200 Subject: [PATCH 13/16] refactor(platform-wallet): remove dead items in the shielded layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `MultiSyncNotesResult::per_account_for` — definition only; its doc said it feeds "the legacy per-wallet SyncNotesResult shape", which coordinator.rs builds inline. - `_unused_payment_address` — a decoy under `#[allow(dead_code)]` claiming to suppress a warning for an `address` field that does not exist (`PaymentAddress` is used by `RecoveredOutgoing.recipient`). - `ShieldedStore::get_activity_ids` with both implementations and `SubwalletState::activity_ids` — the scan deriver takes `existing_cmxs: BTreeMap` and the coordinator uses `get_activity(*id, 0, usize::MAX)`. The test assertion now counts distinct ids on the fetched page. - The default `ShieldedStore::witness` implementation — its only callers were two file_store tests; production goes through `witness_at_depth`. Those tests move to `witness_at_depth(pos, 0)`. - `marked_positions` and `checkpoints` on `InMemoryShieldedStore` — pushed and cleared, never read. They made the in-memory tree look more capable than it is (it cannot produce a witness). Co-Authored-By: Claude Opus 5 --- .../src/wallet/shielded/file_store.rs | 15 +----- .../src/wallet/shielded/store.rs | 47 ++----------------- .../src/wallet/shielded/sync.rs | 19 -------- 3 files changed, 6 insertions(+), 75 deletions(-) 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 ef12bb062f1..e514871a900 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/file_store.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/file_store.rs @@ -530,17 +530,6 @@ impl ShieldedStore for FileBackedShieldedStore { .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 @@ -967,7 +956,7 @@ mod tests { let mut failures = Vec::new(); for pos in 0..N { - match store.witness(pos) { + match store.witness_at_depth(pos, 0) { Ok(Some(_)) => {} Ok(None) => failures.push(format!("position {pos}: witness returned None")), Err(e) => failures.push(format!("position {pos}: {e}")), @@ -1211,7 +1200,7 @@ mod tests { .into_option() .expect("valid cmx"); let spend_anchor = store - .witness(0) + .witness_at_depth(0, 0) .unwrap() .expect("witness for marked position 0") .root(cmx0) diff --git a/packages/rs-platform-wallet/src/wallet/shielded/store.rs b/packages/rs-platform-wallet/src/wallet/shielded/store.rs index e49aac7a445..e46c1ddffd1 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/store.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/store.rs @@ -340,10 +340,6 @@ pub trait ShieldedStore: Send + Sync { 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. @@ -385,18 +381,6 @@ pub trait ShieldedStore: Send + Sync { depth: usize, ) -> Result, Self::Error>; - /// Generate a Merkle authentication path for `position` against the - /// current tree state. Returns `Ok(None)` if no witness is available - /// (position not marked, or pruned). - /// - /// Delegates to [`Self::witness_at_depth`] at depth 0. - fn witness( - &self, - position: u64, - ) -> Result, Self::Error> { - self.witness_at_depth(position, 0) - } - /// Number of leaves currently in the shared commitment tree /// (= highest appended position + 1, or 0 when empty). /// @@ -745,10 +729,6 @@ impl SubwalletState { ) -> Option { self.activity.get(entry_id).cloned() } - - pub(super) fn activity_ids(&self) -> BTreeSet<[u8; 32]> { - self.activity.keys().copied().collect() - } } // ── InMemoryShieldedStore ────────────────────────────────────────────── @@ -776,10 +756,6 @@ pub struct InMemoryShieldedStore { subwallets: BTreeMap, /// Flat list of commitments appended to the tree. commitments: Vec<[u8; 32]>, - /// Mark flag per position. - marked_positions: Vec, - /// Checkpoint ids in order. - checkpoints: Vec, /// Placeholder anchor; production stores compute the real Sinsemilla root. anchor: [u8; 32], } @@ -959,22 +935,12 @@ impl ShieldedStore for InMemoryShieldedStore { .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> { + fn append_commitment(&mut self, cmx: &[u8; 32], _marked: bool) -> Result<(), Self::Error> { self.commitments.push(*cmx); - self.marked_positions.push(marked); Ok(()) } - fn checkpoint_tree(&mut self, checkpoint_id: u32) -> Result<(), Self::Error> { - self.checkpoints.push(checkpoint_id); + fn checkpoint_tree(&mut self, _checkpoint_id: u32) -> Result<(), Self::Error> { Ok(()) } @@ -1034,8 +1000,6 @@ impl ShieldedStore for InMemoryShieldedStore { // next append start at position 0, matching the file-backed // store's reset contract. self.commitments.clear(); - self.marked_positions.clear(); - self.checkpoints.clear(); self.anchor = [0u8; 32]; Ok(()) } @@ -1224,11 +1188,8 @@ mod tests { .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" - ); + let ids: std::collections::BTreeSet<_> = page.iter().map(|e| e.id).collect(); + assert_eq!(ids.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(); diff --git a/packages/rs-platform-wallet/src/wallet/shielded/sync.rs b/packages/rs-platform-wallet/src/wallet/shielded/sync.rs index 00f5b7db1d9..1723d7ec7b7 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/sync.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/sync.rs @@ -148,20 +148,6 @@ impl MultiSyncNotesResult { pub fn total_new_notes(&self) -> usize { self.per_subwallet_new_notes.values().sum() } - - /// Split out the per-account map for `wallet_id`. Useful for - /// callers that want to feed a single wallet's slice back into - /// the legacy per-wallet [`SyncNotesResult`] shape. - pub fn per_account_for( - &self, - wallet_id: crate::wallet::platform_wallet::WalletId, - ) -> BTreeMap { - self.per_subwallet_new_notes - .iter() - .filter(|(id, _)| id.wallet_id == wallet_id) - .map(|(id, &c)| (id.account_index, c)) - .collect() - } } /// Single-fetch, multi-IVK trial-decrypt across an arbitrary set @@ -826,11 +812,6 @@ struct RecoveredOutgoing { block_height: u64, } -// Suppress dead_code on `address` field — kept for future use -// (e.g. surfacing diversifier index per discovered note). -#[allow(dead_code)] -fn _unused_payment_address(_pa: PaymentAddress) {} - /// Serialize an Orchard note to bytes for storage. /// /// Format: `recipient(43) || value(8 LE) || rho(32) || rseed(32)` = 115 bytes. From 68109712e913a1aca74a6fa40f76542697352630 Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Wed, 9 Sep 2026 12:30:31 +0200 Subject: [PATCH 14/16] refactor(platform-wallet): remove dead SpvRuntime methods and narrow masternode helper visibility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `SpvRuntime::get_quorum_public_key` and `SpvRuntime::update_config` have no call site in rs-platform-wallet, rs-platform-wallet-ffi, rs-unified-sdk-jni, kotlin-sdk or swift-sdk. (Beware appearances: `get_quorum_public_key` has 62 hits across 26 files in the workspace, but those are the `ContextProvider` trait method in entirely different crates — only the inherent method on `SpvRuntime` is removed here. `tests/spv_sync.rs` implements that trait; it does not call the method.) Each carried its own error mapping and locking code to keep consistent with the live paths. Visibility narrowing, no consumers outside the crate: - `wallet_masternode_index_blocking` → private (called only by `masternode_locator_blocking`, a few lines below), - `registration_from_transaction` → `pub(crate)` (used only within tracked.rs), - `find_in_summaries`, `locate_in_summaries` and `parse_locator_input` leave the `pub use` list in masternode/mod.rs; they remain reachable inside the crate through their own modules. Co-Authored-By: Claude Opus 5 --- .../rs-platform-wallet/src/masternode/mod.rs | 15 +++---- .../src/masternode/tracked.rs | 2 +- .../rs-platform-wallet/src/spv/runtime.rs | 43 +------------------ 3 files changed, 9 insertions(+), 51 deletions(-) diff --git a/packages/rs-platform-wallet/src/masternode/mod.rs b/packages/rs-platform-wallet/src/masternode/mod.rs index 6c86b526105..fc7d2bb21eb 100644 --- a/packages/rs-platform-wallet/src/masternode/mod.rs +++ b/packages/rs-platform-wallet/src/masternode/mod.rs @@ -14,13 +14,12 @@ pub mod record; pub mod tracked; pub mod update_service; -pub use list::{find_in_summaries, MasternodeListQuery, MasternodeListSummary}; +pub use list::{MasternodeListQuery, MasternodeListSummary}; pub use locator::{ - locate_in_summaries, parse_locator_input, parse_secret_for_role, verify_masternode_key, - verify_masternode_key_text, KeyVerification, LocateOptions, LocatorMatchKind, - LocatorParseError, LocatorSecret, MasternodeKeyReference, MasternodeKeyRole, - MasternodeLocateError, MasternodeLocateMatch, MasternodeLocateResult, MasternodeLocator, - MasternodeLocatorInput, ParsedLocatorInput, PlatformLookup, + parse_secret_for_role, verify_masternode_key, verify_masternode_key_text, KeyVerification, + LocateOptions, LocatorMatchKind, LocatorParseError, LocatorSecret, MasternodeKeyReference, + MasternodeKeyRole, MasternodeLocateError, MasternodeLocateMatch, MasternodeLocateResult, + MasternodeLocator, MasternodeLocatorInput, ParsedLocatorInput, PlatformLookup, }; pub use record::{ aggregate_masternodes, provider_payload_fields, ListMembership, MasternodeRecord, @@ -111,9 +110,7 @@ impl PlatformWalletManager

{ /// proTxHash (wire) ⇒ wallet id for every loaded wallet's masternodes — /// the "already in wallet" index the locator marks matches with. /// Blocking (see [`Self::wallet_masternodes_blocking`]). - pub fn wallet_masternode_index_blocking( - &self, - ) -> std::collections::HashMap<[u8; 32], WalletId> { + fn wallet_masternode_index_blocking(&self) -> std::collections::HashMap<[u8; 32], WalletId> { let mut index = std::collections::HashMap::new(); for wallet_id in self.list_wallet_ids_blocking() { if let Some(masternodes) = self.wallet_masternodes_blocking(&wallet_id) { diff --git a/packages/rs-platform-wallet/src/masternode/tracked.rs b/packages/rs-platform-wallet/src/masternode/tracked.rs index 66e179015b5..9636ac08a42 100644 --- a/packages/rs-platform-wallet/src/masternode/tracked.rs +++ b/packages/rs-platform-wallet/src/masternode/tracked.rs @@ -1067,7 +1067,7 @@ fn p2pkh_address(hash: &[u8; 20], network: Network) -> String { /// Lift a ProRegTx into [`RegistrationDetails`]; `None` for any other /// transaction. -pub fn registration_from_transaction( +pub(crate) fn registration_from_transaction( tx: &dashcore::Transaction, height: u32, ) -> Option { diff --git a/packages/rs-platform-wallet/src/spv/runtime.rs b/packages/rs-platform-wallet/src/spv/runtime.rs index 3c52cb10cd5..e54468c0c1c 100644 --- a/packages/rs-platform-wallet/src/spv/runtime.rs +++ b/packages/rs-platform-wallet/src/spv/runtime.rs @@ -6,14 +6,13 @@ use std::time::Duration; use tokio::sync::RwLock; use tokio::task::JoinHandle; -use dashcore::sml::llmq_type::LLMQType; use dashcore::sml::masternode_list::MasternodeList; -use dashcore::{PubkeyHash, QuorumHash, Transaction}; +use dashcore::{PubkeyHash, Transaction}; use dash_spv::network::PeerNetworkManager; use dash_spv::storage::{DiskStorageManager, StorageManager}; use dash_spv::sync::SyncProgress; -use dash_spv::{BroadcastResult, ClientConfig, DashSpvClient, EventHandler, Hash}; +use dash_spv::{BroadcastResult, ClientConfig, DashSpvClient, EventHandler}; use key_wallet_manager::WalletManager; @@ -275,29 +274,6 @@ impl SpvRuntime { .map_err(classify_spv_send_error) } - /// Look up a quorum public key via the SPV masternode state. - pub async fn get_quorum_public_key( - &self, - quorum_type: u32, - quorum_hash: [u8; 32], - height: u32, - ) -> Result<[u8; 48], PlatformWalletError> { - let client_guard = self.client.read().await; - let client = client_guard.as_ref().ok_or(PlatformWalletError::SpvError( - "SPV Client not started".to_string(), - ))?; - - let llmq_type = LLMQType::from(quorum_type as u8); - let qh = QuorumHash::from_byte_array(quorum_hash).reverse(); - - let quorum = client - .get_quorum_at_height(height, llmq_type, qh) - .await - .map_err(|e| PlatformWalletError::SpvError(e.to_string()))?; - - Ok(*quorum.quorum_entry.quorum_public_key.as_ref()) - } - /// Drive the sync loop of an already-[`start`]ed client until [`stop`] /// is called async fn run(&self) -> Result<(), PlatformWalletError> { @@ -621,21 +597,6 @@ impl SpvRuntime { Ok(()) } - - /// Update the running SPV client's configuration. - /// - /// The network cannot be changed on a running client. - pub async fn update_config(&self, config: ClientConfig) -> Result<(), PlatformWalletError> { - let client_guard = self.client.read().await; - let client = client_guard.as_ref().ok_or(PlatformWalletError::SpvError( - "SPV Client not started".to_string(), - ))?; - - client - .update_config(config) - .await - .map_err(|e| PlatformWalletError::SpvError(e.to_string())) - } } /// The proTxHashes (internal byte order) of every entry in `list` whose From 150e7de505b1ccd0d6c64116cd6856586603696f Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Wed, 9 Sep 2026 15:46:03 +0200 Subject: [PATCH 15/16] fix(platform-wallet): retarget the from_restore_failure test off the removed WalletLocked variant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `v4.2-dev` merge brought in #4586 (`22055ec8ab`), whose `persister_error_constructors_are_not_interchangeable` test picks `PlatformWalletError::WalletLocked` as an arbitrary sample variant to prove `from_restore_failure` preserves the concrete inner error through boxing. This branch removes `WalletLocked` as never-constructed, so the merge is textually clean but does not compile — `wallet_lifecycle.rs` references a variant `error.rs` no longer defines. `WalletLocked` still has no constructor anywhere; the test only needed *some* variant. It now uses `SpvAlreadyRunning`, a unit variant production actually raises (`spv/runtime.rs:160`). The property under test is unchanged. Verified: `cargo check -p platform-wallet -p platform-wallet-ffi --all-targets`, `cargo test -p platform-wallet` (983 passed) and `cargo fmt --all -- --check` are all green on the merge commit. Co-Authored-By: Claude Opus 5 --- packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs b/packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs index fe6da228e4d..04b74b7878c 100644 --- a/packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs +++ b/packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs @@ -2046,11 +2046,11 @@ mod persister_error_tests { // Structural matching must recover the concrete inner variant. let restore_err = - PlatformWalletError::from_restore_failure(PlatformWalletError::WalletLocked); + PlatformWalletError::from_restore_failure(PlatformWalletError::SpvAlreadyRunning); assert!(restore_err.source().is_some()); match restore_err { PlatformWalletError::PersisterRestore(inner) => { - assert!(matches!(*inner, PlatformWalletError::WalletLocked)); + assert!(matches!(*inner, PlatformWalletError::SpvAlreadyRunning)); } other => panic!("expected PersisterRestore, got {other:?}"), } From 65b81f566adc31806e92fb798b4d99df961560b7 Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Thu, 10 Sep 2026 14:22:18 +0200 Subject: [PATCH 16/16] refactor(platform-wallet): hold event handlers in a plain Vec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removing `add_handler` left `PlatformEventManager::handlers` with no writer: it is populated in `new` and never swapped. The `ArcSwap` then bought nothing but an atomic load and a guard lifetime on every wallet, network, sync and progress dispatch. Store the handlers in a `Vec` and iterate by shared reference. `PlatformEventHandler: EventHandler` already carries the thread-safety bounds and the manager is shared as `Arc`, so read-only dispatch stays safe. Also drops the module and struct docs' claim that the manager supports dynamic handler registration, which stopped being true when `add_handler` went away. `arc_swap` stays a dependency — the wallets map and several sync managers still use it. Co-Authored-By: Claude Opus 5 --- packages/rs-platform-wallet/src/events.rs | 51 +++++++++-------------- 1 file changed, 19 insertions(+), 32 deletions(-) diff --git a/packages/rs-platform-wallet/src/events.rs b/packages/rs-platform-wallet/src/events.rs index 713e0f66388..e97a7c41196 100644 --- a/packages/rs-platform-wallet/src/events.rs +++ b/packages/rs-platform-wallet/src/events.rs @@ -4,15 +4,12 @@ //! platform-specific events. Applications implement this trait to receive //! all events by reference (no cloning). //! -//! [`PlatformEventManager`] dispatches events to registered handlers. -//! It implements [`EventHandler`] so it can be passed directly to -//! `DashSpvClient`, and supports dynamic handler registration via -//! lock-free `ArcSwap`. +//! [`PlatformEventManager`] dispatches events to the handlers it was +//! built with. It implements [`EventHandler`] so it can be passed +//! directly to `DashSpvClient`. use std::sync::Arc; -use arc_swap::ArcSwap; - pub use dash_spv::EventHandler; pub use key_wallet_manager::WalletEvent; @@ -108,25 +105,24 @@ pub trait PlatformEventHandler: EventHandler { /// /// Passed to `DashSpvClient` as the `EventHandler` (via `Arc`). /// -/// Read path (every event): one atomic pointer load, then iterate. +/// The handler set is fixed at construction — there is no registration +/// API — so dispatch iterates the `Vec` directly, with no +/// synchronization on the read path. pub struct PlatformEventManager { - handlers: ArcSwap>>, + handlers: Vec>, } impl PlatformEventManager { - /// Create a new event manager with initial handlers. + /// Create a new event manager over a fixed set of handlers. pub fn new(handlers: Vec>) -> Self { - Self { - handlers: ArcSwap::from_pointee(handlers), - } + Self { handlers } } /// Dispatch a platform-address sync completion to every handler. /// /// Not on the SPV hot path — called once per sync pass (~15s). pub fn on_platform_address_sync_completed(&self, summary: &PlatformAddressSyncSummary) { - let handlers = self.handlers.load(); - for h in handlers.iter() { + for h in &self.handlers { h.on_platform_address_sync_completed(summary); } } @@ -136,8 +132,7 @@ impl PlatformEventManager { /// Not on the SPV hot path — called once per DPNS sync pass /// (~60s by default). pub fn on_dpns_marketplace_sync_completed(&self, summary: &DpnsSyncPassSummary) { - let handlers = self.handlers.load(); - for h in handlers.iter() { + for h in &self.handlers { h.on_dpns_marketplace_sync_completed(summary); } } @@ -148,8 +143,7 @@ impl PlatformEventManager { /// (~60s by default). #[cfg(feature = "shielded")] pub fn on_shielded_sync_completed(&self, summary: &ShieldedSyncPassSummary) { - let handlers = self.handlers.load(); - for h in handlers.iter() { + for h in &self.handlers { h.on_shielded_sync_completed(summary); } } @@ -161,8 +155,7 @@ impl PlatformEventManager { /// path during a cold sync. #[cfg(feature = "shielded")] pub fn on_shielded_sync_progress(&self, cumulative_scanned: u64, block_height: u64) { - let handlers = self.handlers.load(); - for h in handlers.iter() { + for h in &self.handlers { h.on_shielded_sync_progress(cumulative_scanned, block_height); } } @@ -177,8 +170,7 @@ impl PlatformEventManager { /// frequent path during a cold sync. #[cfg(feature = "shielded")] pub fn on_shielded_tree_progress(&self, leaves_committed: u64, total_target: u64) { - let handlers = self.handlers.load(); - for h in handlers.iter() { + for h in &self.handlers { h.on_shielded_tree_progress(leaves_committed, total_target); } } @@ -186,36 +178,31 @@ impl PlatformEventManager { impl EventHandler for PlatformEventManager { fn on_sync_event(&self, event: &dash_spv::sync::SyncEvent) { - let handlers = self.handlers.load(); - for h in handlers.iter() { + for h in &self.handlers { h.on_sync_event(event); } } fn on_network_event(&self, event: &dash_spv::network::NetworkEvent) { - let handlers = self.handlers.load(); - for h in handlers.iter() { + for h in &self.handlers { h.on_network_event(event); } } fn on_progress(&self, progress: &dash_spv::sync::SyncProgress) { - let handlers = self.handlers.load(); - for h in handlers.iter() { + for h in &self.handlers { h.on_progress(progress); } } fn on_wallet_event(&self, event: &WalletEvent) { - let handlers = self.handlers.load(); - for h in handlers.iter() { + for h in &self.handlers { h.on_wallet_event(event); } } fn on_error(&self, error: &str) { - let handlers = self.handlers.load(); - for h in handlers.iter() { + for h in &self.handlers { h.on_error(error); } }