fix(drive): charge fees for unshield and shielded withdrawal

packages/rs-dpp/src/shielded/builder/shielded_transfer.rs

@@ -30,8 +30,9 @@ 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.
+/// - `fee` - Optional fee. A shielded transfer's `value_balance` IS the entire fee, and
+///   consensus requires it to equal the minimum exactly (transfers cannot overpay), so this
+///   must be `None` (use the minimum) or `Some(min_fee)`; any other value is rejected.
 /// - `platform_version` - Protocol version
 #[allow(clippy::too_many_arguments)]
 pub fn build_shielded_transfer_transition<P: OrchardProver>(
@@ -53,21 +54,22 @@ pub fn build_shielded_transfer_transition<P: OrchardProver>(
     // 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);
+    // A shielded transfer's `value_balance` IS the entire fee, and consensus requires it to
+    // equal the minimum exactly: overpayment buys nothing (Platform has no fee-priority
+    // market) and would leak a distinguishing fee fingerprint that breaks shielded
+    // uniformity. The fee is therefore fixed — accept `None` (use the minimum) or
+    // `Some(min_fee)`, and reject anything else so the caller fails fast here instead of
+    // building a transition the network will reject.
     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) => {
+        None => min_fee,
+        Some(f) if f == min_fee => min_fee,
+        Some(f) => {
             return Err(ProtocolError::ShieldedBuildError(format!(
-                "fee {} exceeds 1000x the minimum fee {}",
-                f, min_fee
+                "shielded transfer fee must equal the minimum fee {} exactly \
+                 (transfers cannot overpay); got {}",
+                min_fee, f
             )));
         }
-        Some(f) => f,
-        None => min_fee,
     };
 
     let required = transfer_amount.checked_add(effective_fee).ok_or_else(|| {
@@ -172,7 +174,7 @@ mod tests {
         assert!(result.is_err());
         let err = result.unwrap_err().to_string();
         assert!(
-            err.contains("below minimum required fee"),
+            err.contains("must equal the minimum fee"),
             "unexpected error: {}",
             err
         );
@@ -221,8 +223,9 @@ mod tests {
     // --------------------------------------------------------------
 
     #[test]
-    fn test_shielded_transfer_fee_above_upper_bound() {
-        // Fee > 1000x the minimum fee should be rejected.
+    fn test_shielded_transfer_fee_above_minimum_rejected() {
+        // A transfer's fee is fixed at the minimum; even 1 credit above it is rejected
+        // (overpayment is not allowed).
         let platform_version = PlatformVersion::latest();
         let recipient = test_orchard_address();
         let change_address = test_orchard_address();
@@ -237,7 +240,6 @@ mod tests {
 
         // 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,
@@ -249,14 +251,14 @@ mod tests {
             Anchor::empty_tree(),
             &TestProver,
             [0u8; 36],
-            Some(excessive_fee),
+            Some(min_fee + 1),
             platform_version,
         );
 
         assert!(result.is_err());
         let err = result.unwrap_err().to_string();
         assert!(
-            err.contains("exceeds 1000x the minimum fee"),
+            err.contains("must equal the minimum fee"),
             "unexpected error: {}",
             err
         );

packages/rs-dpp/src/shielded/mod.rs

@@ -9,9 +9,31 @@ use sha2::{Digest, Sha256};
 use crate::fee::Credits;
 use platform_version::version::PlatformVersion;
 
-/// 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:
+///   compact note (52 = version 1 + diversifier `d` 11 + value 8 + `rseed` 32)
+///   + memo (36) + 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.

packages/rs-drive-abci/src/execution/validation/state_transition/processor/traits/shielded_proof.rs

@@ -3,9 +3,14 @@ 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::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::state_transitions::address_funds::address_credit_withdrawal_transition::MIN_WITHDRAWAL_AMOUNT;
 use dpp::state_transition::StateTransition;
 use dpp::validation::SimpleConsensusValidationResult;
 use dpp::version::PlatformVersion;
@@ -68,11 +73,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
+///   meet `MIN_WITHDRAWAL_AMOUNT`, because that net becomes a Core `TxOut` and is subject to
+///   the same dust floor as the transparent withdrawal path. Without this, a transition that
+///   only just covers the fee would queue a zero/sub-dust withdrawal document (rejected
+///   downstream as `CorruptedDriveState`, or a malformed entry in the withdrawal queue).
+///   Unshield has no such floor: 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,30 +110,40 @@ 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 {
+                // Extract the checked amount, action count, and the minimum NET amount
+                // (value left after the fee) the transition must leave behind. The net
+                // floor is `MIN_WITHDRAWAL_AMOUNT` only for ShieldedWithdrawal (whose net
+                // becomes a Core `TxOut`); for the others a non-negative net suffices, so
+                // the floor is 0.
+                // `requires_exact_fee` is true only for ShieldedTransfer, whose
+                // `value_balance` IS the entire fee and must equal the minimum exactly.
+                let (fee, num_actions, min_net_amount, requires_exact_fee): (i64, usize, u64, bool) = match self {
                     // Shield: fee is paid from transparent address inputs, not from value_balance.
                     StateTransition::Shield(_) => {
                         return Ok(SimpleConsensusValidationResult::new())
                     }
                     // 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, 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 dust floor 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, false),
                     },
+                    // ShieldedWithdrawal: the net (`unshielding_amount - min_fee`) becomes a
+                    // Core `TxOut`, so it must clear the same `MIN_WITHDRAWAL_AMOUNT` dust
+                    // floor the transparent withdrawal path enforces.
                     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(), MIN_WITHDRAWAL_AMOUNT, false)
                         }
                     },
                     // Other transitions don't go through shielded fee validation.
@@ -156,7 +185,7 @@ impl StateTransitionShieldedMinimumFeeValidationV0 for StateTransition {
                     )))?;
 
                 if (fee as u64) < minimum_shielded_fee {
-                    Ok(SimpleConsensusValidationResult::new_with_error(
+                    return Ok(SimpleConsensusValidationResult::new_with_error(
                         StateError::InsufficientShieldedFeeError(
                             InsufficientShieldedFeeError::new(format!(
                                 "shielded transition fee {} is below minimum required fee {} \
@@ -169,10 +198,51 @@ impl StateTransitionShieldedMinimumFeeValidationV0 for StateTransition {
                             )),
                         )
                         .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 requires_exact_fee && (fee 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 + {} actions × {} per-action); \
+                                 overpayment is not allowed",
+                                fee,
+                                minimum_shielded_fee,
+                                constants.shielded_proof_verification_fee,
+                                num_actions,
+                                per_action_fee,
+                            )),
+                        )
+                        .into(),
+                    ));
+                }
+
+                // For ShieldedWithdrawal, the net value leaving to Core
+                // (`unshielding_amount - min_fee`) must also clear the dust floor; for the
+                // other shielded paths `min_net_amount` is 0 and this check is a no-op.
+                // Safe: `fee as u64 >= minimum_shielded_fee` was just checked above.
+                let net_amount = (fee 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,
+                                platform_version.system_limits.max_withdrawal_amount,
+                            ),
+                        )
+                        .into(),
+                    ));
                 }
