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

Implement Zero DEX Fees for KMD Trading Pairs #2320

Open
shamardy opened this issue Jan 13, 2025 · 3 comments
Open

Implement Zero DEX Fees for KMD Trading Pairs #2320

shamardy opened this issue Jan 13, 2025 · 3 comments
Assignees
Labels
breaking-change: swap feature: swap priority: urgent Critical tasks requiring immediate action.

Comments

@shamardy
Copy link
Collaborator

Overview

With the trading protocol upgrade, DEX fees no longer serve as an anti-DDoS measure, allowing us to implement zero fees for specific trading pairs when advantageous. To increase KMD utility in the DEX, we propose eliminating DEX fees entirely for any trading pair that includes KMD (whether as taker or maker coin). This change could incentivize users to hold and trade with KMD.

Technical Implementation

Required Changes

  1. Add a new variant for zero DEX fee in the codebase
    /// Represents the different types of DEX fees.
    #[derive(Clone, Debug, PartialEq)]
    pub enum DexFee {
    /// Standard dex fee which will be sent to the dex fee address
    Standard(MmNumber),
    /// Dex fee with the burn amount.
    /// - `fee_amount` goes to the dex fee address.
    /// - `burn_amount` will be added as `OP_RETURN` output in the dex fee transaction.
    WithBurn {
    fee_amount: MmNumber,
    burn_amount: MmNumber,
    },
    }
  2. Modify funding transaction logic to exclude DEX fee amount for KMD pairs
  3. Update taker payment spend to remove DEX fee output for KMD pairs
  4. Implement appropriate validation on the maker side
  5. Might require some changes in the etomic swap contract for KMD(Maker)/EVM(Taker) Pairs c.c. @laruh

Dependencies

@shamardy shamardy added priority: urgent Critical tasks requiring immediate action. breaking-change: swap feature: swap labels Jan 13, 2025
@laruh
Copy link
Member

laruh commented Jan 13, 2025

Might require some changes in the etomic swap contract for KMD(Maker)/EVM(Taker) Pairs

yep, currently we have one place (erc20TakerPayment) which checks that dexFee > 0
https://github.com/KomodoPlatform/etomic-swap/blob/5e15641cbf41766cd5b37b4d71842c270773f788/contracts/EtomicSwapTakerV2.sol#L99

need to do smth with this
well we can have additional erc20 payment function without dexfee param and set dexFee == 0 in the paymentHash calculation, this wont take additional computations for taker. heh but new method will lead to kdf eth_taker_swap_v2.rs code update :) I will think about preferable approach

UPD eth swap v2 code update is not a problem its fine. The goal is to ensure the gas usage for the taker does not increase while maintaining the smart contract functions security.

@laruh
Copy link
Member

laruh commented Jan 15, 2025

@shamardy what do you think about removing require(dexFee > 0, "Dex fee must not be zero"); check in erc20TakerPayment function and just allowing dexFee to be zero directly?

This seems acceptable to me since we handle dexFee validation on the KDF side.
Moreover, if we were to provide additional ethTakerPaymentZeroFee function, it would just duplicate the erc20TakerPayment logic, differing only by setting dexFee to zero for the paymentHash calculation. Logically, it makes more sense to allow a zero dexFee directly in the erc20TakerPayment function instead of creating a separate function or adding allow_zero_fee bool flag in erc20TakerPayment(just as another example)

@shamardy
Copy link
Collaborator Author

what do you think about removing require(dexFee > 0, "Dex fee must not be zero"); check in erc20TakerPayment function and just allowing dexFee to be zero directly?

Agreed. Most require statements are not really needed if they are validated by swap parties but we keep them as extra precautions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change: swap feature: swap priority: urgent Critical tasks requiring immediate action.
Projects
None yet
Development

No branches or pull requests

3 participants