Send RPC bug fix and touch-ups #19969

1. 
   Sjors commented at 1:31 pm on September 17, 2020: member

   Followup based on #16378 nits. It also fixes an argument parsing error (uncaught because the test wasn’t sufficiently thorough).

   I marked the RPC as experimental so we can tweak it a bit over the next release cycle.

2. Mark send RPC experimental efc9b85e6f
3. fanquake added the label RPC/REST/ZMQ on Sep 17, 2020
4. [rpc] send: fix parsing replaceable option 0fc1c685e1
5. Sjors renamed this:
   Send RPC touch-ups
   Send RPC bug fix and touch-ups
   on Sep 17, 2020
6. [rpc] send: various touch-ups d813d26f06
7. rpc: add brackets to ConstructTransaction f7b331ea85
8. Sjors force-pushed on Sep 17, 2020
9. 
   fjahr commented at 11:14 pm on September 17, 2020: member

   utACK f7b331ea85d45c7337e527b6e77a45da7a689b7d
10. 
    DrahtBot commented at 1:25 pm on September 19, 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:

    • #20013 (rpc: normalize parameter names by kallewoof)
    • #16546 (External signer support - Wallet Box edition by Sjors)

    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.

11. in src/wallet/rpcwallet.cpp:3880 in f7b331ea85

    3873@@ -3874,9 +3874,10 @@ static UniValue listlabels(const JSONRPCRequest& request)
    3874 static RPCHelpMan send()
    3875 {
    3876     return RPCHelpMan{"send",
    3877+        "\nEXPERIMENTAL warning: this call may be changed in future releases.\n"
    3878         "\nSend a transaction.\n",
    3879         {
    3880-            {"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "a json array with outputs (key-value pairs), where none of the keys are duplicated.\n"
    3881+            {"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "A JSON array with outputs (key-value pairs), where none of the keys are duplicated.\n"
    

    ---

    ---

    

    MarcoFalke commented at 7:28 am on September 20, 2020:

    0            {"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "Outputs (key-value pairs), where none of the keys are duplicated.\n"
    

    The whole point of RPCHelpMan is to deal with types and formatting, so that the documentation strings can be minimal.

    If you repeat the types, it will be rendered twice:

    01. outputs                               (json array, required) a json array with outputs (key-value pairs), where none of the keys are duplicated.
    

    ---

    

    Sjors commented at 2:29 pm on September 21, 2020:

    Will fix if there’s other issues (or just later)
12. 
    MarcoFalke commented at 7:29 am on September 20, 2020: member

    left a style nit
13. 
    meshcollider commented at 3:39 am on September 25, 2020: contributor

    utACK f7b331ea85d45c7337e527b6e77a45da7a689b7d
14. 
    kallewoof commented at 4:35 am on September 28, 2020: member

    ACK f7b331ea85d45c7337e527b6e77a45da7a689b7d

    I find it weird that even though I used “1 sat/b” it complains about failing to do fee estimation. It shouldn’t need fee estimation at all. Anyway, that’s outside the scope of this nit-fix PR.

15. in doc/release-notes-16378.md:4 in f7b331ea85

    0@@ -1,5 +1,6 @@
    1 RPC
    2 ---
    3 - A new `send` RPC with similar syntax to `walletcreatefundedpsbt`, including
    4-  support for coin selection and a custom fee rate. Using the new `send` method
    5-  is encouraged: `sendmany` and `sendtoaddress` may be deprecated in a future release.
    6+  support for coin selection and a custom fee rate. The `send` RPC is experimental
    

    ---

    ---

    

    fanquake commented at 6:51 am on September 29, 2020:

    Using it is encouraged once it's no longer experimental: This sentence seems unnecessary, and somewhat confusing. There’s no reason not to use it (i.e it being unsafe) as long as you know the interface might change in the future, and that is made clear here and in the help output. Will no-doubt be changed in the wiki pre-release.
16. 
    fanquake commented at 6:55 am on September 29, 2020: member

    Will merge this now. Going forward please pick a single prefix style for your commits and use it consistently. If anything it can make it slightly easier to generate release notes from commit logs etc. The prevailing style seems to be foo: bar.
17. fanquake merged this on Sep 29, 2020
18. fanquake closed this on Sep 29, 2020

    ---

19. sidhujag referenced this in commit 4793d4ff1f on Sep 29, 2020
20. Sjors deleted the branch on Sep 29, 2020
21. deadalnix referenced this in commit be507501c5 on Oct 29, 2021
22. DrahtBot locked this on Feb 15, 2022

Contributors
Sjors fjahr DrahtBot MarcoFalke meshcollider kallewoof fanquake

Labels
RPC/REST/ZMQ