From 738dd7c0b52d0b4dc5450f3c9a07a02002e6b31b Mon Sep 17 00:00:00 2001 From: bfoss765 <38437574+bfoss765@users.noreply.github.com> Date: Wed, 5 Aug 2026 12:52:06 -0400 Subject: [PATCH 1/5] feat(shielded): multi-output transfers + output-aware fee predictor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a multi-output ShieldedTransfer so one transition can fund an address with several notes, and fixes the fee predictor that made such a transfer impossible to construct. ## The fee predictor (blocking bug) `build_shielded_transfer_transition` sized its fee from `spends.len().max(2)`, ignoring the output count. 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 (`validate_minimum_shielded_fee` rejects under- AND over-payment), so any transfer publishing three or more outputs would carve `min_fee(2)` while consensus demanded `min_fee(3)` and be rejected on chain. The spends-only form happened to be correct while `num_outputs <= 2`, because `max(n, 1).max(2) == max(n, 2).max(2)` — which is why the single-output builder never hit it. Both builders now size the fee through a shared `shielded_bundle_action_count`, which delegates to Orchard's own `BundleType::num_actions` so the predictor cannot drift from the builder that lays out the bundle. ## Why several outputs Orchard pads any bundle to two actions, and a padding action's dummy nullifier is randomly generated. An identity id derived from published nullifiers is therefore only reproducible offline when at least two REAL notes are spent — with one real note a retry builds a different dummy and a different id. Funding an address with two sub-target notes instead of one full-target note structurally forces a later spend to select BOTH: greedy largest-first selection cannot stop on a note that does not cover the target. That keeps the padding action, and its random nullifier, out of the bundle. `shielded_identity_id_is_reproducible` states that rule as one predicate next to the id derivation it guards, so callers that must recognise an identity their earlier attempt created gate on the note count — no chain lookup, decided before any proving work. ## Shape The multi-output builder ALWAYS emits a change output and 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?". Note selection reserves against the same `recipients + 1` floor, so the reserved and carved fees cannot diverge. Repeating the same address across outputs is allowed and is the point: Orchard derives independent notes regardless. ## Layers - rs-dpp: `shielded_bundle_action_count`, `ShieldedTransferOutput`, `build_shielded_transfer_transition_multi`, `shielded_identity_id_is_reproducible` - rs-platform-wallet: `operations::transfer_multi`, `PlatformWallet::shielded_transfer_multi_to` - rs-platform-wallet-ffi: `platform_wallet_manager_shielded_transfer_multi` - rs-unified-sdk-jni + kotlin-sdk: `shieldedTransferMulti` ## Tests - `multi_output_transfer_fee_matches_on_wire_action_count` builds a REAL 2-spend/3-output bundle and pins `value_balance == fee == min_fee(actions.len()) == min_fee(3)`, asserting it is NOT `min_fee(2)`. - `single_output_transfer_fee_matches_on_wire_action_count` pins the single-output builder against a real bundle so the shared helper cannot regress it. - `shielded_bundle_action_count_*` pin the predictor as `max(spends, outputs)` padded to 2, and against a real bundle's on-wire count. - `test_two_sub_denomination_notes_are_both_selected` / `test_single_full_denomination_note_selects_alone` pin the selector behaviour the two-note layout depends on. - The existing padding tests now also assert `shielded_identity_id_is_reproducible`. Swift parity for the new entry point is a follow-up; the cbindgen header is generated at build time and nothing in the Swift SDK references the new symbol, so the Swift build is unaffected. Co-Authored-By: Claude Opus 4.8 --- .../dashsdk/ffi/FundingNative.kt | 20 + .../dashsdk/wallet/PlatformWalletManager.kt | 55 ++ .../identity_create_from_shielded_pool.rs | 14 + packages/rs-dpp/src/shielded/builder/mod.rs | 88 +++- .../src/shielded/builder/shielded_transfer.rs | 468 +++++++++++++++++- .../mod.rs | 29 ++ .../src/shielded_send.rs | 147 ++++++ .../src/wallet/platform_wallet.rs | 52 ++ .../src/wallet/shielded/note_selection.rs | 80 +++ .../src/wallet/shielded/operations.rs | 197 +++++++- packages/rs-unified-sdk-jni/src/funding.rs | 114 ++++- 11 files changed, 1256 insertions(+), 8 deletions(-) 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 940a9b79639..67ed81b6b4c 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 @@ -1644,6 +1644,61 @@ 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 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" } + 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:)` 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..fe6a720f836 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 @@ -414,6 +414,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 +504,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..42151a50189 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; @@ -103,6 +106,36 @@ 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**. +/// +/// 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. +pub fn shielded_bundle_action_count( + num_spends: usize, + num_outputs: usize, +) -> Result { + 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}" + )) + }) +} + /// Serializes an authorized Orchard bundle into the raw fields used by /// state transition constructors. pub fn serialize_authorized_bundle(bundle: &Bundle) -> SerializedBundle { @@ -781,4 +814,57 @@ 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() { + 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, 7, 7), + ] { + let actual = shielded_bundle_action_count(spends, outputs) + .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 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).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" + ); + } + } } diff --git a/packages/rs-dpp/src/shielded/builder/shielded_transfer.rs b/packages/rs-dpp/src/shielded/builder/shielded_transfer.rs index 3c870f37c3c..c2d0d6c6202 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,14 @@ 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()`. + const MAX_OUTPUTS: usize = 2; // recipient + change + let num_actions = shielded_bundle_action_count(spends.len(), MAX_OUTPUTS)?; // 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 +142,464 @@ 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(), + )); + } + + // 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()) + })?; + let num_actions = shielded_bundle_action_count(spends.len(), num_outputs)?; + 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}" + ); + } + + /// 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}" + ); + } #[test] fn test_shielded_transfer_insufficient_funds() { 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 789995526a7..d2b283f7926 100644 --- a/packages/rs-platform-wallet-ffi/src/shielded_send.rs +++ b/packages/rs-platform-wallet-ffi/src/shielded_send.rs @@ -350,6 +350,153 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_transfer( map_spend_result(result, "shielded transfer") } +/// Defensive upper bound on the recipient count of a multi-output shielded transfer. +/// +/// This is an FFI sanity bound, not the protocol limit: it stops an absurd or corrupt +/// `num_recipients` from driving a huge allocation before anything else can reject it. The real +/// ceiling is the 20 KiB state-transition size limit, which admits roughly six Orchard actions — +/// so a legitimate caller stays far below this. +const MAX_SHIELDED_TRANSFER_RECIPIENTS: usize = 16; + +/// 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 { + 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. /// diff --git a/packages/rs-platform-wallet/src/wallet/platform_wallet.rs b/packages/rs-platform-wallet/src/wallet/platform_wallet.rs index cfb310ea597..75ebc0f5356 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_wallet.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_wallet.rs @@ -1138,6 +1138,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 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..6f27f628526 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/note_selection.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/note_selection.rs @@ -383,6 +383,86 @@ 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, + 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" + ); + } + + /// 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(); diff --git a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs index a79ed4e2d16..d3c3acff301 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs @@ -51,8 +51,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; @@ -989,6 +990,198 @@ 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. + 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, + ) + .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(); + + let (state_transition, fee_used) = 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. The counterparty is only meaningful when + // every output lands on the same address (the fund-an-address-with-N-notes shape); a + // genuine multi-recipient send has no single counterparty to record. + let counterparty = outputs + .first() + .filter(|(first, _)| outputs.iter().all(|(a, _)| a == first)) + .map(|(addr, _)| addr.to_raw_address_bytes().to_vec()); + + pending_entry = record_pending_activity( + store, + persister, + wallet_id, + id, + &views, + LiveEntryParams { + kind: ShieldedActivityKind::Sent, + direction: ShieldedDirection::Out, + amount: total_amount, + fee: Some(fee_used), + counterparty, + memo: non_zero_memo(&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) // ------------------------------------------------------------------------- diff --git a/packages/rs-unified-sdk-jni/src/funding.rs b/packages/rs-unified-sdk-jni/src/funding.rs index f8dc82f050a..a2035b4f33a 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; @@ -845,6 +845,118 @@ 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. +#[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; + } + 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 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 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 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`. /// From 6e59784b295210c857decc6204194023e993dc5a Mon Sep 17 00:00:00 2001 From: bfoss765 <38437574+bfoss765@users.noreply.github.com> Date: Wed, 5 Aug 2026 18:20:03 -0400 Subject: [PATCH 2/5] =?UTF-8?q?fix(shielded):=20review-gate=20round=20?= =?UTF-8?q?=E2=80=94=20action-limit=20gate,=20strict-change=20note=20selec?= =?UTF-8?q?tion,=20FFI=20panic=20guard,=20JNI=20allocation=20bound?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the four findings on dashpay/platform#4301 (2 blocking, 2 suggestions). ## BLOCKING — reject bundles over the consensus action limit before proving `shielded_bundle_action_count` computed the on-wire action count but never compared it with `platform_version.system_limits.max_shielded_transition_actions` (16). `ShieldedTransferTransitionV0::validate_structure` rejects anything above that limit, while `try_from_bundle` performs no structural validation — so the FFI's 16 recipients (17 outputs once the unconditional change output is added, therefore >= 17 actions), or a fragmented wallet's spend count, would build and prove a bundle (~30 s of Halo 2) that consensus is guaranteed to reject. The helper now takes `platform_version` and validates the computed count. Because the count is `max(spends, outputs)` padded to 2, the single comparison bounds BOTH sides. Both transfer builders route through it, so the rejection happens before any spend is added to the Orchard builder. ## BLOCKING — reserve enough input to guarantee positive change `select_notes_with_fee` accepted `total_input == amount + exact_fee`, but `build_shielded_transfer_transition_multi` emits an unconditional change output and rejects equality. With notes `[amount + fee, 1]`, largest-first selection reserved the exact-coverage note alone and the build then failed even though taking the remaining credit would have satisfied the builder. Note selection now carries a `ChangeRequirement`. `StrictlyPositive` (the multi-output transfer) folds one credit into the selection target and into the sufficiency test on every convergence iteration, so the strict postcondition holds against the RE-COMPUTED fee after an added note changes the action count. The other three spends keep `Optional` — their builders accept zero change. The returned fee stays the pure consensus fee the builder carves. ## SUGGESTION — catch panics before crossing the C ABI A panic cannot unwind through `extern "C"`: it aborts the process before the JNI layer's `support::guard` can turn it into a Java exception. `block_on_worker` makes this reachable — it `.expect`s on the tokio `JoinError`, so a panicking proving task re-panics inside the export. The multi-output transfer export's body moved into a plain Rust function invoked under `catch_unwind`. A caught panic maps to `ErrorShieldedSpendUnconfirmed`, whose contract is exactly the conservative one required: the spend may have been broadcast, the reservation stays, and the host must not auto-retry. ## SUGGESTION — enforce the recipient bound before allocating The JNI adapter copied the whole Java recipient array and both amount buffers before the native ceiling could reject the call. It now reads both array LENGTHS first (header reads, no allocation), rejects counts above `MAX_SHIELDED_TRANSFER_RECIPIENTS` (now public so the bridges share the constant instead of duplicating the literal), and only then converts — so every allocation is bounded by the ceiling, not by the caller. `PlatformWalletManager.shieldedTransferMulti` mirrors the check before it flattens its own buffers. Tests: action-count boundary passes / one over fails fast from both the output and spend sides (helper + builder level); the `[amount + fee, 1]` exact-fit case now selects both notes, one credit short reports the extra credit in `required`, and the strict floor survives fee re-convergence; the FFI panic guard maps a panic to the unconfirmed contract and is transparent otherwise. --- .../dashsdk/wallet/PlatformWalletManager.kt | 18 +- packages/rs-dpp/src/shielded/builder/mod.rs | 84 ++++++- .../src/shielded/builder/shielded_transfer.rs | 143 ++++++++++- .../src/shielded_send.rs | 129 +++++++++- .../src/wallet/shielded/note_selection.rs | 227 +++++++++++++++++- .../src/wallet/shielded/operations.rs | 53 +++- packages/rs-unified-sdk-jni/src/funding.rs | 53 +++- 7 files changed, 667 insertions(+), 40 deletions(-) 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 67ed81b6b4c..362e5dc4bae 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 @@ -1662,7 +1662,8 @@ class PlatformWalletManager( * * @param walletId the 32-byte wallet id. * @param outputs (raw 43-byte Orchard address, credits) pairs; must be - * non-empty and every amount must be positive. + * non-empty, hold at most 16 entries (the native ceiling), 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). @@ -1674,6 +1675,12 @@ class PlatformWalletManager( 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) { @@ -2277,6 +2284,15 @@ 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. + */ + const val MAX_SHIELDED_TRANSFER_RECIPIENTS = 16 + /** De-offset `PlatformWalletFFIResultCode::ErrorInvalidParameter`. */ const val PWFFI_INVALID_PARAMETER = 2 } diff --git a/packages/rs-dpp/src/shielded/builder/mod.rs b/packages/rs-dpp/src/shielded/builder/mod.rs index 42151a50189..9ab58623507 100644 --- a/packages/rs-dpp/src/shielded/builder/mod.rs +++ b/packages/rs-dpp/src/shielded/builder/mod.rs @@ -55,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; @@ -107,7 +108,8 @@ 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**. +/// 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 @@ -123,17 +125,41 @@ impl From<&OrchardAddress> for PaymentAddress { /// /// 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 ceiling +/// +/// 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`), but the `try_from_bundle` constructors do NOT run structural +/// validation — so without this gate an over-sized bundle is built, proved (~30 s of Halo 2), +/// and only then rejected on chain. 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. pub fn shielded_bundle_action_count( num_spends: usize, num_outputs: usize, + platform_version: &PlatformVersion, ) -> Result { - BundleType::DEFAULT + 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" + ))); + } + + Ok(num_actions) } /// Serializes an authorized Orchard bundle into the raw fields used by @@ -824,6 +850,7 @@ mod mod_tests { /// 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), @@ -836,7 +863,7 @@ mod mod_tests { (5, 3, 5), (3, 7, 7), ] { - let actual = shielded_bundle_action_count(spends, outputs) + let actual = shielded_bundle_action_count(spends, outputs, platform_version) .expect("DEFAULT bundles accept any spend/output mix"); assert_eq!( actual, expected, @@ -850,6 +877,7 @@ mod mod_tests { /// 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] { @@ -857,8 +885,8 @@ mod mod_tests { 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).expect("valid bundle shape"); + let predicted = shielded_bundle_action_count(0, num_outputs, platform_version) + .expect("valid bundle shape"); assert_eq!( bundle.actions().len(), predicted, @@ -867,4 +895,48 @@ mod mod_tests { ); } } + + /// The predictor is also the CONSENSUS gate: `validate_actions_count` rejects + /// `actions.len() > max_shielded_transition_actions`, but `try_from_bundle` runs no + /// structural validation — so a bundle over the ceiling would be proved (~30 s of Halo 2) + /// and only then rejected on chain. The boundary itself must still pass. + #[test] + fn shielded_bundle_action_count_accepts_the_consensus_boundary() { + let platform_version = PlatformVersion::latest(); + let max = platform_version + .system_limits + .max_shielded_transition_actions as usize; + + // Exactly at the ceiling, from each side. + assert_eq!( + shielded_bundle_action_count(1, max, platform_version) + .expect("the output-side boundary must be accepted"), + max + ); + assert_eq!( + shielded_bundle_action_count(max, 1, platform_version) + .expect("the spend-side boundary must be accepted"), + max + ); + } + + /// One action over the ceiling must fail fast — from the OUTPUT side (the 16-recipient FFI + /// call, which becomes 17 outputs once the unconditional change output is added) and from + /// the SPEND side (a fragmented wallet selecting too many notes). + #[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, 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/shielded_transfer.rs b/packages/rs-dpp/src/shielded/builder/shielded_transfer.rs index c2d0d6c6202..365e1f085e8 100644 --- a/packages/rs-dpp/src/shielded/builder/shielded_transfer.rs +++ b/packages/rs-dpp/src/shielded/builder/shielded_transfer.rs @@ -63,8 +63,12 @@ pub fn build_shielded_transfer_transition( // 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)?; + let num_actions = shielded_bundle_action_count(spends.len(), MAX_OUTPUTS, 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). @@ -238,7 +242,11 @@ pub fn build_shielded_transfer_transition_multi( let num_outputs = outputs.len().checked_add(1).ok_or_else(|| { ProtocolError::ShieldedBuildError("output count overflows usize".to_string()) })?; - let num_actions = shielded_bundle_action_count(spends.len(), num_outputs)?; + // 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, platform_version)?; let fee = compute_minimum_shielded_fee(num_actions, platform_version)?; let required = transfer_total.checked_add(fee).ok_or_else(|| { @@ -601,6 +609,137 @@ mod tests { ); } + // -------------------------------------------------------------- + // 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. This is exactly what the + // FFI's 16-recipient ceiling admits today. + 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 boundary itself must still build: `max - 1` recipients plus change is exactly + /// `max_shielded_transition_actions` actions. The gate must not 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 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 outputs = n_outputs(max - 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"), + "exactly {max} actions is AT the limit and must not be rejected by it, 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() { let platform_version = PlatformVersion::latest(); diff --git a/packages/rs-platform-wallet-ffi/src/shielded_send.rs b/packages/rs-platform-wallet-ffi/src/shielded_send.rs index d2b283f7926..ac036848e5b 100644 --- a/packages/rs-platform-wallet-ffi/src/shielded_send.rs +++ b/packages/rs-platform-wallet-ffi/src/shielded_send.rs @@ -356,7 +356,53 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_transfer( /// `num_recipients` from driving a huge allocation before anything else can reject it. The real /// ceiling is the 20 KiB state-transition size limit, which admits roughly six Orchard actions — /// so a legitimate caller stays far below this. -const MAX_SHIELDED_TRANSFER_RECIPIENTS: usize = 16; +/// +/// 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. +pub const MAX_SHIELDED_TRANSFER_RECIPIENTS: usize = 16; + +/// 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 a shielded-spend export body under [`std::panic::catch_unwind`], converting a panic into a +/// typed FFI error 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 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 { + match std::panic::catch_unwind(std::panic::AssertUnwindSafe(body)) { + Ok(result) => result, + Err(payload) => PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorShieldedSpendUnconfirmed, + format!( + "{operation} panicked: {}. The spend may or may not have been broadcast — do \ + NOT retry; the next shielded sync reconciles the outcome.", + panic_payload_message(payload.as_ref()) + ), + ), + } +} /// Send a shielded → shielded transfer with SEVERAL outputs in one /// atomic transition. @@ -402,6 +448,39 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_transfer_multi( 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); @@ -1614,6 +1693,54 @@ 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. + #[test] + fn catch_spend_panic_maps_a_panic_to_the_unconfirmed_contract() { + let previous = std::panic::take_hook(); + // Silence the default hook's backtrace spew for this deliberate panic. + std::panic::set_hook(Box::new(|_| {})); + let result = catch_spend_panic("shielded multi-output transfer", || { + panic!("tokio worker panicked"); + }); + std::panic::set_hook(previous); + + 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}" + ); + } + /// `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/shielded/note_selection.rs b/packages/rs-platform-wallet/src/wallet/shielded/note_selection.rs index 6f27f628526..18d0a2db811 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,50 @@ 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(), + ) + }) + }; + 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 >= amount.saturating_add(exact_fee).saturating_add(min_change) { return Ok((selected, total, exact_fee)); } @@ -181,17 +233,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 = amount.saturating_add(exact_fee).saturating_add(min_change); + if total < required { return Err(PlatformWalletError::ShieldedInsufficientBalance { available: total, - required: amount.saturating_add(exact_fee), + required, }); } @@ -372,9 +425,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); @@ -409,6 +468,7 @@ mod tests { amount, min_actions, ShieldedFeeKind::Base, + ChangeRequirement::Optional, platform_version, ) .expect("selection ok"); @@ -420,6 +480,141 @@ mod tests { ); } + /// 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:?}"), + } + } + + /// 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. @@ -472,9 +667,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(); @@ -513,6 +714,7 @@ mod tests { amount, 2, ShieldedFeeKind::Withdrawal, + ChangeRequirement::Optional, platform_version, ) .expect("selection ok"); @@ -550,6 +752,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 d3c3acff301..4f04ecf125c 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs @@ -25,7 +25,7 @@ use super::activity_recorder::{ }; 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::changeset::{PlatformWalletChangeSet, ShieldedChangeSet}; @@ -673,8 +673,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, @@ -860,8 +870,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, @@ -1049,6 +1069,11 @@ pub async fn transfer_multi( // 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, @@ -1057,6 +1082,7 @@ pub async fn transfer_multi( total_amount, num_outputs, ShieldedFeeKind::Base, + ChangeRequirement::StrictlyPositive, ) .await?; @@ -1213,8 +1239,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, @@ -2069,13 +2104,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) diff --git a/packages/rs-unified-sdk-jni/src/funding.rs b/packages/rs-unified-sdk-jni/src/funding.rs index a2035b4f33a..a7d1d6cc1ac 100644 --- a/packages/rs-unified-sdk-jni/src/funding.rs +++ b/packages/rs-unified-sdk-jni/src/funding.rs @@ -853,6 +853,11 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_FundingNative_shielde /// `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, @@ -881,14 +886,12 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_FundingNative_shielde throw_sdk_exception(env, 1, "amounts long[] was null"); 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; - } - }; + // 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, _ => { @@ -901,18 +904,48 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_FundingNative_shielde throw_sdk_exception(env, 1, "amounts must contain at least one entry"); return; } - if recipients.len() != amount_len * 43 { + 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() + 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) From d3ecd62f9d823c537faf4f2de6efc0c1f397302d Mon Sep 17 00:00:00 2001 From: bfoss765 Date: Wed, 12 Aug 2026 09:01:05 -0400 Subject: [PATCH 3/5] fix(dpp): enforce the transition-size-derived action ceiling in the shielded pre-proving gate shielded_bundle_action_count only enforced the structural max_shielded_transition_actions cap (16) and ignored the versioned max_state_transition_size (20 KiB). A shielded transition's on-wire size grows ~2,681 B per action on a ~2.9 KiB envelope (measured: 2 actions -> 8,294 B, 6 -> 19,018 B, 7 -> 21,699 B), so 7..16-action bundles passed the gate, burned ~30 s of Halo 2 proving per bundle, and were only then rejected by DAPI's byte prefilter / Tenderdash mempool.max-tx-bytes / the Drive-ABCI consensus decoder. Reachable from the FFI/JNI/Kotlin boundaries, which admitted up to 16 recipients. - shielded/mod.rs: add the measured wire-cost constants (SHIELDED_ACTION_WIRE_BYTES = 408, SHIELDED_PROOF_WIRE_BYTES_PER_ACTION = 2,273, SHIELDED_TRANSITION_WIRE_OVERHEAD_BYTES = 2,932), estimated_shielded_transition_wire_bytes(), and max_shielded_actions_per_transition() - the effective ceiling derived from BOTH versioned limits (min of the structural cap and the largest action count whose estimated size fits max_state_transition_size). 6 at current constants; derived, never hardcoded, so raising max_state_transition_size widens the gate automatically. Pin tests tie the linear model to the measured transitions and the derivation to the value the system_limits doc comments state. - builder/mod.rs: shielded_bundle_action_count now also rejects bundles over the effective ceiling, with a size-derived message naming the estimated byte count (structural-cap check unchanged and still first). - builder routing: shielded_withdrawal / unshield / identity_create_from_shielded_pool swap their ungated spends.len().max(2) for the gated predictor (numerically identical for valid shapes); both shield_from_asset_lock builders gate 1 + dummy_outputs (checked add) before building the bundle. Every shielded builder now fails fast instead of proving a doomed bundle. - tests: output-dominated (1,7), spend-dominated (7,1) and (7,7) shapes rejected pre-proving; (1,6)/(6,1)/(6,6) accepted at the boundary; the multi-output transfer gains a 6-recipient (7-output) pre-proving rejection test and its boundary-accept test moves from the structural cap to the effective ceiling. Validation: cargo test -p dpp --features shielded-client,core_key_wallet,state-transition-signing --lib -> 3931 passed, 0 failed, 6 ignored (includes the 6-action seed_pool_batch_fits_max_state_transition_size signing test through the new gate). --- .../identity_create_from_shielded_pool.rs | 11 +- packages/rs-dpp/src/shielded/builder/mod.rs | 112 ++++++++++---- .../builder/shield_from_asset_lock.rs | 22 ++- .../src/shielded/builder/shielded_transfer.rs | 71 +++++++-- .../shielded/builder/shielded_withdrawal.rs | 11 +- .../rs-dpp/src/shielded/builder/unshield.rs | 11 +- packages/rs-dpp/src/shielded/mod.rs | 138 ++++++++++++++++++ 7 files changed, 327 insertions(+), 49 deletions(-) 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 fe6a720f836..f010608664b 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, shielded_bundle_action_count, + OrchardProver, SpendableNote, +}; /// 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,11 @@ 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. + let num_actions = shielded_bundle_action_count(spends.len(), 1, platform_version)?; let fee = compute_shielded_identity_create_fee(num_actions, public_keys.len(), platform_version)?; diff --git a/packages/rs-dpp/src/shielded/builder/mod.rs b/packages/rs-dpp/src/shielded/builder/mod.rs index 9ab58623507..82acc4892e3 100644 --- a/packages/rs-dpp/src/shielded/builder/mod.rs +++ b/packages/rs-dpp/src/shielded/builder/mod.rs @@ -126,15 +126,29 @@ impl From<&OrchardAddress> for PaymentAddress { /// 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 ceiling +/// # The consensus ceilings /// -/// 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`), but the `try_from_bundle` constructors do NOT run structural -/// validation — so without this gate an over-sized bundle is built, proved (~30 s of Halo 2), -/// and only then rejected on chain. 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. +/// 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_per_transition`], 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. pub fn shielded_bundle_action_count( num_spends: usize, num_outputs: usize, @@ -159,6 +173,18 @@ pub fn shielded_bundle_action_count( ))); } + let effective_max = crate::shielded::max_shielded_actions_per_transition(platform_version); + if num_actions > effective_max { + let estimated = crate::shielded::estimated_shielded_transition_wire_bytes(num_actions); + let max_size = platform_version.system_limits.max_state_transition_size; + 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 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) } @@ -861,7 +887,7 @@ mod mod_tests { (2, 3, 3), (1, 4, 4), (5, 3, 5), - (3, 7, 7), + (3, 6, 6), ] { let actual = shielded_bundle_action_count(spends, outputs, platform_version) .expect("DEFAULT bundles accept any spend/output mix"); @@ -896,33 +922,61 @@ mod mod_tests { } } - /// The predictor is also the CONSENSUS gate: `validate_actions_count` rejects - /// `actions.len() > max_shielded_transition_actions`, but `try_from_bundle` runs no - /// structural validation — so a bundle over the ceiling would be proved (~30 s of Halo 2) - /// and only then rejected on chain. The boundary itself must still pass. + /// 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_consensus_boundary() { + fn shielded_bundle_action_count_accepts_the_effective_boundary() { let platform_version = PlatformVersion::latest(); - let max = platform_version + 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, 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; - - // Exactly at the ceiling, from each side. - assert_eq!( - shielded_bundle_action_count(1, max, platform_version) - .expect("the output-side boundary must be accepted"), - max - ); - assert_eq!( - shielded_bundle_action_count(max, 1, platform_version) - .expect("the spend-side boundary must be accepted"), - max + 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, 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 ceiling must fail fast — from the OUTPUT side (the 16-recipient FFI - /// call, which becomes 17 outputs once the unconditional change output is added) and from - /// the SPEND side (a fragmented wallet selecting too many notes). + /// 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(); 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..026b21b9c44 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,10 @@ 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, shielded_bundle_action_count, + OrchardProver, +}; /// Builds a ShieldFromAssetLock state transition (core asset lock -> shielded pool). /// @@ -46,6 +49,16 @@ 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. + let num_outputs = dummy_outputs.checked_add(1).ok_or_else(|| { + ProtocolError::ShieldedBuildError("dummy_outputs overflows the output count".to_string()) + })?; + shielded_bundle_action_count(0, num_outputs, platform_version)?; + let bundle = build_output_only_bundle( recipient, shield_amount, @@ -127,6 +140,13 @@ 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. + let num_outputs = dummy_outputs.checked_add(1).ok_or_else(|| { + ProtocolError::ShieldedBuildError("dummy_outputs overflows the output count".to_string()) + })?; + shielded_bundle_action_count(0, num_outputs, platform_version)?; + let bundle = build_output_only_bundle( recipient, shield_amount, diff --git a/packages/rs-dpp/src/shielded/builder/shielded_transfer.rs b/packages/rs-dpp/src/shielded/builder/shielded_transfer.rs index 365e1f085e8..1b9ea2eb143 100644 --- a/packages/rs-dpp/src/shielded/builder/shielded_transfer.rs +++ b/packages/rs-dpp/src/shielded/builder/shielded_transfer.rs @@ -641,8 +641,9 @@ mod tests { let ask = SpendAuthorizingKey::from(&sk); let change_address = test_orchard_address(); - // `max` recipients → `max + 1` outputs once change is added. This is exactly what the - // FFI's 16-recipient ceiling admits today. + // `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( @@ -662,23 +663,65 @@ mod tests { ); } - /// The boundary itself must still build: `max - 1` recipients plus change is exactly - /// `max_shielded_transition_actions` actions. The gate must not 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. + /// 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 max = platform_version - .system_limits - .max_shielded_transition_actions as usize; + 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(max - 1, 1_000_000); + let outputs = n_outputs(effective - 1, 1_000_000); let err = build_shielded_transfer_transition_multi( vec![test_spendable_note(u64::MAX / 2)], @@ -693,8 +736,10 @@ mod tests { .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"), - "exactly {max} actions is AT the limit and must not be rejected by it, got: {err}" + !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"), diff --git a/packages/rs-dpp/src/shielded/builder/shielded_withdrawal.rs b/packages/rs-dpp/src/shielded/builder/shielded_withdrawal.rs index aab8ddac1fb..1f8531f52db 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,11 @@ 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, 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..b3c822cdf20 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,11 @@ 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, 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 f01b04303f4..644e3c6bd23 100644 --- a/packages/rs-dpp/src/shielded/mod.rs +++ b/packages/rs-dpp/src/shielded/mod.rs @@ -29,6 +29,91 @@ 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). Transition types with +/// larger envelopes (an instant asset-lock proof embedding its funding +/// transaction, or a large identity key set) eat into the ~1.4 KiB of slack +/// that remains at the derived action ceiling — they do not change the +/// ceiling itself for realistic envelopes, and DAPI's byte prefilter remains +/// the authoritative gate. +pub const SHIELDED_TRANSITION_WIRE_OVERHEAD_BYTES: u64 = 2_932; + +/// Conservative estimate of a shielded transition's on-wire serialized size +/// for a bundle of `num_actions` Orchard actions. +/// +/// `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`]). +pub fn estimated_shielded_transition_wire_bytes(num_actions: usize) -> u64 { + SHIELDED_TRANSITION_WIRE_OVERHEAD_BYTES + + (num_actions as u64) + * (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. +pub fn max_shielded_actions_per_transition( + platform_version: &platform_version::version::PlatformVersion, +) -> 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); + // per_action is a non-zero constant; the division is total. + let by_size = (size_budget / per_action) as usize; + structural.min(by_size) +} + /// Permanent storage bytes per shielded action: 344 bytes total. /// /// - 312 bytes in the BulkAppendTree: 32 (`cmx`, the note commitment) + 32 @@ -202,6 +287,59 @@ 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" + ); + } +} + #[cfg(all( test, feature = "json-conversion", From 0281480f4fc21020013a0fb195cccc214d1437c9 Mon Sep 17 00:00:00 2001 From: bfoss765 Date: Wed, 12 Aug 2026 09:01:27 -0400 Subject: [PATCH 4/5] fix(wallet-ffi): panic-guard every shielded block_on_worker export; clamp the multi-transfer recipient ceiling to the effective action limit Boundary alignment for the size-derived action ceiling: - MAX_SHIELDED_TRANSFER_RECIPIENTS drops 16 -> 5: the effective per-transition Orchard action ceiling (6, bound by the 20 KiB max_state_transition_size) minus the unconditional change output. Recipient counts 6..16 could never execute on chain - they only burned ~30 s of Halo 2 proving before the byte prefilter rejected the transition. A test pins the constant to dpp's max_shielded_actions_per_transition() derivation so a versioned-limit change fails loudly. The JNI adapter enforces the Rust constant symbolically (no change needed); the Kotlin mirror in PlatformWalletManager.kt is updated in lockstep. Panic guards: - catch_spend_panic generalizes to catch_panic_to_code(operation, code, guidance, body). Every remaining block_on_worker export in shielded_send.rs now runs its body under the guard via the established *_inner extraction pattern (previously only transfer_multi was guarded): - transfer, unshield, withdraw, shield -> ErrorShieldedSpendUnconfirmed (the ambiguous, do-NOT-retry spend contract; shield included to match map_spend_result's mapping, with the address-nonce check making a later manual retry self-healing). - identity_create_from_pool -> generic ErrorUnknown: the export's ErrorShieldedBroadcastUnconfirmed ABI contract requires writing out_identity_id, which a panic cannot supply; ErrorShieldedSpendUnconfirmed is documented as scoped to unshield/transfer/withdrawal; every other code promises a definitive outcome; and no dedicated panic code exists in the registry-tracked enum (allocating one risks the cross-branch numeric collisions the codes-28-30 comment warns about). The message carries do-not-resubmit / hold-the-slot guidance. - fund_from_asset_lock, resume_fund_from_asset_lock, seed_pool_notes -> ErrorWalletOperation (the single error code those exports already surface), with tracked-lock/resume guidance in the message. iOS panic=abort - evaluated, NOT flipped: - dev-ios/release-ios keep panic = "abort", so on iOS a panic still aborts before any guard runs; the guards are effective on Android (panic=unwind per the profile comments) and host/test builds. Reasons against flipping now: (1) the iOS profiles exist explicitly as size tuning for the staticlib ('otherwise ships huge'), and panic=abort removes unwind tables and landing pads under fat LTO - a size lever; (2) the Android profile comment ('Unlike iOS, panic stays unwind') shows abort-on-iOS is a deliberate decision, not an accident; (3) the size regression of flipping cannot be measured in this environment (no iOS target build). The workspace Cargo.toml release-ios comment now documents the guard interplay and that a flip requires a measured size delta; until then this is a known, documented iOS limitation. Validation: cargo test -p platform-wallet-ffi --features shielded --lib -> 235 passed, 0 failed; cargo check -p rs-unified-sdk-jni -> clean; platform-wallet note_selection/seed_pool tests -> 24 passed, 0 failed. --- Cargo.toml | 9 + .../dashsdk/wallet/PlatformWalletManager.kt | 12 +- .../src/shielded_send.rs | 447 +++++++++++++++++- 3 files changed, 447 insertions(+), 21 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3c7a1ad0760..9372aa749d9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -66,6 +66,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/wallet/PlatformWalletManager.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt index 362e5dc4bae..bbb3cddab63 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 @@ -1662,8 +1662,9 @@ class PlatformWalletManager( * * @param walletId the 32-byte wallet id. * @param outputs (raw 43-byte Orchard address, credits) pairs; must be - * non-empty, hold at most 16 entries (the native ceiling), and every - * amount must be positive. + * 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). @@ -2290,8 +2291,13 @@ class PlatformWalletManager( * `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 = 16 + const val MAX_SHIELDED_TRANSFER_RECIPIENTS = 5 /** De-offset `PlatformWalletFFIResultCode::ErrorInvalidParameter`. */ const val PWFFI_INVALID_PARAMETER = 2 diff --git a/packages/rs-platform-wallet-ffi/src/shielded_send.rs b/packages/rs-platform-wallet-ffi/src/shielded_send.rs index ac036848e5b..07c3c5855d1 100644 --- a/packages/rs-platform-wallet-ffi/src/shielded_send.rs +++ b/packages/rs-platform-wallet-ffi/src/shielded_send.rs @@ -281,6 +281,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); @@ -350,16 +380,23 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_transfer( map_spend_result(result, "shielded transfer") } -/// Defensive upper bound on the recipient count of a multi-output shielded transfer. +/// Recipient ceiling of a multi-output shielded transfer (Type 16). /// -/// This is an FFI sanity bound, not the protocol limit: it stops an absurd or corrupt -/// `num_recipients` from driving a huge allocation before anything else can reject it. The real -/// ceiling is the 20 KiB state-transition size limit, which admits roughly six Orchard actions — -/// so a legitimate caller stays far below this. +/// 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. -pub const MAX_SHIELDED_TRANSFER_RECIPIENTS: usize = 16; +/// 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 { @@ -372,8 +409,9 @@ fn panic_payload_message(payload: &(dyn std::any::Any + Send)) -> String { } } -/// Run a shielded-spend export body under [`std::panic::catch_unwind`], converting a panic into a -/// typed FFI error instead of letting it reach the `extern "C"` frame. +/// 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 @@ -382,28 +420,78 @@ fn panic_payload_message(payload: &(dyn std::any::Any + Send)) -> String { /// `.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 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( +/// 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( - PlatformWalletFFIResultCode::ErrorShieldedSpendUnconfirmed, + code, format!( - "{operation} panicked: {}. The spend may or may not have been broadcast — do \ - NOT retry; the next shielded sync reconciles the outcome.", + "{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. /// @@ -605,6 +693,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); @@ -679,6 +793,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); @@ -898,6 +1040,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); @@ -1064,6 +1258,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); @@ -1168,6 +1392,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); @@ -1316,6 +1578,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); @@ -1459,6 +1757,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); @@ -1741,6 +2084,74 @@ mod tests { ); } + /// 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`. + #[test] + fn catch_panic_to_code_carries_the_per_operation_contract() { + let previous = std::panic::take_hook(); + // Silence the default hook's backtrace spew for these deliberate panics. + std::panic::set_hook(Box::new(|_| {})); + + 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"), + ); + std::panic::set_hook(previous); + + 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 From 1e46088d739a7ce084acd637865e106d82851e32 Mon Sep 17 00:00:00 2001 From: bfoss765 Date: Wed, 12 Aug 2026 19:13:27 -0400 Subject: [PATCH 5/5] fix(dpp): price transition-specific envelopes into the pre-proving action ceiling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The size-derived ceiling assumed the fixed 2,932-byte envelope measured on a chain-proof ShieldFromAssetLock, but two transition families carry variable non-Orchard fields that can consume the ~1.4 KiB slack it leaves under max_state_transition_size: an instant asset-lock proof embeds its funding transaction and InstantLock (both hold input vectors; DPP admits up to 100 inputs), and identity creation carries up to six variable public keys — so a valid multi-input instant proof could pass the gate, burn the ~30 s Halo 2 proof, and only then be rejected by DAPI's byte prefilter (#4312 review finding e90e9cf15f52). - max_shielded_actions_for_envelope / estimated_..._with_envelope: the ceiling and the estimator now take the transition's extra envelope bytes; the baseline forms delegate with 0. - shielded_bundle_action_count grows an extra_envelope_bytes parameter; the rejection message names the envelope contribution. - ShieldFromAssetLock measures its serialized asset-lock proof (chain proofs cost a few dozen bytes and keep the baseline ceiling; the slight double-count of the baseline's own measured chain proof is deliberate conservatism). Identity-create measures its serialized key set plus a 97-byte per-key allowance for the PoP signatures that are still empty at gate time (BLS 96 B + length prefix). Transfer, unshield, and withdrawal have fixed-size envelopes and pass 0. - serialized_envelope_bytes measures with the same standard().with_big_endian() bincode config the wire serialization uses. Boundary tests: exact-byte ceiling tightening (slack keeps, slack+1 displaces an action, u64::MAX degrades to 0 without wrapping); chain-proof envelope keeps the baseline; 20- and 100-input instant proofs tighten the ceiling and fail the gate pre-proving; a six-key identity create still clears the padded 2-action claim shape. Also rustfmts the d3ecd62f9d test/model hunks that were failing the workspace fmt gate in CI. dpp suite: 3935 passed. Co-Authored-By: Claude Opus 4.8 --- .../identity_create_from_shielded_pool.rs | 79 +++++++- packages/rs-dpp/src/shielded/builder/mod.rs | 70 +++++-- .../builder/shield_from_asset_lock.rs | 173 +++++++++++++++++- .../src/shielded/builder/shielded_transfer.rs | 24 ++- .../shielded/builder/shielded_withdrawal.rs | 10 +- .../rs-dpp/src/shielded/builder/unshield.rs | 10 +- packages/rs-dpp/src/shielded/mod.rs | 122 ++++++++++-- 7 files changed, 449 insertions(+), 39 deletions(-) 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 f010608664b..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 @@ -22,8 +22,8 @@ use platform_value::Identifier; use platform_version::version::PlatformVersion; use super::{ - build_spend_bundle_with, serialize_authorized_bundle, shielded_bundle_action_count, - OrchardProver, SpendableNote, + 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 @@ -164,8 +164,21 @@ where // // 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, platform_version)?; + // 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)?; @@ -358,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 diff --git a/packages/rs-dpp/src/shielded/builder/mod.rs b/packages/rs-dpp/src/shielded/builder/mod.rs index 82acc4892e3..4772e5d1ef6 100644 --- a/packages/rs-dpp/src/shielded/builder/mod.rs +++ b/packages/rs-dpp/src/shielded/builder/mod.rs @@ -144,14 +144,25 @@ impl From<&OrchardAddress> for PaymentAddress { /// 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_per_transition`], never hardcoded, so a +/// 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 @@ -173,21 +184,58 @@ pub fn shielded_bundle_action_count( ))); } - let effective_max = crate::shielded::max_shielded_actions_per_transition(platform_version); + 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(num_actions); + 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 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" + 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()`, per `platform_serialization`), 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(); + 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 { @@ -889,7 +937,7 @@ mod mod_tests { (5, 3, 5), (3, 6, 6), ] { - let actual = shielded_bundle_action_count(spends, outputs, platform_version) + let actual = shielded_bundle_action_count(spends, outputs, 0, platform_version) .expect("DEFAULT bundles accept any spend/output mix"); assert_eq!( actual, expected, @@ -911,7 +959,7 @@ mod mod_tests { 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, platform_version) + let predicted = shielded_bundle_action_count(0, num_outputs, 0, platform_version) .expect("valid bundle shape"); assert_eq!( bundle.actions().len(), @@ -933,7 +981,7 @@ mod mod_tests { // 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, platform_version).unwrap_or_else( + 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 @@ -963,7 +1011,7 @@ mod mod_tests { 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, platform_version) + 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"), @@ -985,7 +1033,7 @@ mod mod_tests { .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, platform_version) + 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"), 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 026b21b9c44..e1f7908ec30 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 @@ -7,8 +7,8 @@ use crate::ProtocolError; use platform_version::version::PlatformVersion; use super::{ - build_output_only_bundle, serialize_authorized_bundle, shielded_bundle_action_count, - OrchardProver, + build_output_only_bundle, serialize_authorized_bundle, serialized_envelope_bytes, + shielded_bundle_action_count, OrchardProver, }; /// Builds a ShieldFromAssetLock state transition (core asset lock -> shielded pool). @@ -54,10 +54,18 @@ pub fn build_shield_from_asset_lock_transition( // 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`. A chain proof serializes to a few dozen bytes, slightly + // double-counting the baseline's own measured chain proof — conservative by design. let num_outputs = dummy_outputs.checked_add(1).ok_or_else(|| { ProtocolError::ShieldedBuildError("dummy_outputs overflows the output count".to_string()) })?; - shielded_bundle_action_count(0, num_outputs, platform_version)?; + let proof_envelope_bytes = + serialized_envelope_bytes(&asset_lock_proof, "the asset-lock proof")?; + shielded_bundle_action_count(0, num_outputs, proof_envelope_bytes, platform_version)?; let bundle = build_output_only_bundle( recipient, @@ -141,11 +149,14 @@ where 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. + // proof, with the same checked output-count arithmetic and the same + // transition-specific proof envelope priced into the size side. let num_outputs = dummy_outputs.checked_add(1).ok_or_else(|| { ProtocolError::ShieldedBuildError("dummy_outputs overflows the output count".to_string()) })?; - shielded_bundle_action_count(0, num_outputs, platform_version)?; + let proof_envelope_bytes = + serialized_envelope_bytes(&asset_lock_proof, "the asset-lock proof")?; + shielded_bundle_action_count(0, num_outputs, proof_envelope_bytes, platform_version)?; let bundle = build_output_only_bundle( recipient, @@ -184,6 +195,158 @@ 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 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}; + + 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 { + let inputs: Vec = (0..num_inputs) + .map(|i| TxIn { + previous_output: OutPoint::new(txid(), i as u32), + script_sig: ScriptBuf::from(vec![0u8; 107]), + 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); + } +} + #[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 1b9ea2eb143..4fa20c05f74 100644 --- a/packages/rs-dpp/src/shielded/builder/shielded_transfer.rs +++ b/packages/rs-dpp/src/shielded/builder/shielded_transfer.rs @@ -68,7 +68,14 @@ pub fn build_shielded_transfer_transition( // (`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, platform_version)?; + 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). @@ -246,7 +253,14 @@ pub fn build_shielded_transfer_transition_multi( // `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, platform_version)?; + 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(|| { @@ -672,8 +686,7 @@ mod tests { #[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); + let effective = crate::shielded::max_shielded_actions_per_transition(platform_version); assert!( effective < platform_version @@ -714,8 +727,7 @@ mod tests { #[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 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); diff --git a/packages/rs-dpp/src/shielded/builder/shielded_withdrawal.rs b/packages/rs-dpp/src/shielded/builder/shielded_withdrawal.rs index 1f8531f52db..176c2a05a2b 100644 --- a/packages/rs-dpp/src/shielded/builder/shielded_withdrawal.rs +++ b/packages/rs-dpp/src/shielded/builder/shielded_withdrawal.rs @@ -77,7 +77,15 @@ pub fn build_shielded_withdrawal_transition( // 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, platform_version)?; + 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 b3c822cdf20..083e76405f1 100644 --- a/packages/rs-dpp/src/shielded/builder/unshield.rs +++ b/packages/rs-dpp/src/shielded/builder/unshield.rs @@ -71,7 +71,15 @@ pub fn build_unshield_transition( // 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, platform_version)?; + 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 644e3c6bd23..79db3329ce8 100644 --- a/packages/rs-dpp/src/shielded/mod.rs +++ b/packages/rs-dpp/src/shielded/mod.rs @@ -57,25 +57,47 @@ pub const SHIELDED_PROOF_WIRE_BYTES_PER_ACTION: u64 = 2_273; /// 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). Transition types with -/// larger envelopes (an instant asset-lock proof embedding its funding -/// transaction, or a large identity key set) eat into the ~1.4 KiB of slack -/// that remains at the derived action ceiling — they do not change the -/// ceiling itself for realistic envelopes, and DAPI's byte prefilter remains -/// the authoritative gate. +/// `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; /// Conservative estimate of a shielded transition's on-wire serialized size -/// for a bundle of `num_actions` Orchard actions. +/// 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`]). +/// [`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 - + (num_actions as u64) - * (SHIELDED_ACTION_WIRE_BYTES + SHIELDED_PROOF_WIRE_BYTES_PER_ACTION) + .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`: @@ -98,8 +120,32 @@ pub fn estimated_shielded_transition_wire_bytes(num_actions: usize) -> u64 { /// `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 @@ -108,7 +154,8 @@ pub fn max_shielded_actions_per_transition( let size_budget = platform_version .system_limits .max_state_transition_size - .saturating_sub(SHIELDED_TRANSITION_WIRE_OVERHEAD_BYTES); + .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) @@ -338,6 +385,59 @@ mod wire_cost_tests { "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(