Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
cea1b2f
fix UtxoTxBuilder::build() tx fee calculation to include change output;
Jan 8, 2025
455403c
Merge branch 'dev' into fix-utxo-tx-fee-for-change
Jan 8, 2025
8d2fe79
fix qrc20 gas fee calc in trade preimage
Jan 9, 2025
bc69a42
revert qtum gas_fee calc in trade preimage,
Jan 9, 2025
40cfbda
fix: err if change < dust (was <=)
Jan 23, 2025
50a38f8
refactor (onur-ozkan): use refs instead of clone in add_tx_inputs
Jan 23, 2025
615ff3e
refactor (mariocynicys): fix return_err_if macro to check if cond is …
Jan 23, 2025
3f709fc
refactor: rename vars to use always 'fee rate' root word
Jan 23, 2025
459a379
fix (mariocynicys): recalc kmd interest when tx inputs reselected and…
Feb 24, 2025
534006e
Merge branch 'dev' into fix-utxo-tx-fee-for-change
Feb 24, 2025
ad0699e
fix after merge
Feb 24, 2025
1b2c538
refactor (mariocynicys) removed unnecessary checks
Feb 24, 2025
110a236
fix fee_amount in AdditionalTxData, get rid of unused_change, add reu…
Feb 25, 2025
b5994e6
add extra test to check tx builder calc with many inputs
Feb 25, 2025
ae8c3ee
remove unused_change in utxo tx builder (var not used any more)
Mar 4, 2025
9126c49
fix utxo tx builder test: add randomizer, catch overpay
Mar 6, 2025
997dcee
Merge remote-tracking branch 'origin/dev' into fix-utxo-tx-fee-for-ch…
shamardy Mar 24, 2025
7ca03f4
review (mariocynicys): improve calc kmd interest performance with get…
May 2, 2025
850f7b3
Merge remote-tracking branch 'origin/dev' into fix-utxo-tx-fee-for-ch…
shamardy May 6, 2025
eab8902
fix after merge: rename the 2 different `is_kmd` methods to a more de…
shamardy May 6, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions mm2src/coins/lightning/ln_platform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ impl FeeEstimator for Platform {
ConfirmationTarget::Normal => self.confirmations_targets.normal,
ConfirmationTarget::HighPriority => self.confirmations_targets.high_priority,
};
let fee_per_kb = tokio::task::block_in_place(move || {
let fee_rate = tokio::task::block_in_place(move || {
block_on_f01(self.rpc_client().estimate_fee_sat(
platform_coin.decimals(),
// Todo: when implementing Native client detect_fee_method should be used for Native and
Expand All @@ -582,16 +582,16 @@ impl FeeEstimator for Platform {

// Set default fee to last known fee for the corresponding confirmation target
match confirmation_target {
ConfirmationTarget::Background => self.latest_fees.set_background_fees(fee_per_kb),
ConfirmationTarget::Normal => self.latest_fees.set_normal_fees(fee_per_kb),
ConfirmationTarget::HighPriority => self.latest_fees.set_high_priority_fees(fee_per_kb),
ConfirmationTarget::Background => self.latest_fees.set_background_fees(fee_rate),
ConfirmationTarget::Normal => self.latest_fees.set_normal_fees(fee_rate),
ConfirmationTarget::HighPriority => self.latest_fees.set_high_priority_fees(fee_rate),
};

// Must be no smaller than 253 (ie 1 satoshi-per-byte rounded up to ensure later round-downs don’t put us below 1 satoshi-per-byte).
// https://docs.rs/lightning/0.0.101/lightning/chain/chaininterface/trait.FeeEstimator.html#tymethod.get_est_sat_per_1000_weight
// This has changed in rust-lightning v0.0.110 as LDK currently wraps get_est_sat_per_1000_weight to ensure that the value returned is
// no smaller than 253. https://github.com/lightningdevkit/rust-lightning/pull/1552
(fee_per_kb as f64 / 4.0).ceil() as u32
(fee_rate as f64 / 4.0).ceil() as u32
}
}

Expand Down
6 changes: 3 additions & 3 deletions mm2src/coins/lp_coins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2144,8 +2144,8 @@ pub trait MarketCoinOps {
/// Is privacy coin like zcash or pirate
fn is_privacy(&self) -> bool { false }

/// Is KMD coin
fn is_kmd(&self) -> bool { false }
/// Returns `true` for coins (like KMD) that should use direct DEX fee burning via OP_RETURN.
fn should_burn_directly(&self) -> bool { false }

/// Should burn part of dex fee coin
fn should_burn_dex_fee(&self) -> bool;
Expand Down Expand Up @@ -3834,7 +3834,7 @@ impl DexFee {
let dex_fee = trade_amount * &rate;
let min_tx_amount = MmNumber::from(taker_coin.min_tx_amount());

if taker_coin.is_kmd() {
if taker_coin.should_burn_directly() {
// use a special dex fee option for kmd
return Self::calc_dex_fee_for_op_return(dex_fee, min_tx_amount);
}
Expand Down
30 changes: 12 additions & 18 deletions mm2src/coins/qrc20.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,10 @@ use crate::utxo::utxo_builder::{UtxoCoinBuildError, UtxoCoinBuildResult, UtxoCoi
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::{qtum, ActualFeeRate, AddrFromStrError, BroadcastTxErr, FeePolicy, GenerateTxError, GetUtxoListOps,
HistoryUtxoTx, HistoryUtxoTxMap, MatureUnspentList, RecentlySpentOutPointsGuard, UnsupportedAddr,
UtxoActivationParams, UtxoAddressFormat, UtxoCoinFields, UtxoCommonOps, UtxoFromLegacyReqErr,
UtxoTx, UtxoTxBroadcastOps, UtxoTxGenerationOps, VerboseTransactionFrom, UTXO_LOCK};
use crate::{BalanceError, BalanceFut, CheckIfMyPaymentSentArgs, CoinBalance, ConfirmPaymentInput, DexFee, Eip1559Ops,
FeeApproxStage, FoundSwapTxSpend, HistorySyncState, IguanaPrivKey, MarketCoinOps, MmCoin,
NegotiateSwapContractAddrErr, PrivKeyBuildPolicy, PrivKeyPolicyNotAllowed, RawTransactionFut,
Expand Down Expand Up @@ -489,8 +488,8 @@ 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<u64, String> {
match try_s!(self.get_tx_fee().await) {
ActualTxFee::Dynamic(amount) | ActualTxFee::FixedPerKb(amount) => Ok(amount + gas_fee),
match try_s!(self.get_fee_rate().await) {
ActualFeeRate::Dynamic(amount) | ActualFeeRate::FixedPerKb(amount) => Ok(amount + gas_fee),
}
}

Expand Down Expand Up @@ -545,10 +544,9 @@ impl Qrc20Coin {
self.utxo.conf.fork_id,
)?;

let miner_fee = data.fee_amount + data.unused_change;
Ok(GenerateQrc20TxResult {
signed,
miner_fee,
miner_fee: data.fee_amount,
gas_fee,
})
}
Expand Down Expand Up @@ -609,17 +607,13 @@ impl UtxoTxBroadcastOps for Qrc20Coin {
#[cfg_attr(test, mockable)]
impl UtxoTxGenerationOps for Qrc20Coin {
/// Get only QTUM transaction fee.
async fn get_tx_fee(&self) -> UtxoRpcResult<ActualTxFee> { utxo_common::get_tx_fee(&self.utxo).await }
async fn get_fee_rate(&self) -> UtxoRpcResult<ActualFeeRate> { utxo_common::get_fee_rate(&self.utxo).await }

async fn calc_interest_if_required(
&self,
unsigned: TransactionInputSigner,
data: AdditionalTxData,
my_script_pub: ScriptBytes,
dust: u64,
) -> UtxoRpcResult<(TransactionInputSigner, AdditionalTxData)> {
utxo_common::calc_interest_if_required(self, unsigned, data, my_script_pub, dust).await
async fn calc_interest_if_required(&self, unsigned: &mut TransactionInputSigner) -> UtxoRpcResult<u64> {
utxo_common::calc_interest_if_required(self, unsigned).await
}

fn supports_interest(&self) -> bool { utxo_common::is_kmd(self) }
}

#[async_trait]
Expand Down
14 changes: 7 additions & 7 deletions mm2src/coins/qrc20/qrc20_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,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: ActualFeeRate) {
let actual_tx_fee = block_on(coin.get_fee_rate()).unwrap();
assert_eq!(actual_tx_fee, expected_tx_fee);
}

Expand Down Expand Up @@ -712,7 +712,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, ActualFeeRate::FixedPerKb(EXPECTED_TX_FEE as u64));

let actual_trade_fee = block_on_f01(coin.get_trade_fee()).unwrap();
let expected_trade_fee_amount = big_decimal_from_sat(
Expand All @@ -739,7 +739,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, ActualFeeRate::FixedPerKb(EXPECTED_TX_FEE as u64));

let allowance = block_on(coin.allowance(coin.swap_contract_address)).expect("!allowance");
assert_eq!(allowance, 0.into());
Expand Down Expand Up @@ -775,7 +775,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, ActualFeeRate::FixedPerKb(EXPECTED_TX_FEE as u64));

let allowance = block_on(coin.allowance(coin.swap_contract_address)).expect("!allowance");
assert_eq!(allowance, 300_000_000.into());
Expand Down Expand Up @@ -886,7 +886,7 @@ 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, ActualFeeRate::FixedPerKb(EXPECTED_TX_FEE as u64));

let actual =
block_on_f01(coin.get_receiver_trade_fee(FeeApproxStage::WithoutApprox)).expect("!get_receiver_trade_fee");
Expand All @@ -911,7 +911,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, ActualFeeRate::FixedPerKb(EXPECTED_TX_FEE as u64));
let expected_balance = CoinBalance {
spendable: BigDecimal::from(5u32),
unspendable: BigDecimal::from(0u32),
Expand Down
2 changes: 1 addition & 1 deletion mm2src/coins/rpc_command/lightning/open_channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ pub async fn open_channel(ctx: MmArc, req: OpenChannelRequest) -> OpenChannelRes
.with_fee_policy(fee_policy);

let fee = platform_coin
.get_tx_fee()
.get_fee_rate()
.await
.map_err(|e| OpenChannelError::RpcError(e.to_string()))?;
tx_builder = tx_builder.with_fee(fee);
Expand Down
2 changes: 1 addition & 1 deletion mm2src/coins/test_coin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ impl MarketCoinOps for TestCoin {

fn min_trading_vol(&self) -> MmNumber { MmNumber::from("0.00777") }

fn is_kmd(&self) -> bool { &self.ticker == "KMD" }
fn should_burn_directly(&self) -> bool { &self.ticker == "KMD" }

fn should_burn_dex_fee(&self) -> bool { false }

Expand Down
58 changes: 43 additions & 15 deletions mm2src/coins/utxo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,29 +260,61 @@ pub struct AdditionalTxData {
pub received_by_me: u64,
pub spent_by_me: u64,
pub fee_amount: u64,
pub unused_change: u64,
pub kmd_rewards: Option<KmdRewardsDetails>,
}

/// The fee set from coins config
#[derive(Debug)]
pub enum TxFee {
pub enum FeeRate {
/// Tell the coin that it should request the fee from daemon RPC and calculate it relying on tx size
Dynamic(EstimateFeeMethod),
/// Tell the coin that it has fixed tx fee per kb.
FixedPerKb(u64),
}

/// The actual "runtime" fee that is received from RPC in case of dynamic calculation
/// The actual "runtime" tx fee rate (per kb) that is received from RPC in case of dynamic calculation
/// or fixed tx fee rate
#[derive(Copy, Clone, Debug, PartialEq)]
pub enum ActualTxFee {
pub enum ActualFeeRate {
/// 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.
/// Use specified fee amount per each 1 kb of transaction and also per each output less than the fee amount.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you explain more about this fee scheme. the per each output part isn't really clear (understood it as 1 fee_rate per 1 output, which i doubt is a correct understanding).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, actually I have the same question:
this fee calc is not fully clear to me. The comment refers to DOGE, I took a look at its daemon code but did not find where this logic is.

/// Used by DOGE, but more coins might support it too.
FixedPerKb(u64),
}

impl ActualFeeRate {
fn get_tx_fee(&self, tx_size: u64) -> u64 {
match self {
ActualFeeRate::Dynamic(fee_rate) => (fee_rate * tx_size) / KILO_BYTE,
// return fee_rate here as swap spend transaction size is always less than 1 kb
ActualFeeRate::FixedPerKb(fee_rate) => {
let tx_size_kb = if tx_size % KILO_BYTE == 0 {
tx_size / KILO_BYTE
} else {
tx_size / KILO_BYTE + 1
};
fee_rate * tx_size_kb
},
}
}

/// Return extra tx fee for the change output as p2pkh
fn get_tx_fee_for_change(&self, tx_size: u64) -> u64 {
match self {
ActualFeeRate::Dynamic(fee_rate) => (*fee_rate * P2PKH_OUTPUT_LEN) / KILO_BYTE,
ActualFeeRate::FixedPerKb(fee_rate) => {
// take into account the change output if tx_size_kb(tx with change) > tx_size_kb(tx without change)
if tx_size % KILO_BYTE + P2PKH_OUTPUT_LEN > KILO_BYTE {
*fee_rate
} else {
0
}
},
}
}
}

/// Fee policy applied on transaction creation
pub enum FeePolicy {
/// Send the exact amount specified in output(s), fee is added to spent input amount
Expand Down Expand Up @@ -577,7 +609,7 @@ pub struct UtxoCoinFields {
/// Emercoin has 6
/// Bitcoin Diamond has 7
pub decimals: u8,
pub tx_fee: TxFee,
pub tx_fee: FeeRate,
/// Minimum transaction value at which the value is not less than fee
pub dust_amount: u64,
/// RPC client
Expand Down Expand Up @@ -839,18 +871,15 @@ pub trait UtxoTxBroadcastOps {
#[async_trait]
#[cfg_attr(test, mockable)]
pub trait UtxoTxGenerationOps {
async fn get_tx_fee(&self) -> UtxoRpcResult<ActualTxFee>;
async fn get_fee_rate(&self) -> UtxoRpcResult<ActualFeeRate>;

/// Calculates interest if the coin is KMD
/// Adds the value to existing output to my_script_pub or creates additional interest output
/// returns transaction and data as is if the coin is not KMD
async fn calc_interest_if_required(
&self,
mut unsigned: TransactionInputSigner,
mut data: AdditionalTxData,
my_script_pub: Bytes,
dust: u64,
) -> UtxoRpcResult<(TransactionInputSigner, AdditionalTxData)>;
async fn calc_interest_if_required(&self, unsigned: &mut TransactionInputSigner) -> UtxoRpcResult<u64>;

/// Returns `true` if this coin supports Komodo-style interest accrual; otherwise, returns `false`.
fn supports_interest(&self) -> bool;
}

/// The UTXO address balance scanner.
Expand Down Expand Up @@ -1747,7 +1776,6 @@ where
{
let my_address = try_tx_s!(coin.as_ref().derivation_method.single_addr_or_err().await);
let key_pair = try_tx_s!(coin.as_ref().priv_key_policy.activated_key_or_err());

let mut builder = UtxoTxBuilder::new(coin)
.await
.add_available_inputs(unspents)
Expand Down
14 changes: 5 additions & 9 deletions mm2src/coins/utxo/bch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -700,17 +700,13 @@ impl UtxoTxBroadcastOps for BchCoin {
#[async_trait]
#[cfg_attr(test, mockable)]
impl UtxoTxGenerationOps for BchCoin {
async fn get_tx_fee(&self) -> UtxoRpcResult<ActualTxFee> { utxo_common::get_tx_fee(&self.utxo_arc).await }
async fn get_fee_rate(&self) -> UtxoRpcResult<ActualFeeRate> { utxo_common::get_fee_rate(&self.utxo_arc).await }

async fn calc_interest_if_required(
&self,
unsigned: TransactionInputSigner,
data: AdditionalTxData,
my_script_pub: Bytes,
dust: u64,
) -> UtxoRpcResult<(TransactionInputSigner, AdditionalTxData)> {
utxo_common::calc_interest_if_required(self, unsigned, data, my_script_pub, dust).await
async fn calc_interest_if_required(&self, unsigned: &mut TransactionInputSigner) -> UtxoRpcResult<u64> {
utxo_common::calc_interest_if_required(self, unsigned).await
}

fn supports_interest(&self) -> bool { utxo_common::is_kmd(self) }
}

#[async_trait]
Expand Down
14 changes: 5 additions & 9 deletions mm2src/coins/utxo/qtum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,17 +313,13 @@ impl UtxoTxBroadcastOps for QtumCoin {
#[async_trait]
#[cfg_attr(test, mockable)]
impl UtxoTxGenerationOps for QtumCoin {
async fn get_tx_fee(&self) -> UtxoRpcResult<ActualTxFee> { utxo_common::get_tx_fee(&self.utxo_arc).await }
async fn get_fee_rate(&self) -> UtxoRpcResult<ActualFeeRate> { utxo_common::get_fee_rate(&self.utxo_arc).await }

async fn calc_interest_if_required(
&self,
unsigned: TransactionInputSigner,
data: AdditionalTxData,
my_script_pub: Bytes,
dust: u64,
) -> UtxoRpcResult<(TransactionInputSigner, AdditionalTxData)> {
utxo_common::calc_interest_if_required(self, unsigned, data, my_script_pub, dust).await
async fn calc_interest_if_required(&self, unsigned: &mut TransactionInputSigner) -> UtxoRpcResult<u64> {
utxo_common::calc_interest_if_required(self, unsigned).await
}

fn supports_interest(&self) -> bool { utxo_common::is_kmd(self) }
}

#[async_trait]
Expand Down
3 changes: 1 addition & 2 deletions mm2src/coins/utxo/qtum_delegation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,10 +294,9 @@ impl QtumCoin {

let signed = sign_tx(unsigned, key_pair, utxo.conf.signature_version, utxo.conf.fork_id)?;

let miner_fee = data.fee_amount + data.unused_change;
let generated_tx = GenerateQrc20TxResult {
signed,
miner_fee,
miner_fee: data.fee_amount,
gas_fee,
};

Expand Down
Loading
Loading