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

Allow to set dynamic fee in coins file. #656

Closed
artemii235 opened this issue Jun 2, 2020 · 8 comments
Closed

Allow to set dynamic fee in coins file. #656

artemii235 opened this issue Jun 2, 2020 · 8 comments
Assignees
Labels
enhancement New feature or request

Comments

@artemii235
Copy link
Member

BCH has incompatible estimate fee request params so our estimate_fee calls do not work. As BCH daemon always returns 1000 sat per Kbyte we can just set this number in config e.g. in form {"fee_type":"Dynamic","sat_per_kb":1000}. This will do the trick.

NB

@cipig
Copy link
Member

cipig commented Jun 16, 2020

i think i found a better solution for the problem: switch wallet to https://github.com/BitcoinUnlimited/BitcoinUnlimited
which supports estimatesmartfee

bch-cli estimatesmartfee 1
{
  "feerate": 0.00001000,
  "blocks": 1
}

will test swaps with this wallet and if everything works, i will run own electrums with BitcoinUnlimited instead of bitcoin-abc

@artemii235
Copy link
Member Author

Hi @cipig, thanks for finding the workaround!
I will keep this issue open because it might be still useful to have such option, but we can postpone the implementation for a while.

@cipig
Copy link
Member

cipig commented Dec 12, 2020

you were right keeping the issue open
unfortunately the estimatesmartfee method in BitcoinUnlimited is broken, after a while it starts showing weird values
start with 0.00001000, couple minutes later it is 0.00005021, than 0.00149997 and back to 0.00002202
that can't be, since all BCH txes are done with 1 sat/byte: https://blockchair.com/bitcoin-cash
and https://gitlab.com/bitcoin-cash-node/bitcoin-cash-node has the same problem as the original BCH wallet all started with

so we indeed need {"fee_type":"Dynamic","sat_per_kb":1000} for BCH

@cipig
Copy link
Member

cipig commented Dec 12, 2020

bch-cli estimatesmartfee 1
{
  "feerate": 0.00150780,
  "blocks": 1
}

bch-cli estimatesmartfee 2
{
  "feerate": 0.00001000,
  "blocks": 1
}

same result on all 3 electrums...
a possibility to set "estimate_fee_blocks": 2 just like "estimate_fee_mode": "ECONOMICAL" (for BTC) would also solve the problem

@artemii235
Copy link
Member Author

a possibility to set "estimate_fee_blocks": 2 just like "estimate_fee_mode": "ECONOMICAL" (for BTC) would also solve the problem

I will focus on this specific topic this week.

@artemii235
Copy link
Member Author

It's done. @cipig could you test the #804, please?

@cipig
Copy link
Member

cipig commented Feb 1, 2021

works perfect, thanks a lot
patched node: get_trade_fee shows 0.00001
unpatched node: get_trade_fee shows 0.00002834
not a big difference this time, but it could have been (the more time passes since last block, the higher the fee gets)
tried a swap too and it worked fine... started one with both nodes (patched and unpatched) and both BCH transactions were included in the next block
i think you can close the issue, since BCH was the only coin with this problem

@artemii235 artemii235 self-assigned this Feb 1, 2021
@artemii235 artemii235 added the enhancement New feature or request label Feb 1, 2021
@artemii235
Copy link
Member Author

Thanks for testing!

artemii235 added a commit that referenced this issue Feb 2, 2021
* Add UTXO-specific estimate_fee_blocks param to coins file #656

* Fix tests.

* Fix tests after merge.

* Make trade_test_electrum_and_eth_coins more stable
by triggering subscription to the topic.

* Make trade_test_electrum_and_eth_coins more stable
by waiting for bob to listen on p2p port.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants