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

BTC fees estimations displaying per kilobyte #710

Closed
tonymorony opened this issue Aug 19, 2020 · 9 comments · Fixed by #797
Closed

BTC fees estimations displaying per kilobyte #710

tonymorony opened this issue Aug 19, 2020 · 9 comments · Fixed by #797
Assignees
Labels
bug Something isn't working

Comments

@tonymorony
Copy link

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: bitcoin/bitcoin#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 tonymorony added the question Further information is requested label Aug 19, 2020
@tonymorony
Copy link
Author

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
Copy link
Member

cipig commented Aug 19, 2020

some infos from me:

  • for takers, BTC txfee shown is double the txfee from estimatesmartfee, because taker using BTC needs to make 2 transactions, dexfee tx in BTC and takerpayment tx, also in BTC
  • electrum does not calculate fees, it simply passes the call to bitcoind, so estimatesmartfee should always be the same as what mm2 shows on get_trade_fee
  • estimatesmartfee indeed shows a higher fee than what is shown on https://blockchair.com/bitcoin as "Recommended fee" and when the fee is declining, estimatesmartfee needs a long time to adapt (without ECONOMICAL it took even longer)
  • the average fee shown on most sites is the average fee from the last 24h, so we can't compare it with that... "Recommended fee" from https://blockchair.com/bitcoin is the most accurate value, from my experience... even lower fees than what is shown there would work most of the time (people are paying much more fee than they would need to in the last months, but in order to take advantage of that, you would need to analyze mempool)
  • using "Recommended fee" from there would indeed lead to lower fees, but looks like they offer only paid API, so we would need to find something else
  • best way to avoid the fees is to not use BTC... especially people who withdraw BTC from CEX to AtomicDEX should instead buy LTC or BCH on that CEX and withdraw those instead (also saves a lot of fees on the CEX, since BTC withdrawals cost min. $5)... at least my prices on AtomicDEX are the same for BTC, LTC, BCH

@tonymorony
Copy link
Author

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.

@tonymorony tonymorony added bug Something isn't working and removed question Further information is requested labels Sep 17, 2020
@tonymorony tonymorony changed the title BTC fees BTC fees estimations displaying per kilobyte Sep 17, 2020
@artemii235
Copy link
Member

@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
Copy link

sergeyboyko0791 commented Dec 25, 2020

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
Copy link
Member

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.

@sergeyboyko0791 sergeyboyko0791 linked a pull request Feb 1, 2021 that will close this issue
@artemii235
Copy link
Member

@sergeyboyko0791 Could you please document the new trade_preimage RPC?

@artemii235 artemii235 reopened this Feb 2, 2021
@sergeyboyko0791
Copy link

I'm working on it already :)

@artemii235
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants