diff --git a/book/src/fees/shielded-fees.md b/book/src/fees/shielded-fees.md index 1875a870fc5..692f462b4a6 100644 --- a/book/src/fees/shielded-fees.md +++ b/book/src/fees/shielded-fees.md @@ -38,9 +38,9 @@ The fee is derived differently depending on the shielded transition type: | Transition | Fee Formula | Explanation | |---|---|---| | **Shield** | Paid from transparent address inputs | Fee comes from the transparent side, not from `value_balance`. Skipped by shielded fee validation. | -| **ShieldedTransfer** | `fee = value_balance` | The entire `value_balance` is fee — nothing leaves the pool except the fee going to proposers. | -| **Unshield** | `fee = value_balance − amount` | `amount` goes to the output address; the remainder is fee. | -| **ShieldedWithdrawal** | `fee = value_balance − amount` | `amount` goes to the withdrawal document; the remainder is fee. | +| **ShieldedTransfer** | `fee = value_balance` (pinned to the minimum) | The entire `value_balance` is the fee and must equal `compute_minimum_shielded_fee(num_actions)` exactly (overpayment is rejected). Nothing leaves the pool except the fee. | +| **Unshield** | `fee = compute_minimum_shielded_fee(num_actions)` | `value_balance` (the transition's `unshielding_amount`) is the **gross** amount leaving the pool. The output address receives `unshielding_amount − fee`; the `fee` is the flat minimum. Validation requires `unshielding_amount ≥ fee`. | +| **ShieldedWithdrawal** | `fee = compute_minimum_shielded_fee(num_actions)` | `value_balance` (`unshielding_amount`) is the **gross** amount leaving the pool. The Core withdrawal document receives `unshielding_amount − fee` (which must also clear `MIN_WITHDRAWAL_AMOUNT`); the `fee` is the flat minimum. | | **ShieldFromAssetLock** | Paid from asset lock | Fee comes from the asset lock mechanism, not from `value_balance`. | For `ShieldedTransfer`, the client constructs the bundle so that `total_spent − @@ -73,10 +73,16 @@ Each action in the bundle requires: - Nullifier duplicate check (hash + tree lookup) - Note commitment insertion into the Sinsemilla-based Merkle tree -The processing cost per action was calibrated at a 33:1 ratio against the proof -verification cost, based on benchmarks of signature verification and tree operations. +The per-action processing fee prices the marginal Halo 2 verification work that +each additional action adds to the bundle (≈1.1 ms/action measured against a +≈5 ms bundle base), so it is calibrated at roughly a 4.5:1 ratio against the +fixed proof-verification fee (100M : 22M) rather than the looser ratio used +before the recalibration. (Note the two ratios on this page use different +baselines: the “30×” in §1 is the proof fee relative to a single RedPallas +signature verification, whereas this 4.5:1 is the proof fee relative to the +per-action processing fee.) -**Current value:** `3,000,000` credits (3M) +**Current value:** `22,000,000` credits (22M) ### 3. Per-Action Storage Fee @@ -84,7 +90,7 @@ Each action permanently stores data in two places: | Storage | Bytes | Contents | |---|---|---| -| BulkAppendTree (commitment tree) | 280 | 32 cmx + 32 nullifier + 216 encrypted note | +| BulkAppendTree (commitment tree) | 280 | 32 cmx + 32 rho + 216 encrypted note | | Nullifier tree | 32 | nullifier key (value is empty) | | **Total** | **312** | | @@ -108,9 +114,9 @@ Combining all three components: | Actions | Proof Fee | Processing | Storage | Total Minimum Fee | |---|---|---|---|---| -| 2 | 100,000,000 | 6,000,000 | 17,097,600 | **123,097,600** | -| 3 | 100,000,000 | 9,000,000 | 25,646,400 | **134,646,400** | -| 4 | 100,000,000 | 12,000,000 | 34,195,200 | **146,195,200** | +| 2 | 100,000,000 | 44,000,000 | 17,097,600 | **161,097,600** | +| 3 | 100,000,000 | 66,000,000 | 25,646,400 | **191,646,400** | +| 4 | 100,000,000 | 88,000,000 | 34,195,200 | **222,195,200** | Note: The Orchard protocol requires a minimum of 2 actions per bundle for privacy (even a single-input single-output transfer produces 2 actions with a dummy padding @@ -158,8 +164,10 @@ pub struct DriveAbciValidationConstants { pub maximum_vote_polls_to_process: u16, pub maximum_contenders_to_consider: u16, pub minimum_pool_notes_for_outgoing: u64, + pub shielded_anchor_retention_blocks: u64, + pub shielded_anchor_pruning_interval: u64, pub shielded_proof_verification_fee: u64, // 100_000_000 - pub shielded_per_action_processing_fee: u64, // 3_000_000 + pub shielded_per_action_processing_fee: u64, // 22_000_000 } ``` @@ -176,23 +184,32 @@ This design means: ## How Fees Flow After Validation Once the fee check passes and the transition is fully validated and executed, the -fee amount is deducted from the shielded pool's total balance and routed to block -proposers via the `PaidFromShieldedPool` execution event: +shielded pool's total balance is decremented and the fee is booked via the +`PaidFromShieldedPool` execution event: ``` -ShieldedTransfer: pool_balance -= fee_amount -Unshield: pool_balance -= (amount + fee_amount) -ShieldedWithdrawal: pool_balance -= (amount + fee_amount) +ShieldedTransfer: pool_balance -= fee_amount // fee == value_balance +Unshield: pool_balance -= unshielding_amount // gross +ShieldedWithdrawal: pool_balance -= unshielding_amount // gross ``` -For `Unshield`, the `amount` goes to the output platform address. For -`ShieldedWithdrawal`, the `amount` goes to a Core withdrawal document. In both -cases, the `fee_amount` goes to proposers. - -For `ShieldedTransfer`, the total pool value decreases by exactly the fee amount. -The rest of the value stays inside the pool (the sender's notes are spent and the -recipient's notes are created, but the pool's aggregate balance only drops by the -fee). +For `Unshield` and `ShieldedWithdrawal`, `unshielding_amount` is the **gross** amount +leaving the pool. Of that, `unshielding_amount − fee_amount` is credited to the output +platform address (`Unshield`) or written into the Core withdrawal document +(`ShieldedWithdrawal`), and `fee_amount` — the flat `compute_minimum_shielded_fee` — is +booked as the transition fee. Validation guarantees `unshielding_amount ≥ fee_amount` +(and, for `ShieldedWithdrawal`, that the net also clears `MIN_WITHDRAWAL_AMOUNT`), so the +subtraction never underflows. + +For `ShieldedTransfer`, the pool decreases by exactly the fee (the sender's notes are +spent and the recipient's notes are created, but the pool's aggregate balance only drops +by the fee). + +In all cases the booked `fee_amount` is split the same way as every other transition's +fee: the storage cost of the permanent shielded writes is routed to the storage pool +(amortized across epochs and subject to the per-epoch fee multiplier at payout), and the +remainder — proof verification plus per-action processing — is the processing fee paid to +the current block proposer. ## Cryptographic Binding diff --git a/packages/rs-dpp/src/shielded/builder/shielded_transfer.rs b/packages/rs-dpp/src/shielded/builder/shielded_transfer.rs index 3abda078c64..3c2429970fd 100644 --- a/packages/rs-dpp/src/shielded/builder/shielded_transfer.rs +++ b/packages/rs-dpp/src/shielded/builder/shielded_transfer.rs @@ -30,9 +30,11 @@ use super::{prove_and_sign_bundle, serialize_authorized_bundle, OrchardProver, S /// - `anchor` - Sinsemilla root of the note commitment tree (Orchard Anchor) /// - `prover` - Orchard prover (holds the Halo 2 proving key) /// - `memo` - 36-byte structured memo for the recipient (4-byte type tag + 32-byte payload) -/// - `fee` - Optional fee override; if `None`, the minimum fee is computed automatically. -/// If `Some`, must be >= the minimum fee. /// - `platform_version` - Protocol version +/// +/// The fee is not a parameter: a shielded transfer's `value_balance` IS the fee and consensus +/// pins it to exactly `compute_minimum_shielded_fee`, so there is nothing for the caller to +/// choose. Returns the built transition together with the fee (in credits) that was applied. #[allow(clippy::too_many_arguments)] pub fn build_shielded_transfer_transition( spends: Vec, @@ -44,39 +46,25 @@ pub fn build_shielded_transfer_transition( anchor: Anchor, prover: &P, memo: [u8; 36], - fee: Option, platform_version: &PlatformVersion, -) -> Result { +) -> 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); - let min_fee = compute_minimum_shielded_fee(num_actions, platform_version); - let effective_fee = match fee { - Some(f) if f < min_fee => { - return Err(ProtocolError::ShieldedBuildError(format!( - "fee {} is below minimum required fee {}", - f, min_fee - ))); - } - Some(f) if f > min_fee.saturating_mul(1000) => { - return Err(ProtocolError::ShieldedBuildError(format!( - "fee {} exceeds 1000x the minimum fee {}", - f, min_fee - ))); - } - Some(f) => f, - None => min_fee, - }; + // 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). + let fee = compute_minimum_shielded_fee(num_actions, platform_version)?; - let required = transfer_amount.checked_add(effective_fee).ok_or_else(|| { + let required = transfer_amount.checked_add(fee).ok_or_else(|| { ProtocolError::ShieldedBuildError("fee + transfer_amount overflows u64".to_string()) })?; if required > total_spent { return Err(ProtocolError::ShieldedBuildError(format!( "transfer amount {} + fee {} = {} exceeds total spendable value {}", - transfer_amount, effective_fee, required, total_spent + transfer_amount, fee, required, total_spent ))); } @@ -123,15 +111,16 @@ pub fn build_shielded_transfer_transition( let bundle = prove_and_sign_bundle(builder, prover, std::slice::from_ref(ask), &[])?; let sb = serialize_authorized_bundle(&bundle); - // value_balance = effective_fee (the amount leaving the shielded pool as fee) - ShieldedTransferTransition::try_from_bundle( + // value_balance = fee (the amount leaving the shielded pool as fee) + 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)] @@ -141,43 +130,6 @@ mod tests { test_orchard_address, test_spendable_note, TestProver, }; - #[test] - fn test_shielded_transfer_fee_below_minimum() { - let platform_version = PlatformVersion::latest(); - let recipient = test_orchard_address(); - let change_address = test_orchard_address(); - - let note = test_spendable_note(1_000_000); - let spends = vec![note]; - - let sk = grovedb_commitment_tree::SpendingKey::from_bytes([42u8; 32]) - .expect("valid spending key bytes"); - let fvk = FullViewingKey::from(&sk); - let ask = SpendAuthorizingKey::from(&sk); - - let result = build_shielded_transfer_transition( - spends, - &recipient, - 100, - &change_address, - &fvk, - &ask, - Anchor::empty_tree(), - &TestProver, - [0u8; 36], - Some(1), // fee = 1, should be below minimum - platform_version, - ); - - assert!(result.is_err()); - let err = result.unwrap_err().to_string(); - assert!( - err.contains("below minimum required fee"), - "unexpected error: {}", - err - ); - } - #[test] fn test_shielded_transfer_insufficient_funds() { let platform_version = PlatformVersion::latest(); @@ -203,7 +155,6 @@ mod tests { Anchor::empty_tree(), &TestProver, [0u8; 36], - None, platform_version, ); @@ -220,48 +171,6 @@ mod tests { // Extra coverage — error/overflow branches // -------------------------------------------------------------- - #[test] - fn test_shielded_transfer_fee_above_upper_bound() { - // Fee > 1000x the minimum fee should be rejected. - let platform_version = PlatformVersion::latest(); - let recipient = test_orchard_address(); - let change_address = test_orchard_address(); - - let note = test_spendable_note(u64::MAX); - let spends = vec![note]; - - let sk = grovedb_commitment_tree::SpendingKey::from_bytes([42u8; 32]) - .expect("valid spending key bytes"); - let fvk = FullViewingKey::from(&sk); - let ask = SpendAuthorizingKey::from(&sk); - - // num_actions is max(spends.len(), 2) = 2. - let min_fee = crate::shielded::compute_minimum_shielded_fee(2, platform_version); - let excessive_fee = min_fee.saturating_mul(1000) + 1; - - let result = build_shielded_transfer_transition( - spends, - &recipient, - 10, - &change_address, - &fvk, - &ask, - Anchor::empty_tree(), - &TestProver, - [0u8; 36], - Some(excessive_fee), - platform_version, - ); - - assert!(result.is_err()); - let err = result.unwrap_err().to_string(); - assert!( - err.contains("exceeds 1000x the minimum fee"), - "unexpected error: {}", - err - ); - } - #[test] fn test_shielded_transfer_fee_plus_amount_overflow_errors() { // transfer_amount + fee overflows u64 → dedicated error branch. @@ -277,11 +186,8 @@ mod tests { let fvk = FullViewingKey::from(&sk); let ask = SpendAuthorizingKey::from(&sk); - // Compute min fee, then craft a fee that lies in [min_fee, 1000*min_fee] - // so we bypass the boundary checks, then pick transfer_amount = u64::MAX - // so amount + fee overflows. - let min_fee = crate::shielded::compute_minimum_shielded_fee(2, platform_version); - + // transfer_amount = u64::MAX so amount + the (internally-computed) minimum fee + // overflows u64, hitting the checked_add error branch. let result = build_shielded_transfer_transition( spends, &recipient, @@ -292,7 +198,6 @@ mod tests { Anchor::empty_tree(), &TestProver, [0u8; 36], - Some(min_fee), // within [min, 1000*min] platform_version, ); @@ -328,7 +233,6 @@ mod tests { Anchor::empty_tree(), &TestProver, [0u8; 36], - None, platform_version, ); assert!(result.is_err()); @@ -341,16 +245,16 @@ mod tests { } #[test] - fn test_shielded_transfer_fee_default_is_min_fee() { - // When fee is None, the default min fee is computed — verify that a - // note *exactly* equal to `transfer_amount + min_fee` on the default - // branch does not spuriously fail the "exceeds total" check (it + fn test_shielded_transfer_uses_min_fee() { + // The fee is always the minimum. Verify that a note *exactly* equal to + // `transfer_amount + min_fee` proceeds past the "exceeds total" check (it then // fails later in add_spend due to anchor mismatch). let platform_version = PlatformVersion::latest(); let recipient = test_orchard_address(); let change_address = test_orchard_address(); - let min_fee = crate::shielded::compute_minimum_shielded_fee(2, platform_version); + let min_fee = crate::shielded::compute_minimum_shielded_fee(2, platform_version) + .expect("fee computation should not overflow"); let transfer_amount = 10u64; let note = test_spendable_note(transfer_amount + min_fee); let spends = vec![note]; @@ -369,7 +273,6 @@ mod tests { Anchor::empty_tree(), &TestProver, [0u8; 36], - None, platform_version, ); diff --git a/packages/rs-dpp/src/shielded/builder/shielded_withdrawal.rs b/packages/rs-dpp/src/shielded/builder/shielded_withdrawal.rs index f5885a5c1d1..329baa880f8 100644 --- a/packages/rs-dpp/src/shielded/builder/shielded_withdrawal.rs +++ b/packages/rs-dpp/src/shielded/builder/shielded_withdrawal.rs @@ -31,9 +31,11 @@ use super::{build_spend_bundle, serialize_authorized_bundle, OrchardProver, Spen /// - `anchor` - Sinsemilla root of the note commitment tree (Orchard Anchor) /// - `prover` - Orchard prover (holds the Halo 2 proving key) /// - `memo` - 36-byte structured memo for the change output (4-byte type tag + 32-byte payload) -/// - `fee` - Optional fee override; if `None`, the minimum fee is computed automatically. -/// If `Some`, must be >= the minimum fee. /// - `platform_version` - Protocol version +/// +/// The fee is not a parameter: consensus always charges exactly +/// `compute_minimum_shielded_fee` and ignores any surplus. Returns the built transition +/// together with the fee (in credits) that was applied. #[allow(clippy::too_many_arguments)] pub fn build_shielded_withdrawal_transition( spends: Vec, @@ -47,9 +49,8 @@ pub fn build_shielded_withdrawal_transition( anchor: Anchor, prover: &P, memo: [u8; 36], - fee: Option, platform_version: &PlatformVersion, -) -> Result { +) -> Result<(StateTransition, Credits), ProtocolError> { if withdrawal_amount > i64::MAX as u64 { return Err(ProtocolError::ShieldedBuildError(format!( "withdrawal amount {} exceeds maximum allowed value {}", @@ -60,45 +61,39 @@ pub fn build_shielded_withdrawal_transition( let total_spent: u64 = spends.iter().map(|s| s.note.value().inner()).sum(); - // Conservative action count: at least (spends, 1) since we have a change output. - let num_actions = spends.len().max(1); - let min_fee = compute_minimum_shielded_fee(num_actions, platform_version); - let effective_fee = match fee { - Some(f) if f < min_fee => { - return Err(ProtocolError::ShieldedBuildError(format!( - "fee {} is below minimum required fee {}", - f, min_fee - ))); - } - Some(f) if f > min_fee.saturating_mul(1000) => { - return Err(ProtocolError::ShieldedBuildError(format!( - "fee {} exceeds 1000x the minimum fee {}", - f, min_fee - ))); - } - Some(f) => f, - None => min_fee, - }; - - let required = withdrawal_amount - .checked_add(effective_fee) - .ok_or_else(|| { - ProtocolError::ShieldedBuildError("fee + withdrawal_amount overflows u64".to_string()) - })?; + // Orchard's BundleType::DEFAULT pads every bundle to a 2-action minimum + // (MIN_ACTIONS), so even a single-spend withdrawal is serialized and proven with 2 + // 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 withdrawal with InsufficientShieldedFeeError (or, + // post-fee, WithdrawalBelowMinAmountError). + let num_actions = spends.len().max(2); + // The fee is fixed at the minimum: consensus always carves exactly + // `compute_minimum_shielded_fee` from the pool, and the net (`withdrawal_amount`) goes to + // the Core withdrawal document. + let fee = compute_minimum_shielded_fee(num_actions, platform_version)?; + + let required = withdrawal_amount.checked_add(fee).ok_or_else(|| { + ProtocolError::ShieldedBuildError("fee + withdrawal_amount overflows u64".to_string()) + })?; if required > total_spent { return Err(ProtocolError::ShieldedBuildError(format!( "withdrawal amount {} + fee {} = {} exceeds total spendable value {}", - withdrawal_amount, effective_fee, required, total_spent + withdrawal_amount, fee, required, total_spent ))); } let change_amount = total_spent - required; - // ShieldedWithdrawal extra_data = output_script || value_balance (le bytes) - // value_balance = withdrawal_amount + fee, becomes v0.unshielding_amount in the state transition - // Must match server-side sighash in shielded_proof.rs - let mut extra_sighash_data = output_script.as_bytes().to_vec(); - extra_sighash_data.extend_from_slice(&required.to_le_bytes()); + // Bind every Core-facing withdrawal field into the Orchard sighash (output_script, + // unshielding_amount == required, core_fee_per_byte, pooling) so the binding signature + // authorizes them. Shared with the consensus verifier in shielded_proof.rs. + let extra_sighash_data = crate::shielded::shielded_withdrawal_extra_sighash_data( + output_script.as_bytes(), + required, + core_fee_per_byte, + pooling, + ); let bundle = build_spend_bundle( spends, @@ -114,7 +109,7 @@ pub fn build_shielded_withdrawal_transition( let sb = serialize_authorized_bundle(&bundle); - ShieldedWithdrawalTransition::try_from_bundle( + let state_transition = ShieldedWithdrawalTransition::try_from_bundle( sb.actions, sb.value_balance as u64, sb.anchor, @@ -124,7 +119,8 @@ pub fn build_shielded_withdrawal_transition( pooling, output_script, platform_version, - ) + )?; + Ok((state_transition, fee)) } #[cfg(test)] @@ -134,87 +130,6 @@ mod tests { test_orchard_address, test_spendable_note, TestProver, }; - #[test] - fn test_shielded_withdrawal_fee_below_minimum() { - let platform_version = PlatformVersion::latest(); - let change_address = test_orchard_address(); - - let note = test_spendable_note(1_000_000); - let spends = vec![note]; - - let sk = grovedb_commitment_tree::SpendingKey::from_bytes([42u8; 32]) - .expect("valid spending key bytes"); - let fvk = FullViewingKey::from(&sk); - let ask = SpendAuthorizingKey::from(&sk); - - let result = build_shielded_withdrawal_transition( - spends, - 100, - CoreScript::new_p2pkh([1u8; 20]), // minimal P2PKH prefix - 1, - Pooling::Never, - &change_address, - &fvk, - &ask, - Anchor::empty_tree(), - &TestProver, - [0u8; 36], - Some(1), // fee = 1, should be below minimum - platform_version, - ); - - assert!(result.is_err()); - let err = result.unwrap_err().to_string(); - assert!( - err.contains("below minimum required fee"), - "unexpected error: {}", - err - ); - } - - #[test] - fn test_shielded_withdrawal_fee_above_upper_bound() { - let platform_version = PlatformVersion::latest(); - let change_address = test_orchard_address(); - - let note = test_spendable_note(u64::MAX); - let spends = vec![note]; - - let sk = grovedb_commitment_tree::SpendingKey::from_bytes([42u8; 32]) - .expect("valid spending key bytes"); - let fvk = FullViewingKey::from(&sk); - let ask = SpendAuthorizingKey::from(&sk); - - // Compute the minimum fee so we can exceed the 1000x bound - let num_actions = 1usize; - let min_fee = crate::shielded::compute_minimum_shielded_fee(num_actions, platform_version); - let excessive_fee = min_fee.saturating_mul(1000) + 1; - - let result = build_shielded_withdrawal_transition( - spends, - 100, - CoreScript::new_p2pkh([1u8; 20]), - 1, - Pooling::Never, - &change_address, - &fvk, - &ask, - Anchor::empty_tree(), - &TestProver, - [0u8; 36], - Some(excessive_fee), - platform_version, - ); - - assert!(result.is_err()); - let err = result.unwrap_err().to_string(); - assert!( - err.contains("exceeds 1000x the minimum fee"), - "unexpected error: {}", - err - ); - } - #[test] fn test_shielded_withdrawal_insufficient_funds() { let platform_version = PlatformVersion::latest(); @@ -240,7 +155,6 @@ mod tests { Anchor::empty_tree(), &TestProver, [0u8; 36], - None, platform_version, ); @@ -283,7 +197,6 @@ mod tests { Anchor::empty_tree(), &TestProver, [0u8; 36], - None, platform_version, ); assert!(result.is_err()); @@ -295,100 +208,9 @@ mod tests { ); } - #[test] - fn test_shielded_withdrawal_fee_at_exact_upper_bound_accepted() { - // Boundary test: fee == 1000x the minimum is the *accepted* upper - // limit (strictly > is rejected). The builder should proceed past - // the fee validation and only fail later at add_spend. - let platform_version = PlatformVersion::latest(); - let change_address = test_orchard_address(); - - let note = test_spendable_note(u64::MAX); - let spends = vec![note]; - - let sk = grovedb_commitment_tree::SpendingKey::from_bytes([42u8; 32]).expect("valid sk"); - let fvk = FullViewingKey::from(&sk); - let ask = SpendAuthorizingKey::from(&sk); - - let min_fee = crate::shielded::compute_minimum_shielded_fee(1, platform_version); - let fee_at_boundary = min_fee.saturating_mul(1000); - - let result = build_shielded_withdrawal_transition( - spends, - 100, - CoreScript::new_p2pkh([1u8; 20]), - 1, - Pooling::Never, - &change_address, - &fvk, - &ask, - Anchor::empty_tree(), - &TestProver, - [0u8; 36], - Some(fee_at_boundary), - platform_version, - ); - // Boundary value passes the validation, so it must NOT fail with - // "exceeds 1000x". A successful build is also acceptable; only a - // later-stage failure (anchor/add_spend) should surface — and never - // as the upper-bound error. - if let Err(err) = result { - let err = err.to_string(); - assert!( - !err.contains("exceeds 1000x"), - "boundary value should not trigger upper-bound error: {}", - err - ); - } - } - - #[test] - fn test_shielded_withdrawal_fee_at_exact_min_accepted() { - // Boundary test: fee == min_fee should be accepted (strictly `<` - // is rejected). - let platform_version = PlatformVersion::latest(); - let change_address = test_orchard_address(); - - let note = test_spendable_note(1_000_000); - let spends = vec![note]; - - let sk = grovedb_commitment_tree::SpendingKey::from_bytes([42u8; 32]).expect("valid sk"); - let fvk = FullViewingKey::from(&sk); - let ask = SpendAuthorizingKey::from(&sk); - - let min_fee = crate::shielded::compute_minimum_shielded_fee(1, platform_version); - - let result = build_shielded_withdrawal_transition( - spends, - 100, - CoreScript::new_p2pkh([1u8; 20]), - 1, - Pooling::Never, - &change_address, - &fvk, - &ask, - Anchor::empty_tree(), - &TestProver, - [0u8; 36], - Some(min_fee), - platform_version, - ); - // Fee == min_fee is accepted by validation; a successful build is - // fine. Only a later-stage failure should surface — and never with - // the "below minimum required fee" message. - if let Err(err) = result { - let err = err.to_string(); - assert!( - !err.contains("below minimum required fee"), - "fee at min must not trip the lower bound: {}", - err - ); - } - } - #[test] fn test_shielded_withdrawal_zero_spends_errors() { - // Empty spends vec → total_spent = 0 and num_actions = 1 (max). + // Empty spends vec → total_spent = 0 and num_actions = 2 (max floor). let platform_version = PlatformVersion::latest(); let change_address = test_orchard_address(); @@ -408,7 +230,6 @@ mod tests { Anchor::empty_tree(), &TestProver, [0u8; 36], - None, platform_version, ); assert!(result.is_err()); diff --git a/packages/rs-dpp/src/shielded/builder/unshield.rs b/packages/rs-dpp/src/shielded/builder/unshield.rs index cd44b74e336..02c26fa6092 100644 --- a/packages/rs-dpp/src/shielded/builder/unshield.rs +++ b/packages/rs-dpp/src/shielded/builder/unshield.rs @@ -27,9 +27,11 @@ use super::{build_spend_bundle, serialize_authorized_bundle, OrchardProver, Spen /// - `anchor` - Sinsemilla root of the note commitment tree (Orchard Anchor) /// - `prover` - Orchard prover (holds the Halo 2 proving key) /// - `memo` - 36-byte structured memo for the change output (4-byte type tag + 32-byte payload) -/// - `fee` - Optional fee override; if `None`, the minimum fee is computed automatically. -/// If `Some`, must be >= the minimum fee. /// - `platform_version` - Protocol version +/// +/// The fee is not a parameter: consensus always charges exactly +/// `compute_minimum_shielded_fee` and ignores any surplus. Returns the built transition +/// together with the fee (in credits) that was applied. #[allow(clippy::too_many_arguments)] pub fn build_unshield_transition( spends: Vec, @@ -41,9 +43,8 @@ pub fn build_unshield_transition( anchor: Anchor, prover: &P, memo: [u8; 36], - fee: Option, platform_version: &PlatformVersion, -) -> Result { +) -> Result<(StateTransition, Credits), ProtocolError> { if unshield_amount > i64::MAX as u64 { return Err(ProtocolError::ShieldedBuildError(format!( "unshield amount {} exceeds maximum allowed value {}", @@ -54,43 +55,34 @@ pub fn build_unshield_transition( let total_spent: u64 = spends.iter().map(|s| s.note.value().inner()).sum(); - // Conservative action count: at least (spends, 1) since we have a change output. - let num_actions = spends.len().max(1); - let min_fee = compute_minimum_shielded_fee(num_actions, platform_version); - let effective_fee = match fee { - Some(f) if f < min_fee => { - return Err(ProtocolError::ShieldedBuildError(format!( - "fee {} is below minimum required fee {}", - f, min_fee - ))); - } - Some(f) if f > min_fee.saturating_mul(1000) => { - return Err(ProtocolError::ShieldedBuildError(format!( - "fee {} exceeds 1000x the minimum fee {}", - f, min_fee - ))); - } - Some(f) => f, - None => min_fee, - }; - - let required = unshield_amount.checked_add(effective_fee).ok_or_else(|| { + // Orchard's BundleType::DEFAULT pads every bundle to a 2-action minimum + // (MIN_ACTIONS), so even a single-spend unshield is serialized and proven with 2 + // 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); + // The fee is fixed at the minimum: consensus always carves exactly + // `compute_minimum_shielded_fee` from the pool, and the net (`unshield_amount`) is credited + // to the output address. + let fee = compute_minimum_shielded_fee(num_actions, platform_version)?; + + let required = unshield_amount.checked_add(fee).ok_or_else(|| { ProtocolError::ShieldedBuildError("fee + unshield_amount overflows u64".to_string()) })?; if required > total_spent { return Err(ProtocolError::ShieldedBuildError(format!( "unshield amount {} + fee {} = {} exceeds total spendable value {}", - unshield_amount, effective_fee, required, total_spent + unshield_amount, fee, required, total_spent ))); } let change_amount = total_spent - required; - // Unshield extra_data = output_address || value_balance (le bytes) - // value_balance = unshield_amount + fee, becomes v0.unshielding_amount in the state transition - // Must match server-side sighash in shielded_proof.rs - let mut extra_sighash_data = output_address.to_bytes(); - extra_sighash_data.extend_from_slice(&required.to_le_bytes()); + // Bind the transparent fields (output_address, unshielding_amount == required) into the + // Orchard sighash. Shared with the consensus verifier in shielded_proof.rs so the signed + // and verified bytes cannot diverge. + let extra_sighash_data = + crate::shielded::unshield_extra_sighash_data(&output_address.to_bytes(), required); let bundle = build_spend_bundle( spends, @@ -106,7 +98,7 @@ pub fn build_unshield_transition( let sb = serialize_authorized_bundle(&bundle); - UnshieldTransition::try_from_bundle( + let state_transition = UnshieldTransition::try_from_bundle( output_address, sb.actions, sb.value_balance as u64, @@ -114,7 +106,8 @@ pub fn build_unshield_transition( sb.proof, sb.binding_signature, platform_version, - ) + )?; + Ok((state_transition, fee)) } #[cfg(test)] @@ -124,85 +117,6 @@ mod tests { test_orchard_address, test_spendable_note, TestProver, }; - #[test] - fn test_unshield_fee_below_minimum() { - let platform_version = PlatformVersion::latest(); - let change_address = test_orchard_address(); - let output_address = PlatformAddress::P2pkh([1u8; 20]); - - let note = test_spendable_note(1_000_000); - let spends = vec![note]; - - let sk = grovedb_commitment_tree::SpendingKey::from_bytes([42u8; 32]) - .expect("valid spending key bytes"); - let fvk = FullViewingKey::from(&sk); - let ask = SpendAuthorizingKey::from(&sk); - - let result = build_unshield_transition( - spends, - output_address, - 100, - &change_address, - &fvk, - &ask, - Anchor::empty_tree(), - &TestProver, - [0u8; 36], - Some(1), // fee = 1, should be below minimum - platform_version, - ); - - assert!(result.is_err()); - let err = result.unwrap_err().to_string(); - assert!( - err.contains("below minimum required fee"), - "unexpected error: {}", - err - ); - } - - #[test] - fn test_unshield_fee_above_upper_bound() { - let platform_version = PlatformVersion::latest(); - let change_address = test_orchard_address(); - let output_address = PlatformAddress::P2pkh([1u8; 20]); - - let note = test_spendable_note(u64::MAX); - let spends = vec![note]; - - let sk = grovedb_commitment_tree::SpendingKey::from_bytes([42u8; 32]) - .expect("valid spending key bytes"); - let fvk = FullViewingKey::from(&sk); - let ask = SpendAuthorizingKey::from(&sk); - - // Compute the minimum fee so we can exceed the 1000x bound - let num_actions = 1usize; - let min_fee = crate::shielded::compute_minimum_shielded_fee(num_actions, platform_version); - let excessive_fee = min_fee.saturating_mul(1000) + 1; - - let result = build_unshield_transition( - spends, - output_address, - 100, - &change_address, - &fvk, - &ask, - Anchor::empty_tree(), - &TestProver, - [0u8; 36], - Some(excessive_fee), - platform_version, - ); - - assert!(result.is_err()); - let err = result.unwrap_err().to_string(); - assert!( - err.contains("exceeds 1000x the minimum fee"), - "unexpected error: {}", - err - ); - } - #[test] fn test_unshield_insufficient_funds() { let platform_version = PlatformVersion::latest(); @@ -227,7 +141,6 @@ mod tests { Anchor::empty_tree(), &TestProver, [0u8; 36], - None, platform_version, ); @@ -267,7 +180,6 @@ mod tests { Anchor::empty_tree(), &TestProver, [0u8; 36], - None, platform_version, ); assert!(result.is_err()); @@ -279,49 +191,6 @@ mod tests { ); } - #[test] - fn test_unshield_fee_at_exact_upper_bound_passes_validation() { - // Boundary: fee == 1000x min is accepted (strictly > fails). - let platform_version = PlatformVersion::latest(); - let change_address = test_orchard_address(); - let output_address = PlatformAddress::P2pkh([1u8; 20]); - - let note = test_spendable_note(u64::MAX); - let spends = vec![note]; - - let sk = grovedb_commitment_tree::SpendingKey::from_bytes([42u8; 32]).expect("valid sk"); - let fvk = FullViewingKey::from(&sk); - let ask = SpendAuthorizingKey::from(&sk); - - let min_fee = crate::shielded::compute_minimum_shielded_fee(1, platform_version); - let boundary = min_fee.saturating_mul(1000); - - let result = build_unshield_transition( - spends, - output_address, - 100, - &change_address, - &fvk, - &ask, - Anchor::empty_tree(), - &TestProver, - [0u8; 36], - Some(boundary), - platform_version, - ); - // Boundary value passes validation; a successful build is fine. If a - // later-stage failure (anchor/add_spend) surfaces, it must NOT be the - // upper-bound error. - if let Err(err) = result { - let err = err.to_string(); - assert!( - !err.contains("exceeds 1000x"), - "boundary fee should be accepted: {}", - err - ); - } - } - #[test] fn test_unshield_amount_exceeds_spendable_with_default_fee() { // unshield_amount + default_min_fee > total_spent should surface @@ -347,7 +216,6 @@ mod tests { Anchor::empty_tree(), &TestProver, [0u8; 36], - None, platform_version, ); let err = result.unwrap_err().to_string(); @@ -378,7 +246,6 @@ mod tests { Anchor::empty_tree(), &TestProver, [0u8; 36], - None, platform_version, ); assert!(result.is_err()); @@ -399,7 +266,8 @@ mod tests { let change_address = test_orchard_address(); let output_address = PlatformAddress::P2pkh([1u8; 20]); - let min_fee = crate::shielded::compute_minimum_shielded_fee(1, platform_version); + let min_fee = crate::shielded::compute_minimum_shielded_fee(2, platform_version) + .expect("fee computation should not overflow"); let unshield_amount = 42u64; let note = test_spendable_note(unshield_amount + min_fee); let spends = vec![note]; @@ -418,7 +286,6 @@ mod tests { Anchor::empty_tree(), &TestProver, [0u8; 36], - None, platform_version, ); let err_msg = result.unwrap_err().to_string(); diff --git a/packages/rs-dpp/src/shielded/compute_minimum_shielded_fee/mod.rs b/packages/rs-dpp/src/shielded/compute_minimum_shielded_fee/mod.rs new file mode 100644 index 00000000000..bfded47732b --- /dev/null +++ b/packages/rs-dpp/src/shielded/compute_minimum_shielded_fee/mod.rs @@ -0,0 +1,33 @@ +mod v0; + +use crate::fee::Credits; +use crate::ProtocolError; +use platform_version::version::PlatformVersion; +use v0::compute_minimum_shielded_fee_v0; + +/// Computes the minimum fee (in credits) for a shielded state transition. +/// +/// Dispatches on the platform-versioned `dpp.methods.compute_minimum_shielded_fee` so the +/// fee formula can evolve across protocol versions without breaking older ones. +/// +/// This is the **single source of truth** for the shielded fee formula: the SDK builders, +/// the unshield/withdrawal transformers (for the fee actually carved from the pool), and the +/// consensus gate `validate_minimum_shielded_fee` all call it, so the carved fee and the +/// validation threshold can never drift. +/// +/// # Parameters +/// - `num_actions` — number of Orchard actions in the bundle +/// - `platform_version` — protocol version (determines the formula version and fee constants) +pub fn compute_minimum_shielded_fee( + num_actions: usize, + platform_version: &PlatformVersion, +) -> Result { + match platform_version.dpp.methods.compute_minimum_shielded_fee { + 0 => compute_minimum_shielded_fee_v0(num_actions, platform_version), + version => Err(ProtocolError::UnknownVersionMismatch { + method: "compute_minimum_shielded_fee".to_string(), + known_versions: vec![0], + received: version, + }), + } +} diff --git a/packages/rs-dpp/src/shielded/compute_minimum_shielded_fee/v0/mod.rs b/packages/rs-dpp/src/shielded/compute_minimum_shielded_fee/v0/mod.rs new file mode 100644 index 00000000000..5fe28e9df80 --- /dev/null +++ b/packages/rs-dpp/src/shielded/compute_minimum_shielded_fee/v0/mod.rs @@ -0,0 +1,46 @@ +use crate::fee::Credits; +use crate::shielded::SHIELDED_STORAGE_BYTES_PER_ACTION; +use crate::ProtocolError; +use platform_version::version::PlatformVersion; + +/// v0 of the shielded minimum-fee formula: +/// +/// `min_fee = proof_verification_fee + num_actions × (processing_fee + storage_fee)` +/// +/// where `storage_fee = SHIELDED_STORAGE_BYTES_PER_ACTION × (disk + processing) credits/byte`. +/// +/// All arithmetic is checked: an overflow (only reachable via pathological fee constants) +/// surfaces as `ProtocolError::Overflow` instead of silently wrapping. +pub fn compute_minimum_shielded_fee_v0( + num_actions: usize, + platform_version: &PlatformVersion, +) -> Result { + let constants = &platform_version + .drive_abci + .validation_and_processing + .event_constants; + let storage = &platform_version.fee_version.storage; + + let per_byte_rate = storage + .storage_disk_usage_credit_per_byte + .checked_add(storage.storage_processing_credit_per_byte) + .ok_or(ProtocolError::Overflow( + "shielded storage per-byte rate overflow", + ))?; + let storage_fee = SHIELDED_STORAGE_BYTES_PER_ACTION + .checked_mul(per_byte_rate) + .ok_or(ProtocolError::Overflow( + "shielded per-action storage fee overflow", + ))?; + let per_action = constants + .shielded_per_action_processing_fee + .checked_add(storage_fee) + .ok_or(ProtocolError::Overflow("shielded per-action fee overflow"))?; + let actions_fee = (num_actions as u64) + .checked_mul(per_action) + .ok_or(ProtocolError::Overflow("shielded actions fee overflow"))?; + constants + .shielded_proof_verification_fee + .checked_add(actions_fee) + .ok_or(ProtocolError::Overflow("shielded minimum fee overflow")) +} diff --git a/packages/rs-dpp/src/shielded/mod.rs b/packages/rs-dpp/src/shielded/mod.rs index 56f09b2e2cd..e48ee56ff1b 100644 --- a/packages/rs-dpp/src/shielded/mod.rs +++ b/packages/rs-dpp/src/shielded/mod.rs @@ -1,17 +1,44 @@ #[cfg(feature = "shielded-client")] pub mod builder; +mod compute_minimum_shielded_fee; + use bincode::{Decode, Encode}; #[cfg(feature = "serde-conversion")] use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; -use crate::fee::Credits; -use platform_version::version::PlatformVersion; +use crate::withdrawal::Pooling; + +// Re-exported so the public path stays `dpp::shielded::compute_minimum_shielded_fee` (the +// module and the function share a name but live in different namespaces). +pub use compute_minimum_shielded_fee::compute_minimum_shielded_fee; -/// Permanent storage bytes per shielded action: -/// 280 bytes in BulkAppendTree (32 cmx + 32 rho + 216 encrypted note) -/// + 32 bytes in nullifier tree = 312 bytes total. +/// Permanent storage bytes per shielded action: 312 bytes total. +/// +/// - 280 bytes in the BulkAppendTree: 32 (`cmx`, the note commitment) + 32 +/// (`rho`) + 216 (the encrypted note ciphertext). +/// - 32 bytes in the nullifier tree. +/// +/// The 216-byte encrypted note is Orchard's `TransmittedNoteCiphertext`, laid +/// out as `epk(32) || enc_ciphertext(104) || out_ciphertext(80)`: +/// +/// - `epk` (32): the note's ephemeral public key, published in the clear. The +/// recipient combines it with their incoming viewing key (Diffie–Hellman) to +/// derive the AEAD key. +/// - `enc_ciphertext` (104): the note encrypted to the recipient (opened with +/// the incoming viewing key) — ChaCha20-Poly1305 over the note plaintext. It +/// holds the compact note (52 = version 1 + diversifier `d` 11 + value 8 + +/// `rseed` 32), the memo (36), and the AEAD tag (16); the 52-byte compact +/// prefix is what wallets trial-decrypt during sync to detect their own notes. +/// - `out_ciphertext` (80): the note encrypted to the sender for wallet +/// recovery (opened with the outgoing viewing key): out plaintext +/// (64 = `pk_d` 32 + `esk` 32) + AEAD tag (16). +/// +/// This is the standard Orchard layout except the memo is 36 bytes (`DashMemo`) +/// instead of Zcash's 512 — the dashpay `orchard` fork makes the memo size a +/// type parameter (`MemoSize`) — which is why each note is 216 bytes +/// (`ENCRYPTED_NOTE_SIZE`) rather than Zcash Orchard's ~692. pub const SHIELDED_STORAGE_BYTES_PER_ACTION: u64 = 312; /// Domain separator for Platform sighash computation. @@ -39,29 +66,53 @@ pub fn compute_platform_sighash(bundle_commitment: &[u8; 32], extra_data: &[u8]) hasher.finalize().into() } -/// Computes the minimum fee (in credits) for a shielded state transition. +/// Builds the transparent `extra_data` bound into a ShieldedWithdrawal's platform +/// sighash, with the byte layout +/// `output_script || unshielding_amount (u64 LE) || core_fee_per_byte (u32 LE) || pooling (u8)`. /// -/// The fee formula mirrors the on-chain validation in `validate_minimum_shielded_fee`: -/// `min_fee = proof_verification_fee + num_actions × (processing_fee + storage_fee)` +/// Every field here is written verbatim by the transformer into the queued withdrawal +/// document that constructs the Core asset-unlock TxOut. Binding all of them into the +/// Orchard sighash means the binding signature authorizes them: since ShieldedWithdrawal +/// has no identity-key signature and no address-witness check, the Orchard signature is +/// the only authorization boundary, so a relay or block proposer cannot malleate +/// `core_fee_per_byte` (or `pooling`, were it ever unpinned from `Never`) — e.g. flip a +/// user's `core_fee_per_byte = 1` to a much larger Fibonacci value to redirect the +/// withdrawn amount into L1 miner fees — without invalidating the proof. /// -/// where `storage_fee = SHIELDED_STORAGE_BYTES_PER_ACTION × (disk + processing) credits/byte`. +/// The signing (client/builder) and verifying (consensus) sides MUST produce identical +/// bytes, so both call this single function. /// -/// # Parameters -/// - `num_actions` — number of Orchard actions in the bundle -/// - `platform_version` — protocol version (determines fee constants) -pub fn compute_minimum_shielded_fee( - num_actions: usize, - platform_version: &PlatformVersion, -) -> Credits { - let constants = &platform_version - .drive_abci - .validation_and_processing - .event_constants; - let storage = &platform_version.fee_version.storage; - let storage_fee = SHIELDED_STORAGE_BYTES_PER_ACTION - * (storage.storage_disk_usage_credit_per_byte + storage.storage_processing_credit_per_byte); - let per_action = constants.shielded_per_action_processing_fee + storage_fee; - constants.shielded_proof_verification_fee + num_actions as u64 * per_action +/// The layout places the variable-length `output_script` first with no length prefix. This +/// is unambiguous only because `validate_structure` runs before proof verification and pins +/// `output_script` to a canonical, fixed-length P2PKH (25 bytes) or P2SH (23 bytes); the +/// remaining fields are fixed-width, so the preimage is well-defined for every accepted +/// transition. If that script-shape restriction is ever relaxed, add a length prefix here. +pub fn shielded_withdrawal_extra_sighash_data( + output_script: &[u8], + unshielding_amount: u64, + core_fee_per_byte: u32, + pooling: Pooling, +) -> Vec { + let mut data = Vec::with_capacity(output_script.len() + 8 + 4 + 1); + data.extend_from_slice(output_script); + data.extend_from_slice(&unshielding_amount.to_le_bytes()); + data.extend_from_slice(&core_fee_per_byte.to_le_bytes()); + data.push(pooling as u8); + data +} + +/// Builds the transparent `extra_data` bound into an Unshield's platform sighash, with the +/// byte layout `output_address || unshielding_amount (u64 LE)`. +/// +/// As with [`shielded_withdrawal_extra_sighash_data`], the signing (client/builder) and +/// verifying (consensus) sides MUST produce identical bytes, so both call this single +/// function. Unshield credits a transparent platform address (not a Core asset-unlock +/// `TxOut`), so it carries no `core_fee_per_byte`/`pooling` to bind. +pub fn unshield_extra_sighash_data(output_address: &[u8], unshielding_amount: u64) -> Vec { + let mut data = Vec::with_capacity(output_address.len() + 8); + data.extend_from_slice(output_address); + data.extend_from_slice(&unshielding_amount.to_le_bytes()); + data } /// Common Orchard bundle parameters shared across all shielded transition types. @@ -155,3 +206,57 @@ pub struct SerializedAction { /// signature from one transition cannot be reused in another. pub spend_auth_sig: [u8; 64], } + +#[cfg(test)] +mod tests { + use super::*; + use crate::identity::core_script::CoreScript; + use crate::withdrawal::Pooling; + + #[test] + fn withdrawal_sighash_data_binds_core_fee_per_byte() { + let script = CoreScript::new_p2pkh([1u8; 20]); + let a = shielded_withdrawal_extra_sighash_data(script.as_bytes(), 1000, 1, Pooling::Never); + let b = shielded_withdrawal_extra_sighash_data(script.as_bytes(), 1000, 2, Pooling::Never); + assert_ne!( + a, b, + "changing core_fee_per_byte must change the sighash preimage" + ); + } + + #[test] + fn withdrawal_sighash_data_binds_pooling() { + // `pooling` is pinned to `Never` by `validate_structure`, so this binding is currently + // dead defense-in-depth; assert it is nonetheless mixed into the preimage so a future + // unpinning would still be authorized by the Orchard binding signature. + let script = CoreScript::new_p2pkh([1u8; 20]); + let a = shielded_withdrawal_extra_sighash_data(script.as_bytes(), 1000, 1, Pooling::Never); + let b = shielded_withdrawal_extra_sighash_data( + script.as_bytes(), + 1000, + 1, + Pooling::IfAvailable, + ); + assert_ne!(a, b, "changing pooling must change the sighash preimage"); + } + + #[test] + fn withdrawal_sighash_data_layout() { + // output_script(2) || unshielding_amount(8) || core_fee_per_byte(4) || pooling(1) + let d = shielded_withdrawal_extra_sighash_data(&[0xAA, 0xBB], 1, 2, Pooling::Never); + assert_eq!(d.len(), 2 + 8 + 4 + 1); + assert_eq!(&d[0..2], &[0xAA, 0xBB]); + assert_eq!(&d[2..10], &1u64.to_le_bytes()); + assert_eq!(&d[10..14], &2u32.to_le_bytes()); + assert_eq!(d[14], Pooling::Never as u8); + } + + #[test] + fn unshield_sighash_data_layout() { + // output_address || unshielding_amount(8) + let d = unshield_extra_sighash_data(&[0xAA, 0xBB, 0xCC], 5); + assert_eq!(d.len(), 3 + 8); + assert_eq!(&d[0..3], &[0xAA, 0xBB, 0xCC]); + assert_eq!(&d[3..11], &5u64.to_le_bytes()); + } +} diff --git a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_credit_withdrawal_transition/mod.rs b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_credit_withdrawal_transition/mod.rs index f01ba6c4a9c..436d71adce8 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_credit_withdrawal_transition/mod.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_credit_withdrawal_transition/mod.rs @@ -32,7 +32,11 @@ use serde::{Deserialize, Serialize}; /// Minimal core per byte. Must be a fibonacci number pub const MIN_CORE_FEE_PER_BYTE: u32 = 1; -/// Minimal amount in credits (x1000) to avoid "dust" error in Core +/// Minimal amount in credits (x1000) to avoid "dust" error in Core. +/// +/// NOTE: This is the protocol-v11-and-below floor (190 duffs). Consensus reads the +/// *versioned* `platform_version.system_limits.min_withdrawal_amount` (raised to 1000 duffs +/// in v12); keep `SYSTEM_LIMITS_V1.min_withdrawal_amount` in sync with this value. pub const MIN_WITHDRAWAL_AMOUNT: u64 = (ASSET_UNLOCK_TX_SIZE as u64) * (MIN_CORE_FEE_PER_BYTE as u64) * CREDITS_PER_DUFF; diff --git a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_credit_withdrawal_transition/v0/state_transition_validation.rs b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_credit_withdrawal_transition/v0/state_transition_validation.rs index b172e876d59..5eba3e5f998 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_credit_withdrawal_transition/v0/state_transition_validation.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_credit_withdrawal_transition/v0/state_transition_validation.rs @@ -13,9 +13,7 @@ use crate::consensus::basic::state_transition::{ }; use crate::consensus::basic::BasicError; use crate::state_transition::address_credit_withdrawal_transition::v0::AddressCreditWithdrawalTransitionV0; -use crate::state_transition::address_credit_withdrawal_transition::{ - MIN_CORE_FEE_PER_BYTE, MIN_WITHDRAWAL_AMOUNT, -}; +use crate::state_transition::address_credit_withdrawal_transition::MIN_CORE_FEE_PER_BYTE; use crate::state_transition::StateTransitionStructureValidation; use crate::util::is_non_zero_fibonacci_number::is_non_zero_fibonacci_number; use crate::validation::SimpleConsensusValidationResult; @@ -227,13 +225,13 @@ impl StateTransitionStructureValidation for AddressCreditWithdrawalTransitionV0 // Validate withdrawal amount meets minimum and maximum let withdrawal_amount = input_sum - output_amount; // Safe: checked input_sum > output_amount above - if withdrawal_amount < MIN_WITHDRAWAL_AMOUNT + if withdrawal_amount < platform_version.system_limits.min_withdrawal_amount || withdrawal_amount > platform_version.system_limits.max_withdrawal_amount { return SimpleConsensusValidationResult::new_with_error( BasicError::WithdrawalBelowMinAmountError(WithdrawalBelowMinAmountError::new( withdrawal_amount, - MIN_WITHDRAWAL_AMOUNT, + platform_version.system_limits.min_withdrawal_amount, platform_version.system_limits.max_withdrawal_amount, )) .into(), diff --git a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_withdrawal_transition/mod.rs b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_withdrawal_transition/mod.rs index 2f7c33a5bd8..46581cf532d 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_withdrawal_transition/mod.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_withdrawal_transition/mod.rs @@ -40,7 +40,11 @@ use serde::{Deserialize, Serialize}; /// Minimal core per byte. Must be a fibonacci number pub const MIN_CORE_FEE_PER_BYTE: u32 = 1; -/// Minimal amount in credits (x1000) to avoid "dust" error in Core +/// Minimal amount in credits (x1000) to avoid "dust" error in Core. +/// +/// NOTE: This is the protocol-v11-and-below floor (190 duffs). Consensus reads the +/// *versioned* `platform_version.system_limits.min_withdrawal_amount` (raised to 1000 duffs +/// in v12); keep `SYSTEM_LIMITS_V1.min_withdrawal_amount` in sync with this value. pub const MIN_WITHDRAWAL_AMOUNT: u64 = (ASSET_UNLOCK_TX_SIZE as u64) * (MIN_CORE_FEE_PER_BYTE as u64) * CREDITS_PER_DUFF; diff --git a/packages/rs-dpp/src/state_transition/state_transitions/shielded/shielded_withdrawal_transition/v0/state_transition_validation.rs b/packages/rs-dpp/src/state_transition/state_transitions/shielded/shielded_withdrawal_transition/v0/state_transition_validation.rs index 8f725c9a067..4e9a67ffc3c 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/shielded/shielded_withdrawal_transition/v0/state_transition_validation.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/shielded/shielded_withdrawal_transition/v0/state_transition_validation.rs @@ -1,12 +1,20 @@ +use crate::consensus::basic::identity::{ + InvalidCreditWithdrawalTransitionCoreFeeError, + InvalidCreditWithdrawalTransitionOutputScriptError, + NotImplementedCreditWithdrawalTransitionPoolingError, +}; use crate::consensus::basic::state_transition::ShieldedInvalidValueBalanceError; use crate::consensus::basic::BasicError; +use crate::state_transition::identity_credit_withdrawal_transition::MIN_CORE_FEE_PER_BYTE; use crate::state_transition::shielded_withdrawal_transition::v0::ShieldedWithdrawalTransitionV0; use crate::state_transition::state_transitions::shielded::common_validation::{ validate_actions_count, validate_anchor_not_zero, validate_encrypted_note_sizes, validate_proof_not_empty, }; use crate::state_transition::StateTransitionStructureValidation; +use crate::util::is_non_zero_fibonacci_number::is_non_zero_fibonacci_number; use crate::validation::SimpleConsensusValidationResult; +use crate::withdrawal::Pooling; use platform_version::version::PlatformVersion; impl StateTransitionStructureValidation for ShieldedWithdrawalTransitionV0 { @@ -67,6 +75,49 @@ impl StateTransitionStructureValidation for ShieldedWithdrawalTransitionV0 { return result; } + // The shielded withdrawal carries the same transparent, Core-facing fields as + // IdentityCreditWithdrawal (output_script, pooling, core_fee_per_byte), and the + // transformer writes them straight into the queued withdrawal document that drives + // the Core asset-unlock TxOut. Mirror the transparent path's invariants so a valid + // Orchard proof cannot enqueue a non-standard/oversized output script (storage + // griefing at the flat shielded fee), an unsupported pooling discriminant, or an + // unrelayable/zero core_fee_per_byte. + + // Pooling is not yet supported: must be Never. + if self.pooling != Pooling::Never { + return SimpleConsensusValidationResult::new_with_error( + BasicError::NotImplementedCreditWithdrawalTransitionPoolingError( + NotImplementedCreditWithdrawalTransitionPoolingError::new(self.pooling as u8), + ) + .into(), + ); + } + + // core_fee_per_byte must be a non-zero Fibonacci number. + if !is_non_zero_fibonacci_number(self.core_fee_per_byte as u64) { + return SimpleConsensusValidationResult::new_with_error( + BasicError::InvalidCreditWithdrawalTransitionCoreFeeError( + InvalidCreditWithdrawalTransitionCoreFeeError::new( + self.core_fee_per_byte, + MIN_CORE_FEE_PER_BYTE, + ), + ) + .into(), + ); + } + + // output_script must be a canonical P2PKH or P2SH script. + if !self.output_script.is_p2pkh() && !self.output_script.is_p2sh() { + return SimpleConsensusValidationResult::new_with_error( + BasicError::InvalidCreditWithdrawalTransitionOutputScriptError( + InvalidCreditWithdrawalTransitionOutputScriptError::new( + self.output_script.clone(), + ), + ) + .into(), + ); + } + SimpleConsensusValidationResult::new() } } @@ -236,4 +287,79 @@ mod tests { )] ); } + + #[test] + fn should_reject_non_never_pooling() { + let platform_version = PlatformVersion::latest(); + let mut transition = valid_shielded_withdrawal_transition(); + transition.pooling = Pooling::IfAvailable; + + let result = transition.validate_structure(platform_version); + assert_matches!( + result.errors.as_slice(), + [ConsensusError::BasicError( + BasicError::NotImplementedCreditWithdrawalTransitionPoolingError(_) + )] + ); + } + + #[test] + fn should_reject_zero_core_fee_per_byte() { + let platform_version = PlatformVersion::latest(); + let mut transition = valid_shielded_withdrawal_transition(); + transition.core_fee_per_byte = 0; + + let result = transition.validate_structure(platform_version); + assert_matches!( + result.errors.as_slice(), + [ConsensusError::BasicError( + BasicError::InvalidCreditWithdrawalTransitionCoreFeeError(_) + )] + ); + } + + #[test] + fn should_reject_non_fibonacci_core_fee_per_byte() { + let platform_version = PlatformVersion::latest(); + let mut transition = valid_shielded_withdrawal_transition(); + transition.core_fee_per_byte = 4; // 4 is not a Fibonacci number + + let result = transition.validate_structure(platform_version); + assert_matches!( + result.errors.as_slice(), + [ConsensusError::BasicError( + BasicError::InvalidCreditWithdrawalTransitionCoreFeeError(_) + )] + ); + } + + #[test] + fn should_reject_non_standard_output_script() { + let platform_version = PlatformVersion::latest(); + let mut transition = valid_shielded_withdrawal_transition(); + // OP_RETURN-style script: neither P2PKH nor P2SH. + transition.output_script = CoreScript::from_bytes(vec![0x6a, 0x01, 0x02]); + + let result = transition.validate_structure(platform_version); + assert_matches!( + result.errors.as_slice(), + [ConsensusError::BasicError( + BasicError::InvalidCreditWithdrawalTransitionOutputScriptError(_) + )] + ); + } + + #[test] + fn should_accept_p2sh_output_script() { + let platform_version = PlatformVersion::latest(); + let mut transition = valid_shielded_withdrawal_transition(); + transition.output_script = CoreScript::new_p2sh([12u8; 20]); + + let result = transition.validate_structure(platform_version); + assert!( + result.is_valid(), + "Expected valid result, got errors: {:?}", + result.errors + ); + } } diff --git a/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/execute_event/v0/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/execute_event/v0/mod.rs index 02bb9150152..3f1a4d5d112 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/execute_event/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/execute_event/v0/mod.rs @@ -516,7 +516,8 @@ where .. } => { if consensus_errors.is_empty() { - self.drive + let applied_fees = self + .drive .apply_drive_operations( operations, true, @@ -527,9 +528,19 @@ where ) .map_err(Error::Drive)?; + // Split the carved fee like every other transition: the real storage + // cost of the (permanent) shielded writes goes to the storage pool, so it + // is amortised to the validators that store it over time and picks up the + // epoch fee multiplier at payout; the remainder (proof verification + + // per-action processing) is the processing fee paid to the current + // proposer. Conservation: storage + processing == fees_to_add_to_pool + // (what was carved from the shielded pool). + let storage_fee = applied_fees.storage_fee.min(fees_to_add_to_pool); + let processing_fee = fees_to_add_to_pool - storage_fee; + Ok(SuccessfulPaidExecution( None, - FeeResult::default_with_fees(0, fees_to_add_to_pool), + FeeResult::default_with_fees(storage_fee, processing_fee), )) } else { Ok(UnpaidConsensusExecutionError(consensus_errors)) @@ -546,7 +557,8 @@ where all_errors.extend(consensus_errors); if all_errors.is_empty() { - self.drive + let applied_fees = self + .drive .apply_drive_operations( operations, true, @@ -557,9 +569,27 @@ where ) .map_err(Error::Drive)?; + // Route the real storage cost of the shielded writes to the storage pool + // (amortised over time, epoch fee multiplier applied at payout); the + // remainder (the asset-lock excess over the shield amount) is the + // processing fee. Conservation: storage + processing == fees_to_add_to_pool. + // + // EDGE CASE: `fees_to_add_to_pool` here is the asset-lock excess the shield + // declares (gated by a flat min-fee that does NOT scale with action count), + // not `compute_minimum_shielded_fee`. If that excess is smaller than the real + // storage cost of the writes (≈ SHIELDED_STORAGE_BYTES_PER_ACTION per action), + // `storage_fee` saturates via `min(..)` and `processing_fee` becomes 0 — the + // proposer earns nothing for the proof verification it ran. This has no + // soundness/conservation impact (the pool is never over- or under-credited), + // but a high-action shield funded at exactly the minimum is a proposer-incentive + // edge; the flat shield min-fee should be revisited if it ceases to cover the + // per-action write cost. + let storage_fee = applied_fees.storage_fee.min(fees_to_add_to_pool); + let processing_fee = fees_to_add_to_pool - storage_fee; + Ok(SuccessfulPaidExecution( None, - FeeResult::default_with_fees(0, fees_to_add_to_pool), + FeeResult::default_with_fees(storage_fee, processing_fee), )) } else { Ok(UnpaidConsensusExecutionError(all_errors)) diff --git a/packages/rs-drive-abci/src/execution/types/execution_event/mod.rs b/packages/rs-drive-abci/src/execution/types/execution_event/mod.rs index d79b07e1213..22667e6dcb0 100644 --- a/packages/rs-drive-abci/src/execution/types/execution_event/mod.rs +++ b/packages/rs-drive-abci/src/execution/types/execution_event/mod.rs @@ -71,6 +71,16 @@ pub(in crate::execution) enum ExecutionEvent<'a> { /// The fee is embedded in the ZK-proven value_balance and validated /// at the processor level (validate_minimum_shielded_fee). /// Nullifiers are stored to recent block storage as part of the drive operations. + /// + /// This variant deliberately carries no `user_fee_increase`. Shielded transitions + /// have no fee-bidding or priority market: the fee is pinned to the flat, + /// client-predictable `compute_minimum_shielded_fee` (transfers must set + /// `value_balance` to exactly that minimum, while unshields and withdrawals derive it + /// from the action count), so every shielded transition of a given size pays an + /// identical fee and no fee fingerprint can distinguish senders. It likewise carries + /// no `execution_operations`: shielded transitions are not charged the per-operation + /// GroveDB cost (the flat fee subsumes it), so the execution context is intentionally + /// not threaded through here. PaidFromShieldedPool { /// the operations that should be performed operations: Vec>, diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/processor/traits/shielded_proof.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/processor/traits/shielded_proof.rs index 816b50b068e..c4e01603ae4 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/processor/traits/shielded_proof.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/processor/traits/shielded_proof.rs @@ -3,9 +3,13 @@ use crate::error::Error; use crate::execution::validation::state_transition::state_transitions::shielded_common::{ reconstruct_and_verify_bundle, FLAGS_OUTPUTS_ONLY, FLAGS_SPENDS_AND_OUTPUTS, }; +use dpp::consensus::basic::identity::InvalidIdentityCreditWithdrawalTransitionAmountError; +use dpp::consensus::basic::state_transition::{ + ShieldedInvalidValueBalanceError, WithdrawalBelowMinAmountError, +}; +use dpp::consensus::basic::BasicError; use dpp::consensus::state::shielded::insufficient_shielded_fee_error::InsufficientShieldedFeeError; use dpp::consensus::state::state_error::StateError; -use dpp::shielded::SHIELDED_STORAGE_BYTES_PER_ACTION; use dpp::state_transition::StateTransition; use dpp::validation::SimpleConsensusValidationResult; use dpp::version::PlatformVersion; @@ -68,11 +72,25 @@ impl StateTransitionHasShieldedProofValidationV0 for StateTransition { /// The minimum fee is computed dynamically based on the number of actions: /// min_fee = proof_verification_fee + num_actions × (processing_fee + storage_fee) /// -/// The fee is derived from the public `value_balance` field (no ZK proof execution needed): -/// - ShieldedTransfer: fee = value_balance -/// - Unshield: fee = value_balance - amount -/// - ShieldedWithdrawal: fee = value_balance - amount -/// - Shield: fee paid by transparent address inputs (skipped here) +/// The amount checked against `min_fee` is derived from public fields (no ZK proof +/// execution needed): +/// - ShieldedTransfer: the whole `value_balance` is the fee, so we require +/// `value_balance == min_fee` *exactly*. There is no recipient to absorb an excess, so a +/// variable fee would only burn credits and leak a distinguishing fee fingerprint that +/// breaks shielded uniformity — overpayment is rejected. (Unshield/Withdrawal use `>=` +/// because their excess is the recipient/net amount.) +/// - Unshield / ShieldedWithdrawal: `unshielding_amount` is the TOTAL value leaving the +/// shielded pool (recipient/net amount + fee). The fee actually charged at execution time +/// is `compute_minimum_shielded_fee` (carved out of `unshielding_amount`), and the +/// recipient/net receives `unshielding_amount - fee`. Requiring `unshielding_amount >= min_fee` +/// guarantees that net amount is non-negative. +/// - ShieldedWithdrawal additionally requires the net (`unshielding_amount - min_fee`) to +/// fall within `[min_withdrawal_amount, max_withdrawal_amount]` — the same two-sided range +/// the transparent withdrawal paths enforce — because that net becomes a Core `TxOut`: the +/// dust floor stops a zero/sub-dust queue entry, and the per-transition policy cap stops a +/// single withdrawal from exceeding the protocol's maximum. Unshield has no such range: its +/// net is credited to a platform address, not Core. +/// - Shield: fee paid by transparent address inputs (skipped here). pub(crate) trait StateTransitionShieldedMinimumFeeValidationV0 { fn validate_minimum_shielded_fee( &self, @@ -91,8 +109,20 @@ impl StateTransitionShieldedMinimumFeeValidationV0 for StateTransition { .validate_minimum_shielded_fee { 0 => { - // Extract the fee and action count from the transition. - let (fee, num_actions): (i64, usize) = match self { + // Destructure per shielded transition type: + // - `validated_amount`: the amount field this type carries. For ShieldedTransfer + // it IS the fee (`value_balance`); for Unshield/ShieldedWithdrawal it is the + // GROSS leaving the pool (`unshielding_amount` = recipient/net + fee). + // - `amount_is_pure_fee`: true only when `validated_amount` is the fee itself + // (ShieldedTransfer), so it must equal the minimum exactly — there is no + // recipient amount and overpaying is disallowed. False for the gross-carrying + // types, where the excess over the fee IS the recipient/net amount. + // - `min_net_amount`/`max_net_amount`: the allowed range for the derived net + // (`validated_amount - min_fee`). `[0, u64::MAX]` (a no-op) for every type + // except ShieldedWithdrawal, whose net becomes a Core `TxOut` and must clear + // the same `[min_withdrawal_amount, max_withdrawal_amount]` range the + // transparent withdrawal paths enforce. + let (validated_amount, num_actions, min_net_amount, max_net_amount, amount_is_pure_fee): (i64, usize, u64, u64, bool) = match self { // Shield: fee is paid from transparent address inputs, not from value_balance. StateTransition::Shield(_) => { return Ok(SimpleConsensusValidationResult::new()) @@ -100,79 +130,142 @@ impl StateTransitionShieldedMinimumFeeValidationV0 for StateTransition { // ShieldedTransfer: value_balance (u64) IS the fee. StateTransition::ShieldedTransfer(st) => match st { dpp::state_transition::shielded_transfer_transition::ShieldedTransferTransition::V0(v0) => { - (v0.value_balance as i64, v0.actions.len()) + (v0.value_balance as i64, v0.actions.len(), 0, u64::MAX, true) } }, - // Unshield: fee = value_balance - amount. + // Unshield: `unshielding_amount` is the TOTAL leaving the pool + // (recipient/net + fee). We check it against `min_fee` so the net + // (`unshielding_amount - compute_minimum_shielded_fee`) credited to + // the recipient at execution time is non-negative. The net is credited + // to a platform address (not Core), so no withdrawal range applies. StateTransition::Unshield(st) => match st { dpp::state_transition::unshield_transition::UnshieldTransition::V0( v0, - ) => { - // unshielding_amount is the total leaving the pool (fee is validated separately) - (v0.unshielding_amount as i64, v0.actions.len()) - } + ) => (v0.unshielding_amount as i64, v0.actions.len(), 0, u64::MAX, false), }, + // ShieldedWithdrawal: the net (`unshielding_amount - min_fee`) becomes a + // Core `TxOut`, so it must fall within the same + // `[min_withdrawal_amount, max_withdrawal_amount]` range the transparent + // withdrawal paths enforce (dust floor and the per-transition policy cap). StateTransition::ShieldedWithdrawal(st) => match st { dpp::state_transition::shielded_withdrawal_transition::ShieldedWithdrawalTransition::V0(v0) => { - (v0.unshielding_amount as i64, v0.actions.len()) + ( + v0.unshielding_amount as i64, + v0.actions.len(), + platform_version.system_limits.min_withdrawal_amount, + platform_version.system_limits.max_withdrawal_amount, + false, + ) } }, // Other transitions don't go through shielded fee validation. _ => return Ok(SimpleConsensusValidationResult::new()), }; + // Defensive overflow guard. `value_balance`/`unshielding_amount` are bounded to + // `<= i64::MAX` by basic structure validation, which runs (with a hard + // early-return) before this stage, so the `as i64` casts above are non-negative + // for validated input. Re-check here so a future reordering or a direct caller + // cannot slip a value `> i64::MAX` past as a wrapped-negative `i64` (which would + // then wrap back to a huge `u64` and sail past the min-fee check below). + if validated_amount < 0 { + return Ok(SimpleConsensusValidationResult::new_with_error( + BasicError::ShieldedInvalidValueBalanceError( + ShieldedInvalidValueBalanceError::new( + "shielded value_balance/unshielding_amount exceeds the maximum \ + allowed value (i64::MAX)" + .to_string(), + ), + ) + .into(), + )); + } + let constants = &platform_version .drive_abci .validation_and_processing .event_constants; - // Storage fee per action: 312 bytes (280 BulkAppendTree + 32 nullifier) - // × (storage_disk_usage_credit_per_byte + storage_processing_credit_per_byte) - let storage_costs = &platform_version.fee_version.storage; - let storage_fee_per_action = storage_costs - .storage_disk_usage_credit_per_byte - .checked_add(storage_costs.storage_processing_credit_per_byte) - .and_then(|sum| SHIELDED_STORAGE_BYTES_PER_ACTION.checked_mul(sum)) - .ok_or(Error::Execution(ExecutionError::Overflow( - "storage fee per action overflow in shielded fee calculation", - )))?; - - // min_fee = proof_verification_fee + num_actions × (processing_fee + storage_fee) - let per_action_fee = constants - .shielded_per_action_processing_fee - .checked_add(storage_fee_per_action) - .ok_or(Error::Execution(ExecutionError::Overflow( - "per-action fee overflow in shielded fee calculation", - )))?; - let minimum_shielded_fee = (num_actions as u64) - .checked_mul(per_action_fee) - .and_then(|actions_fee| { - constants - .shielded_proof_verification_fee - .checked_add(actions_fee) - }) - .ok_or(Error::Execution(ExecutionError::Overflow( - "minimum shielded fee overflow in shielded fee calculation", - )))?; - - if (fee as u64) < minimum_shielded_fee { - Ok(SimpleConsensusValidationResult::new_with_error( + // Single source of truth for the consensus fee formula. The SDK builders + // and the unshield/withdrawal transformers carve the fee with this exact + // checked computation, so the validation threshold here can never drift from + // the fee that is actually charged. See `dpp::shielded::compute_minimum_shielded_fee`. + let minimum_shielded_fee = + dpp::shielded::compute_minimum_shielded_fee(num_actions, platform_version)?; + + if (validated_amount as u64) < minimum_shielded_fee { + return Ok(SimpleConsensusValidationResult::new_with_error( StateError::InsufficientShieldedFeeError( InsufficientShieldedFeeError::new(format!( - "shielded transition fee {} is below minimum required fee {} \ - ({} proof + {} actions × {} per-action)", - fee, + "shielded transition amount {} is below the minimum required fee \ + {} ({} proof-verification + {} actions)", + validated_amount, minimum_shielded_fee, constants.shielded_proof_verification_fee, num_actions, - per_action_fee, )), ) .into(), - )) - } else { - Ok(SimpleConsensusValidationResult::new()) + )); + } + + // ShieldedTransfer: `value_balance` IS the entire fee (there is no recipient + // amount), so it must equal the minimum exactly. Allowing `value_balance > + // min_fee` would let a transfer overpay for no benefit and leak a fee + // fingerprint that breaks shielded uniformity — reject it. Unshield/Withdrawal + // are exempt: their excess over `min_fee` is the recipient/net amount. + if amount_is_pure_fee && (validated_amount as u64) > minimum_shielded_fee { + return Ok(SimpleConsensusValidationResult::new_with_error( + BasicError::ShieldedInvalidValueBalanceError( + ShieldedInvalidValueBalanceError::new(format!( + "shielded transfer value_balance {} must equal the minimum \ + shielded fee {} exactly ({} proof-verification + {} actions); \ + overpayment is not allowed", + validated_amount, + minimum_shielded_fee, + constants.shielded_proof_verification_fee, + num_actions, + )), + ) + .into(), + )); + } + + // For ShieldedWithdrawal, the net value leaving to Core + // (`unshielding_amount - min_fee`) must fall within the same + // `[min_withdrawal_amount, max_withdrawal_amount]` range the transparent + // withdrawal paths enforce — the dust floor AND the per-transition policy + // cap. For the other shielded paths the range is `[0, u64::MAX]`, so both + // checks are no-ops. + // Safe: `validated_amount as u64 >= minimum_shielded_fee` was just checked above. + let net_amount = (validated_amount as u64) - minimum_shielded_fee; + if net_amount < min_net_amount { + return Ok(SimpleConsensusValidationResult::new_with_error( + BasicError::WithdrawalBelowMinAmountError( + WithdrawalBelowMinAmountError::new( + net_amount, + min_net_amount, + max_net_amount, + ), + ) + .into(), + )); + } + if net_amount > max_net_amount { + // Over the per-transition withdrawal cap. Use the same range error the + // transparent withdrawal path uses so the rejection reason is accurate + // (an over-max amount is not "below min"). + return Ok(SimpleConsensusValidationResult::new_with_error( + InvalidIdentityCreditWithdrawalTransitionAmountError::new( + net_amount, + min_net_amount, + max_net_amount, + ) + .into(), + )); } + + Ok(SimpleConsensusValidationResult::new()) } version => Err(Error::Execution(ExecutionError::UnknownVersionMismatch { method: "StateTransition::validate_minimum_shielded_fee".to_string(), @@ -223,9 +316,10 @@ impl StateTransitionShieldedProofValidationV0 for StateTransition { }, StateTransition::Unshield(st) => match st { dpp::state_transition::unshield_transition::UnshieldTransition::V0(v0) => { - let mut extra_sighash_data = v0.output_address.to_bytes(); - extra_sighash_data - .extend_from_slice(&v0.unshielding_amount.to_le_bytes()); + let extra_sighash_data = dpp::shielded::unshield_extra_sighash_data( + &v0.output_address.to_bytes(), + v0.unshielding_amount, + ); reconstruct_and_verify_bundle( &v0.actions, FLAGS_SPENDS_AND_OUTPUTS, @@ -239,10 +333,13 @@ impl StateTransitionShieldedProofValidationV0 for StateTransition { }, StateTransition::ShieldedWithdrawal(st) => match st { dpp::state_transition::shielded_withdrawal_transition::ShieldedWithdrawalTransition::V0(v0) => { - let mut extra_sighash_data = - v0.output_script.as_bytes().to_vec(); - extra_sighash_data - .extend_from_slice(&v0.unshielding_amount.to_le_bytes()); + let extra_sighash_data = + dpp::shielded::shielded_withdrawal_extra_sighash_data( + v0.output_script.as_bytes(), + v0.unshielding_amount, + v0.core_fee_per_byte, + v0.pooling, + ); reconstruct_and_verify_bundle( &v0.actions, FLAGS_SPENDS_AND_OUTPUTS, @@ -451,6 +548,23 @@ mod tests { mod validate_minimum_shielded_fee { use super::*; + use dpp::consensus::ConsensusError; + + /// A ShieldedWithdrawal transition (no actions) with the given gross amount. + fn shielded_withdrawal_with_amount(unshielding_amount: u64) -> StateTransition { + StateTransition::ShieldedWithdrawal(ShieldedWithdrawalTransition::V0( + ShieldedWithdrawalTransitionV0 { + actions: vec![], + unshielding_amount, + anchor: [0u8; 32], + proof: vec![], + binding_signature: [0u8; 64], + core_fee_per_byte: 0, + pooling: Default::default(), + output_script: Default::default(), + }, + )) + } #[test] fn should_pass_for_non_shielded_transition() { @@ -461,6 +575,113 @@ mod tests { .expect("should not error"); assert!(result.is_valid()); } + + #[test] + fn should_reject_shielded_withdrawal_with_net_below_min_withdrawal_amount() { + let platform_version = PlatformVersion::latest(); + let min_fee = dpp::shielded::compute_minimum_shielded_fee(0, platform_version) + .expect("fee computation should not overflow"); + let min_withdrawal = platform_version.system_limits.min_withdrawal_amount; + // net = min_withdrawal_amount - 1 → just below the Core dust floor. + let st = shielded_withdrawal_with_amount(min_fee + min_withdrawal - 1); + let result = st + .validate_minimum_shielded_fee(platform_version) + .expect("should not error"); + assert!(!result.is_valid()); + assert!( + matches!( + result.errors.first(), + Some(ConsensusError::BasicError( + BasicError::WithdrawalBelowMinAmountError(_) + )) + ), + "below-min must reject with WithdrawalBelowMinAmountError; got {:?}", + result.errors + ); + } + + #[test] + fn should_accept_shielded_withdrawal_with_net_at_min_withdrawal_amount() { + let platform_version = PlatformVersion::latest(); + let min_fee = dpp::shielded::compute_minimum_shielded_fee(0, platform_version) + .expect("fee computation should not overflow"); + let min_withdrawal = platform_version.system_limits.min_withdrawal_amount; + // net = min_withdrawal_amount exactly → at the floor, accepted. + let st = shielded_withdrawal_with_amount(min_fee + min_withdrawal); + let result = st + .validate_minimum_shielded_fee(platform_version) + .expect("should not error"); + assert!( + result.is_valid(), + "a withdrawal whose net equals min_withdrawal_amount must be accepted" + ); + } + + #[test] + fn should_reject_shielded_withdrawal_with_net_above_max_withdrawal_amount() { + let platform_version = PlatformVersion::latest(); + let min_fee = dpp::shielded::compute_minimum_shielded_fee(0, platform_version) + .expect("fee computation should not overflow"); + let max = platform_version.system_limits.max_withdrawal_amount; + // net = max_withdrawal_amount + 1 → just over the per-transition policy cap. + let st = shielded_withdrawal_with_amount(min_fee + max + 1); + let result = st + .validate_minimum_shielded_fee(platform_version) + .expect("should not error"); + assert!(!result.is_valid()); + // Must reject with the amount-RANGE error, not the below-min error — locks in the + // "over-max is not below-min" reason accuracy the cap introduced. + assert!( + matches!( + result.errors.first(), + Some(ConsensusError::BasicError( + BasicError::InvalidIdentityCreditWithdrawalTransitionAmountError(_) + )) + ), + "over-max must reject with InvalidIdentityCreditWithdrawalTransitionAmountError; got {:?}", + result.errors + ); + } + + #[test] + fn should_accept_shielded_withdrawal_with_net_at_max_withdrawal_amount() { + let platform_version = PlatformVersion::latest(); + let min_fee = dpp::shielded::compute_minimum_shielded_fee(0, platform_version) + .expect("fee computation should not overflow"); + let max = platform_version.system_limits.max_withdrawal_amount; + // net = max_withdrawal_amount exactly → at the cap, accepted. + let st = shielded_withdrawal_with_amount(min_fee + max); + let result = st + .validate_minimum_shielded_fee(platform_version) + .expect("should not error"); + assert!( + result.is_valid(), + "a withdrawal whose net equals max_withdrawal_amount must be accepted" + ); + } + + #[test] + fn should_reject_amount_exceeding_i64_max_via_guard() { + let platform_version = PlatformVersion::latest(); + // `unshielding_amount > i64::MAX` wraps to a negative i64 in the validator's cast; + // the defensive `fee < 0` guard must reject it (rather than wrapping back to a huge + // u64 and sailing past the min-fee check). + let st = shielded_withdrawal_with_amount((i64::MAX as u64) + 1); + let result = st + .validate_minimum_shielded_fee(platform_version) + .expect("should not error"); + assert!(!result.is_valid()); + assert!( + matches!( + result.errors.first(), + Some(ConsensusError::BasicError( + BasicError::ShieldedInvalidValueBalanceError(_) + )) + ), + "amount > i64::MAX must be rejected by the fee<0 guard; got {:?}", + result.errors + ); + } } mod validate_shielded_proof { diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_withdrawal/structure/v1/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_withdrawal/structure/v1/mod.rs index ba5c693a780..e6131404b11 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_withdrawal/structure/v1/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_withdrawal/structure/v1/mod.rs @@ -9,7 +9,7 @@ use dpp::consensus::ConsensusError; use crate::error::Error; use dpp::state_transition::identity_credit_withdrawal_transition::accessors::IdentityCreditWithdrawalTransitionAccessorsV0; use dpp::state_transition::identity_credit_withdrawal_transition::{ - IdentityCreditWithdrawalTransition, MIN_CORE_FEE_PER_BYTE, MIN_WITHDRAWAL_AMOUNT, + IdentityCreditWithdrawalTransition, MIN_CORE_FEE_PER_BYTE, }; use dpp::util::is_non_zero_fibonacci_number::is_non_zero_fibonacci_number; use dpp::validation::SimpleConsensusValidationResult; @@ -30,18 +30,24 @@ impl IdentityCreditWithdrawalStateTransitionStructureValidationV1 let mut result = SimpleConsensusValidationResult::default(); let amount = self.amount(); - if amount < MIN_WITHDRAWAL_AMOUNT + if amount < platform_version.system_limits.min_withdrawal_amount || amount > platform_version.system_limits.max_withdrawal_amount { result.add_error(ConsensusError::from( InvalidIdentityCreditWithdrawalTransitionAmountError::new( self.amount(), - MIN_WITHDRAWAL_AMOUNT, + platform_version.system_limits.min_withdrawal_amount, platform_version.system_limits.max_withdrawal_amount, ), )); } + // NOTE: the shielded-withdrawal path (v12) re-validates these same three Core-facing + // fields — `pooling`, `core_fee_per_byte`, `output_script` — in + // `dpp .../shielded/shielded_withdrawal_transition/v0/state_transition_validation.rs`, + // reusing the same error types and `MIN_CORE_FEE_PER_BYTE`. Keep the two in sync (or, if + // touching both, factor a shared helper). + // currently we do not support pooling, so we must validate that pooling is `Never` if self.pooling() != Pooling::Never { diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_common/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_common/mod.rs index 3a003d6f308..327c8ffe7b9 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_common/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_common/mod.rs @@ -70,10 +70,13 @@ const _: () = assert!( /// Orchard bundle commitment together with `extra_sighash_data` (transparent fields). /// The same computation must be used when signing the bundle on the client side. /// -/// `extra_sighash_data` binds transparent fields to the Orchard signatures: +/// `extra_sighash_data` binds transparent fields to the Orchard signatures (built by the +/// shared `dpp::shielded::*_extra_sighash_data` helpers so the signer and verifier agree): /// - Shield: empty (no transparent outputs) /// - Shielded transfer: empty (no transparent fields) -/// - Unshield: `output_address.to_bytes() || amount.to_le_bytes()` +/// - Unshield: `output_address || unshielding_amount (u64 LE)` +/// - Shielded withdrawal: `output_script || unshielding_amount (u64 LE) || core_fee_per_byte +/// (u32 LE) || pooling (u8)` — every Core-facing field the withdrawal document commits to. /// /// Returns `Ok(())` if all verification passes, or an `InvalidShieldedProofError` /// if reconstruction or any verification step fails. @@ -745,4 +748,97 @@ mod tests { assert!(FLAGS_OUTPUTS_ONLY != FLAGS_SPENDS_AND_OUTPUTS); } } + + /// Benchmark: how shielded verification scales with the number of actions. + /// + /// Run with: + /// `cargo test -p drive-abci --lib bench_shielded_proof_verification_scaling -- \ + /// --ignored --nocapture` + /// + /// Halo 2 proof verification is one per-bundle check whose cost grows with the action + /// count (one circuit instance per action); RedPallas spend-auth signatures are + /// per-action; the binding signature is per-bundle. So the full consensus verification + /// cost is roughly `proof_verify(n) + n × spend_auth + binding`. This informs whether the + /// flat `shielded_proof_verification_fee` should gain a per-action component. + #[test] + #[ignore = "benchmark; run manually with --ignored --nocapture"] + fn bench_shielded_proof_verification_scaling() { + use grovedb_commitment_tree::{ + Builder, BundleType, FullViewingKey, NoteValue, ProvingKey, Scope, SpendingKey, + }; + use rand::rngs::OsRng; + use std::time::Instant; + + let pk = ProvingKey::build(); + let vk = get_verifying_key(); + + // Build & prove an n-action (outputs-only) bundle. + let build = |n: usize| { + let mut rng = OsRng; + let sk = SpendingKey::from_bytes([7u8; 32]).unwrap(); + let fvk = FullViewingKey::from(&sk); + let recipient = fvk.address_at(0u32, Scope::External); + let anchor: Anchor = Anchor::empty_tree(); + let mut builder = Builder::::new( + BundleType::Transactional { + flags: Flags::SPENDS_DISABLED, + bundle_required: false, + }, + anchor, + ); + for _ in 0..n { + builder + .add_output(None, recipient, NoteValue::from_raw(5000), [0u8; 36]) + .unwrap(); + } + let (unauth, _) = builder.build::(&mut rng).unwrap().unwrap(); + let sighash: [u8; 32] = unauth.commitment().into(); + let proven = unauth.create_proof(&pk, &mut rng).unwrap(); + (proven.apply_signatures(rng, sighash, &[]).unwrap(), sighash) + }; + + let k = 30u32; + eprintln!("\n=== Halo 2 proof verification vs action count ==="); + for n in [1usize, 2, 4, 8, 16] { + let (bundle, _) = build(n); + let instances: Vec<_> = bundle + .actions() + .iter() + .map(|a| a.to_instance(*bundle.flags(), *bundle.anchor())) + .collect(); + let _ = bundle.authorization().proof().verify(vk, &instances); // warm + let start = Instant::now(); + for _ in 0..k { + let _ = bundle.authorization().proof().verify(vk, &instances); + } + eprintln!( + " actions={:2} proof_verify = {:>7} us", + n, + start.elapsed().as_micros() / k as u128 + ); + } + + // Per-action spend-auth sig and per-bundle binding sig (≈ constant each). + let (bundle, sighash) = build(1); + let action = &bundle.actions()[0]; + let (rk, sig) = (action.rk(), action.authorization()); + let start = Instant::now(); + for _ in 0..k { + let _ = rk.verify(&sighash, sig); + } + eprintln!( + " spend_auth_sig (per action) = {} us", + start.elapsed().as_micros() / k as u128 + ); + let bvk = bundle.binding_validating_key(); + let bsig = bundle.authorization().binding_signature(); + let start = Instant::now(); + for _ in 0..k { + let _ = bvk.verify(&sighash, bsig); + } + eprintln!( + " binding_sig (per bundle) = {} us", + start.elapsed().as_micros() / k as u128 + ); + } } diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_transfer/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_transfer/mod.rs index bf0ce41200a..4aa62c80362 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_transfer/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_transfer/mod.rs @@ -3,7 +3,6 @@ mod transform_into_action; #[cfg(test)] mod tests; -use dpp::block::block_info::BlockInfo; use dpp::state_transition::shielded_transfer_transition::ShieldedTransferTransition; use dpp::validation::ConsensusValidationResult; use drive::grovedb::TransactionArg; @@ -11,7 +10,6 @@ use drive::state_transition_action::StateTransitionAction; use crate::error::execution::ExecutionError; use crate::error::Error; -use crate::execution::types::state_transition_execution_context::StateTransitionExecutionContext; use crate::execution::validation::state_transition::shielded_transfer::transform_into_action::v0::ShieldedTransferStateTransitionTransformIntoActionValidationV0; use crate::platform_types::platform::PlatformRef; use crate::rpc::core::CoreRPCLike; @@ -24,8 +22,6 @@ pub trait StateTransitionShieldedTransferTransitionActionTransformer { fn transform_into_action_for_shielded_transfer_transition( &self, platform: &PlatformRef, - block_info: &BlockInfo, - execution_context: &mut StateTransitionExecutionContext, tx: TransactionArg, ) -> Result, Error>; } @@ -34,8 +30,6 @@ impl StateTransitionShieldedTransferTransitionActionTransformer for ShieldedTran fn transform_into_action_for_shielded_transfer_transition( &self, platform: &PlatformRef, - block_info: &BlockInfo, - execution_context: &mut StateTransitionExecutionContext, tx: TransactionArg, ) -> Result, Error> { let platform_version = platform.state.current_platform_version()?; @@ -47,13 +41,7 @@ impl StateTransitionShieldedTransferTransitionActionTransformer for ShieldedTran .shielded_transfer_state_transition .transform_into_action { - 0 => self.transform_into_action_v0( - platform.drive, - tx, - block_info, - execution_context, - platform_version, - ), + 0 => self.transform_into_action_v0(platform.drive, tx, platform_version), version => Err(Error::Execution(ExecutionError::UnknownVersionMismatch { method: "shielded transfer transition: transform_into_action".to_string(), known_versions: vec![0], diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_transfer/tests.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_transfer/tests.rs index fb69c192a37..d20e4db4dae 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_transfer/tests.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_transfer/tests.rs @@ -48,7 +48,7 @@ mod tests { fn create_default_shielded_transfer_transition() -> StateTransition { create_shielded_transfer_transition( vec![create_dummy_serialized_action()], - 111_548_800, // minimum fee for 1 action + 130_548_800, // minimum fee for 1 action [42u8; 32], // non-zero anchor vec![0u8; 100], // dummy proof bytes [0u8; 64], // dummy binding signature @@ -100,7 +100,7 @@ mod tests { let transition = ShieldedTransferTransitionV0 { actions, - value_balance: 111_548_800, + value_balance: 130_548_800, anchor: [42u8; 32], proof: vec![0u8; 100], binding_signature: [0u8; 64], @@ -261,7 +261,8 @@ mod tests { ExtractedNoteCommitment, FullViewingKey, MerklePath, Note, NoteValue, Position, RandomSeed, Retention, Rho, Scope, SpendAuthorizingKey, SpendingKey, }; - use rand::rngs::OsRng; + use rand::rngs::StdRng; + use rand::SeedableRng; #[test] fn test_invalid_proof_returns_shielded_proof_error() { @@ -289,13 +290,13 @@ mod tests { } /// Minimum fee for 2 actions (Orchard builder always produces ≥2). - const MINIMUM_FEE_2_ACTIONS: u64 = 123_097_600; + const MINIMUM_FEE_2_ACTIONS: u64 = 161_097_600; #[test] fn test_valid_shielded_transfer_proof_succeeds() { let platform_version = PlatformVersion::latest(); let platform = setup_platform(); - let mut rng = OsRng; + let mut rng = StdRng::seed_from_u64(0); let pk = get_proving_key(); let spend_amount = 200_000_000u64; @@ -387,7 +388,7 @@ mod tests { let transition = create_shielded_transfer_transition( vec![bad_action], - 111_548_800, // minimum fee for 1 action (fee check runs before proof reconstruction) + 130_548_800, // minimum fee for 1 action (fee check runs before proof reconstruction) anchor, vec![0u8; 100], [0u8; 64], @@ -416,14 +417,14 @@ mod tests { // // With current constants: // proof_verification_fee = 100_000_000 - // per_action_processing_fee = 3_000_000 + // per_action_processing_fee = 22_000_000 // per_action_storage_fee = 312 × (27_000 + 400) = 8_548_800 - // per_action_total = 11_548_800 + // per_action_total = 30_548_800 // // Minimum fees by action count: - // 2 actions: 100_000_000 + 2 × 11_548_800 = 123_097_600 - // 3 actions: 100_000_000 + 3 × 11_548_800 = 134_646_400 - // 4 actions: 100_000_000 + 4 × 11_548_800 = 146_195_200 + // 2 actions: 100_000_000 + 2 × 30_548_800 = 161_097_600 + // 3 actions: 100_000_000 + 3 × 30_548_800 = 191_646_400 + // 4 actions: 100_000_000 + 4 × 30_548_800 = 222_195_200 mod fee_validation { use super::*; @@ -432,11 +433,12 @@ mod tests { ExtractedNoteCommitment, FullViewingKey, MerklePath, Note, NoteValue, Position, RandomSeed, Retention, Rho, Scope, SpendAuthorizingKey, SpendingKey, }; - use rand::rngs::OsRng; + use rand::rngs::StdRng; + use rand::SeedableRng; - const MINIMUM_FEE_2_ACTIONS: u64 = 123_097_600; - const MINIMUM_FEE_3_ACTIONS: u64 = 134_646_400; - const MINIMUM_FEE_4_ACTIONS: u64 = 146_195_200; + const MINIMUM_FEE_2_ACTIONS: u64 = 161_097_600; + const MINIMUM_FEE_3_ACTIONS: u64 = 191_646_400; + const MINIMUM_FEE_4_ACTIONS: u64 = 222_195_200; /// Helper to create a dummy action with a unique seed (avoids duplicate nullifiers). fn create_dummy_action(seed: u8) -> SerializedAction { @@ -485,7 +487,7 @@ mod tests { // 2 actions with fee one credit below minimum let transition = create_shielded_transfer_transition( vec![create_dummy_action(1), create_dummy_action(2)], - MINIMUM_FEE_2_ACTIONS - 1, // 121,343,999 + MINIMUM_FEE_2_ACTIONS - 1, // one credit below the 2-action minimum [42u8; 32], vec![0u8; 100], [0u8; 64], @@ -513,7 +515,7 @@ mod tests { create_dummy_action(2), create_dummy_action(3), ], - MINIMUM_FEE_3_ACTIONS - 1, // 134,646,399 + MINIMUM_FEE_3_ACTIONS - 1, // one credit below the 3-action minimum [42u8; 32], vec![0u8; 100], [0u8; 64], @@ -542,7 +544,7 @@ mod tests { create_dummy_action(3), create_dummy_action(4), ], - MINIMUM_FEE_4_ACTIONS - 1, // 146,195,199 + MINIMUM_FEE_4_ACTIONS - 1, // one credit below the 4-action minimum [42u8; 32], vec![0u8; 100], [0u8; 64], @@ -565,7 +567,7 @@ mod tests { fn build_bundle_with_fee( fee: u64, ) -> (Vec, u64, [u8; 32], Vec, [u8; 64]) { - let mut rng = OsRng; + let mut rng = StdRng::seed_from_u64(0); let pk = get_proving_key(); let sk = SpendingKey::from_bytes([0u8; 32]).unwrap(); @@ -646,7 +648,10 @@ mod tests { } #[test] - fn test_fee_above_minimum_for_2_actions_succeeds() { + fn test_fee_above_minimum_for_2_actions_is_rejected() { + // A shielded transfer's `value_balance` IS the fee (no recipient amount), so it + // must equal the minimum exactly. Paying even 1 credit above the minimum is + // rejected — overpayment buys nothing and would leak a fee fingerprint. let platform_version = PlatformVersion::latest(); let platform = setup_platform(); @@ -672,7 +677,9 @@ mod tests { assert_matches!( processing_result.execution_results().as_slice(), - [StateTransitionExecutionResult::SuccessfulExecution { .. }] + [StateTransitionExecutionResult::UnpaidConsensusError( + ConsensusError::BasicError(BasicError::ShieldedInvalidValueBalanceError(_)) + )] ); } } @@ -693,17 +700,18 @@ mod tests { FullViewingKey, MerklePath, Note, NoteValue, Position, RandomSeed, Retention, Rho, Scope, SpendAuthorizingKey, SpendingKey, }; - use rand::rngs::OsRng; + use rand::rngs::StdRng; + use rand::SeedableRng; /// Minimum fee for 2 actions (Orchard builder always produces ≥2). - const MINIMUM_FEE_2_ACTIONS: u64 = 123_097_600; + const MINIMUM_FEE_2_ACTIONS: u64 = 161_097_600; /// Build a valid Orchard bundle for shielded transfer tests. /// Includes sufficient fee (value_balance = MINIMUM_FEE_2_ACTIONS). /// Returns (actions, value_balance, anchor_bytes, proof_bytes, binding_sig). fn build_valid_shielded_transfer_bundle( ) -> (Vec, u64, [u8; 32], Vec, [u8; 64]) { - let mut rng = OsRng; + let mut rng = StdRng::seed_from_u64(0); let pk = get_proving_key(); let spend_amount = 200_000_000u64; @@ -751,13 +759,14 @@ mod tests { serialize_authorized_bundle_u64(&bundle) } - /// AUDIT REGRESSION: Mutating value_balance is now caught by BatchValidator. + /// AUDIT REGRESSION: Mutating value_balance is rejected. /// - /// Previously, the code only called `bundle.verify_proof(vk)` which did not - /// check the binding signature. Now `BatchValidator` verifies the Halo 2 proof - /// AND the binding signature, which cryptographically binds value_balance to - /// the value commitments (cv_net). Mutating value_balance changes the bundle - /// commitment (sighash), causing signature verification to fail. + /// The binding signature cryptographically binds value_balance to the value + /// commitments (cv_net), so `BatchValidator` rejects any mutation. With the + /// exact-fee rule for shielded transfers (`value_balance == min_fee`), a mutation + /// that bumps value_balance off the minimum *also* fails the fee check — which runs + /// before proof verification — so the mutation is now caught there first. Either way + /// the attack is rejected; this asserts the earlier (fee-check) rejection. /// /// Original severity: CRITICAL — now FIXED. #[test] @@ -784,12 +793,12 @@ mod tests { let processing_result = process_transition(&platform, transition, platform_version); - // FIXED: BatchValidator detects the binding signature mismatch - // because mutating value_balance changes the bundle commitment (sighash). + // The exact-fee rule rejects this before proof verification: the mutation bumps + // value_balance above the minimum, and a transfer must pay exactly the minimum. assert_matches!( processing_result.execution_results().as_slice(), [StateTransitionExecutionResult::UnpaidConsensusError( - ConsensusError::StateError(StateError::InvalidShieldedProofError(_)) + ConsensusError::BasicError(BasicError::ShieldedInvalidValueBalanceError(_)) )] ); } @@ -923,15 +932,16 @@ mod tests { FullViewingKey, Note, NoteValue, Position, RandomSeed, Retention, Rho, Scope, SpendAuthorizingKey, SpendingKey, }; - use rand::rngs::OsRng; + use rand::rngs::StdRng; + use rand::SeedableRng; - const MINIMUM_FEE_2_ACTIONS: u64 = 123_097_600; + const MINIMUM_FEE_2_ACTIONS: u64 = 161_097_600; #[test] fn test_shielded_transfer_prove_and_verify_nullifiers() { let platform_version = PlatformVersion::latest(); let platform = setup_platform(); - let mut rng = OsRng; + let mut rng = StdRng::seed_from_u64(0); let pk = get_proving_key(); let spend_amount = 200_000_000u64; diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_transfer/transform_into_action/v0/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_transfer/transform_into_action/v0/mod.rs index a8ee66d67b5..7f05206998f 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_transfer/transform_into_action/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_transfer/transform_into_action/v0/mod.rs @@ -1,12 +1,7 @@ use crate::error::Error; -use crate::execution::types::execution_operation::ValidationOperation; -use crate::execution::types::state_transition_execution_context::{ - StateTransitionExecutionContext, StateTransitionExecutionContextMethodsV0, -}; use crate::execution::validation::state_transition::state_transitions::shielded_common::{ read_pool_total_balance, validate_anchor_exists, validate_nullifiers, }; -use dpp::block::block_info::BlockInfo; use dpp::consensus::state::state_error::StateError; use dpp::fee::Credits; use dpp::prelude::ConsensusValidationResult; @@ -23,8 +18,6 @@ pub(in crate::execution::validation::state_transition::state_transitions::shield &self, drive: &Drive, transaction: TransactionArg, - block_info: &BlockInfo, - execution_context: &mut StateTransitionExecutionContext, platform_version: &PlatformVersion, ) -> Result, Error>; } @@ -34,8 +27,6 @@ impl ShieldedTransferStateTransitionTransformIntoActionValidationV0 for Shielded &self, drive: &Drive, transaction: TransactionArg, - block_info: &BlockInfo, - execution_context: &mut StateTransitionExecutionContext, platform_version: &PlatformVersion, ) -> Result, Error> { // The value_balance is the fee amount extracted from the shielded pool @@ -109,17 +100,13 @@ impl ShieldedTransferStateTransitionTransformIntoActionValidationV0 for Shielded return Ok(consensus_error); } - // Calculate fees from the GroveDB operations - let fee = Drive::calculate_fee( - None, - Some(drive_operations), - &block_info.epoch, - drive.config.epochs_per_era, - platform_version, - None, - )?; - execution_context.add_operation(ValidationOperation::PrecalculatedOperation(fee)); - + // Shielded transitions do NOT meter the GroveDB operation cost as a fee. They + // pay a flat, client-predictable fee (`compute_minimum_shielded_fee`) baked into + // the ZK-proven `value_balance`: the client must know the exact fee offline to + // build its proof and cannot run `Drive::calculate_fee` (which needs server-side + // state). The flat fee subsumes these validation reads, so the cost accumulated + // in `drive_operations` is intentionally not charged — `PaidFromShieldedPool` + // carves the fee straight from the pool and never consumes the execution context. let result = ShieldedTransferTransitionAction::try_from_transition( self, fee_amount, diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_withdrawal/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_withdrawal/mod.rs index 494bb288179..eed3592d3b8 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_withdrawal/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_withdrawal/mod.rs @@ -10,7 +10,6 @@ use drive::state_transition_action::StateTransitionAction; use crate::error::execution::ExecutionError; use crate::error::Error; -use crate::execution::types::state_transition_execution_context::StateTransitionExecutionContext; use crate::execution::validation::state_transition::shielded_withdrawal::transform_into_action::v0::ShieldedWithdrawalStateTransitionTransformIntoActionValidationV0; use crate::platform_types::platform::PlatformRef; use crate::platform_types::platform_state::PlatformStateV0Methods; @@ -23,7 +22,6 @@ pub trait StateTransitionShieldedWithdrawalTransitionActionTransformer { &self, platform: &PlatformRef, block_info: &BlockInfo, - execution_context: &mut StateTransitionExecutionContext, tx: TransactionArg, ) -> Result, Error>; } @@ -33,7 +31,6 @@ impl StateTransitionShieldedWithdrawalTransitionActionTransformer for ShieldedWi &self, platform: &PlatformRef, block_info: &BlockInfo, - execution_context: &mut StateTransitionExecutionContext, tx: TransactionArg, ) -> Result, Error> { let platform_version = platform.state.current_platform_version()?; @@ -45,13 +42,7 @@ impl StateTransitionShieldedWithdrawalTransitionActionTransformer for ShieldedWi .shielded_withdrawal_state_transition .transform_into_action { - 0 => self.transform_into_action_v0( - platform.drive, - block_info, - execution_context, - tx, - platform_version, - ), + 0 => self.transform_into_action_v0(platform.drive, block_info, tx, platform_version), version => Err(Error::Execution(ExecutionError::UnknownVersionMismatch { method: "shielded withdrawal transition: transform_into_action".to_string(), known_versions: vec![0], diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_withdrawal/tests.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_withdrawal/tests.rs index 0c2f8d4701e..1a4c345eff7 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_withdrawal/tests.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_withdrawal/tests.rs @@ -61,7 +61,7 @@ mod tests { fn create_default_shielded_withdrawal_transition() -> StateTransition { create_shielded_withdrawal_transition( vec![create_dummy_serialized_action()], - 111_549_800, // unshielding_amount: recipient amount + minimum fee for 1 action + 131_548_800, // unshielding_amount: recipient (1_000_000, above MIN_WITHDRAWAL_AMOUNT) + min fee for 1 action [42u8; 32], // non-zero anchor vec![0u8; 100], // dummy proof bytes [0u8; 64], // dummy binding signature @@ -119,7 +119,7 @@ mod tests { let transition = ShieldedWithdrawalTransitionV0 { actions, - unshielding_amount: 111_549_800, + unshielding_amount: 130_549_800, anchor: [42u8; 32], proof: vec![0u8; 100], binding_signature: [0u8; 64], @@ -352,7 +352,8 @@ mod tests { FullViewingKey, Note, NoteValue, Position, RandomSeed, Retention, Rho, Scope, SpendAuthorizingKey, SpendingKey, }; - use rand::rngs::OsRng; + use rand::rngs::StdRng; + use rand::SeedableRng; #[test] fn test_invalid_proof_returns_shielded_proof_error() { @@ -388,7 +389,7 @@ mod tests { let platform_version = PlatformVersion::latest(); let platform = setup_platform(); insert_dummy_encrypted_notes(&platform, 250); - let mut rng = OsRng; + let mut rng = StdRng::seed_from_u64(0); let pk = get_proving_key(); // --- Create keys --- @@ -428,8 +429,12 @@ mod tests { // Compute platform sighash binding transparent fields (output_script, unshielding_amount) let output_script = create_output_script(); let unshielding_amount = 499_995_000u64; // value_balance as u64 - let mut extra_sighash_data = output_script.as_bytes().to_vec(); - extra_sighash_data.extend_from_slice(&unshielding_amount.to_le_bytes()); + let extra_sighash_data = dpp::shielded::shielded_withdrawal_extra_sighash_data( + output_script.as_bytes(), + unshielding_amount, + 1, + Pooling::Never, + ); let bundle_commitment: [u8; 32] = unauthorized.commitment().into(); let sighash = compute_platform_sighash(&bundle_commitment, &extra_sighash_data); @@ -490,7 +495,7 @@ mod tests { let transition = create_shielded_withdrawal_transition( vec![bad_action], - 111_549_800, // unshielding_amount: recipient amount + minimum fee for 1 action + 130_549_800, // unshielding_amount: recipient amount + minimum fee for 1 action anchor, vec![0u8; 100], [0u8; 64], @@ -529,7 +534,8 @@ mod tests { FullViewingKey, Note, NoteValue, Position, RandomSeed, Retention, Rho, Scope, SpendAuthorizingKey, SpendingKey, }; - use rand::rngs::OsRng; + use rand::rngs::StdRng; + use rand::SeedableRng; /// Build a valid Orchard bundle for shielded withdrawal tests (spend > output). /// The `output_script` and `unshielding_amount` are bound to the sighash so that @@ -539,7 +545,7 @@ mod tests { output_script: &CoreScript, unshielding_amount: u64, ) -> (Vec, i64, [u8; 32], Vec, [u8; 64]) { - let mut rng = OsRng; + let mut rng = StdRng::seed_from_u64(0); let pk = get_proving_key(); let sk = SpendingKey::from_bytes([0u8; 32]).unwrap(); @@ -574,8 +580,12 @@ mod tests { let (unauthorized, _) = builder.build::(&mut rng).unwrap().unwrap(); // Bind transparent fields (output_script, unshielding_amount) to the sighash - let mut extra_sighash_data = output_script.as_bytes().to_vec(); - extra_sighash_data.extend_from_slice(&unshielding_amount.to_le_bytes()); + let extra_sighash_data = dpp::shielded::shielded_withdrawal_extra_sighash_data( + output_script.as_bytes(), + unshielding_amount, + 1, + Pooling::Never, + ); let bundle_commitment: [u8; 32] = unauthorized.commitment().into(); let sighash = compute_platform_sighash(&bundle_commitment, &extra_sighash_data); @@ -730,6 +740,57 @@ mod tests { ); } + /// AUDIT REGRESSION: core_fee_per_byte is bound to the platform sighash. + /// + /// `core_fee_per_byte` is written verbatim by the transformer into the queued + /// Core withdrawal document (the asset-unlock TxOut fee rate). Structure + /// validation constrains it to the non-zero Fibonacci set, but that set spans + /// 1..=2,971,215,073 — so without sighash binding a relay or block proposer could + /// flip a user's `core_fee_per_byte = 1` to a much larger Fibonacci value, + /// redirecting the withdrawn amount into L1 miner fees while keeping the Orchard + /// proof valid (ShieldedWithdrawal has no identity-key signature). Binding it into + /// the sighash makes the binding signature authorize it, so any change is rejected. + #[test] + fn test_different_core_fee_per_byte_with_same_valid_bundle_is_rejected() { + let platform_version = PlatformVersion::latest(); + let platform = setup_platform(); + insert_dummy_encrypted_notes(&platform, 250); + + // Bundle is signed for core_fee_per_byte = 1 (build_valid_shielded_withdrawal_bundle). + let output_script = create_output_script(); + let unshielding_amount = 499_995_000u64; + let (actions, value_balance, anchor_bytes, proof_bytes, binding_sig) = + build_valid_shielded_withdrawal_bundle(&output_script, unshielding_amount); + assert_eq!(value_balance, 499_995_000); + + set_pool_total_balance(&platform, 500_000_000); + insert_anchor_into_state(&platform, &anchor_bytes); + + // ATTACK: bump core_fee_per_byte to a *different* Fibonacci value (2), so it still + // passes structure validation but no longer matches the signed sighash. + let transition = create_shielded_withdrawal_transition( + actions, + unshielding_amount, + anchor_bytes, + proof_bytes, + binding_sig, + 2, // mutated core_fee_per_byte (bundle was signed with 1) + Pooling::Never, + output_script, + ); + + let processing_result = process_transition(&platform, transition, platform_version); + + // FIXED: core_fee_per_byte is in the platform sighash, so the mutated value + // yields a different sighash than was signed and signature verification fails. + assert_matches!( + processing_result.execution_results().as_slice(), + [StateTransitionExecutionResult::UnpaidConsensusError( + ConsensusError::StateError(StateError::InvalidShieldedProofError(_)) + )] + ); + } + /// AUDIT REGRESSION: Different unshielding_amount is caught by platform sighash. /// /// The unshielding_amount is bound to the Orchard bundle via sighash. Changing @@ -796,7 +857,7 @@ mod tests { let transition = create_shielded_withdrawal_transition( vec![action1, action2], // Both have nullifier [1u8; 32] - 123_098_600, // unshielding_amount: recipient amount + minimum fee for 2 actions + 162_097_600, // unshielding_amount: recipient (1_000_000, above MIN_WITHDRAWAL_AMOUNT) + min fee for 2 actions anchor, vec![0u8; 100], [0u8; 64], @@ -838,7 +899,8 @@ mod tests { FullViewingKey, Note, NoteValue, Position, RandomSeed, Retention, Rho, Scope, SpendAuthorizingKey, SpendingKey, }; - use rand::rngs::OsRng; + use rand::rngs::StdRng; + use rand::SeedableRng; use std::collections::BTreeMap; use std::sync::Arc; @@ -847,7 +909,7 @@ mod tests { let platform_version = PlatformVersion::latest(); let platform = setup_platform(); insert_dummy_encrypted_notes(&platform, 250); - let mut rng = OsRng; + let mut rng = StdRng::seed_from_u64(0); let pk = get_proving_key(); // --- Create keys --- @@ -887,8 +949,12 @@ mod tests { // Compute platform sighash binding transparent fields (output_script, unshielding_amount) let output_script = create_output_script(); let unshielding_amount = 499_995_000u64; // value_balance as u64 - let mut extra_sighash_data = output_script.as_bytes().to_vec(); - extra_sighash_data.extend_from_slice(&unshielding_amount.to_le_bytes()); + let extra_sighash_data = dpp::shielded::shielded_withdrawal_extra_sighash_data( + output_script.as_bytes(), + unshielding_amount, + 1, + Pooling::Never, + ); let bundle_commitment: [u8; 32] = unauthorized.commitment().into(); let sighash = compute_platform_sighash(&bundle_commitment, &extra_sighash_data); @@ -1056,4 +1122,175 @@ mod tests { ); } } + + mod credit_conservation { + use super::*; + use crate::execution::validation::state_transition::tests::process_state_transitions; + use dpp::block::block_info::BlockInfo; + use grovedb_commitment_tree::{ + Builder, BundleType, ClientMemoryCommitmentTree, DashMemo, ExtractedNoteCommitment, + FullViewingKey, Note, NoteValue, Position, RandomSeed, Retention, Rho, Scope, + SpendAuthorizingKey, SpendingKey, + }; + use rand::rngs::StdRng; + use rand::SeedableRng; + + /// Block-level conservation test for shielded withdrawal. Spends a 500M note + /// and withdraws `value_balance` worth from the pool. Runs the FULL block + /// pipeline (execute + fee distribution + sum-tree validation) and asserts: + /// - total platform credits stay conserved (the invariant whose failure + /// halts the chain), + /// - the shielded pool drops by exactly `unshielding_amount`, and + /// - the system-credit counter drops by the NET (`unshielding_amount - fee`) + /// — only the net leaves the platform to Core; the fee stays in the fee + /// pools. This exercises the `RemoveFromSystemCredits(net)` accounting the + /// conversion-level tests cannot fully validate end-to-end. + #[test] + fn test_shielded_withdrawal_conserves_credits() { + let platform_version = PlatformVersion::latest(); + let platform = setup_platform(); + insert_dummy_encrypted_notes(&platform, 250); + // Seeded RNG for deterministic, bisectable test randomness (repo convention). + let mut rng = StdRng::seed_from_u64(0); + let pk = get_proving_key(); + + let sk = SpendingKey::from_bytes([0u8; 32]).unwrap(); + let fvk = FullViewingKey::from(&sk); + let recipient = fvk.address_at(0u32, Scope::External); + let ask = SpendAuthorizingKey::from(&sk); + + let rho_bytes: [u8; 32] = { + let mut b = [0u8; 32]; + b[0] = 1; + b + }; + let rho = Rho::from_bytes(&rho_bytes).unwrap(); + let rseed = RandomSeed::from_bytes([42u8; 32], &rho).unwrap(); + let note = + Note::from_parts(recipient, NoteValue::from_raw(500_000_000), rho, rseed).unwrap(); + + let cmx = ExtractedNoteCommitment::from(note.commitment()); + let mut tree = ClientMemoryCommitmentTree::new(100); + tree.append(cmx.to_bytes(), Retention::Marked).unwrap(); + tree.checkpoint(0u32).unwrap(); + let anchor = tree.anchor().unwrap(); + let merkle_path = tree.witness(Position::from(0u64), 0).unwrap().unwrap(); + + let mut builder = Builder::::new(BundleType::DEFAULT, anchor); + builder.add_spend(fvk.clone(), note, merkle_path).unwrap(); + builder + .add_output(None, recipient, NoteValue::from_raw(5_000), [0u8; 36]) + .unwrap(); + let (unauthorized, _) = builder.build::(&mut rng).unwrap().unwrap(); + + let output_script = create_output_script(); + let unshielding_amount = 499_995_000u64; + let extra_sighash_data = dpp::shielded::shielded_withdrawal_extra_sighash_data( + output_script.as_bytes(), + unshielding_amount, + 1, + Pooling::Never, + ); + let bundle_commitment: [u8; 32] = unauthorized.commitment().into(); + let sighash = compute_platform_sighash(&bundle_commitment, &extra_sighash_data); + let proven = unauthorized.create_proof(pk, &mut rng).unwrap(); + let bundle = proven.apply_signatures(rng, sighash, &[ask]).unwrap(); + + let (actions, value_balance, anchor_bytes, proof_bytes, binding_sig) = + serialize_authorized_bundle_i64(&bundle); + assert_eq!(value_balance, 499_995_000); + let num_actions = actions.len(); + + insert_anchor_into_state(&platform, &anchor_bytes); + set_pool_total_balance(&platform, 500_000_000); + + // Capture a balanced starting state (set_pool_total_balance adds matching + // system credits, so the counter and the sum trees agree). + let credits_before = platform + .drive + .calculate_total_credits_balance(None, &platform_version.drive) + .expect("should calculate total credits before withdrawal"); + assert!( + credits_before + .ok() + .expect("credit balance check should not overflow"), + "credits must be balanced before the withdrawal: {}", + credits_before + ); + + let transition = create_shielded_withdrawal_transition( + actions, + unshielding_amount, + anchor_bytes, + proof_bytes, + binding_sig, + 1, + Pooling::Never, + output_script, + ); + + let platform_state = platform.state.load(); + let (fee_results, _processed_block_fees) = process_state_transitions( + &platform, + &[transition], + BlockInfo::default(), + &platform_state, + ); + + // The shielded fee is split like every other transition: the (permanent) storage + // cost is routed to `storage_fee` (storage pool, epoch fee multiplier applied at + // payout), not booked entirely as processing. So a successful shielded withdrawal + // reports a non-zero storage_fee and a non-zero processing fee that together equal + // the carved minimum shielded fee. + let fee = &fee_results[0]; + let expected_total = + dpp::shielded::compute_minimum_shielded_fee(num_actions, platform_version) + .expect("fee computation should not overflow"); + assert!( + fee.storage_fee > 0, + "shielded storage must be charged as storage_fee, got {}", + fee.storage_fee + ); + assert!( + fee.processing_fee > 0, + "proof + processing must be charged as processing_fee, got {}", + fee.processing_fee + ); + assert_eq!( + fee.storage_fee + fee.processing_fee, + expected_total, + "storage + processing must equal the carved minimum shielded fee" + ); + + let credits_after = platform + .drive + .calculate_total_credits_balance(None, &platform_version.drive) + .expect("should calculate total credits after withdrawal"); + assert!( + credits_after + .ok() + .expect("credit balance check should not overflow"), + "credits must remain balanced after a shielded withdrawal: {}", + credits_after + ); + + // The shielded pool dropped by exactly the full unshielding_amount. + assert_eq!( + credits_before.total_in_shielded_balances + - credits_after.total_in_shielded_balances, + unshielding_amount as i64, + "shielded pool must drop by the full unshielding amount" + ); + + // The system-credit counter dropped by the NET (unshielding_amount - fee): + // only the net leaves the platform to Core; the fee stays in the fee pools. + let fee = dpp::shielded::compute_minimum_shielded_fee(num_actions, platform_version) + .expect("fee computation should not overflow"); + assert_eq!( + credits_before.total_credits_in_platform - credits_after.total_credits_in_platform, + unshielding_amount - fee, + "system credits must drop by exactly the net withdrawn to Core (amount - fee)" + ); + } + } } diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_withdrawal/transform_into_action/v0/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_withdrawal/transform_into_action/v0/mod.rs index 46145443fec..31f4860ed0c 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_withdrawal/transform_into_action/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_withdrawal/transform_into_action/v0/mod.rs @@ -1,8 +1,4 @@ use crate::error::Error; -use crate::execution::types::execution_operation::ValidationOperation; -use crate::execution::types::state_transition_execution_context::{ - StateTransitionExecutionContext, StateTransitionExecutionContextMethodsV0, -}; use crate::execution::validation::state_transition::state_transitions::shielded_common::{ read_pool_total_balance, validate_anchor_exists, validate_minimum_pool_notes, validate_nullifiers, @@ -23,7 +19,6 @@ pub(in crate::execution::validation::state_transition::state_transitions::shield &self, drive: &Drive, block_info: &BlockInfo, - execution_context: &mut StateTransitionExecutionContext, transaction: TransactionArg, platform_version: &PlatformVersion, ) -> Result, Error>; @@ -36,7 +31,6 @@ impl ShieldedWithdrawalStateTransitionTransformIntoActionValidationV0 &self, drive: &Drive, block_info: &BlockInfo, - execution_context: &mut StateTransitionExecutionContext, transaction: TransactionArg, platform_version: &PlatformVersion, ) -> Result, Error> { @@ -75,8 +69,8 @@ impl ShieldedWithdrawalStateTransitionTransformIntoActionValidationV0 } // Verify the pool has sufficient balance for the withdrawal. - let unshielding_amount = match self { - ShieldedWithdrawalTransition::V0(v0) => v0.unshielding_amount, + let (unshielding_amount, num_actions) = match self { + ShieldedWithdrawalTransition::V0(v0) => (v0.unshielding_amount, v0.actions.len()), }; if current_total_balance < unshielding_amount { @@ -115,16 +109,23 @@ impl ShieldedWithdrawalStateTransitionTransformIntoActionValidationV0 return Ok(consensus_error); } - // Calculate fees from the GroveDB operations - let fee = Drive::calculate_fee( - None, - Some(drive_operations), - &block_info.epoch, - drive.config.epochs_per_era, - platform_version, - None, - )?; - execution_context.add_operation(ValidationOperation::PrecalculatedOperation(fee)); + // Shielded transitions do NOT meter the GroveDB operation cost as a fee. They + // pay a flat, client-predictable fee (`compute_minimum_shielded_fee`, computed + // below): the client must know the exact fee offline to build its proof and + // cannot run `Drive::calculate_fee` (which needs server-side state). The flat fee + // subsumes these validation reads, so the cost accumulated in `drive_operations` + // is intentionally not charged — `PaidFromShieldedPool` carves the fee straight + // from the pool and never consumes the execution context. + + // The fee charged to the shielded pool is the minimum shielded fee computed from the + // same `num_actions` that `validate_minimum_shielded_fee` enforced the net range + // against. Because that check passed, the net amount withdrawn to Core + // (`unshielding_amount - fee_amount`) is guaranteed to fall within + // `[MIN_WITHDRAWAL_AMOUNT, max_withdrawal_amount]` for ShieldedWithdrawal; the action + // transformer re-checks that same range with `checked_sub` as defense in depth, so + // those rejection paths are unreachable for validated input. + let fee_amount = + dpp::shielded::compute_minimum_shielded_fee(num_actions, platform_version)?; // Build the action, which includes creating the withdrawal document let creation_time_ms = block_info.time_ms; @@ -133,6 +134,8 @@ impl ShieldedWithdrawalStateTransitionTransformIntoActionValidationV0 self, current_total_balance, creation_time_ms, + fee_amount, + platform_version, ); Ok(result.map(|action| action.into())) diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/unshield/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/unshield/mod.rs index 3d12c471f22..e798a9037c9 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/unshield/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/unshield/mod.rs @@ -3,7 +3,6 @@ mod transform_into_action; #[cfg(test)] mod tests; -use dpp::block::block_info::BlockInfo; use dpp::state_transition::unshield_transition::UnshieldTransition; use dpp::validation::ConsensusValidationResult; use drive::grovedb::TransactionArg; @@ -11,7 +10,6 @@ use drive::state_transition_action::StateTransitionAction; use crate::error::execution::ExecutionError; use crate::error::Error; -use crate::execution::types::state_transition_execution_context::StateTransitionExecutionContext; use crate::execution::validation::state_transition::unshield::transform_into_action::v0::UnshieldStateTransitionTransformIntoActionValidationV0; use crate::platform_types::platform::PlatformRef; use crate::rpc::core::CoreRPCLike; @@ -24,8 +22,6 @@ pub trait StateTransitionUnshieldTransitionActionTransformer { fn transform_into_action_for_unshield_transition( &self, platform: &PlatformRef, - block_info: &BlockInfo, - execution_context: &mut StateTransitionExecutionContext, tx: TransactionArg, ) -> Result, Error>; } @@ -34,8 +30,6 @@ impl StateTransitionUnshieldTransitionActionTransformer for UnshieldTransition { fn transform_into_action_for_unshield_transition( &self, platform: &PlatformRef, - block_info: &BlockInfo, - execution_context: &mut StateTransitionExecutionContext, tx: TransactionArg, ) -> Result, Error> { let platform_version = platform.state.current_platform_version()?; @@ -47,13 +41,7 @@ impl StateTransitionUnshieldTransitionActionTransformer for UnshieldTransition { .unshield_state_transition .transform_into_action { - 0 => self.transform_into_action_v0( - platform.drive, - tx, - block_info, - execution_context, - platform_version, - ), + 0 => self.transform_into_action_v0(platform.drive, tx, platform_version), version => Err(Error::Execution(ExecutionError::UnknownVersionMismatch { method: "unshield transition: transform_into_action".to_string(), known_versions: vec![0], diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/unshield/tests.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/unshield/tests.rs index a1b982b8670..018df60a5cb 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/unshield/tests.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/unshield/tests.rs @@ -57,7 +57,7 @@ mod tests { create_unshield_transition( create_output_address(), vec![create_dummy_serialized_action()], - 111_549_800, // unshielding_amount: recipient amount + minimum fee for 1 action + 130_549_800, // unshielding_amount: recipient amount + minimum fee for 1 action [42u8; 32], // non-zero anchor vec![0u8; 100], // dummy proof bytes [0u8; 64], // dummy binding signature @@ -111,7 +111,7 @@ mod tests { let transition = UnshieldTransitionV0 { output_address: create_output_address(), actions, - unshielding_amount: 111_549_800, + unshielding_amount: 130_549_800, anchor: [42u8; 32], proof: vec![0u8; 100], binding_signature: [0u8; 64], @@ -324,7 +324,8 @@ mod tests { ExtractedNoteCommitment, FullViewingKey, MerklePath, Note, NoteValue, Position, RandomSeed, Retention, Rho, Scope, SpendAuthorizingKey, SpendingKey, }; - use rand::rngs::OsRng; + use rand::rngs::StdRng; + use rand::SeedableRng; #[test] fn test_invalid_proof_returns_shielded_proof_error() { @@ -357,7 +358,7 @@ mod tests { let platform_version = PlatformVersion::latest(); let platform = setup_platform(); insert_dummy_encrypted_notes(&platform, 250); - let mut rng = OsRng; + let mut rng = StdRng::seed_from_u64(0); let pk = get_proving_key(); // --- Create keys --- @@ -397,8 +398,10 @@ mod tests { // Compute platform sighash binding transparent fields (output_address, unshielding_amount) let output_address = create_output_address(); let unshielding_amount = 499_995_000u64; // value_balance as u64 - let mut extra_sighash_data = output_address.to_bytes(); - extra_sighash_data.extend_from_slice(&unshielding_amount.to_le_bytes()); + let extra_sighash_data = dpp::shielded::unshield_extra_sighash_data( + &output_address.to_bytes(), + unshielding_amount, + ); let bundle_commitment: [u8; 32] = unauthorized.commitment().into(); let sighash = compute_platform_sighash(&bundle_commitment, &extra_sighash_data); @@ -455,7 +458,7 @@ mod tests { let transition = create_unshield_transition( create_output_address(), vec![bad_action], - 111_549_800, // unshielding_amount: recipient amount + minimum fee for 1 action + 130_549_800, // unshielding_amount: recipient amount + minimum fee for 1 action anchor, vec![0u8; 100], [0u8; 64], @@ -486,7 +489,8 @@ mod tests { ExtractedNoteCommitment, FullViewingKey, MerklePath, Note, NoteValue, Position, RandomSeed, Retention, Rho, Scope, SpendAuthorizingKey, SpendingKey, }; - use rand::rngs::OsRng; + use rand::rngs::StdRng; + use rand::SeedableRng; /// Build a valid Orchard bundle for unshield tests (spend > output). /// The `output_address` and `unshielding_amount` are bound to the sighash so that @@ -496,7 +500,7 @@ mod tests { output_address: &PlatformAddress, unshielding_amount: u64, ) -> (Vec, i64, [u8; 32], Vec, [u8; 64]) { - let mut rng = OsRng; + let mut rng = StdRng::seed_from_u64(0); let pk = get_proving_key(); let sk = SpendingKey::from_bytes([0u8; 32]).unwrap(); @@ -531,8 +535,10 @@ mod tests { let (unauthorized, _) = builder.build::(&mut rng).unwrap().unwrap(); // Bind transparent fields (output_address, unshielding_amount) to the sighash - let mut extra_sighash_data = output_address.to_bytes(); - extra_sighash_data.extend_from_slice(&unshielding_amount.to_le_bytes()); + let extra_sighash_data = dpp::shielded::unshield_extra_sighash_data( + &output_address.to_bytes(), + unshielding_amount, + ); let bundle_commitment: [u8; 32] = unauthorized.commitment().into(); let sighash = compute_platform_sighash(&bundle_commitment, &extra_sighash_data); @@ -661,7 +667,7 @@ mod tests { let transition = create_unshield_transition( create_output_address(), vec![action1, action2], // Both have nullifier [1u8; 32] - 123_098_600, // unshielding_amount: recipient amount + minimum fee for 2 actions + 161_098_600, // unshielding_amount: recipient amount + minimum fee for 2 actions anchor, vec![0u8; 100], [0u8; 64], @@ -696,14 +702,15 @@ mod tests { FullViewingKey, Note, NoteValue, Position, RandomSeed, Retention, Rho, Scope, SpendAuthorizingKey, SpendingKey, }; - use rand::rngs::OsRng; + use rand::rngs::StdRng; + use rand::SeedableRng; #[test] fn test_unshield_prove_and_verify_nullifiers_and_address() { let platform_version = PlatformVersion::latest(); let platform = setup_platform(); insert_dummy_encrypted_notes(&platform, 250); - let mut rng = OsRng; + let mut rng = StdRng::seed_from_u64(0); let pk = get_proving_key(); let spend_amount = 500_000_000u64; @@ -751,8 +758,10 @@ mod tests { // Compute platform sighash binding transparent fields (output_address, unshielding_amount) let output_address = create_output_address(); let unshielding_amount = 499_995_000u64; // value_balance as u64 - let mut extra_sighash_data = output_address.to_bytes(); - extra_sighash_data.extend_from_slice(&unshielding_amount.to_le_bytes()); + let extra_sighash_data = dpp::shielded::unshield_extra_sighash_data( + &output_address.to_bytes(), + unshielding_amount, + ); let bundle_commitment: [u8; 32] = unauthorized.commitment().into(); let sighash = compute_platform_sighash(&bundle_commitment, &extra_sighash_data); diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/unshield/transform_into_action/v0/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/unshield/transform_into_action/v0/mod.rs index c0414ad482b..9b5fd326360 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/unshield/transform_into_action/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/unshield/transform_into_action/v0/mod.rs @@ -1,13 +1,8 @@ use crate::error::Error; -use crate::execution::types::execution_operation::ValidationOperation; -use crate::execution::types::state_transition_execution_context::{ - StateTransitionExecutionContext, StateTransitionExecutionContextMethodsV0, -}; use crate::execution::validation::state_transition::state_transitions::shielded_common::{ read_pool_total_balance, validate_anchor_exists, validate_minimum_pool_notes, validate_nullifiers, }; -use dpp::block::block_info::BlockInfo; use dpp::prelude::ConsensusValidationResult; use dpp::state_transition::unshield_transition::UnshieldTransition; use dpp::version::PlatformVersion; @@ -22,8 +17,6 @@ pub(in crate::execution::validation::state_transition::state_transitions::unshie &self, drive: &Drive, transaction: TransactionArg, - block_info: &BlockInfo, - execution_context: &mut StateTransitionExecutionContext, platform_version: &PlatformVersion, ) -> Result, Error>; } @@ -33,8 +26,6 @@ impl UnshieldStateTransitionTransformIntoActionValidationV0 for UnshieldTransiti &self, drive: &Drive, transaction: TransactionArg, - block_info: &BlockInfo, - execution_context: &mut StateTransitionExecutionContext, platform_version: &PlatformVersion, ) -> Result, Error> { // The anchor from the transition (Merkle root of commitment tree) @@ -91,20 +82,17 @@ impl UnshieldStateTransitionTransformIntoActionValidationV0 for UnshieldTransiti return Ok(consensus_error); } - // Calculate fees from the GroveDB operations - let fee = Drive::calculate_fee( - None, - Some(drive_operations), - &block_info.epoch, - drive.config.epochs_per_era, - platform_version, - None, - )?; - execution_context.add_operation(ValidationOperation::PrecalculatedOperation(fee)); + // Shielded transitions do NOT meter the GroveDB operation cost as a fee. They + // pay a flat, client-predictable fee (`compute_minimum_shielded_fee`, computed + // below): the client must know the exact fee offline to build its proof and + // cannot run `Drive::calculate_fee` (which needs server-side state). The flat fee + // subsumes these validation reads, so the cost accumulated in `drive_operations` + // is intentionally not charged — `PaidFromShieldedPool` carves the fee straight + // from the pool and never consumes the execution context. // Verify the pool has sufficient balance for the unshield amount - let amount = match self { - UnshieldTransition::V0(v0) => v0.unshielding_amount, + let (amount, num_actions) = match self { + UnshieldTransition::V0(v0) => (v0.unshielding_amount, v0.actions.len()), }; if current_total_balance < amount { @@ -121,7 +109,15 @@ impl UnshieldStateTransitionTransformIntoActionValidationV0 for UnshieldTransiti )); } - let result = UnshieldTransitionAction::try_from_transition(self, current_total_balance); + // The fee charged to the shielded pool is the minimum shielded fee computed + // from the same `num_actions` that `validate_minimum_shielded_fee` enforced + // `unshielding_amount >=` against. Because that check passed, the net recipient + // amount (`unshielding_amount - fee_amount`) is guaranteed to be non-negative. + let fee_amount = + dpp::shielded::compute_minimum_shielded_fee(num_actions, platform_version)?; + + let result = + UnshieldTransitionAction::try_from_transition(self, current_total_balance, fee_amount); Ok(result.map(|action| action.into())) } diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/transformer/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/transformer/mod.rs index 9a9de3e81d6..f2c02594f1d 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/transformer/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/transformer/mod.rs @@ -249,19 +249,12 @@ impl StateTransitionActionTransformer for StateTransition { tx, ) } - StateTransition::ShieldedTransfer(st) => st - .transform_into_action_for_shielded_transfer_transition( - platform, - block_info, - execution_context, - tx, - ), - StateTransition::Unshield(st) => st.transform_into_action_for_unshield_transition( - platform, - block_info, - execution_context, - tx, - ), + StateTransition::ShieldedTransfer(st) => { + st.transform_into_action_for_shielded_transfer_transition(platform, tx) + } + StateTransition::Unshield(st) => { + st.transform_into_action_for_unshield_transition(platform, tx) + } StateTransition::ShieldFromAssetLock(st) => { let signable_bytes = self.signable_bytes()?; st.transform_into_action_for_shield_from_asset_lock_transition( @@ -274,12 +267,7 @@ impl StateTransitionActionTransformer for StateTransition { ) } StateTransition::ShieldedWithdrawal(st) => st - .transform_into_action_for_shielded_withdrawal_transition( - platform, - block_info, - execution_context, - tx, - ), + .transform_into_action_for_shielded_withdrawal_transition(platform, block_info, tx), } } } diff --git a/packages/rs-drive/src/state_transition_action/action_convert_to_operations/shielded/shielded_transfer_transition.rs b/packages/rs-drive/src/state_transition_action/action_convert_to_operations/shielded/shielded_transfer_transition.rs index f0d0cfa47f0..0ef537802b4 100644 --- a/packages/rs-drive/src/state_transition_action/action_convert_to_operations/shielded/shielded_transfer_transition.rs +++ b/packages/rs-drive/src/state_transition_action/action_convert_to_operations/shielded/shielded_transfer_transition.rs @@ -150,4 +150,76 @@ mod tests { let result = action.into_high_level_drive_operations(&epoch, platform_version); assert!(result.is_err()); } + + /// Audit: the flat `compute_minimum_shielded_fee` must cover the *actual* GroveDB write + /// cost (`Drive::calculate_fee`) of a shielded transition's operations. + /// + /// A shielded transfer is the cleanest per-action case — insert nullifiers + notes + + /// pool-balance update, with no contract document — so it isolates the dominant variable + /// cost (note storage). We use production-sized notes (216-byte encrypted note → 280-byte + /// commitment-tree item) and measure the real cost in estimation mode (`apply = false`). + /// + /// The flat fee covers it with large margin because it also bundles a flat 100M + /// proof-verification fee that `calculate_fee` never charges (Halo 2 verification is CPU, + /// not a GroveDB op). 16 is the max actions per bundle. + #[test] + fn test_minimum_shielded_fee_covers_actual_grovedb_write_cost() { + use crate::util::test_helpers::setup::setup_drive_with_initial_state_structure; + use dpp::block::block_info::BlockInfo; + use dpp::shielded::compute_minimum_shielded_fee; + + let drive = setup_drive_with_initial_state_structure(None); + let platform_version = PlatformVersion::latest(); + let epoch = Epoch::new(0).unwrap(); + + // Production-sized note: 216-byte encrypted note, distinct nullifier/cmx per action. + let realistic_note = |i: u8| ShieldedActionNote { + nullifier: [i.wrapping_add(1); 32], + cmx: [i.wrapping_add(101); 32], + encrypted_note: vec![0x77; 216], + }; + + for num_actions in [1usize, 8, 16] { + let fee_amount = compute_minimum_shielded_fee(num_actions, platform_version) + .expect("fee computation should not overflow"); + let notes: Vec<_> = (0..num_actions as u8).map(realistic_note).collect(); + let action = ShieldedTransferTransitionAction::V0(ShieldedTransferTransitionActionV0 { + notes, + anchor: [0xAA; 32], + fee_amount, + current_total_balance: fee_amount + 1_000_000, + }); + + let ops = action + .into_high_level_drive_operations(&epoch, platform_version) + .expect("operations"); + + // apply = false → estimation mode: no DB mutation, returns the real cost. + let fee_result = drive + .apply_drive_operations( + ops, + false, + &BlockInfo::default(), + None, + platform_version, + None, + ) + .expect("estimate write cost"); + let actual_cost = fee_result.total_base_fee(); + + // The fee must cover the real write cost. Measured margins over GroveDB cost + // (estimation mode, production-sized notes): ~10.9x at 1 action down to ~5.8x at + // the 16-action max. The margin is large and stays well above 1x because the + // per-action fee also prices the per-action Halo 2 verification CPU (which + // calculate_fee does not charge), so it exceeds the per-action GroveDB cost by + // design; see `shielded_per_action_processing_fee`. + assert!( + fee_amount >= actual_cost, + "compute_minimum_shielded_fee({num_actions}) = {fee_amount} must cover the actual \ + GroveDB write cost {actual_cost} (storage {} + processing {})", + fee_result.storage_fee, + fee_result.processing_fee + ); + } + } } diff --git a/packages/rs-drive/src/state_transition_action/action_convert_to_operations/shielded/shielded_withdrawal_transition.rs b/packages/rs-drive/src/state_transition_action/action_convert_to_operations/shielded/shielded_withdrawal_transition.rs index 56308ebbebe..0483c7c693a 100644 --- a/packages/rs-drive/src/state_transition_action/action_convert_to_operations/shielded/shielded_withdrawal_transition.rs +++ b/packages/rs-drive/src/state_transition_action/action_convert_to_operations/shielded/shielded_withdrawal_transition.rs @@ -32,20 +32,17 @@ impl DriveHighLevelOperationConverter for ShieldedWithdrawalTransitionAction { // 2. Insert change notes into CommitmentTree insert_notes(&mut ops, &v0.notes); - // 3. Update total balance: subtract withdrawal amount + fee (both leave the pool) - let total_deduction = - v0.amount.checked_add(v0.fee_amount).ok_or_else(|| { - Error::Drive(DriveError::CorruptedDriveState( - "overflow when adding shielded_withdrawal amount and fee" - .to_string(), - )) - })?; + // 3. Update total balance: the pool decreases by the full `amount` + // (= unshielding_amount). Of that, `amount - fee_amount` leaves the + // platform to Core (see RemoveFromSystemCredits below) and + // `fee_amount` stays in-platform, flowing to the fee pools at block + // finalization, so credits are conserved. let new_total_balance = v0.current_total_balance - .checked_sub(total_deduction) + .checked_sub(v0.amount) .ok_or_else(|| { Error::Drive(DriveError::CorruptedDriveState( - "shielded pool total balance underflow when subtracting shielded_withdrawal amount and fee" + "shielded pool total balance underflow when subtracting shielded_withdrawal amount" .to_string(), )) })?; @@ -64,9 +61,22 @@ impl DriveHighLevelOperationConverter for ShieldedWithdrawalTransitionAction { }, )); - // 5. Remove credits from system (they leave the system to Core) + // 5. Remove credits from the system: only the NET amount + // (`amount - fee_amount`) actually leaves the platform to Core. The + // fee stays in-platform (moves from the shielded pool sum tree to the + // fee pools sum tree), so the `total_credits_in_platform` counter must + // only drop by the net. Validation guarantees `amount >= fee_amount`; + // we still guard the subtraction defensively. + let net_withdrawal_amount = + v0.amount.checked_sub(v0.fee_amount).ok_or_else(|| { + Error::Drive(DriveError::CorruptedDriveState( + "shielded_withdrawal fee exceeds withdrawal amount".to_string(), + )) + })?; ops.push(DriveOperation::SystemOperation( - SystemOperationType::RemoveFromSystemCredits { amount: v0.amount }, + SystemOperationType::RemoveFromSystemCredits { + amount: net_withdrawal_amount, + }, )); Ok(ops) @@ -92,6 +102,9 @@ mod tests { use dpp::document::{Document, DocumentV0, DocumentV0Getters}; use dpp::identity::core_script::CoreScript; use dpp::platform_value::Identifier; + use dpp::shielded::{compute_minimum_shielded_fee, SerializedAction}; + use dpp::state_transition::shielded_withdrawal_transition::ShieldedWithdrawalTransition; + use dpp::state_transition::state_transitions::shielded::shielded_withdrawal_transition::v0::ShieldedWithdrawalTransitionV0; use dpp::version::PlatformVersion; use dpp::withdrawal::Pooling; @@ -175,12 +188,12 @@ mod tests { other => panic!("expected InsertNote, got {:?}", other), } - // Verify UpdateTotalBalance = 10000 - 3000 - 500 = 6500 + // Verify UpdateTotalBalance = 10000 - 3000 (amount only) = 7000 match &ops[2] { DriveOperation::ShieldedPoolOperation( ShieldedPoolOperationType::UpdateTotalBalance { new_total_balance }, ) => { - assert_eq!(*new_total_balance, 6500); + assert_eq!(*new_total_balance, 7000); } other => panic!("expected UpdateTotalBalance, got {:?}", other), } @@ -198,19 +211,21 @@ mod tests { other => panic!("expected AddWithdrawalDocument, got {:?}", other), } - // Verify RemoveFromSystemCredits amount = 3000 + // Verify RemoveFromSystemCredits amount = NET (3000 - 500) = 2500 match &ops[4] { DriveOperation::SystemOperation(SystemOperationType::RemoveFromSystemCredits { amount, }) => { - assert_eq!(*amount, 3000); + assert_eq!(*amount, 2500); } other => panic!("expected RemoveFromSystemCredits, got {:?}", other), } } #[test] - fn test_balance_decreases_by_amount_plus_fee() { + fn test_balance_decreases_by_amount_only() { + // The pool decrements by exactly `amount` (= unshielding_amount); the fee is + // carved out of that amount, not charged on top of it. let action = make_action(); let epoch = Epoch::new(0).unwrap(); let platform_version = PlatformVersion::latest(); @@ -232,7 +247,7 @@ mod tests { DriveOperation::ShieldedPoolOperation( ShieldedPoolOperationType::UpdateTotalBalance { new_total_balance }, ) => { - assert_eq!(*new_total_balance, 6500); // 10000 - 3000 - 500 + assert_eq!(*new_total_balance, 7000); // 10000 - 3000 (amount only) } _ => unreachable!(), } @@ -278,7 +293,9 @@ mod tests { } #[test] - fn test_removes_from_system_credits() { + fn test_removes_net_from_system_credits() { + // Only the NET amount (amount - fee) leaves the platform to Core; the fee + // stays in-platform and flows to the fee pools. let action = make_action(); let epoch = Epoch::new(0).unwrap(); let platform_version = PlatformVersion::latest(); @@ -291,14 +308,137 @@ mod tests { DriveOperation::SystemOperation(SystemOperationType::RemoveFromSystemCredits { amount, }) => { - assert_eq!(*amount, 3000); + assert_eq!(*amount, 2500); // 3000 amount - 500 fee } other => panic!("expected RemoveFromSystemCredits, got {:?}", other), } } + /// A minimal serialized Orchard action (dummy bytes; the transformer only copies + /// these fields into the action's notes — no proof verification happens here). + fn make_serialized_action() -> SerializedAction { + SerializedAction { + nullifier: [0x11; 32], + rk: [0x33; 32], + cmx: [0x22; 32], + encrypted_note: vec![1, 2, 3], + cv_net: [0x44; 32], + spend_auth_sig: [0x55; 64], + } + } + + /// Build a real single-action `ShieldedWithdrawalTransition` with the given gross amount. + fn make_transition(unshielding_amount: u64) -> ShieldedWithdrawalTransition { + ShieldedWithdrawalTransition::V0(ShieldedWithdrawalTransitionV0 { + actions: vec![make_serialized_action()], + unshielding_amount, + anchor: [0xAA; 32], + proof: vec![], + binding_signature: [0u8; 64], + core_fee_per_byte: 1, + pooling: Pooling::Never, + output_script: CoreScript::from_bytes(vec![0x76, 0xA9]), + }) + } + #[test] - fn test_underflow_returns_error() { + fn test_fee_amount_is_non_zero() { + // Regression guard for the fee-bypass bug. Drive the action through the REAL + // transformer (`try_from_transition`) using the REAL fee function — not a hardcoded + // fixture — so the test fails if the fee is computed as zero or dropped on the way + // into the action. + let platform_version = PlatformVersion::latest(); + let fee = compute_minimum_shielded_fee(1, platform_version) + .expect("fee computation should not overflow"); + assert!(fee > 0, "computed minimum shielded fee must be non-zero"); + + // Net (= unshielding_amount - fee) must clear the dust floor for the transform to + // succeed; pad comfortably above it. + let transition = make_transition(fee + 1_000_000); + + let result = ShieldedWithdrawalTransitionAction::try_from_transition( + &transition, + 10_000_000, + 0, + fee, + platform_version, + ); + assert!(result.is_valid(), "errors: {:?}", result.errors); + let action = result.into_data().expect("action"); + match action { + ShieldedWithdrawalTransitionAction::V0(v0) => { + assert_eq!( + v0.fee_amount, fee, + "the action must carry the computed fee, not drop it to zero" + ); + } + } + } + + #[test] + fn test_transform_rejects_net_below_min_withdrawal_amount() { + // A gross amount that covers the fee but leaves a net below the Core dust floor must + // be rejected by the transformer, not turned into a zero/dust withdrawal document. + let platform_version = PlatformVersion::latest(); + let fee = compute_minimum_shielded_fee(1, platform_version) + .expect("fee computation should not overflow"); + let min_withdrawal = platform_version.system_limits.min_withdrawal_amount; + + // net = min_withdrawal_amount - 1 (just under the floor) + let transition = make_transition(fee + min_withdrawal - 1); + + let result = ShieldedWithdrawalTransitionAction::try_from_transition( + &transition, + 10_000_000, + 0, + fee, + platform_version, + ); + assert!( + !result.is_valid(), + "transform must reject a sub-dust net withdrawal amount" + ); + } + + #[test] + fn test_conservation_pool_minus_amount_system_minus_net() { + // Conservation at the operation level: + // - shielded pool sum tree: -amount + // - RemoveFromSystemCredits (counter): -(amount - fee) + // The fee (= amount - net) is reconciled into the fee pools sum tree at block + // finalization, so both the sum-tree total and the counter ultimately drop by + // exactly the net (amount - fee). + let amount = 3000u64; + let fee = 500u64; + let action = make_action(); + let epoch = Epoch::new(0).unwrap(); + let platform_version = PlatformVersion::latest(); + + let ops = action + .into_high_level_drive_operations(&epoch, platform_version) + .expect("expected operations"); + + let mut shielded_delta: i128 = 0; + let mut system_credit_delta: i128 = 0; + for op in &ops { + match op { + DriveOperation::ShieldedPoolOperation( + ShieldedPoolOperationType::UpdateTotalBalance { new_total_balance }, + ) => shielded_delta = *new_total_balance as i128 - 10000i128, + DriveOperation::SystemOperation(SystemOperationType::RemoveFromSystemCredits { + amount, + }) => system_credit_delta = -(*amount as i128), + _ => {} + } + } + + assert_eq!(shielded_delta, -(amount as i128)); + assert_eq!(system_credit_delta, -((amount - fee) as i128)); + } + + #[test] + fn test_pool_underflow_returns_error() { + // Pool has less than `amount`; pool decrement must error. let action = ShieldedWithdrawalTransitionAction::V0(ShieldedWithdrawalTransitionActionV0 { amount: 5000, notes: vec![], @@ -306,8 +446,30 @@ mod tests { core_fee_per_byte: 1, pooling: Pooling::Never, output_script: CoreScript::from_bytes(vec![]), - fee_amount: 6000, - current_total_balance: 10000, // 5000 + 6000 > 10000 + fee_amount: 500, + current_total_balance: 4000, // 4000 < 5000 (amount) + prepared_withdrawal_document: make_document(), + }); + let epoch = Epoch::new(0).unwrap(); + let platform_version = PlatformVersion::latest(); + + let result = action.into_high_level_drive_operations(&epoch, platform_version); + assert!(result.is_err()); + } + + #[test] + fn test_fee_exceeds_amount_returns_error() { + // Defensive guard: if fee somehow exceeds amount, the net-withdrawal + // subtraction in RemoveFromSystemCredits must error rather than underflow. + let action = ShieldedWithdrawalTransitionAction::V0(ShieldedWithdrawalTransitionActionV0 { + amount: 100, + notes: vec![], + anchor: [0x00; 32], + core_fee_per_byte: 1, + pooling: Pooling::Never, + output_script: CoreScript::from_bytes(vec![]), + fee_amount: 500, // fee > amount + current_total_balance: 10000, prepared_withdrawal_document: make_document(), }); let epoch = Epoch::new(0).unwrap(); diff --git a/packages/rs-drive/src/state_transition_action/action_convert_to_operations/shielded/unshield_transition.rs b/packages/rs-drive/src/state_transition_action/action_convert_to_operations/shielded/unshield_transition.rs index 94c2e516c2c..4684ba8a9d7 100644 --- a/packages/rs-drive/src/state_transition_action/action_convert_to_operations/shielded/unshield_transition.rs +++ b/packages/rs-drive/src/state_transition_action/action_convert_to_operations/shielded/unshield_transition.rs @@ -28,31 +28,48 @@ impl DriveHighLevelOperationConverter for UnshieldTransitionAction { // 1. Insert each nullifier (known to not exist after validation) insert_nullifiers(&mut ops, &v0.notes); - // 2. Credit the output address with the unshielded amount - ops.push(DriveOperation::AddressFundsOperation( - AddressFundsOperationType::AddBalanceToAddress { - address: v0.output_address, - balance_to_add: v0.amount, - }, - )); + // 2. Credit the output address with the NET unshielded amount. + // `amount` (= unshielding_amount) is the total leaving the pool; the + // fee is carved out of it and routed to the fee pools via the + // PaidFromShieldedPool execution event, so the recipient receives + // `amount - fee_amount`. Validation (validate_minimum_shielded_fee) + // guarantees `amount >= fee_amount`, so this subtraction cannot + // underflow; we still guard defensively. + let net_recipient_amount = + v0.amount.checked_sub(v0.fee_amount).ok_or_else(|| { + Error::Drive(DriveError::CorruptedDriveState( + "unshield fee exceeds unshielding amount".to_string(), + )) + })?; + // Only credit the output address when the net is positive. A + // net-zero unshield (the whole `unshielding_amount` was consumed by + // the fee) would otherwise insert a spurious zero-balance address + // entry. Skipping it is conservation-neutral (adding 0 changes + // nothing): the pool still decreases by `amount` and the fee still + // flows to the fee pools. + if net_recipient_amount > 0 { + ops.push(DriveOperation::AddressFundsOperation( + AddressFundsOperationType::AddBalanceToAddress { + address: v0.output_address, + balance_to_add: net_recipient_amount, + }, + )); + } // 3. Insert notes into CommitmentTree (change outputs) insert_notes(&mut ops, &v0.notes); - // 4. Update total balance - // Pool decreases by amount (to output address) + fee_amount (to proposers) - let total_deduction = - v0.amount.checked_add(v0.fee_amount).ok_or_else(|| { - Error::Drive(DriveError::CorruptedDriveState( - "overflow when adding unshield amount and fee".to_string(), - )) - })?; + // 4. Update total balance. + // The pool decreases by the full `amount` (= unshielding_amount). + // Of that, `amount - fee_amount` is credited to the output address + // above and `fee_amount` flows to the fee pools at block finalization, + // so credits are conserved. let new_total_balance = v0.current_total_balance - .checked_sub(total_deduction) + .checked_sub(v0.amount) .ok_or_else(|| { Error::Drive(DriveError::CorruptedDriveState( - "shielded pool total balance underflow when subtracting unshield amount and fee" + "shielded pool total balance underflow when subtracting unshield amount" .to_string(), )) })?; @@ -114,7 +131,8 @@ mod tests { } #[test] - fn test_add_balance_to_output_address() { + fn test_add_net_balance_to_output_address() { + // The output address receives the NET amount (amount - fee), not the full amount. let action = make_action(); let epoch = Epoch::new(0).unwrap(); let platform_version = PlatformVersion::latest(); @@ -131,14 +149,16 @@ mod tests { }, ) => { assert_eq!(*address, PlatformAddress::P2pkh([0xBB; 20])); - assert_eq!(*balance_to_add, 3000); + assert_eq!(*balance_to_add, 2500); // 3000 amount - 500 fee } other => panic!("expected AddBalanceToAddress, got {:?}", other), } } #[test] - fn test_balance_decreases_by_amount_plus_fee() { + fn test_balance_decreases_by_amount_only() { + // The pool decrements by exactly `amount` (= unshielding_amount). The fee is + // carved out of that amount, not charged on top of it. let action = make_action(); let epoch = Epoch::new(0).unwrap(); let platform_version = PlatformVersion::latest(); @@ -151,21 +171,67 @@ mod tests { DriveOperation::ShieldedPoolOperation( ShieldedPoolOperationType::UpdateTotalBalance { new_total_balance }, ) => { - assert_eq!(*new_total_balance, 6500); // 10000 - 3000 - 500 + assert_eq!(*new_total_balance, 7000); // 10000 - 3000 (amount only) } other => panic!("expected UpdateTotalBalance, got {:?}", other), } } #[test] - fn test_amount_plus_fee_overflow_returns_error() { + fn test_fee_amount_is_non_zero() { + // Regression guard for the fee-bypass bug: unshield must charge a non-zero fee. + let action = make_action(); + match action { + UnshieldTransitionAction::V0(v0) => assert!(v0.fee_amount > 0), + } + } + + #[test] + fn test_conservation_sum_tree_delta_equals_negative_fee() { + // Conservation at the operation level: the address sum tree gains (amount - fee) + // and the shielded pool sum tree loses `amount`, so the net sum-tree change is + // -fee. The fee is reconciled into the fee pools at block finalization. + let amount = 3000u64; + let fee = 500u64; + let action = make_action(); + let epoch = Epoch::new(0).unwrap(); + let platform_version = PlatformVersion::latest(); + + let ops = action + .into_high_level_drive_operations(&epoch, platform_version) + .expect("expected operations"); + + let mut address_delta: i128 = 0; + let mut shielded_delta: i128 = 0; + for op in &ops { + match op { + DriveOperation::AddressFundsOperation( + AddressFundsOperationType::AddBalanceToAddress { balance_to_add, .. }, + ) => address_delta += *balance_to_add as i128, + DriveOperation::ShieldedPoolOperation( + ShieldedPoolOperationType::UpdateTotalBalance { new_total_balance }, + ) => shielded_delta = *new_total_balance as i128 - 10000i128, + _ => {} + } + } + + assert_eq!(address_delta, (amount - fee) as i128); + assert_eq!(shielded_delta, -(amount as i128)); + // Net sum-tree change (before fee distribution into pools) is -fee. + assert_eq!(address_delta + shielded_delta, -(fee as i128)); + } + + #[test] + fn test_fee_exceeds_amount_returns_error() { + // Defensive guard: if fee somehow exceeds amount, the net-recipient subtraction + // must error rather than underflow. let action = UnshieldTransitionAction::V0(UnshieldTransitionActionV0 { output_address: PlatformAddress::P2pkh([0xBB; 20]), - amount: u64::MAX, + amount: 100, notes: vec![], anchor: [0x00; 32], - fee_amount: 1, - current_total_balance: u64::MAX, + fee_amount: 500, // fee > amount + current_total_balance: 10000, }); let epoch = Epoch::new(0).unwrap(); let platform_version = PlatformVersion::latest(); @@ -182,7 +248,7 @@ mod tests { notes: vec![], anchor: [0x00; 32], fee_amount: 500, - current_total_balance: 5000, // 5000 < 5000 + 500 + current_total_balance: 4000, // 4000 < 5000 (amount) }); let epoch = Epoch::new(0).unwrap(); let platform_version = PlatformVersion::latest(); @@ -190,4 +256,44 @@ mod tests { let result = action.into_high_level_drive_operations(&epoch, platform_version); assert!(result.is_err()); } + + #[test] + fn test_net_zero_unshield_skips_address_credit() { + // When the whole `unshielding_amount` is consumed by the fee (net == 0), + // the conversion must NOT emit a zero-credit AddBalanceToAddress (which + // would create a spurious dust address entry). The pool still decrements + // by `amount` and the fee still flows to the fee pools. + let action = UnshieldTransitionAction::V0(UnshieldTransitionActionV0 { + output_address: PlatformAddress::P2pkh([0xBB; 20]), + amount: 500, + notes: vec![make_note()], + anchor: [0xAA; 32], + fee_amount: 500, // net = amount - fee = 0 + current_total_balance: 10000, + }); + let epoch = Epoch::new(0).unwrap(); + let platform_version = PlatformVersion::latest(); + + let ops = action + .into_high_level_drive_operations(&epoch, platform_version) + .expect("expected operations"); + + assert!( + !ops.iter().any(|op| matches!( + op, + DriveOperation::AddressFundsOperation( + AddressFundsOperationType::AddBalanceToAddress { .. } + ) + )), + "net-zero unshield must not emit a zero-credit AddBalanceToAddress" + ); + + // The pool is still decremented by exactly `amount` (10000 - 500 = 9500). + match ops.last().unwrap() { + DriveOperation::ShieldedPoolOperation( + ShieldedPoolOperationType::UpdateTotalBalance { new_total_balance }, + ) => assert_eq!(*new_total_balance, 9500), + other => panic!("expected UpdateTotalBalance, got {:?}", other), + } + } } diff --git a/packages/rs-drive/src/state_transition_action/shielded/shielded_withdrawal/transformer.rs b/packages/rs-drive/src/state_transition_action/shielded/shielded_withdrawal/transformer.rs index caaa5b3a747..e6b303391ac 100644 --- a/packages/rs-drive/src/state_transition_action/shielded/shielded_withdrawal/transformer.rs +++ b/packages/rs-drive/src/state_transition_action/shielded/shielded_withdrawal/transformer.rs @@ -3,6 +3,7 @@ use crate::state_transition_action::shielded::shielded_withdrawal::ShieldedWithd use dpp::fee::Credits; use dpp::prelude::ConsensusValidationResult; use dpp::state_transition::shielded_withdrawal_transition::ShieldedWithdrawalTransition; +use dpp::version::PlatformVersion; impl ShieldedWithdrawalTransitionAction { /// Transforms the state transition into an action @@ -10,6 +11,8 @@ impl ShieldedWithdrawalTransitionAction { value: &ShieldedWithdrawalTransition, current_total_balance: Credits, creation_time_ms: u64, + fee_amount: Credits, + platform_version: &PlatformVersion, ) -> ConsensusValidationResult { match value { ShieldedWithdrawalTransition::V0(v0) => { @@ -17,6 +20,8 @@ impl ShieldedWithdrawalTransitionAction { v0, current_total_balance, creation_time_ms, + fee_amount, + platform_version, ); result.map(|action| action.into()) } diff --git a/packages/rs-drive/src/state_transition_action/shielded/shielded_withdrawal/v0/mod.rs b/packages/rs-drive/src/state_transition_action/shielded/shielded_withdrawal/v0/mod.rs index 9ff494ca7f1..29e256d73d2 100644 --- a/packages/rs-drive/src/state_transition_action/shielded/shielded_withdrawal/v0/mod.rs +++ b/packages/rs-drive/src/state_transition_action/shielded/shielded_withdrawal/v0/mod.rs @@ -21,7 +21,8 @@ pub struct ShieldedWithdrawalTransitionActionV0 { pub pooling: Pooling, /// Core address receiving funds pub output_script: CoreScript, - /// Fee amount (value_balance - amount), paid to proposers + /// Shielded fee paid to proposers, carved out of `amount` (the net amount + /// withdrawn to Core is `amount - fee_amount`). Equals `compute_minimum_shielded_fee`. pub fee_amount: Credits, /// Current total balance of the shielded pool pub current_total_balance: Credits, diff --git a/packages/rs-drive/src/state_transition_action/shielded/shielded_withdrawal/v0/transformer.rs b/packages/rs-drive/src/state_transition_action/shielded/shielded_withdrawal/v0/transformer.rs index ccf78a23df5..d674e99c965 100644 --- a/packages/rs-drive/src/state_transition_action/shielded/shielded_withdrawal/v0/transformer.rs +++ b/packages/rs-drive/src/state_transition_action/shielded/shielded_withdrawal/v0/transformer.rs @@ -1,5 +1,8 @@ use crate::state_transition_action::shielded::shielded_withdrawal::v0::ShieldedWithdrawalTransitionActionV0; use crate::state_transition_action::shielded::ShieldedActionNote; +use dpp::consensus::basic::identity::InvalidIdentityCreditWithdrawalTransitionAmountError; +use dpp::consensus::basic::state_transition::WithdrawalBelowMinAmountError; +use dpp::consensus::basic::BasicError; use dpp::data_contracts::withdrawals_contract; use dpp::data_contracts::withdrawals_contract::v1::document_types::withdrawal; use dpp::document::{Document, DocumentV0}; @@ -7,6 +10,7 @@ use dpp::fee::Credits; use dpp::platform_value::platform_value; use dpp::prelude::ConsensusValidationResult; use dpp::state_transition::state_transitions::shielded::shielded_withdrawal_transition::v0::ShieldedWithdrawalTransitionV0; +use dpp::version::PlatformVersion; impl ShieldedWithdrawalTransitionActionV0 { /// Transforms the shielded withdrawal transition into an action @@ -14,10 +18,52 @@ impl ShieldedWithdrawalTransitionActionV0 { value: &ShieldedWithdrawalTransitionV0, current_total_balance: Credits, creation_time_ms: u64, + fee_amount: Credits, + platform_version: &PlatformVersion, ) -> ConsensusValidationResult { let notes: Vec = value.actions.iter().map(ShieldedActionNote::from).collect(); + // The withdrawal document records the NET amount actually leaving the platform + // to Core, i.e. `unshielding_amount - fee_amount`. The fee is carved out of the + // unshielding amount and stays in-platform (routed to the fee pools). + // + // That net amount becomes a Core `TxOut`, so it must fall within the same + // `[min_withdrawal_amount, max_withdrawal_amount]` range the transparent withdrawal + // paths enforce (dust floor and per-transition policy cap). Consensus validation + // (`validate_minimum_shielded_fee`) already rejects any transition whose net falls + // outside that range, so for validated input this `checked_sub` is always + // `Some(net)` in range. We re-check here (rather than `saturating_sub`) so a direct + // or future caller that bypasses validation fails loudly instead of silently + // constructing an out-of-range withdrawal document. + let min_withdrawal_amount = platform_version.system_limits.min_withdrawal_amount; + let max_withdrawal_amount = platform_version.system_limits.max_withdrawal_amount; + let net_withdrawal_amount = match value.unshielding_amount.checked_sub(fee_amount) { + Some(net) if net >= min_withdrawal_amount && net <= max_withdrawal_amount => net, + Some(net) if net > max_withdrawal_amount => { + // Over the per-transition withdrawal cap. + return ConsensusValidationResult::new_with_error( + InvalidIdentityCreditWithdrawalTransitionAmountError::new( + net, + min_withdrawal_amount, + max_withdrawal_amount, + ) + .into(), + ); + } + net => { + // Below the dust floor (or fee exceeds the gross — underflow). + return ConsensusValidationResult::new_with_error( + BasicError::WithdrawalBelowMinAmountError(WithdrawalBelowMinAmountError::new( + net.unwrap_or(0), + min_withdrawal_amount, + max_withdrawal_amount, + )) + .into(), + ); + } + }; + // Generate entropy from first nullifier + output_script for document ID let mut entropy = Vec::new(); if let Some(first_note) = notes.first() { @@ -36,7 +82,7 @@ impl ShieldedWithdrawalTransitionActionV0 { ); let document_data = platform_value!({ - withdrawal::properties::AMOUNT: value.unshielding_amount, + withdrawal::properties::AMOUNT: net_withdrawal_amount, withdrawal::properties::CORE_FEE_PER_BYTE: value.core_fee_per_byte, withdrawal::properties::POOLING: value.pooling, withdrawal::properties::OUTPUT_SCRIPT: value.output_script.as_bytes(), @@ -70,7 +116,7 @@ impl ShieldedWithdrawalTransitionActionV0 { core_fee_per_byte: value.core_fee_per_byte, pooling: value.pooling, output_script: value.output_script.clone(), - fee_amount: 0, // TODO: fee calculation for shielded withdrawals + fee_amount, current_total_balance, prepared_withdrawal_document: withdrawal_document, }) diff --git a/packages/rs-drive/src/state_transition_action/shielded/unshield/transformer.rs b/packages/rs-drive/src/state_transition_action/shielded/unshield/transformer.rs index 45ad2fe2b10..2547d9752e5 100644 --- a/packages/rs-drive/src/state_transition_action/shielded/unshield/transformer.rs +++ b/packages/rs-drive/src/state_transition_action/shielded/unshield/transformer.rs @@ -9,11 +9,15 @@ impl UnshieldTransitionAction { pub fn try_from_transition( value: &UnshieldTransition, current_total_balance: Credits, + fee_amount: Credits, ) -> ConsensusValidationResult { match value { UnshieldTransition::V0(v0) => { - let result = - UnshieldTransitionActionV0::try_from_transition(v0, current_total_balance); + let result = UnshieldTransitionActionV0::try_from_transition( + v0, + current_total_balance, + fee_amount, + ); result.map(|action| action.into()) } } diff --git a/packages/rs-drive/src/state_transition_action/shielded/unshield/v0/mod.rs b/packages/rs-drive/src/state_transition_action/shielded/unshield/v0/mod.rs index bb9b113dd74..ae629b427ce 100644 --- a/packages/rs-drive/src/state_transition_action/shielded/unshield/v0/mod.rs +++ b/packages/rs-drive/src/state_transition_action/shielded/unshield/v0/mod.rs @@ -15,7 +15,8 @@ pub struct UnshieldTransitionActionV0 { pub notes: Vec, /// The anchor used for verification pub anchor: [u8; 32], - /// Fee amount (value_balance - amount), paid to proposers + /// Shielded fee paid to proposers, carved out of `amount` (the recipient + /// receives `amount - fee_amount`). Equals `compute_minimum_shielded_fee`. pub fee_amount: Credits, /// Current total balance of the shielded pool pub current_total_balance: Credits, diff --git a/packages/rs-drive/src/state_transition_action/shielded/unshield/v0/transformer.rs b/packages/rs-drive/src/state_transition_action/shielded/unshield/v0/transformer.rs index 6324b8e6d99..095e7b3e8b7 100644 --- a/packages/rs-drive/src/state_transition_action/shielded/unshield/v0/transformer.rs +++ b/packages/rs-drive/src/state_transition_action/shielded/unshield/v0/transformer.rs @@ -9,6 +9,7 @@ impl UnshieldTransitionActionV0 { pub fn try_from_transition( value: &UnshieldTransitionV0, current_total_balance: Credits, + fee_amount: Credits, ) -> ConsensusValidationResult { let notes: Vec = value.actions.iter().map(ShieldedActionNote::from).collect(); @@ -18,7 +19,7 @@ impl UnshieldTransitionActionV0 { amount: value.unshielding_amount, notes, anchor: value.anchor, - fee_amount: 0, // TODO: fee calculation for unshield + fee_amount, current_total_balance, }) } diff --git a/packages/rs-platform-version/src/version/dpp_versions/dpp_method_versions/mod.rs b/packages/rs-platform-version/src/version/dpp_versions/dpp_method_versions/mod.rs index 9d31f321c8d..c3b2001da98 100644 --- a/packages/rs-platform-version/src/version/dpp_versions/dpp_method_versions/mod.rs +++ b/packages/rs-platform-version/src/version/dpp_versions/dpp_method_versions/mod.rs @@ -8,4 +8,5 @@ pub struct DPPMethodVersions { pub epoch_core_reward_credits_for_distribution: FeatureVersion, pub daily_withdrawal_limit: FeatureVersion, pub deduct_fee_from_outputs_or_remaining_balance_of_inputs: FeatureVersion, + pub compute_minimum_shielded_fee: FeatureVersion, } diff --git a/packages/rs-platform-version/src/version/dpp_versions/dpp_method_versions/v1.rs b/packages/rs-platform-version/src/version/dpp_versions/dpp_method_versions/v1.rs index 0a9fff3deeb..cb8f72ba088 100644 --- a/packages/rs-platform-version/src/version/dpp_versions/dpp_method_versions/v1.rs +++ b/packages/rs-platform-version/src/version/dpp_versions/dpp_method_versions/v1.rs @@ -3,4 +3,5 @@ pub const DPP_METHOD_VERSIONS_V1: DPPMethodVersions = DPPMethodVersions { epoch_core_reward_credits_for_distribution: 0, daily_withdrawal_limit: 0, deduct_fee_from_outputs_or_remaining_balance_of_inputs: 0, + compute_minimum_shielded_fee: 0, }; diff --git a/packages/rs-platform-version/src/version/dpp_versions/dpp_method_versions/v2.rs b/packages/rs-platform-version/src/version/dpp_versions/dpp_method_versions/v2.rs index 7ae79169c04..6df00ae6498 100644 --- a/packages/rs-platform-version/src/version/dpp_versions/dpp_method_versions/v2.rs +++ b/packages/rs-platform-version/src/version/dpp_versions/dpp_method_versions/v2.rs @@ -3,4 +3,5 @@ pub const DPP_METHOD_VERSIONS_V2: DPPMethodVersions = DPPMethodVersions { epoch_core_reward_credits_for_distribution: 0, daily_withdrawal_limit: 1, deduct_fee_from_outputs_or_remaining_balance_of_inputs: 0, + compute_minimum_shielded_fee: 0, }; diff --git a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v8.rs b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v8.rs index 8535757a33f..2fdec2b06ca 100644 --- a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v8.rs +++ b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v8.rs @@ -321,6 +321,9 @@ pub const DRIVE_ABCI_VALIDATION_VERSIONS_V8: DriveAbciValidationVersions = shielded_anchor_retention_blocks: 1000, shielded_anchor_pruning_interval: 100, shielded_proof_verification_fee: 100_000_000, - shielded_per_action_processing_fee: 3_000_000, + // Per-action processing prices the ~1.1 ms/action Halo 2 verification CPU at the + // same rate the flat fee prices the ~5 ms base (100M ≈ 4.5× this), so the fee + // tracks the per-action cost and the margin stays uniform as actions grow. + shielded_per_action_processing_fee: 22_000_000, }, }; diff --git a/packages/rs-platform-version/src/version/mocks/v2_test.rs b/packages/rs-platform-version/src/version/mocks/v2_test.rs index f317b8b580d..8428f56d283 100644 --- a/packages/rs-platform-version/src/version/mocks/v2_test.rs +++ b/packages/rs-platform-version/src/version/mocks/v2_test.rs @@ -529,6 +529,7 @@ pub const TEST_PLATFORM_V2: PlatformVersion = PlatformVersion { withdrawal_transactions_per_block_limit: 4, retry_signing_expired_withdrawal_documents_per_block_limit: 1, max_withdrawal_amount: 50_000_000_000_000, + min_withdrawal_amount: 190_000, max_contract_group_size: 256, max_token_redemption_cycles: 128, max_shielded_transition_actions: 16, diff --git a/packages/rs-platform-version/src/version/system_limits/mod.rs b/packages/rs-platform-version/src/version/system_limits/mod.rs index f5531de6762..137681c7e25 100644 --- a/packages/rs-platform-version/src/version/system_limits/mod.rs +++ b/packages/rs-platform-version/src/version/system_limits/mod.rs @@ -1,4 +1,5 @@ pub mod v1; +pub mod v2; #[derive(Clone, Debug, Default)] pub struct SystemLimits { @@ -12,6 +13,11 @@ pub struct SystemLimits { pub withdrawal_transactions_per_block_limit: u16, pub retry_signing_expired_withdrawal_documents_per_block_limit: u16, pub max_withdrawal_amount: u64, + /// Minimum net amount (in credits) a withdrawal may send to Core, shared by the + /// transparent (identity + address) and shielded withdrawal paths. The dust floor that + /// keeps Core from rejecting the resulting `TxOut`. Versioned: see `min_withdrawal_amount` + /// in each `SYSTEM_LIMITS_V*`. + pub min_withdrawal_amount: u64, pub max_contract_group_size: u16, // This the max redemption cycles we can process if we don't use a constant distribution // For a constant perpetual distribution this is very cheap since it's just a multiplication diff --git a/packages/rs-platform-version/src/version/system_limits/v1.rs b/packages/rs-platform-version/src/version/system_limits/v1.rs index c4237df7ca8..d374c858021 100644 --- a/packages/rs-platform-version/src/version/system_limits/v1.rs +++ b/packages/rs-platform-version/src/version/system_limits/v1.rs @@ -21,6 +21,9 @@ pub const SYSTEM_LIMITS_V1: SystemLimits = SystemLimits { withdrawal_transactions_per_block_limit: 4, retry_signing_expired_withdrawal_documents_per_block_limit: 1, max_withdrawal_amount: 50_000_000_000_000, //500 Dash + // = dpp MIN_WITHDRAWAL_AMOUNT: ASSET_UNLOCK_TX_SIZE(190) * MIN_CORE_FEE_PER_BYTE(1) + // * CREDITS_PER_DUFF(1000) = 190_000 credits = 190 duffs. + min_withdrawal_amount: 190_000, max_contract_group_size: 256, max_token_redemption_cycles: 128, // 16 actions x 408 bytes + ~5,305 bytes overhead = ~11,833 bytes (within 20 KiB max_state_transition_size) diff --git a/packages/rs-platform-version/src/version/system_limits/v2.rs b/packages/rs-platform-version/src/version/system_limits/v2.rs new file mode 100644 index 00000000000..68ae793ae6b --- /dev/null +++ b/packages/rs-platform-version/src/version/system_limits/v2.rs @@ -0,0 +1,21 @@ +use crate::version::system_limits::SystemLimits; + +/// System limits for protocol version 12 and above. +/// +/// Identical to [`super::v1::SYSTEM_LIMITS_V1`] except that `min_withdrawal_amount` is raised +/// from 190,000 credits (190 duffs) to 1,000,000 credits (1000 duffs): the previous floor was +/// the bare asset-unlock transaction fee and too low a minimum for a Core `TxOut`. +pub const SYSTEM_LIMITS_V2: SystemLimits = SystemLimits { + estimated_contract_max_serialized_size: 16384, + max_field_value_size: 5120, //5 KiB + max_state_transition_size: 20480, //20 KiB + max_transitions_in_documents_batch: 1, + withdrawal_transactions_per_block_limit: 4, + retry_signing_expired_withdrawal_documents_per_block_limit: 1, + max_withdrawal_amount: 50_000_000_000_000, //500 Dash + min_withdrawal_amount: 1_000_000, //1000 duffs (raised from 190 in v12) + max_contract_group_size: 256, + max_token_redemption_cycles: 128, + // 16 actions x 408 bytes + ~5,305 bytes overhead = ~11,833 bytes (within 20 KiB max_state_transition_size) + max_shielded_transition_actions: 16, +}; diff --git a/packages/rs-platform-version/src/version/v12.rs b/packages/rs-platform-version/src/version/v12.rs index 0c226afaa20..2d334b1fd7d 100644 --- a/packages/rs-platform-version/src/version/v12.rs +++ b/packages/rs-platform-version/src/version/v12.rs @@ -25,7 +25,7 @@ use crate::version::drive_versions::v7::DRIVE_VERSION_V7; use crate::version::fee::v2::FEE_VERSION2; use crate::version::protocol_version::PlatformVersion; use crate::version::system_data_contract_versions::v1::SYSTEM_DATA_CONTRACT_VERSIONS_V1; -use crate::version::system_limits::v1::SYSTEM_LIMITS_V1; +use crate::version::system_limits::v2::SYSTEM_LIMITS_V2; use crate::version::ProtocolVersion; pub const PROTOCOL_VERSION_12: ProtocolVersion = 12; @@ -64,7 +64,7 @@ pub const PLATFORM_V12: PlatformVersion = PlatformVersion { }, system_data_contracts: SYSTEM_DATA_CONTRACT_VERSIONS_V1, fee_version: FEE_VERSION2, - system_limits: SYSTEM_LIMITS_V1, + system_limits: SYSTEM_LIMITS_V2, consensus: ConsensusVersions { tenderdash_consensus_version: 1, }, 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 27238da6b00..3ddfcb704e9 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/note_selection.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/note_selection.rs @@ -78,13 +78,15 @@ pub fn select_notes_with_fee<'a>( min_actions: usize, platform_version: &PlatformVersion, ) -> Result<(Vec<&'a ShieldedNote>, u64, u64), PlatformWalletError> { - let mut fee_estimate = compute_minimum_shielded_fee(min_actions, platform_version); + let mut fee_estimate = compute_minimum_shielded_fee(min_actions, platform_version) + .map_err(|e| PlatformWalletError::ShieldedBuildError(e.to_string()))?; for _ in 0..5 { let selected = select_notes(unspent, amount, fee_estimate)?; let total: u64 = selected.iter().map(|n| n.value).sum(); let num_actions = selected.len().max(min_actions); - let exact_fee = compute_minimum_shielded_fee(num_actions, platform_version); + let exact_fee = compute_minimum_shielded_fee(num_actions, platform_version) + .map_err(|e| PlatformWalletError::ShieldedBuildError(e.to_string()))?; if total >= amount.saturating_add(exact_fee) { return Ok((selected, total, exact_fee)); @@ -97,7 +99,8 @@ pub fn select_notes_with_fee<'a>( let selected = select_notes(unspent, amount, fee_estimate)?; let total: u64 = selected.iter().map(|n| n.value).sum(); let num_actions = selected.len().max(min_actions); - let exact_fee = compute_minimum_shielded_fee(num_actions, platform_version); + let exact_fee = compute_minimum_shielded_fee(num_actions, platform_version) + .map_err(|e| PlatformWalletError::ShieldedBuildError(e.to_string()))?; if total < amount.saturating_add(exact_fee) { return Err(PlatformWalletError::ShieldedInsufficientBalance { @@ -189,4 +192,48 @@ mod tests { let result = select_notes(¬es, u64::MAX, 1); assert!(result.is_err()); } + + #[test] + fn test_select_notes_with_fee_floors_to_min_actions() { + let platform_version = PlatformVersion::latest(); + let min_fee_2 = compute_minimum_shielded_fee(2, platform_version) + .expect("fee computation should not overflow"); + let amount = 1_000_000u64; + // 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, platform_version).expect("selection ok"); + + assert_eq!(selected.len(), 1); + assert_eq!(total, amount + min_fee_2 + 5); + // One selected note → num_actions = max(1, min_actions=2) = 2, so the fee is the + // 2-action minimum even though only one note is spent (the Orchard bundle pads to 2). + assert_eq!(exact_fee, min_fee_2); + } + + #[test] + fn test_select_notes_with_fee_uses_actual_action_count() { + let platform_version = PlatformVersion::latest(); + let amount = 1_000_000u64; + // Many equal mid-size notes so several are needed; the convergence loop must settle on + // a fee that matches the actual selected-note (action) count, not the min_actions floor. + 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, platform_version).expect("selection ok"); + + let expected_fee = + compute_minimum_shielded_fee(selected.len().max(2), platform_version).unwrap(); + assert_eq!( + exact_fee, expected_fee, + "fee must match the selected action count" + ); + assert!( + total >= amount.saturating_add(exact_fee), + "selection must cover amount + fee" + ); + assert!(selected.len() >= 2, "expected multiple notes selected"); + } } diff --git a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs index ec1857bc250..f5bc297f7ce 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs @@ -283,8 +283,11 @@ pub async fn unshield( let change_addr = default_orchard_address(keys)?; let id = SubwalletId::new(wallet_id, account); + // Reserve against the 2-action floor: Orchard's BundleType::DEFAULT pads single-spend + // bundles to 2 actions, and the builder prices the fee at spends.len().max(2). Reserving + // for 1 would under-fee a single-note transition and the builder would reject it locally. let (selected_notes, total_input, exact_fee) = - reserve_unspent_notes(sdk, store, id, amount, 1).await?; + reserve_unspent_notes(sdk, store, id, amount, 2).await?; info!( account, @@ -300,7 +303,9 @@ pub async fn unshield( let result = async { let (spends, anchor) = extract_spends_and_anchor(store, &selected_notes).await?; - let state_transition = build_unshield_transition( + // The builder computes and returns the fee authoritatively; `exact_fee` (== the + // minimum) was already used above for note reservation. + let (state_transition, _fee_used) = build_unshield_transition( spends, *to_address, amount, @@ -310,7 +315,6 @@ pub async fn unshield( anchor, prover, [0u8; 36], - Some(exact_fee), sdk.version(), ) .map_err(|e| PlatformWalletError::ShieldedBuildError(e.to_string()))?; @@ -396,7 +400,9 @@ pub async fn transfer( let result = async { let (spends, anchor) = extract_spends_and_anchor(store, &selected_notes).await?; - let state_transition = build_shielded_transfer_transition( + // The builder computes and returns the fee authoritatively; `exact_fee` (== the + // minimum) was already used above for note reservation. + let (state_transition, _fee_used) = build_shielded_transfer_transition( spends, &recipient_addr, amount, @@ -406,7 +412,6 @@ pub async fn transfer( anchor, prover, [0u8; 36], - Some(exact_fee), sdk.version(), ) .map_err(|e| PlatformWalletError::ShieldedBuildError(e.to_string()))?; @@ -468,8 +473,11 @@ pub async fn withdraw( let id = SubwalletId::new(wallet_id, account); let output_script = CoreScript::from_bytes(to_address.script_pubkey().to_bytes()); + // Reserve against the 2-action floor: Orchard's BundleType::DEFAULT pads single-spend + // bundles to 2 actions, and the builder prices the fee at spends.len().max(2). Reserving + // for 1 would under-fee a single-note transition and the builder would reject it locally. let (selected_notes, total_input, exact_fee) = - reserve_unspent_notes(sdk, store, id, amount, 1).await?; + reserve_unspent_notes(sdk, store, id, amount, 2).await?; info!( account, @@ -483,19 +491,21 @@ pub async fn withdraw( let result = async { let (spends, anchor) = extract_spends_and_anchor(store, &selected_notes).await?; - let state_transition = build_shielded_withdrawal_transition( + // The builder computes and returns the fee authoritatively; `exact_fee` (== the + // minimum) was already used above for note reservation. + let (state_transition, _fee_used) = build_shielded_withdrawal_transition( spends, amount, output_script, core_fee_per_byte, - Pooling::Standard, + // Consensus pins shielded-withdrawal pooling to Never (validate_structure). + Pooling::Never, &change_addr, &keys.full_viewing_key, &keys.spend_auth_key, anchor, prover, [0u8; 36], - Some(exact_fee), sdk.version(), ) .map_err(|e| PlatformWalletError::ShieldedBuildError(e.to_string()))?;