From 2cd22fd9341d0184987b1a4478e7ed0a85c73373 Mon Sep 17 00:00:00 2001 From: Samuel Onoja Date: Fri, 8 Mar 2024 13:09:58 +0100 Subject: [PATCH 01/16] save dev state --- mm2src/coins/eth.rs | 4 + mm2src/coins/eth/eth_tests.rs | 4 + mm2src/coins/lightning.rs | 3 + mm2src/coins/lp_coins.rs | 1 + mm2src/coins/qrc20.rs | 44 +-- mm2src/coins/qrc20/qrc20_tests.rs | 52 +++- .../rpc_command/lightning/open_channel.rs | 6 +- mm2src/coins/tendermint/tendermint_coin.rs | 2 + mm2src/coins/utxo.rs | 38 ++- mm2src/coins/utxo/bch.rs | 6 +- mm2src/coins/utxo/qtum.rs | 6 +- mm2src/coins/utxo/slp.rs | 24 +- mm2src/coins/utxo/utxo_common.rs | 264 ++++++++---------- mm2src/coins/utxo/utxo_standard.rs | 6 +- mm2src/coins/utxo/utxo_tests.rs | 6 +- mm2src/coins/utxo/utxo_withdraw.rs | 10 +- mm2src/coins/z_coin.rs | 27 +- mm2src/mm2_main/src/lp_swap.rs | 1 + mm2src/mm2_main/src/lp_swap/taker_swap.rs | 1 + mm2src/mm2_main/src/lp_swap/taker_swap_v2.rs | 1 + 20 files changed, 275 insertions(+), 231 deletions(-) diff --git a/mm2src/coins/eth.rs b/mm2src/coins/eth.rs index 83a7f9d77f..c279f8b22b 100644 --- a/mm2src/coins/eth.rs +++ b/mm2src/coins/eth.rs @@ -5223,6 +5223,7 @@ impl MmCoin for EthCoin { coin: fee_coin.into(), amount: try_s!(u256_to_big_decimal(fee, ETH_DECIMALS)).into(), paid_from_trading_vol: false, + tx_size: None, }) }), ) @@ -5280,6 +5281,7 @@ impl MmCoin for EthCoin { coin: fee_coin.into(), amount: amount.into(), paid_from_trading_vol: false, + tx_size: None, }) } @@ -5299,6 +5301,7 @@ impl MmCoin for EthCoin { coin: fee_coin.into(), amount: amount.into(), paid_from_trading_vol: false, + tx_size: None, }) }; Box::new(fut.boxed().compat()) @@ -5347,6 +5350,7 @@ impl MmCoin for EthCoin { coin: fee_coin.into(), amount: amount.into(), paid_from_trading_vol: false, + tx_size: None, }) } diff --git a/mm2src/coins/eth/eth_tests.rs b/mm2src/coins/eth/eth_tests.rs index c7e32a78ce..d3bc305f16 100644 --- a/mm2src/coins/eth/eth_tests.rs +++ b/mm2src/coins/eth/eth_tests.rs @@ -627,6 +627,7 @@ fn get_sender_trade_preimage() { coin: "ETH".to_owned(), amount: amount.into(), paid_from_trading_vol: false, + tx_size: None, } } @@ -682,6 +683,7 @@ fn get_erc20_sender_trade_preimage() { coin: "ETH".to_owned(), amount: amount.into(), paid_from_trading_vol: false, + tx_size: None, } } @@ -754,6 +756,7 @@ fn get_receiver_trade_preimage() { coin: "ETH".to_owned(), amount: amount.into(), paid_from_trading_vol: false, + tx_size: None, }; let actual = coin @@ -778,6 +781,7 @@ fn test_get_fee_to_send_taker_fee() { coin: "ETH".to_owned(), amount: amount.into(), paid_from_trading_vol: false, + tx_size: None, }; let dex_fee_amount = u256_to_big_decimal(DEX_FEE_AMOUNT.into(), 18).expect("!u256_to_big_decimal"); diff --git a/mm2src/coins/lightning.rs b/mm2src/coins/lightning.rs index 4698a595ac..f584f9181c 100644 --- a/mm2src/coins/lightning.rs +++ b/mm2src/coins/lightning.rs @@ -1335,6 +1335,7 @@ impl MmCoin for LightningCoin { coin: self.ticker().to_owned(), amount: Default::default(), paid_from_trading_vol: false, + tx_size: None, }) } @@ -1344,6 +1345,7 @@ impl MmCoin for LightningCoin { coin: self.ticker().to_owned(), amount: Default::default(), paid_from_trading_vol: false, + tx_size: None, })) } @@ -1357,6 +1359,7 @@ impl MmCoin for LightningCoin { coin: self.ticker().to_owned(), amount: Default::default(), paid_from_trading_vol: false, + tx_size: None, }) } diff --git a/mm2src/coins/lp_coins.rs b/mm2src/coins/lp_coins.rs index 5179a5db80..46a1c1f714 100644 --- a/mm2src/coins/lp_coins.rs +++ b/mm2src/coins/lp_coins.rs @@ -2166,6 +2166,7 @@ pub struct TradeFee { pub coin: String, pub amount: MmNumber, pub paid_from_trading_vol: bool, + pub tx_size: Option, } #[derive(Clone, Debug, Default, PartialEq, PartialOrd, Serialize)] diff --git a/mm2src/coins/qrc20.rs b/mm2src/coins/qrc20.rs index 806fe3c0e2..cddb186629 100644 --- a/mm2src/coins/qrc20.rs +++ b/mm2src/coins/qrc20.rs @@ -10,12 +10,13 @@ use crate::utxo::tx_cache::{UtxoVerboseCacheOps, UtxoVerboseCacheShared}; use crate::utxo::utxo_builder::{UtxoCoinBuildError, UtxoCoinBuildResult, UtxoCoinBuilder, UtxoCoinBuilderCommonOps, UtxoFieldsWithGlobalHDBuilder, UtxoFieldsWithHardwareWalletBuilder, UtxoFieldsWithIguanaSecretBuilder}; -use crate::utxo::utxo_common::{self, big_decimal_from_sat, check_all_utxo_inputs_signed_by_pub, UtxoTxBuilder}; -use crate::utxo::{qtum, ActualTxFee, AdditionalTxData, AddrFromStrError, BroadcastTxErr, FeePolicy, GenerateTxError, - GetUtxoListOps, HistoryUtxoTx, HistoryUtxoTxMap, MatureUnspentList, RecentlySpentOutPointsGuard, - UnsupportedAddr, UtxoActivationParams, UtxoAddressFormat, UtxoCoinFields, UtxoCommonOps, - UtxoFromLegacyReqErr, UtxoTx, UtxoTxBroadcastOps, UtxoTxGenerationOps, VerboseTransactionFrom, - UTXO_LOCK}; +use crate::utxo::utxo_common::{self, big_decimal_from_sat, check_all_utxo_inputs_signed_by_pub, + PreImageTradeFeeResult, UtxoTxBuilder}; +use crate::utxo::{qtum, AdditionalTxData, AddrFromStrError, BroadcastTxErr, FeePolicy, GenerateTxError, + GetUtxoListOps, HistoryUtxoTx, HistoryUtxoTxMap, HtlcSpendFeeRes, MatureUnspentList, + RecentlySpentOutPointsGuard, UnsupportedAddr, UtxoActivationParams, UtxoAddressFormat, + UtxoCoinFields, UtxoCommonOps, UtxoFromLegacyReqErr, UtxoTx, UtxoTxBroadcastOps, + UtxoTxGenerationOps, VerboseTransactionFrom, UTXO_LOCK}; use crate::{BalanceError, BalanceFut, CheckIfMyPaymentSentArgs, CoinBalance, CoinFutSpawner, ConfirmPaymentInput, DexFee, FeeApproxStage, FoundSwapTxSpend, HistorySyncState, IguanaPrivKey, MakerSwapTakerCoin, MarketCoinOps, MmCoin, MmCoinEnum, NegotiateSwapContractAddrErr, PaymentInstructionArgs, @@ -492,9 +493,7 @@ impl Qrc20Coin { /// `gas_fee` should be calculated by: gas_limit * gas_price * (count of contract calls), /// or should be sum of gas fee of all contract calls. pub async fn get_qrc20_tx_fee(&self, gas_fee: u64) -> Result { - match try_s!(self.get_tx_fee().await) { - ActualTxFee::Dynamic(amount) | ActualTxFee::FixedPerKb(amount) => Ok(amount + gas_fee), - } + Ok(try_s!(self.get_tx_fee_per_kb().await) + gas_fee) } /// Generate and send a transaction with the specified UTXO outputs. @@ -586,7 +585,7 @@ impl Qrc20Coin { &self, contract_outputs: Vec, stage: &FeeApproxStage, - ) -> TradePreimageResult { + ) -> TradePreimageResult { let decimals = self.as_ref().decimals; let mut gas_fee = 0; let mut outputs = Vec::with_capacity(contract_outputs.len()); @@ -599,7 +598,11 @@ impl Qrc20Coin { UtxoCommonOps::preimage_trade_fee_required_to_send_outputs(self, outputs, fee_policy, Some(gas_fee), stage) .await?; let gas_fee = big_decimal_from_sat(gas_fee as i64, decimals); - Ok(miner_fee + gas_fee) + println!("SIZE: {:?}", &miner_fee.tx_size); + Ok(PreImageTradeFeeResult { + tx_size: miner_fee.tx_size, + fee: miner_fee.fee + gas_fee, + }) } } @@ -616,7 +619,7 @@ impl UtxoTxBroadcastOps for Qrc20Coin { #[cfg_attr(test, mockable)] impl UtxoTxGenerationOps for Qrc20Coin { /// Get only QTUM transaction fee. - async fn get_tx_fee(&self) -> UtxoRpcResult { utxo_common::get_tx_fee(&self.utxo).await } + async fn get_tx_fee_per_kb(&self) -> UtxoRpcResult { utxo_common::get_tx_fee_per_kb(&self.utxo).await } async fn calc_interest_if_required( &self, @@ -657,7 +660,7 @@ impl GetUtxoListOps for Qrc20Coin { #[async_trait] #[cfg_attr(test, mockable)] impl UtxoCommonOps for Qrc20Coin { - async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult { + async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult { utxo_common::get_htlc_spend_fee(self, tx_size, stage).await } @@ -722,7 +725,7 @@ impl UtxoCommonOps for Qrc20Coin { fee_policy: FeePolicy, gas_fee: Option, stage: &FeeApproxStage, - ) -> TradePreimageResult { + ) -> TradePreimageResult { utxo_common::preimage_trade_fee_required_to_send_outputs( self, self.platform_ticker(), @@ -1358,6 +1361,7 @@ impl MmCoin for Qrc20Coin { coin: selfi.platform.clone(), amount: big_decimal_from_sat(fee as i64, selfi.utxo.decimals).into(), paid_from_trading_vol: false, + tx_size: None, }) }; Box::new(fut.boxed().compat()) @@ -1405,11 +1409,14 @@ impl MmCoin for Qrc20Coin { .await? }; - let total_fee = erc20_payment_fee + sender_refund_fee; + let total_fee = erc20_payment_fee.fee + sender_refund_fee.fee; + let total_tx_size = + erc20_payment_fee.tx_size.unwrap_or_default() + sender_refund_fee.tx_size.unwrap_or_default(); Ok(TradeFee { coin: self.platform.clone(), amount: total_fee.into(), paid_from_trading_vol: false, + tx_size: Some(total_tx_size), }) } @@ -1430,10 +1437,12 @@ impl MmCoin for Qrc20Coin { let total_fee = selfi .preimage_trade_fee_required_to_send_outputs(vec![output], &stage) .await?; + Ok(TradeFee { coin: selfi.platform.clone(), - amount: total_fee.into(), + amount: total_fee.fee.into(), paid_from_trading_vol: false, + tx_size: total_fee.tx_size, }) }; Box::new(fut.boxed().compat()) @@ -1457,8 +1466,9 @@ impl MmCoin for Qrc20Coin { Ok(TradeFee { coin: self.platform.clone(), - amount: total_fee.into(), + amount: total_fee.fee.into(), paid_from_trading_vol: false, + tx_size: total_fee.tx_size, }) } diff --git a/mm2src/coins/qrc20/qrc20_tests.rs b/mm2src/coins/qrc20/qrc20_tests.rs index 912222d7bc..6e41b50b43 100644 --- a/mm2src/coins/qrc20/qrc20_tests.rs +++ b/mm2src/coins/qrc20/qrc20_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::utxo::KILO_BYTE; use crate::{DexFee, TxFeeDetails, WaitForHTLCTxSpendArgs}; use common::{block_on, wait_until_sec, DEX_FEE_ADDR_RAW_PUBKEY}; use crypto::Secp256k1Secret; @@ -57,8 +58,8 @@ pub fn qrc20_coin_for_test(priv_key: [u8; 32], fallback_swap: Option<&str>) -> ( (ctx, coin) } -fn check_tx_fee(coin: &Qrc20Coin, expected_tx_fee: ActualTxFee) { - let actual_tx_fee = block_on(coin.get_tx_fee()).unwrap(); +fn check_tx_fee(coin: &Qrc20Coin, expected_tx_fee: u64) { + let actual_tx_fee = block_on(coin.get_tx_fee_per_kb()).unwrap(); assert_eq!(actual_tx_fee, expected_tx_fee); } @@ -133,18 +134,36 @@ fn test_withdraw_impl_fee_details() { }; let tx_details = coin.withdraw(withdraw_req).wait().unwrap(); + let tx_size_kb = tx_details.tx_hex.into_vec().len() as f64 / KILO_BYTE; + // In transaction size calculations we assume the script sig size is (2 + MAX_DER_SIGNATURE_LEN + COMPRESSED_PUBKEY_LEN) or 107 bytes + // when in reality signatures can vary by 1 or 2 bytes because of possible zero padding of r and s values of the signature. + // This is why we test for a range of values here instead of a single value. The value we use in fees calculation is the + // highest possible value of 107 to ensure we don't underestimate the fee. + assert!( + (0.297..=0.299).contains(&tx_size_kb), + "Tx size in KB {} is not within the range [{}, {}]", + tx_size_kb, + 0.297, + 0.299 + ); + let tx_size_kb = 0.299; + let fee_per_kb = block_on(coin.get_tx_fee_per_kb()).unwrap() as f64; + let expected_miner_fee_sats = fee_per_kb * tx_size_kb; + let expected_miner_fee = big_decimal_from_sat(expected_miner_fee_sats as i64, coin.utxo.decimals); + let expected: Qrc20FeeDetails = json::from_value(json!({ "coin": "QTUM", // 1000 from satoshi, // where decimals = 8, // 1000 is fixed fee - "miner_fee": "0.00001", + "miner_fee": expected_miner_fee, "gas_limit": 2_500_000, "gas_price": 40, // (gas_limit * gas_price) from satoshi in Qtum "total_gas_fee": "1", })) .unwrap(); + println!("MINER_FEE {expected_miner_fee}"); assert_eq!(tx_details.fee_details, Some(TxFeeDetails::Qrc20(expected))); } @@ -758,7 +777,7 @@ fn test_get_trade_fee() { ]; let (_ctx, coin) = qrc20_coin_for_test(priv_key, None); // check if the coin's tx fee is expected - check_tx_fee(&coin, ActualTxFee::FixedPerKb(EXPECTED_TX_FEE as u64)); + check_tx_fee(&coin, EXPECTED_TX_FEE as u64); let actual_trade_fee = coin.get_trade_fee().wait().unwrap(); let expected_trade_fee_amount = big_decimal_from_sat( @@ -769,6 +788,7 @@ fn test_get_trade_fee() { coin: "QTUM".into(), amount: expected_trade_fee_amount.into(), paid_from_trading_vol: false, + tx_size: None, }; assert_eq!(actual_trade_fee, expected); } @@ -785,7 +805,7 @@ fn test_sender_trade_preimage_zero_allowance() { ]; let (_ctx, coin) = qrc20_coin_for_test(priv_key, None); // check if the coin's tx fee is expected - check_tx_fee(&coin, ActualTxFee::FixedPerKb(EXPECTED_TX_FEE as u64)); + check_tx_fee(&coin, EXPECTED_TX_FEE as u64); let allowance = block_on(coin.allowance(coin.swap_contract_address)).expect("!allowance"); assert_eq!(allowance, 0.into()); @@ -794,16 +814,20 @@ fn test_sender_trade_preimage_zero_allowance() { CONTRACT_CALL_GAS_FEE + SWAP_PAYMENT_GAS_FEE + EXPECTED_TX_FEE, coin.utxo.decimals, ); - let sender_refund_fee = big_decimal_from_sat(CONTRACT_CALL_GAS_FEE + EXPECTED_TX_FEE, coin.utxo.decimals); let actual = block_on(coin.get_sender_trade_fee(TradePreimageValue::Exact(1.into()), FeeApproxStage::WithoutApprox)) .expect("!get_sender_trade_fee"); + + let actual_tx_fee = ((actual.tx_size.unwrap() as i64 * EXPECTED_TX_FEE) as f64 / KILO_BYTE).ceil() as i64; + let sender_refund_fee = big_decimal_from_sat(CONTRACT_CALL_GAS_FEE + actual_tx_fee, coin.utxo.decimals); + // one `approve` contract call should be included into the expected trade fee let expected = TradeFee { coin: "QTUM".to_owned(), amount: (erc20_payment_fee_with_one_approve + sender_refund_fee).into(), paid_from_trading_vol: false, + tx_size: None, }; assert_eq!(actual, expected); } @@ -821,7 +845,7 @@ fn test_sender_trade_preimage_with_allowance() { ]; let (_ctx, coin) = qrc20_coin_for_test(priv_key, None); // check if the coin's tx fee is expected - check_tx_fee(&coin, ActualTxFee::FixedPerKb(EXPECTED_TX_FEE as u64)); + check_tx_fee(&coin, EXPECTED_TX_FEE as u64); let allowance = block_on(coin.allowance(coin.swap_contract_address)).expect("!allowance"); assert_eq!(allowance, 300_000_000.into()); @@ -844,6 +868,7 @@ fn test_sender_trade_preimage_with_allowance() { coin: "QTUM".to_owned(), amount: (erc20_payment_fee_without_approve + sender_refund_fee.clone()).into(), paid_from_trading_vol: false, + tx_size: None, }; assert_eq!(actual, expected); @@ -857,6 +882,7 @@ fn test_sender_trade_preimage_with_allowance() { coin: "QTUM".to_owned(), amount: (erc20_payment_fee_with_two_approves + sender_refund_fee).into(), paid_from_trading_vol: false, + tx_size: None, }; assert_eq!(actual, expected); } @@ -929,18 +955,20 @@ fn test_receiver_trade_preimage() { ]; let (_ctx, coin) = qrc20_coin_for_test(priv_key, None); // check if the coin's tx fee is expected - check_tx_fee(&coin, ActualTxFee::FixedPerKb(EXPECTED_TX_FEE as u64)); + check_tx_fee(&coin, EXPECTED_TX_FEE as u64); let actual = coin .get_receiver_trade_fee(FeeApproxStage::WithoutApprox) .wait() .expect("!get_receiver_trade_fee"); // only one contract call should be included into the expected trade fee - let expected_receiver_fee = big_decimal_from_sat(CONTRACT_CALL_GAS_FEE + EXPECTED_TX_FEE, coin.utxo.decimals); + let actual_tx_fee = ((actual.tx_size.unwrap() as i64 * EXPECTED_TX_FEE) as f64 / KILO_BYTE).ceil() as i64; + let expected_receiver_fee = big_decimal_from_sat(CONTRACT_CALL_GAS_FEE + actual_tx_fee, coin.utxo.decimals); let expected = TradeFee { coin: "QTUM".to_owned(), amount: expected_receiver_fee.into(), paid_from_trading_vol: false, + tx_size: actual.tx_size, }; assert_eq!(actual, expected); } @@ -956,7 +984,7 @@ fn test_taker_fee_tx_fee() { ]; let (_ctx, coin) = qrc20_coin_for_test(priv_key, None); // check if the coin's tx fee is expected - check_tx_fee(&coin, ActualTxFee::FixedPerKb(EXPECTED_TX_FEE as u64)); + check_tx_fee(&coin, EXPECTED_TX_FEE as u64); let expected_balance = CoinBalance { spendable: BigDecimal::from(5u32), unspendable: BigDecimal::from(0u32), @@ -970,11 +998,13 @@ fn test_taker_fee_tx_fee() { )) .expect("!get_fee_to_send_taker_fee"); // only one contract call should be included into the expected trade fee - let expected_receiver_fee = big_decimal_from_sat(CONTRACT_CALL_GAS_FEE + EXPECTED_TX_FEE, coin.utxo.decimals); + let actual_tx_fee = ((actual.tx_size.unwrap() as i64 * EXPECTED_TX_FEE) as f64 / KILO_BYTE).ceil() as i64; + let expected_receiver_fee = big_decimal_from_sat(CONTRACT_CALL_GAS_FEE + actual_tx_fee, coin.utxo.decimals); let expected = TradeFee { coin: "QTUM".to_owned(), amount: expected_receiver_fee.into(), paid_from_trading_vol: false, + tx_size: actual.tx_size, }; assert_eq!(actual, expected); } diff --git a/mm2src/coins/rpc_command/lightning/open_channel.rs b/mm2src/coins/rpc_command/lightning/open_channel.rs index f0e7b48bd7..7c8082f28f 100644 --- a/mm2src/coins/rpc_command/lightning/open_channel.rs +++ b/mm2src/coins/rpc_command/lightning/open_channel.rs @@ -4,7 +4,7 @@ use crate::lightning::ln_p2p::{connect_to_ln_node, ConnectionError}; use crate::lightning::ln_serialization::NodeAddress; use crate::lightning::ln_storage::LightningStorage; use crate::utxo::utxo_common::UtxoTxBuilder; -use crate::utxo::{sat_from_big_decimal, FeePolicy, GetUtxoListOps, UtxoTxGenerationOps}; +use crate::utxo::{sat_from_big_decimal, FeePolicy, GetUtxoListOps, TxFeeType, UtxoTxGenerationOps}; use crate::{lp_coinfind_or_err, BalanceError, CoinFindError, GenerateTxError, MmCoinEnum, NumConversError, UnexpectedDerivationMethod, UtxoRpcError}; use chain::TransactionOutput; @@ -173,10 +173,10 @@ pub async fn open_channel(ctx: MmArc, req: OpenChannelRequest) -> OpenChannelRes .with_fee_policy(fee_policy); let fee = platform_coin - .get_tx_fee() + .get_tx_fee_per_kb() .await .map_err(|e| OpenChannelError::RpcError(e.to_string()))?; - tx_builder = tx_builder.with_fee(fee); + tx_builder = tx_builder.with_fee(TxFeeType::PerKb(fee)); let (unsigned, _) = tx_builder.build().await?; diff --git a/mm2src/coins/tendermint/tendermint_coin.rs b/mm2src/coins/tendermint/tendermint_coin.rs index dbbb609e82..d12210b056 100644 --- a/mm2src/coins/tendermint/tendermint_coin.rs +++ b/mm2src/coins/tendermint/tendermint_coin.rs @@ -1601,6 +1601,7 @@ impl TendermintCoin { coin: ticker, amount: fee_amount.into(), paid_from_trading_vol: false, + tx_size: None, }) } @@ -1653,6 +1654,7 @@ impl TendermintCoin { coin: ticker, amount: fee_amount.into(), paid_from_trading_vol: false, + tx_size: None, }) } diff --git a/mm2src/coins/utxo.rs b/mm2src/coins/utxo.rs index 1bba5184c2..3bbed6910d 100644 --- a/mm2src/coins/utxo.rs +++ b/mm2src/coins/utxo.rs @@ -114,6 +114,7 @@ use crate::hd_wallet::{HDAccountOps, HDAccountsMutex, HDAddress, HDAddressId, HD InvalidBip44ChainError}; use crate::hd_wallet_storage::{HDAccountStorageItem, HDWalletCoinStorage, HDWalletStorageError, HDWalletStorageResult}; use crate::utxo::tx_cache::UtxoVerboseCacheShared; +use crate::utxo::utxo_common::PreImageTradeFeeResult; use crate::{CoinAssocTypes, ToBytes}; pub mod tx_cache; @@ -123,11 +124,11 @@ pub mod utxo_common_tests; #[cfg(test)] pub mod utxo_tests; #[cfg(target_arch = "wasm32")] pub mod utxo_wasm_tests; -const KILO_BYTE: u64 = 1000; +pub const KILO_BYTE: f64 = 1000.; /// https://bitcoin.stackexchange.com/a/77192 const MAX_DER_SIGNATURE_LEN: usize = 72; const COMPRESSED_PUBKEY_LEN: usize = 33; -const P2PKH_OUTPUT_LEN: u64 = 34; +pub const P2PKH_OUTPUT_LEN: u64 = 34; const MATURE_CONFIRMATIONS_DEFAULT: u32 = 100; const UTXO_DUST_AMOUNT: u64 = 1000; /// Block count for KMD median time past calculation @@ -277,6 +278,7 @@ pub struct AdditionalTxData { pub fee_amount: u64, pub unused_change: u64, pub kmd_rewards: Option, + pub tx_v_size: u64, } /// The fee set from coins config @@ -288,14 +290,16 @@ pub enum TxFee { FixedPerKb(u64), } -/// The actual "runtime" fee that is received from RPC in case of dynamic calculation #[derive(Copy, Clone, Debug, PartialEq)] -pub enum ActualTxFee { - /// fee amount per Kbyte received from coin RPC - Dynamic(u64), - /// Use specified amount per each 1 kb of transaction and also per each output less than amount. - /// Used by DOGE, but more coins might support it too. - FixedPerKb(u64), +pub enum TxFeeType { + /// Fee per kb whether it is dynamic (received from RPC) or fixed + PerKb(u64), + /// Use specified fixed amount for the whole transaction that is not dependent on transaction size + Fixed(u64), +} + +impl TxFee { + pub fn is_dynamic(&self) -> bool { matches!(self, TxFee::Dynamic(_)) } } /// Fee policy applied on transaction creation @@ -855,7 +859,7 @@ pub trait UtxoTxBroadcastOps { #[async_trait] #[cfg_attr(test, mockable)] pub trait UtxoTxGenerationOps { - async fn get_tx_fee(&self) -> UtxoRpcResult; + async fn get_tx_fee_per_kb(&self) -> UtxoRpcResult; /// Calculates interest if the coin is KMD /// Adds the value to existing output to my_script_pub or creates additional interest output @@ -961,12 +965,22 @@ impl MatureUnspentList { } } +#[derive(Debug)] +pub struct HtlcSpendFeeRes { + pub fee: u64, + pub tx_size: Option, +} + +impl HtlcSpendFeeRes { + fn from(fee: u64, tx_size: Option) -> Self { Self { fee, tx_size } } +} + #[async_trait] #[cfg_attr(test, mockable)] pub trait UtxoCommonOps: AsRef + UtxoTxGenerationOps + UtxoTxBroadcastOps + Clone + Send + Sync + 'static { - async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult; + async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult; fn addresses_from_script(&self, script: &Script) -> Result, String>; @@ -1020,7 +1034,7 @@ pub trait UtxoCommonOps: fee_policy: FeePolicy, gas_fee: Option, stage: &FeeApproxStage, - ) -> TradePreimageResult; + ) -> TradePreimageResult; /// Increase the given `dynamic_fee` according to the fee approximation `stage`. /// The method is used to predict a possible increase in dynamic fee. diff --git a/mm2src/coins/utxo/bch.rs b/mm2src/coins/utxo/bch.rs index 68ade20347..c743d2aef9 100644 --- a/mm2src/coins/utxo/bch.rs +++ b/mm2src/coins/utxo/bch.rs @@ -690,7 +690,7 @@ impl UtxoTxBroadcastOps for BchCoin { #[async_trait] #[cfg_attr(test, mockable)] impl UtxoTxGenerationOps for BchCoin { - async fn get_tx_fee(&self) -> UtxoRpcResult { utxo_common::get_tx_fee(&self.utxo_arc).await } + async fn get_tx_fee_per_kb(&self) -> UtxoRpcResult { utxo_common::get_tx_fee_per_kb(&self.utxo_arc).await } async fn calc_interest_if_required( &self, @@ -734,7 +734,7 @@ impl GetUtxoListOps for BchCoin { #[async_trait] #[cfg_attr(test, mockable)] impl UtxoCommonOps for BchCoin { - async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult { + async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult { utxo_common::get_htlc_spend_fee(self, tx_size, stage).await } @@ -795,7 +795,7 @@ impl UtxoCommonOps for BchCoin { fee_policy: FeePolicy, gas_fee: Option, stage: &FeeApproxStage, - ) -> TradePreimageResult { + ) -> TradePreimageResult { utxo_common::preimage_trade_fee_required_to_send_outputs( self, self.ticker(), diff --git a/mm2src/coins/utxo/qtum.rs b/mm2src/coins/utxo/qtum.rs index 3bfe1f6959..539fbbe55c 100644 --- a/mm2src/coins/utxo/qtum.rs +++ b/mm2src/coins/utxo/qtum.rs @@ -332,7 +332,7 @@ impl UtxoTxBroadcastOps for QtumCoin { #[async_trait] #[cfg_attr(test, mockable)] impl UtxoTxGenerationOps for QtumCoin { - async fn get_tx_fee(&self) -> UtxoRpcResult { utxo_common::get_tx_fee(&self.utxo_arc).await } + async fn get_tx_fee_per_kb(&self) -> UtxoRpcResult { utxo_common::get_tx_fee_per_kb(&self.utxo_arc).await } async fn calc_interest_if_required( &self, @@ -398,7 +398,7 @@ impl GetUtxoMapOps for QtumCoin { #[async_trait] #[cfg_attr(test, mockable)] impl UtxoCommonOps for QtumCoin { - async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult { + async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult { utxo_common::get_htlc_spend_fee(self, tx_size, stage).await } @@ -463,7 +463,7 @@ impl UtxoCommonOps for QtumCoin { fee_policy: FeePolicy, gas_fee: Option, stage: &FeeApproxStage, - ) -> TradePreimageResult { + ) -> TradePreimageResult { utxo_common::preimage_trade_fee_required_to_send_outputs( self, self.ticker(), diff --git a/mm2src/coins/utxo/slp.rs b/mm2src/coins/utxo/slp.rs index 2b5087e0e0..caec612060 100644 --- a/mm2src/coins/utxo/slp.rs +++ b/mm2src/coins/utxo/slp.rs @@ -10,8 +10,8 @@ use crate::utxo::bch::BchCoin; use crate::utxo::bchd_grpc::{check_slp_transaction, validate_slp_utxos, ValidateSlpUtxosErr}; use crate::utxo::rpc_clients::{UnspentInfo, UtxoRpcClientEnum, UtxoRpcError, UtxoRpcResult}; use crate::utxo::utxo_common::{self, big_decimal_from_sat_unsigned, payment_script, UtxoTxBuilder}; -use crate::utxo::{generate_and_send_tx, sat_from_big_decimal, ActualTxFee, AdditionalTxData, BroadcastTxErr, - FeePolicy, GenerateTxError, RecentlySpentOutPointsGuard, UtxoCoinConf, UtxoCoinFields, +use crate::utxo::{generate_and_send_tx, sat_from_big_decimal, AdditionalTxData, BroadcastTxErr, FeePolicy, + GenerateTxError, RecentlySpentOutPointsGuard, TxFeeType, UtxoCoinConf, UtxoCoinFields, UtxoCommonOps, UtxoTx, UtxoTxBroadcastOps, UtxoTxGenerationOps}; use crate::{BalanceFut, CheckIfMyPaymentSentArgs, CoinBalance, CoinFutSpawner, ConfirmPaymentInput, DexFee, FeeApproxStage, FoundSwapTxSpend, HistorySyncState, MakerSwapTakerCoin, MarketCoinOps, MmCoin, MmCoinEnum, @@ -1072,7 +1072,7 @@ impl UtxoTxBroadcastOps for SlpToken { #[async_trait] impl UtxoTxGenerationOps for SlpToken { - async fn get_tx_fee(&self) -> UtxoRpcResult { self.platform_coin.get_tx_fee().await } + async fn get_tx_fee_per_kb(&self) -> UtxoRpcResult { todo!() } async fn calc_interest_if_required( &self, @@ -1654,11 +1654,11 @@ impl MmCoin for SlpToken { match req.fee { Some(WithdrawFee::UtxoFixed { amount }) => { let fixed = sat_from_big_decimal(&amount, platform_decimals)?; - tx_builder = tx_builder.with_fee(ActualTxFee::FixedPerKb(fixed)) + tx_builder = tx_builder.with_fee(TxFeeType::Fixed(fixed)) }, Some(WithdrawFee::UtxoPerKbyte { amount }) => { - let dynamic = sat_from_big_decimal(&amount, platform_decimals)?; - tx_builder = tx_builder.with_fee(ActualTxFee::Dynamic(dynamic)); + let per_kb = sat_from_big_decimal(&amount, platform_decimals)?; + tx_builder = tx_builder.with_fee(TxFeeType::PerKb(per_kb)); }, Some(fee_policy) => { let error = format!( @@ -1799,8 +1799,9 @@ impl MmCoin for SlpToken { .await?; Ok(TradeFee { coin: self.platform_coin.ticker().into(), - amount: fee.into(), + amount: fee.fee.into(), paid_from_trading_vol: false, + tx_size: fee.tx_size, }) } @@ -1812,12 +1813,14 @@ impl MmCoin for SlpToken { .platform_coin .get_htlc_spend_fee(SLP_HTLC_SPEND_SIZE, &FeeApproxStage::WithoutApprox) .await?; - let amount = - (big_decimal_from_sat_unsigned(htlc_fee, coin.platform_decimals()) + coin.platform_dust_dec()).into(); + let amount = (big_decimal_from_sat_unsigned(htlc_fee.fee, coin.platform_decimals()) + + coin.platform_dust_dec()) + .into(); Ok(TradeFee { coin: coin.platform_coin.ticker().into(), amount, paid_from_trading_vol: false, + tx_size: htlc_fee.tx_size, }) }; @@ -1848,8 +1851,9 @@ impl MmCoin for SlpToken { .await?; Ok(TradeFee { coin: self.platform_coin.ticker().into(), - amount: fee.into(), + amount: fee.fee.into(), paid_from_trading_vol: false, + tx_size: fee.tx_size, }) } diff --git a/mm2src/coins/utxo/utxo_common.rs b/mm2src/coins/utxo/utxo_common.rs index 88ff31c2d8..98fe74cf1d 100644 --- a/mm2src/coins/utxo/utxo_common.rs +++ b/mm2src/coins/utxo/utxo_common.rs @@ -94,18 +94,16 @@ lazy_static! { pub const HISTORY_TOO_LARGE_ERR_CODE: i64 = -1; -pub async fn get_tx_fee(coin: &UtxoCoinFields) -> UtxoRpcResult { +pub async fn get_tx_fee_per_kb(coin: &UtxoCoinFields) -> UtxoRpcResult { let conf = &coin.conf; match &coin.tx_fee { TxFee::Dynamic(method) => { - let fee = coin - .rpc_client + coin.rpc_client .estimate_fee_sat(coin.decimals, method, &conf.estimate_fee_mode, conf.estimate_fee_blocks) .compat() - .await?; - Ok(ActualTxFee::Dynamic(fee)) + .await }, - TxFee::FixedPerKb(satoshis) => Ok(ActualTxFee::FixedPerKb(*satoshis)), + TxFee::FixedPerKb(satoshis) => Ok(*satoshis), } } @@ -594,24 +592,15 @@ pub async fn get_htlc_spend_fee( coin: &T, tx_size: u64, stage: &FeeApproxStage, -) -> UtxoRpcResult { - let coin_fee = coin.get_tx_fee().await?; - let mut fee = match coin_fee { - // atomic swap payment spend transaction is slightly more than 300 bytes in average as of now - ActualTxFee::Dynamic(fee_per_kb) => { - let fee_per_kb = increase_dynamic_fee_by_stage(&coin, fee_per_kb, stage); - (fee_per_kb * tx_size) / KILO_BYTE - }, - // return satoshis here as swap spend transaction size is always less than 1 kb - ActualTxFee::FixedPerKb(satoshis) => { - let tx_size_kb = if tx_size % KILO_BYTE == 0 { - tx_size / KILO_BYTE - } else { - tx_size / KILO_BYTE + 1 - }; - satoshis * tx_size_kb - }, - }; +) -> UtxoRpcResult { + let mut fee_per_kb = coin.get_tx_fee_per_kb().await?; + if coin.as_ref().tx_fee.is_dynamic() { + fee_per_kb = increase_dynamic_fee_by_stage(&coin, fee_per_kb, stage); + } + drop_mutability!(fee_per_kb); + + let mut fee = ((fee_per_kb * tx_size) as f64 / KILO_BYTE).ceil() as u64; + if coin.as_ref().conf.force_min_relay_fee { let relay_fee = coin.as_ref().rpc_client.get_relay_fee().compat().await?; let relay_fee_sat = sat_from_big_decimal(&relay_fee, coin.as_ref().decimals)?; @@ -619,7 +608,8 @@ pub async fn get_htlc_spend_fee( fee = relay_fee_sat; } } - Ok(fee) + + Ok(HtlcSpendFeeRes::from(fee, Some(tx_size))) } pub fn addresses_from_script(coin: &T, script: &Script) -> Result, String> { @@ -792,7 +782,7 @@ pub struct UtxoTxBuilder<'a, T: AsRef + UtxoTxGenerationOps> { /// The available inputs that *can* be included in the resulting tx available_inputs: Vec, fee_policy: FeePolicy, - fee: Option, + fee: Option, gas_fee: Option, tx: TransactionInputSigner, change: u64, @@ -861,7 +851,7 @@ impl<'a, T: AsRef + UtxoTxGenerationOps> UtxoTxBuilder<'a, T> { self } - pub fn with_fee(mut self, fee: ActualTxFee) -> Self { + pub fn with_fee(mut self, fee: TxFeeType) -> Self { self.fee = Some(fee); self } @@ -878,24 +868,15 @@ impl<'a, T: AsRef + UtxoTxGenerationOps> UtxoTxBuilder<'a, T> { fn update_fee_and_check_completeness( &mut self, from_addr_format: &UtxoAddressFormat, - actual_tx_fee: &ActualTxFee, + actual_tx_fee: &TxFeeType, ) -> bool { self.tx_fee = match &actual_tx_fee { - ActualTxFee::Dynamic(f) => { - let transaction = UtxoTx::from(self.tx.clone()); - let v_size = tx_size_in_v_bytes(from_addr_format, &transaction); - (f * v_size as u64) / KILO_BYTE - }, - ActualTxFee::FixedPerKb(f) => { + TxFeeType::PerKb(f) => { let transaction = UtxoTx::from(self.tx.clone()); let v_size = tx_size_in_v_bytes(from_addr_format, &transaction) as u64; - let v_size_kb = if v_size % KILO_BYTE == 0 { - v_size / KILO_BYTE - } else { - v_size / KILO_BYTE + 1 - }; - f * v_size_kb + ((f * v_size) as f64 / KILO_BYTE).ceil() as u64 }, + TxFeeType::Fixed(f) => *f, }; match self.fee_policy { @@ -905,9 +886,9 @@ impl<'a, T: AsRef + UtxoTxGenerationOps> UtxoTxBuilder<'a, T> { self.change = self.sum_inputs - outputs_plus_fee; if self.change > self.dust() { // there will be change output - if let ActualTxFee::Dynamic(ref f) = actual_tx_fee { - self.tx_fee += (f * P2PKH_OUTPUT_LEN) / KILO_BYTE; - outputs_plus_fee += (f * P2PKH_OUTPUT_LEN) / KILO_BYTE; + if let TxFeeType::PerKb(ref f) = actual_tx_fee { + self.tx_fee += ((f * P2PKH_OUTPUT_LEN) as f64 / KILO_BYTE).ceil() as u64; + outputs_plus_fee += ((f * P2PKH_OUTPUT_LEN) as f64 / KILO_BYTE).ceil() as u64; } } if let Some(min_relay) = self.min_relay_fee { @@ -926,8 +907,8 @@ impl<'a, T: AsRef + UtxoTxGenerationOps> UtxoTxBuilder<'a, T> { if self.sum_inputs >= self.sum_outputs_value { self.change = self.sum_inputs - self.sum_outputs_value; if self.change > self.dust() { - if let ActualTxFee::Dynamic(ref f) = actual_tx_fee { - self.tx_fee += (f * P2PKH_OUTPUT_LEN) / KILO_BYTE; + if let TxFeeType::PerKb(f) = actual_tx_fee { + self.tx_fee += ((*f * P2PKH_OUTPUT_LEN) as f64 / KILO_BYTE).ceil() as u64; } } if let Some(min_relay) = self.min_relay_fee { @@ -964,9 +945,11 @@ impl<'a, T: AsRef + UtxoTxGenerationOps> UtxoTxBuilder<'a, T> { let actual_tx_fee = match self.fee { Some(fee) => fee, - None => coin.get_tx_fee().await?, + None => TxFeeType::PerKb(coin.get_tx_fee_per_kb().await?), }; + println!("IS actual_tx_fee: {:?}", actual_tx_fee); + true_or!(!self.tx.outputs.is_empty(), GenerateTxError::EmptyOutputs); let mut received_by_me = 0; @@ -1055,6 +1038,8 @@ impl<'a, T: AsRef + UtxoTxGenerationOps> UtxoTxBuilder<'a, T> { change }; + let transaction = UtxoTx::from(self.tx.clone()); + let tx_v_size = tx_size_in_v_bytes(from.addr_format(), &transaction) as u64; let data = AdditionalTxData { fee_amount: self.tx_fee, received_by_me, @@ -1062,6 +1047,7 @@ impl<'a, T: AsRef + UtxoTxGenerationOps> UtxoTxBuilder<'a, T> { unused_change, // will be changed if the ticker is KMD kmd_rewards: None, + tx_v_size, }; Ok(coin @@ -1329,10 +1315,10 @@ async fn gen_taker_funding_spend_preimage( coin.get_htlc_spend_fee(DEFAULT_SWAP_TX_SPEND_SIZE, &FeeApproxStage::WithoutApprox) .await? }, - FundingSpendFeeSetting::UseExact(f) => f, + FundingSpendFeeSetting::UseExact(f) => HtlcSpendFeeRes::from(f, Some(args.funding_tx.serialized_size() as u64)), }; - let fee_plus_dust = fee + coin.as_ref().dust_amount; + let fee_plus_dust = fee.fee + coin.as_ref().dust_amount; if funding_amount < fee_plus_dust { return MmError::err(TxGenError::TxFeeTooHigh(format!( "Fee + dust {} is larger than funding amount {}", @@ -1341,7 +1327,7 @@ async fn gen_taker_funding_spend_preimage( } let payment_output = TransactionOutput { - value: funding_amount - fee, + value: funding_amount - fee.fee, script_pubkey: Builder::build_p2sh(&AddressHashEnum::AddressHash(dhash160(&payment_redeem_script))).to_bytes(), }; @@ -1423,12 +1409,12 @@ pub async fn validate_taker_funding_spend_preimage( let actual_fee = funding_amount - payment_amount; - let fee_div = expected_fee as f64 / actual_fee as f64; + let fee_div = expected_fee.fee as f64 / actual_fee as f64; if !(0.9..=1.1).contains(&fee_div) { return MmError::err(ValidateTakerFundingSpendPreimageError::UnexpectedPreimageFee(format!( "Too large difference between expected {} and actual {} fees", - expected_fee, actual_fee + expected_fee.fee, actual_fee ))); } @@ -1588,7 +1574,7 @@ async fn gen_taker_payment_spend_preimage( .value - outputs[0].value - outputs[1].value - - tx_fee; + - tx_fee.fee; outputs.push(TransactionOutput { value: maker_value, script_pubkey: script.to_bytes(), @@ -1726,13 +1712,13 @@ pub async fn sign_and_broadcast_taker_payment_spend( .await ); - if miner_fee + coin.as_ref().dust_amount + dex_fee_sat > payment_output.value { + if miner_fee.fee + coin.as_ref().dust_amount + dex_fee_sat > payment_output.value { return TX_PLAIN_ERR!("Payment amount is too small to cover miner fee + dust + dex_fee_sat"); } let maker_address = try_tx_s!(coin.as_ref().derivation_method.single_addr_or_err()); let maker_output = TransactionOutput { - value: payment_output.value - miner_fee - dex_fee_sat, + value: payment_output.value - miner_fee.fee - dex_fee_sat, script_pubkey: try_tx_s!(output_script(maker_address)).to_bytes(), }; signer.outputs.push(maker_output); @@ -1920,16 +1906,16 @@ pub fn send_maker_spends_taker_payment(coin: T, args coin.get_htlc_spend_fee(DEFAULT_SWAP_TX_SPEND_SIZE, &FeeApproxStage::WithoutApprox) .await ); - if fee >= payment_value { + if fee.fee >= payment_value { return TX_PLAIN_ERR!( "HTLC spend fee {} is greater than transaction output {}", - fee, + fee.fee, payment_value ); } let script_pubkey = output_script(&my_address).map(|script| script.to_bytes())?; let output = TransactionOutput { - value: payment_value - fee, + value: payment_value - fee.fee, script_pubkey, }; @@ -2026,16 +2012,16 @@ pub fn create_maker_payment_spend_preimage( .await ); - if fee >= payment_value { + if fee.fee >= payment_value { return TX_PLAIN_ERR!( "HTLC spend fee {} is greater than transaction output {}", - fee, + fee.fee, payment_value ); } let script_pubkey = output_script(&my_address).map(|script| script.to_bytes())?; let output = TransactionOutput { - value: payment_value - fee, + value: payment_value - fee.fee, script_pubkey, }; @@ -2085,16 +2071,16 @@ pub fn create_taker_payment_refund_preimage( coin.get_htlc_spend_fee(DEFAULT_SWAP_TX_SPEND_SIZE, &FeeApproxStage::WatcherPreimage) .await ); - if fee >= payment_value { + if fee.fee >= payment_value { return TX_PLAIN_ERR!( "HTLC spend fee {} is greater than transaction output {}", - fee, + fee.fee, payment_value ); } let script_pubkey = output_script(&my_address).map(|script| script.to_bytes())?; let output = TransactionOutput { - value: payment_value - fee, + value: payment_value - fee.fee, script_pubkey, }; @@ -2142,16 +2128,16 @@ pub fn send_taker_spends_maker_payment(coin: T, args coin.get_htlc_spend_fee(DEFAULT_SWAP_TX_SPEND_SIZE, &FeeApproxStage::WithoutApprox) .await ); - if fee >= payment_value { + if fee.fee >= payment_value { return TX_PLAIN_ERR!( "HTLC spend fee {} is greater than transaction output {}", - fee, + fee.fee, payment_value ); } let script_pubkey = output_script(&my_address).map(|script| script.to_bytes())?; let output = TransactionOutput { - value: payment_value - fee, + value: payment_value - fee.fee, script_pubkey, }; @@ -2198,16 +2184,16 @@ pub async fn refund_htlc_payment( coin.get_htlc_spend_fee(DEFAULT_SWAP_TX_SPEND_SIZE, &FeeApproxStage::WithoutApprox) .await ); - if fee >= payment_value { + if fee.fee >= payment_value { return TX_PLAIN_ERR!( "HTLC spend fee {} is greater than transaction output {}", - fee, + fee.fee, payment_value ); } let script_pubkey = output_script(&my_address).map(|script| script.to_bytes())?; let output = TransactionOutput { - value: payment_value - fee, + value: payment_value - fee.fee, script_pubkey, }; @@ -4186,20 +4172,22 @@ pub fn get_trade_fee(coin: T) -> Box f, - ActualTxFee::FixedPerKb(f) => f, - }; + let fee = try_s!(coin.get_tx_fee_per_kb().await); Ok(TradeFee { coin: ticker, - amount: big_decimal_from_sat(amount as i64, decimals).into(), + amount: big_decimal_from_sat(fee as i64, decimals).into(), paid_from_trading_vol: false, + tx_size: None, }) }; Box::new(fut.boxed().compat()) } +pub struct PreImageTradeFeeResult { + pub fee: BigDecimal, + pub tx_size: Option, +} + /// To ensure the `get_sender_trade_fee(x) <= get_sender_trade_fee(y)` condition is satisfied for any `x < y`, /// we should include a `change` output into the result fee. Imagine this case: /// Let `sum_inputs = 11000` and `total_tx_fee: { 200, if there is no the change output; 230, if there is the change output }`. @@ -4219,82 +4207,57 @@ pub async fn preimage_trade_fee_required_to_send_outputs( fee_policy: FeePolicy, gas_fee: Option, stage: &FeeApproxStage, -) -> TradePreimageResult +) -> TradePreimageResult where T: UtxoCommonOps + GetUtxoListOps, { let decimals = coin.as_ref().decimals; - let tx_fee = coin.get_tx_fee().await?; // [`FeePolicy::DeductFromOutput`] is used if the value is [`TradePreimageValue::UpperBound`] only let is_amount_upper_bound = matches!(fee_policy, FeePolicy::DeductFromOutput(_)); let my_address = coin.as_ref().derivation_method.single_addr_or_err()?; - match tx_fee { - // if it's a dynamic fee, we should generate a swap transaction to get an actual trade fee - ActualTxFee::Dynamic(fee) => { - // take into account that the dynamic tx fee may increase during the swap - let dynamic_fee = coin.increase_dynamic_fee_by_stage(fee, stage); - - let outputs_count = outputs.len(); - let (unspents, _recently_sent_txs) = coin.get_unspent_ordered_list(my_address).await?; + let mut tx_fee_per_kb = coin.get_tx_fee_per_kb().await?; + if coin.as_ref().tx_fee.is_dynamic() { + tx_fee_per_kb = coin.increase_dynamic_fee_by_stage(tx_fee_per_kb, stage) + }; - let actual_tx_fee = ActualTxFee::Dynamic(dynamic_fee); + let outputs_count = outputs.len(); + let (unspents, _recently_sent_txs) = coin.get_unspent_ordered_list(my_address).await?; - let mut tx_builder = UtxoTxBuilder::new(coin) - .add_available_inputs(unspents) - .add_outputs(outputs) - .with_fee_policy(fee_policy) - .with_fee(actual_tx_fee); - if let Some(gas) = gas_fee { - tx_builder = tx_builder.with_gas_fee(gas); - } - let (tx, data) = tx_builder.build().await.mm_err(|e| { - TradePreimageError::from_generate_tx_error(e, ticker.to_owned(), decimals, is_amount_upper_bound) - })?; + let mut tx_builder = UtxoTxBuilder::new(coin) + .add_available_inputs(unspents) + .add_outputs(outputs) + .with_fee_policy(fee_policy) + .with_fee(TxFeeType::PerKb(tx_fee_per_kb)); + if let Some(gas) = gas_fee { + tx_builder = tx_builder.with_gas_fee(gas); + } + let (tx, data) = tx_builder.build().await.mm_err(|e| { + TradePreimageError::from_generate_tx_error(e, ticker.to_owned(), decimals, is_amount_upper_bound) + })?; - let total_fee = if tx.outputs.len() == outputs_count { - // take into account the change output - data.fee_amount + (dynamic_fee * P2PKH_OUTPUT_LEN) / KILO_BYTE + let total_fee = if tx.outputs.len() == outputs_count { + if coin.as_ref().tx_fee.is_dynamic() { + data.fee_amount + ((tx_fee_per_kb * P2PKH_OUTPUT_LEN) as f64 / KILO_BYTE) as u64 + } else { + // take into account the change output if tx_size_kb(tx with change) > tx_size_kb(tx without change) + let tx = UtxoTx::from(tx.clone()); + let tx_bytes_len = serialize(&tx).len(); + if (tx_bytes_len as f64 % KILO_BYTE + P2PKH_OUTPUT_LEN as f64) > KILO_BYTE { + data.fee_amount + tx_fee_per_kb } else { - // the change output is included already data.fee_amount - }; - - Ok(big_decimal_from_sat(total_fee as i64, decimals)) - }, - ActualTxFee::FixedPerKb(fee) => { - let outputs_count = outputs.len(); - let (unspents, _recently_sent_txs) = coin.get_unspent_ordered_list(my_address).await?; - - let mut tx_builder = UtxoTxBuilder::new(coin) - .add_available_inputs(unspents) - .add_outputs(outputs) - .with_fee_policy(fee_policy) - .with_fee(tx_fee); - if let Some(gas) = gas_fee { - tx_builder = tx_builder.with_gas_fee(gas); } - let (tx, data) = tx_builder.build().await.mm_err(|e| { - TradePreimageError::from_generate_tx_error(e, ticker.to_string(), decimals, is_amount_upper_bound) - })?; - - let total_fee = if tx.outputs.len() == outputs_count { - // take into account the change output if tx_size_kb(tx with change) > tx_size_kb(tx without change) - let tx = UtxoTx::from(tx); - let tx_bytes = serialize(&tx); - if tx_bytes.len() as u64 % KILO_BYTE + P2PKH_OUTPUT_LEN > KILO_BYTE { - data.fee_amount + fee - } else { - data.fee_amount - } - } else { - // the change output is included already - data.fee_amount - }; + } + } else { + // the change output is included already + data.fee_amount + }; - Ok(big_decimal_from_sat(total_fee as i64, decimals)) - }, - } + Ok(PreImageTradeFeeResult { + fee: big_decimal_from_sat(total_fee as i64, decimals), + tx_size: Some(data.tx_v_size), + }) } /// Maker or Taker should pay fee only for sending his payment. @@ -4333,13 +4296,14 @@ where ) .map_to_mm(TradePreimageError::InternalError)?; let gas_fee = None; - let fee_amount = coin + let fee = coin .preimage_trade_fee_required_to_send_outputs(outputs, fee_policy, gas_fee, &stage) .await?; Ok(TradeFee { coin: coin.as_ref().conf.ticker.clone(), - amount: fee_amount.into(), + amount: fee.fee.clone().into(), paid_from_trading_vol: false, + tx_size: fee.tx_size, }) } @@ -4347,11 +4311,12 @@ where pub fn get_receiver_trade_fee(coin: T) -> TradePreimageFut { let fut = async move { let amount_sat = get_htlc_spend_fee(&coin, DEFAULT_SWAP_TX_SPEND_SIZE, &FeeApproxStage::WithoutApprox).await?; - let amount = big_decimal_from_sat_unsigned(amount_sat, coin.as_ref().decimals).into(); + let amount = big_decimal_from_sat_unsigned(amount_sat.fee, coin.as_ref().decimals).into(); Ok(TradeFee { coin: coin.as_ref().conf.ticker.clone(), amount, paid_from_trading_vol: true, + tx_size: amount_sat.tx_size, }) }; Box::new(fut.boxed().compat()) @@ -4375,8 +4340,9 @@ where .await?; Ok(TradeFee { coin: coin.ticker().to_owned(), - amount: fee_amount.into(), + amount: fee_amount.fee.into(), paid_from_trading_vol: false, + tx_size: fee_amount.tx_size, }) } @@ -5239,16 +5205,16 @@ where coin.get_htlc_spend_fee(DEFAULT_SWAP_TX_SPEND_SIZE, &FeeApproxStage::WithoutApprox) .await ); - if fee >= payment_value { + if fee.fee >= payment_value { return TX_PLAIN_ERR!( "HTLC spend fee {} is greater than transaction output {}", - fee, + fee.fee, payment_value ); } let script_pubkey = output_script(&my_address).map(|script| script.to_bytes())?; let output = TransactionOutput { - value: payment_value - fee, + value: payment_value - fee.fee, script_pubkey, }; @@ -5427,16 +5393,16 @@ pub async fn spend_maker_payment_v2( coin.get_htlc_spend_fee(DEFAULT_SWAP_TX_SPEND_SIZE, &FeeApproxStage::WithoutApprox) .await ); - if fee >= payment_value { + if fee.fee >= payment_value { return TX_PLAIN_ERR!( "HTLC spend fee {} is greater than transaction output {}", - fee, + fee.fee, payment_value ); } let script_pubkey = try_tx_s!(output_script(&my_address)).to_bytes(); let output = TransactionOutput { - value: payment_value - fee, + value: payment_value - fee.fee, script_pubkey, }; @@ -5488,16 +5454,16 @@ where coin.get_htlc_spend_fee(DEFAULT_SWAP_TX_SPEND_SIZE, &FeeApproxStage::WithoutApprox) .await ); - if fee >= payment_value { + if fee.fee >= payment_value { return TX_PLAIN_ERR!( "HTLC spend fee {} is greater than transaction output {}", - fee, + fee.fee, payment_value ); } let script_pubkey = try_tx_s!(output_script(&my_address)).to_bytes(); let output = TransactionOutput { - value: payment_value - fee, + value: payment_value - fee.fee, script_pubkey, }; diff --git a/mm2src/coins/utxo/utxo_standard.rs b/mm2src/coins/utxo/utxo_standard.rs index 1453acc346..0dc08b4aa7 100644 --- a/mm2src/coins/utxo/utxo_standard.rs +++ b/mm2src/coins/utxo/utxo_standard.rs @@ -113,7 +113,7 @@ impl UtxoTxBroadcastOps for UtxoStandardCoin { #[async_trait] #[cfg_attr(test, mockable)] impl UtxoTxGenerationOps for UtxoStandardCoin { - async fn get_tx_fee(&self) -> UtxoRpcResult { utxo_common::get_tx_fee(&self.utxo_arc).await } + async fn get_tx_fee_per_kb(&self) -> UtxoRpcResult { utxo_common::get_tx_fee_per_kb(&self.utxo_arc).await } async fn calc_interest_if_required( &self, @@ -180,7 +180,7 @@ impl GetUtxoMapOps for UtxoStandardCoin { #[async_trait] #[cfg_attr(test, mockable)] impl UtxoCommonOps for UtxoStandardCoin { - async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult { + async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult { utxo_common::get_htlc_spend_fee(self, tx_size, stage).await } @@ -241,7 +241,7 @@ impl UtxoCommonOps for UtxoStandardCoin { fee_policy: FeePolicy, gas_fee: Option, stage: &FeeApproxStage, - ) -> TradePreimageResult { + ) -> TradePreimageResult { utxo_common::preimage_trade_fee_required_to_send_outputs( self, self.ticker(), diff --git a/mm2src/coins/utxo/utxo_tests.rs b/mm2src/coins/utxo/utxo_tests.rs index a40aaec887..a8ad978a2e 100644 --- a/mm2src/coins/utxo/utxo_tests.rs +++ b/mm2src/coins/utxo/utxo_tests.rs @@ -1158,7 +1158,7 @@ fn test_generate_transaction_relay_fee_is_used_when_dynamic_fee_is_lower() { let builder = UtxoTxBuilder::new(&coin) .add_available_inputs(unspents) .add_outputs(outputs) - .with_fee(ActualTxFee::Dynamic(100)); + .with_fee(TxFeeType::PerKb(100)); let generated = block_on(builder.build()).unwrap(); assert_eq!(generated.0.outputs.len(), 1); @@ -1201,7 +1201,7 @@ fn test_generate_transaction_relay_fee_is_used_when_dynamic_fee_is_lower_and_ded .add_available_inputs(unspents) .add_outputs(outputs) .with_fee_policy(FeePolicy::DeductFromOutput(0)) - .with_fee(ActualTxFee::Dynamic(100)); + .with_fee(TxFeeType::PerKb(100)); let generated = block_on(tx_builder.build()).unwrap(); assert_eq!(generated.0.outputs.len(), 1); @@ -1248,7 +1248,7 @@ fn test_generate_tx_fee_is_correct_when_dynamic_fee_is_larger_than_relay() { let builder = UtxoTxBuilder::new(&coin) .add_available_inputs(unspents) .add_outputs(outputs) - .with_fee(ActualTxFee::Dynamic(1000)); + .with_fee(TxFeeType::PerKb(1000)); let generated = block_on(builder.build()).unwrap(); diff --git a/mm2src/coins/utxo/utxo_withdraw.rs b/mm2src/coins/utxo/utxo_withdraw.rs index 795da12006..313a706b27 100644 --- a/mm2src/coins/utxo/utxo_withdraw.rs +++ b/mm2src/coins/utxo/utxo_withdraw.rs @@ -1,8 +1,8 @@ use crate::rpc_command::init_withdraw::{WithdrawInProgressStatus, WithdrawTaskHandleShared}; use crate::utxo::utxo_common::{big_decimal_from_sat, UtxoTxBuilder}; -use crate::utxo::{output_script, sat_from_big_decimal, ActualTxFee, Address, AddressBuilder, FeePolicy, - GetUtxoListOps, PrivKeyPolicy, UtxoAddressFormat, UtxoCoinFields, UtxoCommonOps, UtxoFeeDetails, - UtxoTx, UTXO_LOCK}; +use crate::utxo::{output_script, sat_from_big_decimal, Address, AddressBuilder, FeePolicy, GetUtxoListOps, + PrivKeyPolicy, TxFeeType, UtxoAddressFormat, UtxoCoinFields, UtxoCommonOps, UtxoFeeDetails, UtxoTx, + UTXO_LOCK}; use crate::{CoinWithDerivationMethod, GetWithdrawSenderAddress, MarketCoinOps, TransactionDetails, WithdrawError, WithdrawFee, WithdrawFrom, WithdrawRequest, WithdrawResult}; use async_trait::async_trait; @@ -171,11 +171,11 @@ where match req.fee { Some(WithdrawFee::UtxoFixed { ref amount }) => { let fixed = sat_from_big_decimal(amount, decimals)?; - tx_builder = tx_builder.with_fee(ActualTxFee::FixedPerKb(fixed)); + tx_builder = tx_builder.with_fee(TxFeeType::Fixed(fixed)); }, Some(WithdrawFee::UtxoPerKbyte { ref amount }) => { let dynamic = sat_from_big_decimal(amount, decimals)?; - tx_builder = tx_builder.with_fee(ActualTxFee::Dynamic(dynamic)); + tx_builder = tx_builder.with_fee(TxFeeType::PerKb(dynamic)); }, Some(ref fee_policy) => { let error = format!( diff --git a/mm2src/coins/z_coin.rs b/mm2src/coins/z_coin.rs index 3ec97cd558..da561750ef 100644 --- a/mm2src/coins/z_coin.rs +++ b/mm2src/coins/z_coin.rs @@ -7,9 +7,9 @@ use crate::utxo::rpc_clients::{ElectrumRpcRequest, UnspentInfo, UtxoRpcClientEnu use crate::utxo::utxo_builder::UtxoCoinBuildError; use crate::utxo::utxo_builder::{UtxoCoinBuilder, UtxoCoinBuilderCommonOps, UtxoFieldsWithGlobalHDBuilder, UtxoFieldsWithHardwareWalletBuilder, UtxoFieldsWithIguanaSecretBuilder}; -use crate::utxo::utxo_common::{big_decimal_from_sat_unsigned, payment_script}; -use crate::utxo::{sat_from_big_decimal, utxo_common, ActualTxFee, AdditionalTxData, AddrFromStrError, Address, - BroadcastTxErr, FeePolicy, GetUtxoListOps, HistoryUtxoTx, HistoryUtxoTxMap, MatureUnspentList, +use crate::utxo::utxo_common::{big_decimal_from_sat_unsigned, payment_script, PreImageTradeFeeResult}; +use crate::utxo::{sat_from_big_decimal, utxo_common, AdditionalTxData, AddrFromStrError, Address, BroadcastTxErr, + FeePolicy, GetUtxoListOps, HistoryUtxoTx, HistoryUtxoTxMap, HtlcSpendFeeRes, MatureUnspentList, RecentlySpentOutPointsGuard, UtxoActivationParams, UtxoAddressFormat, UtxoArc, UtxoCoinFields, UtxoCommonOps, UtxoRpcMode, UtxoTxBroadcastOps, UtxoTxGenerationOps, VerboseTransactionFrom}; use crate::utxo::{UnsupportedAddr, UtxoFeeDetails}; @@ -395,12 +395,8 @@ impl ZCoin { } async fn get_one_kbyte_tx_fee(&self) -> UtxoRpcResult { - let fee = self.get_tx_fee().await?; - match fee { - ActualTxFee::Dynamic(fee) | ActualTxFee::FixedPerKb(fee) => { - Ok(big_decimal_from_sat_unsigned(fee, self.decimals())) - }, - } + let fee = self.get_tx_fee_per_kb().await?; + Ok(big_decimal_from_sat_unsigned(fee, self.decimals())) } /// Generates a tx sending outputs from our address @@ -503,13 +499,18 @@ impl ZCoin { .await? .tx_result?; + let mut tx_bytes = Vec::with_capacity(1024); + tx.write(&mut tx_bytes).expect("Write should not fail"); + let additional_data = AdditionalTxData { received_by_me, spent_by_me: sat_from_big_decimal(&total_input_amount, self.decimals())?, fee_amount: sat_from_big_decimal(&tx_fee, self.decimals())?, unused_change: 0, kmd_rewards: None, + tx_v_size: tx_bytes.len() as u64, }; + Ok((tx, additional_data, sync_guard)) } @@ -1794,6 +1795,7 @@ impl MmCoin for ZCoin { coin: self.ticker().to_owned(), amount: self.get_one_kbyte_tx_fee().await?.into(), paid_from_trading_vol: false, + tx_size: None, }) } @@ -1810,6 +1812,7 @@ impl MmCoin for ZCoin { coin: self.ticker().to_owned(), amount: self.get_one_kbyte_tx_fee().await?.into(), paid_from_trading_vol: false, + tx_size: None, }) } @@ -1852,7 +1855,7 @@ impl MmCoin for ZCoin { #[async_trait] impl UtxoTxGenerationOps for ZCoin { - async fn get_tx_fee(&self) -> UtxoRpcResult { utxo_common::get_tx_fee(&self.utxo_arc).await } + async fn get_tx_fee_per_kb(&self) -> UtxoRpcResult { utxo_common::get_tx_fee_per_kb(&self.utxo_arc).await } async fn calc_interest_if_required( &self, @@ -1902,7 +1905,7 @@ impl GetUtxoListOps for ZCoin { #[async_trait] impl UtxoCommonOps for ZCoin { - async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult { + async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult { utxo_common::get_htlc_spend_fee(self, tx_size, stage).await } @@ -1969,7 +1972,7 @@ impl UtxoCommonOps for ZCoin { fee_policy: FeePolicy, gas_fee: Option, stage: &FeeApproxStage, - ) -> TradePreimageResult { + ) -> TradePreimageResult { utxo_common::preimage_trade_fee_required_to_send_outputs( self, self.ticker(), diff --git a/mm2src/mm2_main/src/lp_swap.rs b/mm2src/mm2_main/src/lp_swap.rs index c4b7a405a0..15d6fcc0bf 100644 --- a/mm2src/mm2_main/src/lp_swap.rs +++ b/mm2src/mm2_main/src/lp_swap.rs @@ -1056,6 +1056,7 @@ impl From for TradeFee { coin: orig.coin, amount: orig.amount.into(), paid_from_trading_vol: orig.paid_from_trading_vol, + tx_size: None, } } } diff --git a/mm2src/mm2_main/src/lp_swap/taker_swap.rs b/mm2src/mm2_main/src/lp_swap/taker_swap.rs index e4acf1b968..ff2c4da6e1 100644 --- a/mm2src/mm2_main/src/lp_swap/taker_swap.rs +++ b/mm2src/mm2_main/src/lp_swap/taker_swap.rs @@ -2454,6 +2454,7 @@ pub async fn taker_swap_trade_preimage( coin: my_coin_ticker.to_owned(), amount: dex_amount.total_spend_amount(), paid_from_trading_vol: false, + tx_size: None, }; let fee_to_send_taker_fee = my_coin diff --git a/mm2src/mm2_main/src/lp_swap/taker_swap_v2.rs b/mm2src/mm2_main/src/lp_swap/taker_swap_v2.rs index ec5c88c0c5..d901632be4 100644 --- a/mm2src/mm2_main/src/lp_swap/taker_swap_v2.rs +++ b/mm2src/mm2_main/src/lp_swap/taker_swap_v2.rs @@ -953,6 +953,7 @@ impl Date: Fri, 8 Mar 2024 17:35:13 +0100 Subject: [PATCH 02/16] fix get_sender_trade_fee and qrc20 unit tests --- mm2src/coins/qrc20.rs | 4 +--- mm2src/coins/qrc20/qrc20_tests.rs | 39 +++++++++++++++++++++---------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/mm2src/coins/qrc20.rs b/mm2src/coins/qrc20.rs index cddb186629..2377260bde 100644 --- a/mm2src/coins/qrc20.rs +++ b/mm2src/coins/qrc20.rs @@ -1410,13 +1410,11 @@ impl MmCoin for Qrc20Coin { }; let total_fee = erc20_payment_fee.fee + sender_refund_fee.fee; - let total_tx_size = - erc20_payment_fee.tx_size.unwrap_or_default() + sender_refund_fee.tx_size.unwrap_or_default(); Ok(TradeFee { coin: self.platform.clone(), amount: total_fee.into(), paid_from_trading_vol: false, - tx_size: Some(total_tx_size), + tx_size: sender_refund_fee.tx_size, }) } diff --git a/mm2src/coins/qrc20/qrc20_tests.rs b/mm2src/coins/qrc20/qrc20_tests.rs index 6e41b50b43..d82e891208 100644 --- a/mm2src/coins/qrc20/qrc20_tests.rs +++ b/mm2src/coins/qrc20/qrc20_tests.rs @@ -23,6 +23,10 @@ const CONTRACT_CALL_GAS_FEE: i64 = (QRC20_GAS_LIMIT_DEFAULT * QRC20_GAS_PRICE_DE const SWAP_PAYMENT_GAS_FEE: i64 = (QRC20_PAYMENT_GAS_LIMIT * QRC20_GAS_PRICE_DEFAULT) as i64; const TAKER_PAYMENT_SPEND_SEARCH_INTERVAL: f64 = 1.; +fn get_expected_trade_fee_per_kb(tx_size: u64) -> i64 { + ((tx_size as i64 * EXPECTED_TX_FEE) as f64 / KILO_BYTE).ceil() as i64 +} + pub fn qrc20_coin_for_test(priv_key: [u8; 32], fallback_swap: Option<&str>) -> (MmArc, Qrc20Coin) { let conf = json!({ "coin":"QRC20", @@ -810,24 +814,25 @@ fn test_sender_trade_preimage_zero_allowance() { let allowance = block_on(coin.allowance(coin.swap_contract_address)).expect("!allowance"); assert_eq!(allowance, 0.into()); + let erc20_payment_fee_tx_size = 535; + let erc20_payment_fee = get_expected_trade_fee_per_kb(erc20_payment_fee_tx_size); let erc20_payment_fee_with_one_approve = big_decimal_from_sat( - CONTRACT_CALL_GAS_FEE + SWAP_PAYMENT_GAS_FEE + EXPECTED_TX_FEE, + CONTRACT_CALL_GAS_FEE + SWAP_PAYMENT_GAS_FEE + erc20_payment_fee, coin.utxo.decimals, ); let actual = block_on(coin.get_sender_trade_fee(TradePreimageValue::Exact(1.into()), FeeApproxStage::WithoutApprox)) .expect("!get_sender_trade_fee"); - - let actual_tx_fee = ((actual.tx_size.unwrap() as i64 * EXPECTED_TX_FEE) as f64 / KILO_BYTE).ceil() as i64; - let sender_refund_fee = big_decimal_from_sat(CONTRACT_CALL_GAS_FEE + actual_tx_fee, coin.utxo.decimals); + let actual_tx_fee = get_expected_trade_fee_per_kb(actual.tx_size.unwrap()); + let sender_refund_fee = big_decimal_from_sat(actual_tx_fee + CONTRACT_CALL_GAS_FEE, coin.utxo.decimals); // one `approve` contract call should be included into the expected trade fee let expected = TradeFee { coin: "QTUM".to_owned(), amount: (erc20_payment_fee_with_one_approve + sender_refund_fee).into(), paid_from_trading_vol: false, - tx_size: None, + tx_size: actual.tx_size, }; assert_eq!(actual, expected); } @@ -850,25 +855,32 @@ fn test_sender_trade_preimage_with_allowance() { let allowance = block_on(coin.allowance(coin.swap_contract_address)).expect("!allowance"); assert_eq!(allowance, 300_000_000.into()); + let erc20_payment_fee_without_approve_tx_size = 576; + let erc20_payment_fee = get_expected_trade_fee_per_kb(erc20_payment_fee_without_approve_tx_size); let erc20_payment_fee_without_approve = - big_decimal_from_sat(SWAP_PAYMENT_GAS_FEE + EXPECTED_TX_FEE, coin.utxo.decimals); + big_decimal_from_sat(SWAP_PAYMENT_GAS_FEE + erc20_payment_fee, coin.utxo.decimals); + + let erc20_payment_fee_tx_size = 790; + let erc20_payment_fee = get_expected_trade_fee_per_kb(erc20_payment_fee_tx_size); let erc20_payment_fee_with_two_approves = big_decimal_from_sat( - 2 * CONTRACT_CALL_GAS_FEE + SWAP_PAYMENT_GAS_FEE + EXPECTED_TX_FEE, + 2 * CONTRACT_CALL_GAS_FEE + SWAP_PAYMENT_GAS_FEE + erc20_payment_fee, coin.utxo.decimals, ); - let sender_refund_fee = big_decimal_from_sat(CONTRACT_CALL_GAS_FEE + EXPECTED_TX_FEE, coin.utxo.decimals); let actual = block_on(coin.get_sender_trade_fee( TradePreimageValue::Exact(BigDecimal::try_from(2.5).unwrap()), FeeApproxStage::WithoutApprox, )) .expect("!get_sender_trade_fee"); + let expected_fee = get_expected_trade_fee_per_kb(actual.tx_size.unwrap()); + let sender_refund_fee = big_decimal_from_sat(CONTRACT_CALL_GAS_FEE + expected_fee, coin.utxo.decimals); + // the expected fee should not include any `approve` contract call let expected = TradeFee { coin: "QTUM".to_owned(), amount: (erc20_payment_fee_without_approve + sender_refund_fee.clone()).into(), paid_from_trading_vol: false, - tx_size: None, + tx_size: actual.tx_size, }; assert_eq!(actual, expected); @@ -877,12 +889,14 @@ fn test_sender_trade_preimage_with_allowance() { FeeApproxStage::WithoutApprox, )) .expect("!get_sender_trade_fee"); + let expected_fee = get_expected_trade_fee_per_kb(actual.tx_size.unwrap()); + let sender_refund_fee = big_decimal_from_sat(CONTRACT_CALL_GAS_FEE + expected_fee, coin.utxo.decimals); // two `approve` contract calls should be included into the expected trade fee let expected = TradeFee { coin: "QTUM".to_owned(), amount: (erc20_payment_fee_with_two_approves + sender_refund_fee).into(), paid_from_trading_vol: false, - tx_size: None, + tx_size: actual.tx_size, }; assert_eq!(actual, expected); } @@ -962,7 +976,8 @@ fn test_receiver_trade_preimage() { .wait() .expect("!get_receiver_trade_fee"); // only one contract call should be included into the expected trade fee - let actual_tx_fee = ((actual.tx_size.unwrap() as i64 * EXPECTED_TX_FEE) as f64 / KILO_BYTE).ceil() as i64; + + let actual_tx_fee = get_expected_trade_fee_per_kb(actual.tx_size.unwrap()); let expected_receiver_fee = big_decimal_from_sat(CONTRACT_CALL_GAS_FEE + actual_tx_fee, coin.utxo.decimals); let expected = TradeFee { coin: "QTUM".to_owned(), @@ -998,7 +1013,7 @@ fn test_taker_fee_tx_fee() { )) .expect("!get_fee_to_send_taker_fee"); // only one contract call should be included into the expected trade fee - let actual_tx_fee = ((actual.tx_size.unwrap() as i64 * EXPECTED_TX_FEE) as f64 / KILO_BYTE).ceil() as i64; + let actual_tx_fee = get_expected_trade_fee_per_kb(actual.tx_size.unwrap()); let expected_receiver_fee = big_decimal_from_sat(CONTRACT_CALL_GAS_FEE + actual_tx_fee, coin.utxo.decimals); let expected = TradeFee { coin: "QTUM".to_owned(), From 944fd57874c6c5d77133ac0dfc169d02880b1f23 Mon Sep 17 00:00:00 2001 From: Samuel Onoja Date: Fri, 8 Mar 2024 22:13:51 +0100 Subject: [PATCH 03/16] minro changes --- mm2src/coins/utxo/utxo_common.rs | 3 +- mm2src/coins/utxo/utxo_tests.rs | 72 +++++++++++++++++++++++++------- 2 files changed, 59 insertions(+), 16 deletions(-) diff --git a/mm2src/coins/utxo/utxo_common.rs b/mm2src/coins/utxo/utxo_common.rs index 98fe74cf1d..9dc8333bd0 100644 --- a/mm2src/coins/utxo/utxo_common.rs +++ b/mm2src/coins/utxo/utxo_common.rs @@ -948,8 +948,6 @@ impl<'a, T: AsRef + UtxoTxGenerationOps> UtxoTxBuilder<'a, T> { None => TxFeeType::PerKb(coin.get_tx_fee_per_kb().await?), }; - println!("IS actual_tx_fee: {:?}", actual_tx_fee); - true_or!(!self.tx.outputs.is_empty(), GenerateTxError::EmptyOutputs); let mut received_by_me = 0; @@ -1156,6 +1154,7 @@ pub async fn calc_interest_if_required( } let rewards_amount = big_decimal_from_sat_unsigned(interest, coin.as_ref().decimals); data.kmd_rewards = Some(KmdRewardsDetails::claimed_by_me(rewards_amount)); + Ok((unsigned, data)) } diff --git a/mm2src/coins/utxo/utxo_tests.rs b/mm2src/coins/utxo/utxo_tests.rs index a8ad978a2e..02c1fd3a33 100644 --- a/mm2src/coins/utxo/utxo_tests.rs +++ b/mm2src/coins/utxo/utxo_tests.rs @@ -21,7 +21,7 @@ use crate::utxo::spv::SimplePaymentVerification; #[cfg(not(target_arch = "wasm32"))] use crate::utxo::utxo_block_header_storage::{BlockHeaderStorage, SqliteBlockHeadersStorage}; use crate::utxo::utxo_builder::{UtxoArcBuilder, UtxoCoinBuilder, UtxoCoinBuilderCommonOps}; -use crate::utxo::utxo_common::UtxoTxBuilder; +use crate::utxo::utxo_common::{tx_size_in_v_bytes, UtxoTxBuilder}; #[cfg(not(target_arch = "wasm32"))] use crate::utxo::utxo_common_tests::TEST_COIN_DECIMALS; use crate::utxo::utxo_common_tests::{self, utxo_coin_fields_for_test, utxo_coin_from_fields, TEST_COIN_NAME}; @@ -218,18 +218,19 @@ fn test_generate_transaction() { let outputs = vec![TransactionOutput { script_pubkey: vec![].into(), - value: 98001, + value: 98781, }]; let builder = UtxoTxBuilder::new(&coin) .add_available_inputs(unspents) .add_outputs(outputs); let generated = block_on(builder.build()).unwrap(); + // the change that is less than dust must be included to miner fee // so no extra outputs should appear in generated transaction assert_eq!(generated.0.outputs.len(), 1); - assert_eq!(generated.1.fee_amount, 1000); + assert_eq!(generated.1.fee_amount, 220); assert_eq!(generated.1.unused_change, 999); assert_eq!(generated.1.received_by_me, 0); assert_eq!(generated.1.spent_by_me, 100000); @@ -254,11 +255,11 @@ fn test_generate_transaction() { let generated = block_on(builder.build()).unwrap(); assert_eq!(generated.0.outputs.len(), 1); - assert_eq!(generated.1.fee_amount, 1000); + assert_eq!(generated.1.fee_amount, 211); assert_eq!(generated.1.unused_change, 0); - assert_eq!(generated.1.received_by_me, 99000); + assert_eq!(generated.1.received_by_me, 99789); assert_eq!(generated.1.spent_by_me, 100000); - assert_eq!(generated.0.outputs[0].value, 99000); + assert_eq!(generated.0.outputs[0].value, 99789); let unspents = vec![UnspentInfo { value: 100000, @@ -879,11 +880,24 @@ fn test_withdraw_kmd_rewards_impl( fee: None, memo: None, }; + let tx_details = coin.withdraw(withdraw_req).wait().unwrap(); + let tx_size_kb = tx_details.tx_hex.len() as f64 / KILO_BYTE; + // In transaction size calculations we assume the script sig size is (2 + MAX_DER_SIGNATURE_LEN + COMPRESSED_PUBKEY_LEN) or 107 bytes + // when in reality signatures can vary by 1 or 2 bytes because of possible zero padding of r and s values of the signature. + // This is why we test for a range of values here instead of a single value. The value we use in fees calculation is the + // highest possible value of 107 to ensure we don't underestimate the fee. + assert!( + (0.243..=0.245).contains(&tx_size_kb), + "Tx size in KB {} is not within the range [{}, {}]", + tx_size_kb, + 0.243, + 0.245 + ); + let tx_size_kb = 0.245; let expected_fee = TxFeeDetails::Utxo(UtxoFeeDetails { coin: Some("KMD".into()), - amount: "0.00001".parse().unwrap(), + amount: big_decimal_from_sat((1000. * tx_size_kb) as i64, coin.as_ref().decimals), }); - let tx_details = coin.withdraw(withdraw_req).wait().unwrap(); assert_eq!(tx_details.fee_details, Some(expected_fee)); let expected_rewards = expected_rewards.map(|amount| KmdRewardsDetails { @@ -954,11 +968,24 @@ fn test_withdraw_rick_rewards_none() { fee: None, memo: None, }; + let tx_details = coin.withdraw(withdraw_req).wait().unwrap(); + let tx_size_kb = tx_details.tx_hex.len() as f64 / KILO_BYTE; + // In transaction size calculations we assume the script sig size is (2 + MAX_DER_SIGNATURE_LEN + COMPRESSED_PUBKEY_LEN) or 107 bytes + // when in reality signatures can vary by 1 or 2 bytes because of possible zero padding of r and s values of the signature. + // This is why we test for a range of values here instead of a single value. The value we use in fees calculation is the + // highest possible value of 107 to ensure we don't underestimate the fee. + assert!( + (0.243..=0.245).contains(&tx_size_kb), + "Tx size in KB {} is not within the range [{}, {}]", + tx_size_kb, + 0.243, + 0.245 + ); + let tx_size_kb = 0.245; let expected_fee = TxFeeDetails::Utxo(UtxoFeeDetails { coin: Some(TEST_COIN_NAME.into()), - amount: "0.00001".parse().unwrap(), + amount: big_decimal_from_sat((1000. * tx_size_kb) as i64, coin.as_ref().decimals), }); - let tx_details = coin.withdraw(withdraw_req).wait().unwrap(); assert_eq!(tx_details.fee_details, Some(expected_fee)); assert_eq!(tx_details.kmd_rewards, None); } @@ -2697,6 +2724,9 @@ fn firo_lelantus_tx_details() { #[test] fn test_generate_tx_doge_fee() { + // Choose a tx fee per kb equal to minimum relay fee to test that the minimum relay fee is used when transaction is below 1kb + const TXFEE_PER_KB: u64 = 100_000; + // A tx below 1kb is always 0,01 doge fee per kb. let config = json!({ "coin": "DOGE", @@ -2706,7 +2736,13 @@ fn test_generate_tx_doge_fee() { "pubtype": 30, "p2shtype": 22, "wiftype": 158, - "txfee": 1000000, + // The minimum relay fee for doge was changed from 1000000 sats to 100000 sats since the below issues were resolved + // The transaction size is below 1 kb so the fee should be 0.001 doge (100000 sats) + // https://github.com/KomodoPlatform/atomicDEX-API/issues/829 + // https://github.com/KomodoPlatform/atomicDEX-API/pull/830 + // https://github.com/KomodoPlatform/atomicDEX-API/commit/faf944ea721bd87816b9b3b1cdf02d4bf3f4c6ea + // https://github.com/dogecoin/dogecoin/discussions/2347 + "txfee": TXFEE_PER_KB, "force_min_relay_fee": true, "mm2": 1, "required_confirmations": 2, @@ -2729,6 +2765,8 @@ fn test_generate_tx_doge_fee() { )) .unwrap(); + let min_relay_fee = block_on(doge.as_ref().rpc_client.get_relay_fee().compat()).unwrap(); + let min_relay_fee_sat = sat_from_big_decimal(&min_relay_fee, doge.as_ref().decimals).unwrap(); let unspents = vec![UnspentInfo { outpoint: Default::default(), value: 1000000000000, @@ -2741,8 +2779,12 @@ fn test_generate_tx_doge_fee() { let builder = UtxoTxBuilder::new(&doge) .add_available_inputs(unspents) .add_outputs(outputs); - let (_, data) = block_on(builder.build()).unwrap(); - let expected_fee = 1000000; + let (input_signer, data) = block_on(builder.build()).unwrap(); + let transaction = UtxoTx::from(input_signer); + let v_size = tx_size_in_v_bytes(&UtxoAddressFormat::Standard, &transaction) as u64; + assert!(v_size < KILO_BYTE as u64); + // The fee should be min_relay_fee_sat because the tx size is below 1 kb + let expected_fee = min_relay_fee_sat; assert_eq!(expected_fee, data.fee_amount); let unspents = vec![UnspentInfo { @@ -2782,7 +2824,9 @@ fn test_generate_tx_doge_fee() { .add_available_inputs(unspents) .add_outputs(outputs); let (_, data) = block_on(builder.build()).unwrap(); - let expected_fee = 3000000; + let v_size = tx_size_in_v_bytes(&UtxoAddressFormat::Standard, &transaction) as u64; + assert!(v_size > KILO_BYTE as u64); + let expected_fee = ((TXFEE_PER_KB * v_size) as f64 / KILO_BYTE).ceil() as u64; assert_eq!(expected_fee, data.fee_amount); } From 0f8426ec0f6c360a5d70f69e29212b44ad183f71 Mon Sep 17 00:00:00 2001 From: Samuel Onoja Date: Fri, 8 Mar 2024 23:01:28 +0100 Subject: [PATCH 04/16] impl calculate_fee helper macro and other minor changes --- mm2src/coins/qrc20.rs | 5 ++--- mm2src/coins/qrc20/qrc20_tests.rs | 6 ++---- mm2src/coins/utxo.rs | 6 +++--- mm2src/coins/utxo/bch.rs | 2 +- mm2src/coins/utxo/qtum.rs | 2 +- mm2src/coins/utxo/utxo_common.rs | 29 ++++++++++++++++++++--------- mm2src/coins/utxo/utxo_standard.rs | 2 +- mm2src/coins/utxo/utxo_tests.rs | 8 ++++---- mm2src/coins/z_coin.rs | 6 +++--- 9 files changed, 37 insertions(+), 29 deletions(-) diff --git a/mm2src/coins/qrc20.rs b/mm2src/coins/qrc20.rs index 2377260bde..862c95aa8f 100644 --- a/mm2src/coins/qrc20.rs +++ b/mm2src/coins/qrc20.rs @@ -13,7 +13,7 @@ use crate::utxo::utxo_builder::{UtxoCoinBuildError, UtxoCoinBuildResult, UtxoCoi use crate::utxo::utxo_common::{self, big_decimal_from_sat, check_all_utxo_inputs_signed_by_pub, PreImageTradeFeeResult, UtxoTxBuilder}; use crate::utxo::{qtum, AdditionalTxData, AddrFromStrError, BroadcastTxErr, FeePolicy, GenerateTxError, - GetUtxoListOps, HistoryUtxoTx, HistoryUtxoTxMap, HtlcSpendFeeRes, MatureUnspentList, + GetUtxoListOps, HistoryUtxoTx, HistoryUtxoTxMap, HtlcSpendFeeResult, MatureUnspentList, RecentlySpentOutPointsGuard, UnsupportedAddr, UtxoActivationParams, UtxoAddressFormat, UtxoCoinFields, UtxoCommonOps, UtxoFromLegacyReqErr, UtxoTx, UtxoTxBroadcastOps, UtxoTxGenerationOps, VerboseTransactionFrom, UTXO_LOCK}; @@ -598,7 +598,6 @@ impl Qrc20Coin { UtxoCommonOps::preimage_trade_fee_required_to_send_outputs(self, outputs, fee_policy, Some(gas_fee), stage) .await?; let gas_fee = big_decimal_from_sat(gas_fee as i64, decimals); - println!("SIZE: {:?}", &miner_fee.tx_size); Ok(PreImageTradeFeeResult { tx_size: miner_fee.tx_size, fee: miner_fee.fee + gas_fee, @@ -660,7 +659,7 @@ impl GetUtxoListOps for Qrc20Coin { #[async_trait] #[cfg_attr(test, mockable)] impl UtxoCommonOps for Qrc20Coin { - async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult { + async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult { utxo_common::get_htlc_spend_fee(self, tx_size, stage).await } diff --git a/mm2src/coins/qrc20/qrc20_tests.rs b/mm2src/coins/qrc20/qrc20_tests.rs index d82e891208..dbf02029d4 100644 --- a/mm2src/coins/qrc20/qrc20_tests.rs +++ b/mm2src/coins/qrc20/qrc20_tests.rs @@ -1,6 +1,6 @@ use super::*; use crate::utxo::KILO_BYTE; -use crate::{DexFee, TxFeeDetails, WaitForHTLCTxSpendArgs}; +use crate::{calculate_fee, DexFee, TxFeeDetails, WaitForHTLCTxSpendArgs}; use common::{block_on, wait_until_sec, DEX_FEE_ADDR_RAW_PUBKEY}; use crypto::Secp256k1Secret; use itertools::Itertools; @@ -23,9 +23,7 @@ const CONTRACT_CALL_GAS_FEE: i64 = (QRC20_GAS_LIMIT_DEFAULT * QRC20_GAS_PRICE_DE const SWAP_PAYMENT_GAS_FEE: i64 = (QRC20_PAYMENT_GAS_LIMIT * QRC20_GAS_PRICE_DEFAULT) as i64; const TAKER_PAYMENT_SPEND_SEARCH_INTERVAL: f64 = 1.; -fn get_expected_trade_fee_per_kb(tx_size: u64) -> i64 { - ((tx_size as i64 * EXPECTED_TX_FEE) as f64 / KILO_BYTE).ceil() as i64 -} +fn get_expected_trade_fee_per_kb(tx_size: u64) -> i64 { calculate_fee!(tx_size, EXPECTED_TX_FEE as u64) as i64 } pub fn qrc20_coin_for_test(priv_key: [u8; 32], fallback_swap: Option<&str>) -> (MmArc, Qrc20Coin) { let conf = json!({ diff --git a/mm2src/coins/utxo.rs b/mm2src/coins/utxo.rs index 3bbed6910d..f2e57ef7a2 100644 --- a/mm2src/coins/utxo.rs +++ b/mm2src/coins/utxo.rs @@ -966,12 +966,12 @@ impl MatureUnspentList { } #[derive(Debug)] -pub struct HtlcSpendFeeRes { +pub struct HtlcSpendFeeResult { pub fee: u64, pub tx_size: Option, } -impl HtlcSpendFeeRes { +impl HtlcSpendFeeResult { fn from(fee: u64, tx_size: Option) -> Self { Self { fee, tx_size } } } @@ -980,7 +980,7 @@ impl HtlcSpendFeeRes { pub trait UtxoCommonOps: AsRef + UtxoTxGenerationOps + UtxoTxBroadcastOps + Clone + Send + Sync + 'static { - async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult; + async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult; fn addresses_from_script(&self, script: &Script) -> Result, String>; diff --git a/mm2src/coins/utxo/bch.rs b/mm2src/coins/utxo/bch.rs index c743d2aef9..95ecd56e71 100644 --- a/mm2src/coins/utxo/bch.rs +++ b/mm2src/coins/utxo/bch.rs @@ -734,7 +734,7 @@ impl GetUtxoListOps for BchCoin { #[async_trait] #[cfg_attr(test, mockable)] impl UtxoCommonOps for BchCoin { - async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult { + async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult { utxo_common::get_htlc_spend_fee(self, tx_size, stage).await } diff --git a/mm2src/coins/utxo/qtum.rs b/mm2src/coins/utxo/qtum.rs index 539fbbe55c..8660952ad4 100644 --- a/mm2src/coins/utxo/qtum.rs +++ b/mm2src/coins/utxo/qtum.rs @@ -398,7 +398,7 @@ impl GetUtxoMapOps for QtumCoin { #[async_trait] #[cfg_attr(test, mockable)] impl UtxoCommonOps for QtumCoin { - async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult { + async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult { utxo_common::get_htlc_spend_fee(self, tx_size, stage).await } diff --git a/mm2src/coins/utxo/utxo_common.rs b/mm2src/coins/utxo/utxo_common.rs index 9dc8333bd0..1d87a17c9a 100644 --- a/mm2src/coins/utxo/utxo_common.rs +++ b/mm2src/coins/utxo/utxo_common.rs @@ -92,6 +92,15 @@ lazy_static! { }); } +/// Calculates a transaction fee for a UTXO, +/// taking a fee per kilobyte (`fee_per_kb`) and transaction size in bytes (`tx_size`). +#[macro_export] +macro_rules! calculate_fee { + ($fee_per_kb:expr, $tx_size:expr) => { + ((($fee_per_kb * $tx_size) as f64) / KILO_BYTE).ceil() + }; +} + pub const HISTORY_TOO_LARGE_ERR_CODE: i64 = -1; pub async fn get_tx_fee_per_kb(coin: &UtxoCoinFields) -> UtxoRpcResult { @@ -592,14 +601,14 @@ pub async fn get_htlc_spend_fee( coin: &T, tx_size: u64, stage: &FeeApproxStage, -) -> UtxoRpcResult { +) -> UtxoRpcResult { let mut fee_per_kb = coin.get_tx_fee_per_kb().await?; if coin.as_ref().tx_fee.is_dynamic() { fee_per_kb = increase_dynamic_fee_by_stage(&coin, fee_per_kb, stage); } drop_mutability!(fee_per_kb); - let mut fee = ((fee_per_kb * tx_size) as f64 / KILO_BYTE).ceil() as u64; + let mut fee = calculate_fee!(fee_per_kb, tx_size) as u64; if coin.as_ref().conf.force_min_relay_fee { let relay_fee = coin.as_ref().rpc_client.get_relay_fee().compat().await?; @@ -609,7 +618,7 @@ pub async fn get_htlc_spend_fee( } } - Ok(HtlcSpendFeeRes::from(fee, Some(tx_size))) + Ok(HtlcSpendFeeResult::from(fee, Some(tx_size))) } pub fn addresses_from_script(coin: &T, script: &Script) -> Result, String> { @@ -874,7 +883,7 @@ impl<'a, T: AsRef + UtxoTxGenerationOps> UtxoTxBuilder<'a, T> { TxFeeType::PerKb(f) => { let transaction = UtxoTx::from(self.tx.clone()); let v_size = tx_size_in_v_bytes(from_addr_format, &transaction) as u64; - ((f * v_size) as f64 / KILO_BYTE).ceil() as u64 + calculate_fee!(f, v_size) as u64 }, TxFeeType::Fixed(f) => *f, }; @@ -887,8 +896,8 @@ impl<'a, T: AsRef + UtxoTxGenerationOps> UtxoTxBuilder<'a, T> { if self.change > self.dust() { // there will be change output if let TxFeeType::PerKb(ref f) = actual_tx_fee { - self.tx_fee += ((f * P2PKH_OUTPUT_LEN) as f64 / KILO_BYTE).ceil() as u64; - outputs_plus_fee += ((f * P2PKH_OUTPUT_LEN) as f64 / KILO_BYTE).ceil() as u64; + self.tx_fee += calculate_fee!(f, P2PKH_OUTPUT_LEN) as u64; + outputs_plus_fee += calculate_fee!(f, P2PKH_OUTPUT_LEN) as u64; } } if let Some(min_relay) = self.min_relay_fee { @@ -908,7 +917,7 @@ impl<'a, T: AsRef + UtxoTxGenerationOps> UtxoTxBuilder<'a, T> { self.change = self.sum_inputs - self.sum_outputs_value; if self.change > self.dust() { if let TxFeeType::PerKb(f) = actual_tx_fee { - self.tx_fee += ((*f * P2PKH_OUTPUT_LEN) as f64 / KILO_BYTE).ceil() as u64; + self.tx_fee += calculate_fee!(f, P2PKH_OUTPUT_LEN) as u64 } } if let Some(min_relay) = self.min_relay_fee { @@ -1314,7 +1323,9 @@ async fn gen_taker_funding_spend_preimage( coin.get_htlc_spend_fee(DEFAULT_SWAP_TX_SPEND_SIZE, &FeeApproxStage::WithoutApprox) .await? }, - FundingSpendFeeSetting::UseExact(f) => HtlcSpendFeeRes::from(f, Some(args.funding_tx.serialized_size() as u64)), + FundingSpendFeeSetting::UseExact(f) => { + HtlcSpendFeeResult::from(f, Some(args.funding_tx.serialized_size() as u64)) + }, }; let fee_plus_dust = fee.fee + coin.as_ref().dust_amount; @@ -4240,7 +4251,7 @@ where data.fee_amount + ((tx_fee_per_kb * P2PKH_OUTPUT_LEN) as f64 / KILO_BYTE) as u64 } else { // take into account the change output if tx_size_kb(tx with change) > tx_size_kb(tx without change) - let tx = UtxoTx::from(tx.clone()); + let tx = UtxoTx::from(tx); let tx_bytes_len = serialize(&tx).len(); if (tx_bytes_len as f64 % KILO_BYTE + P2PKH_OUTPUT_LEN as f64) > KILO_BYTE { data.fee_amount + tx_fee_per_kb diff --git a/mm2src/coins/utxo/utxo_standard.rs b/mm2src/coins/utxo/utxo_standard.rs index 0dc08b4aa7..16044bca2b 100644 --- a/mm2src/coins/utxo/utxo_standard.rs +++ b/mm2src/coins/utxo/utxo_standard.rs @@ -180,7 +180,7 @@ impl GetUtxoMapOps for UtxoStandardCoin { #[async_trait] #[cfg_attr(test, mockable)] impl UtxoCommonOps for UtxoStandardCoin { - async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult { + async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult { utxo_common::get_htlc_spend_fee(self, tx_size, stage).await } diff --git a/mm2src/coins/utxo/utxo_tests.rs b/mm2src/coins/utxo/utxo_tests.rs index 02c1fd3a33..65bc8a9b5f 100644 --- a/mm2src/coins/utxo/utxo_tests.rs +++ b/mm2src/coins/utxo/utxo_tests.rs @@ -27,9 +27,9 @@ use crate::utxo::utxo_common_tests::TEST_COIN_DECIMALS; use crate::utxo::utxo_common_tests::{self, utxo_coin_fields_for_test, utxo_coin_from_fields, TEST_COIN_NAME}; use crate::utxo::utxo_standard::{utxo_standard_coin_with_priv_key, UtxoStandardCoin}; use crate::utxo::utxo_tx_history_v2::{UtxoTxDetailsParams, UtxoTxHistoryOps}; -use crate::{BlockHeightAndTime, CoinBalance, ConfirmPaymentInput, DexFee, IguanaPrivKey, PrivKeyBuildPolicy, - SearchForSwapTxSpendInput, SpendPaymentArgs, StakingInfosDetails, SwapOps, TradePreimageValue, - TxFeeDetails, TxMarshalingErr, ValidateFeeArgs, INVALID_SENDER_ERR_LOG}; +use crate::{calculate_fee, BlockHeightAndTime, CoinBalance, ConfirmPaymentInput, DexFee, IguanaPrivKey, + PrivKeyBuildPolicy, SearchForSwapTxSpendInput, SpendPaymentArgs, StakingInfosDetails, SwapOps, + TradePreimageValue, TxFeeDetails, TxMarshalingErr, ValidateFeeArgs, INVALID_SENDER_ERR_LOG}; #[cfg(not(target_arch = "wasm32"))] use crate::{WaitForHTLCTxSpendArgs, WithdrawFee}; use chain::{BlockHeader, BlockHeaderBits, OutPoint}; @@ -2826,7 +2826,7 @@ fn test_generate_tx_doge_fee() { let (_, data) = block_on(builder.build()).unwrap(); let v_size = tx_size_in_v_bytes(&UtxoAddressFormat::Standard, &transaction) as u64; assert!(v_size > KILO_BYTE as u64); - let expected_fee = ((TXFEE_PER_KB * v_size) as f64 / KILO_BYTE).ceil() as u64; + let expected_fee = calculate_fee!(TXFEE_PER_KB, v_size) as u64; assert_eq!(expected_fee, data.fee_amount); } diff --git a/mm2src/coins/z_coin.rs b/mm2src/coins/z_coin.rs index da561750ef..98ba1456ba 100644 --- a/mm2src/coins/z_coin.rs +++ b/mm2src/coins/z_coin.rs @@ -9,7 +9,7 @@ use crate::utxo::utxo_builder::{UtxoCoinBuilder, UtxoCoinBuilderCommonOps, UtxoF UtxoFieldsWithHardwareWalletBuilder, UtxoFieldsWithIguanaSecretBuilder}; use crate::utxo::utxo_common::{big_decimal_from_sat_unsigned, payment_script, PreImageTradeFeeResult}; use crate::utxo::{sat_from_big_decimal, utxo_common, AdditionalTxData, AddrFromStrError, Address, BroadcastTxErr, - FeePolicy, GetUtxoListOps, HistoryUtxoTx, HistoryUtxoTxMap, HtlcSpendFeeRes, MatureUnspentList, + FeePolicy, GetUtxoListOps, HistoryUtxoTx, HistoryUtxoTxMap, HtlcSpendFeeResult, MatureUnspentList, RecentlySpentOutPointsGuard, UtxoActivationParams, UtxoAddressFormat, UtxoArc, UtxoCoinFields, UtxoCommonOps, UtxoRpcMode, UtxoTxBroadcastOps, UtxoTxGenerationOps, VerboseTransactionFrom}; use crate::utxo::{UnsupportedAddr, UtxoFeeDetails}; @@ -1905,12 +1905,12 @@ impl GetUtxoListOps for ZCoin { #[async_trait] impl UtxoCommonOps for ZCoin { - async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult { + async fn get_htlc_spend_fee(&self, tx_size: u64, stage: &FeeApproxStage) -> UtxoRpcResult { utxo_common::get_htlc_spend_fee(self, tx_size, stage).await } fn addresses_from_script(&self, script: &Script) -> Result, String> { - utxo_common::addresses_from_script(self, script) + addresses_from_script(self, script) } fn denominate_satoshis(&self, satoshi: i64) -> f64 { utxo_common::denominate_satoshis(&self.utxo_arc, satoshi) } From cd019f64e63fd3fa13a1705324394a9dfab4292a Mon Sep 17 00:00:00 2001 From: Samuel Onoja Date: Tue, 12 Mar 2024 15:28:16 +0100 Subject: [PATCH 05/16] save dev state --- mm2src/coins/lp_coins.rs | 14 ++++++++++ mm2src/coins/utxo.rs | 10 +++---- mm2src/coins/utxo/rpc_clients.rs | 2 +- mm2src/coins/utxo/utxo_common.rs | 6 ++-- mm2src/coins/utxo/utxo_withdraw.rs | 44 ++++++++++++++++++++++++++++-- mm2src/coins/z_coin.rs | 2 +- 6 files changed, 66 insertions(+), 12 deletions(-) diff --git a/mm2src/coins/lp_coins.rs b/mm2src/coins/lp_coins.rs index 46a1c1f714..2ac27b4319 100644 --- a/mm2src/coins/lp_coins.rs +++ b/mm2src/coins/lp_coins.rs @@ -1808,6 +1808,17 @@ pub trait MarketCoinOps { fn is_privacy(&self) -> bool { false } } +/// Priority levels for UTXO fee estimation for withdrawal. +#[derive(Clone, Debug, Deserialize, PartialEq)] +pub enum UtxoFeePriority { + /// Low priority: Estimated confirmation within approximately 3 blocks. + Low, + /// Normal priority: Estimated confirmation within approximately 2 blocks. + Normal, + /// High priority: Estimated confirmation within approximately 1 block. + High, +} + #[derive(Clone, Debug, Deserialize, PartialEq)] #[serde(tag = "type")] pub enum WithdrawFee { @@ -1817,6 +1828,9 @@ pub enum WithdrawFee { UtxoPerKbyte { amount: BigDecimal, }, + UtxoPriority { + priority: UtxoFeePriority, + }, EthGas { /// in gwei gas_price: BigDecimal, diff --git a/mm2src/coins/utxo.rs b/mm2src/coins/utxo.rs index f2e57ef7a2..de3a3414c3 100644 --- a/mm2src/coins/utxo.rs +++ b/mm2src/coins/utxo.rs @@ -278,7 +278,7 @@ pub struct AdditionalTxData { pub fee_amount: u64, pub unused_change: u64, pub kmd_rewards: Option, - pub tx_v_size: u64, + pub tx_size: u64, } /// The fee set from coins config @@ -290,6 +290,10 @@ pub enum TxFee { FixedPerKb(u64), } +impl TxFee { + pub fn is_dynamic(&self) -> bool { matches!(self, TxFee::Dynamic(_)) } +} + #[derive(Copy, Clone, Debug, PartialEq)] pub enum TxFeeType { /// Fee per kb whether it is dynamic (received from RPC) or fixed @@ -298,10 +302,6 @@ pub enum TxFeeType { Fixed(u64), } -impl TxFee { - pub fn is_dynamic(&self) -> bool { matches!(self, TxFee::Dynamic(_)) } -} - /// Fee policy applied on transaction creation pub enum FeePolicy { /// Send the exact amount specified in output(s), fee is added to spent input amount diff --git a/mm2src/coins/utxo/rpc_clients.rs b/mm2src/coins/utxo/rpc_clients.rs index 56a1c9289c..50f1b708f8 100644 --- a/mm2src/coins/utxo/rpc_clients.rs +++ b/mm2src/coins/utxo/rpc_clients.rs @@ -1359,7 +1359,7 @@ impl From for BestBlock { } #[allow(clippy::upper_case_acronyms)] -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Clone, Deserialize, Serialize)] pub enum EstimateFeeMode { ECONOMICAL, CONSERVATIVE, diff --git a/mm2src/coins/utxo/utxo_common.rs b/mm2src/coins/utxo/utxo_common.rs index 1d87a17c9a..da12e2d69b 100644 --- a/mm2src/coins/utxo/utxo_common.rs +++ b/mm2src/coins/utxo/utxo_common.rs @@ -1046,7 +1046,7 @@ impl<'a, T: AsRef + UtxoTxGenerationOps> UtxoTxBuilder<'a, T> { }; let transaction = UtxoTx::from(self.tx.clone()); - let tx_v_size = tx_size_in_v_bytes(from.addr_format(), &transaction) as u64; + let tx_size = tx_size_in_v_bytes(from.addr_format(), &transaction) as u64; let data = AdditionalTxData { fee_amount: self.tx_fee, received_by_me, @@ -1054,7 +1054,7 @@ impl<'a, T: AsRef + UtxoTxGenerationOps> UtxoTxBuilder<'a, T> { unused_change, // will be changed if the ticker is KMD kmd_rewards: None, - tx_v_size, + tx_size, }; Ok(coin @@ -4266,7 +4266,7 @@ where Ok(PreImageTradeFeeResult { fee: big_decimal_from_sat(total_fee as i64, decimals), - tx_size: Some(data.tx_v_size), + tx_size: Some(data.tx_size), }) } diff --git a/mm2src/coins/utxo/utxo_withdraw.rs b/mm2src/coins/utxo/utxo_withdraw.rs index 313a706b27..2d8e53ee0a 100644 --- a/mm2src/coins/utxo/utxo_withdraw.rs +++ b/mm2src/coins/utxo/utxo_withdraw.rs @@ -1,10 +1,12 @@ +use crate::common::Future01CompatExt; use crate::rpc_command::init_withdraw::{WithdrawInProgressStatus, WithdrawTaskHandleShared}; +use crate::utxo::rpc_clients::{EstimateFeeMethod, EstimateFeeMode, UtxoRpcClientEnum}; use crate::utxo::utxo_common::{big_decimal_from_sat, UtxoTxBuilder}; use crate::utxo::{output_script, sat_from_big_decimal, Address, AddressBuilder, FeePolicy, GetUtxoListOps, PrivKeyPolicy, TxFeeType, UtxoAddressFormat, UtxoCoinFields, UtxoCommonOps, UtxoFeeDetails, UtxoTx, UTXO_LOCK}; -use crate::{CoinWithDerivationMethod, GetWithdrawSenderAddress, MarketCoinOps, TransactionDetails, WithdrawError, - WithdrawFee, WithdrawFrom, WithdrawRequest, WithdrawResult}; +use crate::{CoinWithDerivationMethod, GetWithdrawSenderAddress, MarketCoinOps, TransactionDetails, UtxoFeePriority, + WithdrawError, WithdrawFee, WithdrawFrom, WithdrawRequest, WithdrawResult}; use async_trait::async_trait; use chain::TransactionOutput; use common::log::info; @@ -16,6 +18,7 @@ use crypto::{from_hw_error, CryptoCtx, CryptoCtxError, DerivationPath, HwError, use keys::{AddressFormat, AddressHashEnum, KeyPair, Private, Public as PublicKey}; use mm2_core::mm_ctx::MmArc; use mm2_err_handle::prelude::*; +use mm2_number::BigDecimal; use rpc::v1::types::ToTxHash; use rpc_task::RpcTaskError; use script::{Builder, Script, SignatureVersion, TransactionInputSigner}; @@ -177,6 +180,16 @@ where let dynamic = sat_from_big_decimal(amount, decimals)?; tx_builder = tx_builder.with_fee(TxFeeType::PerKb(dynamic)); }, + Some(WithdrawFee::UtxoPriority { ref priority, .. }) => { + let builder_with_fee = generate_withdrawal_fee_by_priority( + coin.as_ref().rpc_client.clone(), + priority, + decimals, + tx_builder, + ) + .await?; + tx_builder = builder_with_fee; + }, Some(ref fee_policy) => { let error = format!( "Expected 'UtxoFixed' or 'UtxoPerKbyte' fee types, found {:?}", @@ -227,6 +240,33 @@ where } } +/// estimates the fee for a transaction using the provided RPC client, priority level +/// and then updates the transaction builder with the calculated fee. +async fn generate_withdrawal_fee_by_priority<'a, Coin>( + rpc: UtxoRpcClientEnum, + priority: &UtxoFeePriority, + decimals: u8, + tx_builder: UtxoTxBuilder<'a, Coin>, +) -> MmResult, WithdrawError> +where + Coin: UtxoCommonOps + GetUtxoListOps, +{ + let (blocks, mode) = match priority { + UtxoFeePriority::Low => (3, EstimateFeeMode::ECONOMICAL), + UtxoFeePriority::Normal => (2, EstimateFeeMode::CONSERVATIVE), + UtxoFeePriority::High => (1, EstimateFeeMode::CONSERVATIVE), + }; + let method = EstimateFeeMethod::SmartFee; + let estimated_fee = rpc + .estimate_fee_sat(decimals, &method, &Some(mode.clone()), blocks) + .compat() + .await?; + let final_fee = BigDecimal::from(estimated_fee); + let dynamic = sat_from_big_decimal(&final_fee, decimals)?; + + Ok(tx_builder.with_fee(TxFeeType::PerKb(dynamic))) +} + pub struct InitUtxoWithdraw { ctx: MmArc, coin: Coin, diff --git a/mm2src/coins/z_coin.rs b/mm2src/coins/z_coin.rs index 98ba1456ba..d99995b044 100644 --- a/mm2src/coins/z_coin.rs +++ b/mm2src/coins/z_coin.rs @@ -508,7 +508,7 @@ impl ZCoin { fee_amount: sat_from_big_decimal(&tx_fee, self.decimals())?, unused_change: 0, kmd_rewards: None, - tx_v_size: tx_bytes.len() as u64, + tx_size: tx_bytes.len() as u64, }; Ok((tx, additional_data, sync_guard)) From 8e14509c3df8974e787906ddca95d01f8d28e961 Mon Sep 17 00:00:00 2001 From: Samuel Onoja Date: Tue, 12 Mar 2024 17:39:25 +0100 Subject: [PATCH 06/16] fix WASM build --- mm2src/coins/z_coin.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mm2src/coins/z_coin.rs b/mm2src/coins/z_coin.rs index d99995b044..3ef56138aa 100644 --- a/mm2src/coins/z_coin.rs +++ b/mm2src/coins/z_coin.rs @@ -7,7 +7,8 @@ use crate::utxo::rpc_clients::{ElectrumRpcRequest, UnspentInfo, UtxoRpcClientEnu use crate::utxo::utxo_builder::UtxoCoinBuildError; use crate::utxo::utxo_builder::{UtxoCoinBuilder, UtxoCoinBuilderCommonOps, UtxoFieldsWithGlobalHDBuilder, UtxoFieldsWithHardwareWalletBuilder, UtxoFieldsWithIguanaSecretBuilder}; -use crate::utxo::utxo_common::{big_decimal_from_sat_unsigned, payment_script, PreImageTradeFeeResult}; +use crate::utxo::utxo_common::{addresses_from_script, big_decimal_from_sat_unsigned, payment_script, + PreImageTradeFeeResult}; use crate::utxo::{sat_from_big_decimal, utxo_common, AdditionalTxData, AddrFromStrError, Address, BroadcastTxErr, FeePolicy, GetUtxoListOps, HistoryUtxoTx, HistoryUtxoTxMap, HtlcSpendFeeResult, MatureUnspentList, RecentlySpentOutPointsGuard, UtxoActivationParams, UtxoAddressFormat, UtxoArc, UtxoCoinFields, @@ -85,7 +86,7 @@ use z_rpc::init_light_client; pub use z_rpc::{FirstSyncBlock, SyncStatus}; cfg_native!( - use crate::utxo::utxo_common::{addresses_from_script, big_decimal_from_sat}; + use crate::utxo::utxo_common::big_decimal_from_sat; use common::{async_blocking, sha256_digest, calc_total_pages, PagingOptionsEnum}; use db_common::sqlite::offset_by_id; use db_common::sqlite::rusqlite::{Error as SqlError, Row}; From 0a8f11037eaf14d1a0751d07e2a8e52b0157f8bf Mon Sep 17 00:00:00 2001 From: Samuel Onoja Date: Mon, 18 Mar 2024 02:18:01 +0100 Subject: [PATCH 07/16] implement utxo coin withdrawal fee by priority --- mm2src/coins/lp_coins.rs | 4 +++ mm2src/coins/utxo/utxo_common.rs | 4 +-- mm2src/coins/utxo/utxo_withdraw.rs | 55 +++++++++++------------------- 3 files changed, 25 insertions(+), 38 deletions(-) diff --git a/mm2src/coins/lp_coins.rs b/mm2src/coins/lp_coins.rs index 2ac27b4319..33474ce92b 100644 --- a/mm2src/coins/lp_coins.rs +++ b/mm2src/coins/lp_coins.rs @@ -1822,12 +1822,16 @@ pub enum UtxoFeePriority { #[derive(Clone, Debug, Deserialize, PartialEq)] #[serde(tag = "type")] pub enum WithdrawFee { + /// encapsulates the fixed fee amount for a withdrawal transaction, regardless of transaction size. UtxoFixed { amount: BigDecimal, }, + /// encapsulates the fee amount for a withdrawal transaction calculated based on the transaction size in kilobytes. UtxoPerKbyte { amount: BigDecimal, }, + /// encapsulates the priority of a withdrawal transaction, indicating the desired fee + /// level for transaction processing. UtxoPriority { priority: UtxoFeePriority, }, diff --git a/mm2src/coins/utxo/utxo_common.rs b/mm2src/coins/utxo/utxo_common.rs index da12e2d69b..04e04577d3 100644 --- a/mm2src/coins/utxo/utxo_common.rs +++ b/mm2src/coins/utxo/utxo_common.rs @@ -881,8 +881,8 @@ impl<'a, T: AsRef + UtxoTxGenerationOps> UtxoTxBuilder<'a, T> { ) -> bool { self.tx_fee = match &actual_tx_fee { TxFeeType::PerKb(f) => { - let transaction = UtxoTx::from(self.tx.clone()); - let v_size = tx_size_in_v_bytes(from_addr_format, &transaction) as u64; + let tx_size = UtxoTx::from(self.tx.clone()); + let v_size = tx_size_in_v_bytes(from_addr_format, &tx_size) as u64; calculate_fee!(f, v_size) as u64 }, TxFeeType::Fixed(f) => *f, diff --git a/mm2src/coins/utxo/utxo_withdraw.rs b/mm2src/coins/utxo/utxo_withdraw.rs index 2d8e53ee0a..35be787a49 100644 --- a/mm2src/coins/utxo/utxo_withdraw.rs +++ b/mm2src/coins/utxo/utxo_withdraw.rs @@ -1,6 +1,4 @@ -use crate::common::Future01CompatExt; use crate::rpc_command::init_withdraw::{WithdrawInProgressStatus, WithdrawTaskHandleShared}; -use crate::utxo::rpc_clients::{EstimateFeeMethod, EstimateFeeMode, UtxoRpcClientEnum}; use crate::utxo::utxo_common::{big_decimal_from_sat, UtxoTxBuilder}; use crate::utxo::{output_script, sat_from_big_decimal, Address, AddressBuilder, FeePolicy, GetUtxoListOps, PrivKeyPolicy, TxFeeType, UtxoAddressFormat, UtxoCoinFields, UtxoCommonOps, UtxoFeeDetails, UtxoTx, @@ -18,7 +16,6 @@ use crypto::{from_hw_error, CryptoCtx, CryptoCtxError, DerivationPath, HwError, use keys::{AddressFormat, AddressHashEnum, KeyPair, Private, Public as PublicKey}; use mm2_core::mm_ctx::MmArc; use mm2_err_handle::prelude::*; -use mm2_number::BigDecimal; use rpc::v1::types::ToTxHash; use rpc_task::RpcTaskError; use script::{Builder, Script, SignatureVersion, TransactionInputSigner}; @@ -29,6 +26,18 @@ use utxo_signer::sign_params::{OutputDestination, SendingOutputInfo, SpendingInp use utxo_signer::{with_key_pair, UtxoSignTxError}; use utxo_signer::{SignPolicy, UtxoSignerOps}; +/// Fee (in satoshis) applied to withdrawal transactions +/// that have a high priority, indicating a need for faster confirmation. +pub const HIGH_TX_FEE: u16 = 4500; + +/// Fee (in satoshis) applied to withdrawal transactions +/// that have a normal priority, indicating a moderate confirmation time. +pub const NORMAL_TX_FEE: u16 = 2000; + +/// Fee (in satoshis) applied to withdrawal transactions +/// that have a low priority, indicating a longer confirmation time. +pub const LOW_TX_FEE: u16 = 1000; + impl From for WithdrawError { fn from(sign_err: UtxoSignTxError) -> Self { match sign_err { @@ -181,14 +190,13 @@ where tx_builder = tx_builder.with_fee(TxFeeType::PerKb(dynamic)); }, Some(WithdrawFee::UtxoPriority { ref priority, .. }) => { - let builder_with_fee = generate_withdrawal_fee_by_priority( - coin.as_ref().rpc_client.clone(), - priority, - decimals, - tx_builder, - ) - .await?; - tx_builder = builder_with_fee; + // handle the withdrawal fee calculation based on the priority of the transaction. + let fee = match priority { + UtxoFeePriority::Low => LOW_TX_FEE, + UtxoFeePriority::Normal => NORMAL_TX_FEE, + UtxoFeePriority::High => HIGH_TX_FEE, + }; + tx_builder = tx_builder.with_fee(TxFeeType::PerKb(fee.into())); }, Some(ref fee_policy) => { let error = format!( @@ -242,31 +250,6 @@ where /// estimates the fee for a transaction using the provided RPC client, priority level /// and then updates the transaction builder with the calculated fee. -async fn generate_withdrawal_fee_by_priority<'a, Coin>( - rpc: UtxoRpcClientEnum, - priority: &UtxoFeePriority, - decimals: u8, - tx_builder: UtxoTxBuilder<'a, Coin>, -) -> MmResult, WithdrawError> -where - Coin: UtxoCommonOps + GetUtxoListOps, -{ - let (blocks, mode) = match priority { - UtxoFeePriority::Low => (3, EstimateFeeMode::ECONOMICAL), - UtxoFeePriority::Normal => (2, EstimateFeeMode::CONSERVATIVE), - UtxoFeePriority::High => (1, EstimateFeeMode::CONSERVATIVE), - }; - let method = EstimateFeeMethod::SmartFee; - let estimated_fee = rpc - .estimate_fee_sat(decimals, &method, &Some(mode.clone()), blocks) - .compat() - .await?; - let final_fee = BigDecimal::from(estimated_fee); - let dynamic = sat_from_big_decimal(&final_fee, decimals)?; - - Ok(tx_builder.with_fee(TxFeeType::PerKb(dynamic))) -} - pub struct InitUtxoWithdraw { ctx: MmArc, coin: Coin, From d445554a90f513b5f70f5d8c252bfaebf395f76f Mon Sep 17 00:00:00 2001 From: Samuel Onoja Date: Mon, 18 Mar 2024 20:53:29 +0100 Subject: [PATCH 08/16] fix unnecessary clone in test_sender_trade_preimage_with_allowance --- mm2src/coins/qrc20/qrc20_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm2src/coins/qrc20/qrc20_tests.rs b/mm2src/coins/qrc20/qrc20_tests.rs index 361720efab..7da40f3bf9 100644 --- a/mm2src/coins/qrc20/qrc20_tests.rs +++ b/mm2src/coins/qrc20/qrc20_tests.rs @@ -877,7 +877,7 @@ fn test_sender_trade_preimage_with_allowance() { // the expected fee should not include any `approve` contract call let expected = TradeFee { coin: "QTUM".to_owned(), - amount: (erc20_payment_fee_without_approve + sender_refund_fee.clone()).into(), + amount: (erc20_payment_fee_without_approve + sender_refund_fee).into(), paid_from_trading_vol: false, tx_size: actual.tx_size, }; From 2081c2556fc6efaf08eb33fd8cf53153a6ff21e2 Mon Sep 17 00:00:00 2001 From: Samuel Onoja Date: Thu, 4 Apr 2024 13:15:07 +0100 Subject: [PATCH 09/16] remove debug println --- mm2src/coins/qrc20/qrc20_tests.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/mm2src/coins/qrc20/qrc20_tests.rs b/mm2src/coins/qrc20/qrc20_tests.rs index 7da40f3bf9..292f8d6ce1 100644 --- a/mm2src/coins/qrc20/qrc20_tests.rs +++ b/mm2src/coins/qrc20/qrc20_tests.rs @@ -166,7 +166,6 @@ fn test_withdraw_impl_fee_details() { "total_gas_fee": "1", })) .unwrap(); - println!("MINER_FEE {expected_miner_fee}"); assert_eq!(tx_details.fee_details, Some(TxFeeDetails::Qrc20(expected))); } From d26490f37733f06bda4ef6128fb7cd38c38b8669 Mon Sep 17 00:00:00 2001 From: Samuel Onoja Date: Sun, 12 May 2024 05:36:48 +0100 Subject: [PATCH 10/16] rename total_fee -> qrc20_payment_fee and minor changes in z_coin --- mm2src/coins/qrc20.rs | 4 ++-- mm2src/coins/z_coin.rs | 17 ++++++++--------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/mm2src/coins/qrc20.rs b/mm2src/coins/qrc20.rs index 862c95aa8f..69c9b0de7a 100644 --- a/mm2src/coins/qrc20.rs +++ b/mm2src/coins/qrc20.rs @@ -1408,10 +1408,10 @@ impl MmCoin for Qrc20Coin { .await? }; - let total_fee = erc20_payment_fee.fee + sender_refund_fee.fee; + let qrc20_payment_fee = erc20_payment_fee.fee + sender_refund_fee.fee; Ok(TradeFee { coin: self.platform.clone(), - amount: total_fee.into(), + amount: qrc20_payment_fee.into(), paid_from_trading_vol: false, tx_size: sender_refund_fee.tx_size, }) diff --git a/mm2src/coins/z_coin.rs b/mm2src/coins/z_coin.rs index afb85e1a8e..a523949561 100644 --- a/mm2src/coins/z_coin.rs +++ b/mm2src/coins/z_coin.rs @@ -15,8 +15,8 @@ use crate::utxo::rpc_clients::{ElectrumRpcRequest, UnspentInfo, UtxoRpcClientEnu use crate::utxo::utxo_builder::UtxoCoinBuildError; use crate::utxo::utxo_builder::{UtxoCoinBuilder, UtxoCoinBuilderCommonOps, UtxoFieldsWithGlobalHDBuilder, UtxoFieldsWithHardwareWalletBuilder, UtxoFieldsWithIguanaSecretBuilder}; -use crate::utxo::utxo_common::{addresses_from_script, big_decimal_from_sat, big_decimal_from_sat_unsigned, - payment_script, PreImageTradeFeeResult}; +use crate::utxo::utxo_common::{big_decimal_from_sat, big_decimal_from_sat_unsigned, payment_script, + PreImageTradeFeeResult}; use crate::utxo::{sat_from_big_decimal, utxo_common, AdditionalTxData, AddrFromStrError, Address, BroadcastTxErr, FeePolicy, GetUtxoListOps, HistoryUtxoTx, HistoryUtxoTxMap, HtlcSpendFeeResult, MatureUnspentList, RecentlySpentOutPointsGuard, UtxoActivationParams, UtxoAddressFormat, UtxoArc, UtxoCoinFields, @@ -463,16 +463,13 @@ impl ZCoin { .await? .tx_result?; - let mut tx_bytes = Vec::with_capacity(1024); - tx.write(&mut tx_bytes).expect("Write should not fail"); - let additional_data = AdditionalTxData { received_by_me, spent_by_me: sat_from_big_decimal(&total_input_amount, self.decimals())?, fee_amount: sat_from_big_decimal(&tx_fee, self.decimals())?, unused_change: 0, kmd_rewards: None, - tx_size: tx_bytes.len() as u64, + tx_size: tx.tx_hex().len() as u64, }; Ok((tx, additional_data, sync_guard)) @@ -535,7 +532,9 @@ impl ZCoin { if let Some(spent_output) = prev_tx.vout.get(input.prevout.n() as usize) { transparent_input_amount += spent_output.value; - if let Ok(addresses) = addresses_from_script(self, &spent_output.script_pubkey.0.clone().into()) { + if let Ok(addresses) = + utxo_common::addresses_from_script(self, &spent_output.script_pubkey.0.clone().into()) + { from.extend(addresses.into_iter().map(|a| a.to_string())); } } @@ -548,7 +547,7 @@ impl ZCoin { let mut to = HashSet::new(); for out in z_tx.vout.iter() { - if let Ok(addresses) = addresses_from_script(self, &out.script_pubkey.0.clone().into()) { + if let Ok(addresses) = utxo_common::addresses_from_script(self, &out.script_pubkey.0.clone().into()) { to.extend(addresses.into_iter().map(|a| a.to_string())); } } @@ -1834,7 +1833,7 @@ impl UtxoCommonOps for ZCoin { } fn addresses_from_script(&self, script: &Script) -> Result, String> { - addresses_from_script(self, script) + utxo_common::addresses_from_script(self, script) } fn denominate_satoshis(&self, satoshi: i64) -> f64 { utxo_common::denominate_satoshis(&self.utxo_arc, satoshi) } From 15f6f0f02857c6eb1778362a147e4abfc2a87a11 Mon Sep 17 00:00:00 2001 From: Samuel Onoja Date: Sun, 12 May 2024 22:41:57 +0100 Subject: [PATCH 11/16] fix test_generate_tx_doge_fee --- mm2src/coins/utxo/utxo_tests.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/mm2src/coins/utxo/utxo_tests.rs b/mm2src/coins/utxo/utxo_tests.rs index 9107379d65..002d2493a8 100644 --- a/mm2src/coins/utxo/utxo_tests.rs +++ b/mm2src/coins/utxo/utxo_tests.rs @@ -2799,8 +2799,11 @@ fn test_generate_tx_doge_fee() { let builder = block_on(UtxoTxBuilder::new(&doge)) .add_available_inputs(unspents) .add_outputs(outputs); - let (_, data) = block_on(builder.build()).unwrap(); - let expected_fee = 2000000; + let (input_signer, data) = block_on(builder.build()).unwrap(); + let transaction = UtxoTx::from(input_signer); + let v_size = tx_size_in_v_bytes(&UtxoAddressFormat::Standard, &transaction) as u64; + assert!(v_size > KILO_BYTE as u64); + let expected_fee = calculate_fee!(TXFEE_PER_KB, v_size) as u64; assert_eq!(expected_fee, data.fee_amount); let unspents = vec![UnspentInfo { @@ -2819,7 +2822,8 @@ fn test_generate_tx_doge_fee() { let builder = block_on(UtxoTxBuilder::new(&doge)) .add_available_inputs(unspents) .add_outputs(outputs); - let (_, data) = block_on(builder.build()).unwrap(); + let (input_signer, data) = block_on(builder.build()).unwrap(); + let transaction = UtxoTx::from(input_signer); let v_size = tx_size_in_v_bytes(&UtxoAddressFormat::Standard, &transaction) as u64; assert!(v_size > KILO_BYTE as u64); let expected_fee = calculate_fee!(TXFEE_PER_KB, v_size) as u64; From da566caf71aff1615f68e7123b0f57139468562d Mon Sep 17 00:00:00 2001 From: Samuel Onoja Date: Mon, 13 May 2024 05:51:07 +0100 Subject: [PATCH 12/16] use estimate_fee_sat for calculating fee based on priority --- mm2src/coins/utxo/utxo_withdraw.rs | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/mm2src/coins/utxo/utxo_withdraw.rs b/mm2src/coins/utxo/utxo_withdraw.rs index d67551a52d..622df958f7 100644 --- a/mm2src/coins/utxo/utxo_withdraw.rs +++ b/mm2src/coins/utxo/utxo_withdraw.rs @@ -7,7 +7,7 @@ use crate::{CoinWithDerivationMethod, GetWithdrawSenderAddress, MarketCoinOps, T use async_trait::async_trait; use chain::TransactionOutput; use common::log::info; -use common::now_sec; +use common::{now_sec, Future01CompatExt}; use crypto::hw_rpc_task::HwRpcTaskAwaitingStatus; use crypto::trezor::trezor_rpc_task::{TrezorRequestStatuses, TrezorRpcTaskProcessor}; use crypto::trezor::{TrezorError, TrezorProcessingError}; @@ -25,17 +25,17 @@ use utxo_signer::sign_params::{OutputDestination, SendingOutputInfo, SpendingInp use utxo_signer::{with_key_pair, UtxoSignTxError}; use utxo_signer::{SignPolicy, UtxoSignerOps}; -/// Fee (in satoshis) applied to withdrawal transactions +/// Num of blocks applied to withdrawal transactions /// that have a high priority, indicating a need for faster confirmation. -pub const HIGH_TX_FEE: u16 = 4500; +pub const HIGH_TX_FEE: u8 = 1; -/// Fee (in satoshis) applied to withdrawal transactions +/// Num of blocks applied to withdrawal transactions /// that have a normal priority, indicating a moderate confirmation time. -pub const NORMAL_TX_FEE: u16 = 2000; +pub const NORMAL_TX_FEE: u8 = 2; -/// Fee (in satoshis) applied to withdrawal transactions +/// Num of blocks applied to withdrawal transactions /// that have a low priority, indicating a longer confirmation time. -pub const LOW_TX_FEE: u16 = 1000; +pub const LOW_TX_FEE: u8 = 4; impl From for WithdrawError { fn from(sign_err: UtxoSignTxError) -> Self { @@ -191,12 +191,24 @@ where }, Some(WithdrawFee::UtxoPriority { ref priority, .. }) => { // handle the withdrawal fee calculation based on the priority of the transaction. - let fee = match priority { + let n_blocks = match priority { UtxoFeePriority::Low => LOW_TX_FEE, UtxoFeePriority::Normal => NORMAL_TX_FEE, UtxoFeePriority::High => HIGH_TX_FEE, }; - tx_builder = tx_builder.with_fee(TxFeeType::PerKb(fee.into())); + // Note: If the coin doesn't support smart_fee with n_blocks, we skip using n_blocks. So, smart_fee returns a fee without using n_blocks. + let fee_estimate = coin + .as_ref() + .rpc_client + .estimate_fee_sat( + decimals, + &crate::utxo::rpc_clients::EstimateFeeMethod::Standard, + &None, + n_blocks.into(), + ) + .compat() + .await?; + tx_builder = tx_builder.with_fee(TxFeeType::PerKb(fee_estimate)); }, Some(ref fee_policy) => { let error = format!( From e4fdfb2639a108a276b422cd990c92c50cad337c Mon Sep 17 00:00:00 2001 From: Samuel Onoja Date: Mon, 27 May 2024 16:40:37 +0100 Subject: [PATCH 13/16] fix qrc20 unit tests and minor changes --- mm2src/coins/eth.rs | 42 ++++++++++---------- mm2src/coins/eth/eth_tests.rs | 8 ++-- mm2src/coins/lightning.rs | 6 +-- mm2src/coins/lp_coins.rs | 4 +- mm2src/coins/qrc20.rs | 8 +++- mm2src/coins/qrc20/qrc20_tests.rs | 14 +++---- mm2src/coins/tendermint/tendermint_coin.rs | 4 +- mm2src/coins/utxo.rs | 4 +- mm2src/coins/utxo/utxo_common.rs | 12 +++--- mm2src/coins/utxo/utxo_tests.rs | 4 +- mm2src/coins/z_coin.rs | 4 +- mm2src/mm2_main/src/lp_swap.rs | 2 +- mm2src/mm2_main/src/lp_swap/taker_swap.rs | 2 +- mm2src/mm2_main/src/lp_swap/taker_swap_v2.rs | 2 +- 14 files changed, 60 insertions(+), 56 deletions(-) diff --git a/mm2src/coins/eth.rs b/mm2src/coins/eth.rs index 4bfe733a5d..20f3062dc2 100644 --- a/mm2src/coins/eth.rs +++ b/mm2src/coins/eth.rs @@ -5506,24 +5506,26 @@ impl MmCoin for EthCoin { fn history_sync_status(&self) -> HistorySyncState { self.history_sync_state.lock().unwrap().clone() } fn get_trade_fee(&self) -> Box + Send> { - let coin = self.clone(); + let selfi = self.clone(); Box::new( - self.get_gas_price() - .map_err(|e| e.to_string()) - .and_then(move |gas_price| { - let fee = gas_price * U256::from(ETH_GAS); - let fee_coin = match &coin.coin_type { - EthCoinType::Eth => &coin.ticker, - EthCoinType::Erc20 { platform, .. } => platform, - EthCoinType::Nft { .. } => return ERR!("Nft Protocol is not supported yet!"), - }; - Ok(TradeFee { - coin: fee_coin.into(), - amount: try_s!(u256_to_big_decimal(fee, ETH_DECIMALS)).into(), - paid_from_trading_vol: false, - tx_size: None, - }) - }), + async move { + let gas_price = try_s!(selfi.get_gas_price().await); + let fee = gas_price * U256::from(gas_limit::ETH_MAX_TRADE_GAS); + let fee_coin = match &selfi.coin_type { + EthCoinType::Eth => &selfi.ticker, + EthCoinType::Erc20 { platform, .. } => platform, + EthCoinType::Nft { .. } => return ERR!("Nft Protocol is not supported yet!"), + }; + + Ok(TradeFee { + coin: fee_coin.into(), + amount: try_s!(u256_to_big_decimal(fee, ETH_DECIMALS)).into(), + paid_from_trading_vol: false, + tx_size: 0, + }) + } + .boxed() + .compat(), ) } @@ -5587,7 +5589,7 @@ impl MmCoin for EthCoin { coin: fee_coin.into(), amount: amount.into(), paid_from_trading_vol: false, - tx_size: None, + tx_size: 0, }) } @@ -5614,7 +5616,7 @@ impl MmCoin for EthCoin { coin: fee_coin.into(), amount: amount.into(), paid_from_trading_vol: false, - tx_size: None, + tx_size: 0, }) }; Box::new(fut.boxed().compat()) @@ -5664,7 +5666,7 @@ impl MmCoin for EthCoin { coin: fee_coin.into(), amount: amount.into(), paid_from_trading_vol: false, - tx_size: None, + tx_size: 0, }) } diff --git a/mm2src/coins/eth/eth_tests.rs b/mm2src/coins/eth/eth_tests.rs index e2873c097e..0b0a12516b 100644 --- a/mm2src/coins/eth/eth_tests.rs +++ b/mm2src/coins/eth/eth_tests.rs @@ -315,7 +315,7 @@ fn get_sender_trade_preimage() { coin: "ETH".to_owned(), amount: amount.into(), paid_from_trading_vol: false, - tx_size: None, + tx_size: 0, } } @@ -383,7 +383,7 @@ fn get_erc20_sender_trade_preimage() { coin: "ETH".to_owned(), amount: amount.into(), paid_from_trading_vol: false, - tx_size: None, + tx_size: 0, } } @@ -476,7 +476,7 @@ fn get_receiver_trade_preimage() { coin: "ETH".to_owned(), amount: amount.into(), paid_from_trading_vol: false, - tx_size: None, + tx_size: 0, }; let actual = coin @@ -501,7 +501,7 @@ fn test_get_fee_to_send_taker_fee() { coin: "ETH".to_owned(), amount: amount.into(), paid_from_trading_vol: false, - tx_size: None, + tx_size: 0, }; let dex_fee_amount = u256_to_big_decimal(DEX_FEE_AMOUNT.into(), 18).expect("!u256_to_big_decimal"); diff --git a/mm2src/coins/lightning.rs b/mm2src/coins/lightning.rs index 032297e5ba..7b5c20eba0 100644 --- a/mm2src/coins/lightning.rs +++ b/mm2src/coins/lightning.rs @@ -1342,7 +1342,7 @@ impl MmCoin for LightningCoin { coin: self.ticker().to_owned(), amount: Default::default(), paid_from_trading_vol: false, - tx_size: None, + tx_size: 0, }) } @@ -1352,7 +1352,7 @@ impl MmCoin for LightningCoin { coin: self.ticker().to_owned(), amount: Default::default(), paid_from_trading_vol: false, - tx_size: None, + tx_size: 0, })) } @@ -1366,7 +1366,7 @@ impl MmCoin for LightningCoin { coin: self.ticker().to_owned(), amount: Default::default(), paid_from_trading_vol: false, - tx_size: None, + tx_size: 0, }) } diff --git a/mm2src/coins/lp_coins.rs b/mm2src/coins/lp_coins.rs index 22bda43760..db42fa4f73 100644 --- a/mm2src/coins/lp_coins.rs +++ b/mm2src/coins/lp_coins.rs @@ -2338,7 +2338,7 @@ pub struct TradeFee { pub coin: String, pub amount: MmNumber, pub paid_from_trading_vol: bool, - pub tx_size: Option, + pub tx_size: u64, } /// A type alias for a HashMap where the key is a String representing the coin/token ticker, @@ -4862,7 +4862,7 @@ pub async fn my_tx_history(ctx: MmArc, req: Json) -> Result>, S } /// `get_trade_fee` rpc implementation. -/// There is some consideration about this rpc: +/// There is some consideration about this rpc: /// for eth coin this rpc returns max possible trade fee (estimated for maximum possible gas limit for any kind of swap). /// However for eth coin, as part of fixing this issue https://github.com/KomodoPlatform/komodo-defi-framework/issues/1848, /// `max_taker_vol' and `trade_preimage` rpc now return more accurate required gas calculations. diff --git a/mm2src/coins/qrc20.rs b/mm2src/coins/qrc20.rs index 9928edb74f..1ff1b5e54b 100644 --- a/mm2src/coins/qrc20.rs +++ b/mm2src/coins/qrc20.rs @@ -1354,7 +1354,7 @@ impl MmCoin for Qrc20Coin { coin: selfi.platform.clone(), amount: big_decimal_from_sat(fee as i64, selfi.utxo.decimals).into(), paid_from_trading_vol: false, - tx_size: None, + tx_size: 0, }) }; Box::new(fut.boxed().compat()) @@ -1403,7 +1403,11 @@ impl MmCoin for Qrc20Coin { self.preimage_trade_fee_required_to_send_outputs(vec![sender_refund_output], &stage) .await? } else { - BigDecimal::from(0) // No refund fee if not included. + // No refund fee if not included. + PreImageTradeFeeResult { + fee: 0.into(), + tx_size: 0, + } }; let qrc20_payment_fee = erc20_payment_fee.fee + sender_refund_fee.fee; diff --git a/mm2src/coins/qrc20/qrc20_tests.rs b/mm2src/coins/qrc20/qrc20_tests.rs index bff20be06b..49066f973c 100644 --- a/mm2src/coins/qrc20/qrc20_tests.rs +++ b/mm2src/coins/qrc20/qrc20_tests.rs @@ -145,7 +145,7 @@ fn test_withdraw_impl_fee_details() { }; let tx_details = coin.withdraw(withdraw_req).wait().unwrap(); - let tx_size_kb = tx_details.tx_hex.into_vec().len() as f64 / KILO_BYTE; + let tx_size_kb = tx_details.tx.tx_hex().cloned().unwrap().len() as f64 / KILO_BYTE; // In transaction size calculations we assume the script sig size is (2 + MAX_DER_SIGNATURE_LEN + COMPRESSED_PUBKEY_LEN) or 107 bytes // when in reality signatures can vary by 1 or 2 bytes because of possible zero padding of r and s values of the signature. // This is why we test for a range of values here instead of a single value. The value we use in fees calculation is the @@ -760,7 +760,7 @@ fn test_get_trade_fee() { coin: "QTUM".into(), amount: expected_trade_fee_amount.into(), paid_from_trading_vol: false, - tx_size: None, + tx_size: 0, }; assert_eq!(actual_trade_fee, expected); } @@ -792,7 +792,7 @@ fn test_sender_trade_preimage_zero_allowance() { let actual = block_on(coin.get_sender_trade_fee(TradePreimageValue::Exact(1.into()), FeeApproxStage::WithoutApprox, true)) .expect("!get_sender_trade_fee"); - let actual_tx_fee = get_expected_trade_fee_per_kb(actual.tx_size.unwrap()); + let actual_tx_fee = get_expected_trade_fee_per_kb(actual.tx_size); let sender_refund_fee = big_decimal_from_sat(actual_tx_fee + CONTRACT_CALL_GAS_FEE, coin.utxo.decimals); // one `approve` contract call should be included into the expected trade fee @@ -841,7 +841,7 @@ fn test_sender_trade_preimage_with_allowance() { true, )) .expect("!get_sender_trade_fee"); - let expected_fee = get_expected_trade_fee_per_kb(actual.tx_size.unwrap()); + let expected_fee = get_expected_trade_fee_per_kb(actual.tx_size); let sender_refund_fee = big_decimal_from_sat(CONTRACT_CALL_GAS_FEE + expected_fee, coin.utxo.decimals); // the expected fee should not include any `approve` contract call @@ -859,7 +859,7 @@ fn test_sender_trade_preimage_with_allowance() { true, )) .expect("!get_sender_trade_fee"); - let expected_fee = get_expected_trade_fee_per_kb(actual.tx_size.unwrap()); + let expected_fee = get_expected_trade_fee_per_kb(actual.tx_size); let sender_refund_fee = big_decimal_from_sat(CONTRACT_CALL_GAS_FEE + expected_fee, coin.utxo.decimals); // two `approve` contract calls should be included into the expected trade fee let expected = TradeFee { @@ -948,7 +948,7 @@ fn test_receiver_trade_preimage() { .expect("!get_receiver_trade_fee"); // only one contract call should be included into the expected trade fee - let actual_tx_fee = get_expected_trade_fee_per_kb(actual.tx_size.unwrap()); + let actual_tx_fee = get_expected_trade_fee_per_kb(actual.tx_size); let expected_receiver_fee = big_decimal_from_sat(CONTRACT_CALL_GAS_FEE + actual_tx_fee, coin.utxo.decimals); let expected = TradeFee { coin: "QTUM".to_owned(), @@ -984,7 +984,7 @@ fn test_taker_fee_tx_fee() { )) .expect("!get_fee_to_send_taker_fee"); // only one contract call should be included into the expected trade fee - let actual_tx_fee = get_expected_trade_fee_per_kb(actual.tx_size.unwrap()); + let actual_tx_fee = get_expected_trade_fee_per_kb(actual.tx_size); let expected_receiver_fee = big_decimal_from_sat(CONTRACT_CALL_GAS_FEE + actual_tx_fee, coin.utxo.decimals); let expected = TradeFee { coin: "QTUM".to_owned(), diff --git a/mm2src/coins/tendermint/tendermint_coin.rs b/mm2src/coins/tendermint/tendermint_coin.rs index 1b833b7755..ea75fa197d 100644 --- a/mm2src/coins/tendermint/tendermint_coin.rs +++ b/mm2src/coins/tendermint/tendermint_coin.rs @@ -1697,7 +1697,7 @@ impl TendermintCoin { coin: ticker, amount: fee_amount.into(), paid_from_trading_vol: false, - tx_size: None, + tx_size: 0, }) } @@ -1741,7 +1741,7 @@ impl TendermintCoin { coin: ticker, amount: fee_amount.into(), paid_from_trading_vol: false, - tx_size: None, + tx_size: 0, }) } diff --git a/mm2src/coins/utxo.rs b/mm2src/coins/utxo.rs index 0ee991d43c..5e6d339278 100644 --- a/mm2src/coins/utxo.rs +++ b/mm2src/coins/utxo.rs @@ -964,11 +964,11 @@ impl MatureUnspentList { #[derive(Debug)] pub struct HtlcSpendFeeResult { pub fee: u64, - pub tx_size: Option, + pub tx_size: u64, } impl HtlcSpendFeeResult { - fn from(fee: u64, tx_size: Option) -> Self { Self { fee, tx_size } } + fn from(fee: u64, tx_size: u64) -> Self { Self { fee, tx_size } } } #[async_trait] diff --git a/mm2src/coins/utxo/utxo_common.rs b/mm2src/coins/utxo/utxo_common.rs index b0cc17ebf4..368cf44104 100644 --- a/mm2src/coins/utxo/utxo_common.rs +++ b/mm2src/coins/utxo/utxo_common.rs @@ -295,7 +295,7 @@ pub async fn get_htlc_spend_fee( } } - Ok(HtlcSpendFeeResult::from(fee, Some(tx_size))) + Ok(HtlcSpendFeeResult::from(fee, tx_size)) } pub fn addresses_from_script(coin: &T, script: &Script) -> Result, String> { @@ -1006,9 +1006,7 @@ async fn gen_taker_funding_spend_preimage( coin.get_htlc_spend_fee(DEFAULT_SWAP_TX_SPEND_SIZE, &FeeApproxStage::WithoutApprox) .await? }, - FundingSpendFeeSetting::UseExact(f) => { - HtlcSpendFeeResult::from(f, Some(args.funding_tx.serialized_size() as u64)) - }, + FundingSpendFeeSetting::UseExact(f) => HtlcSpendFeeResult::from(f, args.funding_tx.serialized_size() as u64), }; let fee_plus_dust = fee.fee + coin.as_ref().dust_amount; @@ -3797,7 +3795,7 @@ pub fn get_trade_fee(coin: T) -> Box(coin: T) -> Box, + pub tx_size: u64, } /// To ensure the `get_sender_trade_fee(x) <= get_sender_trade_fee(y)` condition is satisfied for any `x < y`, @@ -3877,7 +3875,7 @@ where Ok(PreImageTradeFeeResult { fee: big_decimal_from_sat(total_fee as i64, decimals), - tx_size: Some(data.tx_size), + tx_size: data.tx_size, }) } diff --git a/mm2src/coins/utxo/utxo_tests.rs b/mm2src/coins/utxo/utxo_tests.rs index df9216b0f4..d782764a1a 100644 --- a/mm2src/coins/utxo/utxo_tests.rs +++ b/mm2src/coins/utxo/utxo_tests.rs @@ -907,7 +907,7 @@ fn test_withdraw_kmd_rewards_impl( ibc_source_channel: None, }; let tx_details = coin.withdraw(withdraw_req).wait().unwrap(); - let tx_size_kb = tx_details.tx_hex.len() as f64 / KILO_BYTE; + let tx_size_kb = tx_details.tx.tx_hex().unwrap().len() as f64 / KILO_BYTE; // In transaction size calculations we assume the script sig size is (2 + MAX_DER_SIGNATURE_LEN + COMPRESSED_PUBKEY_LEN) or 107 bytes // when in reality signatures can vary by 1 or 2 bytes because of possible zero padding of r and s values of the signature. // This is why we test for a range of values here instead of a single value. The value we use in fees calculation is the @@ -999,7 +999,7 @@ fn test_withdraw_rick_rewards_none() { ibc_source_channel: None, }; let tx_details = coin.withdraw(withdraw_req).wait().unwrap(); - let tx_size_kb = tx_details.tx_hex.len() as f64 / KILO_BYTE; + let tx_size_kb = tx_details.tx.tx_hex().unwrap().len() as f64 / KILO_BYTE; // In transaction size calculations we assume the script sig size is (2 + MAX_DER_SIGNATURE_LEN + COMPRESSED_PUBKEY_LEN) or 107 bytes // when in reality signatures can vary by 1 or 2 bytes because of possible zero padding of r and s values of the signature. // This is why we test for a range of values here instead of a single value. The value we use in fees calculation is the diff --git a/mm2src/coins/z_coin.rs b/mm2src/coins/z_coin.rs index 49dcaeaad4..823b6c9065 100644 --- a/mm2src/coins/z_coin.rs +++ b/mm2src/coins/z_coin.rs @@ -1729,7 +1729,7 @@ impl MmCoin for ZCoin { coin: self.ticker().to_owned(), amount: self.get_one_kbyte_tx_fee().await?.into(), paid_from_trading_vol: false, - tx_size: None, + tx_size: 0, }) } @@ -1746,7 +1746,7 @@ impl MmCoin for ZCoin { coin: self.ticker().to_owned(), amount: self.get_one_kbyte_tx_fee().await?.into(), paid_from_trading_vol: false, - tx_size: None, + tx_size: 0, }) } diff --git a/mm2src/mm2_main/src/lp_swap.rs b/mm2src/mm2_main/src/lp_swap.rs index 875ac2e134..ac97d11367 100644 --- a/mm2src/mm2_main/src/lp_swap.rs +++ b/mm2src/mm2_main/src/lp_swap.rs @@ -1056,7 +1056,7 @@ impl From for TradeFee { coin: orig.coin, amount: orig.amount.into(), paid_from_trading_vol: orig.paid_from_trading_vol, - tx_size: None, + tx_size: 0, } } } diff --git a/mm2src/mm2_main/src/lp_swap/taker_swap.rs b/mm2src/mm2_main/src/lp_swap/taker_swap.rs index 09c502d36e..b12a752084 100644 --- a/mm2src/mm2_main/src/lp_swap/taker_swap.rs +++ b/mm2src/mm2_main/src/lp_swap/taker_swap.rs @@ -2470,7 +2470,7 @@ pub async fn taker_swap_trade_preimage( coin: my_coin_ticker.to_owned(), amount: dex_amount.total_spend_amount(), paid_from_trading_vol: false, - tx_size: None, + tx_size: 0, }; let fee_to_send_taker_fee = my_coin diff --git a/mm2src/mm2_main/src/lp_swap/taker_swap_v2.rs b/mm2src/mm2_main/src/lp_swap/taker_swap_v2.rs index 01a11e3534..b626d66c63 100644 --- a/mm2src/mm2_main/src/lp_swap/taker_swap_v2.rs +++ b/mm2src/mm2_main/src/lp_swap/taker_swap_v2.rs @@ -954,7 +954,7 @@ impl Date: Tue, 28 May 2024 10:58:16 +0100 Subject: [PATCH 14/16] fix test_withdraw_p2pk_balance unit test --- mm2src/coins/utxo/utxo_tests.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/mm2src/coins/utxo/utxo_tests.rs b/mm2src/coins/utxo/utxo_tests.rs index d782764a1a..0c18d5f8b8 100644 --- a/mm2src/coins/utxo/utxo_tests.rs +++ b/mm2src/coins/utxo/utxo_tests.rs @@ -3427,6 +3427,10 @@ fn test_withdraw_p2pk_balance() { let coin = utxo_coin_for_test(UtxoRpcClientEnum::Native(client), None, false); + const VALUE: u64 = 1000000000; + const TX_SIZE: u64 = 211; + const TXFEE_PER_KB: u64 = 1000; + UtxoStandardCoin::get_unspent_ordered_list.mock_safe(|coin, _| { let cache = block_on(coin.as_ref().recently_spent_outpoints.lock()); let unspents = vec![UnspentInfo { @@ -3434,7 +3438,7 @@ fn test_withdraw_p2pk_balance() { hash: 1.into(), index: 0, }, - value: 1000000000, + value: VALUE, height: Default::default(), // Use a p2pk output script for this UTXO script: output_script_p2pk( @@ -3449,8 +3453,9 @@ fn test_withdraw_p2pk_balance() { // Create a dummy p2pkh address to withdraw the coins to. let my_p2pkh_address = block_on(coin.as_ref().derivation_method.unwrap_single_addr()); + let total_value_sent = BigDecimal::from(1); let withdraw_req = WithdrawRequest { - amount: 1.into(), + amount: total_value_sent.clone(), from: None, to: my_p2pkh_address.to_string(), coin: TEST_COIN_NAME.into(), @@ -3467,8 +3472,13 @@ fn test_withdraw_p2pk_balance() { let expected_script = Builder::build_p2pkh(my_p2pkh_address.hash()); assert_eq!(output_script, expected_script); + let mut expected_fee = calculate_fee!(TXFEE_PER_KB, TX_SIZE) as u64; + expected_fee += calculate_fee!(TXFEE_PER_KB, P2PKH_OUTPUT_LEN) as u64; + let amount_sent = sat_from_big_decimal(&total_value_sent, TEST_COIN_DECIMALS).unwrap(); + let expected_balance = VALUE - amount_sent - expected_fee; + // And it should have this value (p2pk balance - amount sent - fees). - assert_eq!(transaction.outputs[1].value, 899999000); + assert_eq!(transaction.outputs[1].value, expected_balance); } /// `UtxoStandardCoin` has to check UTXO maturity if `check_utxo_maturity` is `true`. From 0fc8f98b7155d22ad7f08c667166e83d4b9092dc Mon Sep 17 00:00:00 2001 From: Samuel Onoja Date: Tue, 28 May 2024 14:55:16 +0100 Subject: [PATCH 15/16] add setting n_block from coin conf for withdrawal --- mm2src/coins/lp_coins.rs | 6 +- mm2src/coins/utxo.rs | 13 +++++ .../utxo/utxo_builder/utxo_conf_builder.rs | 10 +++- mm2src/coins/utxo/utxo_common_tests.rs | 1 + mm2src/coins/utxo/utxo_withdraw.rs | 58 ++++++++++++------- 5 files changed, 63 insertions(+), 25 deletions(-) diff --git a/mm2src/coins/lp_coins.rs b/mm2src/coins/lp_coins.rs index db42fa4f73..f3d20615d4 100644 --- a/mm2src/coins/lp_coins.rs +++ b/mm2src/coins/lp_coins.rs @@ -1939,11 +1939,11 @@ pub trait MarketCoinOps { /// Priority levels for UTXO fee estimation for withdrawal. #[derive(Clone, Debug, Deserialize, PartialEq)] pub enum UtxoFeePriority { - /// Low priority: Estimated confirmation within approximately 3 blocks. + /// Low priority. Low, - /// Normal priority: Estimated confirmation within approximately 2 blocks. + /// Normal priority. Normal, - /// High priority: Estimated confirmation within approximately 1 block. + /// High priority. High, } diff --git a/mm2src/coins/utxo.rs b/mm2src/coins/utxo.rs index 5e6d339278..db3f387c60 100644 --- a/mm2src/coins/utxo.rs +++ b/mm2src/coins/utxo.rs @@ -504,6 +504,17 @@ impl UtxoSyncStatusLoopHandle { } } +/// Priority levels for UTXO fee estimation for withdrawal. +#[derive(Clone, Debug, Deserialize)] +pub struct UtxoFeePriorities { + /// Low priority. Recommended (8-10 blocks atleast for btc) + pub low: u8, + /// Normal priority. Recommended (6 blocks atleast for btc) + pub normal: u8, + /// High priority. Recommended (2 blocks atleast for btc) + pub high: u8, +} + #[derive(Debug)] pub struct UtxoCoinConf { pub ticker: String, @@ -582,6 +593,8 @@ pub struct UtxoCoinConf { pub derivation_path: Option, /// The average time in seconds needed to mine a new block for this coin. pub avg_blocktime: Option, + /// The average time in seconds needed to mine a new block for this coin. + pub fee_priorities: Option, } pub struct UtxoCoinFields { diff --git a/mm2src/coins/utxo/utxo_builder/utxo_conf_builder.rs b/mm2src/coins/utxo/utxo_builder/utxo_conf_builder.rs index befbae70f9..19867557fc 100644 --- a/mm2src/coins/utxo/utxo_builder/utxo_conf_builder.rs +++ b/mm2src/coins/utxo/utxo_builder/utxo_conf_builder.rs @@ -1,6 +1,6 @@ use crate::utxo::rpc_clients::EstimateFeeMode; -use crate::utxo::{parse_hex_encoded_u32, UtxoCoinConf, DEFAULT_DYNAMIC_FEE_VOLATILITY_PERCENT, KMD_MTP_BLOCK_COUNT, - MATURE_CONFIRMATIONS_DEFAULT}; +use crate::utxo::{parse_hex_encoded_u32, UtxoCoinConf, UtxoFeePriorities, DEFAULT_DYNAMIC_FEE_VOLATILITY_PERCENT, + KMD_MTP_BLOCK_COUNT, MATURE_CONFIRMATIONS_DEFAULT}; use crate::UtxoActivationParams; use bitcrypto::ChecksumType; use crypto::{Bip32Error, HDPathToCoin}; @@ -113,6 +113,7 @@ impl<'a> UtxoConfBuilder<'a> { let derivation_path = self.derivation_path()?; let avg_blocktime = self.avg_blocktime(); let spv_conf = self.spv_conf()?; + let fee_priorities = self.fee_priorities(); Ok(UtxoCoinConf { ticker: self.ticker.to_owned(), @@ -145,6 +146,7 @@ impl<'a> UtxoConfBuilder<'a> { spv_conf, derivation_path, avg_blocktime, + fee_priorities, }) } @@ -313,4 +315,8 @@ impl<'a> UtxoConfBuilder<'a> { } fn avg_blocktime(&self) -> Option { self.conf["avg_blocktime"].as_u64() } + + fn fee_priorities(&self) -> Option { + json::from_value(self.conf["fee_priorities"].clone()).unwrap_or(None) + } } diff --git a/mm2src/coins/utxo/utxo_common_tests.rs b/mm2src/coins/utxo/utxo_common_tests.rs index 851f4a0644..c20d8cf2ad 100644 --- a/mm2src/coins/utxo/utxo_common_tests.rs +++ b/mm2src/coins/utxo/utxo_common_tests.rs @@ -133,6 +133,7 @@ pub(super) fn utxo_coin_fields_for_test( spv_conf: None, derivation_path: None, avg_blocktime: None, + fee_priorities: None, }, decimals: TEST_COIN_DECIMALS, dust_amount: UTXO_DUST_AMOUNT, diff --git a/mm2src/coins/utxo/utxo_withdraw.rs b/mm2src/coins/utxo/utxo_withdraw.rs index 6d8b25d6f5..6b845aae44 100644 --- a/mm2src/coins/utxo/utxo_withdraw.rs +++ b/mm2src/coins/utxo/utxo_withdraw.rs @@ -26,13 +26,15 @@ use utxo_signer::sign_params::{OutputDestination, SendingOutputInfo, SpendingInp use utxo_signer::{with_key_pair, UtxoSignTxError}; use utxo_signer::{SignPolicy, UtxoSignerOps}; +use super::UtxoFeePriorities; + /// Num of blocks applied to withdrawal transactions /// that have a high priority, indicating a need for faster confirmation. -pub const HIGH_TX_FEE: u8 = 1; +pub const HIGH_TX_FEE: u8 = 2; /// Num of blocks applied to withdrawal transactions /// that have a normal priority, indicating a moderate confirmation time. -pub const NORMAL_TX_FEE: u8 = 2; +pub const NORMAL_TX_FEE: u8 = 6; /// Num of blocks applied to withdrawal transactions /// that have a low priority, indicating a longer confirmation time. @@ -179,24 +181,10 @@ where }, Some(WithdrawFee::UtxoPriority { ref priority, .. }) => { // handle the withdrawal fee calculation based on the priority of the transaction. - let n_blocks = match priority { - UtxoFeePriority::Low => LOW_TX_FEE, - UtxoFeePriority::Normal => NORMAL_TX_FEE, - UtxoFeePriority::High => HIGH_TX_FEE, - }; - // Note: If the coin doesn't support smart_fee with n_blocks, we skip using n_blocks. So, smart_fee returns a fee without using n_blocks. - let fee_estimate = coin - .as_ref() - .rpc_client - .estimate_fee_sat( - decimals, - &crate::utxo::rpc_clients::EstimateFeeMethod::Standard, - &None, - n_blocks.into(), - ) - .compat() - .await?; - tx_builder = tx_builder.with_fee(TxFeeType::PerKb(fee_estimate)); + let fee = + generate_withdraw_fee_using_priority(coin, priority, &coin.as_ref().conf.fee_priorities, decimals) + .await?; + tx_builder = tx_builder.with_fee(TxFeeType::PerKb(fee)); }, Some(ref fee_policy) => { let error = format!( @@ -247,6 +235,36 @@ where } } +async fn generate_withdraw_fee_using_priority( + coin: &C, + priority: &UtxoFeePriority, + priorities: &Option, + decimals: u8, +) -> Result> { + let n_blocks = match (priorities, priority) { + (Some(details), UtxoFeePriority::Low) => details.low, + (Some(details), UtxoFeePriority::Normal) => details.normal, + (Some(details), UtxoFeePriority::High) => details.high, + (None, UtxoFeePriority::Low) => LOW_TX_FEE, + (None, UtxoFeePriority::Normal) => NORMAL_TX_FEE, + (None, UtxoFeePriority::High) => HIGH_TX_FEE, + }; + let estimate_fee_sat = coin + .as_ref() + .rpc_client + .estimate_fee_sat( + decimals, + &crate::utxo::rpc_clients::EstimateFeeMethod::Standard, + &None, + n_blocks.into(), + ) + .compat() + .await?; + + // let mem_pool_n_blocks = slurp_url(url) + Ok(estimate_fee_sat) +} + /// estimates the fee for a transaction using the provided RPC client, priority level /// and then updates the transaction builder with the calculated fee. pub struct InitUtxoWithdraw { From 026c5cf2a1a09cd7d6a50fff81198d3f2ecae5a6 Mon Sep 17 00:00:00 2001 From: Samuel Onoja Date: Fri, 31 May 2024 12:29:32 +0100 Subject: [PATCH 16/16] implement BTC mempool fee comp for low priority --- mm2src/coins/utxo/slp.rs | 2 +- mm2src/coins/utxo/utxo_withdraw.rs | 62 ++++++++++++++++++++---------- 2 files changed, 43 insertions(+), 21 deletions(-) diff --git a/mm2src/coins/utxo/slp.rs b/mm2src/coins/utxo/slp.rs index a0d68a19e1..fd5e61f7f9 100644 --- a/mm2src/coins/utxo/slp.rs +++ b/mm2src/coins/utxo/slp.rs @@ -1077,7 +1077,7 @@ impl UtxoTxBroadcastOps for SlpToken { #[async_trait] impl UtxoTxGenerationOps for SlpToken { - async fn get_tx_fee_per_kb(&self) -> UtxoRpcResult { todo!() } + async fn get_tx_fee_per_kb(&self) -> UtxoRpcResult { self.platform_coin.get_tx_fee_per_kb().await } async fn calc_interest_if_required( &self, diff --git a/mm2src/coins/utxo/utxo_withdraw.rs b/mm2src/coins/utxo/utxo_withdraw.rs index 6b845aae44..849178da8c 100644 --- a/mm2src/coins/utxo/utxo_withdraw.rs +++ b/mm2src/coins/utxo/utxo_withdraw.rs @@ -16,6 +16,7 @@ use crypto::{from_hw_error, CryptoCtx, CryptoCtxError, DerivationPath, HwError, use keys::{AddressFormat, KeyPair, Private, Public as PublicKey}; use mm2_core::mm_ctx::MmArc; use mm2_err_handle::prelude::*; +use mm2_net::transport::slurp_url; use rpc::v1::types::ToTxHash; use rpc_task::RpcTaskError; use script::{SignatureVersion, TransactionInputSigner}; @@ -28,17 +29,7 @@ use utxo_signer::{SignPolicy, UtxoSignerOps}; use super::UtxoFeePriorities; -/// Num of blocks applied to withdrawal transactions -/// that have a high priority, indicating a need for faster confirmation. -pub const HIGH_TX_FEE: u8 = 2; - -/// Num of blocks applied to withdrawal transactions -/// that have a normal priority, indicating a moderate confirmation time. -pub const NORMAL_TX_FEE: u8 = 6; - -/// Num of blocks applied to withdrawal transactions -/// that have a low priority, indicating a longer confirmation time. -pub const LOW_TX_FEE: u8 = 4; +const MEMPOOL_BTC_FEE_RATE: &str = "https://mempool.space/api/v1/fees/recommended"; impl From for WithdrawError { fn from(sign_err: UtxoSignTxError) -> Self { @@ -235,21 +226,24 @@ where } } +/// Checks if fee priorities are defined in the coin's configuration, +/// retrieves the fee estimate from mempool, and adjusts the fee for Bitcoin if necessary. async fn generate_withdraw_fee_using_priority( coin: &C, priority: &UtxoFeePriority, priorities: &Option, decimals: u8, ) -> Result> { - let n_blocks = match (priorities, priority) { - (Some(details), UtxoFeePriority::Low) => details.low, - (Some(details), UtxoFeePriority::Normal) => details.normal, - (Some(details), UtxoFeePriority::High) => details.high, - (None, UtxoFeePriority::Low) => LOW_TX_FEE, - (None, UtxoFeePriority::Normal) => NORMAL_TX_FEE, - (None, UtxoFeePriority::High) => HIGH_TX_FEE, + // Check if priorities are defined in coin config. + let Some(detail) = priorities else { + return MmError::err(WithdrawError::InternalError(format!("fee_priorities not found in {} config", coin.as_ref().conf.ticker))) + }; + let n_blocks = match priority { + UtxoFeePriority::Low => detail.low, + UtxoFeePriority::Normal => detail.normal, + UtxoFeePriority::High => detail.high, }; - let estimate_fee_sat = coin + let mut estimate_fee_sat = coin .as_ref() .rpc_client .estimate_fee_sat( @@ -261,10 +255,38 @@ async fn generate_withdraw_fee_using_priority .compat() .await?; - // let mem_pool_n_blocks = slurp_url(url) + if ["BTC"].contains(&coin.as_ref().conf.ticker.as_str()) { + if let UtxoFeePriority::Low = priority { + let mem_pool_fee = fetch_btc_mempool_low_fee_sat(decimals) + .await + .map_err(WithdrawError::InternalError)?; + info!("mem_pool_fee: {mem_pool_fee} - estimate_fee_sat: {estimate_fee_sat}"); + if mem_pool_fee < estimate_fee_sat { + estimate_fee_sat = mem_pool_fee; + } + } + } + Ok(estimate_fee_sat) } +#[derive(Debug, Deserialize)] +struct BTCMempoolFeeRate { + #[serde(rename = "economyFee")] + economy_fee: u8, +} + +/// Fetches the current low (economy) fee rate for Bitcoin transactions from the mempool. +async fn fetch_btc_mempool_low_fee_sat(decimals: u8) -> Result { + let (status, _, body) = try_s!(slurp_url(MEMPOOL_BTC_FEE_RATE).await); + if !status.is_success() { + return Err("Fetch BTC mempool fee rate was unsuccessful".to_owned()); + }; + let fee_rate: BTCMempoolFeeRate = try_s!(serde_json::from_slice(&body)); + let fee_rate = (fee_rate.economy_fee as f64 * 10.0_f64.powf(decimals as f64)) as u64; + Ok(fee_rate) +} + /// estimates the fee for a transaction using the provided RPC client, priority level /// and then updates the transaction builder with the calculated fee. pub struct InitUtxoWithdraw {