Send RPC bug fix and touch-ups #19969
Conversation
af3d1e0 to
f7b331e
Compare
|
utACK f7b331e |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
| "\nSend a transaction.\n", | ||
| { | ||
| {"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "a json array with outputs (key-value pairs), where none of the keys are duplicated.\n" | ||
| {"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "A JSON array with outputs (key-value pairs), where none of the keys are duplicated.\n" |
There was a problem hiding this comment.
| {"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "A JSON array with outputs (key-value pairs), where none of the keys are duplicated.\n" | |
| {"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:
1. outputs (json array, required) a json array with outputs (key-value pairs), where none of the keys are duplicated.
There was a problem hiding this comment.
Will fix if there's other issues (or just later)
There was a problem hiding this comment.
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.
| @@ -1,5 +1,6 @@ | |||
| RPC | |||
| --- | |||
| - A new `send` RPC with similar syntax to `walletcreatefundedpsbt`, including | |||
| support for coin selection and a custom fee rate. Using the new `send` method | |||
| is encouraged: `sendmany` and `sendtoaddress` may be deprecated in a future release. | |||
| support for coin selection and a custom fee rate. The `send` RPC is experimental | |||
There was a problem hiding this comment.
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.
f7b331e rpc: add brackets to ConstructTransaction (Sjors Provoost) d813d26 [rpc] send: various touch-ups (Sjors Provoost) 0fc1c68 [rpc] send: fix parsing replaceable option (Sjors Provoost) efc9b85 Mark send RPC experimental (Sjors Provoost) Pull request description: Followup based on bitcoin#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. ACKs for top commit: meshcollider: utACK f7b331e fjahr: utACK f7b331e kallewoof: ACK f7b331e Tree-SHA512: 82dd8ac76a6558872db3f5249d4d6440469400aaa339153bc627d1ee673a91ecfadecb486bc1939ba87ebbd80e26ff29698e93e358599f3d26fde0e526892afe
Summary: > Mark send RPC experimental bitcoin/bitcoin@efc9b85 > [rpc] send: various touch-ups bitcoin/bitcoin@d813d26 The other two commits are not relevant. The second commit of this PR fixed a bug related to RBF, which did not affect us. This is a backport of [[bitcoin/bitcoin#19969 | core#19969]] Test Plan: ``` ninja all check-all src/bitcoin-cli help send ``` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D10396
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.