diff --git a/packages/rs-platform-wallet-ffi/src/error.rs b/packages/rs-platform-wallet-ffi/src/error.rs index 444573c5db..e5f9cd381a 100644 --- a/packages/rs-platform-wallet-ffi/src/error.rs +++ b/packages/rs-platform-wallet-ffi/src/error.rs @@ -586,6 +586,18 @@ impl From for PlatformWalletFFIResult { PlatformWalletError::PlatformShieldCapacityExceeded { .. } => { PlatformWalletFFIResultCode::ErrorShieldedInsufficientBalance } + // The per-input sibling of the account-capacity variant above: a + // live pre-broadcast per-input shortfall (a stale-snapshot race). + // It rides the SAME capacity code — the host's corrective action is + // identical (refresh preflight, retry) — but as its OWN wallet + // variant so the message names the offending address and the typed + // `available`/`required` are understood as that single input's live + // figures, not an account maximum. Minting a distinct FFI code was + // deliberately avoided to not collide with the in-flight code-space + // frontier; the disambiguation lives in the message. + PlatformWalletError::PlatformShieldInputShortfall { .. } => { + PlatformWalletFFIResultCode::ErrorShieldedInsufficientBalance + } // The core-transaction sibling of the shielded pair above: the // do-not-retry signal must survive the boundary as a typed code // so hosts can distinguish it from a definitive rejection. diff --git a/packages/rs-platform-wallet-ffi/src/shielded_send.rs b/packages/rs-platform-wallet-ffi/src/shielded_send.rs index 21d98fac4d..d753718661 100644 --- a/packages/rs-platform-wallet-ffi/src/shielded_send.rs +++ b/packages/rs-platform-wallet-ffi/src/shielded_send.rs @@ -597,15 +597,19 @@ fn map_spend_result( format!("{operation} failed: {e}"), ), // The cached Platform Payment-account set no longer covers the - // requested claim plus input-0's fee reserve. Keep this distinct from - // generic wallet-operation failures so hosts can refresh preflight and - // re-confirm a smaller amount instead of retrying unchanged. - Err(e @ PlatformWalletError::PlatformShieldCapacityExceeded { .. }) => { - PlatformWalletFFIResult::err( - PlatformWalletFFIResultCode::ErrorShieldedInsufficientBalance, - format!("{operation} failed: {e}"), - ) - } + // requested claim plus input-0's fee reserve (account-wide), or a live + // per-input hard balance check found one input short (a stale-snapshot + // race). Both share this code — the host's corrective action is the same + // (refresh preflight, retry) — and both stay distinct from generic + // wallet-operation failures so a host never retries the stale amount + // unchanged. The per-input variant's message names the short address. + Err( + e @ (PlatformWalletError::PlatformShieldCapacityExceeded { .. } + | PlatformWalletError::PlatformShieldInputShortfall { .. }), + ) => PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorShieldedInsufficientBalance, + format!("{operation} failed: {e}"), + ), Err(e) => PlatformWalletFFIResult::err( PlatformWalletFFIResultCode::ErrorWalletOperation, format!("{operation} failed: {e}"), @@ -1852,6 +1856,35 @@ mod tests { ); } + #[test] + fn map_spend_result_maps_per_input_shortfall_to_same_code_with_address() { + // The per-input shortfall (a live stale-snapshot race) must ride the + // same code as the account-capacity variant — not regress to the + // generic ErrorWalletOperation — and keep the offending address in the + // message so a host never misreads the single input's balance as the + // account maximum. + let result = map_spend_result( + Err(PlatformWalletError::PlatformShieldInputShortfall { + address: "yShieldInputAddrExample".to_string(), + available: 3_623_849_220, + required: 3_623_849_221, + }), + "shielded shield", + ); + + assert_eq!( + result.code, + PlatformWalletFFIResultCode::ErrorShieldedInsufficientBalance + ); + let message = message_of(&result); + assert!( + message.contains("yShieldInputAddrExample"), + "message: {message}" + ); + assert!(message.contains("3623849220")); + assert!(message.contains("3623849221")); + } + #[test] fn map_asset_lock_funding_result_preserves_already_consumed_code_only() { let out_point = dashcore::OutPoint { diff --git a/packages/rs-platform-wallet/src/error.rs b/packages/rs-platform-wallet/src/error.rs index 8349eb1df2..b39bc9582b 100644 --- a/packages/rs-platform-wallet/src/error.rs +++ b/packages/rs-platform-wallet/src/error.rs @@ -494,6 +494,26 @@ pub enum PlatformWalletError { #[error("Platform shield capacity exceeded: available {available}, required {required}")] PlatformShieldCapacityExceeded { available: u64, required: u64 }, + /// A shield's pre-broadcast per-input hard balance check found ONE input + /// address short: its live on-chain balance dropped below what the cached + /// planner snapshot assumed (a stale-snapshot race), so the fetched claim + /// cannot be funded. STRICTLY per-input — `available`/`required` are that + /// single address's live figures, NOT an account-capacity total. Distinct + /// from [`PlatformShieldCapacityExceeded`](Self::PlatformShieldCapacityExceeded) + /// (an account-wide deterministic-selection limit) precisely so a host never + /// misreads this per-address `available` as the account maximum (it would + /// understate capacity by up to the versioned max input count). The + /// offending `address` (bech32m) is preserved in the message rather than + /// dropped. Nothing was built or broadcast; refresh preflight and retry. + #[error( + "Shield input address {address} is short: has {available}, requires at least {required}" + )] + PlatformShieldInputShortfall { + address: String, + available: u64, + required: u64, + }, + #[error("Shielded build error: {0}")] ShieldedBuildError(String), diff --git a/packages/rs-platform-wallet/src/wallet/platform_wallet.rs b/packages/rs-platform-wallet/src/wallet/platform_wallet.rs index b6d29e67c4..cf2cc50076 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_wallet.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_wallet.rs @@ -60,7 +60,11 @@ pub struct ShieldedShieldPreflight { /// versioned maximum address-input count. pub usable_balance_credits: Credits, /// Balance retained on input 0 for the transition fee — the versioned - /// [`shield_fee_reserve_credits`] value the plan was computed with. + /// [`shield_fee_reserve_credits`] value the plan was computed with, scaled + /// to the input count the plan actually ADMITS rather than to the raw + /// funded-address count (see `resolve_shield_fee_reserve`). Addresses the + /// planner excludes — the leading ones that cannot be input 0, and the + /// later ones below the versioned minimum input amount — never inflate it. pub fee_reserve_credits: Credits, /// Maximum claim accepted by the wallet's deterministic selector: /// `usable_balance_credits - fee_reserve_credits`, floored at zero. @@ -94,13 +98,17 @@ impl ShieldedShieldInputPlan { } if amount > self.preflight.max_shieldable_credits { - let available = if self.usable_candidates.is_empty() { - self.preflight.account_balance_credits - } else { - self.preflight.usable_balance_credits - }; + // `available` must be the usable-for-shield amount so a FAILED call + // never reports the self-contradictory `available > required`. When + // no candidate can retain the fee reserve (a fragmented account) + // `usable_balance_credits` is 0 — coherent with a nonzero + // `required = amount + fee_reserve`. The pre-fix fallback to + // `account_balance_credits` reported the FULL account balance here, + // which can exceed `required` and read as a contradictory shortfall; + // the total account balance stays available on the preflight + // snapshot (`account_balance_credits`) and its `reason` for display. return Err(PlatformWalletError::PlatformShieldCapacityExceeded { - available, + available: self.preflight.usable_balance_credits, required: amount.saturating_add(self.preflight.fee_reserve_credits), }); } @@ -159,6 +167,125 @@ fn checked_credit_sum<'a>( }) } +/// The candidate set [`plan_shield_inputs`] admits at one fee reserve. +/// +/// This is the SINGLE definition of the admission rule. The fee-reserve +/// resolver below counts inputs through this same function, so the reserve can +/// never be priced off a candidate count the planner would not actually admit +/// — the defect this helper was extracted to close. +/// +/// `candidates` must already be sorted by Platform address: that order is what +/// the BTreeMap and the network use to identify input 0. +#[cfg(feature = "shielded")] +fn admit_shield_inputs( + candidates: &[(PlatformAddress, Credits)], + fee_reserve: Credits, + min_input_amount: Credits, + max_address_inputs: usize, +) -> Vec<(PlatformAddress, Credits)> { + let Some(index) = candidates + .iter() + .position(|(_, balance)| *balance > fee_reserve) + else { + return Vec::new(); + }; + + // Keep the fee-bearing input 0 regardless of its post-reserve base + // capacity: the shield fee is added to its requested claim before + // structure validation. Later addresses get no fee addition, so a + // full balance below `min_input_amount` can never form a valid + // input and must not inflate preflight capacity. Finally, truncate + // the deterministic sequence before deriving capacity so Max can + // always be represented by a protocol-valid input count. + std::iter::once(candidates[index]) + .chain( + candidates[index + 1..] + .iter() + .copied() + .filter(|(_, balance)| *balance >= min_input_amount), + ) + .take(max_address_inputs) + .collect() +} + +/// Resolve the mutually dependent (fee reserve, admitted input set) pair. +/// +/// Write `A(R)` for the cardinality of [`admit_shield_inputs`] at reserve `R`, +/// and `R(n)` for `fee_reserve_for_inputs(n)`. The two are circular: the +/// reserve decides which address can be input 0, and the admitted input count +/// decides how many per-input `SetBalanceToAddress` writes the reserve must +/// price. Call a count SOUND when `A(R(n)) <= n` — the reserve prices at least +/// as many writes as the plan built with it can contain inputs. That is the +/// property the reserve needs for EVERY requested amount, because +/// [`ShieldedShieldInputPlan::select_inputs`] never uses more than the admitted +/// candidates, whatever the amount. This returns the SMALLEST sound count, i.e. +/// the least post-fixed point of `R |-> R(A(R))`. +/// +/// Why it is well defined, minimal, and terminating: +/// +/// * `A` is non-increasing in `R`. Raising `R` can only move the `balance > R` +/// search for input 0 later (weakly) in the address-sorted list, and a later +/// input 0 leaves a shorter suffix to draw the `balance >= min_input_amount` +/// tail from. Neither the tail filter nor the `max_address_inputs` cap +/// depends on `R`. +/// * `R` is non-decreasing in `n`: each priced input adds one write. +/// * So soundness is UPWARD CLOSED in `n` — if `A(R(n)) <= n` and `n' >= n` +/// then `A(R(n')) <= A(R(n)) <= n <= n'`. The sound counts form an up-set, so +/// the first hit of an upward scan is the minimum. +/// * `min(candidates.len(), max_address_inputs)` is always sound, because +/// `admit_shield_inputs` returns a subsequence of `candidates` truncated to +/// `max_address_inputs`. That bounds the scan at `max_address_inputs + 1` +/// evaluations (17 at the current protocol maximum) and makes the +/// fallthrough below total — there is no unreachable branch and no panic. +/// +/// A single descent from that upper bound (take `n1 = A(R(n_max))`, then use +/// `R(n1)`) would be UNSOUND, which is why this scans upward instead. Because +/// `A` is antitone, iterating it can settle into a two-cycle rather than a +/// fixed point: an address holding exactly `R(n_max)` is not viable at +/// `R(n_max)` (the viability test is a strict `>`) but becomes viable at the +/// lower `R(n1)`, pulling itself and its whole suffix back in, so +/// `A(R(n1)) > n1` and `R(n1)` under-prices its own plan. +/// `reserve_fixed_point_does_not_descend_into_an_unsound_cycle` pins that +/// shape. For the same reason an EXACT fixed point need not exist; the least +/// sound count is the correct target and over-prices by at most `n - A(R(n))` +/// writes, which is the conservative direction. +#[cfg(feature = "shielded")] +fn resolve_shield_fee_reserve( + candidates: &[(PlatformAddress, Credits)], + fee_reserve_for_inputs: F, + min_input_amount: Credits, + max_address_inputs: usize, +) -> Result<(Credits, Vec<(PlatformAddress, Credits)>), PlatformWalletError> +where + F: Fn(usize) -> Result, +{ + let upper_bound = candidates.len().min(max_address_inputs); + + for priced_inputs in 0..upper_bound { + let fee_reserve = fee_reserve_for_inputs(priced_inputs)?; + let admitted = admit_shield_inputs( + candidates, + fee_reserve, + min_input_amount, + max_address_inputs, + ); + if admitted.len() <= priced_inputs { + return Ok((fee_reserve, admitted)); + } + } + + // `upper_bound` is sound by construction (see the doc comment), so this is + // the scan's guaranteed terminal case rather than an error path. + let fee_reserve = fee_reserve_for_inputs(upper_bound)?; + let admitted = admit_shield_inputs( + candidates, + fee_reserve, + min_input_amount, + max_address_inputs, + ); + Ok((fee_reserve, admitted)) +} + /// Analyze funded Platform addresses once for both preflight and execution. /// /// The representable set is the lexicographically earliest usable prefix, @@ -167,43 +294,35 @@ fn checked_credit_sum<'a>( /// larger balances; consequently preflight Max means the maximum accepted by /// this deterministic policy, not a globally balance-optimized subset. /// -/// `fee_reserve` is the versioned [`shield_fee_reserve_credits`] value; it is -/// the balance input 0 must retain unclaimed so execution can deduct the -/// actual metered fee from that input's residue (`DeductFromInput(0)`). +/// `fee_reserve_for_inputs` maps an input count to the versioned +/// [`shield_fee_reserve_credits`] value — a FUNCTION, not a fixed figure, +/// because the reserve and the admitted set are mutually dependent. +/// [`resolve_shield_fee_reserve`] settles that circularity here, inside the +/// planner, so no caller can price a reserve off a candidate count the planner +/// would not admit. The resolved value is the balance input 0 must retain +/// unclaimed so execution can deduct the actual metered fee from that input's +/// residue (`DeductFromInput(0)`), and it is reported on the preflight snapshot +/// as `fee_reserve_credits`. #[cfg(feature = "shielded")] -fn plan_shield_inputs( +fn plan_shield_inputs( mut candidates: Vec<(PlatformAddress, Credits)>, - fee_reserve: Credits, + fee_reserve_for_inputs: F, min_input_amount: Credits, max_address_inputs: usize, -) -> Result { +) -> Result +where + F: Fn(usize) -> Result, +{ candidates.sort_by_key(|(address, _)| *address); let account_balance_credits = checked_credit_sum(candidates.iter().map(|(_, balance)| balance))?; - let viable_input_0 = candidates - .iter() - .position(|(_, balance)| *balance > fee_reserve); - let usable_candidates: Vec<(PlatformAddress, Credits)> = viable_input_0 - .map(|index| { - // Keep the fee-bearing input 0 regardless of its post-reserve base - // capacity: the shield fee is added to its requested claim before - // structure validation. Later addresses get no fee addition, so a - // full balance below `min_input_amount` can never form a valid - // input and must not inflate preflight capacity. Finally, truncate - // the deterministic sequence before deriving capacity so Max can - // always be represented by a protocol-valid input count. - std::iter::once(candidates[index]) - .chain( - candidates[index + 1..] - .iter() - .copied() - .filter(|(_, balance)| *balance >= min_input_amount), - ) - .take(max_address_inputs) - .collect() - }) - .unwrap_or_default(); + let (fee_reserve, usable_candidates) = resolve_shield_fee_reserve( + &candidates, + fee_reserve_for_inputs, + min_input_amount, + max_address_inputs, + )?; let usable_balance_credits = checked_credit_sum(usable_candidates.iter().map(|(_, balance)| balance))?; let max_shieldable_credits = usable_balance_credits.saturating_sub(fee_reserve); @@ -1575,7 +1694,7 @@ impl PlatformWallet { }), account.address_balances.keys().copied(), ); - let candidates = candidate_addresses + let candidates: Vec<(PlatformAddress, Credits)> = candidate_addresses .into_iter() .filter_map(|p2pkh| { let balance = account.address_credit_balance(&p2pkh); @@ -1585,11 +1704,26 @@ impl PlatformWallet { let platform_version = self.sdk.version(); let state_transition_version = &platform_version.dpp.state_transitions; + let max_address_inputs = usize::from(state_transition_version.max_address_inputs); + // The reserve retained on input 0 must cover the per-input + // `SetBalanceToAddress` writes of every input a Max shield from this + // account can admit. Pricing it off the raw funded-candidate count + // OVER-charges, because the planner then drops the leading addresses + // that cannot be input 0 and the later addresses below the versioned + // minimum: one viable address behind fifteen dust addresses would pay a + // 16-input reserve for a transition that can only ever hold one input, + // and the inflated reserve rejects a shield that transition could fund. + // So hand the planner the versioned reserve FUNCTION and let it resolve + // the reserve against its own admitted set (`resolve_shield_fee_reserve` + // — least sound input count, derivation in its doc comment). The + // admitted count bounds the inputs of every requested amount, and + // `shielded_shield_from_account` re-plans through this same helper, so + // the reserve stays sufficient while preflight and execution agree. plan_shield_inputs( candidates, - shield_fee_reserve_credits(platform_version)?, + |input_count| shield_fee_reserve_credits(platform_version, input_count), state_transition_version.address_funds.min_input_amount, - usize::from(state_transition_version.max_address_inputs), + max_address_inputs, ) } @@ -2055,8 +2189,12 @@ mod shield_input_selection_tests { use dpp::address_funds::PlatformAddress; use dpp::version::LATEST_PLATFORM_VERSION; + /// The zero-input base reserve (`2 × F`) — a stable unit for the selection + /// tests below, which exercise `plan_shield_inputs` / `select_inputs` with a + /// controlled reserve value. The input-count scaling of the reserve itself + /// is covered separately in `operations::reserve_shield_fee_tests`. fn reserve() -> Credits { - shield_fee_reserve_credits(LATEST_PLATFORM_VERSION) + shield_fee_reserve_credits(LATEST_PLATFORM_VERSION, 0) .expect("latest shield fee reserve must be computable") } @@ -2088,17 +2226,46 @@ mod shield_input_selection_tests { ) } - fn plan( + fn plan_with_reserve( candidates: Vec<(PlatformAddress, Credits)>, - ) -> Result { + fee_reserve_for_inputs: F, + ) -> Result + where + F: Fn(usize) -> Result, + { plan_shield_inputs( candidates, - reserve(), + fee_reserve_for_inputs, min_input_amount(), max_address_inputs(), ) } + /// Plan against a FIXED reserve. The selection tests below hold the reserve + /// constant so they exercise ordering, viability and greedy-claim rules in + /// isolation; a constant reserve function is trivially its own fixed point, + /// so the resolver returns exactly [`reserve`] for them. + fn plan( + candidates: Vec<(PlatformAddress, Credits)>, + ) -> Result { + plan_with_reserve(candidates, |_| Ok(reserve())) + } + + /// Plan against the real versioned, input-count-scaled reserve — the shape + /// the fee-reserve fixed point actually has to resolve. + fn plan_versioned( + candidates: Vec<(PlatformAddress, Credits)>, + ) -> Result { + plan_with_reserve(candidates, |input_count| { + shield_fee_reserve_credits(LATEST_PLATFORM_VERSION, input_count) + }) + } + + fn versioned_reserve(input_count: usize) -> Credits { + shield_fee_reserve_credits(LATEST_PLATFORM_VERSION, input_count) + .expect("versioned shield fee reserve must be computable") + } + #[test] fn skips_leading_dust_address_below_reserve() { // addr(1) sorts first but is dust (== reserve, not > reserve); @@ -2132,13 +2299,54 @@ mod shield_input_selection_tests { ); assert!(plan.preflight.reason.is_some()); let err = plan.select_inputs(1).unwrap_err(); + // `available` is the usable-for-shield amount (0 here — the sole address + // cannot retain the reserve), NOT the account balance, so it stays below + // `required` for this failed shield. assert!(matches!( err, PlatformWalletError::PlatformShieldCapacityExceeded { available, required } - if available == reserve() && required == 1 + reserve() + if available == 0 && required == 1 + reserve() )); } + #[test] + fn fragmented_account_reports_coherent_available_not_exceeding_required() { + // Several addresses each holding exactly the reserve: none is strictly + // `> reserve`, so `usable_candidates` is empty and `max_shieldable` is 0, + // yet the account holds a large total balance. A failed shield must + // report `available` as the usable-for-shield amount (0), NOT the full + // account balance — otherwise a fragmented account yields the + // self-contradictory `available > required`. + let candidates = vec![ + (addr(1), reserve()), + (addr(2), reserve()), + (addr(3), reserve()), + ]; + let plan = plan(candidates).unwrap(); + assert!(plan.usable_candidates.is_empty()); + assert_eq!(plan.preflight.max_shieldable_credits, 0); + assert_eq!(plan.preflight.account_balance_credits, 3 * reserve()); + + let amount = 1; + match plan.select_inputs(amount).unwrap_err() { + PlatformWalletError::PlatformShieldCapacityExceeded { + available, + required, + } => { + assert_eq!( + available, 0, + "a fragmented account must report 0 usable, not the full balance" + ); + assert_eq!(required, amount + reserve()); + assert!( + available < required, + "a failed shield must never report available > required" + ); + } + other => panic!("expected PlatformShieldCapacityExceeded, got {other:?}"), + } + } + #[test] fn amount_equal_to_total_minus_reserve_claims_exactly_amount() { // Single address holding exactly amount + reserve: claim == @@ -2320,6 +2528,185 @@ mod shield_input_selection_tests { )); } + #[test] + fn one_viable_address_behind_dust_reserves_for_one_input_not_the_input_cap() { + // Regression, CodeRabbit on #4429: the reserve used to be priced off + // every positive-balance address, but the planner excludes the leading + // addresses that cannot be input 0 and the later addresses below the + // versioned minimum. One viable address trailed by enough dust to reach + // the input cap therefore paid a 16-input reserve for a transition that + // can only ever contain ONE input, and the inflated reserve rejected a + // shield that one-input transition could fund. + let max_inputs = max_address_inputs(); + assert!( + max_inputs > 1, + "latest protocol must permit more than one shield input" + ); + + let reserve_one = versioned_reserve(1); + let reserve_capped = versioned_reserve(max_inputs); + assert!( + reserve_capped > reserve_one, + "the reserve must scale with input count for this regression to bite" + ); + + // Dust sits below the versioned minimum input amount, so every one of + // these is dropped from the admitted set — none can ever be an input. + let dust = min_input_amount() - 1; + // Input 0 holds exactly the inflated cap-priced reserve: strictly above + // the correct one-input reserve, but NOT above the cap-priced one (the + // viability test is a strict `>`), so the old pricing zeroed capacity. + let viable_balance = reserve_capped; + + let mut candidates = vec![(indexed_addr(1), viable_balance)]; + candidates.extend((2..=max_inputs).map(|index| (indexed_addr(index), dust))); + assert_eq!( + candidates.len(), + max_inputs, + "the funded-candidate count must reach the cap the old pricing keyed off" + ); + + let plan = plan_versioned(candidates.clone()).unwrap(); + + assert_eq!( + plan.usable_candidates.len(), + 1, + "only the one viable address can be an input" + ); + assert_eq!( + plan.preflight.fee_reserve_credits, reserve_one, + "the reserve must price the ONE admitted input, not the funded-candidate count" + ); + assert_eq!(plan.preflight.usable_balance_credits, viable_balance); + + let amount = reserve_capped - reserve_one; + assert!(amount > 0); + assert!(plan.preflight.can_shield); + assert_eq!(plan.preflight.max_shieldable_credits, amount); + + let chosen = plan.select_inputs(amount).unwrap(); + assert_eq!( + chosen.len(), + 1, + "the funded shield is a one-input transition" + ); + assert_eq!(chosen.get(&indexed_addr(1)), Some(&amount)); + + // The same candidates under the pre-fix cap-priced reserve: no address + // can retain it, so the shield above is rejected outright. + let inflated = plan_with_reserve(candidates, |_| Ok(reserve_capped)).unwrap(); + assert!( + !inflated.preflight.can_shield, + "the pre-fix reserve must be the thing that rejected this shield" + ); + assert_eq!(inflated.preflight.max_shieldable_credits, 0); + assert!(matches!( + inflated.select_inputs(amount).unwrap_err(), + PlatformWalletError::PlatformShieldCapacityExceeded { .. } + )); + } + + #[test] + fn reserve_fixed_point_does_not_descend_into_an_unsound_cycle() { + // The resolver scans UPWARD for the least sound input count instead of + // taking a single descent from the all-candidates upper bound. The + // admitted count is antitone in the reserve, so one descent can land on + // a count that under-prices its own plan. + // + // Shape: `head` holds exactly `R(4)`, so it is not viable at `R(4)` + // (strict `>`) but is viable at the lower `R(3)`, dragging itself and + // its whole suffix back into the admitted set: + // n = 4 -> input 0 is the second address, admits 3 -> 3 <= 4, sound + // n = 3 -> input 0 is `head`, admits 4 -> 4 > 3, UNSOUND + // n < 3 -> the reserve only drops, admits 4 -> unsound + // so the least sound count is the upper bound 4, and the naive one-step + // descent to 3 would have priced three writes for a four-input plan. + let max_inputs = max_address_inputs(); + assert!(max_inputs >= 4, "this shape needs at least four inputs"); + + let head = versioned_reserve(4); + let funded = 2 * versioned_reserve(max_inputs); + let candidates = vec![ + (indexed_addr(1), head), + (indexed_addr(2), funded), + (indexed_addr(3), funded), + (indexed_addr(4), funded), + ]; + + // What the unsound one-step descent would have produced: at `R(3)` the + // head becomes viable and the plan holds four inputs on a three-input + // reserve. + let descended = + plan_with_reserve(candidates.clone(), |_| Ok(versioned_reserve(3))).unwrap(); + assert_eq!( + descended.usable_candidates.len(), + 4, + "the descent's own plan must admit more inputs than the descent priced" + ); + + let plan = plan_versioned(candidates).unwrap(); + assert_eq!( + plan.preflight.fee_reserve_credits, + versioned_reserve(4), + "the resolver must settle on the least SOUND count, here the upper bound" + ); + assert_eq!(plan.usable_candidates.len(), 3); + assert!( + !plan + .usable_candidates + .iter() + .any(|(address, _)| *address == indexed_addr(1)), + "the head cannot retain the settled reserve, so it is not an input" + ); + assert!( + plan.preflight.fee_reserve_credits >= versioned_reserve(plan.usable_candidates.len()), + "the settled reserve must price at least the inputs it admits" + ); + } + + #[test] + fn resolved_reserve_always_prices_at_least_the_inputs_it_admits() { + // The soundness invariant the fixed point exists to guarantee, swept + // over every rung of the reserve ladder crossed with tail shapes that + // are admitted (funded) or dropped (dust). The reserve is non-decreasing + // in the input count, so `R_used >= R(|admitted|)` is exactly + // "the reserve prices at least as many per-input writes as the plan it + // produced contains inputs" — and that bounds every amount, because + // `select_inputs` never uses more than the admitted candidates. + let max_inputs = max_address_inputs(); + let funded = 2 * versioned_reserve(max_inputs); + let dust = min_input_amount() - 1; + + for head_rung in 0..=max_inputs { + for tail_len in 0..=max_inputs { + for dusty_tail in [false, true] { + let mut candidates = vec![(indexed_addr(1), versioned_reserve(head_rung))]; + candidates.extend((0..tail_len).map(|offset| { + ( + indexed_addr(offset + 2), + if dusty_tail { dust } else { funded }, + ) + })); + + let plan = plan_versioned(candidates).expect("plan must be computable"); + let admitted = plan.usable_candidates.len(); + + assert!( + admitted <= max_inputs, + "admitted {admitted} exceeds the protocol input cap \ + (head_rung={head_rung}, tail_len={tail_len}, dusty_tail={dusty_tail})" + ); + assert!( + plan.preflight.fee_reserve_credits >= versioned_reserve(admitted), + "reserve {} under-prices its own {admitted}-input plan \ + (head_rung={head_rung}, tail_len={tail_len}, dusty_tail={dusty_tail})", + plan.preflight.fee_reserve_credits + ); + } + } + } + } + #[test] fn lifts_non_first_greedy_tail_to_versioned_minimum() { let minimum = min_input_amount(); diff --git a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs index 4783ce75f9..e52a9f7048 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs @@ -55,6 +55,7 @@ use dpp::shielded::builder::{ build_unshield_transition, OrchardProver, SpendableNote, }; use dpp::shielded::compute_minimum_shielded_fee; +use dpp::shielded::SHIELDED_UNSHIELD_ADDRESS_STORAGE_BYTES; use dpp::state_transition::proof_result::StateTransitionProofResult; use dpp::state_transition::public_key_in_creation::IdentityPublicKeyInCreation; use dpp::state_transition::StateTransition; @@ -76,39 +77,90 @@ use tracing::{debug, info, trace, warn}; /// count, so the wallet's fee reservation must use the same count. const SHIELD_NUM_ACTIONS: usize = 2; -/// Multiplier applied to the versioned minimum shield fee when sizing the -/// planner's input-0 reserve. -/// -/// Execution deducts the ACTUAL fee — the GroveDB-metered storage/processing -/// of the note/nullifier writes plus `compute_shielded_verification_fee` — -/// from input 0's post-reallocation residue, and rejects the shield when the -/// residue can't cover it. `compute_minimum_shielded_fee` estimates that -/// actual fee with a flat per-action storage term the client cannot meter -/// itself, so the reserve keeps one extra fee of headroom for metering -/// variance. The reserve is NOT what satisfies the structure gate -/// (`Σ claims ≥ amount + fee`) — `reserve_shield_fee_on_input_0` loads the -/// claimed fee for that — so it needs no allowance beyond metering variance. -const SHIELD_FEE_RESERVE_MULTIPLIER: u64 = 2; +/// Extra metering-variance headroom the shield planner keeps on input 0 BEYOND +/// the input-count-scaled modeled fee, expressed as a count of base +/// structure-gate fees `F`. `1` = one extra `F`, preserving the pre-input- +/// scaling "one extra fee of headroom" semantics on top of the now +/// input-count-scaled modeled fee. +const SHIELD_FEE_RESERVE_HEADROOM_FEES: u64 = 1; /// Versioned balance the shield planner keeps unclaimed on the -/// lexicographically first (fee-paying) input. +/// lexicographically first (fee-paying) input, SCALED by the number of address +/// inputs the transition will admit. /// /// The preflight and the execution path both derive capacity from this one /// value, so it directly sets three host-visible numbers: the viability /// threshold an address must exceed to serve as input 0, the account's /// `max_shieldable_credits`, and the residue a Max shield leaves transparent /// (`reserve − actual fee`). Deriving it from the versioned fee formula keeps -/// all three tracking fee-constant bumps instead of freezing a magic number -/// that overstates the fee and understates capacity. +/// all three tracking fee-constant bumps instead of freezing a magic number. +/// +/// The whole transition fee is deducted from input 0's post-reallocation +/// residue (`DeductFromInput(0)`), so input 0 must retain at least the actual +/// metered fee. For a transparent Shield that fee is: +/// +/// * the flat Orchard-bundle fee `F = compute_minimum_shielded_fee(2)` — the +/// ZK compute plus the two output-bundle actions' note storage, which is +/// also the amount the consensus structure check requires +/// (`Σ claims ≥ amount + F`); PLUS +/// * one `SetBalanceToAddress` address-balance write PER INPUT that drive +/// meters as storage. `compute_minimum_shielded_fee` prices NONE of these +/// per-input writes, so a FLAT reserve thins as inputs grow (blind to input +/// count): a large Max shield can land in the estimate-vs-actual band that +/// risks the `InternalError`/`TxAction::Removed` app-hash-divergence class +/// (the family of the mainnet shield-halt). Scaling the reserve by input +/// count keeps the headroom from thinning. +/// +/// The per-input write is priced off the SAME versioned per-byte storage rate +/// the executor reads (never a hardcoded credit figure), using the reviewed +/// [`SHIELDED_UNSHIELD_ADDRESS_STORAGE_BYTES`] address-write model. That +/// constant sizes a NEW-address `AddBalanceToAddress`; a shield's +/// `SetBalanceToAddress` reduces an EXISTING address balance (a value +/// replacement), which meters no more than a fresh subtree write — so pricing +/// every input at the new-address figure is a deliberate, conservative UPPER +/// BOUND on the true per-input cost. +/// +/// `reserve = F + input_count × per_input_write + headroom`, with +/// `headroom = SHIELD_FEE_RESERVE_HEADROOM_FEES × F`. At `input_count == 0` +/// this equals the pre-scaling flat `2 × F`. +/// +/// SAFETY / RESIDUAL FLAGGED FOR REVIEW: the per-input term is an upper bound +/// derived from an existing calibrated constant, NOT a re-derivation of drive's +/// exact `SetBalanceToAddress` metering. It stays conservative for the +/// protocol-max input count (16), but a reviewer should confirm drive never +/// charges MORE than a new-address write per input; if it can, raise the +/// per-input byte model or `SHIELD_FEE_RESERVE_HEADROOM_FEES`. pub fn shield_fee_reserve_credits( platform_version: &PlatformVersion, + input_count: usize, ) -> Result { - let fee = compute_minimum_shielded_fee(SHIELD_NUM_ACTIONS, platform_version) + let overflow = + || PlatformWalletError::ShieldedBuildError("shield fee reserve overflows u64".to_string()); + + let base_fee = compute_minimum_shielded_fee(SHIELD_NUM_ACTIONS, platform_version) .map_err(|e| PlatformWalletError::ShieldedBuildError(e.to_string()))?; - fee.checked_mul(SHIELD_FEE_RESERVE_MULTIPLIER) - .ok_or_else(|| { - PlatformWalletError::ShieldedBuildError("shield fee reserve overflows u64".to_string()) - }) + + // Per-input `SetBalanceToAddress` storage, priced off the versioned rate the + // executor reads (disk + processing credits/byte), so it tracks fee-constant + // bumps rather than freezing a magic number. + let storage = &platform_version.fee_version.storage; + let per_byte_rate = storage + .storage_disk_usage_credit_per_byte + .checked_add(storage.storage_processing_credit_per_byte) + .ok_or_else(overflow)?; + let per_input_write = SHIELDED_UNSHIELD_ADDRESS_STORAGE_BYTES + .checked_mul(per_byte_rate) + .ok_or_else(overflow)?; + let inputs_cost = (input_count as u64) + .checked_mul(per_input_write) + .ok_or_else(overflow)?; + + // modeled_fee = F + Σ per-input writes; reserve adds one extra F of headroom. + let modeled_fee = base_fee.checked_add(inputs_cost).ok_or_else(overflow)?; + let headroom = base_fee + .checked_mul(SHIELD_FEE_RESERVE_HEADROOM_FEES) + .ok_or_else(overflow)?; + modeled_fee.checked_add(headroom).ok_or_else(overflow) } /// Try to extract a structured `AddressesNotEnoughFundsError` from @@ -157,16 +209,27 @@ fn address_not_enough_funds( } } -/// Promote the shield pre-broadcast hard balance check into the typed capacity +/// Promote the shield pre-broadcast per-input hard balance check into the typed /// error the FFI and Swift layers recognize. /// /// The values are Platform's live per-input view, not the cached planner -/// snapshot. Their `Display` rendering therefore preserves the actionable -/// available/required diagnostic while the typed variant lets the host refresh -/// preflight instead of retrying the stale amount unchanged. -fn map_shield_input_fetch_error(e: &dash_sdk::Error) -> PlatformWalletError { +/// snapshot. `AddressNotEnoughFundsError` is STRICTLY per-address: `balance()` / +/// `required_balance()` describe the ONE short input, not the account. Mapping +/// them onto the account-wide `PlatformShieldCapacityExceeded` (as the pre-fix +/// code did) let a host misread that single address's `available` as the +/// account maximum — understating capacity by up to the versioned input-count +/// cap — and DROPPED the offending address entirely. We map instead to the +/// distinct [`PlatformShieldInputShortfall`](PlatformWalletError::PlatformShieldInputShortfall) +/// variant, which restores the bech32m address in the message and keeps the +/// per-input figures typed as per-input. Both variants ride the same FFI code +/// (the host's corrective action — refresh preflight, retry — is identical). +fn map_shield_input_fetch_error( + e: &dash_sdk::Error, + network: key_wallet::Network, +) -> PlatformWalletError { match address_not_enough_funds(e) { - Some(short) => PlatformWalletError::PlatformShieldCapacityExceeded { + Some(short) => PlatformWalletError::PlatformShieldInputShortfall { + address: short.address().to_bech32m_string(network), available: short.balance(), required: short.required_balance(), }, @@ -499,7 +562,7 @@ pub async fn shield, P: OrchardPr let fetched = fetch_inputs_with_nonce(sdk, &inputs) .await - .map_err(|error| map_shield_input_fetch_error(&error))?; + .map_err(|error| map_shield_input_fetch_error(&error, sdk.network))?; let mut inputs_with_nonce: BTreeMap = BTreeMap::new(); for (addr, (nonce, credits)) in fetched { @@ -2928,22 +2991,36 @@ mod shield_input_fetch_error_tests { use dpp::consensus::state::address_funds::AddressNotEnoughFundsError; #[test] - fn live_address_shortfall_maps_to_typed_shield_capacity_error() { + fn live_per_input_shortfall_maps_to_typed_input_shortfall_with_address() { + let network = key_wallet::Network::Mainnet; + let short_addr = PlatformAddress::P2pkh([7; 20]); let sdk_error = dash_sdk::Error::from(AddressNotEnoughFundsError::new( - PlatformAddress::P2pkh([7; 20]), + short_addr, 3_623_849_220, 3_623_849_221, )); - let mapped = map_shield_input_fetch_error(&sdk_error); - assert!(matches!( - &mapped, - PlatformWalletError::PlatformShieldCapacityExceeded { available, required } - if *available == 3_623_849_220 && *required == 3_623_849_221 - )); - assert_eq!( - mapped.to_string(), - "Platform shield capacity exceeded: available 3623849220, required 3623849221" + let mapped = map_shield_input_fetch_error(&sdk_error, network); + // A per-input shortfall must map to the DISTINCT per-input variant, not + // the account-capacity one — so a host never misreads this single + // input's balance as the account maximum. + let expected_address = short_addr.to_bech32m_string(network); + match &mapped { + PlatformWalletError::PlatformShieldInputShortfall { + address, + available, + required, + } => { + assert_eq!(address, &expected_address); + assert_eq!(*available, 3_623_849_220); + assert_eq!(*required, 3_623_849_221); + } + other => panic!("expected PlatformShieldInputShortfall, got {other:?}"), + } + // The offending address is restored in the message (dropped pre-fix). + assert!( + mapped.to_string().contains(&expected_address), + "message must name the short address: {mapped}" ); } } @@ -2985,10 +3062,14 @@ mod reserve_shield_fee_tests { .state_transitions .address_funds .min_input_amount; + let max_inputs = usize::from( + LATEST_PLATFORM_VERSION + .dpp + .state_transitions + .max_address_inputs, + ); let shield_fee = compute_minimum_shielded_fee(SHIELD_NUM_ACTIONS, LATEST_PLATFORM_VERSION) .expect("latest shield fee must be computable"); - let reserve = shield_fee_reserve_credits(LATEST_PLATFORM_VERSION) - .expect("latest shield fee reserve must be computable"); let smallest_fee_inclusive_claim = shield_fee .checked_add(1) .expect("latest shield fee plus one credit must fit"); @@ -2997,15 +3078,83 @@ mod reserve_shield_fee_tests { smallest_fee_inclusive_claim >= min_input_amount, "adding the fee must lift even input 0's smallest positive base claim above the protocol minimum" ); + + // `input_count == 0` reproduces the pre-scaling flat 2×F, so the change + // is a superset of the old behavior at the base. + let reserve_flat = shield_fee_reserve_credits(LATEST_PLATFORM_VERSION, 0) + .expect("zero-input reserve must be computable"); + assert_eq!( + reserve_flat, + shield_fee.saturating_mul(2), + "the zero-input reserve must equal the pre-scaling flat 2×F" + ); + + let reserve_max = shield_fee_reserve_credits(LATEST_PLATFORM_VERSION, max_inputs) + .expect("max-input reserve must be computable"); assert!( - reserve >= shield_fee, + reserve_max >= shield_fee, "the retained input-0 headroom must cover the versioned shield fee" ); assert!( - reserve <= shield_fee.saturating_mul(4), - "the reserve must stay a small multiple of the versioned fee — an oversized \ - reserve silently understates preflight capacity and strands the excess \ - below the input-0 viability threshold after a Max shield" + reserve_max > reserve_flat, + "a Max shield at the protocol-max input count must reserve strictly more than the flat base" + ); + // Even scaled to the protocol-max input count the reserve stays a small + // multiple of the versioned fee (~2.6×F at latest constants), so it does + // not silently understate preflight capacity: the old 4×F ceiling still + // holds and is deliberately kept. + assert!( + reserve_max <= shield_fee.saturating_mul(4), + "even at the max input count the reserve must stay within 4×F — an \ + oversized reserve silently understates preflight capacity and strands \ + the excess below the input-0 viability threshold after a Max shield" + ); + } + + #[test] + fn reserve_scales_with_admitted_input_count() { + let max_inputs = usize::from( + LATEST_PLATFORM_VERSION + .dpp + .state_transitions + .max_address_inputs, + ); + + // Strictly monotonic in input count: every extra input prices in one + // more metered `SetBalanceToAddress` write, so the headroom cannot thin + // as inputs grow. + let mut previous = + shield_fee_reserve_credits(LATEST_PLATFORM_VERSION, 0).expect("reserve at 0 inputs"); + for n in 1..=max_inputs { + let reserve = + shield_fee_reserve_credits(LATEST_PLATFORM_VERSION, n).expect("reserve computable"); + assert!( + reserve > previous, + "reserve must strictly increase with input count (n={n})" + ); + previous = reserve; + } + + // At the 16-input boundary the reserve must equal the modeled worst-case + // fee (F + 16 × per-input SetBalanceToAddress write) plus one extra F of + // metering-variance headroom, all derived from the versioned constants. + let shield_fee = compute_minimum_shielded_fee(SHIELD_NUM_ACTIONS, LATEST_PLATFORM_VERSION) + .expect("shield fee computable"); + let storage = &LATEST_PLATFORM_VERSION.fee_version.storage; + let per_byte_rate = + storage.storage_disk_usage_credit_per_byte + storage.storage_processing_credit_per_byte; + let per_input = SHIELDED_UNSHIELD_ADDRESS_STORAGE_BYTES * per_byte_rate; + let modeled_fee_16 = shield_fee + (max_inputs as u64) * per_input; + let reserve_16 = shield_fee_reserve_credits(LATEST_PLATFORM_VERSION, max_inputs) + .expect("reserve at 16 inputs"); + assert_eq!( + reserve_16, + modeled_fee_16 + shield_fee, + "reserve(16) must equal modeled_fee(16) + one F of headroom" + ); + assert!( + reserve_16 > modeled_fee_16, + "the reserve must exceed the modeled worst-case 16-input fee" ); }