wallet: deprecate feeRate in fundrawtransaction/walletcreatefundedpsbt #20483

1. 
   jonatack commented at 9:52 pm on November 24, 2020: member

   Continuing work on #19543 and following the plan outlined in #20484 (comment), deprecate the feeRate option in fundrawtransaction and walletcreatefundedpsbt to avoid confusion with fee_rate, as the two options have different units (BTC/kvB and sat/vB) and similar spellings.

   This PR currently targets deprecation for 0.21 and so no release notes are added here, as the wiki would be edited instead. Will update if tagged for 0.22 instead.

   The last commit, “test: add feeRate tests to rpc_deprecate.py,” is best reviewed with -w.

   Related to #20391.

2. test: update mempool_limit.py from feeRate to fee_rate 530cd92b12
3. test: update wallet_keypool.py from feeRate to fee_rate 564f2f95d2
4. wallet: deprecate feeRate in fundrawtxn and walletcreatefundedpsbt c38d784ee4
5. test: add feeRate tests to rpc_deprecate.py

   best reviewed with -w

   dcd6d7a5be
6. fanquake added the label Wallet on Nov 24, 2020
7. 
   DrahtBot commented at 6:52 am on November 25, 2020: member

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #20753 (rpc: Allow to ignore specific policy reject reasons by MarcoFalke)
   • #20267 (Disable and fix tests for when BDB is not compiled by achow101)
   • #19168 (Refactor: Improve setup_clean_chain semantics by fjahr)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

8. laanwj added this to the "Blockers" column in a project

   ---

9. laanwj added this to the milestone 0.21.0 on Nov 26, 2020
10. 
    MarcoFalke commented at 7:31 pm on November 26, 2020: member

    Do we need to be aggressive in deprecating this? The estimatesmartfee is still in BTC/kB, so merging this would force all users to do the conversion themselves (or force to pass -deprecatedrpc).
11. 
    jonatack commented at 7:58 pm on November 26, 2020: member

    Several reviewers gave feedback that this would be good to fix. If that includes updating estimatesmartfee for a coherent whole, then, like this patch, it’s fairly trivial to write and review. If reviewers prefer to hold off, I’ll update this patch for 0.22.

    Update: currently following the plan outlined in #20484 (comment).

12. 
    MarcoFalke commented at 11:02 am on November 30, 2020: member

    Several reviewers gave feedback that this would be good to fix

    Where? I am the first one to comment on this pull.

    Regardless, the feature freeze of 0.21 has been a month ago, so I stand by my NACK (at least for 0.21). For 22.0, this could be reconsidered.

13. 
    jonatack commented at 11:16 am on November 30, 2020: member

    Where? I am the first one to comment on this pull.

    In #20305.

    I agree it makes sense to do this fix as part of the described larger plan, but I hope users won’t find it too confusing between now and when the fix arrives in a release.

14. laanwj removed this from the "Blockers" column in a project

    ---

15. MarcoFalke removed this from the milestone 0.21.0 on Dec 3, 2020
16. 
    MarcoFalke commented at 8:14 pm on December 3, 2020: member

    Removed milestone for now
17. 
    jonatack commented at 6:52 pm on January 3, 2021: member

    Update: currently following the plan outlined in #20484 (comment).

    This pull proposed to deprecate the feeRate option in fundrawtransaction and walletcreatefundedpsbt to avoid user confusion with fee_rate, as the two params have different units (BTC/kvB and sat/vB) and very similar spellings. However, review has been negative, the plan is currently somewhat stalled, and there has been no interest in resolving this. I think it was necessary to at least propose a solution, can re-open if/when there is demand.

18. jonatack closed this on Jan 3, 2021

    ---

19. DrahtBot locked this on Aug 16, 2022

Contributors
jonatack DrahtBot MarcoFalke

Labels
Wallet