DON'T MERGE: Adding Psuedo-Type Info To RPC/REST JSON

[harding]

This is a quick (but functional) mock-up of @darioteixeira's recent suggestion to bitcoin-development implemented for just a single RPC, AddMultiSigAddress.

We add a new section (preview) for types, which starts with a warning that the extended type definitions are non-normative.

The essential characteristics of each type are described. For this quick mock-up, I provided type details off the top of my head, and only did the types used by AddMultiSigAddress. For an actual pull, I'd of course check the code or run tests.

Then we use those types in the AddMultiSignature description (HTML preview), which looks like this:

To make some extra room, I renamed the Presence column to "#" and used symbolic representations of the previous text description. I dislike this---it's less clear---but space is limited.

That's it. Making some crude guesses, I think there's probably between 20 to 40 "types" that would need to be researched and written, and then all the tables would need to be updated, making for probably about a 10 to 20 hour update. The change seems useful enough, although probably not a high priority.

@darioteixeira comments? Anyone else, comments?

[darioteixeira]

Thanks for this -- it's a good first step. Some remarks:

• Using just a comma to separate the "Key or address" variants is somewhat unorthodox. I would explicitly use ", or" as a separator, just to be clear that it's one-or-the-other and not one-and-the-other (the comma is typically used for the latter, and the pipe character for the former).
• The union-type "pubkey | P2PKH address" shows up at least three times in the API. So many times, in fact, that it may be worth defining a type alias for it. I've done so in the OCaml-bitcoin API. (Note that the names I used are just a placeholder, though).
• I agree that the Presence column is not very clear. Is there really no wiggle room for increasing the table width?

In the meantime i've brought OCaml-bitcoin up-to-date with Bitcoin Core 0.10. Even if you don't have experience with OCaml, understanding the API is simple enough. Consider for instance the type signature for addmultisigaddress: parameters are separated by the -> operator, optional parameters are preceded by ?, the conn parameter is an optional connection handler (common to all RPC calls), and I've defined a type alias named multi_t for "pubkey | P2PKH address". Also, you may safely ignore the monad_t type at the end of each signature.

While most of the grunt work of updating the Bitcoin API reference could be reduced by using the OCaml-bitcoin API as a reference, I actually advise against it. I'm pretty sure it contains errors and/or inconsistencies, and one would be just propagating those errors. Also, some of the names I used for the types are obviously not explicit enough.

[harding]

@darioteixeira Thanks!

Using just a comma to separate the "Key or address" variants is somewhat unorthodox.

Agreed. I tried using a pipe, as you suggested in your email, but found that it only works well when everything is on one line, and we don't have space for that in the table unless we use much less descriptive names for the types. I'll try using "or", but I do note that I think (at least in this case), the Description field makes it clear that its either an address or key (as does the description of the array). The developer needs to read that text description anyway because the key set is further reduced to only those belonging to the wallet.

The union-type "pubkey | P2PKH address" shows up at least three times in the API. [I]t may be worth defining a type alias for it.

Definitely something to think about when this gets expanded.

I agree that the Presence column is not very clear. Is there really no wiggle room for increasing the table width?

I can add abbr tooltips so that when the user hovers over ">=1" it says "Required (1 or more)". As for expanding the tables, we have pull #723 open about that, but I haven't found an easy way except for reducing the font size. I think I'd like to have tables similar to the way Wikipedia does TV episode listings, with a row of stats followed by a text description:

But our Markdown parser doesn't support rows that support multiple columns. Since all of our RPC tables are already in a class, maybe I can write a plugin to reformat the output. I'll see if I can find time to experiment with that.

Thanks again!

[darioteixeira]

Agreed. I tried using a pipe, as you suggested in your email, but found that it only works well when
everything is on one line, and we don't have space for that in the table unless we use much less
descriptive names for the types. I'll try using "or", but I do note that I think (at least in this case),
the Description field makes it clear that its either an address or key (as does the description of the
array). The developer needs to read that text description anyway because the key set is further
reduced to only those belonging to the wallet.

Yeah, I agree you can get away with just using a comma as long as the description field makes it clear you should be ORing the types.

But our Markdown parser doesn't support rows that support multiple columns. Since all of our
RPC tables are already in a class, maybe I can write a plugin to reformat the output. I'll see if I
can find time to experiment with that.

Yes, Markdown may be convenient, but it's sorely lacking for more complex requirements. Slightly reducing the font size and using hover-tips may indeed be the best way around those limitations.

[harding]

Note: I think I've found a hack using CSS flex that will allow us to create multi-row entries (like the Wikipedia screenshot above) using plain Markdown. I hope to implement and test it this afternoon, and I'll submit a separate pull request when I'm done.

If it works out, it'll make the adding type information easier as I won't have to re-layout each table.

[darioteixeira]

@harding: things seem to be quiet on this front. Is there anything I can do to help move this forward?

[harding]

@darioteixeira sorry about that; this is still on my todo list. As we merged pull #758, we can now easily support alternative table layouts (including multiple rows per entry) on a per-table basis. I'll see if I can find time to update this pull later today to use that feature as a demonstration.

I usually go through and update the RPC documentation when a new major version of Bitcoin Core enters the release candidate stage, so I think that would be an opportune time to add the type information.

[darioteixeira]

@harding: I agree that the most straightforward way to fix this is to review the RPC calls one by one and add the type information manually. Feel free to page me if you encounter any non-straightforward ones, and thanks again for your attention!