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 Coinswap Fees #308

Open
mojoX911 opened this issue Nov 20, 2024 · 0 comments · May be fixed by #312
Open

Fix Coinswap Fees #308

mojoX911 opened this issue Nov 20, 2024 · 0 comments · May be fixed by #312
Assignees
Labels
bug Something isn't working enhancement This enhances the code and improves stuffs
Milestone

Comments

@mojoX911
Copy link

mojoX911 commented Nov 20, 2024

The Problem

The coinswap fee calculation is weird (broken) and it came from the legacy code. Which makes it very complex to expose to the users.

There are two fee_rate variables in PPB, which is hard for non-dev users to grasp. The same calculation could be easily converted to % feerate. These two fee rates are also exposed in the config variables, and changing them to 0 (if the user wants to disable it) creates arithmetic overflow inside calculate_coinswap_fee function.

The time_relative_fee_ppb rate is intended to be the extra tax maker charges because of how much time their funds get locked in case of a refund. If the time is large, they charge larger fees. This variable is set by the Taker (Currently hardcoded at 48 blocks), and adding a fee there incentivizes the market to choose a quicker refund timeout.

But this is not what the impl does. Instead, the current impl multiplies the fee rate with no. of conf required for funding tx. Which doesn't make any sense.

We need some fixes to make these fee rates configurable by the users.

Proposed Solution.

  • Hardcode the feerates for now. Move from PPB to % representation.
  • Find suitable default values for both rates.
  • Modify the calculate_coinswap_fee to not have overflow bugs if any of the rate is set to 0. That would mean just disabling that rate.
  • Explain clearly how the new fee rate calculation works.
  • Decide later whether to expose this to the users. And expose the refund timelock for taker users.

I am working on this. PR landing soon.

@mojoX911 mojoX911 added bug Something isn't working enhancement This enhances the code and improves stuffs labels Nov 20, 2024
@mojoX911 mojoX911 added this to the v0.1.0 milestone Nov 20, 2024
@mojoX911 mojoX911 self-assigned this Nov 20, 2024
@mojoX911 mojoX911 linked a pull request Nov 25, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement This enhances the code and improves stuffs
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

1 participant