diff --git a/Cargo.toml b/Cargo.toml index 23dcd8d051c..83ac8fc313e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -67,6 +67,15 @@ tokio-metrics = "0.5" # Size-tuned profile for the iOS `rs-unified-sdk-ffi` staticlib, which # otherwise ships huge. Inherits `release` and is ONLY used by the iOS # build (`build_ios.sh --profile release`) +# +# NOTE: `panic = "abort"` (here and in dev-ios) also disables the FFI +# panic guards (`catch_panic_to_code` in platform-wallet-ffi's +# shielded_send.rs) — on iOS a panic aborts the process before any +# `catch_unwind` runs; the guards are effective on Android and host +# builds, which keep `panic = "unwind"`. Flipping iOS to "unwind" would +# activate them at a binary-size cost (unwind tables + landing pads +# under fat LTO) that must be measured against this profile's size +# budget before shipping. [profile.release-ios] inherits = "release" panic = "abort" diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/FundingNative.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/FundingNative.kt index d85f538d31d..bc540b9fd24 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/FundingNative.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/FundingNative.kt @@ -165,6 +165,26 @@ internal object FundingNative { memoText: String?, ) + /** + * Multi-output shielded → shielded transfer, Type 16 (bridges + * `platform_wallet_manager_shielded_transfer_multi`). + * + * [recipientsRaw43] holds `amounts.size` raw 43-byte Orchard addresses + * laid out back to back (length must be `43 * amounts.size`), and + * [amounts] the matching credit values. Each pair becomes its own note; + * repeating the same address funds it with several independent notes. + * [memoText] is attached to every recipient note. + */ + external fun shieldedTransferMulti( + managerHandle: Long, + walletId: ByteArray, + resolverHandle: Long, + account: Int, + recipientsRaw43: ByteArray, + amounts: LongArray, + memoText: String?, + ) + /** * Shielded → Platform unshield, Type 17 (bridges * `platform_wallet_manager_shielded_unshield`). [toPlatformAddress] is a diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt index 07d143c1c57..941a8a4b22f 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt @@ -1700,6 +1700,69 @@ class PlatformWalletManager( } } + /** + * Multi-output shielded → shielded transfer (Type 16). Spends notes from + * [account] on [walletId] and creates ONE note per entry of [outputs] in + * a single atomic transition. + * + * Repeating the same address across entries is allowed and is the point + * of this call: it funds one address with several independent notes, so + * a later spend of that address spends several REAL notes rather than + * one real note plus an Orchard padding dummy (whose nullifier is + * randomly generated and therefore not reproducible offline). + * + * The transition always emits a change note, so the spendable balance + * must strictly exceed the summed amounts plus the fee. The fee grows + * with the output count: the bundle publishes + * `max(spentNotes, outputs.size + 1, 2)` Orchard actions. + * + * @param walletId the 32-byte wallet id. + * @param outputs (raw 43-byte Orchard address, credits) pairs; must be + * non-empty, hold at most [MAX_SHIELDED_TRANSFER_RECIPIENTS] entries + * (the native ceiling — 5, bound by the 20 KiB transition-size limit), + * and every amount must be positive. + * @param account the ZIP-32 shielded account to spend from (usually 0). + * @param memo optional UTF-8 memo attached to EVERY recipient note + * (null / empty = no memo; at most 32 UTF-8 bytes). + */ + suspend fun shieldedTransferMulti( + walletId: ByteArray, + outputs: List>, + account: Int = 0, + memo: String? = null, + ): Unit = teardownGate.op { + require(outputs.isNotEmpty()) { "outputs must not be empty" } + // Mirror the native ceiling BEFORE flattening: the arrays built below are sized by + // `outputs.size`, and the native layer would reject an oversized call anyway — after + // this side had already allocated for it. + require(outputs.size <= MAX_SHIELDED_TRANSFER_RECIPIENTS) { + "outputs must hold at most $MAX_SHIELDED_TRANSFER_RECIPIENTS entries, got ${outputs.size}" + } + require(account >= 0) { "account must be non-negative, got $account" } + outputs.forEachIndexed { index, (recipientRaw43, amount) -> + require(recipientRaw43.size == 43) { + "outputs[$index] address must be exactly 43 bytes, got ${recipientRaw43.size}" + } + require(amount > 0) { "outputs[$index] amount must be positive, got $amount" } + } + val recipientsRaw43 = ByteArray(outputs.size * 43) + outputs.forEachIndexed { index, (recipientRaw43, _) -> + recipientRaw43.copyInto(recipientsRaw43, index * 43) + } + val amounts = LongArray(outputs.size) { outputs[it].second } + mapNativeErrors { + FundingNative.shieldedTransferMulti( + managerHandle, + walletId, + mnemonicResolver.nativeHandle, + account, + recipientsRaw43, + amounts, + memo?.takeIf { it.isNotEmpty() }, + ) + } + } + /** * Shielded → Platform unshield (Type 17) — port of Swift's * `PlatformWalletManager.shieldedUnshield(walletId:account:toPlatformAddress:amount:)` @@ -2320,6 +2383,20 @@ class PlatformWalletManager( /** SPV progress poll cadence — matches Swift's 1 Hz `startProgressPolling`. */ const val POLL_INTERVAL_MS = 1_000L + /** + * Recipient ceiling of [shieldedTransferMulti] — mirrors + * `MAX_SHIELDED_TRANSFER_RECIPIENTS` in + * `packages/rs-platform-wallet-ffi/src/shielded_send.rs`, which the JNI adapter enforces + * from the array lengths before allocating. Checked here too so an oversized call is + * refused before this side flattens caller-sized buffers. + * + * 5 = the effective per-transition Orchard action ceiling (6, bound by the 20 KiB + * `max_state_transition_size` — a 7-action transition serializes to ~21.7 KiB) minus + * the unconditional change output. The native constant is pinned to the dpp derivation + * by a Rust test; raise this only in lockstep with it. + */ + const val MAX_SHIELDED_TRANSFER_RECIPIENTS = 5 + /** De-offset `PlatformWalletFFIResultCode::ErrorInvalidParameter`. */ const val PWFFI_INVALID_PARAMETER = 2 } diff --git a/packages/rs-dpp/src/shielded/builder/identity_create_from_shielded_pool.rs b/packages/rs-dpp/src/shielded/builder/identity_create_from_shielded_pool.rs index 6d2823008ac..ab47d7e47c9 100644 --- a/packages/rs-dpp/src/shielded/builder/identity_create_from_shielded_pool.rs +++ b/packages/rs-dpp/src/shielded/builder/identity_create_from_shielded_pool.rs @@ -21,7 +21,10 @@ use crate::ProtocolError; use platform_value::Identifier; use platform_version::version::PlatformVersion; -use super::{build_spend_bundle_with, serialize_authorized_bundle, OrchardProver, SpendableNote}; +use super::{ + build_spend_bundle_with, serialize_authorized_bundle, serialized_envelope_bytes, + shielded_bundle_action_count, OrchardProver, SpendableNote, PER_KEY_SIGNATURE_ALLOWANCE_BYTES, +}; /// Output of [`build_identity_create_from_shielded_pool_transition`]: everything the SDK's /// `IdentityCreateFromShieldedPool::identity_create_from_shielded_pool` broadcast helper needs. @@ -158,7 +161,24 @@ where // Orchard's BundleType::DEFAULT pads single-spend bundles to a 2-action minimum, matching the // other spend-side builders. The fee predictor is only informational here (the metered fee at // execution is authoritative); we report it so the caller's reservation math lines up. - let num_actions = spends.len().max(2); + // + // Routed through the shared predictor (1 shielded output — the change note), which is + // numerically `spends.len().max(2)` AND enforces both consensus ceilings (the structural + // action cap and the transition-size-derived one) BEFORE the ~30 s Halo 2 proof. The size + // side must price THIS transition's variable key set: up to six identity keys ride the + // envelope, and at gate time their PoP `signature` fields are still empty — so add a + // per-key allowance for the largest signature a key type can carry (BLS, 96 bytes, plus + // its length prefix), keeping the estimate conservative rather than optimistic. + let key_set_envelope_bytes = serialized_envelope_bytes( + &public_keys + .iter() + .map(|(_, c)| c.clone()) + .collect::>(), + "the identity key set", + )? + .saturating_add(public_keys.len() as u64 * PER_KEY_SIGNATURE_ALLOWANCE_BYTES); + let num_actions = + shielded_bundle_action_count(spends.len(), 1, key_set_envelope_bytes, platform_version)?; let fee = compute_shielded_identity_create_fee(num_actions, public_keys.len(), platform_version)?; @@ -351,6 +371,64 @@ mod tests { /// 0.1 DASH in credits — the smallest member of the versioned exit-denomination set. const DENOMINATION: u64 = 10_000_000_000; + /// The identity-create gate must price its variable key set into the size + /// budget (#4312 review finding e90e9cf15f52): a maximal six-key set — + /// measured pre-PoP-signing plus the per-key signature allowance — is a + /// real envelope cost, and at current constants the padded two-action + /// claim shape must still clear the gate with it (a maximal key set must + /// not brick identity creation; it only tightens how many spends fit). + #[test] + fn identity_key_set_envelope_is_priced_into_the_gate() { + use crate::shielded::builder::{ + serialized_envelope_bytes, shielded_bundle_action_count, + PER_KEY_SIGNATURE_ALLOWANCE_BYTES, + }; + use crate::shielded::{ + max_shielded_actions_for_envelope, max_shielded_actions_per_transition, + }; + + let platform_version = PlatformVersion::latest(); + let baseline = max_shielded_actions_per_transition(platform_version); + + // Six keys — the identity-create maximum the finding names. + let keys: Vec = (0..6u32).map(|id| key_pair(id).1).collect(); + let measured = + serialized_envelope_bytes(&keys, "the identity key set").expect("measurable key set"); + let envelope = measured + keys.len() as u64 * PER_KEY_SIGNATURE_ALLOWANCE_BYTES; + assert!( + measured > 0, + "a six-key set must have a nonzero serialized envelope" + ); + + let ceiling = max_shielded_actions_for_envelope(platform_version, envelope); + assert!( + (2..=baseline).contains(&ceiling), + "a six-key envelope ({envelope} bytes) must leave at least the padded 2-action \ + claim shape and never exceed the baseline ceiling {baseline}, got {ceiling}" + ); + + // The padded single-spend claim (2 actions on the wire) must pass the + // gate under the maximal key set. + let num_actions = shielded_bundle_action_count(1, 1, envelope, platform_version) + .expect("the padded 2-action identity create must clear the gate with six keys"); + assert_eq!(num_actions, 2); + + // A spend-fragmented claim at the BASELINE ceiling must be rejected + // once the key envelope eats the slack — or accepted if the envelope + // still fits; either way the gate's verdict must match the derived + // ceiling exactly (no drift between the gate and the derivation). + match shielded_bundle_action_count(baseline, 1, envelope, platform_version) { + Ok(n) => { + assert_eq!(n, baseline); + assert_eq!(ceiling, baseline); + } + Err(e) => { + assert!(ceiling < baseline, "rejection requires a tightened ceiling"); + assert!(e.to_string().contains("max_state_transition_size")); + } + } + } + /// The padded-bundle regression test for the dummy-nullifier bug: a SINGLE-spend bundle is /// padded by `BundleType::DEFAULT` to the 2-action minimum, and the padding action's random /// dummy nullifier is published on the wire. The identity id MUST be derived from the FULL @@ -414,6 +492,13 @@ mod tests { identity_id_from_nullifiers(&[real_nullifier]), "the padding action's dummy nullifier must participate in the id derivation" ); + // …which is precisely what `shielded_identity_id_is_reproducible` reports: with one real + // spend the published set contains fresh randomness, so the id cannot be re-derived + // offline (a retry would build a different dummy and thus a different id). + assert!( + !crate::state_transition::identity_create_from_shielded_pool_transition::shielded_identity_id_is_reproducible(1), + "a single-spend bundle is padded, so its id must be reported as NOT reproducible" + ); assert!( result.predicted_fee < DENOMINATION, "predicted fee must leave the new identity a positive balance" @@ -497,5 +582,12 @@ mod tests { identity_id_from_nullifiers(&[nf_a, nf_b]), "with no padding, the published set is exactly the real spends' nullifiers" ); + // …which is precisely what `shielded_identity_id_is_reproducible` reports: with two real + // spends no padding is added, so the id is a pure function of the spent notes and a retry + // re-derives the SAME id. This is the property two-note funding buys. + assert!( + crate::state_transition::identity_create_from_shielded_pool_transition::shielded_identity_id_is_reproducible(2), + "a two-spend bundle needs no padding, so its id must be reported as reproducible" + ); } } diff --git a/packages/rs-dpp/src/shielded/builder/mod.rs b/packages/rs-dpp/src/shielded/builder/mod.rs index ae7b9047752..95b6cff53b6 100644 --- a/packages/rs-dpp/src/shielded/builder/mod.rs +++ b/packages/rs-dpp/src/shielded/builder/mod.rs @@ -43,7 +43,10 @@ pub use identity_create_from_shielded_pool::{ pub use shield_from_asset_lock::build_shield_from_asset_lock_transition; #[cfg(feature = "core_key_wallet")] pub use shield_from_asset_lock::build_shield_from_asset_lock_transition_with_signer; -pub use shielded_transfer::build_shielded_transfer_transition; +pub use shielded_transfer::{ + build_shielded_transfer_transition, build_shielded_transfer_transition_multi, + ShieldedTransferOutput, +}; pub use shielded_withdrawal::build_shielded_withdrawal_transition; pub use unshield::build_unshield_transition; @@ -52,6 +55,7 @@ use grovedb_commitment_tree::{ FullViewingKey, MerklePath, Note, NoteValue, OutgoingViewingKey, PaymentAddress, ProvingKey, Scope, SpendAuthorizingKey, SpendingKey, }; +use platform_version::version::PlatformVersion; use rand::rngs::OsRng; use rand::RngCore; @@ -103,6 +107,138 @@ impl From<&OrchardAddress> for PaymentAddress { } } +/// The number of Orchard actions a `BundleType::DEFAULT` bundle built from `num_spends` spends +/// and `num_outputs` outputs will publish **on the wire**, validated against the consensus +/// action ceiling. +/// +/// Every shielded fee predictor MUST size its fee with this function, because consensus prices +/// the fee off the on-wire `actions.len()` (see +/// `StateTransitionShieldedMinimumFeeValidationV0::validate_minimum_shielded_fee`, which reads +/// `v0.actions.len()`), and an Orchard action is a *joined* spend/output slot: the action count +/// is `max(num_spends, num_outputs)`, then padded up to Orchard's `MIN_ACTIONS = 2`. +/// +/// The output side matters. A predictor that looks only at the spend count is correct **only** +/// while `num_outputs <= 2`, because `max(n, 1).max(2) == max(n, 2).max(2)`. As soon as a +/// transition publishes three or more outputs (a multi-recipient transfer plus change), a +/// spends-only predictor under-counts and carves a fee below the one consensus computes — fatal +/// for `ShieldedTransfer`, whose `value_balance` must equal the minimum fee **exactly**. +/// +/// This delegates to Orchard's own [`BundleType::num_actions`] rather than re-deriving the rule, +/// so the predictor cannot drift from the builder that actually lays out the bundle. +/// +/// # The consensus ceilings +/// +/// TWO versioned limits bound a shielded bundle, and this gate enforces both +/// BEFORE any proving work starts: +/// +/// 1. **Structural**: every shielded transition's `validate_structure` rejects a bundle whose +/// `actions.len()` exceeds `platform_version.system_limits.max_shielded_transition_actions` +/// (via `validate_actions_count`). +/// 2. **Size**: the serialized transition must fit +/// `platform_version.system_limits.max_state_transition_size` (20 KiB), enforced by DAPI's +/// byte prefilter / Tenderdash `mempool.max-tx-bytes` and the Drive-ABCI consensus decoder +/// — which run BEFORE structural validation ever sees the transition. At current constants +/// this is the binding limit: 6 actions serialize to ~19.0 KiB while 7 need ~21.7 KiB, so +/// 7..16-action bundles pass the structural check yet are guaranteed dead on arrival. +/// +/// The `try_from_bundle` constructors run no structural validation and nothing checks the byte +/// size client-side — so without this gate an over-limit bundle is built, proved (~30 s of +/// Halo 2 *per bundle*), and only then rejected. The effective ceiling is derived from BOTH +/// limits via [`crate::shielded::max_shielded_actions_for_envelope`], never hardcoded, so a +/// future `max_state_transition_size` raise widens this gate automatically. Because the action +/// count is `max(spends, outputs)`, bounding it here bounds BOTH sides: a fragmented wallet +/// spending too many notes and a caller asking for too many outputs are rejected by the same +/// comparison, before any proving work starts. +/// +/// `extra_envelope_bytes` is the serialized size of the transition's variable-length +/// non-Orchard fields beyond the measured baseline envelope — an embedded instant +/// asset-lock proof (its funding transaction and `InstantLock` both carry input +/// vectors; DPP admits up to 100 inputs) or an identity-create key set. Callers whose +/// envelope is covered by the baseline (transfer, unshield, withdrawal) pass `0`; +/// `ShieldFromAssetLock` passes the serialized proof size and identity creation the +/// serialized key-set size (see [`serialized_envelope_bytes`]), so an oversized +/// envelope tightens the ceiling here instead of after the proof (#4312 review +/// finding e90e9cf15f52). +pub fn shielded_bundle_action_count( + num_spends: usize, + num_outputs: usize, + extra_envelope_bytes: u64, + platform_version: &PlatformVersion, +) -> Result { + let num_actions = BundleType::DEFAULT + .num_actions(num_spends, num_outputs) + .map_err(|e| { + ProtocolError::ShieldedBuildError(format!( + "invalid Orchard bundle shape ({num_spends} spends, {num_outputs} outputs): {e}" + )) + })?; + + let max_actions = platform_version + .system_limits + .max_shielded_transition_actions as usize; + if num_actions > max_actions { + return Err(ProtocolError::ShieldedBuildError(format!( + "a bundle of {num_spends} spends and {num_outputs} outputs publishes {num_actions} \ + Orchard actions, exceeding the consensus limit of {max_actions} \ + (max_shielded_transition_actions); consensus would reject the proved transition" + ))); + } + + let effective_max = + crate::shielded::max_shielded_actions_for_envelope(platform_version, extra_envelope_bytes); + if num_actions > effective_max { + let estimated = crate::shielded::estimated_shielded_transition_wire_bytes_with_envelope( + num_actions, + extra_envelope_bytes, + ); + let max_size = platform_version.system_limits.max_state_transition_size; + let envelope_note = if extra_envelope_bytes > 0 { + format!(" (including {extra_envelope_bytes} bytes of transition-specific envelope)") + } else { + String::new() + }; + return Err(ProtocolError::ShieldedBuildError(format!( + "a bundle of {num_spends} spends and {num_outputs} outputs publishes {num_actions} \ + Orchard actions, which serializes to an estimated {estimated} bytes{envelope_note} \ + and exceeds max_state_transition_size ({max_size} bytes); at most {effective_max} \ + actions fit, so DAPI's byte prefilter would reject the proved transition" + ))); + } + + Ok(num_actions) +} + +/// Conservative per-key allowance for an identity key's proof-of-possession +/// signature at pre-proving gate time: the keys are measured BEFORE PoP +/// signing, when each `IdentityPublicKeyInCreation.signature` is still empty, +/// while the wire form carries the signature. 97 bytes = the largest +/// signature a key type can produce (BLS12-381, 96 bytes) plus its one-byte +/// length prefix; ECDSA (65) and EdDSA (64) keys under-fill the allowance, +/// which errs on the conservative (smaller-ceiling) side. +pub const PER_KEY_SIGNATURE_ALLOWANCE_BYTES: u64 = 97; + +/// Serialized size of one variable-length transition envelope field, measured +/// with the same bincode configuration the transition's own wire serialization +/// uses (`standard().with_big_endian().with_no_limit()`, per +/// `platform_serialization`'s unversioned `PlatformSerialize` path), so the +/// pre-proving gate prices exactly the bytes the byte prefilter will see. +/// `what` names the field in the error. +pub fn serialized_envelope_bytes( + field: &T, + what: &str, +) -> Result { + let config = bincode::config::standard() + .with_big_endian() + .with_no_limit(); + bincode::encode_to_vec(field, config) + .map(|bytes| bytes.len() as u64) + .map_err(|e| { + ProtocolError::ShieldedBuildError(format!( + "failed to measure the serialized size of {what} for the pre-proving size gate: {e}" + )) + }) +} + /// Serializes an authorized Orchard bundle into the raw fields used by /// state transition constructors. pub fn serialize_authorized_bundle(bundle: &Bundle) -> SerializedBundle { @@ -781,4 +917,131 @@ mod mod_tests { other => panic!("expected the closure's error to propagate, got {:?}", other), } } + + // ------------------------------------------------------------------ + // `shielded_bundle_action_count` — the shared fee-sizing predictor. + // ------------------------------------------------------------------ + + /// The predictor must be `max(num_spends, num_outputs)` padded to Orchard's 2-action + /// minimum — for the OUTPUT side as well as the spend side. The `num_outputs >= 3` rows are + /// the ones a spends-only predictor gets wrong. + #[test] + fn shielded_bundle_action_count_is_max_spends_outputs_padded_to_two() { + let platform_version = PlatformVersion::latest(); + for (spends, outputs, expected) in [ + (0usize, 1usize, 2usize), + (1, 1, 2), + (1, 2, 2), + (2, 2, 2), + // Output-dominated shapes: the spend count no longer determines the fee. + (1, 3, 3), + (2, 3, 3), + (1, 4, 4), + (5, 3, 5), + (3, 6, 6), + ] { + let actual = shielded_bundle_action_count(spends, outputs, 0, platform_version) + .expect("DEFAULT bundles accept any spend/output mix"); + assert_eq!( + actual, expected, + "action count for {spends} spends / {outputs} outputs" + ); + } + } + + /// A real bundle's on-wire `actions.len()` — the number consensus prices the fee off — must + /// equal what the predictor said. Exercised through the output-only builder because it is + /// the cheapest real bundle to construct at several output counts. + #[test] + fn shielded_bundle_action_count_matches_a_real_bundle() { + let platform_version = PlatformVersion::latest(); + let recipient = test_orchard_address(); + // (dummy_outputs, total outputs = 1 real + dummies) + for dummies in [0usize, 1, 4] { + let num_outputs = 1 + dummies; + let bundle = + build_output_only_bundle(&recipient, 10_000, [0u8; 36], None, dummies, &TestProver) + .expect("bundle should build"); + let predicted = shielded_bundle_action_count(0, num_outputs, 0, platform_version) + .expect("valid bundle shape"); + assert_eq!( + bundle.actions().len(), + predicted, + "predicted action count must match the real bundle's on-wire count for \ + {num_outputs} outputs" + ); + } + } + + /// The predictor is also the CONSENSUS gate — for BOTH ceilings. The effective (size-derived) + /// boundary itself must still pass, from each side: at current constants that is 6 actions + /// (~19.0 KiB against the 20 KiB `max_state_transition_size`). + #[test] + fn shielded_bundle_action_count_accepts_the_effective_boundary() { + let platform_version = PlatformVersion::latest(); + let effective = crate::shielded::max_shielded_actions_per_transition(platform_version); + + // Exactly at the effective ceiling, from each side and from both at once. + for (spends, outputs) in [(1usize, effective), (effective, 1), (effective, effective)] { + assert_eq!( + shielded_bundle_action_count(spends, outputs, 0, platform_version).unwrap_or_else( + |e| panic!("{spends} spends / {outputs} outputs is AT the effective ceiling and must be accepted: {e}") + ), + effective + ); + } + } + + /// One action over the EFFECTIVE ceiling must fail fast, pre-proving — from the OUTPUT side + /// (a 6-recipient multi transfer becomes 7 outputs once the unconditional change output is + /// added) and from the SPEND side (a fragmented wallet selecting 7 notes). These shapes pass + /// the 16-action structural cap, but a 7-action transition serializes to ~21.7 KiB and is + /// rejected by DAPI's 20 KiB byte prefilter — AFTER ~30 s of Halo 2 proving, without this + /// gate. This test completing in milliseconds is itself part of the assertion. + #[test] + fn shielded_bundle_action_count_rejects_over_the_size_derived_ceiling() { + let platform_version = PlatformVersion::latest(); + let effective = crate::shielded::max_shielded_actions_per_transition(platform_version); + let structural = platform_version + .system_limits + .max_shielded_transition_actions as usize; + assert!( + effective < structural, + "this test requires the size limit to be the binding one (effective {effective} < \ + structural {structural}); if the size limit was raised, retire or rework this test" + ); + + let over = effective + 1; + // Output-dominated, spend-dominated, and both-sided 7-action shapes. + for (spends, outputs) in [(1usize, over), (over, 1), (over, over)] { + let err = shielded_bundle_action_count(spends, outputs, 0, platform_version) + .expect_err("a bundle over the size-derived ceiling must be rejected pre-proving"); + assert!( + err.to_string().contains("max_state_transition_size"), + "unexpected error for {spends} spends / {outputs} outputs: {err}" + ); + } + } + + /// One action over the STRUCTURAL ceiling must fail fast — from the OUTPUT side (a + /// 16-recipient call, which becomes 17 outputs once the unconditional change output is + /// added) and from the SPEND side (a fragmented wallet selecting too many notes). The + /// structural check fires first, so these carry the `max_shielded_transition_actions` + /// message rather than the size-derived one. + #[test] + fn shielded_bundle_action_count_rejects_over_the_consensus_limit() { + let platform_version = PlatformVersion::latest(); + let max = platform_version + .system_limits + .max_shielded_transition_actions as usize; + + for (spends, outputs) in [(1usize, max + 1), (max + 1, 1), (max + 1, max + 1)] { + let err = shielded_bundle_action_count(spends, outputs, 0, platform_version) + .expect_err("a bundle over the consensus action limit must be rejected"); + assert!( + err.to_string().contains("exceeding the consensus limit"), + "unexpected error for {spends} spends / {outputs} outputs: {err}" + ); + } + } } diff --git a/packages/rs-dpp/src/shielded/builder/shield_from_asset_lock.rs b/packages/rs-dpp/src/shielded/builder/shield_from_asset_lock.rs index cb9a3d70503..88850cc2367 100644 --- a/packages/rs-dpp/src/shielded/builder/shield_from_asset_lock.rs +++ b/packages/rs-dpp/src/shielded/builder/shield_from_asset_lock.rs @@ -6,7 +6,34 @@ use crate::state_transition::StateTransition; use crate::ProtocolError; use platform_version::version::PlatformVersion; -use super::{build_output_only_bundle, serialize_authorized_bundle, OrchardProver}; +use super::{ + build_output_only_bundle, serialize_authorized_bundle, serialized_envelope_bytes, + shielded_bundle_action_count, OrchardProver, +}; + +/// The asset-lock proof's envelope contribution BEYOND the measured baseline. +/// +/// [`crate::shielded::SHIELDED_TRANSITION_WIRE_OVERHEAD_BYTES`] was calibrated from complete +/// `ShieldFromAssetLock` transitions that already carried a chain asset-lock proof, so the +/// baseline ALREADY contains one proof's worth of bytes. A supplied proof REPLACES that field +/// rather than adding to it; only the difference grows the transition. Modelling +/// `baseline + full proof` instead is conservative, but it can reject valid transitions at an +/// action boundary — the proof-size window between two action counts is only a few dozen bytes +/// wide (#4312 review finding b6f78dd76eb7). +/// +/// Saturating: a chain proof at or below the baseline's own size yields `0` and leaves the +/// baseline ceiling untouched, while an instant proof's multi-KiB delta still tightens it. +/// +/// Both builders below route their pre-proving size gate through this one function, so the gate +/// and its boundary tests cannot drift apart. +fn asset_lock_proof_envelope_delta_bytes( + asset_lock_proof: &AssetLockProof, +) -> Result { + Ok( + serialized_envelope_bytes(asset_lock_proof, "the asset-lock proof")? + .saturating_sub(crate::shielded::SHIELDED_BASELINE_ASSET_LOCK_PROOF_BYTES), + ) +} /// Builds a ShieldFromAssetLock state transition (core asset lock -> shielded pool). /// @@ -46,6 +73,28 @@ pub fn build_shield_from_asset_lock_transition( dummy_outputs: usize, platform_version: &PlatformVersion, ) -> Result { + // Gate the on-wire action count (1 real output + the anonymity-set fillers) against both + // consensus ceilings — the structural action cap and the transition-size-derived one — + // BEFORE the ~30 s-per-bundle Halo 2 proof. The seeding flow's `MAX_ACTIONS_PER_BATCH` + // stays within this, but the parameter is caller-controlled. Checked: `usize::MAX` dummies + // must not wrap past the gate in release builds. + // + // The size side must price THIS transition's embedded asset-lock proof: an instant proof + // carries its funding transaction and `InstantLock` (both hold input vectors — DPP admits + // up to 100 inputs), which can consume the slack the baseline envelope leaves under + // `max_state_transition_size`. + // + // Price the DELTA, not the whole proof: the baseline envelope was measured from + // `ShieldFromAssetLock` transitions that already carried a chain asset-lock proof, and this + // proof REPLACES that field rather than adding to it. Passing the full serialized size + // would model `baseline + full proof` and reject valid transitions at an action boundary + // (#4312 review finding b6f78dd76eb7). + let num_outputs = dummy_outputs.checked_add(1).ok_or_else(|| { + ProtocolError::ShieldedBuildError("dummy_outputs overflows the output count".to_string()) + })?; + let proof_envelope_delta_bytes = asset_lock_proof_envelope_delta_bytes(&asset_lock_proof)?; + shielded_bundle_action_count(0, num_outputs, proof_envelope_delta_bytes, platform_version)?; + let bundle = build_output_only_bundle( recipient, shield_amount, @@ -127,6 +176,16 @@ where P: OrchardProver, AS: ::key_wallet::signer::Signer, { + // Same pre-proving gate as the non-signer sibling: both consensus ceilings, before the + // proof, with the same checked output-count arithmetic and the same + // transition-specific proof envelope DELTA priced into the size side (the baseline already + // carries a chain proof — see the sibling). + let num_outputs = dummy_outputs.checked_add(1).ok_or_else(|| { + ProtocolError::ShieldedBuildError("dummy_outputs overflows the output count".to_string()) + })?; + let proof_envelope_delta_bytes = asset_lock_proof_envelope_delta_bytes(&asset_lock_proof)?; + shielded_bundle_action_count(0, num_outputs, proof_envelope_delta_bytes, platform_version)?; + let bundle = build_output_only_bundle( recipient, shield_amount, @@ -164,6 +223,283 @@ where .await } +#[cfg(test)] +mod envelope_gate_tests { + //! Serialized-size boundary coverage for the transition-specific envelope + //! in the pre-proving gate (#4312 review finding e90e9cf15f52): a chain + //! proof keeps the baseline ceiling, while a realistic multi-input + //! instant proof consumes the slack and must tighten the ceiling BEFORE + //! any Halo 2 work. + use std::str::FromStr; + + use dashcore::bls_sig_utils::BLSSignature; + use dashcore::hash_types::CycleHash; + use dashcore::transaction::special_transaction::asset_lock::AssetLockPayload; + use dashcore::transaction::special_transaction::TransactionPayload; + use dashcore::{InstantLock, OutPoint, ScriptBuf, Transaction, TxIn, TxOut, Txid}; + use platform_version::version::PlatformVersion; + + use super::asset_lock_proof_envelope_delta_bytes; + use crate::identity::state_transition::asset_lock_proof::chain::ChainAssetLockProof; + use crate::identity::state_transition::asset_lock_proof::{ + AssetLockProof, InstantAssetLockProof, + }; + use crate::shielded::builder::{serialized_envelope_bytes, shielded_bundle_action_count}; + use crate::shielded::{ + max_shielded_actions_for_envelope, max_shielded_actions_per_transition, + SHIELDED_BASELINE_ASSET_LOCK_PROOF_BYTES, + }; + + fn txid() -> Txid { + Txid::from_str("a477af6b2667c29670467e4e0728b685ee07b240235771862318e29ddbe58458").unwrap() + } + + /// An instant asset-lock proof whose funding transaction and + /// `InstantLock` both carry `num_inputs` inputs — the DPP-admitted shape + /// (up to 100 inputs) that the fixed baseline envelope cannot cover. + /// Script sigs are sized like real signed P2PKH inputs (~107 bytes). + fn instant_proof_with_inputs(num_inputs: usize) -> AssetLockProof { + instant_proof_with_padded_input(num_inputs, 0) + } + + /// [`instant_proof_with_inputs`] with `pad` extra bytes on the FIRST input's script sig, so + /// the proof's serialized size can be tuned a byte at a time. Whole inputs move the size in + /// ~190-byte steps, which is far coarser than the boundary windows the gate tests probe. + fn instant_proof_with_padded_input(num_inputs: usize, pad: usize) -> AssetLockProof { + let inputs: Vec = (0..num_inputs) + .map(|i| TxIn { + previous_output: OutPoint::new(txid(), i as u32), + script_sig: ScriptBuf::from(vec![0u8; 107 + if i == 0 { pad } else { 0 }]), + sequence: 0, + witness: Default::default(), + }) + .collect(); + let transaction = Transaction { + version: 3, + lock_time: 0, + input: inputs, + output: vec![TxOut { + value: 100_000_000, + script_pubkey: ScriptBuf::new_op_return(&[]), + }], + special_transaction_payload: Some(TransactionPayload::AssetLockPayloadType( + AssetLockPayload { + version: 0, + credit_outputs: vec![TxOut { + value: 100_000_000, + script_pubkey: ScriptBuf::from(vec![0u8; 25]), + }], + }, + )), + }; + let instant_lock = InstantLock { + version: 1, + inputs: (0..num_inputs) + .map(|i| OutPoint::new(txid(), i as u32)) + .collect(), + txid: transaction.txid(), + cyclehash: CycleHash::from_str( + "7c30826123d0f29fe4c4a8895d7ba4eb469b1fafa6ad7b23896a1a591766a536", + ) + .unwrap(), + signature: BLSSignature::from_str( + "8967c46529a967b3822e1ba8a173066296d02593f0f59b3a78a30a7eef9c8a120847729e62e\ + 4a32954339286b79fe7590221331cd28d576887a263f45b595d499272f656c3f5176987c976\ + 239cac16f972d796ad82931d532102a4f95eec7d80", + ) + .unwrap(), + }; + AssetLockProof::Instant(InstantAssetLockProof::new(instant_lock, transaction, 0)) + } + + /// A chain proof serializes to a few dozen bytes, so pricing it into the + /// gate must NOT move the ceiling off the baseline. + #[test] + fn chain_proof_envelope_keeps_the_baseline_ceiling() { + let platform_version = PlatformVersion::latest(); + let baseline = max_shielded_actions_per_transition(platform_version); + + let proof = AssetLockProof::Chain(ChainAssetLockProof { + core_chain_locked_height: 1_042_000, + out_point: OutPoint::new(txid(), 0), + }); + let bytes = + serialized_envelope_bytes(&proof, "the asset-lock proof").expect("measurable proof"); + assert!( + bytes < 128, + "a chain proof must serialize to a few dozen bytes, got {bytes}" + ); + assert_eq!( + max_shielded_actions_for_envelope(platform_version, bytes), + baseline, + "a chain-proof envelope must keep the baseline ceiling" + ); + // The full baseline-ceiling output shape still passes the gate. + shielded_bundle_action_count(0, baseline, bytes, platform_version) + .expect("a chain-proof shield at the baseline ceiling must pass the pre-proving gate"); + } + + /// A realistic multi-input instant proof consumes more than the slack the + /// baseline ceiling leaves under `max_state_transition_size`, so the gate + /// must reject the baseline-ceiling shape BEFORE proving — the exact + /// scenario the fixed envelope let through. + #[test] + fn multi_input_instant_proof_envelope_tightens_the_ceiling() { + let platform_version = PlatformVersion::latest(); + let baseline = max_shielded_actions_per_transition(platform_version); + + // 20 inputs is well inside DPP's 100-input admission and already + // costs ~3.5 KiB of envelope — more than the ~1.4 KiB slack. + let proof = instant_proof_with_inputs(20); + let bytes = + serialized_envelope_bytes(&proof, "the asset-lock proof").expect("measurable proof"); + let ceiling = max_shielded_actions_for_envelope(platform_version, bytes); + assert!( + ceiling < baseline, + "a {bytes}-byte instant-proof envelope must tighten the ceiling below the \ + baseline {baseline}" + ); + + // The shape that passes with a chain proof must now fail, pre-proving, + // with the size-derived message naming the envelope. + let err = shielded_bundle_action_count(0, baseline, bytes, platform_version).expect_err( + "the baseline-ceiling shape must be rejected pre-proving under a multi-input \ + instant-proof envelope", + ); + let msg = err.to_string(); + assert!( + msg.contains("max_state_transition_size") && msg.contains("transition-specific"), + "unexpected error: {msg}" + ); + + // At the tightened ceiling the gate accepts again — the gate tightens, + // it does not shut. + shielded_bundle_action_count(0, ceiling.max(1), bytes, platform_version) + .expect("the tightened ceiling itself must pass"); + + // And a DPP-maximal 100-input proof must tighten further, never panic. + let max_proof = instant_proof_with_inputs(100); + let max_bytes = serialized_envelope_bytes(&max_proof, "the asset-lock proof") + .expect("measurable proof"); + assert!(max_bytes > bytes); + assert!(max_shielded_actions_for_envelope(platform_version, max_bytes) <= ceiling); + } + + /// `SHIELDED_BASELINE_ASSET_LOCK_PROOF_BYTES` must equal the encoded size of the very proof + /// the baseline envelope was calibrated with — the chain proof from + /// `shield_from_asset_lock_transition/signing_tests.rs::make_chain_asset_lock_proof`, whose + /// transitions produced the 2-, 6- and 7-action measurements behind + /// `SHIELDED_TRANSITION_WIRE_OVERHEAD_BYTES`. If the encoding moves, this fails instead of + /// silently biasing the gate. + #[test] + fn baseline_asset_lock_proof_bytes_matches_the_calibration_proof() { + let calibration_proof = AssetLockProof::Chain(ChainAssetLockProof { + core_chain_locked_height: 100, + out_point: OutPoint::from([11u8; 36]), + }); + let measured = serialized_envelope_bytes(&calibration_proof, "the asset-lock proof") + .expect("measurable proof"); + assert_eq!( + measured, SHIELDED_BASELINE_ASSET_LOCK_PROOF_BYTES, + "the baseline proof allowance must equal the calibration proof's encoded size" + ); + + // The calibration proof itself must therefore cost NOTHING extra: it is exactly what the + // baseline already models. + assert_eq!( + asset_lock_proof_envelope_delta_bytes(&calibration_proof).expect("measurable proof"), + 0, + "the calibration proof must not be charged twice" + ); + assert_eq!( + max_shielded_actions_for_envelope(PlatformVersion::latest(), 0), + max_shielded_actions_per_transition(PlatformVersion::latest()), + ); + } + + /// The fix, at the boundary it actually matters: a proof whose FULL serialized size pushes + /// the action ceiling down by one, but whose DELTA above the baseline does not. + /// + /// Under the old `baseline + full proof` model such a transition was rejected pre-proving + /// even though the real transition — which carries the proof INSTEAD of the baseline's own + /// chain proof — fits under `max_state_transition_size`. The window is only + /// `SHIELDED_BASELINE_ASSET_LOCK_PROOF_BYTES` wide, so it is searched for rather than + /// hardcoded: a limits change moves it, and the test follows. + #[test] + fn proof_envelope_prices_only_the_delta_above_the_baseline() { + let platform_version = PlatformVersion::latest(); + let baseline = max_shielded_actions_per_transition(platform_version); + + // Whole inputs step ~190 B at a time — far coarser than the ~40 B window where the two + // models disagree — so widen by whole inputs first, then by SINGLE script bytes. Take + // the largest input count still below the ceiling drop, then pad one byte at a time + // until the FULL size first costs an action; that first crossing is by construction + // within a byte or two of the threshold, hence inside the window. + let full_bytes = |proof: &AssetLockProof| { + serialized_envelope_bytes(proof, "the asset-lock proof").expect("measurable proof") + }; + let costs_an_action = + |bytes: u64| max_shielded_actions_for_envelope(platform_version, bytes) < baseline; + + let mut num_inputs = 1; + while num_inputs < 100 + && !costs_an_action(full_bytes(&instant_proof_with_inputs(num_inputs + 1))) + { + num_inputs += 1; + } + assert!( + !costs_an_action(full_bytes(&instant_proof_with_inputs(num_inputs))), + "the starting input count must still afford the baseline ceiling" + ); + + let mut boundary = None; + for pad in 0..1_024 { + let proof = instant_proof_with_padded_input(num_inputs, pad); + let full = full_bytes(&proof); + if costs_an_action(full) { + boundary = Some((proof, full)); + break; + } + } + let (proof, full) = boundary.expect("byte-level padding must cross the threshold"); + let delta = asset_lock_proof_envelope_delta_bytes(&proof).expect("measurable proof"); + + assert_eq!( + delta, + full - SHIELDED_BASELINE_ASSET_LOCK_PROOF_BYTES, + "the delta must be the full proof minus the baseline's own proof" + ); + assert_eq!( + max_shielded_actions_for_envelope(platform_version, delta), + baseline, + "at this boundary the delta must still afford the baseline ceiling ({baseline}), \ + while the full proof ({full} bytes) does not — that gap is the double-count" + ); + + // The gate as the BUILDERS call it (same helper) now accepts the baseline-ceiling shape + // that the old full-proof model rejected pre-proving. + shielded_bundle_action_count(0, baseline, delta, platform_version).expect( + "the boundary transition must pass the pre-proving gate once the proof is priced \ + as a delta", + ); + shielded_bundle_action_count(0, baseline, full, platform_version).expect_err( + "the old full-proof model rejected this exact shape — that is the regression this \ + test pins", + ); + + // The correction is a bounded credit, not a hole: a proof large enough on its own still + // tightens the ceiling even after the baseline is subtracted. + let big = instant_proof_with_inputs(100); + let big_delta = asset_lock_proof_envelope_delta_bytes(&big).expect("measurable proof"); + assert!( + max_shielded_actions_for_envelope(platform_version, big_delta) < baseline, + "a DPP-maximal instant proof must still tighten the ceiling" + ); + shielded_bundle_action_count(0, baseline, big_delta, platform_version) + .expect_err("a DPP-maximal instant proof must still be rejected at the baseline shape"); + } +} + #[cfg(test)] mod tests { use super::super::{build_output_only_bundle, serialize_authorized_bundle}; diff --git a/packages/rs-dpp/src/shielded/builder/shielded_transfer.rs b/packages/rs-dpp/src/shielded/builder/shielded_transfer.rs index 3c870f37c3c..3d2fb3251f1 100644 --- a/packages/rs-dpp/src/shielded/builder/shielded_transfer.rs +++ b/packages/rs-dpp/src/shielded/builder/shielded_transfer.rs @@ -12,7 +12,10 @@ use crate::state_transition::StateTransition; use crate::ProtocolError; use platform_version::version::PlatformVersion; -use super::{prove_and_sign_bundle, serialize_authorized_bundle, OrchardProver, SpendableNote}; +use super::{ + prove_and_sign_bundle, serialize_authorized_bundle, shielded_bundle_action_count, + OrchardProver, SpendableNote, +}; /// Builds a ShieldedTransfer state transition (shielded pool -> shielded pool). /// @@ -54,9 +57,25 @@ pub fn build_shielded_transfer_transition( ) -> Result<(StateTransition, Credits), ProtocolError> { let total_spent: u64 = spends.iter().map(|s| s.note.value().inner()).sum(); - // Conservative action count: at least (spends, 2) since we always have - // a recipient output and likely a change output. - let num_actions = spends.len().max(2); + // Action count = max(spends, outputs), padded to Orchard's 2-action minimum. This bundle + // publishes at most two outputs (recipient + change), and the no-change case collapses to the + // same number because of that padding: `max(n, 1).max(2) == max(n, 2).max(2)`. So sizing the + // fee for the with-change shape is exact in BOTH branches — see the + // `single_output_transfer_fee_matches_on_wire_action_count` test, which pins the carved fee + // against the bundle's real `actions.len()`. + // + // The helper also enforces the consensus action ceiling + // (`max_shielded_transition_actions`), so a wallet fragmented enough to need more spends + // than consensus allows fails here rather than after the ~30 s proof. + const MAX_OUTPUTS: usize = 2; // recipient + change + let num_actions = shielded_bundle_action_count( + spends.len(), + MAX_OUTPUTS, + // No variable-length envelope beyond the baseline: a transfer's + // non-Orchard fields are fixed-size. + 0, + platform_version, + )?; // The fee is fixed at the minimum: a transfer's `value_balance` IS the fee and consensus // pins it to exactly this amount (overpayment buys nothing and would leak a distinguishing // fee fingerprint that breaks shielded uniformity). @@ -134,12 +153,743 @@ pub fn build_shielded_transfer_transition( Ok((state_transition, fee)) } +/// One recipient output of a multi-output [`build_shielded_transfer_transition_multi`]. +/// +/// Each entry becomes its own Orchard output — its own note, with its own randomness and so its +/// own (deterministic) nullifier when later spent. Two entries may name the SAME `recipient` +/// address: Orchard derives independent notes regardless, which is exactly how a single transfer +/// funds one address with several notes. +#[derive(Clone, Copy, Debug)] +pub struct ShieldedTransferOutput { + /// Orchard address receiving this note. + pub recipient: OrchardAddress, + /// Value of this note, in credits. + pub amount: u64, + /// 36-byte structured memo (4-byte type tag + 32-byte payload) for this note. + pub memo: [u8; 36], +} + +/// Builds a ShieldedTransfer state transition with **several** recipient outputs in one atomic +/// bundle (shielded pool -> shielded pool). +/// +/// This is the multi-output sibling of [`build_shielded_transfer_transition`]. It exists because +/// some flows must land more than one note in a single transition — most importantly, funding an +/// address with two sub-target notes so that a later spend of that address is forced to spend +/// BOTH of them. +/// +/// # Why more than one output changes the fee +/// +/// An Orchard action is a joined spend/output slot, so the on-wire action count is +/// `max(num_spends, num_outputs)` padded to `MIN_ACTIONS = 2`. A `ShieldedTransfer`'s +/// `value_balance` IS its fee and consensus pins it to `compute_minimum_shielded_fee(actions.len())` +/// **exactly**. With three or more outputs the output side sets the action count, so the fee MUST +/// be sized from it — see [`shielded_bundle_action_count`]. +/// +/// # Deterministic shape +/// +/// This builder ALWAYS emits a change output and therefore requires the spent value to STRICTLY +/// exceed `sum(amounts) + fee`. That makes the output count — and hence the action count and the +/// fee — a pure function of the inputs (`max(spends, recipients + 1, 2)`), with no circular +/// dependency between "is there change?" and "what is the fee?". A caller that spends *exactly* +/// `sum(amounts) + fee` is rejected rather than silently re-shaped into a different action count; +/// note selection always reserves against the same `recipients + 1` floor, so the reserved fee and +/// the carved fee cannot diverge. +/// +/// All recipient outputs and the change output are encrypted with the sender's External-scope OVK, +/// so the sender can recover its own send history from chain data (see +/// [`build_shielded_transfer_transition`]). +/// +/// # Parameters +/// - `spends` - Notes to spend with their Merkle paths +/// - `outputs` - Recipient outputs; must be non-empty +/// - `change_address` - Orchard address for the (always present) change output +/// - `fvk` / `ask` - Full viewing key and spend authorizing key +/// - `anchor` - Sinsemilla root of the note commitment tree +/// - `prover` - Orchard prover (holds the Halo 2 proving key) +/// - `platform_version` - Protocol version +/// +/// Returns the built transition together with the fee (in credits) that was applied. +#[allow(clippy::too_many_arguments)] +pub fn build_shielded_transfer_transition_multi( + spends: Vec, + outputs: &[ShieldedTransferOutput], + change_address: &OrchardAddress, + fvk: &FullViewingKey, + ask: &SpendAuthorizingKey, + anchor: Anchor, + prover: &P, + platform_version: &PlatformVersion, +) -> Result<(StateTransition, Credits), ProtocolError> { + if outputs.is_empty() { + return Err(ProtocolError::ShieldedBuildError( + "a multi-output shielded transfer needs at least one recipient output".to_string(), + )); + } + // Every recipient amount must be positive, enforced HERE — the lowest public builder — so + // the invariant does not depend on the entry point. The C, JNI and Kotlin boundaries all + // reject a zero amount already; a direct Rust caller bypassed them and could mint a + // zero-valued recipient note. + // + // That also protects the motivating two-note layout: with `[0, D]` a greedy claim + // selection covers the target from the full-value note alone and stops, leaving the + // zero-value note unspent — which restores exactly the random-padding nullifier the + // two-note split exists to avoid (#4312 review finding 1720257964f4). + if let Some((index, _)) = outputs + .iter() + .enumerate() + .find(|(_, output)| output.amount == 0) + { + return Err(ProtocolError::ShieldedBuildError(format!( + "multi-output shielded transfer amount at index {index} must be positive" + ))); + } + + // Checked: a crafted output set could otherwise wrap u64 in release builds. + let transfer_total = outputs + .iter() + .try_fold(0u64, |acc, o| acc.checked_add(o.amount)) + .ok_or_else(|| { + ProtocolError::ShieldedBuildError( + "multi-output shielded transfer amounts overflow u64".to_string(), + ) + })?; + let total_spent = spends + .iter() + .try_fold(0u64, |acc, s| acc.checked_add(s.note.value().inner())) + .ok_or_else(|| { + ProtocolError::ShieldedBuildError( + "multi-output shielded transfer total spent value overflows u64".to_string(), + ) + })?; + + // A change output is always emitted (see the doc comment), so the output count — and with it + // the action count and the fee — is fixed before any value arithmetic. + let num_outputs = outputs.len().checked_add(1).ok_or_else(|| { + ProtocolError::ShieldedBuildError("output count overflows usize".to_string()) + })?; + // Also the consensus action-ceiling gate: `max(spends, recipients + 1)` must stay within + // `max_shielded_transition_actions`, or the transition is doomed at `validate_structure`. + // `try_from_bundle` performs no structural validation, so without this the caller would burn + // the ~30 s Halo 2 proof on a bundle consensus is guaranteed to reject. + let num_actions = shielded_bundle_action_count( + spends.len(), + num_outputs, + // No variable-length envelope beyond the baseline: a transfer's + // non-Orchard fields are fixed-size. + 0, + platform_version, + )?; + let fee = compute_minimum_shielded_fee(num_actions, platform_version)?; + + let required = transfer_total.checked_add(fee).ok_or_else(|| { + ProtocolError::ShieldedBuildError("fee + transfer amounts overflow u64".to_string()) + })?; + // STRICTLY greater: the change output is unconditional, so it must carry a positive value. + if required >= total_spent { + return Err(ProtocolError::ShieldedBuildError(format!( + "transfer amounts {} + fee {} = {} must be strictly less than the total spendable \ + value {} (a multi-output transfer always emits a change output)", + transfer_total, fee, required, total_spent + ))); + } + let change_amount = total_spent - required; + + let sender_ovk = fvk.to_ovk(Scope::External); + let mut builder = Builder::::new(BundleType::DEFAULT, anchor); + + for spend in spends { + builder + .add_spend(fvk.clone(), spend.note, spend.merkle_path) + .map_err(|e| { + ProtocolError::ShieldedBuildError(format!("failed to add spend: {:?}", e)) + })?; + } + + for output in outputs { + builder + .add_output( + Some(sender_ovk.clone()), + PaymentAddress::from(&output.recipient), + NoteValue::from_raw(output.amount), + output.memo, + ) + .map_err(|e| { + ProtocolError::ShieldedBuildError(format!("failed to add output: {:?}", e)) + })?; + } + + builder + .add_output( + Some(sender_ovk), + PaymentAddress::from(change_address), + NoteValue::from_raw(change_amount), + [0u8; 36], + ) + .map_err(|e| { + ProtocolError::ShieldedBuildError(format!("failed to add change output: {:?}", e)) + })?; + + // ShieldedTransfer has no extra_data in sighash + let bundle = prove_and_sign_bundle(builder, prover, std::slice::from_ref(ask), &[])?; + let sb = serialize_authorized_bundle(&bundle); + + // The fee was predicted before the bundle existed; consensus recomputes it from the ON-WIRE + // action count and demands exact equality. Catch any divergence here (cheap) instead of as an + // opaque rejection after the ~30 s proof. + if sb.actions.len() != num_actions { + return Err(ProtocolError::ShieldedBuildError(format!( + "predicted {} actions but the bundle published {}; the carved fee would not match \ + the consensus minimum", + num_actions, + sb.actions.len() + ))); + } + + let state_transition = ShieldedTransferTransition::try_from_bundle( + sb.actions, + sb.value_balance as u64, + sb.anchor, + sb.proof, + sb.binding_signature, + platform_version, + )?; + Ok((state_transition, fee)) +} + #[cfg(test)] mod tests { use super::*; use crate::shielded::builder::test_helpers::{ test_orchard_address, test_spendable_note, TestProver, }; + use grovedb_commitment_tree::{ + ExtractedNoteCommitment, Hashable, MerkleHashOrchard, MerklePath, SpendingKey, + NOTE_COMMITMENT_TREE_DEPTH, + }; + + /// Two distinct notes witnessed in one two-leaf commitment tree, plus the shared anchor. + /// + /// Each path's level-0 sibling is the other leaf and the upper siblings are shared, so both + /// witnesses compute the SAME root — a consistent anchor the Orchard circuit accepts. (Same + /// construction the identity-create builder's two-spend test uses.) + fn two_spends_in_one_tree( + value_a: u64, + value_b: u64, + fvk: &FullViewingKey, + ) -> (Vec, Anchor, [[u8; 32]; 2]) { + let note_a = test_spendable_note(value_a).note; + let note_b = test_spendable_note(value_b).note; + let cmx_a = ExtractedNoteCommitment::from(note_a.commitment()); + let cmx_b = ExtractedNoteCommitment::from(note_b.commitment()); + + let mut auth_path_a = [MerkleHashOrchard::empty_leaf(); NOTE_COMMITMENT_TREE_DEPTH]; + auth_path_a[0] = MerkleHashOrchard::from_cmx(&cmx_b); + let mut auth_path_b = [MerkleHashOrchard::empty_leaf(); NOTE_COMMITMENT_TREE_DEPTH]; + auth_path_b[0] = MerkleHashOrchard::from_cmx(&cmx_a); + let path_a = MerklePath::from_parts(0, auth_path_a); + let path_b = MerklePath::from_parts(1, auth_path_b); + + let anchor = path_a.root(cmx_a); + assert_eq!( + anchor.to_bytes(), + path_b.root(cmx_b).to_bytes(), + "both witnesses must compute the same anchor" + ); + + let nullifiers = [ + note_a.nullifier(fvk).to_bytes(), + note_b.nullifier(fvk).to_bytes(), + ]; + ( + vec![ + SpendableNote { + note: note_a, + merkle_path: path_a, + }, + SpendableNote { + note: note_b, + merkle_path: path_b, + }, + ], + anchor, + nullifiers, + ) + } + + /// Destructure a built `ShieldedTransfer` into `(actions.len(), value_balance)` — the two + /// fields consensus reads when it recomputes and pins the fee. + fn on_wire_actions_and_value_balance(st: &StateTransition) -> (usize, u64) { + match st { + StateTransition::ShieldedTransfer( + crate::state_transition::shielded_transfer_transition::ShieldedTransferTransition::V0(v0), + ) => (v0.actions.len(), v0.value_balance), + other => panic!("expected a ShieldedTransfer transition, got {other:?}"), + } + } + + /// THE regression pin for the multi-output fee predictor. + /// + /// A `ShieldedTransfer`'s `value_balance` IS its fee, and consensus pins it to + /// `compute_minimum_shielded_fee(actions.len())` EXACTLY (see + /// `validate_minimum_shielded_fee`: `amount_is_pure_fee` rejects both under- and + /// over-payment). With two recipient outputs plus change the bundle publishes THREE actions, + /// so a spends-only predictor (`spends.len().max(2)`) would carve `min_fee(2)` and be + /// rejected on chain. This asserts the carved fee equals `min_fee(on-wire actions.len())` + /// and, explicitly, that it is NOT the 2-action fee. + /// + /// It also pins the two-notes-to-one-address shape: both outputs name the SAME recipient and + /// must still become two DISTINCT notes (distinct commitments), which is what makes a later + /// spend of that address spend two real notes rather than one real note plus a random dummy. + #[test] + fn multi_output_transfer_fee_matches_on_wire_action_count() { + let platform_version = PlatformVersion::latest(); + let sk = SpendingKey::from_bytes([42u8; 32]).expect("valid spending key"); + let fvk = FullViewingKey::from(&sk); + let ask = SpendAuthorizingKey::from(&sk); + let change_address = test_orchard_address(); + let recipient = test_orchard_address(); + + let (spends, anchor, _) = two_spends_in_one_tree(6_000_000_000, 7_000_000_000, &fvk); + + // The two-note invite funding shape: D split into floor(D/2) + ceil(D/2), both to the + // SAME one-time address, each strictly below D. + const D: u64 = 3_000_000_000; // 0.03 DASH in credits + let outputs = vec![ + ShieldedTransferOutput { + recipient, + amount: D / 2, + memo: [0u8; 36], + }, + ShieldedTransferOutput { + recipient, + amount: D - D / 2, + memo: [0u8; 36], + }, + ]; + + let (st, fee) = build_shielded_transfer_transition_multi( + spends, + &outputs, + &change_address, + &fvk, + &ask, + anchor, + &TestProver, + platform_version, + ) + .expect("a two-spend, three-output transfer must build"); + + let (num_actions, value_balance) = on_wire_actions_and_value_balance(&st); + assert_eq!( + num_actions, 3, + "2 spends + 3 outputs (2 recipients + change) must publish max(2,3) = 3 actions" + ); + + let expected_fee = compute_minimum_shielded_fee(num_actions, platform_version) + .expect("fee computation should not overflow"); + assert_eq!( + fee, expected_fee, + "the carved fee must equal compute_minimum_shielded_fee(on-wire actions.len())" + ); + assert_eq!( + value_balance, expected_fee, + "value_balance IS the fee and consensus pins it to the minimum for the on-wire \ + action count exactly" + ); + + // The bug this fixes: the old spends-only predictor would have carved the 2-action fee. + let two_action_fee = compute_minimum_shielded_fee(2, platform_version) + .expect("fee computation should not overflow"); + assert!( + expected_fee > two_action_fee, + "a 3-action bundle must cost strictly more than a 2-action one, otherwise this test \ + cannot detect the under-count" + ); + assert_ne!( + fee, two_action_fee, + "a spends-only fee predictor would carve the 2-action fee and be rejected on chain" + ); + + // Two outputs to the SAME address are still two distinct notes. + let commitments: Vec<[u8; 32]> = match &st { + StateTransition::ShieldedTransfer( + crate::state_transition::shielded_transfer_transition::ShieldedTransferTransition::V0(v0), + ) => v0.actions.iter().map(|a| a.cmx).collect(), + _ => unreachable!(), + }; + let unique: std::collections::BTreeSet<[u8; 32]> = commitments.iter().copied().collect(); + assert_eq!( + unique.len(), + commitments.len(), + "every published note commitment must be distinct, including the two notes paid to \ + the same address" + ); + } + + /// The single-output builder's fee must ALSO equal `min_fee(on-wire actions.len())`. Its + /// output count (recipient + change = 2) can never exceed Orchard's 2-action minimum, so + /// routing it through `shielded_bundle_action_count` is numerically a no-op — this pins that + /// claim against a real bundle so the shared helper cannot regress it. + #[test] + fn single_output_transfer_fee_matches_on_wire_action_count() { + let platform_version = PlatformVersion::latest(); + let sk = SpendingKey::from_bytes([42u8; 32]).expect("valid spending key"); + let fvk = FullViewingKey::from(&sk); + let ask = SpendAuthorizingKey::from(&sk); + let change_address = test_orchard_address(); + let recipient = test_orchard_address(); + + let (spends, anchor, _) = two_spends_in_one_tree(6_000_000_000, 7_000_000_000, &fvk); + + let (st, fee) = build_shielded_transfer_transition( + spends, + &recipient, + 3_000_000_000, + &change_address, + &fvk, + &ask, + anchor, + &TestProver, + [0u8; 36], + platform_version, + ) + .expect("a two-spend, two-output transfer must build"); + + let (num_actions, value_balance) = on_wire_actions_and_value_balance(&st); + assert_eq!( + num_actions, 2, + "2 spends + 2 outputs must publish 2 actions" + ); + let expected_fee = compute_minimum_shielded_fee(num_actions, platform_version) + .expect("fee computation should not overflow"); + assert_eq!(fee, expected_fee); + assert_eq!( + value_balance, expected_fee, + "value_balance must equal the minimum fee for the on-wire action count exactly" + ); + } + + #[test] + fn multi_output_transfer_rejects_empty_output_set() { + let platform_version = PlatformVersion::latest(); + let sk = SpendingKey::from_bytes([42u8; 32]).expect("valid sk"); + let fvk = FullViewingKey::from(&sk); + let ask = SpendAuthorizingKey::from(&sk); + let change_address = test_orchard_address(); + + let err = build_shielded_transfer_transition_multi( + vec![test_spendable_note(u64::MAX / 2)], + &[], + &change_address, + &fvk, + &ask, + Anchor::empty_tree(), + &TestProver, + platform_version, + ) + .expect_err("an empty output set must be rejected"); + assert!( + err.to_string().contains("at least one recipient output"), + "unexpected error: {err}" + ); + } + + /// A zero-valued recipient output must be refused by the BUILDER, not only by the C / JNI / + /// Kotlin boundaries — otherwise the amount invariant depends on the entry point, and the + /// `[0, D]` shape lets greedy claim selection stop after the full-value note and leave the + /// zero-value note unspent, restoring the random-padding nullifier the two-note layout + /// exists to avoid (#4312 review finding 1720257964f4). + #[test] + fn multi_output_transfer_rejects_zero_valued_recipient_output() { + let platform_version = PlatformVersion::latest(); + let sk = SpendingKey::from_bytes([42u8; 32]).expect("valid sk"); + let fvk = FullViewingKey::from(&sk); + let ask = SpendAuthorizingKey::from(&sk); + let change_address = test_orchard_address(); + let recipient = test_orchard_address(); + + // The motivating `[0, D]` two-note shape: the zero output sits FIRST, so the error must + // name index 0 and the rejection must not depend on the position of the full-value note. + let outputs = vec![ + ShieldedTransferOutput { + recipient, + amount: 0, + memo: [0u8; 36], + }, + ShieldedTransferOutput { + recipient, + amount: 1_000_000, + memo: [0u8; 36], + }, + ]; + + let err = build_shielded_transfer_transition_multi( + vec![test_spendable_note(u64::MAX / 2)], + &outputs, + &change_address, + &fvk, + &ask, + Anchor::empty_tree(), + &TestProver, + platform_version, + ) + .expect_err("a zero-valued recipient output must be rejected"); + let msg = err.to_string(); + assert!( + msg.contains("amount at index 0") && msg.contains("must be positive"), + "unexpected error: {msg}" + ); + + // A zero in any LATER position is rejected the same way, with its own index. + let outputs = vec![ + ShieldedTransferOutput { + recipient, + amount: 1_000_000, + memo: [0u8; 36], + }, + ShieldedTransferOutput { + recipient, + amount: 0, + memo: [0u8; 36], + }, + ]; + let err = build_shielded_transfer_transition_multi( + vec![test_spendable_note(u64::MAX / 2)], + &outputs, + &change_address, + &fvk, + &ask, + Anchor::empty_tree(), + &TestProver, + platform_version, + ) + .expect_err("a trailing zero-valued recipient output must be rejected"); + assert!( + err.to_string().contains("amount at index 1"), + "unexpected error: {err}" + ); + } + + /// The multi-output builder always emits a change output, so it requires the spent value to + /// STRICTLY exceed `sum(amounts) + fee`. Spending exactly that much is rejected rather than + /// silently re-shaped into a different (and differently priced) action count. + #[test] + fn multi_output_transfer_requires_strictly_positive_change() { + let platform_version = PlatformVersion::latest(); + let sk = SpendingKey::from_bytes([42u8; 32]).expect("valid sk"); + let fvk = FullViewingKey::from(&sk); + let ask = SpendAuthorizingKey::from(&sk); + let change_address = test_orchard_address(); + let recipient = test_orchard_address(); + + // One spend + 3 outputs (2 recipients + change) → max(1, 3) = 3 actions. + let fee = compute_minimum_shielded_fee(3, platform_version).expect("fee"); + let amount = 1_000_000u64; + // Exactly `sum + fee` — the boundary that must be rejected. + let note = test_spendable_note(2 * amount + fee); + let outputs = vec![ + ShieldedTransferOutput { + recipient, + amount, + memo: [0u8; 36], + }, + ShieldedTransferOutput { + recipient, + amount, + memo: [0u8; 36], + }, + ]; + + let err = build_shielded_transfer_transition_multi( + vec![note], + &outputs, + &change_address, + &fvk, + &ask, + Anchor::empty_tree(), + &TestProver, + platform_version, + ) + .expect_err("spending exactly sum + fee must be rejected"); + assert!( + err.to_string().contains("strictly less than"), + "unexpected error: {err}" + ); + } + + // -------------------------------------------------------------- + // Consensus action ceiling (`max_shielded_transition_actions`) + // -------------------------------------------------------------- + + /// Build `count` recipient outputs of `amount` each, all to the same test address. + fn n_outputs(count: usize, amount: u64) -> Vec { + let recipient = test_orchard_address(); + (0..count) + .map(|_| ShieldedTransferOutput { + recipient, + amount, + memo: [0u8; 36], + }) + .collect() + } + + /// `max_shielded_transition_actions` recipient outputs plus the unconditional change output + /// publish one action too many. `try_from_bundle` runs no structural validation, so without + /// an up-front gate this bundle would be laid out, proved (~30 s of Halo 2) and only then + /// rejected by consensus at `validate_structure`. It must fail BEFORE proving — this test + /// completing in milliseconds is itself part of the assertion. + #[test] + fn multi_output_transfer_rejects_output_count_over_the_action_limit() { + let platform_version = PlatformVersion::latest(); + let max = platform_version + .system_limits + .max_shielded_transition_actions as usize; + let sk = SpendingKey::from_bytes([42u8; 32]).expect("valid sk"); + let fvk = FullViewingKey::from(&sk); + let ask = SpendAuthorizingKey::from(&sk); + let change_address = test_orchard_address(); + + // `max` recipients → `max + 1` outputs once change is added — over even the structural + // cap, so this carries the `max_shielded_transition_actions` message (the size-derived + // ceiling below 16 is exercised separately). + let outputs = n_outputs(max, 1_000_000); + + let err = build_shielded_transfer_transition_multi( + vec![test_spendable_note(u64::MAX / 2)], + &outputs, + &change_address, + &fvk, + &ask, + Anchor::empty_tree(), + &TestProver, + platform_version, + ) + .expect_err("a bundle over the consensus action limit must be rejected"); + assert!( + err.to_string().contains("exceeding the consensus limit"), + "unexpected error: {err}" + ); + } + + /// The size-derived ceiling binds BELOW the structural cap: `effective` recipients plus the + /// unconditional change output is `effective + 1` actions (7 at current constants) — within + /// `max_shielded_transition_actions` (16), yet the transition would serialize to ~21.7 KiB + /// against the 20 KiB `max_state_transition_size` and die at DAPI's byte prefilter after + /// ~30 s of proving. It must fail BEFORE proving with the size-derived message — this test + /// completing in milliseconds is itself part of the assertion. + #[test] + fn multi_output_transfer_rejects_output_count_over_the_size_ceiling() { + let platform_version = PlatformVersion::latest(); + let effective = crate::shielded::max_shielded_actions_per_transition(platform_version); + assert!( + effective + < platform_version + .system_limits + .max_shielded_transition_actions as usize, + "this test requires the size limit to be the binding one; if it was raised, rework" + ); + let sk = SpendingKey::from_bytes([42u8; 32]).expect("valid sk"); + let fvk = FullViewingKey::from(&sk); + let ask = SpendAuthorizingKey::from(&sk); + let change_address = test_orchard_address(); + + // `effective` recipients → `effective + 1` outputs once change is added. + let outputs = n_outputs(effective, 1_000_000); + + let err = build_shielded_transfer_transition_multi( + vec![test_spendable_note(u64::MAX / 2)], + &outputs, + &change_address, + &fvk, + &ask, + Anchor::empty_tree(), + &TestProver, + platform_version, + ) + .expect_err("a bundle over the size-derived ceiling must be rejected pre-proving"); + assert!( + err.to_string().contains("max_state_transition_size"), + "unexpected error: {err}" + ); + } + + /// The boundary itself must still build: `effective - 1` recipients plus change is exactly + /// the effective action ceiling (6 at current constants). Neither gate may reject it — the + /// build gets past the fee/limit arithmetic and only stops at the (unrelated) `add_spend` + /// anchor mismatch of the test note, which is how the other builder tests pin "proceeded + /// past the value checks" without paying for a real proof. + #[test] + fn multi_output_transfer_accepts_the_action_limit_boundary() { + let platform_version = PlatformVersion::latest(); + let effective = crate::shielded::max_shielded_actions_per_transition(platform_version); + let sk = SpendingKey::from_bytes([42u8; 32]).expect("valid sk"); + let fvk = FullViewingKey::from(&sk); + let ask = SpendAuthorizingKey::from(&sk); + let change_address = test_orchard_address(); + + let outputs = n_outputs(effective - 1, 1_000_000); + + let err = build_shielded_transfer_transition_multi( + vec![test_spendable_note(u64::MAX / 2)], + &outputs, + &change_address, + &fvk, + &ask, + Anchor::empty_tree(), + &TestProver, + platform_version, + ) + .expect_err("the test note's all-zero path mismatches the empty-tree anchor"); + let err = err.to_string(); + assert!( + !err.contains("exceeding the consensus limit") + && !err.contains("max_state_transition_size"), + "exactly {effective} actions is AT the effective ceiling and must not be rejected \ + by either gate, got: {err}" + ); + assert!( + err.contains("failed to add spend") || err.contains("nchor"), + "expected the downstream add_spend error, got: {err}" + ); + } + + /// The spend side can breach the ceiling too: a fragmented wallet selecting more notes than + /// `max_shielded_transition_actions` publishes one action per spend. The single-output + /// builder shares the same gate, so it must fail fast as well. + #[test] + fn transfer_rejects_spend_count_over_the_action_limit() { + let platform_version = PlatformVersion::latest(); + let max = platform_version + .system_limits + .max_shielded_transition_actions as usize; + let sk = SpendingKey::from_bytes([42u8; 32]).expect("valid sk"); + let fvk = FullViewingKey::from(&sk); + let ask = SpendAuthorizingKey::from(&sk); + let change_address = test_orchard_address(); + let recipient = test_orchard_address(); + + let spends: Vec = (0..max + 1) + .map(|_| test_spendable_note(1_000_000_000)) + .collect(); + + let err = build_shielded_transfer_transition( + spends, + &recipient, + 1_000, + &change_address, + &fvk, + &ask, + Anchor::empty_tree(), + &TestProver, + [0u8; 36], + platform_version, + ) + .expect_err("more spends than the consensus action limit must be rejected"); + assert!( + err.to_string().contains("exceeding the consensus limit"), + "unexpected error: {err}" + ); + } #[test] fn test_shielded_transfer_insufficient_funds() { diff --git a/packages/rs-dpp/src/shielded/builder/shielded_withdrawal.rs b/packages/rs-dpp/src/shielded/builder/shielded_withdrawal.rs index aab8ddac1fb..176c2a05a2b 100644 --- a/packages/rs-dpp/src/shielded/builder/shielded_withdrawal.rs +++ b/packages/rs-dpp/src/shielded/builder/shielded_withdrawal.rs @@ -11,7 +11,10 @@ use crate::withdrawal::Pooling; use crate::ProtocolError; use platform_version::version::PlatformVersion; -use super::{build_spend_bundle, serialize_authorized_bundle, OrchardProver, SpendableNote}; +use super::{ + build_spend_bundle, serialize_authorized_bundle, shielded_bundle_action_count, OrchardProver, + SpendableNote, +}; /// Builds a ShieldedWithdrawal state transition (shielded pool -> core L1 address). /// @@ -70,7 +73,19 @@ pub fn build_shielded_withdrawal_transition( // otherwise consensus recomputes min_fee from the on-wire actions.len() == 2 and // rejects an honest single-spend withdrawal with InsufficientShieldedFeeError (or, // post-fee, WithdrawalBelowMinAmountError). - let num_actions = spends.len().max(2); + // + // Routed through the shared predictor (1 shielded output — the change note), which is + // numerically `spends.len().max(2)` AND enforces both consensus ceilings (the structural + // action cap and the transition-size-derived one) BEFORE the ~30 s Halo 2 proof. + let num_actions = shielded_bundle_action_count( + spends.len(), + 1, + // No variable-length envelope beyond the measured baseline: this + // transition's non-Orchard fields are fixed-size (no embedded + // asset-lock proof or identity key set). + 0, + platform_version, + )?; // The fee is fixed at the withdrawal minimum: consensus always carves exactly // `compute_shielded_withdrawal_fee` from the pool — the base shielded minimum fee PLUS the // flat storage cost of the Core withdrawal document this transition inserts — and the net diff --git a/packages/rs-dpp/src/shielded/builder/unshield.rs b/packages/rs-dpp/src/shielded/builder/unshield.rs index b024117782e..083e76405f1 100644 --- a/packages/rs-dpp/src/shielded/builder/unshield.rs +++ b/packages/rs-dpp/src/shielded/builder/unshield.rs @@ -9,7 +9,10 @@ use crate::state_transition::StateTransition; use crate::ProtocolError; use platform_version::version::PlatformVersion; -use super::{build_spend_bundle, serialize_authorized_bundle, OrchardProver, SpendableNote}; +use super::{ + build_spend_bundle, serialize_authorized_bundle, shielded_bundle_action_count, OrchardProver, + SpendableNote, +}; /// Builds an Unshield state transition (shielded pool -> platform address). /// @@ -64,7 +67,19 @@ pub fn build_unshield_transition( // actions. Price the fee against that same floor (matching shielded_transfer); // otherwise consensus recomputes min_fee from the on-wire actions.len() == 2 and // rejects an honest single-spend unshield with InsufficientShieldedFeeError. - let num_actions = spends.len().max(2); + // + // Routed through the shared predictor (1 shielded output — the change note), which is + // numerically `spends.len().max(2)` AND enforces both consensus ceilings (the structural + // action cap and the transition-size-derived one) BEFORE the ~30 s Halo 2 proof. + let num_actions = shielded_bundle_action_count( + spends.len(), + 1, + // No variable-length envelope beyond the measured baseline: this + // transition's non-Orchard fields are fixed-size (no embedded + // asset-lock proof or identity key set). + 0, + platform_version, + )?; // The fee is fixed at the unshield minimum: consensus always carves exactly // `compute_shielded_unshield_fee` from the pool — the base shielded minimum fee PLUS the flat // storage cost of the single `AddBalanceToAddress` write this transition performs — and the net diff --git a/packages/rs-dpp/src/shielded/mod.rs b/packages/rs-dpp/src/shielded/mod.rs index edf4a0a7de2..622985c36cc 100644 --- a/packages/rs-dpp/src/shielded/mod.rs +++ b/packages/rs-dpp/src/shielded/mod.rs @@ -5,6 +5,9 @@ mod compute_minimum_shielded_fee; pub mod memo; mod sighash; +#[cfg(all(test, feature = "shielded-client"))] +mod wire_cost_measured_tests; + pub use memo::{ShieldedMemo, MEMO_PAYLOAD_SIZE, MEMO_SIZE}; use bincode::{Decode, Encode}; @@ -29,6 +32,162 @@ pub use sighash::{ unshield_extra_sighash_data_v0, }; +/// On-wire serialized size of one [`SerializedAction`]: 408 bytes. +/// +/// `nullifier` (32) + `rk` (32) + `cmx` (32) + `encrypted_note` (216) + +/// `cv_net` (32) + `spend_auth_sig` (64). This is the per-action cost in the +/// transition's `actions` vector, EXCLUDING the Halo 2 proof's per-action +/// growth (see [`SHIELDED_PROOF_WIRE_BYTES_PER_ACTION`]). +pub const SHIELDED_ACTION_WIRE_BYTES: u64 = 408; + +/// On-wire growth of the Halo 2 proof per additional Orchard action: 2,273 bytes. +/// +/// The proof over the Orchard circuit grows linearly with the number of action +/// instances. Measured on real proved transitions (see the +/// `seed_pool_batch_fits_max_state_transition_size` signing test in +/// `shield_from_asset_lock_transition/signing_tests.rs`): 2 actions → 8,294 B +/// total, 6 → 19,018 B, 7 → 21,699 B — an exactly linear 2,681 B/action, of +/// which 408 B is the serialized action ([`SHIELDED_ACTION_WIRE_BYTES`]) and +/// 2,273 B is proof growth. Pinned by +/// `shielded_wire_cost_model_matches_measured_transitions` below. +pub const SHIELDED_PROOF_WIRE_BYTES_PER_ACTION: u64 = 2_273; + +/// Fixed on-wire envelope overhead of a shielded state transition: 2,932 bytes. +/// +/// Everything that does not scale with the action count: the transition's +/// non-action fields (anchor, value balance, flags, signatures, asset-lock +/// proof / identity keys where present) plus the proof's fixed portion. +/// Derived from the same measured points as +/// [`SHIELDED_PROOF_WIRE_BYTES_PER_ACTION`] (8,294 − 2 × 2,681 = 2,932, +/// consistent across the 2-, 6- and 7-action measurements of a +/// `ShieldFromAssetLock` with a chain asset-lock proof). This is the BASELINE +/// envelope: transition types whose non-Orchard fields have VARIABLE +/// serialized size — an instant asset-lock proof embedding its funding +/// transaction and `InstantLock` (both carry input vectors; DPP admits +/// asset-lock transactions with up to 100 inputs), or an identity-create key +/// set of up to six keys — must account for those bytes on top of this +/// constant via the `extra_envelope_bytes` argument of +/// [`max_shielded_actions_for_envelope`] / +/// [`estimated_shielded_transition_wire_bytes_with_envelope`], so the +/// pre-proving gate sees the size the byte prefilter will see. DAPI's byte +/// prefilter remains the authoritative gate. +pub const SHIELDED_TRANSITION_WIRE_OVERHEAD_BYTES: u64 = 2_932; + +/// Encoded size of the CHAIN asset-lock proof that is already counted inside +/// [`SHIELDED_TRANSITION_WIRE_OVERHEAD_BYTES`]: 40 bytes. +/// +/// The baseline envelope was measured from complete `ShieldFromAssetLock` +/// transitions that each carried a chain asset-lock proof, so those bytes are +/// part of the 2,932. A `ShieldFromAssetLock` that supplies its own proof +/// REPLACES that field rather than adding to it — passing the full serialized +/// proof as `extra_envelope_bytes` would model `baseline + full proof` and +/// double-count this much. Callers therefore price the DELTA +/// (`serialized proof − this constant`, saturating), which is the only part +/// that actually grows the transition beyond the measured baseline. +/// +/// Subtracting is safe in both directions: a chain proof is at or near this +/// size so the delta floors at 0 and keeps the baseline ceiling, while an +/// instant proof's several-KiB delta still tightens the ceiling. The +/// remaining error is bounded by the few bytes a chain proof's varint fields +/// vary by (`core_chain_locked_height` / `vout` magnitude), which is far below +/// the ~2.7 KiB granularity of one action. +/// +/// Pinned to the calibration proof by +/// `baseline_asset_lock_proof_bytes_matches_the_calibration_proof` (#4312 +/// review finding b6f78dd76eb7). +pub const SHIELDED_BASELINE_ASSET_LOCK_PROOF_BYTES: u64 = 40; + +/// Conservative estimate of a shielded transition's on-wire serialized size +/// for a bundle of `num_actions` Orchard actions with the baseline envelope. +/// +/// `SHIELDED_TRANSITION_WIRE_OVERHEAD_BYTES + num_actions × +/// (SHIELDED_ACTION_WIRE_BYTES + SHIELDED_PROOF_WIRE_BYTES_PER_ACTION)` — +/// the linear model pinned against measured proved transitions (see +/// [`SHIELDED_PROOF_WIRE_BYTES_PER_ACTION`]). Transitions with +/// variable-size non-Orchard fields use +/// [`estimated_shielded_transition_wire_bytes_with_envelope`]. +pub fn estimated_shielded_transition_wire_bytes(num_actions: usize) -> u64 { + estimated_shielded_transition_wire_bytes_with_envelope(num_actions, 0) +} + +/// [`estimated_shielded_transition_wire_bytes`] plus `extra_envelope_bytes` +/// of transition-specific envelope beyond the measured baseline — the +/// serialized size of the transition's variable-length non-Orchard fields +/// (an embedded instant asset-lock proof, an identity key set), which the +/// fixed [`SHIELDED_TRANSITION_WIRE_OVERHEAD_BYTES`] does not cover. +pub fn estimated_shielded_transition_wire_bytes_with_envelope( + num_actions: usize, + extra_envelope_bytes: u64, +) -> u64 { + SHIELDED_TRANSITION_WIRE_OVERHEAD_BYTES + .saturating_add(extra_envelope_bytes) + .saturating_add( + (num_actions as u64) + .saturating_mul(SHIELDED_ACTION_WIRE_BYTES + SHIELDED_PROOF_WIRE_BYTES_PER_ACTION), + ) +} + +/// The EFFECTIVE per-transition Orchard action ceiling under `platform_version`: +/// the largest action count that satisfies BOTH versioned limits. +/// +/// Two independent consensus limits bound a shielded bundle: +/// +/// 1. the structural cap `system_limits.max_shielded_transition_actions` +/// (enforced by every shielded `validate_structure`), and +/// 2. the byte cap `system_limits.max_state_transition_size` (enforced by +/// DAPI's byte prefilter / Tenderdash `mempool.max-tx-bytes` and the +/// Drive-ABCI consensus decoder BEFORE structural validation runs). +/// +/// Because the on-wire size grows ~2,681 B per action on a ~2.9 KiB envelope +/// (see [`estimated_shielded_transition_wire_bytes`]), the byte cap is the +/// binding constraint at current constants: 6 actions serialize to ~19.0 KiB +/// while 7 need ~21.7 KiB against the 20 KiB limit — so the structural cap of +/// 16 is unreachable unless `max_state_transition_size` is raised. Builders +/// MUST gate on this derived ceiling before proving (via +/// `shielded_bundle_action_count`); otherwise a 7..16-action bundle passes the +/// structural check, burns the expensive Halo 2 proof, and is only then +/// rejected by the byte prefilter. +/// +/// This is the ceiling for the BASELINE envelope. Transition types with +/// variable-size non-Orchard fields (instant asset-lock proofs, identity key +/// sets) must use [`max_shielded_actions_for_envelope`] with their measured +/// extra bytes — a large enough envelope tightens the ceiling below 6. +pub fn max_shielded_actions_per_transition( + platform_version: &platform_version::version::PlatformVersion, +) -> usize { + max_shielded_actions_for_envelope(platform_version, 0) +} + +/// [`max_shielded_actions_per_transition`], with the size budget reduced by +/// `extra_envelope_bytes` of transition-specific envelope beyond the measured +/// baseline (see [`SHIELDED_TRANSITION_WIRE_OVERHEAD_BYTES`]). +/// +/// An instant asset-lock proof embeds its funding transaction and +/// `InstantLock` — both carry input vectors, and DPP admits asset-lock +/// transactions with up to 100 inputs — and an identity create carries up to +/// six variable public keys; either can consume the ~1.4 KiB of slack the +/// baseline ceiling leaves under the byte cap, so the pre-proving gate must +/// price them in or a bundle passes the gate, burns the Halo 2 proof, and is +/// only then rejected by DAPI's byte prefilter (#4312 review finding +/// e90e9cf15f52). +pub fn max_shielded_actions_for_envelope( + platform_version: &platform_version::version::PlatformVersion, + extra_envelope_bytes: u64, +) -> usize { + let structural = platform_version + .system_limits + .max_shielded_transition_actions as usize; + let per_action = SHIELDED_ACTION_WIRE_BYTES + SHIELDED_PROOF_WIRE_BYTES_PER_ACTION; + let size_budget = platform_version + .system_limits + .max_state_transition_size + .saturating_sub(SHIELDED_TRANSITION_WIRE_OVERHEAD_BYTES) + .saturating_sub(extra_envelope_bytes); + // per_action is a non-zero constant; the division is total. + let by_size = (size_budget / per_action) as usize; + structural.min(by_size) +} + /// Calibrated effective storage-byte cost of the Core withdrawal document a /// `ShieldedWithdrawal` creates. /// @@ -174,6 +333,112 @@ impl crate::serialization::JsonConvertible for SerializedAction {} #[cfg(all(feature = "value-conversion", feature = "serde-conversion"))] impl crate::serialization::ValueConvertible for SerializedAction {} +#[cfg(test)] +mod wire_cost_tests { + use super::*; + use platform_version::version::PlatformVersion; + + /// Pin the linear wire-cost model to the sizes measured on real proved + /// transitions (recorded in the `seed_pool_batch_fits_max_state_transition_size` + /// signing test: 2 actions → 8,294 B, 6 → 19,018 B, 7 → 21,699 B — the last + /// rejected by tenderdash's `mempool.max-tx-bytes = 20480` as "Tx too + /// large"). If a proof- or action-encoding change moves these numbers, this + /// fails alongside that signing test and the constants must be re-measured. + #[test] + fn shielded_wire_cost_model_matches_measured_transitions() { + assert_eq!(estimated_shielded_transition_wire_bytes(2), 8_294); + assert_eq!(estimated_shielded_transition_wire_bytes(6), 19_018); + assert_eq!(estimated_shielded_transition_wire_bytes(7), 21_699); + } + + /// The effective ceiling must be derived from BOTH versioned limits, and at + /// the current constants (20 KiB size limit, 16-action structural cap) the + /// size limit is the binding one: 6 actions fit, 7 do not. This is the + /// number the `system_limits` doc comments state; a version bump that + /// changes either constant moves this derivation with it. + #[test] + fn effective_action_ceiling_is_size_bound_at_current_limits() { + let platform_version = PlatformVersion::latest(); + let effective = max_shielded_actions_per_transition(platform_version); + let structural = platform_version + .system_limits + .max_shielded_transition_actions as usize; + let max_size = platform_version.system_limits.max_state_transition_size; + + assert_eq!( + effective, 6, + "at a 20 KiB size limit the derived ceiling must be 6 actions" + ); + assert!( + effective <= structural, + "the effective ceiling can never exceed the structural cap" + ); + // The derivation must be exactly "largest n whose estimated size fits". + assert!( + estimated_shielded_transition_wire_bytes(effective) <= max_size, + "the ceiling itself must fit the size limit" + ); + assert!( + effective == structural + || estimated_shielded_transition_wire_bytes(effective + 1) > max_size, + "one more action than the (size-bound) ceiling must NOT fit" + ); + } + + /// Transition-specific envelope bytes must tighten the ceiling at the + /// exact byte boundary (#4312 review finding e90e9cf15f52): extra bytes + /// within the slack the baseline ceiling leaves under the size limit keep + /// the ceiling; one byte past the slack displaces an action; an envelope + /// larger than the whole budget must degrade to a zero ceiling, never + /// panic or wrap. + #[test] + fn envelope_bytes_tighten_the_action_ceiling_at_the_exact_boundary() { + let platform_version = PlatformVersion::latest(); + let baseline = max_shielded_actions_per_transition(platform_version); + let per_action = SHIELDED_ACTION_WIRE_BYTES + SHIELDED_PROOF_WIRE_BYTES_PER_ACTION; + let max_size = platform_version.system_limits.max_state_transition_size; + // Bytes left under the size limit once the baseline envelope and the + // baseline-ceiling actions are paid for. + let slack = + max_size - SHIELDED_TRANSITION_WIRE_OVERHEAD_BYTES - baseline as u64 * per_action; + + assert_eq!( + max_shielded_actions_for_envelope(platform_version, 0), + baseline, + "a zero extra envelope must reproduce the baseline ceiling" + ); + assert_eq!( + max_shielded_actions_for_envelope(platform_version, slack), + baseline, + "an envelope exactly filling the slack must keep the ceiling" + ); + assert_eq!( + max_shielded_actions_for_envelope(platform_version, slack + 1), + baseline - 1, + "one byte past the slack must displace one action" + ); + assert_eq!( + max_shielded_actions_for_envelope(platform_version, u64::MAX), + 0, + "an envelope beyond the whole budget must degrade to zero, not wrap" + ); + + // The estimator and the ceiling must agree: the ceiling is exactly + // the largest action count whose estimated size (with the same + // envelope) fits the limit. + for extra in [0, slack, slack + 1] { + let ceiling = max_shielded_actions_for_envelope(platform_version, extra); + assert!( + estimated_shielded_transition_wire_bytes_with_envelope(ceiling, extra) <= max_size + ); + assert!( + estimated_shielded_transition_wire_bytes_with_envelope(ceiling + 1, extra) + > max_size + ); + } + } +} + #[cfg(all( test, feature = "json-conversion", diff --git a/packages/rs-dpp/src/shielded/wire_cost_measured_tests.rs b/packages/rs-dpp/src/shielded/wire_cost_measured_tests.rs new file mode 100644 index 00000000000..d8ba4b7e618 --- /dev/null +++ b/packages/rs-dpp/src/shielded/wire_cost_measured_tests.rs @@ -0,0 +1,581 @@ +//! MEASURED regression coverage for the shielded wire-cost model. +//! +//! [`super::wire_cost_tests::shielded_wire_cost_model_matches_measured_transitions`] compares the +//! estimator against the same three literals its constants were *derived* from, so it stays green +//! no matter how Orchard proof encoding or platform serialization moves. The one real-serialization +//! test that existed (`seed_pool_batch_fits_max_state_transition_size`) only checked that a +//! six-action transition stays under the limit — it never compared the measured length with the +//! model, and nothing serialized a real SEVEN-action transition to confirm the rejected boundary. +//! +//! These tests close both gaps. For each of the three envelope shapes the pre-proving gate prices, +//! they BUILD and PROVE a real transition, serialize it through the same +//! [`PlatformSerializable`] path DAPI's byte prefilter reads, and check the measured length against +//! the model at BOTH the largest action count the gate accepts and the next one: +//! +//! - **transfer** — the baseline envelope (`extra_envelope_bytes == 0`), shared with unshield and +//! withdrawal. At the ceiling this is exactly the five-recipient multi-output shape this PR +//! introduces (5 recipients + change = 6 actions). +//! - **asset-lock** — `ShieldFromAssetLock`, which prices its asset-lock proof as a delta over +//! [`SHIELDED_BASELINE_ASSET_LOCK_PROOF_BYTES`]. This is the shape the constants were CALIBRATED +//! from, so its measured length must still equal the model exactly. +//! - **identity-key** — `IdentityCreateFromShieldedPool` with the maximal six-key set, whose +//! serialized keys plus [`PER_KEY_SIGNATURE_ALLOWANCE_BYTES`] ride the envelope. +//! +//! Two properties are asserted at every measured point: the model must never UNDER-estimate (or the +//! gate admits a bundle the byte prefilter kills after the proof is paid for), and it must not +//! over-estimate by more than a small documented budget (or encoding shrinkage has silently left +//! the model — and the public recipient ceiling derived from it — needlessly restrictive). +//! +//! Cost: six real Halo 2 proofs plus the one-time proving-key build (~80 s cold in a debug build). +//! They run as ordinary tests rather than `#[ignore]`d ones, matching every other real-proving +//! shielded test in this crate; CI runs the whole shielded suite in one process, so the proving key +//! is built once and shared (see the `Run shielded tests with cargo test (shared process for VK +//! reuse)` step). +#![cfg(all(test, feature = "shielded-client"))] + +use crate::address_funds::PlatformAddress; +use crate::identity::identity_public_key::v0::IdentityPublicKeyV0; +use crate::identity::signer::Signer; +use crate::identity::state_transition::asset_lock_proof::chain::ChainAssetLockProof; +use crate::identity::{IdentityPublicKey, KeyType, Purpose, SecurityLevel}; +use crate::prelude::AssetLockProof; +use crate::serialization::PlatformSerializable; +use crate::shielded::builder::test_helpers::{ + test_orchard_address, test_spendable_note, TestProver, +}; +use crate::shielded::builder::{ + build_identity_create_from_shielded_pool_transition, build_shield_from_asset_lock_transition, + build_shielded_transfer_transition_multi, serialized_envelope_bytes, shielded_bundle_action_count, + ShieldedTransferOutput, SpendableNote, PER_KEY_SIGNATURE_ALLOWANCE_BYTES, +}; +use crate::shielded::{ + estimated_shielded_transition_wire_bytes_with_envelope, max_shielded_actions_for_envelope, + SHIELDED_ACTION_WIRE_BYTES, SHIELDED_BASELINE_ASSET_LOCK_PROOF_BYTES, + SHIELDED_PROOF_WIRE_BYTES_PER_ACTION, +}; +use crate::state_transition::public_key_in_creation::v0::IdentityPublicKeyInCreationV0; +use crate::state_transition::public_key_in_creation::IdentityPublicKeyInCreation; +use crate::state_transition::state_transitions::shielded::identity_create_from_shielded_pool_transition::methods::IdentityCreateFromShieldedPoolTransitionMethodsV0; +use crate::state_transition::state_transitions::shielded::identity_create_from_shielded_pool_transition::IdentityCreateFromShieldedPoolTransition; +use crate::state_transition::shield_from_asset_lock_transition::ShieldFromAssetLockTransition; +use crate::state_transition::shielded_transfer_transition::ShieldedTransferTransition; +use crate::state_transition::StateTransition; +use crate::ProtocolError; +use dashcore::OutPoint; +use grovedb_commitment_tree::{ + Anchor, ExtractedNoteCommitment, FullViewingKey, Hashable, Level, MerkleHashOrchard, MerklePath, + SpendAuthorizingKey, SpendingKey, NOTE_COMMITMENT_TREE_DEPTH, +}; +use platform_value::BinaryData; +use platform_version::version::PlatformVersion; + +/// 1 DASH in credits — the largest member of the versioned exit-denomination set, chosen so the +/// identity-create fee predictor clears its `fee < denomination` gate at six actions and six keys. +const DENOMINATION: u64 = 100_000_000_000; + +/// Value of each note the spend-side shapes are funded with. Large enough that six of them cover +/// the denomination and every carved fee with room to spare. +const NOTE_VALUE: u64 = 500_000_000_000; + +// --------------------------------------------------------------------------- +// Per-shape slack budgets +// +// Each is the gap measured between the model and the real serialized length at the time of +// writing. The model is calibrated on `ShieldFromAssetLock`, so other transition shapes come in +// slightly UNDER it; the budgets pin how far under, tightly enough that real encoding shrinkage +// trips them while staying far below one action's +// `SHIELDED_ACTION_WIRE_BYTES + SHIELDED_PROOF_WIRE_BYTES_PER_ACTION` (2,681 B) — i.e. long before +// the slack could cost a recipient slot. +// --------------------------------------------------------------------------- + +/// `ShieldFromAssetLock` is the CALIBRATION shape: the constants were derived from exactly these +/// serialized lengths, so it must still match the model to the byte. Any drift here means the +/// constants are stale and every other shape's ceiling is wrong with them. +const ASSET_LOCK_SLACK_BUDGET_BYTES: u64 = 0; + +/// A `ShieldedTransfer` envelope is 105 B leaner than the calibrated `ShieldFromAssetLock` one +/// (measured: 18,913 B vs the model's 19,018 B at six actions). The budget leaves headroom for +/// small field-encoding movement. +const TRANSFER_SLACK_BUDGET_BYTES: u64 = 256; + +/// `IdentityCreateFromShieldedPool` with six keys comes in 240 B under the model (measured: +/// 19,613 B vs 19,853 B at six actions). Most of that is deliberate: +/// [`PER_KEY_SIGNATURE_ALLOWANCE_BYTES`] budgets a 96-byte BLS signature per key while these +/// ECDSA keys carry 65, and the gate measures the keys BEFORE they are PoP-signed. The budget +/// admits that intentional conservatism and nothing much more. +const IDENTITY_SLACK_BUDGET_BYTES: u64 = 512; + +// --------------------------------------------------------------------------- +// Fixtures +// --------------------------------------------------------------------------- + +/// Witness every leaf in ONE consistent tree: each path's siblings are the real neighbouring +/// subtree roots (empty-subtree roots past the frontier), so all leaves compute the SAME anchor +/// and the Orchard circuit accepts the spends. Generalises the two-leaf construction in +/// `shielded_transfer`'s tests to the six-spend bundles the ceiling needs. +fn witness_all(leaves: &[ExtractedNoteCommitment]) -> (Vec, Anchor) { + let mut nodes: Vec = + leaves.iter().map(MerkleHashOrchard::from_cmx).collect(); + let mut positions: Vec = (0..leaves.len()).collect(); + let mut auth: Vec> = vec![Vec::new(); leaves.len()]; + + for depth in 0..NOTE_COMMITMENT_TREE_DEPTH { + let level = Level::from(depth as u8); + let empty = MerkleHashOrchard::empty_root(level); + for (leaf, pos) in positions.iter().enumerate() { + auth[leaf].push(nodes.get(pos ^ 1).copied().unwrap_or(empty)); + } + let mut next = Vec::with_capacity(nodes.len().div_ceil(2)); + for pair in nodes.chunks(2) { + let right = pair.get(1).copied().unwrap_or(empty); + next.push(MerkleHashOrchard::combine(level, &pair[0], &right)); + } + nodes = next; + for pos in positions.iter_mut() { + *pos /= 2; + } + } + + let paths: Vec = auth + .into_iter() + .enumerate() + .map(|(i, path)| { + let fixed: [MerkleHashOrchard; NOTE_COMMITMENT_TREE_DEPTH] = + path.try_into().expect("exactly one sibling per level"); + MerklePath::from_parts(i as u32, fixed) + }) + .collect(); + + let anchor = paths[0].root(leaves[0]); + for (i, path) in paths.iter().enumerate() { + assert_eq!( + path.root(leaves[i]).to_bytes(), + anchor.to_bytes(), + "every witness must compute the same anchor, but leaf {i} disagrees" + ); + } + (paths, anchor) +} + +/// `count` spendable notes of [`NOTE_VALUE`] witnessed in one tree, with their shared anchor. +fn spendable_notes(count: usize) -> (Vec, Anchor) { + let notes: Vec<_> = (0..count) + .map(|_| test_spendable_note(NOTE_VALUE).note) + .collect(); + let cmxs: Vec = notes + .iter() + .map(|note| ExtractedNoteCommitment::from(note.commitment())) + .collect(); + let (paths, anchor) = witness_all(&cmxs); + let spends = notes + .into_iter() + .zip(paths) + .map(|(note, merkle_path)| SpendableNote { note, merkle_path }) + .collect(); + (spends, anchor) +} + +/// `PlatformVersion::latest()` with the transition-size limit lifted. +/// +/// Used ONLY to get a past-the-ceiling bundle through the pre-proving gate so it can actually be +/// proved and serialized — the rejected boundary cannot be measured otherwise. Every assertion +/// still compares against the REAL limit from `PlatformVersion::latest()`, and +/// `*_rejects_the_next_action_count` separately pins that the real gate refuses that count. +fn size_limit_lifted() -> PlatformVersion { + let mut version = PlatformVersion::latest().clone(); + version.system_limits.max_state_transition_size = 1_000_000; + version +} + +fn chain_asset_lock_proof() -> AssetLockProof { + AssetLockProof::Chain(ChainAssetLockProof { + core_chain_locked_height: 100, + out_point: OutPoint::from([11u8; 36]), + }) +} + +/// The envelope delta `ShieldFromAssetLock` hands the gate for a chain proof. +fn asset_lock_envelope_bytes() -> u64 { + serialized_envelope_bytes(&chain_asset_lock_proof(), "the asset-lock proof") + .expect("measurable asset-lock proof") + .saturating_sub(SHIELDED_BASELINE_ASSET_LOCK_PROOF_BYTES) +} + +/// The envelope `IdentityCreateFromShieldedPool` hands the gate for a maximal six-key set. +fn identity_envelope_bytes() -> u64 { + let keys: Vec = (0..6u32).map(|id| key_pair(id).1).collect(); + serialized_envelope_bytes(&keys, "the identity key set") + .expect("measurable key set") + .saturating_add(keys.len() as u64 * PER_KEY_SIGNATURE_ALLOWANCE_BYTES) +} + +/// Fixed 65-byte proof-of-possession stub; the builder fills and never verifies these. +#[derive(Debug)] +struct DummySigner; + +#[async_trait::async_trait] +impl Signer for DummySigner { + async fn sign( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + Ok(BinaryData::new(vec![0u8; 65])) + } + + async fn sign_create_witness( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + Err(ProtocolError::ShieldedBuildError( + "identity PoP signer never creates address witnesses".to_string(), + )) + } + + fn can_sign_with(&self, _key: &IdentityPublicKey) -> bool { + true + } +} + +/// One AUTHENTICATION/MASTER ECDSA key in both forms the identity-create builder takes. +fn key_pair(id: u32) -> (IdentityPublicKey, IdentityPublicKeyInCreation) { + let public = IdentityPublicKey::V0(IdentityPublicKeyV0 { + id, + purpose: Purpose::AUTHENTICATION, + security_level: SecurityLevel::MASTER, + contract_bounds: None, + key_type: KeyType::ECDSA_SECP256K1, + read_only: false, + data: BinaryData::new(vec![0xAB; 33]), + disabled_at: None, + }); + let in_creation = IdentityPublicKeyInCreation::V0(IdentityPublicKeyInCreationV0 { + id, + key_type: KeyType::ECDSA_SECP256K1, + purpose: Purpose::AUTHENTICATION, + security_level: SecurityLevel::MASTER, + contract_bounds: None, + read_only: false, + data: BinaryData::new(vec![0xAB; 33]), + signature: BinaryData::new(vec![]), + }); + (public, in_creation) +} + +// --------------------------------------------------------------------------- +// Builders: one real, proved, serialized transition of each shape +// --------------------------------------------------------------------------- + +/// A `ShieldedTransfer` publishing exactly `num_actions` on-wire actions: one spend plus +/// `num_actions - 1` recipient outputs and the change note (`max(spends, recipients + 1)`). +fn transfer_wire_bytes(num_actions: usize, version: &PlatformVersion) -> usize { + let spending_key = SpendingKey::from_bytes([42u8; 32]).expect("valid spending key"); + let fvk = FullViewingKey::from(&spending_key); + let ask = SpendAuthorizingKey::from(&spending_key); + let recipient = test_orchard_address(); + let change_address = test_orchard_address(); + let (spends, anchor) = spendable_notes(1); + + let outputs: Vec = (0..num_actions - 1) + .map(|_| ShieldedTransferOutput { + recipient, + amount: 1_000_000_000, + memo: [0u8; 36], + }) + .collect(); + + let (state_transition, _fee) = build_shielded_transfer_transition_multi( + spends, + &outputs, + &change_address, + &fvk, + &ask, + anchor, + &TestProver, + version, + ) + .expect("a multi-output transfer at this action count must build"); + + let bytes = state_transition + .serialize_to_bytes() + .expect("serialize the proved transfer"); + assert_action_count(&state_transition, num_actions); + bytes.len() +} + +/// A `ShieldFromAssetLock` publishing exactly `num_actions` on-wire actions: the real recipient +/// output plus `num_actions - 1` zero-value fillers (`max(1 + dummies, 2)`). +fn asset_lock_wire_bytes(num_actions: usize, version: &PlatformVersion) -> usize { + let state_transition = build_shield_from_asset_lock_transition( + &test_orchard_address(), + 50_000u64, + chain_asset_lock_proof(), + &[7u8; 32], + &TestProver, + [0u8; 36], + None, // sender_ovk + None, // surplus_output + num_actions - 1, + version, + ) + .expect("a shield-from-asset-lock at this action count must build"); + + let bytes = state_transition + .serialize_to_bytes() + .expect("serialize the proved shield-from-asset-lock"); + assert_action_count(&state_transition, num_actions); + bytes.len() +} + +/// An `IdentityCreateFromShieldedPool` with the maximal six-key set publishing exactly +/// `num_actions` on-wire actions (one output — the change note — so the spend count drives it). +/// +/// The builder hands back the PoP-signed keys and the proved bundle; re-assembling them through +/// `try_from_bundle` is exactly what the SDK broadcast helper does, so this measures the bytes +/// that really go on the wire. +async fn identity_create_wire_bytes(num_actions: usize, version: &PlatformVersion) -> usize { + let spending_key = SpendingKey::from_bytes([42u8; 32]).expect("valid spending key"); + let fvk = FullViewingKey::from(&spending_key); + let ask = SpendAuthorizingKey::from(&spending_key); + let change_address = test_orchard_address(); + let (spends, anchor) = spendable_notes(num_actions); + let failure_address = PlatformAddress::P2pkh([0u8; 20]); + + let build = build_identity_create_from_shielded_pool_transition( + (0..6u32).map(key_pair).collect(), + DENOMINATION, + failure_address, + spends, + &change_address, + &fvk, + &ask, + anchor, + &TestProver, + &DummySigner, + [0u8; 36], + version, + ) + .await + .expect("an identity create at this action count must build"); + + let state_transition = IdentityCreateFromShieldedPoolTransition::try_from_bundle( + build.public_keys, + DENOMINATION, + failure_address, + build.bundle.actions, + build.bundle.anchor, + build.bundle.proof, + build.bundle.binding_signature, + version, + ) + .expect("re-assemble the proved identity-create transition"); + + let bytes = state_transition + .serialize_to_bytes() + .expect("serialize the proved identity create"); + assert_action_count(&state_transition, num_actions); + bytes.len() +} + +/// Pin that the bundle really published the action count the measurement is attributed to — a +/// silently padded or truncated bundle would compare the model at the wrong point. +fn assert_action_count(state_transition: &StateTransition, expected: usize) { + let actual = match state_transition { + StateTransition::ShieldedTransfer(ShieldedTransferTransition::V0(v0)) => v0.actions.len(), + StateTransition::ShieldFromAssetLock(ShieldFromAssetLockTransition::V0(v0)) => { + v0.actions.len() + } + StateTransition::IdentityCreateFromShieldedPool( + IdentityCreateFromShieldedPoolTransition::V0(v0), + ) => v0.actions.len(), + other => panic!("unexpected transition variant under measurement: {other:?}"), + }; + assert_eq!( + actual, expected, + "the built bundle must publish exactly the action count under test" + ); +} + +// --------------------------------------------------------------------------- +// Shared assertions +// --------------------------------------------------------------------------- + +/// The model must BRACKET reality at a measured point: never under-estimate (safety), and never +/// over-estimate by more than this shape's documented budget (tightness). +fn assert_model_brackets_reality( + shape: &str, + num_actions: usize, + extra_envelope_bytes: u64, + actual: usize, + slack_budget: u64, +) { + let actual = actual as u64; + let estimated = + estimated_shielded_transition_wire_bytes_with_envelope(num_actions, extra_envelope_bytes); + + assert!( + actual <= estimated, + "{shape} at {num_actions} actions serializes to {actual} B, ABOVE the model's \ + {estimated} B. The pre-proving gate would admit a bundle DAPI's byte prefilter then \ + rejects — after the ~30 s Halo 2 proof has been paid for. Re-measure \ + SHIELDED_TRANSITION_WIRE_OVERHEAD_BYTES / SHIELDED_PROOF_WIRE_BYTES_PER_ACTION." + ); + + let slack = estimated - actual; + assert!( + slack <= slack_budget, + "{shape} at {num_actions} actions serializes to {actual} B — {slack} B under the model's \ + {estimated} B, past the {slack_budget} B budget for this shape. The encoding shrank (or \ + an envelope allowance went stale), so the model and every ceiling derived from it — \ + including the public multi-recipient ceiling — are needlessly restrictive. Re-measure \ + the constants." + ); +} + +/// The measured boundary must be a REAL boundary under the shipping limit: the accepted count +/// fits and the next one does not. +fn assert_real_size_boundary(shape: &str, ceiling: usize, at_ceiling: usize, past_ceiling: usize) { + let max_size = PlatformVersion::latest() + .system_limits + .max_state_transition_size; + + assert!( + at_ceiling as u64 <= max_size, + "{shape}: the gate's ceiling of {ceiling} actions serializes to {at_ceiling} B, over the \ + {max_size} B limit — the gate accepts a count that cannot be broadcast" + ); + assert!( + past_ceiling as u64 > max_size, + "{shape}: {} actions serializes to {past_ceiling} B, which FITS the {max_size} B limit — \ + the gate is rejecting a count that would have been accepted on chain, costing a \ + recipient slot", + ceiling + 1 + ); +} + +// --------------------------------------------------------------------------- +// Transfer (baseline envelope) — also the five-recipient public ceiling +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn shielded_transfer_measured_wire_bytes_match_the_model_at_the_action_ceiling() { + let version = PlatformVersion::latest(); + let ceiling = max_shielded_actions_for_envelope(version, 0); + assert_eq!( + ceiling, 6, + "the baseline ceiling is the five-recipients-plus-change shape this PR exposes" + ); + + let lifted = size_limit_lifted(); + let at_ceiling = transfer_wire_bytes(ceiling, version); + let past_ceiling = transfer_wire_bytes(ceiling + 1, &lifted); + + assert_model_brackets_reality( + "ShieldedTransfer", + ceiling, + 0, + at_ceiling, + TRANSFER_SLACK_BUDGET_BYTES, + ); + assert_model_brackets_reality( + "ShieldedTransfer", + ceiling + 1, + 0, + past_ceiling, + TRANSFER_SLACK_BUDGET_BYTES, + ); + assert_real_size_boundary("ShieldedTransfer", ceiling, at_ceiling, past_ceiling); + + // The measured per-action growth must be the constant the model prices actions at, or the + // model is only accidentally right at these two points. + assert_eq!( + (past_ceiling - at_ceiling) as u64, + SHIELDED_ACTION_WIRE_BYTES + SHIELDED_PROOF_WIRE_BYTES_PER_ACTION, + "one more action must cost exactly the modelled per-action bytes" + ); + + // And the shipping gate must refuse the count that genuinely does not fit. + shielded_bundle_action_count(1, ceiling + 1, 0, version) + .expect_err("the real gate must reject the first action count that does not fit"); +} + +// --------------------------------------------------------------------------- +// Asset-lock envelope — the CALIBRATION shape +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn shield_from_asset_lock_measured_wire_bytes_match_the_model_exactly() { + let version = PlatformVersion::latest(); + let envelope = asset_lock_envelope_bytes(); + let ceiling = max_shielded_actions_for_envelope(version, envelope); + + let lifted = size_limit_lifted(); + let at_ceiling = asset_lock_wire_bytes(ceiling, version); + let past_ceiling = asset_lock_wire_bytes(ceiling + 1, &lifted); + + // Budget 0: the constants were derived from exactly these lengths. This is the assertion + // `shielded_wire_cost_model_matches_measured_transitions` could never make, because it + // compares the estimator with the literals instead of with a real transition. + assert_model_brackets_reality( + "ShieldFromAssetLock", + ceiling, + envelope, + at_ceiling, + ASSET_LOCK_SLACK_BUDGET_BYTES, + ); + assert_model_brackets_reality( + "ShieldFromAssetLock", + ceiling + 1, + envelope, + past_ceiling, + ASSET_LOCK_SLACK_BUDGET_BYTES, + ); + assert_real_size_boundary("ShieldFromAssetLock", ceiling, at_ceiling, past_ceiling); + + shielded_bundle_action_count(0, ceiling + 1, envelope, version) + .expect_err("the real gate must reject the first action count that does not fit"); +} + +// --------------------------------------------------------------------------- +// Identity-key envelope +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn shielded_identity_create_measured_wire_bytes_match_the_model_with_a_maximal_key_set() { + let version = PlatformVersion::latest(); + let envelope = identity_envelope_bytes(); + let ceiling = max_shielded_actions_for_envelope(version, envelope); + assert!( + ceiling >= 2, + "a maximal key set must not brick identity creation" + ); + + let lifted = size_limit_lifted(); + let at_ceiling = identity_create_wire_bytes(ceiling, version).await; + let past_ceiling = identity_create_wire_bytes(ceiling + 1, &lifted).await; + + assert_model_brackets_reality( + "IdentityCreateFromShieldedPool", + ceiling, + envelope, + at_ceiling, + IDENTITY_SLACK_BUDGET_BYTES, + ); + assert_model_brackets_reality( + "IdentityCreateFromShieldedPool", + ceiling + 1, + envelope, + past_ceiling, + IDENTITY_SLACK_BUDGET_BYTES, + ); + assert_real_size_boundary( + "IdentityCreateFromShieldedPool", + ceiling, + at_ceiling, + past_ceiling, + ); + + shielded_bundle_action_count(ceiling + 1, 1, envelope, version) + .expect_err("the real gate must reject the first action count that does not fit"); +} diff --git a/packages/rs-dpp/src/state_transition/state_transitions/shielded/identity_create_from_shielded_pool_transition/mod.rs b/packages/rs-dpp/src/state_transition/state_transitions/shielded/identity_create_from_shielded_pool_transition/mod.rs index 0e11c927edb..079a66c698d 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/shielded/identity_create_from_shielded_pool_transition/mod.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/shielded/identity_create_from_shielded_pool_transition/mod.rs @@ -78,6 +78,35 @@ pub fn identity_id_from_nullifiers(nullifiers: &[[u8; 32]]) -> Identifier { Identifier::new(hash_double(buf)) } +/// Whether the identity id an `IdentityCreateFromShieldedPool` will publish can be reproduced +/// OFFLINE, before (or after) the bundle is built, from the spent-note set alone. +/// +/// The id is derived over the bundle's PUBLISHED nullifiers — every action's nullifier, padding +/// included. Orchard's `BundleType::DEFAULT` pads any bundle to `MIN_ACTIONS = 2`, and a padding +/// action carries a **randomly generated** dummy nullifier. So: +/// +/// - `num_real_spends >= 2` — no padding is added, every published nullifier is the deterministic +/// nullifier of a real note, and the id is a pure function of the spent notes. It can be +/// predicted before building and RE-derived identically on a later retry. +/// - `num_real_spends < 2` — the bundle is padded and at least one published nullifier is fresh +/// randomness. The id is unpredictable beforehand and, critically, **not reproducible**: a retry +/// builds a different dummy and therefore a different identity id. +/// +/// Any flow that must recognise "this identity is the one my earlier attempt created" — idempotent +/// claim recovery being the motivating case — MUST gate on this. When it returns `false` the +/// caller cannot derive an expected id and has to treat recovery as unreliable rather than +/// computing an id that will not match. Guarding on the *note count* is the cheapest correct check: +/// it needs no chain lookup and is decided before any proving work. +/// +/// The corollary drives note layout: funding an address with two sub-target notes (instead of one +/// note covering the whole target) forces a later spend of that address to select BOTH — greedy +/// largest-first selection cannot stop after one note that does not cover the target — which keeps +/// the padding action, and its random nullifier, out of the bundle entirely. +pub fn shielded_identity_id_is_reproducible(num_real_spends: usize) -> bool { + // Mirrors Orchard's `MIN_ACTIONS = 2`: at or above it, no padding action is appended. + num_real_spends >= 2 +} + /// Convenience wrapper around [`identity_id_from_nullifiers`] that extracts the nullifiers from a /// slice of serialized Orchard actions. Shared by the SDK builder and the consensus re-derivation /// check so both compute the id identically. diff --git a/packages/rs-platform-wallet-ffi/src/shielded_send.rs b/packages/rs-platform-wallet-ffi/src/shielded_send.rs index 0926f6eff14..6be74c15dc5 100644 --- a/packages/rs-platform-wallet-ffi/src/shielded_send.rs +++ b/packages/rs-platform-wallet-ffi/src/shielded_send.rs @@ -282,6 +282,36 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_transfer( recipient_raw_43: *const u8, amount: u64, memo_text: *const c_char, +) -> PlatformWalletFFIResult { + // Guarded: a panic (most concretely `block_on_worker`'s `.expect` on a panicking proving + // task) must NOT reach this `extern "C"` frame, where it would abort the process instead of + // surfacing to the host as a typed error. + catch_spend_panic("shielded transfer", || { + shielded_transfer_inner( + handle, + wallet_id_bytes, + mnemonic_resolver_handle, + account, + recipient_raw_43, + amount, + memo_text, + ) + }) +} + +/// Body of [`platform_wallet_manager_shielded_transfer`], as an ordinary Rust function so a +/// panic unwinds into [`catch_spend_panic`] instead of across the C ABI. +/// +/// # Safety +/// Identical contract to the export that calls it. +unsafe fn shielded_transfer_inner( + handle: Handle, + wallet_id_bytes: *const u8, + mnemonic_resolver_handle: *mut MnemonicResolverHandle, + account: u32, + recipient_raw_43: *const u8, + amount: u64, + memo_text: *const c_char, ) -> PlatformWalletFFIResult { check_ptr!(wallet_id_bytes); check_ptr!(mnemonic_resolver_handle); @@ -351,6 +381,290 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_transfer( map_spend_result(result, "shielded transfer") } +/// Recipient ceiling of a multi-output shielded transfer (Type 16). +/// +/// Derived from the EFFECTIVE per-transition Orchard action ceiling — the largest bundle that +/// satisfies BOTH versioned consensus limits: the structural +/// `max_shielded_transition_actions` cap AND the 20 KiB `max_state_transition_size`, whose byte +/// prefilter is the binding one at current constants (6 actions ≈ 19.0 KiB on the wire, 7 ≈ +/// 21.7 KiB — see `dpp::shielded::max_shielded_actions_per_transition`). A multi-output +/// transfer always appends a change output, so recipients = effective actions − 1. Admitting +/// more would only burn ~30 s of Halo 2 proving per bundle before DAPI's byte prefilter (or the +/// dpp builder's own pre-proving gate) rejects the transition. +/// +/// Public so language bridges (the JNI adapter, Swift) can enforce the SAME bound before they +/// allocate their own caller-sized buffers, rather than duplicating the literal. Pinned against +/// the dpp derivation by the `max_recipients_matches_the_effective_action_ceiling` test below — +/// raise it only in lockstep with the versioned limits, together with the Kotlin mirror +/// (`MAX_SHIELDED_TRANSFER_RECIPIENTS` in `kotlin-sdk`'s `PlatformWalletManager.kt`). +pub const MAX_SHIELDED_TRANSFER_RECIPIENTS: usize = 5; + +/// Render a caught panic payload as a human-readable string. +fn panic_payload_message(payload: &(dyn std::any::Any + Send)) -> String { + if let Some(s) = payload.downcast_ref::<&'static str>() { + (*s).to_string() + } else if let Some(s) = payload.downcast_ref::() { + s.clone() + } else { + "".to_string() + } +} + +/// Run an FFI export body under [`std::panic::catch_unwind`], converting a panic into the +/// operation's contract-appropriate result `code` (with `guidance` appended to the message) +/// instead of letting it reach the `extern "C"` frame. +/// +/// A Rust panic cannot unwind through a C ABI boundary: it aborts the process. The JNI layer +/// wraps its calls in `support::guard` (which catches panics and raises a Java exception), but +/// that guard sits on the FAR side of this `extern "C"` export, so it never sees the unwind — the +/// process is already gone. `block_on_worker` makes this reachable rather than theoretical: it +/// `.expect`s on the tokio `JoinError`, so any panic inside the proving future (Halo 2 synthesis, +/// note bookkeeping, the SDK) re-panics right here inside the export. +/// +/// The `code` must be chosen per operation to preserve that operation's result contract — a +/// panic can strike after side effects (note reservation, a broadcast) have happened, so the +/// outcome is genuinely ambiguous and the code must never promise a definitive failure. See +/// [`catch_spend_panic`] and the per-export call sites. +/// +/// NOTE: this guard is only effective where panics unwind. The Android (`*-android`) and +/// host/test profiles build with `panic = "unwind"`, so it works there; the iOS profiles +/// (`dev-ios` / `release-ios`) build with `panic = "abort"` as part of their staticlib size +/// tuning (see the workspace `Cargo.toml` profile comments), so on iOS a panic still aborts the +/// process before this guard can see it. +fn catch_panic_to_code( + operation: &str, + code: PlatformWalletFFIResultCode, + guidance: &str, + body: impl FnOnce() -> PlatformWalletFFIResult, +) -> PlatformWalletFFIResult { + match std::panic::catch_unwind(std::panic::AssertUnwindSafe(body)) { + Ok(result) => result, + Err(payload) => PlatformWalletFFIResult::err( + code, + format!( + "{operation} panicked: {}. {guidance}", + panic_payload_message(payload.as_ref()) + ), + ), + } +} + +/// Post-panic guidance for the note-spending operations (transfer / multi-transfer / unshield / +/// withdraw / shield). Paired with `ErrorShieldedSpendUnconfirmed` in [`catch_spend_panic`]. +const SPEND_PANIC_GUIDANCE: &str = "The spend may or may not have been broadcast — do NOT \ + retry; the next shielded sync reconciles the outcome."; + +/// Post-panic guidance for Type 20 identity creation. Paired with the generic +/// [`PlatformWalletFFIResultCode::ErrorUnknown`] — see the export's guard call site for why no +/// richer code fits. +const IDENTITY_CREATE_PANIC_GUIDANCE: &str = "The transition may or may not have been broadcast \ + and the new identity may already exist on chain — do NOT re-submit and do NOT release the \ + identity slot; out_identity_id was NOT written. The next shielded sync reconciles the \ + outcome."; + +/// Post-panic guidance for the asset-lock funding operations (fresh fund / resume). Paired with +/// `ErrorWalletOperation`, the single error code those exports already surface. +const ASSET_LOCK_FUNDING_PANIC_GUIDANCE: &str = "The funding may have partially executed — an \ + asset-lock transaction may already be broadcast and tracked. Do NOT blindly re-run (a fresh \ + call would build and fund a NEW asset lock); check the wallet's tracked asset locks after \ + the next sync and use the resume entry point to complete an in-flight funding."; + +/// Post-panic guidance for the devnet/testnet pool-seeding loop. Paired with +/// `ErrorWalletOperation`, the single error code that export already surfaces. +const SEED_POOL_PANIC_GUIDANCE: &str = "Batches that completed before the panic are on chain; \ + re-running after the next sync resumes seeding toward the target."; + +/// [`catch_panic_to_code`] specialized for the note-spending exports. +/// +/// The panic is mapped to [`PlatformWalletFFIResultCode::ErrorShieldedSpendUnconfirmed`], NOT to +/// a definitive failure code: a panic can strike after the notes were reserved and even after the +/// transition was broadcast, so the outcome is genuinely ambiguous. That code's contract is +/// exactly the conservative one this needs — the host must not auto-retry, the reservation stays +/// in place, and the next nullifier sync (or an app restart) reconciles whether the spend landed. +fn catch_spend_panic( + operation: &str, + body: impl FnOnce() -> PlatformWalletFFIResult, +) -> PlatformWalletFFIResult { + catch_panic_to_code( + operation, + PlatformWalletFFIResultCode::ErrorShieldedSpendUnconfirmed, + SPEND_PANIC_GUIDANCE, + body, + ) +} + +/// Send a shielded → shielded transfer with SEVERAL outputs in one +/// atomic transition. +/// +/// Multi-output sibling of +/// [`platform_wallet_manager_shielded_transfer`]. `recipients_raw_43` +/// is `num_recipients` raw 43-byte Orchard payment addresses laid out +/// back to back, and `amounts` is the matching array of +/// `num_recipients` credit amounts. Each pair becomes its own note. +/// +/// Repeating the same address is allowed and is the primary use: it +/// funds one address with several independent notes, so a later spend +/// of that address spends several REAL notes rather than one real note +/// plus an Orchard padding dummy (whose nullifier is randomly +/// generated and so cannot be reproduced offline). +/// +/// `memo_text` is attached to EVERY recipient note (same encoding and +/// 32-byte UTF-8 limit as the single-output call). The change note +/// always carries the empty memo. +/// +/// A multi-output transfer always emits a change output, so the spent +/// value must strictly exceed `sum(amounts) + fee`. +/// +/// `mnemonic_resolver_handle` supplies the per-operation Orchard spend +/// authority (see `platform_wallet_manager_shielded_transfer`). +/// +/// # Safety +/// - `wallet_id_bytes` must point to 32 readable bytes. +/// - `mnemonic_resolver_handle` must come from +/// `dash_sdk_mnemonic_resolver_create` and outlive this call; the +/// caller retains ownership. +/// - `recipients_raw_43` must point to `num_recipients * 43` readable +/// bytes and `amounts` to `num_recipients` readable `u64`s. +/// - `memo_text`, when non-null, must be a valid NUL-terminated UTF-8 +/// C string for the duration of the call. +#[no_mangle] +pub unsafe extern "C" fn platform_wallet_manager_shielded_transfer_multi( + handle: Handle, + wallet_id_bytes: *const u8, + mnemonic_resolver_handle: *mut MnemonicResolverHandle, + account: u32, + recipients_raw_43: *const u8, + amounts: *const u64, + num_recipients: usize, + memo_text: *const c_char, +) -> PlatformWalletFFIResult { + // The whole body runs under `catch_unwind`: a panic (most concretely `block_on_worker`'s + // `.expect` on a panicking proving task) must NOT reach this `extern "C"` frame, where it + // would abort the process instead of surfacing to the host as a typed error. + catch_spend_panic("shielded multi-output transfer", || { + shielded_transfer_multi_inner( + handle, + wallet_id_bytes, + mnemonic_resolver_handle, + account, + recipients_raw_43, + amounts, + num_recipients, + memo_text, + ) + }) +} + +/// Body of [`platform_wallet_manager_shielded_transfer_multi`], as an ordinary Rust function so a +/// panic unwinds into [`catch_spend_panic`] instead of across the C ABI. +/// +/// # Safety +/// Identical contract to the export that calls it. +#[allow(clippy::too_many_arguments)] +unsafe fn shielded_transfer_multi_inner( + handle: Handle, + wallet_id_bytes: *const u8, + mnemonic_resolver_handle: *mut MnemonicResolverHandle, + account: u32, + recipients_raw_43: *const u8, + amounts: *const u64, + num_recipients: usize, + memo_text: *const c_char, +) -> PlatformWalletFFIResult { + check_ptr!(wallet_id_bytes); + check_ptr!(mnemonic_resolver_handle); + check_ptr!(recipients_raw_43); + check_ptr!(amounts); + + if num_recipients == 0 { + return PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorInvalidParameter, + "num_recipients must be at least 1".to_string(), + ); + } + if num_recipients > MAX_SHIELDED_TRANSFER_RECIPIENTS { + return PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorInvalidParameter, + format!( + "num_recipients {num_recipients} exceeds the maximum of \ + {MAX_SHIELDED_TRANSFER_RECIPIENTS}" + ), + ); + } + + let mut wallet_id = [0u8; 32]; + std::ptr::copy_nonoverlapping(wallet_id_bytes, wallet_id.as_mut_ptr(), 32); + + let amount_slice = std::slice::from_raw_parts(amounts, num_recipients); + let mut outputs: Vec<([u8; 43], u64)> = Vec::with_capacity(num_recipients); + for (index, &amount) in amount_slice.iter().enumerate() { + if amount == 0 { + return PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorInvalidParameter, + format!("amount at index {index} must be positive"), + ); + } + let mut recipient = [0u8; 43]; + std::ptr::copy_nonoverlapping( + recipients_raw_43.add(index * 43), + recipient.as_mut_ptr(), + 43, + ); + outputs.push((recipient, amount)); + } + + // Decode the optional memo before touching wallet state so a malformed memo fails fast. + let memo_str = if memo_text.is_null() { + None + } else { + match CStr::from_ptr(memo_text).to_str() { + Ok(s) => Some(s), + Err(e) => { + return PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorUtf8Conversion, + format!("memo_text is not valid UTF-8: {e}"), + ); + } + } + }; + let memo = match encode_memo_text(memo_str) { + Ok(m) => m, + Err(result) => return result, + }; + + let (wallet, coordinator) = match resolve_wallet_and_coordinator(handle, &wallet_id) { + Ok(p) => p, + Err(result) => return result, + }; + + let seed = match crate::identity_keys_from_mnemonic::resolve_seed_from_resolver( + mnemonic_resolver_handle, + &wallet_id, + ) { + Ok(seed) => seed, + Err(result) => return result, + }; + + // Prove on a worker thread with an 8 MB stack (see + // `platform_wallet_manager_shielded_transfer`). + let result = block_on_worker(async move { + let prover = CachedOrchardProver::new(); + let r = wallet + .shielded_transfer_multi_to( + &coordinator, + seed.as_ref(), + account, + &outputs, + memo, + &prover, + ) + .await; + poke_sync_on_unconfirmed(&r, handle); + r + }); + map_spend_result(result, "shielded multi-output transfer") +} + /// Unshield: spend shielded notes and send `amount` credits to a /// platform address. /// @@ -380,6 +694,32 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_unshield( account: u32, to_platform_addr_cstr: *const c_char, amount: u64, +) -> PlatformWalletFFIResult { + // Guarded: a panic must NOT reach this `extern "C"` frame (see `catch_panic_to_code`). + catch_spend_panic("shielded unshield", || { + shielded_unshield_inner( + handle, + wallet_id_bytes, + mnemonic_resolver_handle, + account, + to_platform_addr_cstr, + amount, + ) + }) +} + +/// Body of [`platform_wallet_manager_shielded_unshield`], as an ordinary Rust function so a +/// panic unwinds into [`catch_spend_panic`] instead of across the C ABI. +/// +/// # Safety +/// Identical contract to the export that calls it. +unsafe fn shielded_unshield_inner( + handle: Handle, + wallet_id_bytes: *const u8, + mnemonic_resolver_handle: *mut MnemonicResolverHandle, + account: u32, + to_platform_addr_cstr: *const c_char, + amount: u64, ) -> PlatformWalletFFIResult { check_ptr!(wallet_id_bytes); check_ptr!(mnemonic_resolver_handle); @@ -454,6 +794,34 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_withdraw( to_core_address_cstr: *const c_char, amount: u64, core_fee_per_byte: u32, +) -> PlatformWalletFFIResult { + // Guarded: a panic must NOT reach this `extern "C"` frame (see `catch_panic_to_code`). + catch_spend_panic("shielded withdraw", || { + shielded_withdraw_inner( + handle, + wallet_id_bytes, + mnemonic_resolver_handle, + account, + to_core_address_cstr, + amount, + core_fee_per_byte, + ) + }) +} + +/// Body of [`platform_wallet_manager_shielded_withdraw`], as an ordinary Rust function so a +/// panic unwinds into [`catch_spend_panic`] instead of across the C ABI. +/// +/// # Safety +/// Identical contract to the export that calls it. +unsafe fn shielded_withdraw_inner( + handle: Handle, + wallet_id_bytes: *const u8, + mnemonic_resolver_handle: *mut MnemonicResolverHandle, + account: u32, + to_core_address_cstr: *const c_char, + amount: u64, + core_fee_per_byte: u32, ) -> PlatformWalletFFIResult { check_ptr!(wallet_id_bytes); check_ptr!(mnemonic_resolver_handle); @@ -701,6 +1069,58 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_identity_create_from_p send_to_address_on_creation_failure_bytes: *const u8, signer_identity_handle: *mut SignerHandle, out_identity_id: *mut [u8; 32], +) -> PlatformWalletFFIResult { + // Guarded: a panic must NOT reach this `extern "C"` frame (see `catch_panic_to_code`). + // + // The panic code is the generic `ErrorUnknown`, deliberately NOT one of this export's + // richer codes: `ErrorShieldedBroadcastUnconfirmed`'s ABI contract says `out_identity_id` + // IS written on that code, and a panic destroyed the result so there is no id to write; + // `ErrorShieldedSpendUnconfirmed` is documented as scoped to the unshield / transfer / + // withdrawal spends; and every other code promises a definitive outcome a panic cannot + // promise. No dedicated panic code exists in the (registry-tracked) enum, and allocating + // one here would risk the silent cross-branch numeric collisions the codes 28-30 comment + // warns about — so the generic internal code carries it, with the do-not-resubmit guidance + // in the message. + catch_panic_to_code( + "shielded identity-create-from-pool", + PlatformWalletFFIResultCode::ErrorUnknown, + IDENTITY_CREATE_PANIC_GUIDANCE, + || { + shielded_identity_create_from_pool_inner( + handle, + wallet_id_bytes, + mnemonic_resolver_handle, + account, + identity_index, + identity_pubkeys, + identity_pubkeys_count, + denomination, + send_to_address_on_creation_failure_bytes, + signer_identity_handle, + out_identity_id, + ) + }, + ) +} + +/// Body of [`platform_wallet_manager_shielded_identity_create_from_pool`], as an ordinary Rust +/// function so a panic unwinds into [`catch_panic_to_code`] instead of across the C ABI. +/// +/// # Safety +/// Identical contract to the export that calls it. +#[allow(clippy::too_many_arguments)] +unsafe fn shielded_identity_create_from_pool_inner( + handle: Handle, + wallet_id_bytes: *const u8, + mnemonic_resolver_handle: *mut MnemonicResolverHandle, + account: u32, + identity_index: u32, + identity_pubkeys: *const IdentityPubkeyFFI, + identity_pubkeys_count: usize, + denomination: u64, + send_to_address_on_creation_failure_bytes: *const u8, + signer_identity_handle: *mut SignerHandle, + out_identity_id: *mut [u8; 32], ) -> PlatformWalletFFIResult { check_ptr!(wallet_id_bytes); check_ptr!(mnemonic_resolver_handle); @@ -926,6 +1346,36 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_shield( payment_account: u32, amount: u64, signer_address_handle: *const SignerHandle, +) -> PlatformWalletFFIResult { + // Guarded: a panic must NOT reach this `extern "C"` frame (see `catch_panic_to_code`). A + // shield reserves no notes, but the transition may already have been broadcast when the + // panic struck, so the same ambiguous spend-unconfirmed contract applies (matching + // `map_spend_result`'s mapping for this operation); a later manual retry self-heals through + // the address-nonce check. + catch_spend_panic("shielded shield", || { + shielded_shield_inner( + handle, + wallet_id_bytes, + shielded_account, + payment_account, + amount, + signer_address_handle, + ) + }) +} + +/// Body of [`platform_wallet_manager_shielded_shield`], as an ordinary Rust function so a panic +/// unwinds into [`catch_spend_panic`] instead of across the C ABI. +/// +/// # Safety +/// Identical contract to the export that calls it. +unsafe fn shielded_shield_inner( + handle: Handle, + wallet_id_bytes: *const u8, + shielded_account: u32, + payment_account: u32, + amount: u64, + signer_address_handle: *const SignerHandle, ) -> PlatformWalletFFIResult { check_ptr!(wallet_id_bytes); check_ptr!(signer_address_handle); @@ -1033,6 +1483,44 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_fund_from_asset_lock( surplus_output_ptr: *const u8, surplus_output_len: usize, core_signer_handle: *mut MnemonicResolverHandle, +) -> PlatformWalletFFIResult { + // Guarded: a panic must NOT reach this `extern "C"` frame (see `catch_panic_to_code`). The + // panic maps to `ErrorWalletOperation` — the single error code this export already surfaces + // — with the tracked-lock / resume guidance in the message. + catch_panic_to_code( + "shielded fund-from-asset-lock", + PlatformWalletFFIResultCode::ErrorWalletOperation, + ASSET_LOCK_FUNDING_PANIC_GUIDANCE, + || { + shielded_fund_from_asset_lock_inner( + handle, + wallet_id_bytes, + account_index, + amount_duffs, + recipient_raw_43, + surplus_output_ptr, + surplus_output_len, + core_signer_handle, + ) + }, + ) +} + +/// Body of [`platform_wallet_manager_shielded_fund_from_asset_lock`], as an ordinary Rust +/// function so a panic unwinds into [`catch_panic_to_code`] instead of across the C ABI. +/// +/// # Safety +/// Identical contract to the export that calls it. +#[allow(clippy::too_many_arguments)] +unsafe fn shielded_fund_from_asset_lock_inner( + handle: Handle, + wallet_id_bytes: *const u8, + account_index: u32, + amount_duffs: u64, + recipient_raw_43: *const u8, + surplus_output_ptr: *const u8, + surplus_output_len: usize, + core_signer_handle: *mut MnemonicResolverHandle, ) -> PlatformWalletFFIResult { check_ptr!(wallet_id_bytes); check_ptr!(recipient_raw_43); @@ -1294,6 +1782,42 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_resume_fund_from_asset surplus_output_ptr: *const u8, surplus_output_len: usize, core_signer_handle: *mut MnemonicResolverHandle, +) -> PlatformWalletFFIResult { + // Guarded: a panic must NOT reach this `extern "C"` frame (see `catch_panic_to_code`). + // Same code + guidance as the fresh-build sibling; a resume of the same outpoint is + // additionally protected by the lock's one-shot consumption on Platform. + catch_panic_to_code( + "shielded resume fund-from-asset-lock", + PlatformWalletFFIResultCode::ErrorWalletOperation, + ASSET_LOCK_FUNDING_PANIC_GUIDANCE, + || { + shielded_resume_fund_from_asset_lock_inner( + handle, + wallet_id_bytes, + out_point, + recipient_raw_43, + surplus_output_ptr, + surplus_output_len, + core_signer_handle, + ) + }, + ) +} + +/// Body of [`platform_wallet_manager_shielded_resume_fund_from_asset_lock`], as an ordinary +/// Rust function so a panic unwinds into [`catch_panic_to_code`] instead of across the C ABI. +/// +/// # Safety +/// Identical contract to the export that calls it. +#[allow(clippy::too_many_arguments)] +unsafe fn shielded_resume_fund_from_asset_lock_inner( + handle: Handle, + wallet_id_bytes: *const u8, + out_point: *const OutPointFFI, + recipient_raw_43: *const u8, + surplus_output_ptr: *const u8, + surplus_output_len: usize, + core_signer_handle: *mut MnemonicResolverHandle, ) -> PlatformWalletFFIResult { check_ptr!(wallet_id_bytes); check_ptr!(out_point); @@ -1434,6 +1958,51 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_seed_pool_notes( ), >, progress_ctx: *mut std::os::raw::c_void, +) -> PlatformWalletFFIResult { + // Guarded: a panic must NOT reach this `extern "C"` frame (see `catch_panic_to_code`). The + // panic maps to `ErrorWalletOperation` — the single error code this export already surfaces. + catch_panic_to_code( + "shielded seed-pool-notes", + PlatformWalletFFIResultCode::ErrorWalletOperation, + SEED_POOL_PANIC_GUIDANCE, + || { + shielded_seed_pool_notes_inner( + handle, + wallet_id_bytes, + account, + target_total_notes, + funding_account_index, + core_signer_handle, + progress_fn, + progress_ctx, + ) + }, + ) +} + +/// Body of [`platform_wallet_manager_shielded_seed_pool_notes`], as an ordinary Rust function +/// so a panic unwinds into [`catch_panic_to_code`] instead of across the C ABI. +/// +/// # Safety +/// Identical contract to the export that calls it. +#[allow(clippy::too_many_arguments)] +unsafe fn shielded_seed_pool_notes_inner( + handle: Handle, + wallet_id_bytes: *const u8, + account: u32, + target_total_notes: u64, + funding_account_index: u32, + core_signer_handle: *mut MnemonicResolverHandle, + progress_fn: Option< + unsafe extern "C" fn( + context: *mut std::os::raw::c_void, + batch_index: u64, + batches_total_estimate: u64, + pool_notes_now: u64, + target: u64, + ), + >, + progress_ctx: *mut std::os::raw::c_void, ) -> PlatformWalletFFIResult { check_ptr!(wallet_id_bytes); check_ptr!(core_signer_handle); @@ -1718,6 +2287,123 @@ mod tests { .into_owned() } + /// A non-panicking body passes its result straight through — the guard must be invisible on + /// the happy path. + #[test] + fn catch_spend_panic_passes_results_through() { + let ok = catch_spend_panic("test", PlatformWalletFFIResult::ok); + assert_eq!(ok.code, PlatformWalletFFIResultCode::Success); + + let err = catch_spend_panic("test", || { + PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorInvalidParameter, + "bad input", + ) + }); + assert_eq!(err.code, PlatformWalletFFIResultCode::ErrorInvalidParameter); + assert_eq!(message_of(&err), "bad input"); + } + + /// A panic inside a shielded-spend export must NOT unwind into the `extern "C"` frame (that + /// aborts the process). It becomes `ErrorShieldedSpendUnconfirmed` — the conservative + /// "may have been broadcast, do NOT retry" contract, because a panic can strike after the + /// notes are reserved and after the transition is submitted. + /// + /// The panic hook is deliberately left alone: it is process-global, `cargo test` runs tests + /// concurrently, and `take_hook` + restore from two tests can interleave so that one restores + /// the other's temporary hook last — suppressing panic diagnostics for the rest of the + /// process, including unrelated concurrent panics. The libtest harness already captures this + /// test's output, so the deliberate panic's backtrace does not reach the console anyway + /// (#4312 review finding 60c26fc233d3). + #[test] + fn catch_spend_panic_maps_a_panic_to_the_unconfirmed_contract() { + let result = catch_spend_panic("shielded multi-output transfer", || { + panic!("tokio worker panicked"); + }); + + assert_eq!( + result.code, + PlatformWalletFFIResultCode::ErrorShieldedSpendUnconfirmed, + "a panic must map to the ambiguous, do-not-retry code" + ); + let message = message_of(&result); + assert!( + message.contains("shielded multi-output transfer panicked") + && message.contains("tokio worker panicked"), + "the panic payload must survive into the FFI message: {message}" + ); + assert!( + message.contains("do NOT retry"), + "the message must carry the do-not-retry guidance: {message}" + ); + } + + /// The public recipient ceiling must equal the EFFECTIVE per-transition action ceiling + /// (derived in dpp from BOTH `max_shielded_transition_actions` and + /// `max_state_transition_size`) minus the unconditional change output. If a versioned limit + /// moves, this fails and the constant — plus its Kotlin mirror in + /// `PlatformWalletManager.kt` — must be raised in lockstep. + #[test] + fn max_recipients_matches_the_effective_action_ceiling() { + let effective = dpp::shielded::max_shielded_actions_per_transition( + dpp::version::PlatformVersion::latest(), + ); + assert_eq!( + MAX_SHIELDED_TRANSFER_RECIPIENTS + 1, + effective, + "recipients + the unconditional change output must equal the effective action \ + ceiling; update MAX_SHIELDED_TRANSFER_RECIPIENTS (and the Kotlin mirror) in \ + lockstep with the versioned limits" + ); + } + + /// The generalized guard must carry the per-operation code and guidance: identity creation + /// maps a panic to the generic `ErrorUnknown` (its richer codes all promise things a panic + /// cannot deliver — see the export's call site), and the asset-lock funding exports map it + /// to their single existing error code, `ErrorWalletOperation`. + /// + /// Like the sibling guard test, this leaves the process-global panic hook untouched — see + /// `catch_spend_panic_maps_a_panic_to_the_unconfirmed_contract`. + #[test] + fn catch_panic_to_code_carries_the_per_operation_contract() { + let identity = catch_panic_to_code( + "shielded identity-create-from-pool", + PlatformWalletFFIResultCode::ErrorUnknown, + IDENTITY_CREATE_PANIC_GUIDANCE, + || panic!("proving task panicked"), + ); + let funding = catch_panic_to_code( + "shielded fund-from-asset-lock", + PlatformWalletFFIResultCode::ErrorWalletOperation, + ASSET_LOCK_FUNDING_PANIC_GUIDANCE, + || panic!("proving task panicked"), + ); + + assert_eq!( + identity.code, + PlatformWalletFFIResultCode::ErrorUnknown, + "identity creation must NOT reuse the spend-scoped unconfirmed code" + ); + let message = message_of(&identity); + assert!( + message.contains("shielded identity-create-from-pool panicked") + && message.contains("do NOT re-submit") + && message.contains("out_identity_id was NOT written"), + "identity-create guidance must survive into the message: {message}" + ); + + assert_eq!( + funding.code, + PlatformWalletFFIResultCode::ErrorWalletOperation, + "asset-lock funding must keep its single existing error code" + ); + let message = message_of(&funding); + assert!( + message.contains("Do NOT blindly re-run") && message.contains("resume"), + "funding guidance must survive into the message: {message}" + ); + } + /// `map_spend_result` pins the retry-relevant code split the three spend /// entry points depend on: /// - `ShieldedSpendUnconfirmed` → `ErrorShieldedSpendUnconfirmed` (host diff --git a/packages/rs-platform-wallet/src/wallet/platform_wallet.rs b/packages/rs-platform-wallet/src/wallet/platform_wallet.rs index b6d29e67c40..a292e818427 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_wallet.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_wallet.rs @@ -1359,6 +1359,58 @@ impl PlatformWallet { .await } + /// Multi-output sibling of [`shielded_transfer_to`](Self::shielded_transfer_to): spend + /// `account`'s notes and create SEVERAL notes in one atomic Type-16 transition. + /// + /// `outputs` pairs each recipient (43 raw Orchard address bytes) with its amount in credits. + /// Repeating the same address is allowed and is the point of this call: it funds one address + /// with several independent notes, so a later spend of that address spends several REAL + /// notes instead of one real note plus an Orchard padding dummy (whose nullifier is random + /// and therefore not reproducible offline). + /// + /// `memo` is attached to every recipient note. `seed` supplies the transient spend authority + /// (see [`shielded_transfer_to`](Self::shielded_transfer_to)). + #[cfg(feature = "shielded")] + #[allow(clippy::too_many_arguments)] + pub async fn shielded_transfer_multi_to( + &self, + coordinator: &Arc, + seed: &[u8], + account: u32, + outputs: &[([u8; 43], u64)], + memo: [u8; 36], + prover: P, + ) -> Result<(), PlatformWalletError> { + let keyset = self.derive_spend_keyset(seed, account).await?; + let parsed: Vec<(grovedb_commitment_tree::PaymentAddress, u64)> = outputs + .iter() + .map(|(raw, amount)| { + Option::::from( + grovedb_commitment_tree::PaymentAddress::from_raw_address_bytes(raw), + ) + .map(|addr| (addr, *amount)) + .ok_or_else(|| { + PlatformWalletError::ShieldedBuildError( + "invalid Orchard payment address bytes".to_string(), + ) + }) + }) + .collect::>()?; + + super::shielded::operations::transfer_multi( + &self.sdk, + coordinator.store(), + Some(&self.persister), + self.wallet_id, + &keyset, + account, + &parsed, + memo, + &prover, + ) + .await + } + /// Unshield from `account`'s notes to a transparent platform /// address (`"dash1…"` / `"tdash1…"`). Parsed via /// `PlatformAddress::from_bech32m_string`; the recipient's HRP is @@ -2179,22 +2231,24 @@ mod shield_input_selection_tests { #[test] fn regression_reports_max_from_usable_suffix_not_total_account_balance() { - // Real account snapshot: the leading address is below the reserve, so - // capacity must come from the usable suffix, not the account total. - assert!( - 297_264_780 <= reserve(), - "regression shape requires the leading address to stay below the reserve; \ - re-seed the balances if the versioned reserve drops under 297_264_780" - ); + // Real account snapshot shape: the leading address must not qualify as + // the fee-paying input 0 (eligibility requires strictly exceeding the + // reserve), so capacity must come from the usable suffix, not the + // account total. Seed it AT the versioned reserve — deriving it keeps + // the shape valid across fee rebalances, where the old 297_264_780 + // literal broke the moment protocol 14 dropped the reserve under it. let candidates = vec![ - (addr(1), 297_264_780), + (addr(1), reserve()), (addr(2), 2_000_000_000), (addr(3), 1_623_849_220), ]; let plan = plan(candidates).unwrap(); let expected_max = 3_623_849_220 - reserve(); - assert_eq!(plan.preflight.account_balance_credits, 3_921_114_000); + assert_eq!( + plan.preflight.account_balance_credits, + reserve() + 3_623_849_220 + ); assert_eq!(plan.preflight.usable_balance_credits, 3_623_849_220); assert_eq!(plan.preflight.fee_reserve_credits, reserve()); assert_eq!(plan.preflight.max_shieldable_credits, expected_max); diff --git a/packages/rs-platform-wallet/src/wallet/shielded/activity.rs b/packages/rs-platform-wallet/src/wallet/shielded/activity.rs index 61d7f513db4..9613175353e 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/activity.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/activity.rs @@ -41,6 +41,8 @@ use std::collections::BTreeMap; +use grovedb_commitment_tree::{IncomingViewingKey, PaymentAddress}; + use crate::wallet::shielded::store::{ShieldedNote, ShieldedOutgoingNote}; /// How an entry's net direction reads relative to the wallet. @@ -385,9 +387,13 @@ fn cluster_events(input: &ScanDeriveInput) -> BTreeMap { /// Classification (client-side only — Option B), in priority order: /// - cluster has OVK outgoing note(s) to a NON-own address → [`Sent`] /// (counterparty / value / memo from the outgoing notes; own receipts -/// are the change and excluded from the amount). When the rho linkage -/// (see [`note_rho`]) identifies the consumed note(s), the exact fee -/// (spent − sent − change) is recovered too. +/// are the change and excluded from the amount). The amount is the SUM +/// over every external output, so a multi-output transfer yields one +/// aggregate row; its `counterparty` / `memo` are filled only when all +/// those outputs agree (see [`unanimous_bytes`]), matching the live +/// `transfer_multi` recorder. When the rho linkage (see [`note_rho`]) +/// identifies the consumed note(s), the exact fee (spent − sent − +/// change) is recovered too. /// - rho-linked cluster with no external recipient → [`ShieldedSpend`] /// with direction `Out` and the exact amount that left the pool /// (spent − change). This is provably our own spend (unshield / @@ -552,12 +558,23 @@ pub fn derive_activity_from_scan_data( } let change_total: u64 = cluster.received.iter().map(|n| n.value).sum(); - let entry = if let Some(send) = external.first() { + let entry = if !external.is_empty() { // SENT: at least one outgoing note to an address we don't own. // Amount = sum of external sends; own receipts in the cluster // are the change and are excluded. When the rho linkage // identified the consumed note(s), the exact fee falls out: // spent − sent − change. + // + // A multi-output transfer (Type 16 paying several recipients + // in one transition) restores as ONE such cluster, so + // `counterparty` and `memo` go through the shared + // [`unanimous_bytes`] rule: recorded only when every external + // output agrees, `None` otherwise. That is the same rule the + // live `transfer_multi` recorder applies — literally the same + // function — so a restored row names a recipient exactly when + // the live row would. Copying `external.first()` here instead + // would pin the whole aggregate amount on one of several + // distinct recipients. let amount: u64 = external.iter().map(|o| o.value).sum(); let fee = (!linked_nullifiers.is_empty()) .then(|| { @@ -572,8 +589,9 @@ pub fn derive_activity_from_scan_data( direction: ShieldedDirection::Out, amount, fee, - counterparty: Some(send.recipient.clone()), - memo: non_zero_memo(&send.memo), + counterparty: unanimous_bytes(external.iter().map(|o| o.recipient.as_slice())), + memo: unanimous_bytes(external.iter().map(|o| o.memo.as_slice())) + .and_then(|m| non_zero_memo(&m)), block_height: None, status: ShieldedActivityStatus::Confirmed, created_at_ms: 0, @@ -673,6 +691,36 @@ pub fn derive_activity_from_scan_data( out } +/// Collapse a transfer's per-output byte values into the ONE value that +/// describes the whole transfer, or `None` when the outputs disagree. +/// +/// A shielded transfer publishes exactly one activity row per +/// transition — the row's id is `sha256(sorted visible output cmxs)` +/// over the WHOLE cluster, so per-recipient rows could never dedupe +/// against the live row and a rescan would double-count. That single +/// row's scalar fields (`counterparty`, `memo`) are therefore only +/// meaningful when EVERY external output agrees on them: the +/// fund-one-address-with-N-notes shape. When they disagree there is no +/// honest single answer, and picking one output's value would attribute +/// the full aggregate amount to one of several distinct recipients. +/// "The first one" is not even a stable choice — Orchard's builder +/// shuffles outputs before pairing them into actions, so the winner +/// varies per build (and, on the restore path, per scan order). +/// +/// Both the live recorder (`transfer_multi`) and the cold-restore +/// deriver ([`derive_activity_from_scan_data`]) route their +/// `counterparty` through this one function over the same canonical +/// 43-byte raw address encoding, so the two paths reach an identical +/// verdict for a given transition by construction rather than by two +/// copies of the rule agreeing. +/// +/// Returns `None` for an empty iterator (no outputs, nothing to name). +pub(crate) fn unanimous_bytes<'a>(values: impl IntoIterator) -> Option> { + let mut values = values.into_iter(); + let first = values.next()?; + values.all(|v| v == first).then(|| first.to_vec()) +} + /// Return `Some(memo)` when `memo` is non-empty and not all-zero; /// `None` otherwise. A zero-filled 36-byte `DashMemo` is the "no memo" /// sentinel and shouldn't surface as an attached memo. @@ -684,6 +732,54 @@ pub(crate) fn non_zero_memo(memo: &[u8]) -> Option> { } } +/// The live `transfer_multi` activity ROW: classify ONE multi-output +/// shielded transfer's requested `(recipient, amount)` outputs into the +/// `(kind, amount, counterparty)` the live recorder writes. +/// +/// Partition the outputs into EXTERNAL payments vs WALLET-OWNED +/// receipts by testing each recipient against the account's +/// `IncomingViewingKey::diversifier_index` — Orchard addresses are +/// diversified, so a fixed-address comparison cannot work; this is the +/// same ownership test `coordinator::is_own_orchard_recipient` uses to +/// build the deriver's own-address set — then derive the row from the +/// external subset only: +/// +/// - No external output: nothing was paid to anyone, only the fee left +/// the pool → (`ShieldedSpend`, `fee`, no counterparty). Mirrors the +/// deriver's all-own arm, which has no external recipient to +/// aggregate and can never classify the cluster as `Sent`. +/// - Otherwise → (`Sent`, sum of the external amounts, counterparty by +/// the shared [`unanimous_bytes`] rule over the external recipients' +/// canonical 43-byte raw encodings — the exact form the deriver +/// recovers from the OVK-decrypted outgoing notes). +/// +/// `operations::transfer_multi` records its live row through THIS +/// function, and the `live_and_restored_agree_*` parity tests below run +/// the same function against [`derive_activity_from_scan_data`] — so +/// the parity those tests prove binds the production code path, not a +/// test-local restatement of it (#4312 review finding 379da4cc0ad0). +pub(crate) fn live_transfer_multi_row( + ivk: &IncomingViewingKey, + outputs: &[(PaymentAddress, u64)], + fee: u64, +) -> (ShieldedActivityKind, u64, Option>) { + let external: Vec<&(PaymentAddress, u64)> = outputs + .iter() + .filter(|(recipient, _)| ivk.diversifier_index(recipient).is_none()) + .collect(); + if external.is_empty() { + (ShieldedActivityKind::ShieldedSpend, fee, None) + } else { + let amount = external.iter().map(|(_, amount)| *amount).sum(); + let recipients: Vec> = external + .iter() + .map(|(recipient, _)| recipient.to_raw_address_bytes().to_vec()) + .collect(); + let counterparty = unanimous_bytes(recipients.iter().map(|r| r.as_slice())); + (ShieldedActivityKind::Sent, amount, counterparty) + } +} + /// Sort entries for display, in four bands. Mutates in place. /// /// 1. `Pending` STATUS rows float to the very top. Pending is a status, @@ -927,6 +1023,320 @@ mod tests { assert_eq!(d[0].memo, Some(memo)); } + // ── multi-output transfers on the restore path ───────────────── + + /// The live `transfer_multi` counterparty rule, evaluated exactly as + /// `operations::transfer_multi` evaluates it: the raw 43-byte + /// encoding of each `(address, amount)` output, in call order, + /// through the shared [`unanimous_bytes`] helper. The restore path + /// must agree with this for the same transition. + fn live_counterparty(recipients: &[Vec]) -> Option> { + unanimous_bytes(recipients.iter().map(|r| r.as_slice())) + } + + /// Real Orchard key material for the live/restored parity tests. The + /// live row's ownership predicate is the account IVK itself + /// (`IncomingViewingKey::diversifier_index`, inside + /// [`live_transfer_multi_row`]), so exercising the production + /// classifier needs addresses a real IVK does and does not recognize. + fn parity_keys(seed_byte: u8) -> crate::wallet::shielded::keys::OrchardKeySet { + crate::wallet::shielded::keys::OrchardKeySet::from_seed( + &[seed_byte; 64], + dashcore::Network::Testnet, + 0, + ) + .expect("a 64-byte seed satisfies the ZIP-32 bounds") + } + + /// Canonical 43-byte raw encoding of `addr` — the form outgoing notes + /// store recipients in and the deriver matches own-addresses against. + fn raw(addr: &PaymentAddress) -> Vec { + addr.to_raw_address_bytes().to_vec() + } + + #[test] + fn live_and_restored_agree_on_a_mixed_external_and_own_output_set() { + // The reviewer's mixed case: ONE Type-16 transition paying 10 + // credits to an EXTERNAL address and 20 to one of the account's + // OWN diversified addresses (the public API accepts both). + // + // The live side of this check is the PRODUCTION classifier — + // [`live_transfer_multi_row`], the function + // `operations::transfer_multi` records its row through — run with + // real Orchard key material, so the ownership predicate under test + // is the real IVK one. + // + // Restoration removes the own output before aggregating, so it + // records a 10-credit send to the external address. The live + // recorder must reach the same verdict — before the fix it summed + // every requested output and recorded a 30-credit send with no + // counterparty (#4312 review finding 379da4cc0ad0). + let ours = parity_keys(7); + let theirs = parity_keys(9); + let external = theirs.default_address; + // A non-default diversified address: only the IVK predicate can + // recognize it as ours — a fixed-address comparison against the + // default address could not (`coordinator::is_own_orchard_recipient` + // builds the deriver's own-address set from the same predicate). + let own = ours.address_at(5); + let fee = 7u64; + + let input = ScanDeriveInput { + notes: vec![], + outgoing: vec![ + outgoing(0x70, raw(&external), 800, 10, vec![0u8; 36]), + outgoing(0x71, raw(&own), 800, 20, vec![0u8; 36]), + ], + own_addresses: vec![raw(&own)], + }; + let d = derive_activity_from_scan_data(&input, &BTreeMap::new()).new_entries; + assert_eq!(d.len(), 1, "one transition restores as one activity row"); + + let (kind, amount, counterparty) = live_transfer_multi_row( + &ours.incoming_viewing_key, + &[(external, 10), (own, 20)], + fee, + ); + + assert_eq!(d[0].kind, kind, "kind must agree"); + assert_eq!(kind, ShieldedActivityKind::Sent); + assert_eq!( + d[0].amount, amount, + "amount must agree, and must count only the external payment" + ); + assert_eq!(amount, 10, "the own output is change, not a payment"); + assert_eq!(d[0].counterparty, counterparty, "counterparty must agree"); + assert_eq!( + counterparty, + Some(raw(&external)), + "one external recipient names itself" + ); + + // Pin the regression itself: the pre-fix live row was the SUM over + // every output with no counterparty, which restoration never + // produces for this transition. + assert_ne!(amount, 30, "the own output must not inflate the amount"); + } + + #[test] + fn live_and_restored_agree_that_an_all_own_output_set_is_not_a_send() { + // Every output lands on an address this account's IVK recognizes, + // so nothing was paid to anyone. Restoration cannot classify this + // as `Sent` (it has no external recipient to aggregate); the live + // classifier — the production [`live_transfer_multi_row`] that + // `operations::transfer_multi` records through — must not either. + let ours = parity_keys(7); + let own_a = ours.default_address; + let own_b = ours.address_at(3); + let fee = 7u64; + + let (kind, amount, counterparty) = + live_transfer_multi_row(&ours.incoming_viewing_key, &[(own_a, 10), (own_b, 20)], fee); + assert_eq!( + kind, + ShieldedActivityKind::ShieldedSpend, + "an all-own output set is a shielded spend, not a send" + ); + assert_eq!(amount, fee, "only the fee left the pool"); + assert_eq!(counterparty, None, "there is no counterparty to name"); + + // The restore side agrees on the kind: with both outgoing notes + // recognized as own, the cluster has no external recipient, so the + // `Sent` arm cannot fire. + let input = ScanDeriveInput { + notes: vec![own_note(0x80, 0x81, 900, 1_000, true)], + outgoing: vec![ + outgoing(0x82, raw(&own_a), 900, 10, vec![0u8; 36]), + outgoing(0x83, raw(&own_b), 900, 20, vec![0u8; 36]), + ], + own_addresses: vec![raw(&own_a), raw(&own_b)], + }; + let d = derive_activity_from_scan_data(&input, &BTreeMap::new()).new_entries; + assert_eq!(d.len(), 1); + assert_ne!( + d[0].kind, + ShieldedActivityKind::Sent, + "restoration must not call an all-own output set a send" + ); + assert_eq!(d[0].kind, kind, "kind must agree with the live row"); + } + + #[test] + fn multi_recipient_restore_aggregates_without_attributing_to_one_recipient() { + // The reviewer's case: ONE Type-16 transition paying 10 credits to + // A and 20 to B. Both outgoing notes are OVK-recovered into the + // same height cluster, so restoration must produce ONE aggregate + // row of 30 — and must NOT pin that 30 on either recipient. + let a = addr(0xAA); + let b = addr(0xBB); + let input = ScanDeriveInput { + notes: vec![], + outgoing: vec![ + outgoing(0x60, a.clone(), 700, 10, vec![0u8; 36]), + outgoing(0x61, b.clone(), 700, 20, vec![0u8; 36]), + ], + own_addresses: vec![addr(0x01)], + }; + let d = derive_activity_from_scan_data(&input, &BTreeMap::new()).new_entries; + + assert_eq!(d.len(), 1, "one transition restores as one activity row"); + assert_eq!(d[0].kind, ShieldedActivityKind::Sent); + assert_eq!(d[0].direction, ShieldedDirection::Out); + assert_eq!(d[0].amount, 30, "amount is the sum over both outputs"); + assert_eq!( + d[0].counterparty, None, + "a 30-credit row covering two distinct recipients must name \ + neither of them" + ); + // Live-path parity: the same rule, over the same raw encodings, + // is what `transfer_multi` records for this transition. + assert_eq!( + d[0].counterparty, + live_counterparty(&[a, b]), + "restored attribution must match what the live recorder writes" + ); + } + + #[test] + fn multi_recipient_restore_is_independent_of_output_order() { + // Orchard's builder shuffles outputs before pairing them into + // actions, so "the first external output" is not a stable choice. + // Feeding the same two outputs in the opposite order must derive + // a byte-identical row (id included). + let a = addr(0xAA); + let b = addr(0xBB); + let forward = outgoing(0x60, a.clone(), 700, 10, vec![0u8; 36]); + let reverse = outgoing(0x61, b.clone(), 700, 20, vec![0u8; 36]); + + let mut d1 = derive_activity_from_scan_data( + &ScanDeriveInput { + notes: vec![], + outgoing: vec![forward.clone(), reverse.clone()], + own_addresses: vec![addr(0x01)], + }, + &BTreeMap::new(), + ) + .new_entries; + let mut d2 = derive_activity_from_scan_data( + &ScanDeriveInput { + notes: vec![], + outgoing: vec![reverse, forward], + own_addresses: vec![addr(0x01)], + }, + &BTreeMap::new(), + ) + .new_entries; + + assert_eq!(d1.len(), 1); + assert_eq!(d2.len(), 1); + // `created_at_ms` is wall-clock; everything that describes the + // transition must match. `note_cmxs` is stored in encounter + // order (only `compute_activity_id` sorts), so it is compared as + // the SET it represents — which is what the id contract keys on. + let (e1, e2) = (d1.remove(0), d2.remove(0)); + assert_eq!(e1.id, e2.id, "cluster id is order-independent"); + assert_eq!(e1.amount, e2.amount); + assert_eq!( + e1.counterparty, e2.counterparty, + "attribution must not depend on which output the scan saw first" + ); + assert_eq!(e1.memo, e2.memo); + let (mut c1, mut c2) = (e1.note_cmxs.clone(), e2.note_cmxs.clone()); + c1.sort_unstable(); + c2.sort_unstable(); + assert_eq!(c1, c2, "same cluster covers the same cmx set"); + } + + #[test] + fn multi_note_restore_to_a_single_address_keeps_the_counterparty() { + // The fund-one-address-with-N-notes shape (how a two-note invite + // is funded): every output names the SAME address, so the row + // still has one honest counterparty and must keep it — the fix + // must not over-correct into always dropping attribution. + let target = addr(0xCC); + let memo = { + let mut m = vec![0u8; 36]; + m[0] = 1; + m + }; + let input = ScanDeriveInput { + notes: vec![], + outgoing: vec![ + outgoing(0x70, target.clone(), 800, 100, memo.clone()), + outgoing(0x71, target.clone(), 800, 250, memo.clone()), + ], + own_addresses: vec![addr(0x01)], + }; + let d = derive_activity_from_scan_data(&input, &BTreeMap::new()).new_entries; + + assert_eq!(d.len(), 1); + assert_eq!(d[0].kind, ShieldedActivityKind::Sent); + assert_eq!(d[0].amount, 350); + assert_eq!( + d[0].counterparty.as_deref(), + Some(target.as_slice()), + "all outputs agree on the address, so the row keeps it" + ); + assert_eq!( + d[0].memo, + Some(memo), + "transfer_multi attaches one memo to every recipient note, so \ + the unanimous memo survives" + ); + assert_eq!( + d[0].counterparty, + live_counterparty(&[target.clone(), target]), + "restored attribution must match what the live recorder writes" + ); + } + + #[test] + fn multi_recipient_restore_drops_a_memo_the_outputs_disagree_on() { + // Same misattribution class as the counterparty: a memo that only + // one output carried must not be presented as the whole + // transfer's memo. + let mut memo_a = vec![0u8; 36]; + memo_a[0] = 1; + let input = ScanDeriveInput { + notes: vec![], + outgoing: vec![ + outgoing(0x80, addr(0xAA), 900, 10, memo_a), + outgoing(0x81, addr(0xBB), 900, 20, vec![0u8; 36]), + ], + own_addresses: vec![addr(0x01)], + }; + let d = derive_activity_from_scan_data(&input, &BTreeMap::new()).new_entries; + + assert_eq!(d.len(), 1); + assert_eq!(d[0].amount, 30); + assert_eq!(d[0].counterparty, None); + assert_eq!(d[0].memo, None, "outputs disagree, so no memo is claimed"); + } + + #[test] + fn unanimous_bytes_rule() { + assert_eq!(unanimous_bytes(std::iter::empty()), None, "no outputs"); + assert_eq!( + unanimous_bytes([b"abc".as_slice()]), + Some(b"abc".to_vec()), + "a single output always names itself" + ); + assert_eq!( + unanimous_bytes([b"abc".as_slice(), b"abc".as_slice()]), + Some(b"abc".to_vec()) + ); + assert_eq!( + unanimous_bytes([b"abc".as_slice(), b"abd".as_slice()]), + None + ); + // Disagreement anywhere in the list counts, not just against the + // second element. + assert_eq!( + unanimous_bytes([b"abc".as_slice(), b"abc".as_slice(), b"zzz".as_slice()]), + None + ); + } + #[test] fn sent_with_own_change_in_same_cluster_excludes_change_from_amount() { // A send that also produced an own change note at the same height: diff --git a/packages/rs-platform-wallet/src/wallet/shielded/note_selection.rs b/packages/rs-platform-wallet/src/wallet/shielded/note_selection.rs index b4e0b7ea81f..30ad5a3a224 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/note_selection.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/note_selection.rs @@ -70,6 +70,35 @@ impl ShieldedFeeKind { } } +/// Whether the spend's builder tolerates a zero-valued change output. +/// +/// The two shapes differ by exactly one credit at the boundary, and note selection MUST reserve +/// against the shape its builder actually enforces — otherwise a selection is reserved, the +/// builder rejects it, and a wallet with sufficient balance reports a failed spend (the +/// reservation is released, so nothing is stranded, but the spend is refused). +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum ChangeRequirement { + /// The builder tolerates zero change: `build_shielded_transfer_transition` simply omits its + /// change output, and `build_unshield_transition` / + /// `build_shielded_withdrawal_transition` emit a zero-valued one. All three reject only + /// `required > total_spent`, so exact coverage is a fundable selection. + Optional, + /// The builder ALWAYS emits a change output, which must carry a positive value, so it + /// rejects `total_spent == amount + fee` (`build_shielded_transfer_transition_multi` tests + /// `required >= total_spent`). Selection must therefore cover `amount + fee + 1`. + StrictlyPositive, +} + +impl ChangeRequirement { + /// Credits the selection must cover ON TOP of `amount + fee`. + fn min_change_credits(self) -> u64 { + match self { + ChangeRequirement::Optional => 0, + ChangeRequirement::StrictlyPositive => 1, + } + } +} + /// Select unspent notes to cover `amount + fee` using a greedy algorithm. /// /// Notes are sorted by value descending and accumulated until the target is met. @@ -153,27 +182,66 @@ pub fn select_notes( /// ShieldedTransfer. This MUST match the fee the builder/consensus will charge, otherwise the spend /// is under-funded. /// -/// Returns the selected notes, total input value, and the exact fee. +/// `change` states whether the builder can omit its change output. Pass +/// [`ChangeRequirement::StrictlyPositive`] for the multi-output transfer builder, whose change +/// output is unconditional and must carry a positive value: an exact-coverage selection +/// (`total_input == amount + fee`) satisfies this function's `>=` test but is then REJECTED by +/// that builder, so a wallet that could fund the spend by selecting one more note would report a +/// failure instead. The extra credit is folded into the selection target on every iteration, so +/// the fee re-computation that follows an added note (and the action count that added note +/// implies) is applied to the strict target too. +/// +/// Returns the selected notes, total input value, and the exact fee. The returned fee is the pure +/// consensus fee — the change-requirement credit is a selection-side floor only and is NOT part +/// of what the builder carves. pub fn select_notes_with_fee<'a>( unspent: &'a [ShieldedNote], amount: u64, min_actions: usize, fee_kind: ShieldedFeeKind, + change: ChangeRequirement, platform_version: &PlatformVersion, ) -> Result<(Vec<&'a ShieldedNote>, u64, u64), PlatformWalletError> { + let min_change = change.min_change_credits(); let mut fee_estimate = fee_kind .compute(min_actions, platform_version) .map_err(|e| PlatformWalletError::ShieldedBuildError(e.to_string()))?; + // Target for `select_notes`, which adds it to `amount`: the fee plus the minimum change the + // builder demands. + let selection_target = |fee: u64| -> Result { + fee.checked_add(min_change).ok_or_else(|| { + PlatformWalletError::ShieldedBuildError( + "fee + minimum change overflows u64".to_string(), + ) + }) + }; + + // Full value the selection must cover: `amount + fee + min_change`, CHECKED. Saturating here + // would turn an unrepresentable requirement into `u64::MAX`, which a (corrupt/crafted) store + // totaling exactly `u64::MAX` then satisfies via `>=` — returning `Ok` for a selection whose + // strictly-positive-change postcondition cannot hold, so the builder rejects it downstream. + // An unrepresentable requirement is an error, exactly like `select_notes`' own overflow gates. + let required_total = |fee: u64| -> Result { + amount + .checked_add(fee) + .and_then(|v| v.checked_add(min_change)) + .ok_or_else(|| { + PlatformWalletError::ShieldedBuildError( + "amount + fee + minimum change overflows u64".to_string(), + ) + }) + }; + for _ in 0..5 { - let selected = select_notes(unspent, amount, fee_estimate)?; + let selected = select_notes(unspent, amount, selection_target(fee_estimate)?)?; let total: u64 = selected.iter().map(|n| n.value).sum(); let num_actions = selected.len().max(min_actions); let exact_fee = fee_kind .compute(num_actions, platform_version) .map_err(|e| PlatformWalletError::ShieldedBuildError(e.to_string()))?; - if total >= amount.saturating_add(exact_fee) { + if total >= required_total(exact_fee)? { return Ok((selected, total, exact_fee)); } @@ -181,17 +249,18 @@ pub fn select_notes_with_fee<'a>( } // Final attempt with last computed fee - let selected = select_notes(unspent, amount, fee_estimate)?; + let selected = select_notes(unspent, amount, selection_target(fee_estimate)?)?; let total: u64 = selected.iter().map(|n| n.value).sum(); let num_actions = selected.len().max(min_actions); let exact_fee = fee_kind .compute(num_actions, platform_version) .map_err(|e| PlatformWalletError::ShieldedBuildError(e.to_string()))?; - if total < amount.saturating_add(exact_fee) { + let required = required_total(exact_fee)?; + if total < required { return Err(PlatformWalletError::ShieldedInsufficientBalance { available: total, - required: amount.saturating_add(exact_fee), + required, }); } @@ -372,9 +441,15 @@ mod tests { // A single note covering amount + the 2-action fee. let notes = vec![test_note(amount + min_fee_2 + 5, 0)]; - let (selected, total, exact_fee) = - select_notes_with_fee(¬es, amount, 2, ShieldedFeeKind::Base, platform_version) - .expect("selection ok"); + let (selected, total, exact_fee) = select_notes_with_fee( + ¬es, + amount, + 2, + ShieldedFeeKind::Base, + ChangeRequirement::Optional, + platform_version, + ) + .expect("selection ok"); assert_eq!(selected.len(), 1); assert_eq!(total, amount + min_fee_2 + 5); @@ -383,6 +458,272 @@ mod tests { assert_eq!(exact_fee, min_fee_2); } + /// A multi-output transfer reserves against `recipients + 1` outputs, because the bundle + /// publishes `max(spends, recipients + 1, 2)` actions and a ShieldedTransfer's + /// `value_balance` must equal `compute_minimum_shielded_fee(actions.len())` EXACTLY. If the + /// reservation used the 2-action floor instead, it would under-reserve and the builder's + /// carved fee would not match what was reserved. + #[test] + fn test_select_notes_with_fee_reserves_multi_output_action_floor() { + let platform_version = PlatformVersion::latest(); + // Two recipient notes + change = 3 outputs → a 3-action floor. + let min_actions = 3; + let min_fee_3 = compute_minimum_shielded_fee(3, platform_version).expect("fee"); + let min_fee_2 = compute_minimum_shielded_fee(2, platform_version).expect("fee"); + assert!( + min_fee_3 > min_fee_2, + "a 3-action bundle must cost more than a 2-action one" + ); + + let amount = 3_000_000_000u64; // the invite denomination, split across two notes + // A single note covering amount + the 3-action fee (plus change). + let notes = vec![test_note(amount + min_fee_3 + 1, 0)]; + + let (selected, _total, exact_fee) = select_notes_with_fee( + ¬es, + amount, + min_actions, + ShieldedFeeKind::Base, + ChangeRequirement::Optional, + platform_version, + ) + .expect("selection ok"); + + assert_eq!(selected.len(), 1); + assert_eq!( + exact_fee, min_fee_3, + "one spend but three outputs must reserve the 3-action fee, not the 2-action floor" + ); + } + + /// THE regression pin for the exact-fit selection bug. + /// + /// `build_shielded_transfer_transition_multi` always emits a change output and so requires + /// the spent value to STRICTLY exceed `sum(amounts) + fee`. With notes valued + /// `[amount + fee, 1]`, largest-first selection used to stop on the exact-coverage note + /// alone: the reservation succeeded, the builder then rejected the spend, and a wallet that + /// could have funded it by taking the remaining credit reported a failure. + /// [`ChangeRequirement::StrictlyPositive`] makes the selector demand that extra credit. + #[test] + fn test_select_notes_with_fee_strict_change_takes_one_more_credit() { + let platform_version = PlatformVersion::latest(); + // Two recipient notes + change = 3 outputs → the 3-action floor the multi-output + // transfer reserves against. + let min_actions = 3; + let fee = compute_minimum_shielded_fee(min_actions, platform_version).expect("fee"); + let amount = 3_000_000_000u64; + + // The reviewer's shape: one note covering `amount + fee` exactly, plus a single credit. + let notes = vec![test_note(amount + fee, 0), test_note(1, 1)]; + + let (selected, total, exact_fee) = select_notes_with_fee( + ¬es, + amount, + min_actions, + ShieldedFeeKind::Base, + ChangeRequirement::StrictlyPositive, + platform_version, + ) + .expect("a wallet holding amount + fee + 1 must be able to fund a multi-output transfer"); + + assert_eq!( + selected.len(), + 2, + "the exact-coverage note alone leaves zero change; the extra credit must be selected" + ); + assert_eq!( + exact_fee, fee, + "two spends still sit under the 3-action floor" + ); + assert!( + total > amount.saturating_add(exact_fee), + "the selection must leave STRICTLY positive change ({total} > {amount} + {exact_fee})" + ); + + // Same wallet under the permissive contract stops on the exact-coverage note — the state + // the builder rejects. This is what the strict variant exists to prevent. + let (permissive, permissive_total, permissive_fee) = select_notes_with_fee( + ¬es, + amount, + min_actions, + ShieldedFeeKind::Base, + ChangeRequirement::Optional, + platform_version, + ) + .expect("selection ok"); + assert_eq!(permissive.len(), 1); + assert_eq!( + permissive_total, + amount + permissive_fee, + "the permissive contract accepts exact coverage — zero change" + ); + } + + /// One credit short of the strict requirement is a genuine insufficient balance, and the + /// reported `required` must include the change credit so the caller sees the real shortfall. + #[test] + fn test_select_notes_with_fee_strict_change_reports_the_extra_credit_as_required() { + let platform_version = PlatformVersion::latest(); + let min_actions = 3; + let fee = compute_minimum_shielded_fee(min_actions, platform_version).expect("fee"); + let amount = 3_000_000_000u64; + // Exactly `amount + fee` and not a credit more. + let notes = vec![test_note(amount + fee, 0)]; + + let err = select_notes_with_fee( + ¬es, + amount, + min_actions, + ShieldedFeeKind::Base, + ChangeRequirement::StrictlyPositive, + platform_version, + ) + .expect_err("exact coverage cannot fund a builder that demands positive change"); + + match err { + PlatformWalletError::ShieldedInsufficientBalance { + available, + required, + } => { + assert_eq!(available, amount + fee); + assert_eq!( + required, + amount + fee + 1, + "the required figure must include the change credit" + ); + } + other => panic!("unexpected error: {other:?}"), + } + } + + /// An UNREPRESENTABLE strict-change requirement must be an error, never a saturated success. + /// + /// Boundary: `amount + exact_fee == u64::MAX` with strict change, so the full requirement + /// (`amount + exact_fee + 1`) overflows u64. A saturating comparison would clamp it to + /// `u64::MAX`, which a crafted/corrupt store totaling exactly `u64::MAX` then satisfies via + /// `>=` — an `Ok` selection whose positive-change postcondition cannot hold (the builder + /// rejects it downstream). The note shape forces the greedy pass past the estimate-phase + /// target so the OVERFLOWING requirement is only reachable at the convergence re-check, + /// where the old code saturated (`select_notes`' own gates are checked and never see it). + #[test] + fn test_select_notes_with_fee_strict_change_overflow_is_an_error_not_saturation() { + let platform_version = PlatformVersion::latest(); + let min_actions = 2; + let fee_2 = compute_minimum_shielded_fee(2, platform_version).expect("fee"); + let fee_3 = compute_minimum_shielded_fee(3, platform_version).expect("fee"); + assert!(fee_3 > fee_2, "the fee must grow with the action count"); + let fee_step = fee_3 - fee_2; + + // amount + fee_3 == u64::MAX exactly → the strict requirement is u64::MAX + 1. + let amount = u64::MAX - fee_3; + + // Three notes summing to exactly u64::MAX, shaped so largest-first must take all three + // to reach the estimate-phase target (the top two together stay strictly below it): + // the selection lands on 3 actions, the fee converges from fee_2 up to fee_3, and the + // re-check computes the overflowing full requirement. + let notes = vec![ + test_note(u64::MAX - 2 * (fee_step + 10), 0), + test_note(fee_step + 10, 1), + test_note(fee_step + 10, 2), + ]; + + let err = select_notes_with_fee( + ¬es, + amount, + min_actions, + ShieldedFeeKind::Base, + ChangeRequirement::StrictlyPositive, + platform_version, + ) + .expect_err("an unrepresentable requirement must be an error, not a saturated Ok"); + assert!( + matches!( + err, + PlatformWalletError::ShieldedBuildError(ref m) + if m.contains("overflows") + ), + "expected an overflow ShieldedBuildError, got: {err:?}" + ); + } + + /// The strict floor must survive fee convergence: when the selector adds notes, the action + /// count (and therefore the fee) is recomputed, and the strict `total > amount + fee` + /// postcondition must hold against the RECOMPUTED fee, not the initial estimate. + #[test] + fn test_select_notes_with_fee_strict_change_holds_after_fee_reconvergence() { + let platform_version = PlatformVersion::latest(); + let min_actions = 3; + let amount = 1_000_000u64; + // Many equal mid-size notes, so several must be selected and the action count — and with + // it the fee — climbs past the 3-action floor during convergence. + let notes: Vec = (0..20).map(|i| test_note(60_000_000, i)).collect(); + + let (selected, total, exact_fee) = select_notes_with_fee( + ¬es, + amount, + min_actions, + ShieldedFeeKind::Base, + ChangeRequirement::StrictlyPositive, + platform_version, + ) + .expect("selection ok"); + + let expected_fee = + compute_minimum_shielded_fee(selected.len().max(min_actions), platform_version) + .expect("fee"); + assert_eq!( + exact_fee, expected_fee, + "the returned fee must match the selected action count" + ); + assert!( + total > amount.saturating_add(exact_fee), + "strict change must hold against the recomputed fee" + ); + } + + /// Two sub-denomination notes on one key are STRUCTURALLY forced to both be selected when + /// the spend targets the full denomination: the greedy selector takes the largest note first + /// and only stops once the accumulated value covers the target, and neither note alone can. + /// This is what removes Orchard's padding action (and its random, unreproducible dummy + /// nullifier) from the claim bundle. + #[test] + fn test_two_sub_denomination_notes_are_both_selected() { + // The two shipped invite denominations, each split floor(D/2) + ceil(D/2). + for denomination in [3_000_000_000u64, 25_000_000_000u64] { + let lo = denomination / 2; + let hi = denomination - lo; + assert!( + lo < denomination && hi < denomination, + "each half must be strictly below the denomination" + ); + + let notes = vec![test_note(hi, 0), test_note(lo, 1)]; + // The claim targets the denomination exactly (fee metered from it, not added). + let selected = select_notes(¬es, denomination, 0).expect("selection ok"); + assert_eq!( + selected.len(), + 2, + "both sub-denomination notes must be selected for denomination {denomination}" + ); + let total: u64 = selected.iter().map(|n| n.value).sum(); + assert_eq!(total, denomination); + } + } + + /// Contrast: a SINGLE note worth the whole denomination stops the greedy selector after one + /// note — the one-note invite shape that leaves Orchard to pad the bundle with a dummy. + #[test] + fn test_single_full_denomination_note_selects_alone() { + let denomination = 3_000_000_000u64; + let notes = vec![test_note(denomination, 0)]; + let selected = select_notes(¬es, denomination, 0).expect("selection ok"); + assert_eq!( + selected.len(), + 1, + "a single full-denomination note covers the target alone, so the bundle needs padding" + ); + } + #[test] fn test_select_notes_with_fee_uses_actual_action_count() { let platform_version = PlatformVersion::latest(); @@ -392,9 +733,15 @@ mod tests { let note_val = 60_000_000u64; let notes: Vec = (0..20).map(|i| test_note(note_val, i)).collect(); - let (selected, total, exact_fee) = - select_notes_with_fee(¬es, amount, 2, ShieldedFeeKind::Base, platform_version) - .expect("selection ok"); + let (selected, total, exact_fee) = select_notes_with_fee( + ¬es, + amount, + 2, + ShieldedFeeKind::Base, + ChangeRequirement::Optional, + platform_version, + ) + .expect("selection ok"); let expected_fee = compute_minimum_shielded_fee(selected.len().max(2), platform_version).unwrap(); @@ -433,6 +780,7 @@ mod tests { amount, 2, ShieldedFeeKind::Withdrawal, + ChangeRequirement::Optional, platform_version, ) .expect("selection ok"); @@ -470,6 +818,7 @@ mod tests { amount, 2, ShieldedFeeKind::Unshield, + ChangeRequirement::Optional, platform_version, ) .expect("selection ok"); diff --git a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs index 2b75fbc4609..2279320b622 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs @@ -19,13 +19,15 @@ //! - **IdentityCreateFromShieldedPool** (Type 20): shielded pool → a brand-new Platform identity //! funded by a fixed denomination leaving the pool (any excess re-enters as a change note) -use super::activity::{ShieldedActivityKind, ShieldedActivityStatus, ShieldedDirection}; +use super::activity::{ + live_transfer_multi_row, ShieldedActivityKind, ShieldedActivityStatus, ShieldedDirection, +}; use super::activity_recorder::{ build_pending_entry, changeset_for_entry, non_zero_memo, with_status, LiveEntryParams, }; use super::keys::{AccountViewingKeys, OrchardKeySet}; use super::note_selection::{ - select_notes_for_denomination, select_notes_with_fee, ShieldedFeeKind, + select_notes_for_denomination, select_notes_with_fee, ChangeRequirement, ShieldedFeeKind, }; use super::store::{PendingRedrive, ShieldedNote, ShieldedStore, SubwalletId}; use crate::broadcast_outcome::{broadcast_definitely_failed, carries_consensus_rejection}; @@ -52,8 +54,9 @@ use dpp::identity::{Identity, IdentityPublicKey}; use dpp::prelude::Identifier; use dpp::shielded::builder::{ build_identity_create_from_shielded_pool_transition, build_shield_transition, - build_shielded_transfer_transition, build_shielded_withdrawal_transition, - build_unshield_transition, OrchardProver, SpendableNote, + build_shielded_transfer_transition, build_shielded_transfer_transition_multi, + build_shielded_withdrawal_transition, build_unshield_transition, OrchardProver, + ShieldedTransferOutput, SpendableNote, }; use dpp::shielded::compute_minimum_shielded_fee; use dpp::state_transition::proof_result::StateTransitionProofResult; @@ -713,8 +716,18 @@ pub async fn unshield( // reserve against `ShieldedFeeKind::Unshield` — reserving the base fee here would under-fund the // address-write cost and the builder would reject the spend (and the `fee_used == exact_fee` // debug assert below would fire). - let (selected_notes, total_input, exact_fee) = - reserve_unspent_notes(sdk, store, id, amount, 2, ShieldedFeeKind::Unshield).await?; + let (selected_notes, total_input, exact_fee) = reserve_unspent_notes( + sdk, + store, + id, + amount, + 2, + ShieldedFeeKind::Unshield, + // `build_unshield_transition` accepts a zero-valued change output (it rejects only + // `required > total_spent`), so exact coverage is a fundable selection. + ChangeRequirement::Optional, + ) + .await?; info!( account, @@ -744,18 +757,26 @@ pub async fn unshield( // The builder computes and returns the fee authoritatively; `exact_fee` (== the // minimum) was already used above for note reservation. - let (state_transition, fee_used) = build_unshield_transition( - spends, - *to_address, - amount, - &change_addr, - &keys.full_viewing_key, - &keys.spend_auth_key, - anchor, - prover, - [0u8; 36], - sdk.version(), - ) + // + // Build + prove runs under the pre-broadcast panic guard (see + // `catch_pre_broadcast_panic`): a prover panic here is provably pre-broadcast, so it + // must become a DEFINITIVE error the outer failure arm releases the reservation on — + // not an unwind past these match arms into the FFI guard's ambiguous contract, which + // strands the anchor-less reservation for the rest of the process lifetime. + let (state_transition, fee_used) = catch_pre_broadcast_panic("unshield", || { + build_unshield_transition( + spends, + *to_address, + amount, + &change_addr, + &keys.full_viewing_key, + &keys.spend_auth_key, + anchor, + prover, + [0u8; 36], + sdk.version(), + ) + })? .map_err(|e| PlatformWalletError::ShieldedBuildError(e.to_string()))?; // The builder's fee and the wallet's reserved `exact_fee` both come from // compute_shielded_unshield_fee with the same action count; lock that they agree. @@ -900,8 +921,18 @@ pub async fn transfer( // ShieldedTransfer is carved with the base `compute_minimum_shielded_fee`, so reserve // against `ShieldedFeeKind::Base`. - let (selected_notes, total_input, exact_fee) = - reserve_unspent_notes(sdk, store, id, amount, 2, ShieldedFeeKind::Base).await?; + let (selected_notes, total_input, exact_fee) = reserve_unspent_notes( + sdk, + store, + id, + amount, + 2, + ShieldedFeeKind::Base, + // `build_shielded_transfer_transition` emits change only when there is some, so an + // exact-coverage selection is fundable. + ChangeRequirement::Optional, + ) + .await?; info!( account, @@ -922,18 +953,24 @@ pub async fn transfer( // The builder computes and returns the fee authoritatively; `exact_fee` (== the // minimum) was already used above for note reservation. - let (state_transition, fee_used) = build_shielded_transfer_transition( - spends, - &recipient_addr, - amount, - &change_addr, - &keys.full_viewing_key, - &keys.spend_auth_key, - anchor, - prover, - memo, - sdk.version(), - ) + // + // Build + prove runs under the pre-broadcast panic guard, exactly as in `transfer_multi` + // (see `catch_pre_broadcast_panic`): a prover panic must release the reservation rather + // than unwind past these match arms into the FFI guard's ambiguous contract. + let (state_transition, fee_used) = catch_pre_broadcast_panic("transfer", || { + build_shielded_transfer_transition( + spends, + &recipient_addr, + amount, + &change_addr, + &keys.full_viewing_key, + &keys.spend_auth_key, + anchor, + prover, + memo, + sdk.version(), + ) + })? .map_err(|e| PlatformWalletError::ShieldedBuildError(e.to_string()))?; // The builder's fee and the wallet's reserved `exact_fee` both come from // compute_minimum_shielded_fee with the same action count; lock that they agree. @@ -1030,6 +1067,236 @@ pub async fn transfer( } } +/// Transfer funds privately from `account`'s shielded notes to +/// SEVERAL Orchard outputs in one atomic transition (Type 16). +/// +/// Multi-output sibling of [`transfer`]. Each `(address, amount)` pair becomes its own note — +/// including when several pairs name the SAME address, which is how one transition funds an +/// address with more than one note. +/// +/// `memo` is attached to every recipient note (the change note always carries the empty memo). +/// +/// # Fee sizing +/// +/// The bundle publishes `max(spends, recipients + 1, 2)` actions and a `ShieldedTransfer`'s +/// `value_balance` must equal `compute_minimum_shielded_fee(actions.len())` EXACTLY. Note +/// selection therefore reserves against `recipients.len() + 1` outputs — the same floor the +/// builder sizes its fee from — so the reserved and carved fees cannot diverge. +#[allow(clippy::too_many_arguments)] +pub async fn transfer_multi( + sdk: &Arc, + store: &Arc>, + persister: Option<&WalletPersister>, + wallet_id: WalletId, + keys: &OrchardKeySet, + account: u32, + outputs: &[(PaymentAddress, u64)], + memo: [u8; 36], + prover: &P, +) -> Result<(), PlatformWalletError> { + if outputs.is_empty() { + return Err(PlatformWalletError::ShieldedBuildError( + "a multi-output shielded transfer needs at least one recipient output".to_string(), + )); + } + + let builder_outputs: Vec = outputs + .iter() + .map(|(addr, amount)| { + Ok(ShieldedTransferOutput { + recipient: payment_address_to_orchard(addr)?, + amount: *amount, + memo, + }) + }) + .collect::>()?; + + let total_amount = outputs + .iter() + .try_fold(0u64, |acc, (_, amount)| acc.checked_add(*amount)) + .ok_or_else(|| { + PlatformWalletError::ShieldedBuildError( + "multi-output shielded transfer amounts overflow u64".to_string(), + ) + })?; + + let views = keys.viewing_keys(); + let change_addr = default_orchard_address(&views)?; + let id = SubwalletId::new(wallet_id, account); + + // Reserve against the SAME output count the builder sizes its fee from: every recipient + // output plus the unconditional change output. + // + // That change output is unconditional and must carry a positive value, so the builder + // rejects `total_input == total_amount + fee`. Selection must therefore demand STRICTLY + // more, or a wallet holding e.g. `[total_amount + fee, 1]` would have the exact-coverage + // note reserved on its own and the build would fail despite the balance being sufficient. + let num_outputs = builder_outputs.len() + 1; + let (selected_notes, total_input, exact_fee) = reserve_unspent_notes( + sdk, + store, + id, + total_amount, + num_outputs, + ShieldedFeeKind::Base, + ChangeRequirement::StrictlyPositive, + ) + .await?; + + info!( + account, + credits = total_amount, + note_outputs = builder_outputs.len(), + fee = exact_fee, + inputs = selected_notes.len(), + total_input, + "Shielded multi-output transfer" + ); + + let mut pending_entry = None; + let result = async { + let (spends, anchor) = extract_spends_and_anchor(sdk, store, &selected_notes).await?; + let anchor_bytes = anchor.to_bytes(); + + // Build + prove under a pre-broadcast panic guard: a prover panic here must become a + // DEFINITIVE error (broadcast provably never happened) so the outer failure arm + // releases the reservation — otherwise the panic unwinds to the FFI boundary, which + // keeps the (anchor-less) reservation under its ambiguous-outcome contract and no + // sync can ever free the notes (`stale_pending_spends` skips unarmed reservations). + let (state_transition, fee_used) = catch_pre_broadcast_panic("transfer_multi", || { + build_shielded_transfer_transition_multi( + spends, + &builder_outputs, + &change_addr, + &keys.full_viewing_key, + &keys.spend_auth_key, + anchor, + prover, + sdk.version(), + ) + })? + .map_err(|e| PlatformWalletError::ShieldedBuildError(e.to_string()))?; + debug_assert_eq!( + fee_used, exact_fee, + "builder fee must match the reserved minimum fee" + ); + + // One activity row for the whole transition. + // + // Kind / amount / counterparty come from the shared + // `live_transfer_multi_row` rule (activity.rs): partition the + // requested outputs into EXTERNAL payments vs WALLET-OWNED receipts + // with this account's `IncomingViewingKey::diversifier_index` — the + // same ownership test `coordinator::is_own_orchard_recipient` feeds + // the cold-restore deriver — then derive the row from the external + // subset only, with the counterparty through the shared + // `unanimous_bytes` rule over the canonical 43-byte raw address + // encodings. The `live_and_restored_agree_*` parity tests + // (activity.rs) run THIS function against the deriver, so the two + // paths cannot drift apart silently (#4312 review finding + // 379da4cc0ad0). + // + // The public multi-transfer API accepts ANY valid Orchard address, + // including this account's own diversified ones. Without the split, + // 10 credits to an external address plus 20 to an own address + // recorded live as a 30-credit `Sent` with no counterparty but + // restored as a 10-credit `Sent` to the external address, and an + // all-own output set recorded live as `Sent` but restored as a + // shielded spend. + let (kind, amount, counterparty) = + live_transfer_multi_row(&views.incoming_viewing_key, outputs, fee_used); + + // No memo is claimed on a row that paid no one: the deriver's + // own-only arms record none, and the only credits that left the + // pool are the fee. + let entry_memo = match kind { + ShieldedActivityKind::Sent => non_zero_memo(&memo), + _ => None, + }; + + pending_entry = record_pending_activity( + store, + persister, + wallet_id, + id, + &views, + LiveEntryParams { + kind, + direction: ShieldedDirection::Out, + amount, + fee: Some(fee_used), + counterparty, + memo: entry_memo, + actions: shielded_actions(&state_transition), + spent_notes: &selected_notes, + }, + ) + .await; + arm_pending_release(store, id, anchor_bytes, &pending_entry, &selected_notes).await; + + trace!("Shielded multi-output transfer: state transition built, broadcasting..."); + broadcast_shielded_spend_with_redrive( + sdk, + store, + id, + &pending_entry, + anchor_bytes, + &selected_notes, + &state_transition, + "transfer_multi", + ) + .await + } + .await; + + match result { + Ok(()) => { + record_activity_status( + store, + persister, + wallet_id, + id, + &pending_entry, + ShieldedActivityStatus::Confirmed, + None, + ) + .await; + if let Err(e) = finalize_pending(store, persister, wallet_id, id, &selected_notes).await + { + warn!( + account, + error = %e, + "Shielded multi-output transfer broadcast succeeded but local spent-state \ + update failed; will heal on next sync" + ); + } + info!( + account, + credits = total_amount, + "Shielded multi-output transfer broadcast succeeded" + ); + Ok(()) + } + // Ambiguous post-broadcast confirmation failure: leave the reservation (and the Pending + // activity row) in place — a later scan flips it to Confirmed (see `unshield`). + Err(e @ PlatformWalletError::ShieldedSpendUnconfirmed { .. }) => Err(e), + Err(e) => { + record_activity_status( + store, + persister, + wallet_id, + id, + &pending_entry, + ShieldedActivityStatus::Failed, + None, + ) + .await; + cancel_pending(store, id, &selected_notes).await; + Err(e) + } + } +} + // ------------------------------------------------------------------------- // Withdraw: shielded pool -> Core L1 address (Type 19) // ------------------------------------------------------------------------- @@ -1061,8 +1328,17 @@ pub async fn withdraw( // `ShieldedFeeKind::Withdrawal` — reserving the base fee here would under-fund the document // cost and the builder would reject the spend (and the `fee_used == exact_fee` debug assert // below would fire). - let (selected_notes, total_input, exact_fee) = - reserve_unspent_notes(sdk, store, id, amount, 2, ShieldedFeeKind::Withdrawal).await?; + let (selected_notes, total_input, exact_fee) = reserve_unspent_notes( + sdk, + store, + id, + amount, + 2, + ShieldedFeeKind::Withdrawal, + // `build_shielded_withdrawal_transition` likewise accepts zero change. + ChangeRequirement::Optional, + ) + .await?; info!( account, @@ -1088,21 +1364,27 @@ pub async fn withdraw( // The builder computes and returns the fee authoritatively; `exact_fee` (== the // minimum) was already used above for note reservation. - let (state_transition, fee_used) = build_shielded_withdrawal_transition( - spends, - amount, - output_script, - core_fee_per_byte, - // Consensus pins shielded-withdrawal pooling to Never (validate_structure). - Pooling::Never, - &change_addr, - &keys.full_viewing_key, - &keys.spend_auth_key, - anchor, - prover, - [0u8; 36], - sdk.version(), - ) + // + // Build + prove runs under the pre-broadcast panic guard (see + // `catch_pre_broadcast_panic`): a prover panic must release the reservation rather than + // unwind past these match arms into the FFI guard's ambiguous contract. + let (state_transition, fee_used) = catch_pre_broadcast_panic("withdraw", || { + build_shielded_withdrawal_transition( + spends, + amount, + output_script, + core_fee_per_byte, + // Consensus pins shielded-withdrawal pooling to Never (validate_structure). + Pooling::Never, + &change_addr, + &keys.full_viewing_key, + &keys.spend_auth_key, + anchor, + prover, + [0u8; 36], + sdk.version(), + ) + })? .map_err(|e| PlatformWalletError::ShieldedBuildError(e.to_string()))?; // The builder's fee and the wallet's reserved `exact_fee` both come from // compute_shielded_withdrawal_fee with the same action count; lock that they agree. @@ -1291,21 +1573,32 @@ where // this anchor is pruned from Platform's recorded set. let anchor_bytes = anchor.to_bytes(); - let build = build_identity_create_from_shielded_pool_transition( - public_keys, - denomination, - send_to_address_on_creation_failure, - spends, - &change_addr, - &keys.full_viewing_key, - &keys.spend_auth_key, - anchor, - prover, - identity_signer, - [0u8; 36], - sdk.version(), + // Build + prove runs under the ASYNC pre-broadcast panic guard (see + // `catch_pre_broadcast_panic_async`). This builder is a future — it awaits the per-key + // proof-of-possession signer, which on a host build is a foreign callback — but it still + // runs strictly before the broadcast below, so a panic on any of its polls is provably + // pre-broadcast and must become a DEFINITIVE error the outer failure arm releases the + // reservation on, rather than unwinding past these match arms into the FFI guard's + // ambiguous contract (which strands the anchor-less reservation for the process + // lifetime). + let build = catch_pre_broadcast_panic_async( + "identity_create", + build_identity_create_from_shielded_pool_transition( + public_keys, + denomination, + send_to_address_on_creation_failure, + spends, + &change_addr, + &keys.full_viewing_key, + &keys.spend_auth_key, + anchor, + prover, + identity_signer, + [0u8; 36], + sdk.version(), + ), ) - .await + .await? .map_err(|e| PlatformWalletError::ShieldedBuildError(e.to_string()))?; let identity_id = build.identity_id; @@ -1916,13 +2209,15 @@ async fn reserve_unspent_notes( amount: u64, outputs: usize, fee_kind: ShieldedFeeKind, + change: ChangeRequirement, ) -> Result<(Vec, u64, u64), PlatformWalletError> { let mut store = store.write().await; 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(); + select_notes_with_fee(&unspent, amount, outputs, fee_kind, change, sdk.version())? + .into_owned(); for note in &selected { store .mark_pending(id, ¬e.nullifier) @@ -1998,6 +2293,72 @@ async fn cancel_pending( } } +/// Run a synchronous build/prove `step` that executes strictly BEFORE any broadcast, +/// converting a panic into a definitive [`PlatformWalletError::ShieldedBuildError`]. +/// +/// Why: a panic that unwinds out of an operation skips its `match result` arms, so +/// `cancel_pending` never runs and the FFI boundary's panic guard has no choice but the +/// conservative ambiguous contract ("the spend may have broadcast — keep the reservation"). +/// For a reservation that was never armed with an anchor that contract is a trap: +/// `stale_pending_spends` skips anchor-less reservations, so no later sync can release the +/// notes — they stay unselectable for the rest of the process lifetime, and the host is +/// explicitly told not to retry. A panic INSIDE the build/prove step, by contrast, is +/// provably pre-broadcast (the broadcast call sits strictly after it in the same operation), +/// so it is safe — and required — to convert it to a definitive error here, while the +/// operation still owns the selected notes: the operation's ordinary failure arm then +/// releases the reservation via `cancel_pending`. Panics during or after broadcast are NOT +/// caught by this helper; those remain genuinely ambiguous and keep the FFI guard's +/// conservative contract. +fn catch_pre_broadcast_panic( + operation: &'static str, + step: impl FnOnce() -> T, +) -> Result { + std::panic::catch_unwind(std::panic::AssertUnwindSafe(step)) + .map_err(|payload| pre_broadcast_panic_error(operation, payload)) +} + +/// `async` sibling of [`catch_pre_broadcast_panic`] for an operation whose build/prove step is a +/// future (`identity_create_from_shielded_pool`, whose builder awaits the per-key +/// proof-of-possession signer). +/// +/// Same contract, same conversion — only the shape of the guarded step differs. `catch_unwind` +/// cannot wrap a future's *polls* from the outside, so this drives the future through +/// `FutureExt::catch_unwind`, which applies the guard to every poll: a panic raised on any poll +/// of the builder (Halo 2 synthesis, note bookkeeping, a host signer callback) is caught while +/// the operation still owns the reservation. `AssertUnwindSafe` is the same assertion the +/// synchronous guard makes and is sound for the same reason: the caught panic is converted into +/// a definitive error whose failure arm RELEASES the shared state (the reservation) rather than +/// continuing to use it. +async fn catch_pre_broadcast_panic_async( + operation: &'static str, + step: impl std::future::Future, +) -> Result { + use futures::FutureExt; + + std::panic::AssertUnwindSafe(step) + .catch_unwind() + .await + .map_err(|payload| pre_broadcast_panic_error(operation, payload)) +} + +/// The single panic-payload -> definitive-error conversion both pre-broadcast guards funnel +/// through, so the synchronous and asynchronous entry points cannot drift in the error they +/// produce (both must stay a `ShieldedBuildError`, the class +/// [`error_releases_note_reservation`] releases on). +fn pre_broadcast_panic_error( + operation: &'static str, + payload: Box, +) -> PlatformWalletError { + let message = payload + .downcast_ref::<&str>() + .map(|s| (*s).to_string()) + .or_else(|| payload.downcast_ref::().cloned()) + .unwrap_or_else(|| "non-string panic payload".to_string()); + PlatformWalletError::ShieldedBuildError(format!( + "{operation}: build/prove panicked before broadcast: {message}" + )) +} + /// Record the recorded `anchor` the spend was built against and the /// linked activity entry on every selected note's reservation, so a /// spend that ends broadcast-accepted-but-unconfirmed can be released @@ -2963,6 +3324,451 @@ mod note_reservation_release_tests { } } +#[cfg(test)] +mod pre_broadcast_panic_guard_tests { + use super::*; + use crate::wallet::shielded::store::InMemoryShieldedStore; + + /// A panic inside the guarded build/prove step becomes a DEFINITIVE `ShieldedBuildError` + /// carrying the operation and the panic message — the class of error whose failure arm + /// releases the note reservation (see `error_releases_note_reservation`). + #[test] + fn prover_panic_maps_to_a_releasing_build_error() { + let err = catch_pre_broadcast_panic::<()>("transfer_multi", || { + panic!("halo2 constraint system failure") + }) + .expect_err("a panicking step must surface as an error"); + match &err { + PlatformWalletError::ShieldedBuildError(m) => { + assert!(m.contains("transfer_multi"), "missing operation: {m}"); + assert!( + m.contains("halo2 constraint system failure"), + "missing panic message: {m}" + ); + assert!(m.contains("before broadcast"), "missing provenance: {m}"); + } + other => panic!("expected ShieldedBuildError, got: {other:?}"), + } + assert!( + error_releases_note_reservation(&err), + "the converted error must be one that RELEASES the reservation — the whole point \ + of catching pre-broadcast" + ); + } + + /// Owned-String panic payloads (`panic!("{x}")` formatting) are preserved too. + #[test] + fn string_panic_payloads_are_preserved() { + let position = 7u64; + let err = catch_pre_broadcast_panic::<()>("transfer_multi", || { + panic!("witness for position {position} out of range") + }) + .expect_err("a panicking step must surface as an error"); + match err { + PlatformWalletError::ShieldedBuildError(m) => { + assert!(m.contains("witness for position 7"), "got: {m}"); + } + other => panic!("expected ShieldedBuildError, got: {other:?}"), + } + } + + /// A non-panicking step passes its value through untouched. + #[test] + fn success_passes_through() { + let out = catch_pre_broadcast_panic("transfer_multi", || 41 + 1) + .expect("a non-panicking step must succeed"); + assert_eq!(out, 42); + } + + /// End of the chain the guard exists for: once the definitive error reaches the operation's + /// failure arm, `cancel_pending` releases the reservation and the notes are SELECTABLE + /// again — unlike the unwinding path, where the anchor-less reservation was unreleasable + /// for the process lifetime (`stale_pending_spends` skips unarmed reservations). + #[tokio::test] + async fn cancel_pending_makes_reserved_notes_selectable_again() { + let store = Arc::new(RwLock::new(InMemoryShieldedStore::new())); + let id = SubwalletId::new([0xEE; 32], 0); + let note = ShieldedNote { + position: 0, + cmx: [1u8; 32], + nullifier: [2u8; 32], + block_height: 10, + is_spent: false, + value: 5_000, + note_data: vec![0u8; 115], + }; + { + let mut s = store.write().await; + s.save_note(id, ¬e).unwrap(); + s.mark_pending(id, ¬e.nullifier).unwrap(); + assert!( + s.get_unspent_notes(id).unwrap().is_empty(), + "a reserved note must be excluded from selection" + ); + } + + cancel_pending(&store, id, std::slice::from_ref(¬e)).await; + + let unspent = store.read().await.get_unspent_notes(id).unwrap(); + assert_eq!( + unspent.len(), + 1, + "a cancelled reservation must make the note selectable again" + ); + } +} + +/// OPERATION-level coverage for the pre-broadcast panic guard: every note-spending operation is +/// driven for real with a prover that panics, and the notes it reserved must be selectable again +/// by the time it returns. +/// +/// The helper-level tests above pin the conversion; these pin that each operation actually +/// APPLIES it. They run the real `transfer` / `unshield` / `withdraw` / +/// `identity_create_from_shielded_pool` bodies over a real SQLite commitment tree — so note +/// selection, the anchor probe and the builder all execute for real — with only the recorded- +/// anchor fetch served by the mock SDK. Before the guard was extended to these four paths the +/// prover panic unwound straight out of the operation, skipping the failure arm that calls +/// `cancel_pending`, and each test here failed with that panic instead of an error; the +/// reservation left behind was never armed with an anchor, which `stale_pending_spends` skips, +/// so nothing could release it for the rest of the process lifetime. +#[cfg(test)] +mod operation_pre_broadcast_panic_tests { + use super::*; + use crate::wallet::shielded::file_store::FileBackedShieldedStore; + use dash_sdk::query_types::{NoParamQuery, ShieldedAnchors}; + use dashcore::Network; + use dpp::identity::identity_public_key::v0::IdentityPublicKeyV0; + use dpp::identity::{KeyType, Purpose, SecurityLevel}; + use dpp::platform_value::BinaryData; + use dpp::state_transition::public_key_in_creation::v0::IdentityPublicKeyInCreationV0; + use grovedb_commitment_tree::{ + ExtractedNoteCommitment, Note, NoteValue, ProvingKey, RandomSeed, Rho, + }; + + const WALLET_ID: WalletId = [0x5A; 32]; + const ACCOUNT: u32 = 0; + /// 0.6 DASH in credits — comfortably above every amount + fee exercised below, and above + /// the identity-create denomination, so note selection always succeeds and the operations + /// reach their build/prove step. + const NOTE_VALUE: u64 = 60_000_000_000; + /// 0.1 DASH in credits: a member of the versioned exit-denomination set. + const DENOMINATION: u64 = 10_000_000_000; + + /// An `OrchardProver` whose proving-key access panics. + /// + /// This is the faithful injection point: `prove_and_sign_bundle` evaluates + /// `prover.proving_key()` after `Builder::build` has fixed the action set and before + /// `create_proof` — i.e. deep inside the guarded build/prove step and strictly before every + /// one of these operations broadcasts anything. + struct PanicProver; + + impl dpp::shielded::builder::OrchardProver for PanicProver { + fn proving_key(&self) -> &ProvingKey { + panic!("halo2 proving key unavailable") + } + } + + /// Fixed 65-byte proof-of-possession stub for the identity-create key set. The builder + /// fills (and never verifies) these signatures, and the panic fires before the signing loop + /// is reached anyway. + #[derive(Debug)] + struct DummyIdentitySigner; + + #[async_trait::async_trait] + impl Signer for DummyIdentitySigner { + async fn sign( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + Ok(BinaryData::new(vec![0u8; 65])) + } + + async fn sign_create_witness( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + Err(dpp::ProtocolError::ShieldedBuildError( + "identity PoP signer never creates address witnesses".to_string(), + )) + } + + fn can_sign_with(&self, _key: &IdentityPublicKey) -> bool { + true + } + } + + /// One AUTHENTICATION/MASTER ECDSA key in both forms the identity-create builder takes. + fn key_pair() -> (IdentityPublicKey, IdentityPublicKeyInCreation) { + let public = IdentityPublicKey::V0(IdentityPublicKeyV0 { + id: 0, + purpose: Purpose::AUTHENTICATION, + security_level: SecurityLevel::MASTER, + contract_bounds: None, + key_type: KeyType::ECDSA_SECP256K1, + read_only: false, + data: BinaryData::new(vec![0xAB; 33]), + disabled_at: None, + }); + let in_creation = IdentityPublicKeyInCreation::V0(IdentityPublicKeyInCreationV0 { + id: 0, + key_type: KeyType::ECDSA_SECP256K1, + purpose: Purpose::AUTHENTICATION, + security_level: SecurityLevel::MASTER, + contract_bounds: None, + read_only: false, + data: BinaryData::new(vec![0xAB; 33]), + signature: BinaryData::new(vec![]), + }); + (public, in_creation) + } + + /// Serialize an Orchard note into the 115-byte store form. + /// + /// Mirrors `sync.rs`'s `serialize_note` (private to that module); [`deserialize_note`] is + /// the parser the operations run it back through, and `fixture` asserts the round trip so + /// this copy cannot drift from the canonical layout undetected. + fn serialize_note(note: &Note) -> Vec { + let mut data = Vec::with_capacity(115); + data.extend_from_slice(¬e.recipient().to_raw_address_bytes()); + data.extend_from_slice(¬e.value().inner().to_le_bytes()); + data.extend_from_slice(¬e.rho().to_bytes()); + data.extend_from_slice(note.rseed().as_bytes()); + data + } + + /// Unique temp path for a test tree (no `tempfile` dev-dep — the convention `file_store`'s + /// own tests use). + fn temp_tree_path(tag: &str) -> std::path::PathBuf { + let nanos = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("system clock after the unix epoch") + .as_nanos(); + std::env::temp_dir().join(format!("shielded_panic_guard_{tag}_{nanos}.sqlite")) + } + + /// A wallet holding exactly one real, witnessable note over a real SQLite commitment tree, + /// with the mock SDK serving that tree's own root as the single recorded anchor — everything + /// the operations need to get past note selection and the anchor probe and into build/prove. + struct Fixture { + sdk: Arc, + store: Arc>, + keys: OrchardKeySet, + id: SubwalletId, + path: std::path::PathBuf, + } + + impl Drop for Fixture { + fn drop(&mut self) { + let _ = std::fs::remove_file(&self.path); + } + } + + async fn fixture(tag: &str) -> Fixture { + let keys = OrchardKeySet::from_seed(&[0x42u8; 32], Network::Testnet, ACCOUNT) + .expect("ZIP-32 derivation from a fixed seed should succeed"); + let id = SubwalletId::new(WALLET_ID, ACCOUNT); + + // A real Orchard note paying this account's own default address, so the builders' + // `add_spend` accepts it under `keys.full_viewing_key`. + let rho: Rho = + Option::from(Rho::from_bytes(&[0u8; 32])).expect("zero is a valid pallas::Base"); + let rseed: RandomSeed = + Option::from(RandomSeed::from_bytes([1u8; 32], &rho)).expect("valid random seed"); + let note: Note = Option::from(Note::from_parts( + keys.default_address, + NoteValue::from_raw(NOTE_VALUE), + rho, + rseed, + )) + .expect("note commitment should be valid"); + + let cmx = ExtractedNoteCommitment::from(note.commitment()); + let note_data = serialize_note(¬e); + assert!( + deserialize_note(¬e_data).is_some(), + "the test note serialization must round-trip through the parser the operations use" + ); + + let path = temp_tree_path(tag); + let mut store = FileBackedShieldedStore::open_path(&path, 100).expect("open test tree"); + store + .append_commitment(&cmx.to_bytes(), true) + .expect("append the note commitment"); + store.checkpoint_tree(1).expect("checkpoint the tree"); + store + .save_note( + id, + &ShieldedNote { + position: 0, + cmx: cmx.to_bytes(), + nullifier: note.nullifier(&keys.full_viewing_key).to_bytes(), + block_height: 1, + is_spent: false, + value: NOTE_VALUE, + note_data, + }, + ) + .expect("save the note"); + + // The anchor each operation derives is the root the note's OWN witness computes + // (`select_recorded_spends`), so serve exactly that as the recorded set — otherwise the + // probe fails with `ShieldedNoRecordedAnchor` long before the prover is reached. + let witness = store + .witness_at_depth(0, 0) + .expect("witness query should succeed") + .expect("a marked, checkpointed note must be witnessable at depth 0"); + let anchor = witness.root(cmx).to_bytes(); + + // Pin the newest protocol version: an unpinned mock seeds at its network's protocol + // FLOOR, whose `shielded_identity_create_denominations` set is still empty, so + // identity-create would be rejected on the denomination long before the prover ran. + let mut sdk = dash_sdk::SdkBuilder::new_mock() + .with_version(PlatformVersion::latest()) + .build() + .expect("mock sdk should build"); + sdk.mock() + .expect_fetch::( + NoParamQuery {}, + Some(ShieldedAnchors(vec![anchor])), + ) + .await + .expect("recorded-anchor expectation should register"); + + Fixture { + sdk: Arc::new(sdk), + store: Arc::new(RwLock::new(store)), + keys, + id, + path, + } + } + + /// The shared assertion: the prover panic surfaced as a DEFINITIVE, reservation-releasing + /// error naming this operation, and the note it had reserved is selectable again. + async fn assert_panic_released_the_reservation( + fx: &Fixture, + result: Result, + operation: &str, + ) { + let err = result.expect_err("a panicking prover must surface as an error, not a value"); + match &err { + PlatformWalletError::ShieldedBuildError(m) => { + assert!(m.contains(operation), "error must name the operation: {m}"); + assert!( + m.contains("before broadcast"), + "error must record that the panic was provably pre-broadcast: {m}" + ); + assert!( + m.contains("halo2 proving key unavailable"), + "error must carry the prover's panic message — otherwise the build failed \ + before the prover was ever reached and this test proves nothing: {m}" + ); + } + other => panic!("expected a definitive ShieldedBuildError, got {other:?}"), + } + assert!( + error_releases_note_reservation(&err), + "the converted error must be one whose failure arm releases the reservation" + ); + + let unspent = fx + .store + .read() + .await + .get_unspent_notes(fx.id) + .expect("unspent notes"); + assert_eq!( + unspent.len(), + 1, + "the reserved note must be SELECTABLE again after a pre-broadcast prover panic: the \ + reservation was never armed with an anchor, and `stale_pending_spends` skips \ + anchor-less entries, so if the operation does not release it here nothing ever can" + ); + } + + #[tokio::test] + async fn transfer_prover_panic_releases_the_note_reservation() { + let fx = fixture("transfer").await; + let recipient = fx.keys.default_address; + let result = transfer( + &fx.sdk, + &fx.store, + None, + WALLET_ID, + &fx.keys, + ACCOUNT, + &recipient, + 1_000_000_000, + [0u8; 36], + &PanicProver, + ) + .await; + assert_panic_released_the_reservation(&fx, result, "transfer").await; + } + + #[tokio::test] + async fn unshield_prover_panic_releases_the_note_reservation() { + let fx = fixture("unshield").await; + let result = unshield( + &fx.sdk, + &fx.store, + None, + WALLET_ID, + &fx.keys, + ACCOUNT, + &PlatformAddress::P2pkh([0xCD; 20]), + 1_000_000_000, + &PanicProver, + ) + .await; + assert_panic_released_the_reservation(&fx, result, "unshield").await; + } + + #[tokio::test] + async fn withdraw_prover_panic_releases_the_note_reservation() { + let fx = fixture("withdraw").await; + let to = dashcore::Address::dummy(Network::Testnet, 42); + let result = withdraw( + &fx.sdk, + &fx.store, + None, + WALLET_ID, + &fx.keys, + ACCOUNT, + &to, + 1_000_000_000, + 1, + &PanicProver, + ) + .await; + assert_panic_released_the_reservation(&fx, result, "withdraw").await; + } + + /// The async sibling: identity-create's builder is a future, so its panic is caught by + /// `catch_pre_broadcast_panic_async` rather than the synchronous guard. Same contract. + #[tokio::test] + async fn identity_create_prover_panic_releases_the_note_reservation() { + let fx = fixture("identity_create").await; + let result = identity_create_from_shielded_pool( + &fx.sdk, + &fx.store, + None, + WALLET_ID, + &fx.keys, + ACCOUNT, + vec![key_pair()], + DENOMINATION, + PlatformAddress::P2pkh([0xCD; 20]), + &DummyIdentitySigner, + &PanicProver, + ) + .await; + assert_panic_released_the_reservation(&fx, result, "identity_create").await; + } +} + #[cfg(test)] mod record_activity_status_tests { use super::*; diff --git a/packages/rs-unified-sdk-jni/src/funding.rs b/packages/rs-unified-sdk-jni/src/funding.rs index f8dc82f050a..03ba6702713 100644 --- a/packages/rs-unified-sdk-jni/src/funding.rs +++ b/packages/rs-unified-sdk-jni/src/funding.rs @@ -41,7 +41,7 @@ use crate::pubkey_rows::decode_registration_pubkeys_blob; use crate::support::{guard, take_pwffi_error, throw_sdk_exception, JVM}; -use jni::objects::{GlobalRef, JByteArray, JClass, JObject, JString}; +use jni::objects::{GlobalRef, JByteArray, JClass, JLongArray, JObject, JString}; use jni::sys::{jboolean, jint, jlong, JNI_FALSE, JNI_TRUE}; use jni::JNIEnv; use platform_wallet_ffi::handle::Handle; @@ -226,11 +226,15 @@ fn read_cstring_required(env: &mut JNIEnv, s: &JString, field: &str) -> Option`. JVM null (or an -/// empty string) → `Ok(None)` — the FFI treats a null memo pointer as "no -/// memo", and Rust's `encode_memo_text` maps an empty string to the same -/// all-zero memo anyway, so both normalize to null here. Returns `Err(())` -/// (after throwing) on an interior NUL; a JNI read error is treated as null. +/// Read an OPTIONAL Java `String` into `Option`. ONLY a JVM null or +/// an empty string means "absent" (`Ok(None)`) — the FFI treats a null memo +/// pointer as "no memo", and Rust's `encode_memo_text` maps an empty string to +/// the same all-zero memo anyway, so both normalize to null here. Returns +/// `Err(())` (after throwing) on an interior NUL **or on a JNI read failure**: +/// a non-null string the JVM could not hand over must abort the call, not +/// silently degrade to "absent" — the callers are irreversible spend paths, +/// and dropping a requested memo would prove and broadcast the transfer +/// without it. fn read_cstring_opt( env: &mut JNIEnv, s: &JString, @@ -243,7 +247,8 @@ fn read_cstring_opt( Ok(v) => v.into(), Err(_) => { let _ = env.exception_clear(); - return Ok(None); + throw_sdk_exception(env, 1, &format!("{field} String could not be read")); + return Err(()); } }; if owned.is_empty() { @@ -845,6 +850,151 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_FundingNative_shielde }) } +/// Multi-output shielded → shielded transfer (Type 16) — bridges +/// `platform_wallet_manager_shielded_transfer_multi`. +/// +/// `recipientsRaw43` is `amounts.length` raw 43-byte Orchard addresses laid +/// out back to back (so its length must be `43 * amounts.length`), and +/// `amounts` holds the matching credit amounts. Each pair becomes its own +/// note; repeating the same address funds that address with several +/// independent notes. `memoText` is attached to every recipient note. +/// +/// At most `platform_wallet_ffi::MAX_SHIELDED_TRANSFER_RECIPIENTS` recipients +/// are accepted, and that ceiling is enforced from the Java array LENGTHS +/// before either array is copied into a native buffer — so no allocation is +/// ever sized by an unvalidated caller-supplied count. +#[no_mangle] +pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_FundingNative_shieldedTransferMulti( + mut env: JNIEnv, + _class: JClass, + manager_handle: jlong, + wallet_id: JByteArray, + resolver_handle: jlong, + account: jint, + recipients_raw43: JByteArray, + amounts: JLongArray, + memo_text: JString, +) { + guard(&mut env, (), |env| { + if account < 0 { + throw_sdk_exception(env, 1, "account must be non-negative"); + return; + } + let Some(wid) = read_id32(env, &wallet_id, "walletId") else { + return; + }; + if recipients_raw43.is_null() { + throw_sdk_exception(env, 1, "recipientsRaw43 byte[] was null"); + return; + } + if amounts.is_null() { + throw_sdk_exception(env, 1, "amounts long[] was null"); + return; + } + // Establish the recipient count and bound it BEFORE any caller-sized allocation. Both + // `convert_byte_array` (43 bytes per recipient) and the amount buffers below are sized + // from Java-supplied lengths, so an accidental or hostile oversized call would otherwise + // drive several large allocations — and possibly an allocator OOM — on its way to the + // native layer's clean `ErrorInvalidParameter`. `get_array_length` reads a header field + // and allocates nothing. + let amount_len = match env.get_array_length(&amounts) { + Ok(n) if n >= 0 => n as usize, + _ => { + let _ = env.exception_clear(); + throw_sdk_exception(env, 1, "amounts long[] was invalid"); + return; + } + }; + if amount_len == 0 { + throw_sdk_exception(env, 1, "amounts must contain at least one entry"); + return; + } + if amount_len > platform_wallet_ffi::MAX_SHIELDED_TRANSFER_RECIPIENTS { + throw_sdk_exception( + env, + 1, + &format!( + "amounts must hold at most {} entries, got {amount_len}", + platform_wallet_ffi::MAX_SHIELDED_TRANSFER_RECIPIENTS + ), + ); + return; + } + // Same rule for the address blob: verify its LENGTH (a header read) against the bounded + // recipient count before copying it into a native buffer, so the copy that follows is + // bounded by `MAX_SHIELDED_TRANSFER_RECIPIENTS * 43` rather than by the caller. + let recipients_len = match env.get_array_length(&recipients_raw43) { + Ok(n) if n >= 0 => n as usize, + _ => { + let _ = env.exception_clear(); + throw_sdk_exception(env, 1, "recipientsRaw43 byte[] was invalid"); + return; + } + }; + if recipients_len != amount_len * 43 { + throw_sdk_exception( + env, + 1, + &format!( + "recipientsRaw43 must be 43 bytes per amount ({} expected), got {}", + amount_len * 43, + recipients_len + ), + ); + return; + } + let recipients = match env.convert_byte_array(&recipients_raw43) { + Ok(b) => b, + Err(_) => { + let _ = env.exception_clear(); + throw_sdk_exception(env, 1, "recipientsRaw43 byte[] was invalid"); + return; + } + }; + let mut amount_buf = vec![0i64; amount_len]; + if env + .get_long_array_region(&amounts, 0, &mut amount_buf) + .is_err() + { + let _ = env.exception_clear(); + throw_sdk_exception(env, 1, "amounts long[] could not be read"); + return; + } + // Reject sign errors at the boundary — negatives would otherwise bit-cast to huge + // unsigned values (never clamp). + for (index, &amount) in amount_buf.iter().enumerate() { + if amount <= 0 { + throw_sdk_exception( + env, + 1, + &format!("amounts[{index}] must be positive, got {amount}"), + ); + return; + } + } + let amounts_u64: Vec = amount_buf.iter().map(|&a| a as u64).collect(); + + let memo = match read_cstring_opt(env, &memo_text, "memoText") { + Ok(m) => m, + Err(()) => return, + }; + let memo_ptr = memo.as_ref().map_or(ptr::null(), |c| c.as_ptr()); + let result = unsafe { + platform_wallet_ffi::platform_wallet_manager_shielded_transfer_multi( + manager_handle as Handle, + wid.as_ptr(), + resolver_handle as *mut MnemonicResolverHandle, + account as u32, + recipients.as_ptr(), + amounts_u64.as_ptr(), + amount_len, + memo_ptr, + ) + }; + let _ = take_pwffi_error(env, result); + }) +} + /// Shielded → Platform unshield (Type 17) — bridges /// `platform_wallet_manager_shielded_unshield`. ///