rpc: Don’t ignore -maxtxfee when wallet is disabled #15357

1. 
   JBaczuk commented at 6:47 pm on February 6, 2019: contributor

   Resolves #15355
2. in src/init.cpp:814 in 81e9788de0 outdated

   805@@ -806,6 +806,22 @@ void InitParameterInteraction()
   806             LogPrintf("%s: parameter interaction: -whitelistforcerelay=1 -> setting -whitelistrelay=1\n", __func__);
   807     }
   808 
   809+    // This is required by both the wallet and node
   810+    if (gArgs.IsArgSet("-maxtxfee"))
   811+    {
   812+        CAmount nMaxFee = 0;
   813+        if (!ParseMoney(gArgs.GetArg("-maxtxfee", ""), nMaxFee))
   814+            InitError(AmountErrMsg("maxtxfee", gArgs.GetArg("-maxtxfee", "")));
   

   ---

   ---

   

   ryanofsky commented at 7:00 pm on February 6, 2019:

   return InitError has been changed to just InitError here so I think this changes behavior.

   Maybe this code should move to the AppInitParameterInterfaction function instead of InitParameterInterfaction to avoid needing this change.

   ---

   

   JBaczuk commented at 7:55 pm on February 6, 2019:

   I wasn’t sure about that, but WalletInit::ParameterInteraction returned a bool and InitParameterInteraction returns void, so I removed it. InitParameterInteraction looks like it will work, though.

   ---

   

   MarcoFalke commented at 8:03 pm on February 6, 2019:

   looks like it will work

   Did you test that the initialization is aborted when you supply an invalid maxtxfee?

   ---

   

   JBaczuk commented at 5:19 am on February 7, 2019:

   Did you test that the initialization is aborted when you supply an invalid maxtxfee?

   It will not abort unless it returns. I’ll move it to AppInitParameterInteraction so it can return and abort.

3. 
   DrahtBot commented at 8:45 pm on February 6, 2019: 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:

   • #15340 (gui: Introduce bilingual GUI error messages by hebasto)

   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.

4. fanquake added the label Refactoring on Feb 6, 2019
5. 
   JBaczuk commented at 5:23 am on February 7, 2019: contributor

   Ok I’ve tested and it does abort if invalid, and issues a warning if fee is too high.
6. 
   Empact commented at 12:47 pm on February 7, 2019: member

   Maybe move it up to around line 1099, to sit alongside the other fee-related options handling?
7. MarcoFalke renamed this:
   Move maxTxFee initialization to init.cpp
   rpc: Don't ignore `-maxtxfee` when wallet is disabled
   on Feb 7, 2019
8. MarcoFalke added the label RPC/REST/ZMQ on Feb 7, 2019
9. MarcoFalke removed the label Refactoring on Feb 7, 2019
10. 
    JBaczuk commented at 5:51 am on February 8, 2019: contributor

    Maybe move it up to around line 1099, to sit alongside the other fee-related options handling?

    done

11. fanquake deleted a comment on Feb 8, 2019
12. 
    MarcoFalke commented at 1:50 pm on February 8, 2019: member

    Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits. There is no need to keep the git history around in this pull request.
13. Move maxTxFee initialization to init.cpp dfbf117bbb
14. JBaczuk force-pushed on Feb 8, 2019
15. 
    jnewbery commented at 2:23 pm on February 8, 2019: member

    utACK dfbf117bbb82c76c8cb60dba73ae7d653e3b8348 because it at least makes behaviour consistent. I still don’t think that -maxtxfee should be shared between the node and wallet, so this doesn’t fully address my concerns in #15355. Reasoning here: #15355 (comment).
16. 
    MarcoFalke commented at 4:29 pm on February 8, 2019: member

    ACK dfbf117bbb82c76c8cb60dba73ae7d653e3b8348.

    Checked that it is no longer ignored when the wallet is not compiled or not enabled:

    0$ ./configure --disable-wallet && make
    1$ ./src/qt/bitcoin-qt -maxtxfee=foobar
    2Error: Invalid amount for -maxtxfee=<amount>: 'foobar'
    3
    4$ ./configure && make
    5$ ./src/qt/bitcoin-qt -maxtxfee=foobar -disablewallet
    6Error: Invalid amount for -maxtxfee=<amount>: 'foobar'
    

    

17. MarcoFalke added this to the milestone 0.18.0 on Feb 8, 2019
18. ryanofsky approved
19. 
    ryanofsky commented at 7:20 pm on February 8, 2019: member

    utACK dfbf117bbb82c76c8cb60dba73ae7d653e3b8348, confirmed move only.
20. MarcoFalke merged this on Feb 8, 2019
21. MarcoFalke closed this on Feb 8, 2019

    ---

22. MarcoFalke referenced this in commit 2945492424 on Feb 8, 2019
23. Munkybooty referenced this in commit 139a98561d on Aug 27, 2021
24. Munkybooty referenced this in commit 56a4ffa535 on Aug 28, 2021
25. Munkybooty referenced this in commit 5e9a6baeab on Aug 29, 2021
26. Munkybooty referenced this in commit 504f40e3de on Sep 1, 2021
27. Munkybooty referenced this in commit e47609493c on Sep 5, 2021
28. Munkybooty referenced this in commit 4efa0a69b6 on Sep 7, 2021
29. Munkybooty referenced this in commit 3a9a1cb2eb on Sep 7, 2021
30. Munkybooty referenced this in commit c4b8697a7f on Sep 9, 2021
31. Munkybooty referenced this in commit 2d5cdd45b8 on Sep 10, 2021
32. Munkybooty referenced this in commit ae4bea6052 on Sep 13, 2021
33. DrahtBot locked this on Dec 16, 2021

Contributors
JBaczuk ryanofsky MarcoFalke DrahtBot Empact jnewbery

Labels
RPC/REST/ZMQ

Milestone
0.18.0