+
+                Ok(SimpleConsensusValidationResult::new())
             }
             version => Err(Error::Execution(ExecutionError::UnknownVersionMismatch {
                 method: "StateTransition::validate_minimum_shielded_fee".to_string(),
@@ -452,6 +522,22 @@ mod tests {
     mod validate_minimum_shielded_fee {
         use super::*;
 
+        /// 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() {
             let platform_version = &platform_version::version::v9::PLATFORM_V9;
@@ -461,6 +547,36 @@ 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);
+            // net = MIN_WITHDRAWAL_AMOUNT - 1 → just below the Core dust floor.
+            let st = shielded_withdrawal_with_amount(min_fee + MIN_WITHDRAWAL_AMOUNT - 1);
+            let result = st
+                .validate_minimum_shielded_fee(platform_version)
+                .expect("should not error");
+            assert!(
+                !result.is_valid(),
+                "a withdrawal whose net is below MIN_WITHDRAWAL_AMOUNT must be rejected at validation"
+            );
+        }
+
+        #[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);
+            // net = MIN_WITHDRAWAL_AMOUNT exactly → at the floor, accepted.
+            let st = shielded_withdrawal_with_amount(min_fee + MIN_WITHDRAWAL_AMOUNT);
+            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"
+            );
+        }
     }
 
     mod validate_shielded_proof {