KomodoPlatform / komodo-defi-framework

This is the official Komodo DeFi Framework repository
https://komodoplatform.com/en/docs/komodo-defi-framework/
104 stars 94 forks source link

BTC fees estimations displaying per kilobyte #710

Closed tonymorony closed 3 years ago

tonymorony commented 4 years ago

as a user faced no-go BTC<->X swaps problem, BTC tx fees from estimatesmartfee are way to high if compare with average tx fee:

curl --url "http://127.0.0.1:7783" --data "{\"userpass\":\"$userpass\",\"method\":\"get_trade_fee\",\"coin\":\"BTC\"}"
{"result":{"amount":"0.00148896","amount_fraction":{"denom":"3125000","numer":"4653"},"amount_rat":[[1,[4653]],[1,[3125000]]],"coin":"BTC"}}
bitcoin-cli estimatesmartfee 1 ECONOMICAL
{
  "feerate": 0.00148896,
  "blocks": 2
}

image

What is 15$ per transaction, but according to https://bitinfocharts.com/comparison/bitcoin-transactionfees.html or https://txstreet.com/ avg. fee is ~3-5$ at the moment, what is few times less (segwit fees are 30-40% lower afaik so it might be the case partially)

btw according to that issue this call might have bugs: https://github.com/bitcoin/bitcoin/issues/11800

BTC fees calc is quite complicated matter as I understand (after reding https://bitcointechtalk.com/an-introduction-to-bitcoin-core-fee-estimation-27920880ad0 and https://scholarworks.sjsu.edu/cgi/viewcontent.cgi?article=1637&context=etd_projects https://bitcointechtalk.com/an-introduction-to-bitcoin-core-fee-estimation-27920880ad0)

It's reflections mostly, unlikely it's possible to get BTC fees some trustable way (maybe from oracle?). Therefore we getting fees for ETH from API (gasstation) :)

My only idea on how to improve that without API usage is "custom tx fee" for BTC txs in trades which expereienced users might set similar as it's possible to set for withdrawal calls.

Most practical solution imo is to use some BTC pegged asset for such BTC applications as atomic swaps. There is https://wbtc.network/ for example, but funny thing is that now avg. ETH tx fees almost on the same level with BTC avg. tx fees (but since we getting it from gasstation it's 4-5 times lower than for BTC fees atm in adex)

tonymorony commented 4 years ago

btw, fee I'm getting on withdrawal is 2.5$ atm what is ~6 times less than for trade (or from bitcoin-cli estimatesmartfee 1 ECONOMICAL). Is it calculating by electrumx?

image

cipig commented 4 years ago

some infos from me:

tonymorony commented 4 years ago

So as we've found out in discussion with @artemii235 it's kind of UX bug - estimatefee returns approximate fee per kilobyte. However, actual swap transactions usually contains much lower amount of information. So visually it became a no go for user to start a swap. It would be great to find a way to display actual swap txs estimates.

artemii235 commented 4 years ago

@tonymorony Thanks for your comment!

To calculate the expected fee more precisely we need to know the amount to be traded so get_trade_fee should have 1 more argument. The call will look like get_trade_fee("BTC", "1"). This way we can generate atomic swap transactions preimages and calculate fee using it's size. The real fee can be different actually, but it shouldn't happen often, UTXO split should happen so resulting transaction size will be bigger than preimage size.

sergeyboyko0791 commented 3 years ago

Could someone tell me, when the get_trade_fee used in GUI? On the swap start page? If we add a swap transaction generation on get_trade_fee and the user has insufficient balance, we should return an intelligible error, or return for example estimatefee result?

artemii235 commented 3 years ago

GUIs call the get_trade_fee before buy/sell to display the expected miner fee to be paid during a swap.

If we add a swap transaction generation on get_trade_fee and the user has insufficient balance, we should return an intelligible error, or return for example estimatefee result?

If an address has an insufficient balance, then we should return an error. It will be quite unclear to return the estimatefee result in case of lacking funds - it can create the impression that the provided amount can be used to buy/sell, which is wrong.

I think we can add the trade_preimage method (the name is the subject of discussion). It will accept the same parameters as buy/sell/setprice, check balance, calculate all the required fees, and return them in 1 response.

Let's keep get_trade_fee unchanged to maintain backward compatibility.

artemii235 commented 3 years ago

@sergeyboyko0791 Could you please document the new trade_preimage RPC?

sergeyboyko0791 commented 3 years ago

I'm working on it already :)

artemii235 commented 3 years ago

Nice! Please also add the deprecation warning to the get_trade_fee in docs with a recommendation to use trade_preimage instead.