Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(UTXO): calc of txfee with change, min relay fee #2316

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from
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
8 changes: 4 additions & 4 deletions mm2src/coins/qrc20.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ 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,
use crate::utxo::{qtum, ActualFeeRate, AdditionalTxData, AddrFromStrError, BroadcastTxErr, FeePolicy, GenerateTxError,
GetUtxoListOps, HistoryUtxoTx, HistoryUtxoTxMap, MatureUnspentList, RecentlySpentOutPointsGuard,
UnsupportedAddr, UtxoActivationParams, UtxoAddressFormat, UtxoCoinFields, UtxoCommonOps,
UtxoFromLegacyReqErr, UtxoTx, UtxoTxBroadcastOps, UtxoTxGenerationOps, VerboseTransactionFrom,
Expand Down Expand Up @@ -493,8 +493,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 @@ -613,7 +613,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<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,
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 @@ -58,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: ActualFeeRate) {
let actual_tx_fee = block_on(coin.get_fee_rate()).unwrap();
assert_eq!(actual_tx_fee, expected_tx_fee);
}

Expand Down Expand Up @@ -718,7 +718,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 @@ -745,7 +745,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 @@ -781,7 +781,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 @@ -892,7 +892,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 @@ -917,7 +917,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
46 changes: 39 additions & 7 deletions mm2src/coins/utxo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,23 +267,56 @@ pub struct AdditionalTxData {

/// 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 @@ -578,7 +611,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 @@ -840,7 +873,7 @@ 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
Expand Down Expand Up @@ -1741,7 +1774,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
2 changes: 1 addition & 1 deletion mm2src/coins/utxo/bch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,7 @@ 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,
Expand Down
2 changes: 1 addition & 1 deletion mm2src/coins/utxo/qtum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ 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,
Expand Down
8 changes: 4 additions & 4 deletions mm2src/coins/utxo/slp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ 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,
use crate::utxo::{generate_and_send_tx, sat_from_big_decimal, ActualFeeRate, AdditionalTxData, BroadcastTxErr,
FeePolicy, GenerateTxError, RecentlySpentOutPointsGuard, UtxoCoinConf, UtxoCoinFields,
UtxoCommonOps, UtxoTx, UtxoTxBroadcastOps, UtxoTxGenerationOps};
use crate::{BalanceFut, CheckIfMyPaymentSentArgs, CoinBalance, CoinFutSpawner, ConfirmPaymentInput, DerivationMethod,
Expand Down Expand Up @@ -1078,7 +1078,7 @@ impl UtxoTxBroadcastOps for SlpToken {

#[async_trait]
impl UtxoTxGenerationOps for SlpToken {
async fn get_tx_fee(&self) -> UtxoRpcResult<ActualTxFee> { self.platform_coin.get_tx_fee().await }
async fn get_fee_rate(&self) -> UtxoRpcResult<ActualFeeRate> { self.platform_coin.get_fee_rate().await }

async fn calc_interest_if_required(
&self,
Expand Down Expand Up @@ -1674,11 +1674,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(ActualFeeRate::FixedPerKb(fixed))
},
Some(WithdrawFee::UtxoPerKbyte { amount }) => {
let dynamic = sat_from_big_decimal(&amount, platform_decimals)?;
tx_builder = tx_builder.with_fee(ActualTxFee::Dynamic(dynamic));
tx_builder = tx_builder.with_fee(ActualFeeRate::Dynamic(dynamic));
},
Some(fee_policy) => {
let error = format!(
Expand Down
12 changes: 6 additions & 6 deletions mm2src/coins/utxo/utxo_builder/utxo_coin_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ use crate::utxo::rpc_clients::{ElectrumClient, ElectrumClientSettings, ElectrumC
use crate::utxo::tx_cache::{UtxoVerboseCacheOps, UtxoVerboseCacheShared};
use crate::utxo::utxo_block_header_storage::BlockHeaderStorage;
use crate::utxo::utxo_builder::utxo_conf_builder::{UtxoConfBuilder, UtxoConfError};
use crate::utxo::{output_script, ElectrumBuilderArgs, RecentlySpentOutPoints, ScripthashNotification,
ScripthashNotificationSender, TxFee, UtxoCoinConf, UtxoCoinFields, UtxoHDWallet, UtxoRpcMode,
use crate::utxo::{output_script, ElectrumBuilderArgs, FeeRate, RecentlySpentOutPoints, ScripthashNotification,
ScripthashNotificationSender, UtxoCoinConf, UtxoCoinFields, UtxoHDWallet, UtxoRpcMode,
UtxoSyncStatus, UtxoSyncStatusLoopHandle, UTXO_DUST_AMOUNT};
use crate::{BlockchainNetwork, CoinTransportMetrics, DerivationMethod, HistorySyncState, IguanaPrivKey,
PrivKeyBuildPolicy, PrivKeyPolicy, PrivKeyPolicyNotAllowed, RpcClientType,
Expand Down Expand Up @@ -502,9 +502,9 @@ pub trait UtxoCoinBuilderCommonOps {
Ok(self.conf()["decimals"].as_u64().unwrap_or(8) as u8)
}

async fn tx_fee(&self, rpc_client: &UtxoRpcClientEnum) -> UtxoCoinBuildResult<TxFee> {
async fn tx_fee(&self, rpc_client: &UtxoRpcClientEnum) -> UtxoCoinBuildResult<FeeRate> {
let tx_fee = match self.conf()["txfee"].as_u64() {
None => TxFee::FixedPerKb(1000),
None => FeeRate::FixedPerKb(1000),
Some(0) => {
let fee_method = match &rpc_client {
UtxoRpcClientEnum::Electrum(_) => EstimateFeeMethod::Standard,
Expand All @@ -514,9 +514,9 @@ pub trait UtxoCoinBuilderCommonOps {
.await
.map_to_mm(UtxoCoinBuildError::ErrorDetectingFeeMethod)?,
};
TxFee::Dynamic(fee_method)
FeeRate::Dynamic(fee_method)
},
Some(fee) => TxFee::FixedPerKb(fee),
Some(fee) => FeeRate::FixedPerKb(fee),
};
Ok(tx_fee)
}
Expand Down
Loading
Loading