wallet: Make RPC help compile-time static #19250

1. 
   MarcoFalke commented at 2:24 pm on June 11, 2020: member

   Currently calling help on a wallet RPC method will either return help: unknown command: getnewaddress or the actual help. This runtime dependency of the help is a bug that complicates any tool that relies on documentation. Also, the code that enables the bug is overly complicated and confusing.

   The fix is split into two commits:

   • First, a commit that can be reviewed with the --color-moved=dimmed-zebra option and tested with the included test.
   • Second, a commit that removes the complicated and confusing code.
2. 
   MarcoFalke commented at 2:33 pm on June 11, 2020: member

   This bugfix has been split out from #18531
3. MarcoFalke added this to the milestone 0.21.0 on Jun 11, 2020
4. 
   achow101 commented at 3:32 pm on June 11, 2020: member

   ACK fae7923a0b9be0585ff2b8ce9792cdd52f3b7286
5. DrahtBot added the label RPC/REST/ZMQ on Jun 11, 2020
6. DrahtBot added the label Tests on Jun 11, 2020
7. DrahtBot added the label Wallet on Jun 11, 2020
8. MarcoFalke removed the label Tests on Jun 11, 2020
9. 
   laanwj commented at 4:31 pm on June 11, 2020: member

   I don’t think calling this ‘move-only’, while strictly correct, is what we usually mean with that. It implies that whole functions are moved keeping the functionality the same hence ‘move-only’.

   ACK otherwise. The help doesn’t depend on the wallet being loaded, or what wallet is selected, so moving this upfront makes sense.

10. MarcoFalke renamed this:
    wallet: [move-only bugfix] Make RPC help compile-time static
    wallet: Make RPC help compile-time static
    on Jun 11, 2020
11. wallet: Make RPC help compile-time static fad889cbf0
12. refactor: Remove unused request.fHelp fadf6bd04f
13. MarcoFalke force-pushed on Jun 11, 2020
14. 
    MarcoFalke commented at 4:40 pm on June 11, 2020: member

    Force pushed to removed “move-only bugfix” from the pull request title and commit subject line. No other changes:

     0$ git range-diff  bitcoin-core/master  fae7923a0b fadf6bd04f
     11:  fafcc0d8f5 ! 1:  fad889cbf0 wallet move-only bugfix: Make RPC help compile-time static
     2    @@ Metadata
     3     Author: MarcoFalke <falke.marco@gmail.com>
     4     
     5      ## Commit message ##
     6    -    wallet move-only bugfix: Make RPC help compile-time static
     7    +    wallet: Make RPC help compile-time static
     8     
     9      ## src/wallet/rpcdump.cpp ##
    10     @@ src/wallet/rpcdump.cpp: static void RescanWallet(CWallet& wallet, const WalletRescanReserver& reserver,
    112:  fae7923a0b = 2:  fadf6bd04f refactor: Remove unused request.fHelp
    

15. in src/wallet/rpcwallet.cpp:109 in fadf6bd04f

    107         return wallets[0];
    108     }
    109 
    110-    if (request.fHelp) return nullptr;
    111-
    112     if (!HasWallets()) {
    

    ---

    ---

    

    promag commented at 9:44 am on June 12, 2020:

    if (if wallets.size() == 0) and ditch HasWallets.

    ---

    

    MarcoFalke commented at 11:11 am on June 12, 2020:

    This has one ACK and two stale ACKs, so I’d prefer to keep it as is for now. Maybe @ryanofsky can combine this into the other vpwallets cleaunup?

    ---

    

    promag commented at 12:29 pm on June 12, 2020:

    I can open a PR but it’s too damn height. No rush really.

    ---

    

    MarcoFalke commented at 8:55 pm on June 12, 2020:

    I am happy to review if you open a pull request

    ---

    

    promag commented at 9:35 pm on June 12, 2020:

    Ok #19261
16. 
    promag commented at 9:57 am on June 12, 2020: member

    Tested ACK fadf6bd04f002d05aaff8eba74015e25a41966bc.

    I’ve pushed f11f86451caa1e84e4182118cbadce349a4dfd10 that I think you could cherry pick here.

    Only change is that now you can call help on invalid wallets, like src/bitcoin-cli -rpcwallet=404 -regtest help listunspent and it doesn’t error.

17. 
    achow101 commented at 2:54 pm on June 12, 2020: member

    re-ACK fadf6bd04f002d05aaff8eba74015e25a41966bc
18. MarcoFalke merged this on Jun 12, 2020
19. MarcoFalke closed this on Jun 12, 2020

    ---

20. sidhujag referenced this in commit 65c29c7574 on Jun 12, 2020
21. MarcoFalke deleted the branch on Jun 12, 2020
22. MarcoFalke referenced this in commit eac65d99dd on Jun 13, 2020
23. sidhujag referenced this in commit f308262ced on Jun 14, 2020
24. MarcoFalke referenced this in commit 804ca26629 on Jul 15, 2020
25. sidhujag referenced this in commit ea73ed0a45 on Jul 16, 2020
26. MarcoFalke referenced this in commit 31760bb7c9 on Aug 14, 2020
27. sidhujag referenced this in commit b761dae549 on Aug 16, 2020
28. sidhujag referenced this in commit 010a3815db on Aug 16, 2020
29. MarcoFalke referenced this in commit 89a8299a14 on Aug 31, 2020
30. sidhujag referenced this in commit 4c2ef86224 on Aug 31, 2020
31. MarcoFalke referenced this in commit d692d192cd on Sep 22, 2020
32. sidhujag referenced this in commit f769a0d8b6 on Sep 23, 2020
33. MarcoFalke referenced this in commit 5e14fafb31 on Sep 23, 2020
34. sidhujag referenced this in commit 63306f71f5 on Sep 23, 2020
35. MarcoFalke referenced this in commit 71d068db40 on Nov 19, 2020
36. sidhujag referenced this in commit 92bd89791a on Nov 19, 2020
37. Fabcien referenced this in commit 1a57c26881 on Apr 29, 2021
38. Fabcien referenced this in commit b8f1ee7692 on Apr 29, 2021
39. Fabcien referenced this in commit d66b0479d4 on Apr 29, 2021
40. cryptapus referenced this in commit 72b476a0cf on May 3, 2021
41. cryptapus referenced this in commit d481525cc9 on May 3, 2021
42. DrahtBot locked this on Feb 15, 2022

Contributors
MarcoFalke achow101 laanwj promag

Labels
Wallet RPC/REST/ZMQ

Milestone
0.21.0