allow feerate argument in bumpfee in addition to confTarget #16203

1. 
   instagibbs commented at 1:31 pm on June 13, 2019: member

   If someone has their own home-grown fee estimator, let’s allow them to use it on command line.
2. fanquake added the label RPC/REST/ZMQ on Jun 13, 2019
3. 
   Sjors commented at 3:23 pm on July 11, 2019: member

   You may be able to borrow some code from signerbumpfee in #15876 which takes a feeRate argument.
4. 
   instagibbs commented at 3:44 pm on July 11, 2019: member

   Was hoping someone newer would tackle this, I think it’s a pretty easy change :)
5. MarcoFalke added the label good first issue on Jul 11, 2019
6. 
   ezegom commented at 0:20 am on July 12, 2019: contributor

   Was hoping someone newer would tackle this, I think it’s a pretty easy change :)

   I’ll take a look at this issue :)

7. 
   instagibbs commented at 8:19 pm on July 22, 2019: member

   @ezegom I wasn’t able to log in to my irc for a bit, sorry if I missed a message
8. 
   jonatack commented at 3:45 pm on July 29, 2019: member

   @ezegom are you still interested? If not, I can do it this weekend, along with updating the bumpfee tests to not use the deprecated totalFee argument.
9. 
   ezegom commented at 5:14 pm on July 29, 2019: contributor

   @jonatack actually, I’m almost done with this feature only thing missing is to write a couple of tests (draft PR should be up today). I’ll add you as a reviewer so you can take a look.
10. 
    jonatack commented at 5:24 pm on July 29, 2019: member

    @ezegom thanks, sounds good.
11. 
    ezegom commented at 7:32 pm on July 29, 2019: contributor

    @instagibbs @Sjors @MarcoFalke Quick question I was taking a look at @Sjors code for signerbumpfee which also allows the user to specify a feeRate. https://github.com/bitcoin/bitcoin/blob/428dfe0e670be210cb774877a04b24f117ff892f/src/wallet/rpcsigner.cpp#L161-L164

    The signerbumpfee rpc eventually calls CreateRateBumpTransaction, which calculates the feeRate to be used as the max between the feeRate passed (minFeeRate) and the old_fee + incrementalRelayFee (feeRate). Thus the feeRate provided by the user is only used if it is larger than old_fee+incrementalRelayFeehttps://github.com/bitcoin/bitcoin/blob/428dfe0e670be210cb774877a04b24f117ff892f/src/wallet/feebumper.cpp#L253-L257

    The implementation I’ve worked on, always creates the transaction using the feeRate passed in unless the feeRate is invalid. To check whether the feeRate passed in is valid or not, I’ve made a helper method that follows the same logic used to check the fee for when the TotalFee argument is used.

    My question is: What is the desired behavior if the user entered a feeRate that is lower than the old_fee + incrementalRelayFee? Should we:

    1. Display error notifying the user that the fee is too small
    2. Use the old_fee + incrementalRelayFee as our feeRate (as @Sjors does)
    3. Use the feeRate passed in by the user unless its less than the old_fee or greater than the wallet’s maximum fee.
12. 
    instagibbs commented at 8:35 pm on July 29, 2019: member

    I’d have to understand more what the implementation looks like, but if a feerate is given, I’d expect it to either pass or fail, not do a fallback. (option 1)
13. 
    MarcoFalke commented at 9:12 pm on July 29, 2019: member

    I think any option is fine, as long as it is documented and the behavior is the same in signerbumpfee and bumpfee
14. 
    ezegom commented at 1:07 am on July 30, 2019: contributor

    @instagibbs, @MarcoFalke : I’ve made a draft PR. Any feedback is greatly appreciated!
15. 
    Sjors commented at 8:04 am on August 2, 2019: member

    1. Display error notifying the user that the fee is too small

    This

    2. Use the old_fee + incrementalRelayFee as our feeRate (as @Sjors does)

    I shouldn’t do that :-)

16. 
    instagibbs commented at 2:06 pm on October 2, 2019: member

    Now merged in master, slated for 0.19, closing
17. instagibbs closed this on Oct 2, 2019

    ---

18. MarcoFalke locked this on Dec 16, 2021

Contributors
instagibbs Sjors ezegom jonatack MarcoFalke

Labels
RPC/REST/ZMQ good first issue