From d835589a302752c9d355aeb9621e4ccca29f44c2 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 24 Feb 2026 20:03:11 +0100 Subject: [PATCH 1/8] fix(wallet): consolidate Core tx fee estimation and fix hardcoded 1000 duffs fee The RPC wallet payment path used a hardcoded DEFAULT_TX_FEE of 1000 duffs, which was rejected by Dash Core when the transaction size required a higher minimum relay fee (e.g. "min relay fee not met, 1000 < 1405"). Changes: - Add Core (L1) transaction fee utilities to fee_estimation.rs: estimate_p2pkh_tx_size, estimate_asset_lock_tx_size, calculate_relay_fee, DUST_THRESHOLD, MIN_ASSET_LOCK_FEE, AssetLockFeeResult, and calculate_asset_lock_fee - Fix send_wallet_payment_via_rpc to use size-based fee calculation with iterative estimation instead of hardcoded 1000 duffs - Replace 3 duplicate estimate_p2pkh_tx_size implementations (mod.rs, send_single_key_wallet_payment.rs, single_key_send_screen.rs) with the consolidated function - Move asset lock fee logic from asset_lock_transaction.rs to fee_estimation.rs - Add regression test: relay_fee_exceeds_1000_for_large_tx Co-Authored-By: Claude Opus 4.6 --- src/backend_task/core/mod.rs | 64 ++-- .../core/send_single_key_wallet_payment.rs | 27 +- src/model/fee_estimation.rs | 301 +++++++++++++++++- src/model/wallet/asset_lock_transaction.rs | 255 +-------------- src/ui/wallets/single_key_send_screen.rs | 33 +- 5 files changed, 371 insertions(+), 309 deletions(-) diff --git a/src/backend_task/core/mod.rs b/src/backend_task/core/mod.rs index 84b762fb6..52c47e283 100644 --- a/src/backend_task/core/mod.rs +++ b/src/backend_task/core/mod.rs @@ -372,22 +372,55 @@ impl AppContext { wallet: Arc>, request: WalletPaymentRequest, ) -> Result { + use crate::model::fee_estimation::{calculate_relay_fee, estimate_p2pkh_tx_size}; + let parsed_recipients = self.parse_recipients(&request)?; - const DEFAULT_TX_FEE: u64 = 1_000; + // Iterative fee estimation: estimate fee based on assumed input count, + // build the transaction, then verify the fee covers the actual tx size. + // If the caller supplied an override_fee (e.g. retry after a min-relay- + // fee rejection), use that directly and skip the iteration. + let num_recipient_outputs = parsed_recipients.len() + 1; // +1 for change + let initial_fee = request.override_fee.unwrap_or_else(|| { + calculate_relay_fee(estimate_p2pkh_tx_size(5, num_recipient_outputs)) + }); let tx = { let mut wallet_guard = wallet.write().map_err(|e| e.to_string())?; if !wallet_guard.is_open() { return Err("Wallet must be unlocked".to_string()); } - wallet_guard.build_multi_recipient_payment_transaction( + + // First attempt with estimated fee. + let mut tx = wallet_guard.build_multi_recipient_payment_transaction( self.network, &parsed_recipients, - DEFAULT_TX_FEE, + initial_fee, request.subtract_fee_from_amount, Some(self), - )? + )?; + + // If override_fee was supplied, trust it — skip recalculation. + if request.override_fee.is_none() { + let actual_fee = + calculate_relay_fee(estimate_p2pkh_tx_size(tx.input.len(), tx.output.len())); + if actual_fee > initial_fee { + // The real fee is higher; rebuild with the correct fee. + // The first build already removed UTXOs, so reload the wallet + // state and rebuild. In practice this rarely triggers because + // the initial 5-input estimate is usually sufficient. + wallet_guard.reload_utxos(self)?; + tx = wallet_guard.build_multi_recipient_payment_transaction( + self.network, + &parsed_recipients, + actual_fee, + request.subtract_fee_from_amount, + Some(self), + )?; + } + } + + tx }; let txid = self @@ -613,8 +646,9 @@ impl AppContext { return Err("No spendable funds available".to_string()); } - let estimated_size = Self::estimate_p2pkh_tx_size(spendable_inputs, 1); - let fee = FeeLevel::Normal.fee_rate().calculate_fee(estimated_size); + let estimated_size = + crate::model::fee_estimation::estimate_p2pkh_tx_size(spendable_inputs, 1); + let fee = crate::model::fee_estimation::calculate_relay_fee(estimated_size); Ok(spendable_total.saturating_sub(fee)) } @@ -699,22 +733,4 @@ impl AppContext { } if total == 0 { None } else { Some(total) } } - - fn estimate_p2pkh_tx_size(inputs: usize, outputs: usize) -> usize { - fn varint_size(value: usize) -> usize { - match value { - 0..=0xfc => 1, - 0xfd..=0xffff => 3, - 0x1_0000..=0xffff_ffff => 5, - _ => 9, - } - } - - let mut size = 8; // version/type/lock_time - size += varint_size(inputs); - size += varint_size(outputs); - size += inputs * 148; - size += outputs * 34; - size - } } diff --git a/src/backend_task/core/send_single_key_wallet_payment.rs b/src/backend_task/core/send_single_key_wallet_payment.rs index 490438819..801ecc1c5 100644 --- a/src/backend_task/core/send_single_key_wallet_payment.rs +++ b/src/backend_task/core/send_single_key_wallet_payment.rs @@ -3,13 +3,13 @@ use crate::backend_task::BackendTaskSuccessResult; use crate::backend_task::core::WalletPaymentRequest; use crate::context::AppContext; +use crate::model::fee_estimation::{DUST_THRESHOLD, calculate_relay_fee, estimate_p2pkh_tx_size}; use crate::model::wallet::single_key::SingleKeyWallet; use dash_sdk::dashcore_rpc::RpcApi; use dash_sdk::dashcore_rpc::dashcore::{Address, OutPoint, ScriptBuf, Transaction, TxIn, TxOut}; use dash_sdk::dpp::dashcore::hashes::Hash; use dash_sdk::dpp::dashcore::sighash::SighashCache; use dash_sdk::dpp::dashcore::{EcdsaSighashType, secp256k1::Secp256k1}; -use dash_sdk::dpp::key_wallet::wallet::managed_wallet_info::fee::FeeLevel; use std::str::FromStr; use std::sync::{Arc, RwLock}; @@ -62,12 +62,10 @@ impl AppContext { // Select UTXOs to cover the amount + estimated fee // Start with an estimate assuming ~10 inputs, then refine let num_outputs = outputs.len() + 1; // +1 for change - let initial_fee_estimate = Self::estimate_p2pkh_tx_size(10, num_outputs); - let initial_fee = request.override_fee.unwrap_or_else(|| { - FeeLevel::Normal - .fee_rate() - .calculate_fee(initial_fee_estimate) - }); + let initial_fee_estimate = estimate_p2pkh_tx_size(10, num_outputs); + let initial_fee = request + .override_fee + .unwrap_or_else(|| calculate_relay_fee(initial_fee_estimate)); let _target_amount = total_output + initial_fee; @@ -88,10 +86,10 @@ impl AppContext { selected_total += tx_out.value; // Recalculate fee with current input count - let current_size = Self::estimate_p2pkh_tx_size(selected.len(), num_outputs); + let current_size = estimate_p2pkh_tx_size(selected.len(), num_outputs); let current_fee = request .override_fee - .unwrap_or_else(|| FeeLevel::Normal.fee_rate().calculate_fee(current_size)); + .unwrap_or_else(|| calculate_relay_fee(current_size)); if selected_total >= total_output + current_fee { break; @@ -99,10 +97,10 @@ impl AppContext { } // Final check if we have enough - let final_size = Self::estimate_p2pkh_tx_size(selected.len(), num_outputs); + let final_size = estimate_p2pkh_tx_size(selected.len(), num_outputs); let final_fee = request .override_fee - .unwrap_or_else(|| FeeLevel::Normal.fee_rate().calculate_fee(final_size)); + .unwrap_or_else(|| calculate_relay_fee(final_size)); if selected_total < total_output + final_fee { return Err(format!( @@ -120,11 +118,10 @@ impl AppContext { // Calculate final fee with selected UTXOs let num_outputs_with_change = outputs.len() + 1; - let estimated_size = - Self::estimate_p2pkh_tx_size(selected_utxos.len(), num_outputs_with_change); + let estimated_size = estimate_p2pkh_tx_size(selected_utxos.len(), num_outputs_with_change); let fee = request .override_fee - .unwrap_or_else(|| FeeLevel::Normal.fee_rate().calculate_fee(estimated_size)); + .unwrap_or_else(|| calculate_relay_fee(estimated_size)); let total_input: u64 = selected_utxos.iter().map(|(_, tx_out)| tx_out.value).sum(); @@ -144,7 +141,7 @@ impl AppContext { }; // Add change output if significant (above dust threshold) - if change_amount > 546 { + if change_amount > DUST_THRESHOLD { outputs.push(TxOut { value: change_amount, script_pubkey: change_address.script_pubkey(), diff --git a/src/model/fee_estimation.rs b/src/model/fee_estimation.rs index 07bf082af..94fdcdea1 100644 --- a/src/model/fee_estimation.rs +++ b/src/model/fee_estimation.rs @@ -1,7 +1,16 @@ -//! Fee estimation utilities for Dash Platform state transitions. +//! Fee estimation utilities for Dash transactions. //! -//! This module provides fee estimation for various state transition types, -//! using the fee structure from the platform version. +//! ## Core (L1) transaction fees +//! +//! Size-based fee estimation for P2PKH and asset-lock transactions broadcast to +//! the Dash Core network. Provides [`estimate_p2pkh_tx_size`], +//! [`estimate_asset_lock_tx_size`], [`calculate_relay_fee`], and the iterative +//! [`calculate_asset_lock_fee`] helper used by the wallet. +//! +//! ## Platform (L2) state-transition fees +//! +//! Fee estimation for various state transition types using the fee structure +//! from the platform version. //! //! Fee calculation is based on: //! - Storage fees: Bytes stored × storage_disk_usage_credit_per_byte (27,000) @@ -12,8 +21,146 @@ //! performed by Platform. For accurate fees, use Platform's EstimateStateTransitionFee //! endpoint (when available). +use dash_sdk::dpp::key_wallet::wallet::managed_wallet_info::fee::FeeLevel; use dash_sdk::dpp::version::PlatformVersion; +// ── Core (L1) transaction fee estimation ──────────────────────────────────── + +/// Dust threshold for P2PKH outputs (duffs). Outputs below this are +/// rejected by the network as non-standard. +pub const DUST_THRESHOLD: u64 = 546; + +/// Minimum fee for asset lock transactions (duffs). +pub const MIN_ASSET_LOCK_FEE: u64 = 3_000; + +/// Size of the asset-lock special transaction payload (bytes). +const ASSET_LOCK_PAYLOAD_SIZE: usize = 60; + +/// Estimate varint encoding size. +fn varint_size(value: usize) -> usize { + match value { + 0..=0xfc => 1, + 0xfd..=0xffff => 3, + 0x1_0000..=0xffff_ffff => 5, + _ => 9, + } +} + +/// Estimate P2PKH transaction size in bytes. +/// +/// Layout: 8-byte header (version + lock_time) + varint input/output counts +/// + 148 bytes per P2PKH input + 34 bytes per output. +pub fn estimate_p2pkh_tx_size(inputs: usize, outputs: usize) -> usize { + 8 + varint_size(inputs) + varint_size(outputs) + inputs * 148 + outputs * 34 +} + +/// Estimate asset-lock transaction size (P2PKH inputs + special payload). +pub fn estimate_asset_lock_tx_size(inputs: usize, outputs: usize) -> usize { + // Asset-lock txs use a 10-byte header variant (version/type/lock_time) + // instead of the standard 8-byte P2PKH header. + 10 + varint_size(inputs) + + varint_size(outputs) + + inputs * 148 + + outputs * 34 + + ASSET_LOCK_PAYLOAD_SIZE +} + +/// Calculate the relay fee for a given transaction size using the SDK's +/// normal fee rate. +pub fn calculate_relay_fee(tx_size: usize) -> u64 { + FeeLevel::Normal.fee_rate().calculate_fee(tx_size) +} + +/// Result of asset lock fee calculation. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AssetLockFeeResult { + /// Transaction fee in duffs. + pub fee: u64, + /// Actual amount for the asset lock output (may differ from requested if + /// `allow_take_fee_from_amount` is true). + pub actual_amount: u64, + /// Change to return, or `None` if no change output is needed. + pub change: Option, +} + +/// Calculate fee, actual amount, and change for an asset lock transaction. +/// +/// Uses an iterative approach: starts assuming a change output exists, +/// then recomputes if the change disappears under the real fee. +/// +/// # Errors +/// +/// Returns an error string if there are insufficient funds. +pub fn calculate_asset_lock_fee( + total_input_value: u64, + requested_amount: u64, + num_inputs: usize, + allow_take_fee_from_amount: bool, +) -> Result { + // First pass: assume 2 outputs (1 burn + 1 change). + let fee_with_change = std::cmp::max( + MIN_ASSET_LOCK_FEE, + estimate_asset_lock_tx_size(num_inputs, 2) as u64, + ); + + let required_with_change = requested_amount + .checked_add(fee_with_change) + .ok_or("Overflow computing required amount + fee")?; + let tentative_change = total_input_value.checked_sub(required_with_change); + + // If change exceeds dust threshold, include it as an output. + if let Some(change) = tentative_change + && change >= DUST_THRESHOLD + { + return Ok(AssetLockFeeResult { + fee: fee_with_change, + actual_amount: requested_amount, + change: Some(change), + }); + } + + // Change is zero or negative under the 2-output fee. + // Recompute with 1 output (no change). + let fee_no_change = std::cmp::max( + MIN_ASSET_LOCK_FEE, + estimate_asset_lock_tx_size(num_inputs, 1) as u64, + ); + + let required_no_change = requested_amount + .checked_add(fee_no_change) + .ok_or("Overflow computing required amount + fee")?; + + if total_input_value >= required_no_change { + // Enough funds without a change output. Any leftover becomes + // additional fee (it is too small for a viable change output). + return Ok(AssetLockFeeResult { + fee: total_input_value - requested_amount, + actual_amount: requested_amount, + change: None, + }); + } + + // Insufficient for the requested amount at the 1-output fee rate. + if allow_take_fee_from_amount { + let adjusted = total_input_value.saturating_sub(fee_no_change); + if adjusted == 0 { + return Err("Insufficient funds for transaction fee".to_string()); + } + Ok(AssetLockFeeResult { + fee: fee_no_change, + actual_amount: adjusted, + change: None, + }) + } else { + Err(format!( + "Insufficient funds: need {} + {} fee, have {}", + requested_amount, fee_no_change, total_input_value + )) + } +} + +// ── Platform (L2) state-transition fee estimation ─────────────────────────── + /// Storage fee constants from FEE_STORAGE_VERSION1 in rs-platform-version. /// These determine the cost of storing and processing data on Platform. #[derive(Debug, Clone, Copy)] @@ -655,6 +802,154 @@ pub fn format_credits(credits: u64) -> String { mod tests { use super::*; + // ── Core (L1) fee tests ───────────────────────────────────────────── + + #[test] + fn p2pkh_tx_size_single_input() { + // 1 input, 2 outputs: 8 + 1 + 1 + 148 + 68 = 226 + assert_eq!(estimate_p2pkh_tx_size(1, 2), 226); + } + + #[test] + fn p2pkh_tx_size_many_inputs() { + // 10 inputs, 2 outputs: 8 + 1 + 1 + 1480 + 68 = 1558 + assert_eq!(estimate_p2pkh_tx_size(10, 2), 1558); + } + + #[test] + fn asset_lock_tx_size_single_input() { + // 1 input, 2 outputs: 10 + 1 + 1 + 148 + 68 + 60 = 288 + assert_eq!(estimate_asset_lock_tx_size(1, 2), 288); + } + + #[test] + fn relay_fee_exceeds_1000_for_large_tx() { + // Regression: the bug was a hardcoded 1000-duffs fee for any tx size. + // 10 inputs × 2 outputs → ~1558 bytes → relay fee must exceed 1000. + let size = estimate_p2pkh_tx_size(10, 2); + let fee = calculate_relay_fee(size); + assert!( + fee > 1_000, + "relay fee for 10-input tx should exceed 1000 duffs, got {fee}" + ); + } + + #[test] + fn asset_lock_fee_single_input_minimum() { + // 1 input, amount=10000, total=50000. + // With 2 outputs: size = 288 -> fee = max(3000, 288) = 3000. + // Change = 50000 - 10000 - 3000 = 37000. + let result = calculate_asset_lock_fee(50_000, 10_000, 1, false).expect("should succeed"); + assert_eq!(result.fee, 3_000); + assert_eq!(result.actual_amount, 10_000); + assert_eq!(result.change, Some(37_000)); + } + + #[test] + fn asset_lock_fee_scales_with_inputs() { + // 21 inputs, amount=50000, total=200000. + // With 2 outputs: size = 10 + varint(21)+varint(2) + 21*148 + 2*34 + 60 = 3248. + // fee = max(3000, 3248) = 3248. + let result = calculate_asset_lock_fee(200_000, 50_000, 21, false).expect("should succeed"); + assert!( + result.fee > MIN_ASSET_LOCK_FEE, + "fee should exceed minimum for many inputs" + ); + assert_eq!(result.fee, 3_248); + assert_eq!(result.actual_amount, 50_000); + assert_eq!(result.change, Some(146_752)); + } + + #[test] + fn asset_lock_fee_exact_no_change() { + // 1 input, amount=47000, total=50000. + // With 2 outputs: fee = 3000. change = 50000 - 47000 - 3000 = 0 -> None. + // Re-check with 1 output: fee = 3000 (minimum). + // 50000 >= 47000 + 3000 -> actual fee = 50000 - 47000 = 3000. + let result = calculate_asset_lock_fee(50_000, 47_000, 1, false).expect("should succeed"); + assert_eq!(result.actual_amount, 47_000); + assert_eq!(result.change, None); + } + + #[test] + fn asset_lock_fee_take_from_amount() { + // 1 input, amount=50000, total=50000, allow_take_fee=true. + // Take from amount: actual = 50000 - 3000 = 47000. + let result = calculate_asset_lock_fee(50_000, 50_000, 1, true).expect("should succeed"); + assert_eq!(result.actual_amount, 47_000); + assert_eq!(result.change, None); + } + + #[test] + fn asset_lock_fee_insufficient_funds() { + let result = calculate_asset_lock_fee(50_000, 50_000, 1, false); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("Insufficient funds")); + } + + #[test] + fn asset_lock_fee_insufficient_for_fee_alone() { + // Fee = 3000 > total 2000 -> error. + let result = calculate_asset_lock_fee(2_000, 2_000, 1, true); + assert!(result.is_err()); + assert!( + result.unwrap_err().contains("Insufficient funds"), + "should error when total cannot even cover the fee" + ); + } + + #[test] + fn asset_lock_fee_change_eliminated_by_real_fee_should_succeed() { + // 21 inputs, amount=50000, total=53220. + // Correct code recalculates with 1 output when change disappears. + let result = calculate_asset_lock_fee(53_220, 50_000, 21, false); + assert!( + result.is_ok(), + "should NOT fail with insufficient funds; got: {:?}", + result + ); + let result = result.unwrap(); + assert_eq!(result.actual_amount, 50_000); + assert_eq!(result.change, None); + assert_eq!(result.fee, 3_220); + } + + #[test] + fn asset_lock_fee_overestimate_when_change_disappears() { + // 21 inputs, amount=50000, total=53240. + // 1-output path succeeds; fee = 3240, no change. + let result = calculate_asset_lock_fee(53_240, 50_000, 21, false); + assert!( + result.is_ok(), + "should succeed when recalculated without change output; got: {:?}", + result + ); + let result = result.unwrap(); + assert_eq!(result.actual_amount, 50_000); + assert_eq!(result.change, None); + assert!( + result.fee < 3_246, + "fee should be less than the 2-output estimate of 3246, got {}", + result.fee + ); + assert_eq!(result.fee, 3_240); + } + + #[test] + fn asset_lock_fee_dust_change_absorbed_into_fee() { + // 1 input, amount=46500, total=50000. + // change = 500 < DUST_THRESHOLD → absorbed into fee. + let result = calculate_asset_lock_fee(50_000, 46_500, 1, false).expect("should succeed"); + assert_eq!(result.actual_amount, 46_500); + assert_eq!( + result.change, None, + "dust change should not create an output" + ); + assert_eq!(result.fee, 3_500, "dust should be absorbed into fee"); + } + + // ── Platform (L2) fee tests ───────────────────────────────────────── + #[test] fn test_credit_transfer_estimate() { let estimator = PlatformFeeEstimator::new(); diff --git a/src/model/wallet/asset_lock_transaction.rs b/src/model/wallet/asset_lock_transaction.rs index 2ad56b61f..a51dec020 100644 --- a/src/model/wallet/asset_lock_transaction.rs +++ b/src/model/wallet/asset_lock_transaction.rs @@ -1,4 +1,7 @@ use crate::context::AppContext; +use crate::model::fee_estimation::{ + AssetLockFeeResult, MIN_ASSET_LOCK_FEE, calculate_asset_lock_fee, estimate_asset_lock_tx_size, +}; use crate::model::wallet::Wallet; use dash_sdk::dashcore_rpc::dashcore::key::Secp256k1; use dash_sdk::dpp::dashcore::secp256k1::Message; @@ -11,104 +14,6 @@ use dash_sdk::dpp::dashcore::{ use dash_sdk::dpp::key_wallet::psbt::serialize::Serialize; use std::collections::BTreeMap; -/// Result of asset lock fee calculation. -#[derive(Debug, Clone, PartialEq, Eq)] -pub(crate) struct AssetLockFeeResult { - /// Transaction fee in duffs. - pub fee: u64, - /// Actual amount for the asset lock output (may differ from requested if - /// `allow_take_fee_from_amount` is true). - pub actual_amount: u64, - /// Change to return, or `None` if no change output is needed. - pub change: Option, -} - -/// Minimum fee for an asset lock transaction (duffs). -const MIN_ASSET_LOCK_FEE: u64 = 3_000; - -/// Minimum value for a change output (duffs). Outputs below this -/// threshold are considered dust and will be rejected by the network. -/// For P2PKH outputs the Dash Core dust limit is 546 duffs. -const DUST_THRESHOLD: u64 = 546; - -/// Estimate the transaction size in bytes. -/// -/// Assumes P2PKH inputs (~148 B each), standard outputs (~34 B each), -/// a ~10 B header, and a ~60 B asset-lock payload. -fn estimate_tx_size(num_inputs: usize, num_outputs: usize) -> usize { - 10 + (num_inputs * 148) + (num_outputs * 34) + 60 -} - -/// Calculate fee, actual amount, and change for an asset lock transaction. -/// -/// Uses an iterative approach: starts assuming a change output exists, -/// then recomputes if the change disappears under the real fee. -/// -/// # Errors -/// -/// Returns an error string if there are insufficient funds. -pub(crate) fn calculate_asset_lock_fee( - total_input_value: u64, - requested_amount: u64, - num_inputs: usize, - allow_take_fee_from_amount: bool, -) -> Result { - // First pass: assume 2 outputs (1 burn + 1 change). - let fee_with_change = std::cmp::max(MIN_ASSET_LOCK_FEE, estimate_tx_size(num_inputs, 2) as u64); - - let required_with_change = requested_amount - .checked_add(fee_with_change) - .ok_or("Overflow computing required amount + fee")?; - let tentative_change = total_input_value.checked_sub(required_with_change); - - // If change exceeds dust threshold, include it as an output. - if let Some(change) = tentative_change - && change >= DUST_THRESHOLD - { - return Ok(AssetLockFeeResult { - fee: fee_with_change, - actual_amount: requested_amount, - change: Some(change), - }); - } - - // Change is zero or negative under the 2-output fee. - // Recompute with 1 output (no change). - let fee_no_change = std::cmp::max(MIN_ASSET_LOCK_FEE, estimate_tx_size(num_inputs, 1) as u64); - - let required_no_change = requested_amount - .checked_add(fee_no_change) - .ok_or("Overflow computing required amount + fee")?; - - if total_input_value >= required_no_change { - // Enough funds without a change output. Any leftover becomes - // additional fee (it is too small for a viable change output). - return Ok(AssetLockFeeResult { - fee: total_input_value - requested_amount, - actual_amount: requested_amount, - change: None, - }); - } - - // Insufficient for the requested amount at the 1-output fee rate. - if allow_take_fee_from_amount { - let adjusted = total_input_value.saturating_sub(fee_no_change); - if adjusted == 0 { - return Err("Insufficient funds for transaction fee".to_string()); - } - Ok(AssetLockFeeResult { - fee: fee_no_change, - actual_amount: adjusted, - change: None, - }) - } else { - Err(format!( - "Insufficient funds: need {} + {} fee, have {}", - requested_amount, fee_no_change, total_input_value - )) - } -} - impl Wallet { /// Select UTXOs and compute fee, retrying with the real fee if the initial /// estimate was too low and additional UTXOs are available. @@ -145,8 +50,10 @@ impl Wallet { // The real fee may exceed our initial estimate. Recompute // with a 2-output size estimate and retry UTXO selection so // we can pick up any additional marginal UTXOs. - fee_estimate = - std::cmp::max(MIN_ASSET_LOCK_FEE, estimate_tx_size(num_inputs, 2) as u64); + fee_estimate = std::cmp::max( + MIN_ASSET_LOCK_FEE, + estimate_asset_lock_tx_size(num_inputs, 2) as u64, + ); continue; } Err(e) => return Err(e), @@ -500,8 +407,9 @@ impl Wallet { let one_time_key_hash = asset_lock_public_key.pubkey_hash(); - // Single-UTXO path: use calculate_asset_lock_fee for consistency with - // the multi-input path, ensuring dust check and overflow safety. + // Single-UTXO path: use calculate_asset_lock_fee (from fee_estimation) + // for consistency with the multi-input path, ensuring dust check and + // overflow safety. let fee_result = calculate_asset_lock_fee( previous_tx_output.value, previous_tx_output.value.saturating_sub(MIN_ASSET_LOCK_FEE), @@ -598,144 +506,5 @@ impl Wallet { Ok((tx, private_key)) } } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn fee_single_input_minimum() { - // 1 input, amount=10000, total=50000. - // With 2 outputs: size = 10 + 148 + 2*34 + 60 = 286 -> fee = max(3000, 286) = 3000. - // Change = 50000 - 10000 - 3000 = 37000. - let result = calculate_asset_lock_fee(50_000, 10_000, 1, false).expect("should succeed"); - assert_eq!(result.fee, 3_000); - assert_eq!(result.actual_amount, 10_000); - assert_eq!(result.change, Some(37_000)); - } - - #[test] - fn fee_scales_with_inputs() { - // 21 inputs, amount=50000, total=200000. - // With 2 outputs: size = 10 + 21*148 + 2*34 + 60 = 3246, fee = max(3000, 3246) = 3246. - // Change = 200000 - 50000 - 3246 = 146754. - let result = calculate_asset_lock_fee(200_000, 50_000, 21, false).expect("should succeed"); - assert!( - result.fee > MIN_ASSET_LOCK_FEE, - "fee should exceed minimum for many inputs" - ); - assert_eq!(result.fee, 3_246); - assert_eq!(result.actual_amount, 50_000); - assert_eq!(result.change, Some(146_754)); - } - - #[test] - fn fee_exact_no_change() { - // 1 input, amount=47000, total=50000. - // With 2 outputs: size = 286, fee = 3000. change = 50000 - 47000 - 3000 = 0 -> None. - // Re-check with 1 output: size = 10 + 148 + 34 + 60 = 252, fee = 3000. - // 50000 >= 47000 + 3000 -> actual fee = 50000 - 47000 = 3000. - let result = calculate_asset_lock_fee(50_000, 47_000, 1, false).expect("should succeed"); - assert_eq!(result.actual_amount, 47_000); - assert_eq!(result.change, None); - } - - #[test] - fn fee_take_from_amount() { - // 1 input, amount=50000, total=50000, allow_take_fee=true. - // With 2 outputs: fee = 3000. change = 50000 - 50000 - 3000 < 0. - // With 1 output: fee = 3000. 50000 < 50000 + 3000. - // Take from amount: actual = 50000 - 3000 = 47000. - let result = calculate_asset_lock_fee(50_000, 50_000, 1, true).expect("should succeed"); - assert_eq!(result.actual_amount, 47_000); - assert_eq!(result.change, None); - } - - #[test] - fn fee_insufficient_funds() { - // 1 input, amount=50000, total=50000, allow_take_fee=false. - let result = calculate_asset_lock_fee(50_000, 50_000, 1, false); - assert!(result.is_err()); - assert!(result.unwrap_err().contains("Insufficient funds")); - } - - #[test] - fn fee_insufficient_for_fee_alone() { - // 1 input, amount=2000, total=2000, allow_take_fee=true. - // Fee = 3000 > total -> adjusted = 2000 - 3000 = 0 (saturating) -> error. - let result = calculate_asset_lock_fee(2_000, 2_000, 1, true); - assert!(result.is_err()); - assert!( - result.unwrap_err().contains("Insufficient funds"), - "should error when total cannot even cover the fee" - ); - } - - #[test] - fn fee_change_eliminated_by_real_fee_should_succeed() { - // BUG TEST: 21 inputs, amount=50000, total=53220. - // - // Initial UTXO selection uses fee estimate of 3000: - // change = 53220 - 50000 - 3000 = 220 -> has_initial_change = true - // - // Buggy code uses 2 outputs: fee = 10 + 21*148 + 2*34 + 60 = 3246 - // 53220 < 50000 + 3246 = 53246 -> ERROR (false insufficient funds) - // - // Correct code recalculates with 1 output: fee = 10 + 21*148 + 1*34 + 60 = 3212 - // 53220 >= 50000 + 3212 -> succeeds, no change, fee = 3220 - let result = calculate_asset_lock_fee(53_220, 50_000, 21, false); - assert!( - result.is_ok(), - "should NOT fail with insufficient funds; got: {:?}", - result - ); - let result = result.unwrap(); - assert_eq!(result.actual_amount, 50_000); - assert_eq!(result.change, None); - // Fee absorbs the leftover: 53220 - 50000 = 3220 - assert_eq!(result.fee, 3_220); - } - - #[test] - fn fee_overestimate_when_change_disappears() { - // BUG TEST: Verify that when initial change exists under 3000 estimate but - // disappears under the real fee, we do not overcharge by counting the - // phantom change output. - // - // 21 inputs, amount=50000, total=53240. - // 2-output fee = 3246 -> 53240 < 53246, would fail with buggy code. - // 1-output fee = 3212 -> 53240 >= 53212, succeeds. fee = 3240, no change. - let result = calculate_asset_lock_fee(53_240, 50_000, 21, false); - assert!( - result.is_ok(), - "should succeed when recalculated without change output; got: {:?}", - result - ); - let result = result.unwrap(); - assert_eq!(result.actual_amount, 50_000); - assert_eq!(result.change, None); - // The fee with 2 outputs (3246) would be wrong here; only 1 output is needed. - // Actual fee = leftover = 53240 - 50000 = 3240, which is >= 1-output minimum (3212). - assert!( - result.fee < 3_246, - "fee should be less than the 2-output estimate of 3246, got {}", - result.fee - ); - assert_eq!(result.fee, 3_240); - } - #[test] - fn fee_dust_change_absorbed_into_fee() { - // 1 input, amount=46500, total=50000. - // With 2 outputs: fee = 3000. change = 50000 - 46500 - 3000 = 500. - // 500 < DUST_THRESHOLD (546) → change is dust, absorbed into fee. - // Falls through to 1-output path: fee = 3000. - // 50000 >= 46500 + 3000 → fee = 50000 - 46500 = 3500 (absorbs dust). - let result = calculate_asset_lock_fee(50_000, 46_500, 1, false).expect("should succeed"); - assert_eq!(result.actual_amount, 46_500); - assert_eq!( - result.change, None, - "dust change should not create an output" - ); - assert_eq!(result.fee, 3_500, "dust should be absorbed into fee"); - } -} +// Tests for calculate_asset_lock_fee and related functions have been +// consolidated into src/model/fee_estimation.rs. diff --git a/src/ui/wallets/single_key_send_screen.rs b/src/ui/wallets/single_key_send_screen.rs index 9528d9020..f451e8d5d 100644 --- a/src/ui/wallets/single_key_send_screen.rs +++ b/src/ui/wallets/single_key_send_screen.rs @@ -5,6 +5,7 @@ use crate::backend_task::BackendTask; use crate::backend_task::core::{CoreTask, PaymentRecipient, WalletPaymentRequest}; use crate::context::AppContext; use crate::model::amount::{Amount, DASH_DECIMAL_PLACES}; +use crate::model::fee_estimation::{calculate_relay_fee, estimate_p2pkh_tx_size}; use crate::model::wallet::single_key::SingleKeyWallet; use crate::ui::components::left_panel::add_left_panel; use crate::ui::components::styled::island_central_panel; @@ -12,7 +13,6 @@ use crate::ui::components::top_panel::add_top_panel; use crate::ui::theme::DashColors; use crate::ui::{MessageType, RootScreenType, ScreenLike}; use chrono::{DateTime, Utc}; -use dash_sdk::dpp::key_wallet::wallet::managed_wallet_info::fee::FeeLevel; use eframe::egui::{self, Context, RichText, Ui}; use egui::{Color32, Frame, Margin}; use std::sync::{Arc, RwLock}; @@ -114,23 +114,8 @@ impl SingleKeyWalletSendScreen { amount.dash_to_duffs() } - /// Estimate transaction size for P2PKH transactions - fn estimate_p2pkh_tx_size(inputs: usize, outputs: usize) -> usize { - fn varint_size(value: usize) -> usize { - match value { - 0..=0xfc => 1, - 0xfd..=0xffff => 3, - 0x1_0000..=0xffff_ffff => 5, - _ => 9, - } - } - let mut size = 8; // version/type/lock_time - size += varint_size(inputs); - size += varint_size(outputs); - size += inputs * 148; // P2PKH input size - size += outputs * 34; // P2PKH output size - size - } + // Transaction size estimation is provided by + // crate::model::fee_estimation::estimate_p2pkh_tx_size. /// Calculate estimated fee based on UTXO selection for the send amount fn estimate_fee(&self) -> Option<(u64, usize, usize)> { @@ -151,8 +136,8 @@ impl SingleKeyWalletSendScreen { if total_output == 0 { // No valid amounts entered yet, show estimate for minimum tx let output_count = self.recipients.len().max(1) + 1; - let estimated_size = Self::estimate_p2pkh_tx_size(1, output_count); - let fee = FeeLevel::Normal.fee_rate().calculate_fee(estimated_size); + let estimated_size = estimate_p2pkh_tx_size(1, output_count); + let fee = calculate_relay_fee(estimated_size); return Some((fee, 1, estimated_size)); } @@ -171,8 +156,8 @@ impl SingleKeyWalletSendScreen { selected_total += value; // Recalculate fee with current input count - let current_size = Self::estimate_p2pkh_tx_size(selected_count, output_count); - let current_fee = FeeLevel::Normal.fee_rate().calculate_fee(current_size); + let current_size = estimate_p2pkh_tx_size(selected_count, output_count); + let current_fee = calculate_relay_fee(current_size); if selected_total >= total_output + current_fee { return Some((current_fee, selected_count, current_size)); @@ -180,8 +165,8 @@ impl SingleKeyWalletSendScreen { } // Not enough funds - show what we'd need with all UTXOs - let estimated_size = Self::estimate_p2pkh_tx_size(selected_count, output_count); - let fee = FeeLevel::Normal.fee_rate().calculate_fee(estimated_size); + let estimated_size = estimate_p2pkh_tx_size(selected_count, output_count); + let fee = calculate_relay_fee(estimated_size); Some((fee, selected_count, estimated_size)) } From 7ba0a70e533c1166ebcda631f9ac1ac3e402083f Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 24 Feb 2026 20:38:34 +0100 Subject: [PATCH 2/8] feat(fees): consolidate Platform fee estimation with transition-based max() Add unified fee methods to PlatformFeeEstimator that compute max(legacy, transition-based) internally, so callers don't need to know about the dual estimation strategy. Consolidates changes from PR #521 and PR #514 into centralized estimator methods. New public methods: - estimate_platform_to_core_fee() - estimate_platform_to_platform_fee() - estimate_core_to_platform_fee() - estimate_identity_create_from_addresses_fee() - estimate_identity_topup_from_addresses_fee() Reduce public API surface: - Make internal helpers private (calculate_processing_fee, calculate_seek_fee, calculate_storage_fee, etc.) - Remove dead code (estimate_masternode_vote, storage_fees(), estimate_address_credit_withdrawal, estimate_contract_create_detailed) Callers updated: send_screen, register_identity, identity/mod, add_new_identity_screen, top_up_identity_screen. Co-Authored-By: Claude Opus 4.6 --- src/backend_task/identity/mod.rs | 15 +- .../identity/register_identity.rs | 8 +- src/model/fee_estimation.rs | 358 ++++++++++++------ .../by_platform_address.rs | 25 +- .../by_platform_address.rs | 19 +- src/ui/wallets/send_screen.rs | 283 +++++++------- 6 files changed, 420 insertions(+), 288 deletions(-) diff --git a/src/backend_task/identity/mod.rs b/src/backend_task/identity/mod.rs index 86199807f..cf4d03f55 100644 --- a/src/backend_task/identity/mod.rs +++ b/src/backend_task/identity/mod.rs @@ -601,10 +601,23 @@ impl AppContext { wallet_seed_hash: WalletSeedHash, ) -> Result { use crate::model::fee_estimation::PlatformFeeEstimator; + use dash_sdk::dpp::prelude::AddressNonce; use dash_sdk::platform::transition::top_up_identity_from_addresses::TopUpIdentityFromAddresses; // Estimate fee for top-up from platform addresses - let estimated_fee = PlatformFeeEstimator::new().estimate_identity_topup(); + let inputs_with_nonce: std::collections::BTreeMap< + dash_sdk::dpp::address_funds::PlatformAddress, + (AddressNonce, Credits), + > = inputs + .iter() + .map(|(addr, credits)| (*addr, (0, *credits))) + .collect(); + let estimated_fee = PlatformFeeEstimator::new().estimate_identity_topup_from_addresses_fee( + sdk.version(), + &inputs_with_nonce, + None, + qualified_identity.identity.id(), + ); tracing::info!( "top_up_identity_from_platform_addresses: identity={}, inputs={:?}", diff --git a/src/backend_task/identity/register_identity.rs b/src/backend_task/identity/register_identity.rs index 8e9221802..63ec342ac 100644 --- a/src/backend_task/identity/register_identity.rs +++ b/src/backend_task/identity/register_identity.rs @@ -617,12 +617,8 @@ impl AppContext { // Calculate fee estimate for identity creation from platform addresses let key_count = public_keys.len(); - let input_count = inputs.len(); - let estimated_fee = PlatformFeeEstimator::new().estimate_identity_create_from_addresses( - input_count, - false, - key_count, - ); + let estimated_fee = PlatformFeeEstimator::new() + .estimate_identity_create_from_addresses_fee(sdk.version(), &inputs, None, key_count); // Clone the wallet for use as the address signer (needed across async boundary) let wallet_clone = { wallet.read().map_err(|e| e.to_string())?.clone() }; diff --git a/src/model/fee_estimation.rs b/src/model/fee_estimation.rs index 94fdcdea1..eb6c4763b 100644 --- a/src/model/fee_estimation.rs +++ b/src/model/fee_estimation.rs @@ -21,8 +21,26 @@ //! performed by Platform. For accurate fees, use Platform's EstimateStateTransitionFee //! endpoint (when available). +use dash_sdk::dpp::address_funds::{AddressFundsFeeStrategyStep, PlatformAddress}; +use dash_sdk::dpp::balances::credits::Credits; +use dash_sdk::dpp::identity::core_script::CoreScript; use dash_sdk::dpp::key_wallet::wallet::managed_wallet_info::fee::FeeLevel; +use dash_sdk::dpp::prelude::{AddressNonce, AssetLockProof, Identifier}; +use dash_sdk::dpp::state_transition::StateTransitionEstimatedFeeValidation; +use dash_sdk::dpp::state_transition::address_credit_withdrawal_transition::AddressCreditWithdrawalTransition; +use dash_sdk::dpp::state_transition::address_credit_withdrawal_transition::v0::AddressCreditWithdrawalTransitionV0; +use dash_sdk::dpp::state_transition::address_funding_from_asset_lock_transition::AddressFundingFromAssetLockTransition; +use dash_sdk::dpp::state_transition::address_funding_from_asset_lock_transition::v0::AddressFundingFromAssetLockTransitionV0; +use dash_sdk::dpp::state_transition::address_funds_transfer_transition::AddressFundsTransferTransition; +use dash_sdk::dpp::state_transition::address_funds_transfer_transition::v0::AddressFundsTransferTransitionV0; +use dash_sdk::dpp::state_transition::identity_create_from_addresses_transition::IdentityCreateFromAddressesTransition; +use dash_sdk::dpp::state_transition::identity_create_from_addresses_transition::v0::IdentityCreateFromAddressesTransitionV0; +use dash_sdk::dpp::state_transition::identity_topup_from_addresses_transition::IdentityTopUpFromAddressesTransition; +use dash_sdk::dpp::state_transition::identity_topup_from_addresses_transition::v0::IdentityTopUpFromAddressesTransitionV0; +use dash_sdk::dpp::state_transition::public_key_in_creation::IdentityPublicKeyInCreation; use dash_sdk::dpp::version::PlatformVersion; +use dash_sdk::dpp::withdrawal::Pooling; +use std::collections::BTreeMap; // ── Core (L1) transaction fee estimation ──────────────────────────────────── @@ -343,18 +361,18 @@ impl PlatformFeeEstimator { /// Calculate storage fee for a given number of bytes. /// This is the main cost component for storing data on Platform. - pub fn calculate_storage_fee(&self, bytes: usize) -> u64 { + fn calculate_storage_fee(&self, bytes: usize) -> u64 { (bytes as u64).saturating_mul(self.storage_fees.storage_disk_usage_credit_per_byte) } /// Calculate processing fee for writing data. - pub fn calculate_processing_fee(&self, bytes: usize) -> u64 { + fn calculate_processing_fee(&self, bytes: usize) -> u64 { (bytes as u64).saturating_mul(self.storage_fees.storage_processing_credit_per_byte) } /// Calculate fee for tree seek operations. /// Contracts and documents require multiple seeks for tree traversal. - pub fn calculate_seek_fee(&self, seek_count: usize) -> u64 { + fn calculate_seek_fee(&self, seek_count: usize) -> u64 { (seek_count as u64).saturating_mul(self.storage_fees.storage_seek_cost) } @@ -394,11 +412,6 @@ impl PlatformFeeEstimator { self.apply_multiplier(self.min_fees.credit_withdrawal) } - /// Estimate fee for address-based credit withdrawal - pub fn estimate_address_credit_withdrawal(&self) -> u64 { - self.apply_multiplier(self.min_fees.address_credit_withdrawal) - } - /// Estimate fee for funding a platform address from an asset lock. /// This includes the asset lock processing cost and transfer costs. /// Returns fee in duffs (not credits). @@ -433,10 +446,11 @@ impl PlatformFeeEstimator { self.apply_multiplier(base_fee) } - /// Estimate fee for identity creation from addresses (asset lock). - /// This includes base cost, asset lock cost, input/output costs, per-key costs, - /// storage-based fees, and a 20% safety buffer to account for fee variability. - pub fn estimate_identity_create_from_addresses( + /// Legacy fee estimate for identity creation from addresses. + /// + /// TODO(dashpay/platform#3040): Remove once transition-based estimation is + /// accurate. Use [`Self::estimate_identity_create_from_addresses_fee`] instead. + fn estimate_identity_create_from_addresses( &self, input_count: usize, has_output: bool, @@ -500,10 +514,11 @@ impl PlatformFeeEstimator { self.apply_multiplier(base_fee) } - /// Estimate fee for identity top-up from platform addresses. - /// This includes base cost, asset lock cost, input costs, storage-based fees, - /// and a 20% safety buffer to account for fee variability. - pub fn estimate_identity_topup_from_addresses(&self, input_count: usize) -> u64 { + /// Legacy fee estimate for identity top-up from platform addresses. + /// + /// TODO(dashpay/platform#3040): Remove once transition-based estimation is + /// accurate. Use [`Self::estimate_identity_topup_from_addresses_fee`] instead. + fn estimate_identity_topup_from_addresses(&self, input_count: usize) -> u64 { // Estimated serialized bytes per input (address + signature/witness data) const ESTIMATED_BYTES_PER_INPUT: usize = 225; // Estimated bytes for top-up transaction structure @@ -548,7 +563,7 @@ impl PlatformFeeEstimator { /// Estimate fee for document creation with known size. /// Documents are stored in the contract's document tree. /// Estimated seeks: ~10 for tree traversal and insertion. - pub fn estimate_document_create_with_size(&self, document_bytes: usize) -> u64 { + fn estimate_document_create_with_size(&self, document_bytes: usize) -> u64 { const ESTIMATED_SEEKS: usize = 10; let base_fee = self .min_fees @@ -636,7 +651,7 @@ impl PlatformFeeEstimator { /// Estimate fee for data contract creation with known size. /// Includes base registration fee (0.1 DASH) plus storage costs. /// For contracts with tokens, document types, or indexes, use the detailed method. - pub fn estimate_contract_create_with_size(&self, contract_bytes: usize) -> u64 { + fn estimate_contract_create_with_size(&self, contract_bytes: usize) -> u64 { const ESTIMATED_SEEKS: usize = 20; let base_fee = self .registration_fees @@ -646,89 +661,15 @@ impl PlatformFeeEstimator { self.apply_multiplier(base_fee) } - /// Estimate fee for data contract creation with detailed component counts. - /// This provides the most accurate estimate by accounting for all registration fees. - #[allow(clippy::too_many_arguments)] - pub fn estimate_contract_create_detailed( - &self, - contract_bytes: usize, - document_type_count: usize, - non_unique_index_count: usize, - unique_index_count: usize, - contested_index_count: usize, - has_token: bool, - has_perpetual_distribution: bool, - has_pre_programmed_distribution: bool, - search_keyword_count: usize, - ) -> u64 { - const ESTIMATED_SEEKS: usize = 20; - - let mut base_fee = self.registration_fees.base_contract_registration_fee; - - // Document type fees - base_fee = base_fee.saturating_add( - self.registration_fees - .document_type_registration_fee - .saturating_mul(document_type_count as u64), - ); - - // Index fees - base_fee = base_fee.saturating_add( - self.registration_fees - .document_type_base_non_unique_index_registration_fee - .saturating_mul(non_unique_index_count as u64), - ); - base_fee = base_fee.saturating_add( - self.registration_fees - .document_type_base_unique_index_registration_fee - .saturating_mul(unique_index_count as u64), - ); - base_fee = base_fee.saturating_add( - self.registration_fees - .document_type_base_contested_index_registration_fee - .saturating_mul(contested_index_count as u64), - ); - - // Token fees - if has_token { - base_fee = base_fee.saturating_add(self.registration_fees.token_registration_fee); - } - if has_perpetual_distribution { - base_fee = base_fee - .saturating_add(self.registration_fees.token_uses_perpetual_distribution_fee); - } - if has_pre_programmed_distribution { - base_fee = base_fee.saturating_add( - self.registration_fees - .token_uses_pre_programmed_distribution_fee, - ); - } - - // Search keyword fees - base_fee = base_fee.saturating_add( - self.registration_fees - .search_keyword_fee - .saturating_mul(search_keyword_count as u64), - ); - - // Add state transition minimum and storage fees - base_fee = base_fee.saturating_add(self.min_fees.contract_create); - base_fee = base_fee - .saturating_add(self.calculate_storage_based_fee(contract_bytes, ESTIMATED_SEEKS)); - - self.apply_multiplier(base_fee) - } - /// Estimate fee for data contract creation (uses base registration fee only). - /// For more accurate estimates, use estimate_contract_create_with_size or - /// estimate_contract_create_detailed. + /// For more accurate estimates, use estimate_contract_create_with_size. pub fn estimate_contract_create_base(&self) -> u64 { // Base registration fee (0.1 DASH) + minimal storage estimate self.estimate_contract_create_with_size(500) } /// Estimate fee for data contract update with known size of changes. - pub fn estimate_contract_update_with_size(&self, update_bytes: usize) -> u64 { + fn estimate_contract_update_with_size(&self, update_bytes: usize) -> u64 { const ESTIMATED_SEEKS: usize = 15; let base_fee = self .min_fees @@ -747,11 +688,6 @@ impl PlatformFeeEstimator { &self.registration_fees } - /// Estimate fee for masternode vote - pub fn estimate_masternode_vote(&self) -> u64 { - self.apply_multiplier(self.min_fees.masternode_vote) - } - /// Estimate fee for address funds transfer. /// Applies the current fee multiplier. pub fn estimate_address_funds_transfer(&self, input_count: usize, output_count: usize) -> u64 { @@ -772,9 +708,206 @@ impl PlatformFeeEstimator { &self.min_fees } - /// Get the storage fee constants - pub fn storage_fees(&self) -> &StorageFeeConstants { - &self.storage_fees + // ── Unified fee estimation (max of legacy + transition-based) ──────── + // + // Each method below computes fees using both the legacy heuristic and a + // constructed state transition via `calculate_min_required_fee()`. The + // transition-based approach is more accurate but currently underestimates + // (dashpay/platform#3040). We return `max(legacy, transition)` until + // that is fixed, at which point the legacy paths can be removed. + + /// Estimate fee from an already-constructed state transition. + /// Returns 0 if estimation fails. + fn estimate_fee_from_transition( + transition: &T, + platform_version: &PlatformVersion, + ) -> u64 { + transition + .calculate_min_required_fee(platform_version) + .unwrap_or(0) + } + + /// Legacy platform transfer/withdrawal fee estimate (base + storage + 20% + /// buffer). Mirrors the old `estimate_platform_fee()` free function from + /// `send_screen.rs`. + /// + /// TODO(dashpay/platform#3040): Remove once transition-based estimation is + /// accurate and this fallback is no longer needed. + fn legacy_platform_transfer_fee(&self, input_count: usize) -> u64 { + /// Estimated serialized bytes per input (address + signature/witness data). + const ESTIMATED_BYTES_PER_INPUT: usize = 225; + + let inputs = input_count.max(1); + + // Base fee from Platform's min fee structure + let base_fee = self.estimate_address_funds_transfer(inputs, 1); + + // Add storage fees for serialized input bytes + let estimated_bytes = inputs * ESTIMATED_BYTES_PER_INPUT; + let storage_fee = self.estimate_storage_based_fee(estimated_bytes, inputs); + + // Total with 20% safety buffer + let total = base_fee.saturating_add(storage_fee); + total.saturating_add(total / 5) + } + + /// Estimate fee for a Platform → Core withdrawal. + /// + /// Computes `max(legacy, transition)` internally. + pub fn estimate_platform_to_core_fee( + &self, + platform_version: &PlatformVersion, + inputs: &BTreeMap, + output_script: &CoreScript, + ) -> u64 { + let legacy = self.legacy_platform_transfer_fee(inputs.len()); + + let inputs_with_nonce: BTreeMap = inputs + .iter() + .map(|(addr, amount)| (*addr, (0, *amount))) + .collect(); + + let transition = + AddressCreditWithdrawalTransition::V0(AddressCreditWithdrawalTransitionV0 { + inputs: inputs_with_nonce, + output: None, + fee_strategy: vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], + core_fee_per_byte: 1, + pooling: Pooling::Never, + output_script: output_script.clone(), + user_fee_increase: 0, + input_witnesses: Vec::new(), + }); + + let transition_fee = Self::estimate_fee_from_transition(&transition, platform_version); + + // TODO(dashpay/platform#3040): Remove legacy estimation once transition-based is accurate. + legacy.max(transition_fee) + } + + /// Estimate fee for a Platform → Platform transfer. + /// + /// Computes `max(legacy, transition)` internally. + pub fn estimate_platform_to_platform_fee( + &self, + platform_version: &PlatformVersion, + inputs: &BTreeMap, + destination: &PlatformAddress, + ) -> u64 { + let legacy = self.legacy_platform_transfer_fee(inputs.len()); + + let inputs_with_nonce: BTreeMap = inputs + .iter() + .map(|(addr, amount)| (*addr, (0, *amount))) + .collect(); + + let mut outputs = BTreeMap::new(); + outputs.insert(*destination, 1); + + let transition = AddressFundsTransferTransition::V0(AddressFundsTransferTransitionV0 { + inputs: inputs_with_nonce, + outputs, + fee_strategy: vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], + user_fee_increase: 0, + input_witnesses: Vec::new(), + }); + + let transition_fee = Self::estimate_fee_from_transition(&transition, platform_version); + + // TODO(dashpay/platform#3040): Remove legacy estimation once transition-based is accurate. + legacy.max(transition_fee) + } + + /// Estimate fee for a Core → Platform address funding (via asset lock). + /// + /// Computes `max(legacy, transition)` internally. + pub fn estimate_core_to_platform_fee( + &self, + platform_version: &PlatformVersion, + destination: &PlatformAddress, + ) -> u64 { + // Legacy estimate + let legacy = self.estimate_address_funding_from_asset_lock_duffs(1); + + let mut outputs = BTreeMap::new(); + outputs.insert(*destination, None); + + let transition = + AddressFundingFromAssetLockTransition::V0(AddressFundingFromAssetLockTransitionV0 { + asset_lock_proof: AssetLockProof::default(), + inputs: BTreeMap::new(), + outputs, + fee_strategy: vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], + user_fee_increase: 0, + ..Default::default() + }); + + let transition_fee = Self::estimate_fee_from_transition(&transition, platform_version); + + // TODO(dashpay/platform#3040): Remove legacy estimation once transition-based is accurate. + legacy.max(transition_fee) + } + + /// Estimate fee for identity creation from Platform addresses. + /// + /// Computes `max(legacy, transition)` internally. + pub fn estimate_identity_create_from_addresses_fee( + &self, + platform_version: &PlatformVersion, + inputs: &BTreeMap, + output: Option<(PlatformAddress, Credits)>, + key_count: usize, + ) -> u64 { + let legacy = + self.estimate_identity_create_from_addresses(inputs.len(), output.is_some(), key_count); + + let public_keys = match IdentityPublicKeyInCreation::default_versioned(platform_version) { + Ok(key) => std::iter::repeat_n(key, key_count).collect(), + Err(_) => Vec::new(), + }; + + let transition = + IdentityCreateFromAddressesTransition::V0(IdentityCreateFromAddressesTransitionV0 { + public_keys, + inputs: inputs.clone(), + output, + fee_strategy: vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], + user_fee_increase: 0, + input_witnesses: Vec::new(), + }); + + let transition_fee = Self::estimate_fee_from_transition(&transition, platform_version); + + // TODO(dashpay/platform#3040): Remove legacy estimation once transition-based is accurate. + legacy.max(transition_fee) + } + + /// Estimate fee for identity top-up from Platform addresses. + /// + /// Computes `max(legacy, transition)` internally. + pub fn estimate_identity_topup_from_addresses_fee( + &self, + platform_version: &PlatformVersion, + inputs: &BTreeMap, + output: Option<(PlatformAddress, Credits)>, + identity_id: Identifier, + ) -> u64 { + let legacy = self.estimate_identity_topup_from_addresses(inputs.len()); + + let transition = + IdentityTopUpFromAddressesTransition::V0(IdentityTopUpFromAddressesTransitionV0 { + inputs: inputs.clone(), + output, + identity_id, + fee_strategy: vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], + user_fee_increase: 0, + input_witnesses: Vec::new(), + }); + + let transition_fee = Self::estimate_fee_from_transition(&transition, platform_version); + + // TODO(dashpay/platform#3040): Remove legacy estimation once transition-based is accurate. + legacy.max(transition_fee) } } @@ -1001,27 +1134,6 @@ mod tests { // ~0.1 DASH for a simple contract (base registration fee dominates) } - #[test] - fn test_contract_create_detailed_with_token() { - let estimator = PlatformFeeEstimator::new(); - // Contract with a token - let fee = estimator.estimate_contract_create_detailed( - 500, // contract bytes - 1, // 1 document type - 1, // 1 non-unique index - 0, // 0 unique indexes - 0, // 0 contested indexes - true, // has token - false, // no perpetual distribution - false, // no pre-programmed distribution - 0, // 0 search keywords - ); - // Base: 0.1 DASH + Document type: 0.02 DASH + Index: 0.01 DASH + Token: 0.1 DASH - // = 0.23 DASH + storage fees - let expected_registration = 10_000_000_000 + 2_000_000_000 + 1_000_000_000 + 10_000_000_000; - assert!(fee >= expected_registration); - } - #[test] fn test_format_credits() { // 1 DASH = 100,000,000,000 credits diff --git a/src/ui/identities/add_new_identity_screen/by_platform_address.rs b/src/ui/identities/add_new_identity_screen/by_platform_address.rs index 4984d30df..95fe5dd13 100644 --- a/src/ui/identities/add_new_identity_screen/by_platform_address.rs +++ b/src/ui/identities/add_new_identity_screen/by_platform_address.rs @@ -153,15 +153,28 @@ impl AddNewIdentityScreen { // Calculate estimated fee for identity creation (needed for max amount calculation) let key_count = self.identity_keys.keys_input.len() + 1; // +1 for master key - let input_count = if self.selected_platform_address_for_funding.is_some() { - 1 - } else { - 0 - }; + let inputs: std::collections::BTreeMap< + PlatformAddress, + ( + dash_sdk::dpp::prelude::AddressNonce, + dash_sdk::dpp::balances::credits::Credits, + ), + > = self + .selected_platform_address_for_funding + .as_ref() + .map(|(platform_addr, amount)| { + std::collections::BTreeMap::from([(*platform_addr, (0, *amount))]) + }) + .unwrap_or_default(); let estimated_fee = self .app_context .fee_estimator() - .estimate_identity_create_from_addresses(input_count, false, key_count); + .estimate_identity_create_from_addresses_fee( + self.app_context.platform_version(), + &inputs, + None, + key_count, + ); // Calculate max amount with fee reserved let max_amount_with_fee_reserved = diff --git a/src/ui/identities/top_up_identity_screen/by_platform_address.rs b/src/ui/identities/top_up_identity_screen/by_platform_address.rs index 59d2f21fb..8ea571837 100644 --- a/src/ui/identities/top_up_identity_screen/by_platform_address.rs +++ b/src/ui/identities/top_up_identity_screen/by_platform_address.rs @@ -13,6 +13,7 @@ use crate::ui::theme::DashColors; use dash_sdk::dpp::address_funds::PlatformAddress; use dash_sdk::dpp::balances::credits::Credits; use dash_sdk::dpp::dashcore::Address; +use dash_sdk::dpp::identity::accessors::IdentityGettersV0; use egui::{Frame, Margin, RichText, Ui}; use std::collections::BTreeMap; @@ -99,8 +100,22 @@ impl TopUpIdentityScreen { .map(|(_, _, balance)| *balance); // Calculate estimated fee for top-up from platform address (needed for max amount calculation) - let fee_estimator = self.app_context.fee_estimator(); - let estimated_fee = fee_estimator.estimate_identity_topup_from_addresses(1); + let inputs: BTreeMap = + self.selected_platform_address + .as_ref() + .map(|(_, platform_addr, balance)| { + BTreeMap::from([(*platform_addr, (0, *balance))]) + }) + .unwrap_or_default(); + let estimated_fee = self + .app_context + .fee_estimator() + .estimate_identity_topup_from_addresses_fee( + self.app_context.platform_version(), + &inputs, + None, + self.identity.identity.id(), + ); // Calculate max amount with fee reserved let max_amount_with_fee_reserved = diff --git a/src/ui/wallets/send_screen.rs b/src/ui/wallets/send_screen.rs index e27300f80..887ba2d4b 100644 --- a/src/ui/wallets/send_screen.rs +++ b/src/ui/wallets/send_screen.rs @@ -18,18 +18,9 @@ use crate::ui::theme::DashColors; use crate::ui::{MessageType, RootScreenType, ScreenLike}; use dash_sdk::dashcore_rpc::dashcore::Address; use dash_sdk::dashcore_rpc::dashcore::address::NetworkUnchecked; -use dash_sdk::dpp::address_funds::AddressFundsFeeStrategyStep; use dash_sdk::dpp::address_funds::PlatformAddress; use dash_sdk::dpp::balances::credits::{CREDITS_PER_DUFF, Credits}; use dash_sdk::dpp::identity::core_script::CoreScript; -use dash_sdk::dpp::prelude::AddressNonce; -use dash_sdk::dpp::prelude::AssetLockProof; -use dash_sdk::dpp::state_transition::StateTransitionEstimatedFeeValidation; -use dash_sdk::dpp::state_transition::address_credit_withdrawal_transition::AddressCreditWithdrawalTransition; -use dash_sdk::dpp::state_transition::address_credit_withdrawal_transition::v0::AddressCreditWithdrawalTransitionV0; -use dash_sdk::dpp::state_transition::address_funding_from_asset_lock_transition::AddressFundingFromAssetLockTransition; -use dash_sdk::dpp::state_transition::address_funding_from_asset_lock_transition::v0::AddressFundingFromAssetLockTransitionV0; -use dash_sdk::dpp::withdrawal::Pooling; use eframe::egui::{self, Context, RichText, Ui}; use egui::{Color32, Frame, Margin}; use std::collections::BTreeMap; @@ -41,80 +32,6 @@ const MAX_PLATFORM_INPUTS: usize = 16; use crate::model::fee_estimation::PlatformFeeEstimator; -/// Estimated serialized bytes per input (address + signature/witness data) -const ESTIMATED_BYTES_PER_INPUT: usize = 225; - -/// Calculate the estimated fee for a platform address funds transfer. -/// -/// Uses PlatformFeeEstimator for base costs (input/output fees) plus storage fees. -fn estimate_platform_fee(estimator: &PlatformFeeEstimator, input_count: usize) -> u64 { - let inputs = input_count.max(1); - - // Base fee from Platform's min fee structure - // - 500,000 credits per input (address_funds_transfer_input_cost) - // - 6,000,000 credits per output (address_funds_transfer_output_cost) - let base_fee = estimator.estimate_address_funds_transfer(inputs, 1); - - // Add storage fees for serialized input bytes only - // (outputs don't add significant serialization overhead) - let estimated_bytes = inputs * ESTIMATED_BYTES_PER_INPUT; - let storage_fee = estimator.estimate_storage_based_fee(estimated_bytes, inputs); - - // Total with 20% safety buffer - let total = base_fee.saturating_add(storage_fee); - total.saturating_add(total / 5) -} - -/// Calculate the estimated fee for a Platform address withdrawal using a constructed state transition. -fn estimate_withdrawal_fee_from_transition( - platform_version: &dash_sdk::dpp::version::PlatformVersion, - inputs: &BTreeMap, - output_script: &CoreScript, -) -> u64 { - let inputs_with_nonce: BTreeMap = inputs - .iter() - .map(|(addr, amount)| (*addr, (0, *amount))) - .collect(); - - let transition = AddressCreditWithdrawalTransition::V0(AddressCreditWithdrawalTransitionV0 { - inputs: inputs_with_nonce, - output: None, - fee_strategy: vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], - core_fee_per_byte: 1, - pooling: Pooling::Never, - output_script: output_script.clone(), - user_fee_increase: 0, - input_witnesses: Vec::new(), - }); - - transition - .calculate_min_required_fee(platform_version) - .unwrap_or(0) -} - -/// Calculate the estimated fee for funding a Platform address from an asset lock. -fn estimate_address_funding_fee_from_transition( - platform_version: &dash_sdk::dpp::version::PlatformVersion, - destination: &PlatformAddress, -) -> u64 { - let mut outputs = BTreeMap::new(); - outputs.insert(*destination, None); - - let transition = - AddressFundingFromAssetLockTransition::V0(AddressFundingFromAssetLockTransitionV0 { - asset_lock_proof: AssetLockProof::default(), - inputs: BTreeMap::new(), - outputs, - fee_strategy: vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], - user_fee_increase: 0, - ..Default::default() - }); - - transition - .calculate_min_required_fee(platform_version) - .unwrap_or(0) -} - /// Result of allocating platform addresses for a transfer. #[derive(Debug, Clone)] struct AddressAllocationResult { @@ -136,9 +53,9 @@ fn allocate_platform_addresses_with_fee( amount_credits: u64, destination: Option<&PlatformAddress>, fee_for_inputs: F, -) -> AddressAllocationResult +) -> Result where - F: Fn(&BTreeMap) -> u64, + F: Fn(&BTreeMap) -> Result, { // Filter out the destination address if provided (protocol doesn't allow same address as input and output) let filtered: Vec<_> = addresses @@ -153,19 +70,19 @@ where // Early return if no addresses available after filtering if sorted_addresses.is_empty() { - return AddressAllocationResult { + return Ok(AddressAllocationResult { inputs: BTreeMap::new(), fee_payer_index: 0, - estimated_fee: fee_for_inputs(&BTreeMap::new()), + estimated_fee: fee_for_inputs(&BTreeMap::new())?, shortfall: amount_credits, sorted_addresses: vec![], - }; + }); } // The highest-balance address (first in sorted order) will pay the fee let fee_payer_addr = sorted_addresses.first().map(|(addr, _, _)| *addr); - let mut estimated_fee = fee_for_inputs(&BTreeMap::new()); + let mut estimated_fee = fee_for_inputs(&BTreeMap::new())?; let mut inputs: BTreeMap = BTreeMap::new(); // Iterate until fee estimate stabilizes (input count affects fee) @@ -190,7 +107,7 @@ where } } - let new_fee = fee_for_inputs(&inputs); + let new_fee = fee_for_inputs(&inputs)?; if new_fee == estimated_fee { break; } @@ -224,41 +141,12 @@ where }) .unwrap_or(0); - AddressAllocationResult { + Ok(AddressAllocationResult { inputs, fee_payer_index, estimated_fee, shortfall, sorted_addresses, - } -} - -/// Allocates platform addresses for a transfer, selecting which addresses to use -/// and how much from each. -/// -/// Algorithm: -/// 1. Filters out the destination address (can't be both input and output) -/// 2. Sorts addresses by balance descending (largest first) -/// 3. The highest-balance address pays the fee -/// 4. Iteratively allocates until fee estimate converges -/// 5. Fee payer is always included in inputs (even with 0 contribution) so fee can be deducted -/// -/// Returns the allocation result with inputs, fee payer index, and any shortfall. -fn allocate_platform_addresses( - estimator: &PlatformFeeEstimator, - addresses: &[(PlatformAddress, Address, u64)], - amount_credits: u64, - destination: Option<&PlatformAddress>, -) -> AddressAllocationResult { - let max_inputs = addresses - .iter() - .filter(|(platform_addr, _, _)| destination != Some(platform_addr)) - .count() - .min(MAX_PLATFORM_INPUTS); - - allocate_platform_addresses_with_fee(addresses, amount_credits, destination, |_| { - // Keep the legacy behavior: use a worst-case fee based on max possible inputs. - estimate_platform_fee(estimator, max_inputs.max(1)) }) } @@ -427,7 +315,7 @@ impl WalletSendScreen { fee_estimator: &PlatformFeeEstimator, addresses: &[(PlatformAddress, Address, u64)], destination: Option<&PlatformAddress>, - ) -> u64 { + ) -> Result { let mut sorted_addresses: Vec<_> = addresses .iter() .filter(|(addr, _, _)| destination != Some(addr)) @@ -437,10 +325,12 @@ impl WalletSendScreen { let usable_count = sorted_addresses.len().min(MAX_PLATFORM_INPUTS); if usable_count == 0 { - return estimate_platform_fee(fee_estimator, 1); + return Ok(fee_estimator.estimate_address_funds_transfer(1, 1)); } let dest_type = Self::detect_address_type(&self.destination_address); + let platform_version = self.app_context.platform_version(); + if dest_type == AddressType::Core { let output_script = self .destination_address @@ -455,15 +345,29 @@ impl WalletSendScreen { .take(usable_count) .map(|(addr, _, _)| (*addr, 0)) .collect(); - return estimate_withdrawal_fee_from_transition( - self.app_context.platform_version(), + return Ok(fee_estimator.estimate_platform_to_core_fee( + platform_version, &max_fee_inputs, &output_script, - ); + )); } + } else if dest_type == AddressType::Platform + && let Some(destination) = destination + { + let max_fee_inputs: BTreeMap = sorted_addresses + .iter() + .take(usable_count) + .map(|(addr, _, _)| (*addr, 0)) + .collect(); + return Ok(fee_estimator.estimate_platform_to_platform_fee( + platform_version, + &max_fee_inputs, + destination, + )); } - estimate_platform_fee(fee_estimator, usable_count) + // Fallback: use legacy estimate based on address count + Ok(fee_estimator.estimate_address_funds_transfer(usable_count, 1)) } fn reset_form(&mut self) { @@ -841,12 +745,20 @@ impl WalletSendScreen { .map_err(|e| format!("Invalid platform address: {}", e))?; // Allocate addresses using the helper function - let allocation = allocate_platform_addresses( - &fee_estimator, + let platform_version = self.app_context.platform_version(); + let allocation = allocate_platform_addresses_with_fee( &addresses, amount_credits, Some(&destination), - ); + |inputs| { + Ok(fee_estimator.estimate_platform_to_platform_fee( + platform_version, + inputs, + &destination, + )) + }, + ) + .map_err(|e| format!("Fee estimation failed: {}", e))?; if allocation.sorted_addresses.is_empty() { return Err( @@ -874,11 +786,13 @@ impl WalletSendScreen { .take(MAX_PLATFORM_INPUTS) .map(|(_, _, b)| *b) .sum(); - let max_fee = self.estimate_max_fee_for_platform_send( - &fee_estimator, - &allocation.sorted_addresses, - Some(&destination), - ); + let max_fee = self + .estimate_max_fee_for_platform_send( + &fee_estimator, + &allocation.sorted_addresses, + Some(&destination), + ) + .unwrap_or(allocation.estimated_fee); let max_sendable = max_balance.saturating_sub(max_fee); return Err(format!( @@ -972,12 +886,18 @@ impl WalletSendScreen { let output_script = CoreScript::new(dest_address.script_pubkey()); let platform_version = self.app_context.platform_version(); + let fee_estimator = self.app_context.fee_estimator(); // Allocate addresses using state-transition-based fee estimation (no destination filter) let allocation = allocate_platform_addresses_with_fee(&addresses, amount_credits, None, |inputs| { - estimate_withdrawal_fee_from_transition(platform_version, inputs, &output_script) - }); + Ok(fee_estimator.estimate_platform_to_core_fee( + platform_version, + inputs, + &output_script, + )) + }) + .map_err(|e| format!("Fee estimation failed: {}", e))?; if allocation.shortfall > 0 { // Calculate the max we can send with MAX_PLATFORM_INPUTS addresses (minus fees) @@ -994,7 +914,7 @@ impl WalletSendScreen { .take(addresses_available) .map(|(addr, _, _)| (*addr, 0)) .collect(); - let max_fee = estimate_withdrawal_fee_from_transition( + let max_fee = fee_estimator.estimate_platform_to_core_fee( platform_version, &max_fee_inputs, &output_script, @@ -1435,11 +1355,10 @@ impl WalletSendScreen { .map(|(addr, _)| addr) .ok(); if let Some(destination) = destination { - let estimated_fee = estimate_address_funding_fee_from_transition( + let estimated_fee = fee_estimator.estimate_core_to_platform_fee( self.app_context.platform_version(), &destination, ); - // max = max.map(|amount| amount.saturating_sub(estimated_fee)); Some(format!( "Estimated platform fee ~{} (deducted from amount)", Self::format_credits(estimated_fee) @@ -1473,21 +1392,41 @@ impl WalletSendScreen { .take(MAX_PLATFORM_INPUTS) .map(|(_, _, balance)| *balance) .sum(); - let max_fee = self.estimate_max_fee_for_platform_send( + let usable_count = sorted_addresses.len().min(MAX_PLATFORM_INPUTS); + let (max_fee, fee_error) = match self.estimate_max_fee_for_platform_send( &fee_estimator, &sorted_addresses, destination.as_ref(), - ); + ) { + Ok(fee) => (fee, None), + Err(e) => { + tracing::warn!("Fee estimation failed for Max calculation: {}", e); + ( + fee_estimator.estimate_address_funds_transfer(usable_count, 1), + Some(e), + ) + } + }; // Build hint explaining the limit + let fallback_note = if fee_error.is_some() { + " (fallback estimate)" + } else { + "" + }; let hint = if sorted_addresses.len() > MAX_PLATFORM_INPUTS { format!( - "Limited to {} input addresses per transaction, ~{} reserved for fees", + "Limited to {} input addresses per transaction, ~{} reserved for fees{}", MAX_PLATFORM_INPUTS, - Self::format_credits(max_fee) + Self::format_credits(max_fee), + fallback_note ) } else { - format!("~{} reserved for fees", Self::format_credits(max_fee)) + format!( + "~{} reserved for fees{}", + Self::format_credits(max_fee), + fallback_note + ) }; (Some(total.saturating_sub(max_fee)), Some(hint)) } @@ -1587,12 +1526,56 @@ impl WalletSendScreen { .ok(); // Use the same allocation algorithm as the send logic, filtering out the destination - let allocation = allocate_platform_addresses( - &fee_estimator, - addresses, - amount_credits, - destination.as_ref(), - ); + let platform_version = self.app_context.platform_version(); + let dest_type = Self::detect_address_type(&self.destination_address); + + let allocation = if dest_type == AddressType::Platform { + if let Some(dest) = &destination { + allocate_platform_addresses_with_fee( + addresses, + amount_credits, + destination.as_ref(), + |inputs| { + Ok(fee_estimator.estimate_platform_to_platform_fee( + platform_version, + inputs, + dest, + )) + }, + ) + } else { + return; + } + } else if dest_type == AddressType::Core { + let output_script = self + .destination_address + .trim() + .parse::>() + .ok() + .and_then(|addr| addr.require_network(self.app_context.network).ok()) + .map(|addr| CoreScript::new(addr.script_pubkey())); + if let Some(output_script) = output_script { + allocate_platform_addresses_with_fee(addresses, amount_credits, None, |inputs| { + Ok(fee_estimator.estimate_platform_to_core_fee( + platform_version, + inputs, + &output_script, + )) + }) + } else { + return; + } + } else { + return; + }; + + let allocation = match allocation { + Ok(a) => a, + Err(e) => { + tracing::warn!("Fee estimation failed for breakdown: {}", e); + return; + } + }; if allocation.inputs.is_empty() { return; From d851b4bd9125425a2c2dce983a968461c9eb4a71 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 24 Feb 2026 20:40:46 +0100 Subject: [PATCH 3/8] test(fees): add tests for unified transition-based fee estimation Add 9 tests covering the new public unified methods that compute max(legacy, transition-based) fees: - platform-to-core fee (positive, scales with inputs) - platform-to-platform fee (positive) - core-to-platform fee (positive) - identity-create-from-addresses fee (positive, scales with keys) - identity-topup-from-addresses fee (positive) - unified fees >= legacy (invariant check for max() contract) Co-Authored-By: Claude Opus 4.6 --- src/model/fee_estimation.rs | 152 ++++++++++++++++++++++++++++++++++++ 1 file changed, 152 insertions(+) diff --git a/src/model/fee_estimation.rs b/src/model/fee_estimation.rs index eb6c4763b..2227a214a 100644 --- a/src/model/fee_estimation.rs +++ b/src/model/fee_estimation.rs @@ -1134,6 +1134,158 @@ mod tests { // ~0.1 DASH for a simple contract (base registration fee dominates) } + // ── Unified (max of legacy + transition) fee tests ───────────────── + + /// Helper: create a dummy PlatformAddress for testing. + fn test_platform_address(byte: u8) -> PlatformAddress { + PlatformAddress::P2pkh([byte; 20]) + } + + fn test_platform_version() -> &'static PlatformVersion { + PlatformVersion::latest() + } + + #[test] + fn test_platform_to_core_fee_is_positive() { + let estimator = PlatformFeeEstimator::new(); + let pv = test_platform_version(); + let addr = test_platform_address(1); + let mut inputs = BTreeMap::new(); + inputs.insert(addr, 1_000_000_000u64); + let output_script = CoreScript::from_bytes(vec![0x76, 0xa9, 0x14]); // dummy P2PKH prefix + let fee = estimator.estimate_platform_to_core_fee(pv, &inputs, &output_script); + assert!(fee > 0, "platform-to-core fee must be positive, got {fee}"); + } + + #[test] + fn test_platform_to_core_fee_grows_with_inputs() { + let estimator = PlatformFeeEstimator::new(); + let pv = test_platform_version(); + let output_script = CoreScript::from_bytes(vec![0x76, 0xa9, 0x14]); + + let mut inputs_1 = BTreeMap::new(); + inputs_1.insert(test_platform_address(1), 1_000_000_000u64); + let fee_1 = estimator.estimate_platform_to_core_fee(pv, &inputs_1, &output_script); + + let mut inputs_3 = BTreeMap::new(); + for i in 0..3u8 { + inputs_3.insert(test_platform_address(i), 500_000_000u64); + } + let fee_3 = estimator.estimate_platform_to_core_fee(pv, &inputs_3, &output_script); + assert!( + fee_3 > fee_1, + "fee with 3 inputs ({fee_3}) should exceed fee with 1 input ({fee_1})" + ); + } + + #[test] + fn test_platform_to_platform_fee_is_positive() { + let estimator = PlatformFeeEstimator::new(); + let pv = test_platform_version(); + let mut inputs = BTreeMap::new(); + inputs.insert(test_platform_address(1), 1_000_000_000u64); + let dest = test_platform_address(2); + let fee = estimator.estimate_platform_to_platform_fee(pv, &inputs, &dest); + assert!( + fee > 0, + "platform-to-platform fee must be positive, got {fee}" + ); + } + + #[test] + fn test_core_to_platform_fee_is_positive() { + let estimator = PlatformFeeEstimator::new(); + let pv = test_platform_version(); + let dest = test_platform_address(1); + let fee = estimator.estimate_core_to_platform_fee(pv, &dest); + assert!(fee > 0, "core-to-platform fee must be positive, got {fee}"); + } + + #[test] + fn test_identity_create_from_addresses_fee_is_positive() { + let estimator = PlatformFeeEstimator::new(); + let pv = test_platform_version(); + let mut inputs = BTreeMap::new(); + inputs.insert(test_platform_address(1), (0u32, 5_000_000_000u64)); + let fee = estimator.estimate_identity_create_from_addresses_fee(pv, &inputs, None, 2); + assert!( + fee > 0, + "identity-create-from-addresses fee must be positive, got {fee}" + ); + } + + #[test] + fn test_identity_create_from_addresses_fee_grows_with_keys() { + let estimator = PlatformFeeEstimator::new(); + let pv = test_platform_version(); + let mut inputs = BTreeMap::new(); + inputs.insert(test_platform_address(1), (0u32, 5_000_000_000u64)); + let fee_2 = estimator.estimate_identity_create_from_addresses_fee(pv, &inputs, None, 2); + let fee_5 = estimator.estimate_identity_create_from_addresses_fee(pv, &inputs, None, 5); + assert!( + fee_5 > fee_2, + "fee with 5 keys ({fee_5}) should exceed fee with 2 keys ({fee_2})" + ); + } + + #[test] + fn test_identity_topup_from_addresses_fee_is_positive() { + let estimator = PlatformFeeEstimator::new(); + let pv = test_platform_version(); + let mut inputs = BTreeMap::new(); + inputs.insert(test_platform_address(1), (0u32, 1_000_000_000u64)); + let identity_id = Identifier::default(); + let fee = + estimator.estimate_identity_topup_from_addresses_fee(pv, &inputs, None, identity_id); + assert!( + fee > 0, + "identity-topup-from-addresses fee must be positive, got {fee}" + ); + } + + #[test] + fn test_unified_fees_at_least_as_high_as_legacy() { + // The unified methods return max(legacy, transition). Verify they are + // at least as high as the legacy estimate alone. + let estimator = PlatformFeeEstimator::new(); + let pv = test_platform_version(); + + // Platform → Platform + let mut inputs = BTreeMap::new(); + inputs.insert(test_platform_address(1), 1_000_000_000u64); + let legacy_transfer = estimator.legacy_platform_transfer_fee(1); + let unified_transfer = + estimator.estimate_platform_to_platform_fee(pv, &inputs, &test_platform_address(2)); + assert!( + unified_transfer >= legacy_transfer, + "unified ({unified_transfer}) must be >= legacy ({legacy_transfer})" + ); + + // Identity create from addresses + let mut id_inputs = BTreeMap::new(); + id_inputs.insert(test_platform_address(1), (0u32, 5_000_000_000u64)); + let legacy_create = estimator.estimate_identity_create_from_addresses(1, false, 2); + let unified_create = + estimator.estimate_identity_create_from_addresses_fee(pv, &id_inputs, None, 2); + assert!( + unified_create >= legacy_create, + "unified create ({unified_create}) must be >= legacy ({legacy_create})" + ); + + // Identity topup from addresses + let legacy_topup = estimator.estimate_identity_topup_from_addresses(1); + let unified_topup = estimator.estimate_identity_topup_from_addresses_fee( + pv, + &id_inputs, + None, + Identifier::default(), + ); + assert!( + unified_topup >= legacy_topup, + "unified topup ({unified_topup}) must be >= legacy ({legacy_topup})" + ); + } + #[test] fn test_format_credits() { // 1 DASH = 100,000,000,000 credits From b69876ef533de642d78ca0bc51566ba63cb29c3a Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 24 Feb 2026 21:46:04 +0100 Subject: [PATCH 4/8] fix(fees): address review findings FEE-001 through FEE-005 - FEE-001: guard reload_utxos() return in SPV mode to prevent silent UTXO loss during fee re-estimation - FEE-002: use calculate_relay_fee() on asset lock tx size instead of raw byte count as fee amount - FEE-003: replace manual utxos.retain()/drop_utxo() in FundWithUtxo paths with consolidated remove_selected_utxos() - FEE-004: remove unnecessary Result wrapping from allocate_platform_addresses_with_fee (infallible fee estimation) - FEE-005: replace expect() on Core client lock with map_err for graceful error handling Co-Authored-By: Claude Opus 4.6 --- src/backend_task/core/mod.rs | 10 +++- .../identity/register_identity.rs | 16 ++----- src/backend_task/identity/top_up_identity.rs | 16 +++---- src/model/fee_estimation.rs | 4 +- src/model/wallet/asset_lock_transaction.rs | 5 +- src/ui/wallets/send_screen.rs | 48 ++++++++----------- 6 files changed, 43 insertions(+), 56 deletions(-) diff --git a/src/backend_task/core/mod.rs b/src/backend_task/core/mod.rs index 52c47e283..6671dd624 100644 --- a/src/backend_task/core/mod.rs +++ b/src/backend_task/core/mod.rs @@ -409,7 +409,13 @@ impl AppContext { // The first build already removed UTXOs, so reload the wallet // state and rebuild. In practice this rarely triggers because // the initial 5-input estimate is usually sufficient. - wallet_guard.reload_utxos(self)?; + if !wallet_guard.reload_utxos(self)? { + // SPV mode: reload is a no-op, cannot recover removed UTXOs. + // This path should only be reached in RPC mode. + return Err( + "Fee re-estimation failed: cannot reload UTXOs in SPV mode".to_string() + ); + } tx = wallet_guard.build_multi_recipient_payment_transaction( self.network, &parsed_recipients, @@ -426,7 +432,7 @@ impl AppContext { let txid = self .core_client .read() - .expect("Core client lock was poisoned") + .map_err(|e| format!("Core client lock was poisoned: {}", e))? .send_raw_transaction(&tx) .map_err(|e| format!("Failed to broadcast transaction: {e}"))?; diff --git a/src/backend_task/identity/register_identity.rs b/src/backend_task/identity/register_identity.rs index 63ec342ac..bd0aecb7c 100644 --- a/src/backend_task/identity/register_identity.rs +++ b/src/backend_task/identity/register_identity.rs @@ -233,18 +233,12 @@ impl AppContext { ) .map_err(|e| format!("Failed to store asset lock transaction: {}", e))?; - // TODO: UTXO removal timing issue - see comment above for FundWithWallet case. + // Remove the spent UTXO using the consolidated method. { - let mut wallet = wallet.write().unwrap(); - wallet.utxos.retain(|_, utxo_map| { - utxo_map.retain(|outpoint, _| outpoint != &utxo); - !utxo_map.is_empty() - }); - self.db - .drop_utxo(&utxo, &self.network.to_string()) - .map_err(|e| e.to_string())?; - - wallet.recalculate_address_balance(&input_address, self)?; + let mut wallet = wallet.write().map_err(|e| e.to_string())?; + let mut selected = BTreeMap::new(); + selected.insert(utxo, (tx_out.clone(), input_address.clone())); + wallet.remove_selected_utxos(&selected, &self.db, self.network)?; } let asset_lock_proof = self.wait_for_asset_lock_proof(tx_id).await?; diff --git a/src/backend_task/identity/top_up_identity.rs b/src/backend_task/identity/top_up_identity.rs index 48ef5621b..315aebe27 100644 --- a/src/backend_task/identity/top_up_identity.rs +++ b/src/backend_task/identity/top_up_identity.rs @@ -15,6 +15,7 @@ use dash_sdk::dpp::state_transition::identity_topup_transition::IdentityTopUpTra use dash_sdk::dpp::state_transition::identity_topup_transition::methods::IdentityTopUpTransitionMethodsV0; use dash_sdk::platform::Fetch; use dash_sdk::platform::transition::top_up_identity::TopUpIdentity; +use std::collections::BTreeMap; impl AppContext { pub(super) async fn top_up_identity( @@ -219,17 +220,12 @@ impl AppContext { ) .map_err(|e| format!("Failed to store asset lock transaction: {}", e))?; + // Remove the spent UTXO using the consolidated method. { - let mut wallet = wallet.write().unwrap(); - wallet.utxos.retain(|_, utxo_map| { - utxo_map.retain(|outpoint, _| outpoint != &utxo); - !utxo_map.is_empty() - }); - self.db - .drop_utxo(&utxo, &self.network.to_string()) - .map_err(|e| e.to_string())?; - - wallet.recalculate_address_balance(&input_address, self)?; + let mut wallet = wallet.write().map_err(|e| e.to_string())?; + let mut selected = BTreeMap::new(); + selected.insert(utxo, (tx_out.clone(), input_address.clone())); + wallet.remove_selected_utxos(&selected, &self.db, self.network)?; } let asset_lock_proof = self.wait_for_asset_lock_proof(tx_id).await?; diff --git a/src/model/fee_estimation.rs b/src/model/fee_estimation.rs index 2227a214a..def64a053 100644 --- a/src/model/fee_estimation.rs +++ b/src/model/fee_estimation.rs @@ -118,7 +118,7 @@ pub fn calculate_asset_lock_fee( // First pass: assume 2 outputs (1 burn + 1 change). let fee_with_change = std::cmp::max( MIN_ASSET_LOCK_FEE, - estimate_asset_lock_tx_size(num_inputs, 2) as u64, + calculate_relay_fee(estimate_asset_lock_tx_size(num_inputs, 2)), ); let required_with_change = requested_amount @@ -141,7 +141,7 @@ pub fn calculate_asset_lock_fee( // Recompute with 1 output (no change). let fee_no_change = std::cmp::max( MIN_ASSET_LOCK_FEE, - estimate_asset_lock_tx_size(num_inputs, 1) as u64, + calculate_relay_fee(estimate_asset_lock_tx_size(num_inputs, 1)), ); let required_no_change = requested_amount diff --git a/src/model/wallet/asset_lock_transaction.rs b/src/model/wallet/asset_lock_transaction.rs index a51dec020..41de2d375 100644 --- a/src/model/wallet/asset_lock_transaction.rs +++ b/src/model/wallet/asset_lock_transaction.rs @@ -1,6 +1,7 @@ use crate::context::AppContext; use crate::model::fee_estimation::{ - AssetLockFeeResult, MIN_ASSET_LOCK_FEE, calculate_asset_lock_fee, estimate_asset_lock_tx_size, + AssetLockFeeResult, MIN_ASSET_LOCK_FEE, calculate_asset_lock_fee, calculate_relay_fee, + estimate_asset_lock_tx_size, }; use crate::model::wallet::Wallet; use dash_sdk::dashcore_rpc::dashcore::key::Secp256k1; @@ -52,7 +53,7 @@ impl Wallet { // we can pick up any additional marginal UTXOs. fee_estimate = std::cmp::max( MIN_ASSET_LOCK_FEE, - estimate_asset_lock_tx_size(num_inputs, 2) as u64, + calculate_relay_fee(estimate_asset_lock_tx_size(num_inputs, 2)), ); continue; } diff --git a/src/ui/wallets/send_screen.rs b/src/ui/wallets/send_screen.rs index 887ba2d4b..e4606e102 100644 --- a/src/ui/wallets/send_screen.rs +++ b/src/ui/wallets/send_screen.rs @@ -53,9 +53,9 @@ fn allocate_platform_addresses_with_fee( amount_credits: u64, destination: Option<&PlatformAddress>, fee_for_inputs: F, -) -> Result +) -> AddressAllocationResult where - F: Fn(&BTreeMap) -> Result, + F: Fn(&BTreeMap) -> u64, { // Filter out the destination address if provided (protocol doesn't allow same address as input and output) let filtered: Vec<_> = addresses @@ -70,19 +70,19 @@ where // Early return if no addresses available after filtering if sorted_addresses.is_empty() { - return Ok(AddressAllocationResult { + return AddressAllocationResult { inputs: BTreeMap::new(), fee_payer_index: 0, - estimated_fee: fee_for_inputs(&BTreeMap::new())?, + estimated_fee: fee_for_inputs(&BTreeMap::new()), shortfall: amount_credits, sorted_addresses: vec![], - }); + }; } // The highest-balance address (first in sorted order) will pay the fee let fee_payer_addr = sorted_addresses.first().map(|(addr, _, _)| *addr); - let mut estimated_fee = fee_for_inputs(&BTreeMap::new())?; + let mut estimated_fee = fee_for_inputs(&BTreeMap::new()); let mut inputs: BTreeMap = BTreeMap::new(); // Iterate until fee estimate stabilizes (input count affects fee) @@ -107,7 +107,7 @@ where } } - let new_fee = fee_for_inputs(&inputs)?; + let new_fee = fee_for_inputs(&inputs); if new_fee == estimated_fee { break; } @@ -141,13 +141,13 @@ where }) .unwrap_or(0); - Ok(AddressAllocationResult { + AddressAllocationResult { inputs, fee_payer_index, estimated_fee, shortfall, sorted_addresses, - }) + } } /// Detected address type @@ -751,14 +751,13 @@ impl WalletSendScreen { amount_credits, Some(&destination), |inputs| { - Ok(fee_estimator.estimate_platform_to_platform_fee( + fee_estimator.estimate_platform_to_platform_fee( platform_version, inputs, &destination, - )) + ) }, - ) - .map_err(|e| format!("Fee estimation failed: {}", e))?; + ); if allocation.sorted_addresses.is_empty() { return Err( @@ -891,13 +890,12 @@ impl WalletSendScreen { // Allocate addresses using state-transition-based fee estimation (no destination filter) let allocation = allocate_platform_addresses_with_fee(&addresses, amount_credits, None, |inputs| { - Ok(fee_estimator.estimate_platform_to_core_fee( + fee_estimator.estimate_platform_to_core_fee( platform_version, inputs, &output_script, - )) - }) - .map_err(|e| format!("Fee estimation failed: {}", e))?; + ) + }); if allocation.shortfall > 0 { // Calculate the max we can send with MAX_PLATFORM_INPUTS addresses (minus fees) @@ -1536,11 +1534,11 @@ impl WalletSendScreen { amount_credits, destination.as_ref(), |inputs| { - Ok(fee_estimator.estimate_platform_to_platform_fee( + fee_estimator.estimate_platform_to_platform_fee( platform_version, inputs, dest, - )) + ) }, ) } else { @@ -1556,11 +1554,11 @@ impl WalletSendScreen { .map(|addr| CoreScript::new(addr.script_pubkey())); if let Some(output_script) = output_script { allocate_platform_addresses_with_fee(addresses, amount_credits, None, |inputs| { - Ok(fee_estimator.estimate_platform_to_core_fee( + fee_estimator.estimate_platform_to_core_fee( platform_version, inputs, &output_script, - )) + ) }) } else { return; @@ -1569,14 +1567,6 @@ impl WalletSendScreen { return; }; - let allocation = match allocation { - Ok(a) => a, - Err(e) => { - tracing::warn!("Fee estimation failed for breakdown: {}", e); - return; - } - }; - if allocation.inputs.is_empty() { return; } From 3d388e34e3f6d08dac3495a5877d95b526007446 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 24 Feb 2026 21:59:29 +0100 Subject: [PATCH 5/8] fix(fees): address CodeRabbit nitpicks from PR #651 review - Add tracing::warn sanity check after fee rebuild to detect under-estimation edge case - Simplify verbose type annotations in by_platform_address.rs - Log errors in estimate_fee_from_transition instead of silent unwrap_or(0) - Log default_versioned key creation failures in fee estimation - Add platform-to-core and core-to-platform to unified fee invariant test - Use i128 cast in fee mismatch logging to prevent i64 overflow Co-Authored-By: Claude Opus 4.6 --- src/backend_task/core/mod.rs | 12 ++++++ src/backend_task/identity/mod.rs | 2 +- src/model/fee_estimation.rs | 39 ++++++++++++++++--- .../by_platform_address.rs | 15 +++---- 4 files changed, 52 insertions(+), 16 deletions(-) diff --git a/src/backend_task/core/mod.rs b/src/backend_task/core/mod.rs index 6671dd624..abc479ae6 100644 --- a/src/backend_task/core/mod.rs +++ b/src/backend_task/core/mod.rs @@ -423,6 +423,18 @@ impl AppContext { request.subtract_fee_from_amount, Some(self), )?; + // Sanity check: rebuilt tx should not need a higher fee + let rebuilt_fee = calculate_relay_fee(estimate_p2pkh_tx_size( + tx.input.len(), + tx.output.len(), + )); + if actual_fee < rebuilt_fee { + tracing::warn!( + "Fee still insufficient after rebuild: {} < {}", + actual_fee, + rebuilt_fee + ); + } } } diff --git a/src/backend_task/identity/mod.rs b/src/backend_task/identity/mod.rs index cf4d03f55..b01660637 100644 --- a/src/backend_task/identity/mod.rs +++ b/src/backend_task/identity/mod.rs @@ -755,7 +755,7 @@ impl AppContext { "Fee mismatch: estimated {} vs actual {} (diff: {})", estimated_fee, actual_fee, - actual_fee as i64 - estimated_fee as i64 + actual_fee as i128 - estimated_fee as i128 ); } diff --git a/src/model/fee_estimation.rs b/src/model/fee_estimation.rs index def64a053..35323e1a4 100644 --- a/src/model/fee_estimation.rs +++ b/src/model/fee_estimation.rs @@ -717,14 +717,21 @@ impl PlatformFeeEstimator { // that is fixed, at which point the legacy paths can be removed. /// Estimate fee from an already-constructed state transition. - /// Returns 0 if estimation fails. + /// Returns 0 if estimation fails (masked by the `max(legacy, transition)` pattern). fn estimate_fee_from_transition( transition: &T, platform_version: &PlatformVersion, ) -> u64 { - transition - .calculate_min_required_fee(platform_version) - .unwrap_or(0) + match transition.calculate_min_required_fee(platform_version) { + Ok(fee) => fee, + Err(e) => { + tracing::debug!( + "Transition-based fee estimation failed for {}: {e}", + std::any::type_name::() + ); + 0 + } + } } /// Legacy platform transfer/withdrawal fee estimate (base + storage + 20% @@ -863,7 +870,10 @@ impl PlatformFeeEstimator { let public_keys = match IdentityPublicKeyInCreation::default_versioned(platform_version) { Ok(key) => std::iter::repeat_n(key, key_count).collect(), - Err(_) => Vec::new(), + Err(e) => { + tracing::debug!("Failed to create default key for fee estimation: {e}"); + Vec::new() + } }; let transition = @@ -1284,6 +1294,25 @@ mod tests { unified_topup >= legacy_topup, "unified topup ({unified_topup}) must be >= legacy ({legacy_topup})" ); + + // Platform → Core + let output_script = CoreScript::from_bytes(vec![0x76, 0xa9, 0x14]); + let legacy_withdrawal = estimator.legacy_platform_transfer_fee(1); + let unified_withdrawal = + estimator.estimate_platform_to_core_fee(pv, &inputs, &output_script); + assert!( + unified_withdrawal >= legacy_withdrawal, + "unified withdrawal ({unified_withdrawal}) must be >= legacy ({legacy_withdrawal})" + ); + + // Core → Platform + let legacy_funding = estimator.estimate_address_funding_from_asset_lock_duffs(1); + let unified_funding = + estimator.estimate_core_to_platform_fee(pv, &test_platform_address(1)); + assert!( + unified_funding >= legacy_funding, + "unified funding ({unified_funding}) must be >= legacy ({legacy_funding})" + ); } #[test] diff --git a/src/ui/identities/add_new_identity_screen/by_platform_address.rs b/src/ui/identities/add_new_identity_screen/by_platform_address.rs index 95fe5dd13..9c2429439 100644 --- a/src/ui/identities/add_new_identity_screen/by_platform_address.rs +++ b/src/ui/identities/add_new_identity_screen/by_platform_address.rs @@ -8,7 +8,10 @@ use crate::ui::identities::add_new_identity_screen::{ }; use crate::ui::theme::DashColors; use dash_sdk::dpp::address_funds::PlatformAddress; +use dash_sdk::dpp::balances::credits::Credits; +use dash_sdk::dpp::prelude::AddressNonce; use egui::{Color32, ComboBox, RichText, Ui}; +use std::collections::BTreeMap; /// Constants for credit/DASH conversion const CREDITS_PER_DUFF: u64 = 1000; @@ -153,18 +156,10 @@ impl AddNewIdentityScreen { // Calculate estimated fee for identity creation (needed for max amount calculation) let key_count = self.identity_keys.keys_input.len() + 1; // +1 for master key - let inputs: std::collections::BTreeMap< - PlatformAddress, - ( - dash_sdk::dpp::prelude::AddressNonce, - dash_sdk::dpp::balances::credits::Credits, - ), - > = self + let inputs: BTreeMap = self .selected_platform_address_for_funding .as_ref() - .map(|(platform_addr, amount)| { - std::collections::BTreeMap::from([(*platform_addr, (0, *amount))]) - }) + .map(|(platform_addr, amount)| BTreeMap::from([(*platform_addr, (0, *amount))])) .unwrap_or_default(); let estimated_fee = self .app_context From 04f994785a14831e3335ca4946b8555583079eed Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 24 Feb 2026 22:05:10 +0100 Subject: [PATCH 6/8] docs: update manual test scenarios for fee consolidation PR Replace the old asset-lock-fee-fix test scenarios (from PR #636) with comprehensive scenarios covering the full PR #651 scope: Core wallet payment fees, asset lock fees, Platform fee consolidation, UTXO removal consistency, and edge cases (SPV mode, dust threshold, large input count). Co-Authored-By: Claude Opus 4.6 --- .../manual-test-scenarios.md | 272 -------- .../manual-test-scenarios.md | 596 ++++++++++++++++++ 2 files changed, 596 insertions(+), 272 deletions(-) delete mode 100644 docs/ai-design/2026-02-24-asset-lock-fee-fix/manual-test-scenarios.md create mode 100644 docs/ai-design/2026-02-24-fee-consolidation/manual-test-scenarios.md diff --git a/docs/ai-design/2026-02-24-asset-lock-fee-fix/manual-test-scenarios.md b/docs/ai-design/2026-02-24-asset-lock-fee-fix/manual-test-scenarios.md deleted file mode 100644 index c24dca633..000000000 --- a/docs/ai-design/2026-02-24-asset-lock-fee-fix/manual-test-scenarios.md +++ /dev/null @@ -1,272 +0,0 @@ -# Manual Test Scenarios: Dynamic Asset Lock Fee Calculation - -**PR:** #636 (`zk-extract/asset-lock-fee-fix`) -**Date:** 2026-02-24 -**Component:** `src/model/wallet/asset_lock_transaction.rs` - -## Background - -This PR replaces the hardcoded 3000 duff fee in `asset_lock_transaction_from_private_key()` with a -dynamic fee calculated from the estimated transaction size: -- 10 bytes header + (inputs x 148 bytes) + (outputs x 34 bytes) + 60 bytes payload -- Minimum fee remains 3000 duffs (via `std::cmp::max`) -- Change output is recalculated after the real fee is determined -- Fee shortfall is handled gracefully when `allow_take_fee_from_amount` is set - -The change affects three public entry points: -- `registration_asset_lock_transaction()` -- identity registration -- `top_up_asset_lock_transaction()` -- identity top-up -- `generic_asset_lock_transaction()` -- platform address funding - -**Note:** `asset_lock_transaction_for_utxo_from_private_key()` (single-UTXO variant) is NOT changed -by this PR and still uses a hardcoded 3000 duff fee. - ---- - -## Preconditions (all scenarios) - -1. Dash Evo Tool is running and connected to **Testnet** (or a Devnet). -2. A wallet is loaded with known UTXO distribution (check wallet screen for balances). -3. The wallet has been synced recently (UTXOs are up to date). -4. Dash Core node is reachable and accepting transactions. - ---- - -## Scenario 1: Single-Input Asset Lock (fee equals minimum) - -**Purpose:** Verify that a transaction with 1 input still works correctly and the fee is at least -3000 duffs. With 1 input and 2 outputs: `10 + 148 + 68 + 60 = 286 bytes`, which is below 3000, so -the minimum fee of 3000 applies. - -### Preconditions -- Wallet has a single UTXO of at least 50,000 duffs (0.0005 DASH). - -### Steps -1. Navigate to the Identity screen. -2. Initiate a new identity registration with an amount of 10,000 duffs. -3. Confirm the transaction. - -### Expected Results -- The asset lock transaction is created and broadcast successfully. -- The transaction is accepted by the network (no "min relay fee not met" error). -- The wallet balance decreases by approximately 10,000 + 3,000 = 13,000 duffs. -- If the UTXO was larger than the required amount, a change output is returned to the wallet. -- The identity registration proceeds to completion (or awaits proof confirmation). - ---- - -## Scenario 2: Multiple-Input Asset Lock (fee scales with inputs) - -**Purpose:** Verify that when many UTXOs are consumed, the fee scales upward with the number of -inputs, exceeding the 3000 duff minimum. - -### Preconditions -- Wallet has many small UTXOs (e.g., 30 UTXOs of 5,000 duffs each = 150,000 duffs total). -- No single UTXO is large enough to cover the requested amount alone. - -### Steps -1. Navigate to the Identity screen. -2. Initiate a new identity registration with an amount of 100,000 duffs (0.001 DASH). -3. Confirm the transaction. - -### Expected Results -- The transaction consumes multiple UTXOs (approximately 21+ inputs needed for 100,000 + fee). -- With ~21 inputs: estimated size = `10 + (21 * 148) + (2 * 34) + 60 = 3246 bytes`, so fee should - be 3246 duffs (above the 3000 minimum). -- The transaction is accepted by the network without "min relay fee not met" errors. -- The wallet balance decreases by the requested amount plus the dynamically calculated fee. -- Previous behavior with a hardcoded 3000 fee would have been rejected by the network for large - input counts, so acceptance confirms the fix works. - ---- - -## Scenario 3: Tight Balance with Fee Deduction from Amount - -**Purpose:** Verify that when `allow_take_fee_from_amount` is true and the wallet balance barely -covers the amount, the fee is correctly deducted from the output amount. - -### Preconditions -- Wallet has exactly one UTXO of 10,000 duffs. - -### Steps -1. Navigate to the platform address funding screen (or identity top-up). -2. Initiate a funding/top-up for 10,000 duffs with the "deduct fee from amount" option enabled. -3. Confirm the transaction. - -### Expected Results -- The transaction is created successfully. -- The actual funded amount is 10,000 - 3,000 = 7,000 duffs (fee deducted from amount). -- No change output is present in the transaction. -- The transaction is accepted by the network. -- The wallet balance drops to 0. - ---- - -## Scenario 4: Insufficient Funds (no fee deduction allowed) - -**Purpose:** Verify that a clear error message is shown when the wallet cannot cover the amount -plus fee and fee deduction is not allowed. - -### Preconditions -- Wallet has exactly one UTXO of 10,000 duffs. - -### Steps -1. Navigate to the platform address funding screen. -2. Initiate a funding for 10,000 duffs with the "deduct fee from amount" option DISABLED. -3. Confirm the transaction. - -### Expected Results -- The operation fails with an error message similar to: - `"Insufficient funds: need 10000 + 3000 fee, have 10000"` -- The error message includes the specific amounts (requested, fee, available). -- No transaction is broadcast to the network. -- The wallet UTXOs remain unchanged. - ---- - -## Scenario 5: Insufficient Funds (fee deduction allowed but amount too small) - -**Purpose:** Verify that when the entire balance would be consumed by the fee alone, a clear error -is returned even when fee deduction is allowed. - -### Preconditions -- Wallet has exactly one UTXO of 2,500 duffs (less than the minimum 3000 fee). - -### Steps -1. Navigate to the identity registration screen. -2. Initiate a registration with an amount of 2,500 duffs, with fee deduction from amount enabled. -3. Confirm the transaction. - -### Expected Results -- The operation fails with `"Insufficient funds for transaction fee"` or the UTXO selection - returns None (displayed as an appropriate error in the UI). -- No transaction is broadcast. -- Wallet UTXOs remain unchanged. - ---- - -## Scenario 6: Change Output Presence and Correctness - -**Purpose:** Verify that the change output is correctly calculated after the dynamic fee is applied, -not the initial estimate. - -### Preconditions -- Wallet has a single UTXO of 100,000 duffs. - -### Steps -1. Navigate to the Identity screen. -2. Initiate a new identity registration with an amount of 50,000 duffs. -3. Confirm the transaction. - -### Expected Results -- The transaction has exactly 2 outputs: the burn output (50,000 duffs) and a change output. -- Change = 100,000 - 50,000 - fee. With 1 input, 2 outputs: fee = max(3000, 10+148+68+60) = 3000. -- Change should be 100,000 - 50,000 - 3,000 = 47,000 duffs. -- The change output goes to a wallet-controlled change address. -- After the transaction confirms, the wallet shows approximately 47,000 duffs remaining. - ---- - -## Scenario 7: No Change Output (exact amount consumed) - -**Purpose:** Verify that when inputs exactly equal amount + fee, no change output is created. - -### Preconditions -- Wallet has a single UTXO of exactly 53,000 duffs. - -### Steps -1. Navigate to the Identity screen. -2. Initiate a new identity registration with an amount of 50,000 duffs. -3. Confirm the transaction. - -### Expected Results -- The transaction has exactly 1 output (the burn output for 50,000 duffs). No change output. -- Fee = 3,000 duffs (1 input, 1 output: `10 + 148 + 34 + 60 = 252`, min 3000 applies). -- 53,000 - 50,000 - 3,000 = 0, so no change. -- The wallet balance drops to 0 after confirmation. - ---- - -## Scenario 8: Transaction Acceptance by Network (regression) - -**Purpose:** This is the core regression test -- confirm that transactions which previously failed -with "min relay fee not met" now succeed. - -### Preconditions -- Wallet has 50+ small UTXOs (e.g., from prior dust consolidation or faucet drips). - -### Steps -1. Attempt to register an identity or top up an identity using an amount that requires consuming - many UTXOs (e.g., 200,000 duffs spread across 50+ UTXOs of 4,000 duffs each). -2. Confirm the transaction. - -### Expected Results -- The transaction is broadcast successfully (no RPC error). -- The transaction is accepted into the mempool (no rejection). -- With 50 inputs: estimated size = `10 + (50 * 148) + (2 * 34) + 60 = 7538 bytes`. -- Fee should be 7,538 duffs (well above the old hardcoded 3,000). -- The identity registration or top-up completes normally after proof confirmation. - ---- - -## Scenario 9: Identity Top-Up with Dynamic Fee - -**Purpose:** Verify the fix works for identity top-up flows (not just registration). - -### Preconditions -- An identity already exists in the wallet. -- Wallet has sufficient balance (e.g., 100,000 duffs). - -### Steps -1. Navigate to the identity detail screen. -2. Initiate a top-up of 20,000 duffs. -3. Confirm the transaction. - -### Expected Results -- The asset lock transaction is created with a dynamically calculated fee. -- The transaction is accepted by the network. -- The identity balance increases by approximately 20,000 duffs (in credits). -- Wallet balance decreases by 20,000 + fee. - ---- - -## Scenario 10: Platform Address Funding with Dynamic Fee - -**Purpose:** Verify the fix works for the generic platform address funding flow. - -### Preconditions -- Wallet has sufficient balance (e.g., 100,000 duffs). -- A valid platform address is available to fund. - -### Steps -1. Navigate to the platform address funding screen. -2. Enter a destination platform address and amount of 30,000 duffs. -3. Choose "deduct fee from amount" = disabled. -4. Confirm the transaction. - -### Expected Results -- The asset lock transaction is created with a dynamically calculated fee. -- The transaction is accepted by the network. -- The destination platform address receives exactly 30,000 duffs worth of credits (minus platform - fees, but not minus Core tx fees). -- Wallet balance decreases by more than 30,000 duffs (amount + estimated platform fee + Core fee). - ---- - -## Edge Cases - -### E1: Very Large Number of Inputs -- If a wallet has 100+ tiny UTXOs and attempts to spend them all, the fee could exceed 15,000 duffs. - Verify the transaction is still accepted and the fee does not consume an unreasonable portion of - the amount. - -### E2: UTXO Selection Crosses Initial Estimate Threshold -- The initial estimate of 3000 duffs is used for UTXO selection. If the real fee is higher and - causes a shortfall (total inputs < amount + real fee), the code should handle this via the - `allow_take_fee_from_amount` path or return an error. Verify no panic or silent data loss occurs. - - -### E3: Concurrent Asset Lock Creation -- If two asset lock transactions are created in quick succession (e.g., rapid identity registration - attempts), verify that UTXO double-spend is prevented and the second attempt either uses different - UTXOs or fails gracefully. diff --git a/docs/ai-design/2026-02-24-fee-consolidation/manual-test-scenarios.md b/docs/ai-design/2026-02-24-fee-consolidation/manual-test-scenarios.md new file mode 100644 index 000000000..2d5f36e97 --- /dev/null +++ b/docs/ai-design/2026-02-24-fee-consolidation/manual-test-scenarios.md @@ -0,0 +1,596 @@ +# Manual Test Scenarios: Consolidated Core and Platform Fee Estimation + +**PR:** #651 (`fix/consolidate-core-fee-estimation`) +**Date:** 2026-02-24 +**Components:** +- `src/model/fee_estimation.rs` (unified fee utilities) +- `src/backend_task/core/mod.rs` (RPC wallet payment with iterative fee, SPV guard) +- `src/model/wallet/asset_lock_transaction.rs` (asset lock with calculate_relay_fee) +- `src/ui/wallets/send_screen.rs` (Platform send with unified fee estimator) +- `src/backend_task/identity/register_identity.rs` (consolidated UTXO removal) +- `src/backend_task/identity/top_up_identity.rs` (consolidated UTXO removal) + +## Background + +PR #651 consolidates fee estimation across the entire app: + +1. **Core tx fee estimation** — RPC wallet payment now uses iterative fee estimation + (`estimate_p2pkh_tx_size` + `calculate_relay_fee`) instead of a hardcoded 1,000 duff fee. + If the actual tx size after building exceeds the initial 5-input estimate, the fee is + recalculated and the tx is rebuilt. SPV mode is guarded: fee re-estimation returns an error + instead of silently losing UTXOs. + +2. **Asset lock fee estimation** — the multi-input path (`asset_lock_transaction_from_private_key`) + now uses `calculate_relay_fee(estimate_asset_lock_tx_size(...))` plus a retry loop. The + single-UTXO path (`asset_lock_transaction_for_utxo_from_private_key`) also uses + `calculate_asset_lock_fee` for consistency (replacing a former hardcoded 3,000 duff fee). + +3. **Platform fee consolidation** — Platform-to-Platform transfers, Platform-to-Core + withdrawals, Core-to-Platform asset locks, and identity creation/top-up from platform + addresses all use the unified `PlatformFeeEstimator` with `max(legacy, transition)` logic. + +4. **UTXO removal consistency** — `FundWithUtxo` paths in identity registration and top-up + now call `remove_selected_utxos()` via the shared helper instead of manual `retain()`. + +--- + +## Preconditions (all scenarios) + +1. Dash Evo Tool is running and connected to **Testnet** (or a Devnet). +2. A wallet is loaded with known UTXO distribution (check wallet screen for balances). +3. The wallet has been synced recently (UTXOs are up to date). +4. Dash Core node is reachable and accepting transactions. + +--- + +## Section A: Core Wallet Payment Fee Estimation + +These scenarios verify the RPC wallet payment path which previously used a hardcoded 1,000 duff +fee and now uses iterative size-based estimation. + +### A1: Single-Recipient Payment — Fee Covers Actual Size + +**Purpose:** Verify that a normal send from the Core wallet uses a dynamically calculated fee +(not a hardcoded 1,000 duffs) and succeeds. + +#### Preconditions +- Core wallet has at least one UTXO of 100,000 duffs. + +#### Steps +1. Navigate to the Wallets screen. +2. Select a wallet and click Send (Core wallet payment). +3. Enter a valid Testnet recipient address and an amount of 50,000 duffs. +4. Leave "subtract fee from amount" unchecked. +5. Confirm the transaction. + +#### Expected Results +- The transaction broadcasts successfully. +- The fee deducted is based on the actual tx size (not a fixed 1,000 duffs). + With 1–2 inputs and 2 outputs: `8 + 148*n + 34*2 = ~224–372 bytes` → fee ~224–372 duffs. +- The wallet balance decreases by 50,000 + actual fee. +- No "min relay fee not met" rejection from the network. + +--- + +### A2: Payment with Many Inputs — Iterative Fee Recalculation + +**Purpose:** Verify that when the actual number of inputs exceeds the initial 5-input estimate, +the fee is recalculated and the tx is rebuilt with the correct fee. + +#### Preconditions +- Core wallet has 10+ small UTXOs (e.g., 10 UTXOs of 6,000 duffs each). +- No single UTXO covers the full amount + fee alone. + +#### Steps +1. Navigate to the Wallets screen → Send. +2. Enter a valid recipient address and an amount of 50,000 duffs. +3. Confirm the transaction. + +#### Expected Results +- The transaction consumes multiple UTXOs. +- The fee reflects the actual input count (> 5 inputs → fee may be recalculated in the second + pass). +- The transaction is accepted by the network without fee errors. +- The success dialog shows a valid txid. + +--- + +### A3: Subtract Fee from Amount + +**Purpose:** Verify that "subtract fee from amount" correctly deducts the dynamically computed +fee from the output, not a hardcoded value. + +#### Preconditions +- Core wallet has a single UTXO of 50,000 duffs. + +#### Steps +1. Navigate to the Wallets screen → Send. +2. Enter a valid recipient address and 50,000 duffs. +3. Check "subtract fee from amount". +4. Confirm. + +#### Expected Results +- The transaction broadcasts successfully. +- The recipient receives 50,000 − (dynamic fee) duffs, not 50,000 − 1,000 duffs (old hardcoded). +- The wallet balance drops to 0 (or near 0 if dust remains). + +--- + +### A4: Multiple Recipients + +**Purpose:** Verify that the fee estimate accounts for multiple output scripts. + +#### Preconditions +- Core wallet has sufficient balance (100,000+ duffs). + +#### Steps +1. Navigate to the Wallets screen → Send (advanced mode if available). +2. Add two distinct Testnet recipient addresses, each receiving 20,000 duffs. +3. Confirm the transaction. + +#### Expected Results +- The transaction has 3 outputs (2 recipients + 1 change). +- Fee is estimated as: `estimate_p2pkh_tx_size(inputs, 3)` × rate. +- Both recipients receive the exact amounts. Change is returned to wallet. +- Transaction accepted by the network. + +--- + +## Section B: Asset Lock Fee Estimation + +These scenarios cover the asset lock transaction path used for identity registration, top-up, +and generic platform address funding. + +### B1: Single-Input Asset Lock — Minimum Fee Applied + +**Purpose:** Verify a 1-input asset lock uses at least the 3,000 duff minimum fee. + +With 1 input and 2 outputs: `10 + 148 + 68 + 60 = 286 bytes` → relay fee ~286 duffs, but the +minimum of 3,000 duffs applies. + +#### Preconditions +- Wallet has a single UTXO of at least 50,000 duffs. + +#### Steps +1. Navigate to the Identity screen. +2. Initiate a new identity registration with 10,000 duffs. +3. Confirm. + +#### Expected Results +- The asset lock transaction is created and broadcast. +- Fee = 3,000 duffs (minimum, since size-derived fee is lower). +- Wallet balance decreases by 10,000 + 3,000 = 13,000 duffs. +- A change output is returned if the UTXO exceeds 13,000 duffs. + +--- + +### B2: Multi-Input Asset Lock — Fee Scales with Input Count + +**Purpose:** Verify that for many inputs, the dynamic fee exceeds the 3,000 minimum and the +transaction is still accepted. + +#### Preconditions +- Wallet has 30+ small UTXOs (e.g., 30 UTXOs of 5,000 duffs each). +- No single UTXO covers the full amount. + +#### Steps +1. Navigate to the Identity screen. +2. Initiate identity registration with 100,000 duffs. +3. Confirm. + +#### Expected Results +- Transaction consumes ~21+ UTXOs. +- With ~21 inputs and 2 outputs: `10 + (21×148) + (2×34) + 60 = 3,246 bytes` → fee 3,246 duffs. +- Fee exceeds the 3,000 minimum; network accepts the transaction. +- Wallet balance decreases by 100,000 + ~3,246 duffs. + +--- + +### B3: Asset Lock Fee Retry — Initial Estimate Insufficient + +**Purpose:** Verify the fee-retry loop picks up additional UTXOs when the initial minimum-fee +estimate was too low for the actual input count. + +#### Preconditions +- Wallet has many small UTXOs where the initial 3,000 duff fee estimate would require one fewer + UTXO than the real fee (edge case — requires careful setup, best confirmed via unit test or + with a wallet of ~30 UTXOs of just-over-100 duffs each). + +#### Steps +1. Initiate identity registration with an amount that triggers the retry (amount ~= sum of + available UTXOs minus fee). +2. Confirm. + +#### Expected Results +- The transaction succeeds; no "not enough spendable funds" error. +- The retry loop internally recalculates the fee and selects the correct UTXOs. + +--- + +### B4: Single-UTXO Identity Registration via UTXO Selection + +**Purpose:** Verify the single-UTXO asset lock path (selected from the wallet's UTXO list) uses +the fee calculation helper rather than a hardcoded value, and removes the UTXO correctly. + +#### Preconditions +- Wallet has a known UTXO (visible in the wallet UTXO list). + +#### Steps +1. Navigate to the wallet UTXO list. +2. Select one UTXO and choose "Register Identity with this UTXO". +3. Confirm. + +#### Expected Results +- The asset lock transaction is created with fee = `calculate_asset_lock_fee(utxo.value, ...)`. +- The transaction is broadcast and accepted. +- The UTXO is removed from the wallet UTXO list after the transaction (not before or on failure). +- The identity registration proceeds. + +--- + +### B5: Single-UTXO Identity Top-Up via UTXO Selection + +**Purpose:** Same as B4 but for identity top-up. + +#### Preconditions +- An existing identity is registered in the wallet. +- Wallet has a known UTXO. + +#### Steps +1. Navigate to the wallet UTXO list. +2. Select a UTXO and choose "Top Up Identity with this UTXO". +3. Confirm. + +#### Expected Results +- Asset lock is created and broadcast. +- UTXO is removed from the wallet after confirmation. +- Identity balance increases by (UTXO value − fee) × credits_per_duff. + +--- + +### B6: Tight Balance — Fee Deducted from Amount + +**Purpose:** Verify that when the wallet barely covers the amount, the fee is deducted from the +output rather than failing, when the option is enabled. + +#### Preconditions +- Wallet has exactly one UTXO of 10,000 duffs. + +#### Steps +1. Navigate to platform address funding or identity top-up. +2. Set amount to 10,000 duffs with "deduct fee from amount" enabled. +3. Confirm. + +#### Expected Results +- Transaction succeeds; actual credited amount = 10,000 − 3,000 = 7,000 duffs. +- No change output. Wallet balance drops to 0. + +--- + +### B7: Insufficient Funds — Clear Error Message + +**Purpose:** Verify a clear error is shown when the wallet cannot cover the amount + fee and fee +deduction is disabled. + +#### Preconditions +- Wallet has exactly one UTXO of 10,000 duffs. + +#### Steps +1. Navigate to platform address funding. +2. Set amount to 10,000 duffs with "deduct fee from amount" DISABLED. +3. Confirm. + +#### Expected Results +- Error message includes the specific amounts: `"Insufficient funds: need 10000 + 3000 fee, have 10000"` (or similar). +- No transaction broadcast. Wallet UTXOs unchanged. + +--- + +## Section C: Platform Fee Consolidation + +These scenarios verify that Platform-side operations use the unified `PlatformFeeEstimator` +with `max(legacy, transition)` fee logic. + +### C1: Platform-to-Platform Transfer — Fee Display and Deduction + +**Purpose:** Verify that the Send screen correctly estimates and displays the Platform fee for +a Platform-to-Platform transfer, using the unified estimator. + +#### Preconditions +- Wallet has at least one platform address with a balance of 500,000+ credits. +- A second valid platform address is available as destination. + +#### Steps +1. Navigate to Wallets → Send. +2. Select "Platform Addresses" as the source. +3. Enter a destination platform address and an amount of 200,000 credits. +4. Observe the estimated fee shown in the UI before confirming. +5. Confirm the transfer. + +#### Expected Results +- The estimated fee is shown in the UI before submission. +- The fee is computed as `max(legacy_estimate, transition_based_estimate)` — the higher of the + two estimators is used. +- After the transition completes, the source address balance decreases by amount + fee. +- The destination address balance increases by the transferred amount. +- No "insufficient funds" or fee-related errors. + +--- + +### C2: Platform-to-Core Withdrawal — Unified Fee + +**Purpose:** Verify that a Platform-to-Core withdrawal uses the unified fee estimator. + +#### Preconditions +- Wallet has a platform address with at least 1,000,000 credits (to cover withdrawal fee). +- A valid Core wallet address is available for the destination. + +#### Steps +1. Navigate to Wallets → Send (or the dedicated withdrawal screen). +2. Select a Platform address as source and a Core wallet address as destination. +3. Enter a withdrawal amount. +4. Note the displayed fee estimate. +5. Confirm the withdrawal. + +#### Expected Results +- The fee estimate uses the unified `PlatformFeeEstimator` (withdrawal transition fee). +- The transition is submitted successfully. +- After finality, the Core wallet receives the withdrawal amount (minus fee). +- The platform address balance decreases by the amount + fee. + +--- + +### C3: Core-to-Platform Address Funding — Asset Lock with Platform Fee + +**Purpose:** Verify that funding a platform address from Core wallet UTXOs correctly estimates +and includes the Platform fee in the asset lock amount. + +#### Preconditions +- Core wallet has at least 100,000 duffs. +- A destination platform address is available. + +#### Steps +1. Navigate to Wallets → Fund Platform Address (or the platform funding screen). +2. Enter a destination platform address and an amount of 50,000 duffs. +3. Leave "deduct fee from amount" unchecked. +4. Confirm. + +#### Expected Results +- The asset lock amount is `50,000 + estimated_platform_fee_duffs` (fee paid from extra wallet + balance, not deducted from the recipient amount). +- The destination platform address receives ~50,000 duffs worth of credits. +- The Core wallet balance decreases by more than 50,000 duffs (amount + Core fee + Platform fee). +- The transaction is accepted by the network. + +--- + +### C4: Core-to-Platform Address Funding — Fee Deducted from Amount + +**Purpose:** Verify that when "deduct fee from amount" is enabled, the recipient receives less +than the nominal amount but the Core fee is correctly reflected. + +#### Preconditions +- Core wallet has at least 50,000 duffs. + +#### Steps +1. Navigate to Wallets → Fund Platform Address. +2. Enter a destination and 50,000 duffs with "deduct fee from amount" enabled. +3. Confirm. + +#### Expected Results +- The asset lock amount is 50,000 duffs (no Platform fee added to the lock). +- The Core fee is deducted from the amount, so the recipient receives 50,000 − fee credits. +- Wallet balance drops by exactly 50,000 duffs. + +--- + +### C5: Identity Creation from Platform Address — Unified Fee + +**Purpose:** Verify that creating an identity funded from a platform address uses the unified +Platform fee estimator. + +#### Preconditions +- Wallet has a platform address with sufficient balance (≥ 500,000 credits). + +#### Steps +1. Navigate to the Identity registration screen. +2. Select "Fund from Platform Address" (or equivalent). +3. Choose the platform address as the funding source. +4. Initiate registration. + +#### Expected Results +- The estimated fee shown before confirmation uses `max(legacy, transition)` logic. +- Identity creation transition is submitted successfully. +- The platform address balance decreases by the registration amount + fee. +- The new identity appears in the identity list. + +--- + +### C6: Identity Top-Up from Platform Address — Unified Fee + +**Purpose:** Verify that topping up an identity funded from a platform address uses the unified +fee estimator. + +#### Preconditions +- An existing identity is in the wallet. +- A platform address has at least 200,000 credits. + +#### Steps +1. Navigate to the identity detail screen. +2. Initiate a top-up using the platform address as the funding source. +3. Enter 100,000 credits as the top-up amount. +4. Confirm. + +#### Expected Results +- Fee is calculated using the unified `PlatformFeeEstimator`. +- The identity balance increases by approximately 100,000 credits (minus Platform fee). +- The platform address balance decreases accordingly. + +--- + +## Section D: UTXO Removal Consistency + +These scenarios verify that UTXOs are removed exactly once, after a transaction is fully built +and signed, and that the `remove_selected_utxos()` helper is used consistently. + +### D1: Identity Registration Removes UTXO Exactly Once + +**Purpose:** Verify that after a successful identity registration, the consumed UTXO is no +longer shown in the wallet, and that no duplicate or premature removal occurs. + +#### Preconditions +- Wallet has 2–3 UTXOs of known values. +- Record the UTXO list before the test. + +#### Steps +1. Navigate to the Identity registration screen. +2. Initiate registration using one of the known UTXOs. +3. Wait for the transaction to broadcast (or fail). +4. Navigate back to the wallet UTXO list. + +#### Expected Results (success path) +- The consumed UTXO(s) are no longer in the list. +- Remaining UTXOs are intact. +- If a change output was created, the new UTXO appears after the wallet refreshes. + +#### Expected Results (failure path — e.g., network error before broadcast) +- No UTXOs are removed from the wallet list. +- The wallet state is unchanged. + +--- + +### D2: Identity Top-Up Removes UTXO Exactly Once + +**Purpose:** Same as D1 but for identity top-up. + +#### Preconditions +- An existing identity is in the wallet. +- Wallet has 2–3 known UTXOs. + +#### Steps +1. Navigate to the identity detail screen → Top Up. +2. Initiate top-up using the wallet's UTXOs. +3. Wait for broadcast. +4. Check the UTXO list. + +#### Expected Results (success path) +- Consumed UTXOs are removed from the list. +- Remaining UTXOs are intact. + +#### Expected Results (failure path) +- UTXOs remain in the wallet list; no phantom removal. + +--- + +### D3: No Double-Spend on Rapid Consecutive Operations + +**Purpose:** Verify that if two operations are initiated in quick succession, the second does +not attempt to spend UTXOs already consumed by the first. + +#### Preconditions +- Wallet has exactly 2 UTXOs of similar value. + +#### Steps +1. Initiate identity registration (operation A) — do not wait for confirmation. +2. Immediately initiate a wallet send (operation B) for a different amount. +3. Observe both operations. + +#### Expected Results +- Operation A and B each use distinct UTXOs (no overlap). +- Both succeed, or the second fails gracefully with "insufficient funds" if UTXOs are exhausted. +- No panic, no silent UTXO duplication. + +--- + +## Section E: Edge Cases + +### E1: SPV Mode — Fee Re-Estimation Error + +**Purpose:** Verify that in SPV mode, if the initial fee estimate is too low and UTXOs cannot +be reloaded, an error is returned rather than silently losing UTXOs or panicking. + +#### Preconditions +- App is running in SPV mode (no Dash Core node configured). +- Wallet has UTXOs loaded via SPV sync. + +#### Steps +1. In SPV mode, attempt a wallet payment where the actual tx input count exceeds the 5-input + estimate (e.g., 8+ small UTXOs needed). +2. Observe the result. + +#### Expected Results +- If the initial fee was sufficient, the payment succeeds normally. +- If the initial fee was insufficient AND the UTXOs cannot be reloaded (SPV mode), + the error message is: + `"Fee re-estimation failed: cannot reload UTXOs in SPV mode"`. +- No UTXOs are lost from the wallet state. +- The UI displays the error in the message banner. + +--- + +### E2: SPV Mode — Override Fee Retry + +**Purpose:** Verify that a payment rejected by the network due to low fee can be retried with +an explicit fee override (if this UI flow is exposed). + +#### Preconditions +- App in SPV mode. Wallet has funds. + +#### Steps +1. Attempt a payment that is rejected by the network with "min relay fee not met". +2. If the UI shows a "retry with higher fee" option, confirm the retry. + +#### Expected Results +- The retry uses the `override_fee` field, bypassing re-estimation. +- The retried transaction is accepted by the network. +- Only the override fee amount is deducted, not a double fee. + +--- + +### E3: Poisoned Lock — Graceful Error (not a manual test) + +**Note:** The poisoned-lock fix (review finding FEE-005) ensures that `core_client.read()` and +`wallets.read()` return a descriptive error string instead of panicking when a lock is poisoned. +This scenario cannot be triggered manually in normal usage, but is verified by the fact that +all lock accesses now use `.map_err(|e| e.to_string())` or equivalent. Confirm via code review +that no `.unwrap()` or `.expect()` calls remain on these locks in the modified files. + +--- + +### E4: Very Large Number of Inputs + +**Purpose:** Confirm that transactions with 100+ inputs are still accepted after the fee +consolidation. + +#### Preconditions +- Wallet has 100+ dust/tiny UTXOs (e.g., from faucet drips). + +#### Steps +1. Attempt a payment or identity registration that consumes 100+ UTXOs. +2. Confirm. + +#### Expected Results +- With 100 inputs: estimated size ≈ `10 + (100×148) + (2×34) + 60 = 15,198 bytes`. + Fee ≈ 15,198 duffs. The transaction is accepted by the network. +- No overflow or panic from large input count arithmetic. + +--- + +### E5: Minimum-Balance Wallet — Dust Check Prevents Invalid Output + +**Purpose:** Verify that when the change amount falls below the dust threshold (546 duffs), +no dust change output is created. + +#### Preconditions +- Wallet has exactly one UTXO of 53,500 duffs. + +#### Steps +1. Attempt identity registration with 50,000 duffs. +2. With 1 input: fee = 3,000 duffs, change = 53,500 − 50,000 − 3,000 = 500 duffs (below 546). +3. Confirm. + +#### Expected Results +- No change output is created (500 duffs < 546 duff dust threshold). +- The 500 duffs are added to the fee instead. +- Transaction has 1 output (the burn output). Accepted by the network. +- Wallet balance drops to 0. From 2226f7e62d8087a03f9291916e13a5499eac41e8 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 24 Feb 2026 22:18:11 +0100 Subject: [PATCH 7/8] doc: test scenarios --- .../manual-test-scenarios.md | 58 +++++++++---------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/docs/ai-design/2026-02-24-fee-consolidation/manual-test-scenarios.md b/docs/ai-design/2026-02-24-fee-consolidation/manual-test-scenarios.md index 2d5f36e97..7accaa410 100644 --- a/docs/ai-design/2026-02-24-fee-consolidation/manual-test-scenarios.md +++ b/docs/ai-design/2026-02-24-fee-consolidation/manual-test-scenarios.md @@ -66,7 +66,7 @@ fee and now uses iterative size-based estimation. #### Expected Results - The transaction broadcasts successfully. - The fee deducted is based on the actual tx size (not a fixed 1,000 duffs). - With 1–2 inputs and 2 outputs: `8 + 148*n + 34*2 = ~224–372 bytes` → fee ~224–372 duffs. + With 1-2 inputs and 2 outputs: `8 + 148*n + 34*2 = ~224-372 bytes` -> fee ~224-372 duffs. - The wallet balance decreases by 50,000 + actual fee. - No "min relay fee not met" rejection from the network. @@ -82,13 +82,13 @@ the fee is recalculated and the tx is rebuilt with the correct fee. - No single UTXO covers the full amount + fee alone. #### Steps -1. Navigate to the Wallets screen → Send. +1. Navigate to the Wallets screen -> Send. 2. Enter a valid recipient address and an amount of 50,000 duffs. 3. Confirm the transaction. #### Expected Results - The transaction consumes multiple UTXOs. -- The fee reflects the actual input count (> 5 inputs → fee may be recalculated in the second +- The fee reflects the actual input count (> 5 inputs -> fee may be recalculated in the second pass). - The transaction is accepted by the network without fee errors. - The success dialog shows a valid txid. @@ -104,14 +104,14 @@ fee from the output, not a hardcoded value. - Core wallet has a single UTXO of 50,000 duffs. #### Steps -1. Navigate to the Wallets screen → Send. +1. Navigate to the Wallets screen -> Send. 2. Enter a valid recipient address and 50,000 duffs. 3. Check "subtract fee from amount". 4. Confirm. #### Expected Results - The transaction broadcasts successfully. -- The recipient receives 50,000 − (dynamic fee) duffs, not 50,000 − 1,000 duffs (old hardcoded). +- The recipient receives 50,000 - (dynamic fee) duffs, not 50,000 - 1,000 duffs (old hardcoded). - The wallet balance drops to 0 (or near 0 if dust remains). --- @@ -124,13 +124,13 @@ fee from the output, not a hardcoded value. - Core wallet has sufficient balance (100,000+ duffs). #### Steps -1. Navigate to the Wallets screen → Send (advanced mode if available). +1. Navigate to the Wallets screen -> Send (advanced mode if available). 2. Add two distinct Testnet recipient addresses, each receiving 20,000 duffs. 3. Confirm the transaction. #### Expected Results - The transaction has 3 outputs (2 recipients + 1 change). -- Fee is estimated as: `estimate_p2pkh_tx_size(inputs, 3)` × rate. +- Fee is estimated as: `estimate_p2pkh_tx_size(inputs, 3)` x rate. - Both recipients receive the exact amounts. Change is returned to wallet. - Transaction accepted by the network. @@ -145,7 +145,7 @@ and generic platform address funding. **Purpose:** Verify a 1-input asset lock uses at least the 3,000 duff minimum fee. -With 1 input and 2 outputs: `10 + 148 + 68 + 60 = 286 bytes` → relay fee ~286 duffs, but the +With 1 input and 2 outputs: `10 + 148 + 68 + 60 = 286 bytes` -> relay fee ~286 duffs, but the minimum of 3,000 duffs applies. #### Preconditions @@ -180,7 +180,7 @@ transaction is still accepted. #### Expected Results - Transaction consumes ~21+ UTXOs. -- With ~21 inputs and 2 outputs: `10 + (21×148) + (2×34) + 60 = 3,246 bytes` → fee 3,246 duffs. +- With ~21 inputs and 2 outputs: `10 + (21x148) + (2x34) + 60 = 3,246 bytes` -> fee 3,246 duffs. - Fee exceeds the 3,000 minimum; network accepts the transaction. - Wallet balance decreases by 100,000 + ~3,246 duffs. @@ -193,7 +193,7 @@ estimate was too low for the actual input count. #### Preconditions - Wallet has many small UTXOs where the initial 3,000 duff fee estimate would require one fewer - UTXO than the real fee (edge case — requires careful setup, best confirmed via unit test or + UTXO than the real fee (edge case -- requires careful setup, best confirmed via unit test or with a wallet of ~30 UTXOs of just-over-100 duffs each). #### Steps @@ -244,7 +244,7 @@ the fee calculation helper rather than a hardcoded value, and removes the UTXO c #### Expected Results - Asset lock is created and broadcast. - UTXO is removed from the wallet after confirmation. -- Identity balance increases by (UTXO value − fee) × credits_per_duff. +- Identity balance increases by (UTXO value - fee) x credits_per_duff. --- @@ -262,7 +262,7 @@ output rather than failing, when the option is enabled. 3. Confirm. #### Expected Results -- Transaction succeeds; actual credited amount = 10,000 − 3,000 = 7,000 duffs. +- Transaction succeeds; actual credited amount = 10,000 - 3,000 = 7,000 duffs. - No change output. Wallet balance drops to 0. --- @@ -281,7 +281,7 @@ deduction is disabled. 3. Confirm. #### Expected Results -- Error message includes the specific amounts: `"Insufficient funds: need 10000 + 3000 fee, have 10000"` (or similar). +- Error message includes the specific amounts: "Insufficient funds: need 10000 + 3000 fee, have 10000" (or similar). - No transaction broadcast. Wallet UTXOs unchanged. --- @@ -301,7 +301,7 @@ a Platform-to-Platform transfer, using the unified estimator. - A second valid platform address is available as destination. #### Steps -1. Navigate to Wallets → Send. +1. Navigate to Wallets -> Send. 2. Select "Platform Addresses" as the source. 3. Enter a destination platform address and an amount of 200,000 credits. 4. Observe the estimated fee shown in the UI before confirming. @@ -309,7 +309,7 @@ a Platform-to-Platform transfer, using the unified estimator. #### Expected Results - The estimated fee is shown in the UI before submission. -- The fee is computed as `max(legacy_estimate, transition_based_estimate)` — the higher of the +- The fee is computed as `max(legacy_estimate, transition_based_estimate)` -- the higher of the two estimators is used. - After the transition completes, the source address balance decreases by amount + fee. - The destination address balance increases by the transferred amount. @@ -326,7 +326,7 @@ a Platform-to-Platform transfer, using the unified estimator. - A valid Core wallet address is available for the destination. #### Steps -1. Navigate to Wallets → Send (or the dedicated withdrawal screen). +1. Navigate to Wallets -> Send (or the dedicated withdrawal screen). 2. Select a Platform address as source and a Core wallet address as destination. 3. Enter a withdrawal amount. 4. Note the displayed fee estimate. @@ -350,7 +350,7 @@ and includes the Platform fee in the asset lock amount. - A destination platform address is available. #### Steps -1. Navigate to Wallets → Fund Platform Address (or the platform funding screen). +1. Navigate to Wallets -> Fund Platform Address (or the platform funding screen). 2. Enter a destination platform address and an amount of 50,000 duffs. 3. Leave "deduct fee from amount" unchecked. 4. Confirm. @@ -373,13 +373,13 @@ than the nominal amount but the Core fee is correctly reflected. - Core wallet has at least 50,000 duffs. #### Steps -1. Navigate to Wallets → Fund Platform Address. +1. Navigate to Wallets -> Fund Platform Address. 2. Enter a destination and 50,000 duffs with "deduct fee from amount" enabled. 3. Confirm. #### Expected Results - The asset lock amount is 50,000 duffs (no Platform fee added to the lock). -- The Core fee is deducted from the amount, so the recipient receives 50,000 − fee credits. +- The Core fee is deducted from the amount, so the recipient receives 50,000 - fee credits. - Wallet balance drops by exactly 50,000 duffs. --- @@ -390,7 +390,7 @@ than the nominal amount but the Core fee is correctly reflected. Platform fee estimator. #### Preconditions -- Wallet has a platform address with sufficient balance (≥ 500,000 credits). +- Wallet has a platform address with sufficient balance (>= 500,000 credits). #### Steps 1. Navigate to the Identity registration screen. @@ -439,7 +439,7 @@ and signed, and that the `remove_selected_utxos()` helper is used consistently. longer shown in the wallet, and that no duplicate or premature removal occurs. #### Preconditions -- Wallet has 2–3 UTXOs of known values. +- Wallet has 2-3 UTXOs of known values. - Record the UTXO list before the test. #### Steps @@ -453,7 +453,7 @@ longer shown in the wallet, and that no duplicate or premature removal occurs. - Remaining UTXOs are intact. - If a change output was created, the new UTXO appears after the wallet refreshes. -#### Expected Results (failure path — e.g., network error before broadcast) +#### Expected Results (failure path -- e.g., network error before broadcast) - No UTXOs are removed from the wallet list. - The wallet state is unchanged. @@ -465,10 +465,10 @@ longer shown in the wallet, and that no duplicate or premature removal occurs. #### Preconditions - An existing identity is in the wallet. -- Wallet has 2–3 known UTXOs. +- Wallet has 2-3 known UTXOs. #### Steps -1. Navigate to the identity detail screen → Top Up. +1. Navigate to the identity detail screen -> Top Up. 2. Initiate top-up using the wallet's UTXOs. 3. Wait for broadcast. 4. Check the UTXO list. @@ -491,7 +491,7 @@ not attempt to spend UTXOs already consumed by the first. - Wallet has exactly 2 UTXOs of similar value. #### Steps -1. Initiate identity registration (operation A) — do not wait for confirmation. +1. Initiate identity registration (operation A) -- do not wait for confirmation. 2. Immediately initiate a wallet send (operation B) for a different amount. 3. Observe both operations. @@ -522,7 +522,7 @@ be reloaded, an error is returned rather than silently losing UTXOs or panicking - If the initial fee was sufficient, the payment succeeds normally. - If the initial fee was insufficient AND the UTXOs cannot be reloaded (SPV mode), the error message is: - `"Fee re-estimation failed: cannot reload UTXOs in SPV mode"`. + "Fee re-estimation failed: cannot reload UTXOs in SPV mode". - No UTXOs are lost from the wallet state. - The UI displays the error in the message banner. @@ -570,8 +570,8 @@ consolidation. 2. Confirm. #### Expected Results -- With 100 inputs: estimated size ≈ `10 + (100×148) + (2×34) + 60 = 15,198 bytes`. - Fee ≈ 15,198 duffs. The transaction is accepted by the network. +- With 100 inputs: estimated size ~= `10 + (100x148) + (2x34) + 60 = 15,198 bytes`. + Fee ~= 15,198 duffs. The transaction is accepted by the network. - No overflow or panic from large input count arithmetic. --- @@ -586,7 +586,7 @@ no dust change output is created. #### Steps 1. Attempt identity registration with 50,000 duffs. -2. With 1 input: fee = 3,000 duffs, change = 53,500 − 50,000 − 3,000 = 500 duffs (below 546). +2. With 1 input: fee = 3,000 duffs, change = 53,500 - 50,000 - 3,000 = 500 duffs (below 546). 3. Confirm. #### Expected Results From aa259e68af6334fb8bbb5269fed26bf3c1338912 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 24 Feb 2026 22:26:50 +0100 Subject: [PATCH 8/8] fix(wallet): subtract-fee-from-amount checkbox now works with sufficient funds The "subtract fee from amount" checkbox in core-to-core sends was silently ignored when the wallet had enough funds to cover amount + fee. The fee subtraction logic only activated in the edge case where UTXOs covered the amount but not amount + fee (change_option.is_none()). Now when the flag is set, UTXOs are selected for just the amount (fee=0) and the fee is always deducted proportionally from recipient outputs, regardless of wallet balance. Co-Authored-By: Claude Opus 4.6 --- .../manual-test-scenarios.md | 34 ++++- src/model/wallet/mod.rs | 135 ++++++++++++++++-- 2 files changed, 155 insertions(+), 14 deletions(-) diff --git a/docs/ai-design/2026-02-24-fee-consolidation/manual-test-scenarios.md b/docs/ai-design/2026-02-24-fee-consolidation/manual-test-scenarios.md index 7accaa410..bf7b4f364 100644 --- a/docs/ai-design/2026-02-24-fee-consolidation/manual-test-scenarios.md +++ b/docs/ai-design/2026-02-24-fee-consolidation/manual-test-scenarios.md @@ -95,10 +95,10 @@ the fee is recalculated and the tx is rebuilt with the correct fee. --- -### A3: Subtract Fee from Amount +### A3: Subtract Fee from Amount — Tight Balance (All Funds Sent) **Purpose:** Verify that "subtract fee from amount" correctly deducts the dynamically computed -fee from the output, not a hardcoded value. +fee from the output when the wallet only has enough to cover the amount, not amount + fee. #### Preconditions - Core wallet has a single UTXO of 50,000 duffs. @@ -111,8 +111,34 @@ fee from the output, not a hardcoded value. #### Expected Results - The transaction broadcasts successfully. -- The recipient receives 50,000 - (dynamic fee) duffs, not 50,000 - 1,000 duffs (old hardcoded). -- The wallet balance drops to 0 (or near 0 if dust remains). +- The recipient receives 50,000 - (dynamic fee) duffs. +- The wallet balance drops to 0 (no change output). + +--- + +### A3a: Subtract Fee from Amount — Normal Balance (Sufficient Funds) + +**Purpose:** Verify that "subtract fee from amount" works when the wallet has plenty of funds. +Previously this was broken: the checkbox was only effective in edge cases where the wallet +barely covered the amount. Now the fee is always deducted from the recipient's amount. + +#### Preconditions +- Core wallet has at least 200,000 duffs in UTXOs (well above the send amount). + +#### Steps +1. Navigate to the Wallets screen -> Send. +2. Enter a valid recipient address and 100,000 duffs. +3. Check "subtract fee from amount". +4. Note the estimated fee displayed (e.g., ~300 duffs for a 1-input tx). +5. Confirm. + +#### Expected Results +- The transaction broadcasts successfully. +- The recipient receives 100,000 - (dynamic fee) duffs (e.g., ~99,700 duffs). +- The wallet balance decreases by exactly 100,000 duffs (not 100,000 + fee). + Change = total_input - 100,000. +- Compare with a transaction WITHOUT the checkbox: in that case the recipient gets + the full 100,000 and the wallet balance drops by 100,000 + fee. --- diff --git a/src/model/wallet/mod.rs b/src/model/wallet/mod.rs index 8128fda22..e06b81e8d 100644 --- a/src/model/wallet/mod.rs +++ b/src/model/wallet/mod.rs @@ -1691,19 +1691,25 @@ impl Wallet { // Select UTXOs without removing them yet — UTXOs are only removed after // the transaction is fully built and signed, so that a failure at any later // step cannot permanently drop UTXOs from the wallet. - let (utxos, change_option) = self - .select_unspent_utxos_for(total_amount, fee, subtract_fee_from_amount) - .ok_or_else(|| "Insufficient funds".to_string())?; + // + // When subtract_fee_from_amount is true the fee is taken from the + // recipient amount(s), so we only need UTXOs covering total_amount + // (not total_amount + fee). + let (utxos, change_option) = if subtract_fee_from_amount { + self.select_unspent_utxos_for(total_amount, 0, false) + .ok_or_else(|| "Insufficient funds".to_string())? + } else { + self.select_unspent_utxos_for(total_amount, fee, false) + .ok_or_else(|| "Insufficient funds".to_string())? + }; // Build outputs for each recipient - let mut outputs: Vec = if change_option.is_none() && subtract_fee_from_amount { - // If we're subtracting fee and using all funds, we need to reduce recipient amounts proportionally - let total_input: u64 = utxos.values().map(|(tx_out, _)| tx_out.value).sum(); - let available_after_fee = total_input + let mut outputs: Vec = if subtract_fee_from_amount { + // Distribute the fee reduction proportionally across recipients. + // Each recipient receives: amount - (amount / total_amount) * fee. + let available_after_fee = total_amount .checked_sub(fee) - .ok_or_else(|| "Fee exceeds available amount".to_string())?; - - // Distribute the reduction proportionally across recipients + .ok_or_else(|| "Fee exceeds send amount".to_string())?; let reduction_ratio = available_after_fee as f64 / total_amount as f64; recipients @@ -2768,6 +2774,115 @@ mod tests { assert!(change.is_none()); } + // ======================================================================== + // build_multi_recipient_payment_transaction tests + // ======================================================================== + + /// Helper: create a test wallet with a BIP44-derived address and a single + /// UTXO of the given value. The address is registered in `known_addresses` + /// so that `private_key_for_address` can sign for it. + fn test_wallet_with_bip44_utxo(value: u64) -> (Wallet, Address) { + let mut wallet = test_wallet(); + let network = Network::Testnet; + let secp = Secp256k1::new(); + + // Derive the first BIP44 receive address: m/44'/1'/0'/0/0 + let extension = DerivationPath::from( + [ + ChildNumber::Normal { index: 0 }, + ChildNumber::Normal { index: 0 }, + ] + .as_slice(), + ); + let pubkey = wallet + .master_bip44_ecdsa_extended_public_key + .derive_pub(&secp, &extension) + .expect("bip44 derivation") + .to_pub(); + let address = Address::p2pkh(&pubkey, network); + + let full_path = DerivationPath::from(vec![ + ChildNumber::Hardened { index: 44 }, + ChildNumber::Hardened { index: 1 }, + ChildNumber::Hardened { index: 0 }, + ChildNumber::Normal { index: 0 }, + ChildNumber::Normal { index: 0 }, + ]); + wallet.known_addresses.insert(address.clone(), full_path); + + add_utxo(&mut wallet, &address, 1, 0, value); + (wallet, address) + } + + #[test] + fn test_subtract_fee_from_amount_with_sufficient_funds() { + let (mut wallet, _utxo_addr) = test_wallet_with_bip44_utxo(200_000); + let recipient = test_address(10); + let send_amount = 100_000u64; + let fee = 10_000u64; + + // With subtract_fee_from_amount=true, recipient should receive + // send_amount - fee, and the fee comes from the recipient share. + let tx = wallet + .build_multi_recipient_payment_transaction( + Network::Testnet, + &[(recipient.clone(), send_amount)], + fee, + true, // subtract_fee_from_amount + None, + ) + .expect("should build tx"); + + let recipient_output = tx + .output + .iter() + .find(|o| o.script_pubkey == recipient.script_pubkey()) + .expect("should have recipient output"); + assert_eq!( + recipient_output.value, + send_amount - fee, + "recipient should receive amount minus fee" + ); + + // Implied fee must equal the requested fee. + let total_input = 200_000u64; + let total_output: u64 = tx.output.iter().map(|o| o.value).sum(); + assert_eq!(total_input - total_output, fee, "implied fee must match"); + } + + #[test] + fn test_no_subtract_fee_sends_full_amount() { + let (mut wallet, _utxo_addr) = test_wallet_with_bip44_utxo(200_000); + let recipient = test_address(10); + let send_amount = 100_000u64; + let fee = 10_000u64; + + // With subtract_fee_from_amount=false, recipient gets the full amount. + let tx = wallet + .build_multi_recipient_payment_transaction( + Network::Testnet, + &[(recipient.clone(), send_amount)], + fee, + false, // subtract_fee_from_amount + None, + ) + .expect("should build tx"); + + let recipient_output = tx + .output + .iter() + .find(|o| o.script_pubkey == recipient.script_pubkey()) + .expect("should have recipient output"); + assert_eq!( + recipient_output.value, send_amount, + "recipient should receive full amount" + ); + + let total_input = 200_000u64; + let total_output: u64 = tx.output.iter().map(|o| o.value).sum(); + assert_eq!(total_input - total_output, fee, "implied fee must match"); + } + /// Helper: register a wallet address in the test database so that /// `update_address_balance` can find the row. fn register_test_address(db: &Database, wallet: &Wallet, address: &Address) {