-
Notifications
You must be signed in to change notification settings - Fork 813
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
wallet: add set tx fee endpoint #518
base: master
Are you sure you want to change the base?
Conversation
Great! Would suggest also adding a |
9a7fe58
to
b1d4943
Compare
Codecov Report
@@ Coverage Diff @@
## master #518 +/- ##
==========================================
- Coverage 52.88% 52.86% -0.03%
==========================================
Files 104 104
Lines 27700 27715 +15
Branches 4748 4750 +2
==========================================
+ Hits 14649 14651 +2
- Misses 13051 13064 +13
Continue to review full report at Codecov.
|
Yeah, I think that would make sense to keep bclient in sync with the server. |
b1d4943
to
d06cb13
Compare
Bcoin HTTP API for fees does not use floats. E.g. Create TX -- we are using Satoshis at HTTP level, same for fees. Note: This won't persist. |
This will force a fixed fee for a transaction? |
Yes, however as nodar said:
I think we should add clear documentation about this.
@tuxcanfly is it possible to 'reset' the behavior to 'floating' (fee estimation) after setting a fee using http/rpc? Doesn't seem obvious this is possible. Currently when I set it from rpc I always have to restart bcoin to return to using estimation. Perhaps allow Would also be cool to expose the rpc |
A configuration option could be a way to persist such fee rates, albiet it will require a restart. Also setting the rate when creating each transaction could be another, with less side effects, way to have configuration here, if it's not already. |
Probably best way would be to persist it within wallet database, even though it might need migration, I believe it can be implemented without migration as well (Don't crash when field is not there). You will have per wallet fee rates. You can overwrite fee rates when creating transaction, it's there already. |
Any updates on this PR? Seems like a candidate for getting merged |
Needs rebase on master. Test would be nice too. And since the client is now integrated into the repo, we could quickly add |
Refs #512