Skip to content
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
9059f93
impl zero dex fee for KMD
borngraced Jan 14, 2025
450ced6
fix swap unit test
borngraced Jan 14, 2025
0cc613a
fix docker test and dissallow sending dex fee
borngraced Jan 14, 2025
49f08ac
fix gen_taker_payment_spend_preimage
borngraced Jan 15, 2025
1a8539a
check for zero dex fee in other protocol
borngraced Jan 15, 2025
dc45a35
eth and tendermint
borngraced Jan 15, 2025
f62a0de
check for KMD ticker in swap_v2
borngraced Jan 16, 2025
2d6ae32
save dev state
borngraced Jan 20, 2025
a6cb780
update taker fee validated log info
borngraced Jan 20, 2025
06ccb20
fix review notes, add MmNumber from ref BigDecimal
borngraced Feb 18, 2025
76cd46e
delegate Zero dex fee check to taker_swap
borngraced Feb 18, 2025
4358025
fix nits
borngraced Feb 19, 2025
c05963e
Merge branch 'dev' into 0-kmd-dex-fee
borngraced Feb 19, 2025
634c5f5
Merge branch 'dev' into 0-kmd-dex-fee
borngraced Feb 23, 2025
29a8460
remove impl for legacy swap
borngraced Feb 24, 2025
a50238f
revert taker_swap
borngraced Mar 3, 2025
d2e766e
nits
borngraced Mar 3, 2025
da5fc4e
revert dex_fee_amount
borngraced Mar 4, 2025
8ba9aab
fix review notes
borngraced Mar 4, 2025
dd33004
update test constant and taker_send_approve_and_spennd_erc20 test
borngraced Mar 6, 2025
84e5b7f
Merge branch 'dev' into 0-kmd-dex-fee
borngraced Mar 24, 2025
0d3be66
Merge branch 'refs/heads/dev' into 0-kmd-dex-fee
borngraced Mar 25, 2025
c714e4e
post merge fix
borngraced Mar 25, 2025
1afe061
NoFee for swap with KMD coins
borngraced Mar 25, 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
4 changes: 1 addition & 3 deletions mm2src/coins/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5806,10 +5806,10 @@ impl MmCoin for EthCoin {
stage: FeeApproxStage,
) -> TradePreimageResult<TradeFee> {
let dex_fee_amount = wei_from_big_decimal(&dex_fee_amount.fee_amount().into(), self.decimals)?;

// pass the dummy params
let to_addr = addr_from_raw_pubkey(&DEX_FEE_ADDR_RAW_PUBKEY)
.expect("addr_from_raw_pubkey should never fail with DEX_FEE_ADDR_RAW_PUBKEY");
let my_address = self.derivation_method.single_addr_or_err().await?;
let (eth_value, data, call_addr, fee_coin) = match &self.coin_type {
EthCoinType::Eth => (dex_fee_amount, Vec::new(), &to_addr, &self.ticker),
EthCoinType::Erc20 { platform, token_addr } => {
Expand All @@ -5819,8 +5819,6 @@ impl MmCoin for EthCoin {
},
EthCoinType::Nft { .. } => return MmError::err(TradePreimageError::NftProtocolNotSupported),
};

let my_address = self.derivation_method.single_addr_or_err().await?;
let fee_policy_for_estimate = get_swap_fee_policy_for_estimate(self.get_swap_transaction_fee_policy());
let pay_for_gas_option = self.get_swap_pay_for_gas_option(fee_policy_for_estimate).await?;
let pay_for_gas_option = increase_gas_price_by_stage(pay_for_gas_option, &stage);
Expand Down
2 changes: 1 addition & 1 deletion mm2src/coins/eth/eth_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ fn test_get_fee_to_send_taker_fee() {

let (_ctx, coin) = eth_coin_for_test(EthCoinType::Eth, &["http://dummy.dummy"], None, ETH_SEPOLIA_CHAIN_ID);
let actual = block_on(coin.get_fee_to_send_taker_fee(
DexFee::Standard(MmNumber::from(dex_fee_amount.clone())),
DexFee::Standard(MmNumber::from(&dex_fee_amount)),
FeeApproxStage::WithoutApprox,
))
.expect("!get_fee_to_send_taker_fee");
Expand Down
9 changes: 8 additions & 1 deletion mm2src/coins/lp_coins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3687,6 +3687,8 @@ pub enum DexFee {
fee_amount: MmNumber,
burn_amount: MmNumber,
},
/// Zero dex fee, exclusive to KMD only.
Zero,
}

impl DexFee {
Expand All @@ -3703,13 +3705,14 @@ impl DexFee {
match self {
DexFee::Standard(t) => t.clone(),
DexFee::WithBurn { fee_amount, .. } => fee_amount.clone(),
DexFee::Zero => MmNumber::default(),
}
}

/// Gets the burn amount associated with the dex fee, if applicable.
pub fn burn_amount(&self) -> Option<MmNumber> {
match self {
DexFee::Standard(_) => None,
DexFee::Standard(_) | DexFee::Zero => None,
DexFee::WithBurn { burn_amount, .. } => Some(burn_amount.clone()),
}
}
Expand All @@ -3722,6 +3725,7 @@ impl DexFee {
fee_amount,
burn_amount,
} => fee_amount + burn_amount,
DexFee::Zero => MmNumber::default(),
}
}

Expand All @@ -3739,6 +3743,9 @@ impl DexFee {
Ok(None)
}
}

/// Whether this is DexFee::Zero.
pub fn zero_fee(&self) -> bool { matches!(self, Self::Zero) }
Comment thread
laruh marked this conversation as resolved.
Outdated
}

pub struct CoinsContext {
Expand Down
8 changes: 8 additions & 0 deletions mm2src/coins/qrc20.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1287,6 +1287,14 @@ impl MmCoin for Qrc20Coin {
dex_fee_amount: DexFee,
stage: FeeApproxStage,
) -> TradePreimageResult<TradeFee> {
if dex_fee_amount.zero_fee() {
return Ok(TradeFee {
coin: self.platform.clone(),
amount: MmNumber::default(),
paid_from_trading_vol: false,
});
}

Comment thread
laruh marked this conversation as resolved.
Outdated
let amount = wei_from_big_decimal(&dex_fee_amount.fee_amount().into(), self.utxo.decimals)?;

// pass the dummy params
Expand Down
8 changes: 8 additions & 0 deletions mm2src/coins/tendermint/tendermint_coin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2828,6 +2828,14 @@ impl MmCoin for TendermintCoin {
dex_fee_amount: DexFee,
_stage: FeeApproxStage,
) -> TradePreimageResult<TradeFee> {
if dex_fee_amount.zero_fee() {
return Ok(TradeFee {
coin: self.ticker.clone(),
amount: MmNumber::default(),
paid_from_trading_vol: false,
});
}
Comment thread
laruh marked this conversation as resolved.
Outdated

self.get_fee_to_send_taker_fee_for_denom(self.ticker.clone(), self.denom.clone(), self.decimals, dex_fee_amount)
.await
}
Expand Down
1 change: 1 addition & 0 deletions mm2src/coins/utxo/slp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1734,6 +1734,7 @@ impl MmCoin for SlpToken {
&stage,
)
.await?;

Ok(TradeFee {
coin: self.platform_coin.ticker().into(),
amount: fee.into(),
Expand Down
86 changes: 48 additions & 38 deletions mm2src/coins/utxo/utxo_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1244,39 +1244,50 @@ async fn gen_taker_payment_spend_preimage<T: UtxoCommonOps>(
args: &GenTakerPaymentSpendArgs<'_, T>,
n_time: NTimeSetting,
) -> GenPreimageResInner {
let dex_fee_address = address_from_raw_pubkey(
args.dex_fee_pub,
coin.as_ref().conf.address_prefixes.clone(),
coin.as_ref().conf.checksum_type,
coin.as_ref().conf.bech32_hrp.clone(),
coin.addr_format().clone(),
)
.map_to_mm(|e| TxGenError::AddressDerivation(format!("Failed to derive dex_fee_address: {}", e)))?;

let mut outputs = generate_taker_fee_tx_outputs(coin.as_ref().decimals, dex_fee_address.hash(), args.dex_fee)?;
if let DexFee::WithBurn { .. } = args.dex_fee {
let script = output_script(args.maker_address).map_to_mm(|e| {
TxGenError::Other(format!(
"Couldn't generate output script for maker address {}, error {}",
args.maker_address, e
))
})?;
let tx_fee = coin
.get_htlc_spend_fee(DEFAULT_SWAP_TX_SPEND_SIZE, &FeeApproxStage::WithoutApprox)
.await?;
let maker_value = args
.taker_tx
.first_output()
.map_to_mm(|e| TxGenError::PrevTxIsNotValid(e.to_string()))?
.value
- outputs[0].value
- outputs[1].value
- tx_fee;
outputs.push(TransactionOutput {
value: maker_value,
script_pubkey: script.to_bytes(),
})
}
let outputs = match args.dex_fee {
DexFee::Zero => vec![],
dex_fee => {
let dex_fee_address = address_from_raw_pubkey(
args.dex_fee_pub,
coin.as_ref().conf.address_prefixes.clone(),
coin.as_ref().conf.checksum_type,
coin.as_ref().conf.bech32_hrp.clone(),
coin.addr_format().clone(),
)
.map_to_mm(|e| TxGenError::AddressDerivation(format!("Failed to derive dex_fee_address: {}", e)))?;

let mut fee_outputs =
generate_taker_fee_tx_outputs(coin.as_ref().decimals, dex_fee_address.hash(), dex_fee)?;

if let DexFee::WithBurn { .. } = dex_fee {
let script = output_script(args.maker_address).map_to_mm(|e| {
TxGenError::Other(format!(
"Couldn't generate output script for maker address {}, error {}",
args.maker_address, e
))
})?;

let tx_fee = coin
.get_htlc_spend_fee(DEFAULT_SWAP_TX_SPEND_SIZE, &FeeApproxStage::WithoutApprox)
.await?;

let total_fee_value: u64 = fee_outputs.iter().map(|out| out.value).sum();
let first_output_value = args
.taker_tx
.first_output()
.map_to_mm(|e| TxGenError::PrevTxIsNotValid(e.to_string()))?
.value;

let maker_value = first_output_value - total_fee_value - tx_fee;
fee_outputs.push(TransactionOutput {
value: maker_value,
script_pubkey: script.to_bytes(),
});
}

fee_outputs
},
};

p2sh_spending_tx_preimage(
coin,
Expand Down Expand Up @@ -1307,7 +1318,7 @@ pub async fn gen_and_sign_taker_payment_spend_preimage<T: UtxoCommonOps>(

let sig_hash_type = match args.dex_fee {
DexFee::Standard(_) => SIGHASH_SINGLE,
DexFee::WithBurn { .. } => SIGHASH_ALL,
DexFee::WithBurn { .. } | DexFee::Zero => SIGHASH_ALL,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could you clarify why we should use SIGHASH_ALL for zero fee?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

SIGHASH_ALL: Signs all inputs and outputs, preventing any changes after signing
SIGHASH_SINGLE: Only signs the input and the corresponding output at the same index, allowing other outputs to be modified.

I guess using either for zero fees won't make a significant diff. cc @dimxy

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ok I was just curious will it be the difference for zero fee, as for Standard dex fee we use SIGHASH_SINGLE

@dimxy dimxy Mar 6, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes, we should use SIGHASH_ALL wherever possible.
SIGHASH_SINGLE could be used only in a special case (DexFee::Standard) when maker can add an output to the spending taker payment transaction (with only a single dexfee output signed by the taker with the SIGHASH_SINGLE flag), without the other party signature.
(this is how the SIGHASH_SINGLE feature works)

};

let signature = calc_and_sign_sighash(
Expand Down Expand Up @@ -1350,7 +1361,7 @@ pub async fn validate_taker_payment_spend_preimage<T: UtxoCommonOps + SwapOps>(

let sig_hash_type = match gen_args.dex_fee {
DexFee::Standard(_) => SIGHASH_SINGLE,
DexFee::WithBurn { .. } => SIGHASH_ALL,
DexFee::WithBurn { .. } | DexFee::Zero => SIGHASH_ALL,
};

let sig_hash = signature_hash_to_sign(
Expand Down Expand Up @@ -1434,7 +1445,7 @@ pub async fn sign_and_broadcast_taker_payment_spend<T: UtxoCommonOps>(
let mut taker_signature_with_sighash = preimage.signature.to_vec();
let taker_sig_hash = match gen_args.dex_fee {
DexFee::Standard(_) => (SIGHASH_SINGLE | coin.as_ref().conf.fork_id) as u8,
DexFee::WithBurn { .. } => (SIGHASH_ALL | coin.as_ref().conf.fork_id) as u8,
DexFee::WithBurn { .. } | DexFee::Zero => (SIGHASH_ALL | coin.as_ref().conf.fork_id) as u8,
};

taker_signature_with_sighash.push(taker_sig_hash);
Expand Down Expand Up @@ -3999,13 +4010,12 @@ where
T: MarketCoinOps + UtxoCommonOps,
{
let decimals = coin.as_ref().decimals;

let outputs = generate_taker_fee_tx_outputs(decimals, &AddressHashEnum::default_address_hash(), &dex_fee)?;

let gas_fee = None;
let fee_amount = coin
.preimage_trade_fee_required_to_send_outputs(outputs, FeePolicy::SendExact, gas_fee, &stage)
.await?;

Ok(TradeFee {
coin: coin.ticker().to_owned(),
amount: fee_amount.into(),
Expand Down
75 changes: 19 additions & 56 deletions mm2src/mm2_main/src/lp_swap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -802,36 +802,10 @@ fn dex_fee_rate(base: &str, rel: &str) -> MmNumber {
pub fn dex_fee_amount(base: &str, rel: &str, trade_amount: &MmNumber, min_tx_amount: &MmNumber) -> DexFee {
let rate = dex_fee_rate(base, rel);
let fee = trade_amount * &rate;

if &fee <= min_tx_amount {
return DexFee::Standard(min_tx_amount.clone());
}

if base == "KMD" {
// Drop the fee by 25%, which will be burned during the taker fee payment.
//
// This cut will be dropped before return if the final amount is less than
// the minimum transaction amount.

// Fee with 25% cut
let new_fee = &fee * &MmNumber::from("0.75");

let (fee, burn) = if &new_fee >= min_tx_amount {
// Use the max burn value, which is 25%.
let burn_amount = &fee - &new_fee;

(new_fee, burn_amount)
} else {
// Burn only the exceed amount because fee after 25% cut is less
// than `min_tx_amount`.
let burn_amount = &fee - min_tx_amount;

(min_tx_amount.clone(), burn_amount)
};

return DexFee::with_burn(fee, burn);
}

Comment thread
laruh marked this conversation as resolved.
Outdated
DexFee::Standard(fee)
}

Expand Down Expand Up @@ -996,7 +970,7 @@ impl SwapTxDataMsg {
}
}

#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)]
pub struct TransactionIdentifier {
/// Raw bytes of signed transaction in hexadecimal string, this should be sent as is to send_raw_transaction RPC to broadcast the transaction.
/// Some payments like lightning payments don't have a tx_hex, for such payments tx_hex will be equal to tx_hash.
Expand Down Expand Up @@ -1841,36 +1815,27 @@ mod lp_swap_tests {
let rel = "ETH";
let amount = 1.into();
let actual_fee = dex_fee_amount(base, rel, &amount, &min_tx_amount);
let expected_fee = amount.clone() * (9, 7770).into() * MmNumber::from("0.75");
let expected_burn_amount = amount * (9, 7770).into() * MmNumber::from("0.25");
assert_eq!(DexFee::with_burn(expected_fee, expected_burn_amount), actual_fee);
assert_eq!(DexFee::Zero, actual_fee);

// check the case when KMD taker fee is close to dust
let base = "KMD";
let rel = "BTC";
let amount = (1001 * 777, 90000000).into();
let min_tx_amount = "0.00001".into();
let actual_fee = dex_fee_amount(base, rel, &amount, &min_tx_amount);
assert_eq!(
DexFee::WithBurn {
fee_amount: "0.00001".into(),
burn_amount: "0.00000001".into()
},
actual_fee
);
assert_eq!(DexFee::Zero, actual_fee);

let base = "BTC";
let rel = "KMD";
let amount = 1.into();
let actual_fee = dex_fee_amount(base, rel, &amount, &min_tx_amount);
let expected_fee = DexFee::Standard(amount * (9, 7770).into());
assert_eq!(expected_fee, actual_fee);
assert_eq!(DexFee::Zero, actual_fee);

let base = "BTC";
let rel = "KMD";
let amount: MmNumber = "0.001".parse::<BigDecimal>().unwrap().into();
let actual_fee = dex_fee_amount(base, rel, &amount, &min_tx_amount);
assert_eq!(DexFee::Standard(min_tx_amount), actual_fee);
assert_eq!(DexFee::Zero, actual_fee);
}

#[test]
Expand Down Expand Up @@ -2390,45 +2355,43 @@ mod lp_swap_tests {
std::env::set_var("MYCOIN_FEE_DISCOUNT", "");

let kmd = coins::TestCoin::new("KMD");
let (kmd_taker_fee, kmd_burn_amount) = match dex_fee_amount_from_taker_coin(&kmd, "", &MmNumber::from(6150)) {
DexFee::Standard(_) => panic!("Wrong variant returned for KMD from `dex_fee_amount_from_taker_coin`."),
DexFee::WithBurn {
fee_amount,
burn_amount,
} => (fee_amount, burn_amount),
let kmd_burn_amount = match dex_fee_amount_from_taker_coin(&kmd, "MYCOIN", &MmNumber::from(6150)) {
DexFee::Standard(_) | DexFee::WithBurn { .. } => {
panic!("Wrong variant returned for KMD from `dex_fee_amount_from_taker_coin`.")
},
DexFee::Zero => MmNumber::default(),
};

let mycoin = coins::TestCoin::new("MYCOIN");
let mycoin_taker_fee = match dex_fee_amount_from_taker_coin(&mycoin, "", &MmNumber::from(6150)) {
DexFee::Standard(t) => t,
DexFee::WithBurn { .. } => {
let mycoin_taker_fee = match dex_fee_amount_from_taker_coin(&mycoin, "KMD", &MmNumber::from(6150)) {
DexFee::Zero => MmNumber::default(),
DexFee::WithBurn { .. } | DexFee::Standard(_) => {
panic!("Wrong variant returned for MYCOIN from `dex_fee_amount_from_taker_coin`.")
},
};

let expected_mycoin_taker_fee = &kmd_taker_fee / &MmNumber::from("0.75");
let expected_kmd_burn_amount = &mycoin_taker_fee - &kmd_taker_fee;
let zero_amount = MmNumber::default();

assert_eq!(expected_mycoin_taker_fee, mycoin_taker_fee);
assert_eq!(expected_kmd_burn_amount, kmd_burn_amount);
assert_eq!(zero_amount, mycoin_taker_fee);
assert_eq!(zero_amount, kmd_burn_amount);
}

#[test]
fn test_dex_fee_amount_from_taker_coin_discount() {
std::env::set_var("MYCOIN_FEE_DISCOUNT", "");

let mycoin = coins::TestCoin::new("MYCOIN");
let mycoin_taker_fee = match dex_fee_amount_from_taker_coin(&mycoin, "", &MmNumber::from(6150)) {
let mycoin_taker_fee = match dex_fee_amount_from_taker_coin(&mycoin, "MYCOIN", &MmNumber::from(6150)) {
DexFee::Standard(t) => t,
DexFee::WithBurn { .. } => {
DexFee::WithBurn { .. } | DexFee::Zero => {
panic!("Wrong variant returned for MYCOIN from `dex_fee_amount_from_taker_coin`.")
},
};

let testcoin = coins::TestCoin::default();
let testcoin_taker_fee = match dex_fee_amount_from_taker_coin(&testcoin, "", &MmNumber::from(6150)) {
DexFee::Standard(t) => t,
DexFee::WithBurn { .. } => {
DexFee::WithBurn { .. } | DexFee::Zero => {
panic!("Wrong variant returned for TEST coin from `dex_fee_amount_from_taker_coin`.")
},
};
Expand Down
Loading