Accept options as named-only parameters (rpc/rest/zmq)

Host: stickies-v  -  PR author: ryanofsky

The PR branch HEAD was 2808c33ba at the time of this review club meeting.


  • RPC methods can take parameters in three ways:
    • Using positional parameters, e.g. bitcoin-cli getblock "00000000c937983704a73af28acdec37b049d214adbda81d7e2a3dd146f6ed09"
    • Using named parameters, e.g. bitcoin-cli named getblock blockhash="00000000c937983704a73af28acdec37b049d214adbda81d7e2a3dd146f6ed09"
    • Using an options object, e.g. bitcoin-cli -named bumpfee "66975ce3ea2b0815d677eaac1f1822276943cf7361d3eb920ad3cc278b473609" options='{"fee_rate": 10}'
  • For an end-user (especially through bitcoin-cli), the options notation can be quite verbose, especially when they just want to specify a single parameter.

  • Some endpoints, such as send, allow passing parameters such as conf_target either as a options field or a named/positional parameter, but this comes with code overhead and needs to be implemented by every RPC method.

  • To simplify the interface, #26485 allows any options parameter to also be passed as a named parameter.


  1. Did you review the PR? Concept ACK, approach ACK, tested ACK, or NACK? What was your review approach?

  2. Why do some RPCs use an options parameter? Do we still need it? If so, what for? If not, can we remove it?

  3. Which function is responsible for checking if an options field is passed as a named parameter? What other approaches can you think of to achieve the same goal this PR is trying to achieve?

  4. The documentation for send lists conf_target both as a named argument (#2) as well as a field in options. When looking at the code, however, it seems like conf_target is defined only once. How is this possible?

  5. Why does RPCHelpMan::GetArgNames() now return a std::vector<std::pair<std::string, bool>> instead of a std::vector<std::string>? What does the bool represent?

  6. In transformNamedArguments, why do we use __pushKV instead of pushKV?

  7. What is the fr input parameter? Why are we handling this case separately?

Meeting Log

  117:00 <stickies-v> #startmeeting
  217:00 <kevkevin> hi
  317:01 <pablomartin> hello
  417:01 <stickies-v> welcome everyone! Today we're looking at #26485, authored by ryanofsky. The notes and questions are available on
  517:01 <effexzi> Hi every1
  617:01 <abubakarsadiq> hello
  717:01 <stickies-v> anyone joining us for the first time today? even if you're just lurking, feel free to say hi!
  817:02 <stickies-v> who got the chance to review the PR or read the notes? (y/n)
  917:02 <kevkevin> n
 1017:02 <kevkevin> well read some of the notes
 1117:02 <pablomartin> n - not yet, it was on my list of pendings
 1217:03 <abubakarsadiq> I read the PR and concept Ack
 1317:04 <stickies-v> nice one abubakarsadiq ! looks like not too many have gone in-depth so we can stay a bit more on the high level too, today, let's see.
 1417:04 <stickies-v> how would you summarize this PR in your own words?
 1517:04 <LarryRuane> hi
 1617:05 <pablomartin> it adds a possibilty to avoid verbose options and passing "named args" as a dict
 1717:05 <yashraj> this PR simplifies the syntax of some RPC commands?
 1817:06 <stickies-v> pablomartin: could you go in a bit more detail on the "passing named args as a dict" bit?
 1917:06 <abubakarsadiq> the PR enables rpc options parameter keys to be also passed as parameters
 2017:06 <stickies-v> yashraj: is it a backwards compatible simplification?
 2117:07 <pablomartin> sorry, the other way around, haha, my bad... instead of options='{"fee_rate": 10}' as a named arg fee_rate=10
 2217:07 <stickies-v> abubakarsadiq: named parameters, to be precise! can they both be passed as an options items as well as a named parameter?
 2317:08 <pablomartin> *params, not args yeah
 2417:08 <stickies-v> pablomartin: well, that's just the cli syntax. on the RPC side (which is the only thing this PR is touching), we're indeed passing named arguments as a dict/object, but that's nothing new - we already had named arguments
 2517:09 <pablomartin> true, i was referring to the bitcoin-cli side
 2617:09 <yashraj> stickies-v: yeah you can still use the options={}
 2717:09 <abubakarsadiq> you can pass as either named params or option items, I am not sure though
 2817:09 <stickies-v> yashraj: exactly! it's just an additional way to interface with RPC, applications can keep using the `options` parameter
 2917:09 <LarryRuane> I like how the PR updates all the tests to use the simplified syntax!
 3017:10 <LarryRuane> (er... i'm not sure if "all" but many at least)
 3117:11 <stickies-v> abubakarsadiq: only one of both is allowed, but you can mix and match (pass some as options keys, and others as named args):
 3217:12 <stickies-v> LarryRuane: yeah, it definitely does make things more readable 👍
 3317:12 <stickies-v> Which function is responsible for checking if an `options` field is passed as a named parameter? What other approaches can you think of to achieve the same goal this PR is trying to achieve?
 3417:12 <abubakarsadiq> thanks stickes: talking about that whats fr?
 3517:13 <LarryRuane> Probably some of the tests should use the old syntax to make sure it doesn't break (and some may still, I didn't check)
 3617:13 <abubakarsadiq> transformNamedArguments
 3717:13 <stickies-v> abubakarsadiq: we'll get to that in a later question, actually!
 3817:13 <stickies-v> abubakarsadiq: yeah I did kinda give it away with my previous link already hahaha
 3917:14 <yashraj> u for real
 4017:14 <stickies-v> so, does anyone have ideas for alternative approaches for this PR?
 4117:14 <stickies-v> yashraj: ?
 4217:14 <abubakarsadiq> yeah :)
 4317:15 <yashraj> sorry, ignore!
 4417:15 <LarryRuane> another proposal is (was):
 4517:15 <LarryRuane> but I haven't looked to see how it differs
 4617:17 <stickies-v> yeah, I still need to look into it myself actually hah
 4717:17 <stickies-v> alright, moving on
 4817:17 <stickies-v> The documentation for `send` lists `conf_target` both as a named argument (#2) as well as a field in `options`. When looking at the code, however, it seems like `conf_target` is defined only once. How is this possible?
 4917:17 <pablomartin> yeah, same LarryRuane... it seems simpler/ less changes... but not sure about the use of it
 5017:18 <stickies-v> (links: , <a href='' target='blank'></a> ,
 5117:20 <abubakarsadiq> initially is their a reason why some arguments are passed through `options` not named parameters e.g `conf_target` for send rpc
 5217:21 <stickies-v> ohh that's a great question abubakarsadiq and actually one that i meant to cover in the previous question
 5317:23 <stickies-v> named arguments have only been added to bitcoin core since v14.0
 5417:24 <stickies-v> and for RPCs with a lot of parameters, such as e.g. the `send` family, it's quite cumbersome to provide a whole bunch of null/default values for every single RPC call.
 5517:24 <stickies-v> so the `options` parameter was used instead
 5617:24 <stickies-v> and now we have both for backwards compatibility
 5717:25 <LarryRuane> would the old way eventually be removed? I'm guessing probably not?
 5817:25 <stickies-v> I think that's the main reason, but I wasn't there when all of this was done, so I may be missing something
 5917:25 <abubakarsadiq> thats cool, you can use any. thanks stickies-v.
 6017:26 <stickies-v> LarryRuane: seems pretty low priority, probably, at least until we more drastically overhaul the RPC interface?
 6117:26 <LarryRuane> stickies-v: +1 thanks
 6217:26 <stickies-v> (hint for the current question:
 6317:29 <pablomartin> indicates if /*named_only=*/?
 6417:31 <stickies-v> so my point is if you run `bitcoin-cli help send`, the help shows definitions for e.g. `conf_target` twice: once as a named parameter, and once as an `options` field. but in the code, we only seem to be defining it once
 6517:31 <stickies-v> but so the trick is that we sometimes just put a bunch of cli args in a function so we can reuse it in multiple places, which is what's happening here with
 6617:32 <stickies-v> and then we use the `Cat` helper function to just concatenate both vectors:
 6717:33 <abubakarsadiq> because because it's passed in FundTxDoc, with other args like pubkeys
 6817:33 <stickies-v> anyway, not something super relevant to the PR but i found interesting to highlight since it's a pattern used in quite a few methods
 6917:33 <LarryRuane> oh I see, it's in there also:
 7017:33 <stickies-v> abubakarsadiq: LarryRuane yup!
 7117:33 <stickies-v> Why does `RPCHelpMan::GetArgNames()` now return a `std::vector<std::pair<std::string, bool>>` instead of a `std::vector<std::string>`? What does the `bool` represent?
 7217:33 <pablomartin> I see
 7317:34 <stickies-v> (link:
 7417:35 <pablomartin> oh, my last answer was for this one actually
 7517:35 <LarryRuane> the comment for that function kind of gives it away: "Return list of arguments and whether they are named-only"
 7617:36 <pablomartin> like here:
 7717:37 <stickies-v> LarryRuane: yeah I guess it does haha, but why do we need to distinguish here between which arguments are named-only?
 7817:41 <stickies-v> the answer is pretty simple actually, we just want to be able to specify for which objects we enable passing keys as named parameters
 7917:42 <stickies-v> alright, moving on:
 8017:42 <yashraj> someone might use named-only with the options syntax?
 8117:42 <stickies-v> In `transformNamedArguments`, why do we use `__pushKV` instead of `pushKV`?
 8217:42 <stickies-v> (link:
 8317:42 <stickies-v> yashraj: this is an internal API, not something we'd expose to the end user
 8417:43 <stickies-v> whoever implements the RPC method needs to define the parameters and how they can be specified
 8517:45 <yashraj> thanks
 8617:45 <pablomartin> yeah you need to be able to distinguish cos you need to pass/ push it to a diff section
 8717:46 <LarryRuane> looks like __pushKV allows multiple of same key?
 8817:48 <stickies-v> pablomartin: oh yeah, absolutely, the end-user decides whether they pass positional or named args. but adding the `bool` as a `pair` item allows us to let the developer specify per-method how they want this behaviour to work, as opposed to for example automatically enabling it for all `OBJ` parameters, or for all `OBJ` parameters named `options`
 8917:48 <stickies-v> LarryRuane: exactly. (why) is that safe?
 9017:49 <LarryRuane> for anyone who would like a link:
 9117:50 <LarryRuane> oh because we've already checked that it doesn't exist:
 9217:51 <pablomartin> stickies-v: thanks
 9317:51 <LarryRuane> so it's a small performance improvement (?)
 9417:53 <stickies-v> LarryRuane: yeah exactly. kinda like how even though when accessing a vector element it's safer and generally recommended to use ``, you'll often see `v[i]` used in our codebase but (typically/hopefully) only if we've ensured that `i` definitely is in range, because then it's just a bit faster
 9517:54 <stickies-v> alright abubakarsadiq now we're coming back to the q you had earlier
 9617:54 <stickies-v> What is the `fr` input parameter? Why are we handling this case separately?
 9717:54 <stickies-v> link:
 9817:55 <LarryRuane> stickies-v: +1 and also I'd say using `__pushKV` documents the code better... because it describes exactly what the effect will be
 9917:59 <stickies-v> i think (but didn't name the variable) that `fr` just stands for `find_result` or something (not a huge fan of 1-2 letter named variables...)
10018:00 <LarryRuane> OH! that makes sense, I couldn't figure that out!
10118:00 <stickies-v> we just want to check if we've already processed the key/value pair earlier, and if so throw an error, to avoid people passing the same parameter as option/positional/named parameter
10218:00 <abubakarsadiq> yeah find_result is much better
10318:00 <stickies-v> alright i think that's all for today folks, thank you for attending and see you next week!
10418:01 <stickies-v> #endmeeting