This enforces most syntax rules of the RPCResult at compile time (or some at run time during unit and functional tests)
Apart from normalizing the syntax, by separating stylistic formatting from the structure, we could in theory directly generate the html for e.g. https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/importmulti/
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, 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.
844@@ -843,8 +845,8 @@ static UniValue sendmany(const JSONRPCRequest& request)
845 " \"CONSERVATIVE\""},
846 },
847 RPCResult{
848- "\"txid\" (string) The transaction id for the send. Only 1 transaction is created regardless of \n"
849- " the number of addresses.\n"
850+ RPCResult::Type::STR, "txid", "The transaction id for the send. Only 1 transaction is created regardless of\n"
Can this be of type HEX_STR?
I’m still new to Bitcoin, my current understanding is that transactions are represented in hex.
968@@ -967,10 +969,11 @@ static UniValue addmultisigaddress(const JSONRPCRequest& request)
969 {"address_type", RPCArg::Type::STR, /* default */ "set by -addresstype", "The address type to use. Options are \"legacy\", \"p2sh-segwit\", and \"bech32\"."},
970 },
971 RPCResult{
972- "{\n"
973- " \"address\":\"multisigaddress\", (string) The value of the new multisig address.\n"
974- " \"redeemScript\":\"script\" (string) The string value of the hex-encoded redemption script.\n"
975- "}\n"
976+ RPCResult::Type::OBJ, "", "",
The first one is the name and it is only used when it is needed to name it (i.e. as a key in a dictionary). Not sure if it makes sense to omit it from the constructor when it is not needed. I’ll wait for others to chime in.
The second string is the description, which should never be empty. Or at least it shouldn’t be made easy to leave empty. I prefer if new code is explicit about not providing a description.
974- " \"redeemScript\":\"script\" (string) The string value of the hex-encoded redemption script.\n"
975- "}\n"
976+ RPCResult::Type::OBJ, "", "",
977+ {
978+ {RPCResult::Type::STR, "address", "The value of the new multisig address"},
979+ {RPCResult::Type::STR, "redeemScript", "The string value of the hex-encoded redemption script"},
hex-econded
1221+ {RPCResult::Type::STR_AMOUNT, "amount", "The total amount in " + CURRENCY_UNIT + " received by the address"},
1222+ {RPCResult::Type::NUM, "confirmations", "The number of confirmations of the most recent transaction included"},
1223+ {RPCResult::Type::STR, "label", "The label of the receiving address. The default label is \"\""},
1224+ {RPCResult::Type::ARR, "txids", "",
1225+ {
1226+ {RPCResult::Type::STR, "txid", "The ids of transactions received with the address"},
1436+ RPCResult::Type::ARR, "", "",
1437+ {
1438+ {RPCResult::Type::OBJ, "", "", Cat(Cat<std::vector<RPCResult>>(
1439+ {
1440+ {RPCResult::Type::BOOL, "involvesWatchonly", "Only returns true if imported addresses were involved in transaction."},
1441+ {RPCResult::Type::STR, "address", "The bitcoin address of the transaction."},
STR_HEX?
1438+ {RPCResult::Type::OBJ, "", "", Cat(Cat<std::vector<RPCResult>>(
1439+ {
1440+ {RPCResult::Type::BOOL, "involvesWatchonly", "Only returns true if imported addresses were involved in transaction."},
1441+ {RPCResult::Type::STR, "address", "The bitcoin address of the transaction."},
1442+ {RPCResult::Type::STR, "category", "The transaction category.\n"
1443+ "\"send\" Transactions sent.\n"
3312+ {
3313+ {RPCResult::Type::OBJ, "", "",
3314+ {
3315+ {RPCResult::Type::STR_HEX, "txid", "The hash of the referenced, previous transaction"},
3316+ {RPCResult::Type::NUM, "vout", "The index of the output to spent and used as input"},
3317+ {RPCResult::Type::STR, "scriptSig", "The hex-encoded signature script"},
STR_HEX?
410+ RPCResult{RPCResult::Type::NUM, "size", "(DEPRECATED) same as vsize. Only returned if bitcoind is started with -deprecatedrpc=size\n"
411+ "size will be completely removed in v0.20."},
412+ RPCResult{RPCResult::Type::NUM, "weight", "transaction weight as defined in BIP 141."},
413+ RPCResult{RPCResult::Type::STR_AMOUNT, "fee", "transaction fee in " + CURRENCY_UNIT + " (DEPRECATED)"},
414+ RPCResult{RPCResult::Type::STR_AMOUNT, "modifiedfee", "transaction fee with fee deltas used for mining priority (DEPRECATED)"},
415+ RPCResult{RPCResult::Type::NUM, "time", "local time transaction entered pool in seconds since 1 Jan 1970 GMT"},
NUM_TIME?
978+ {RPCResult::Type::NUM, "transactions", "The number of transactions with unspent outputs"},
979+ {RPCResult::Type::NUM, "txouts", "The number of unspent transaction outputs"},
980+ {RPCResult::Type::NUM, "bogosize", "A meaningless metric for UTXO set size"},
981+ {RPCResult::Type::STR_HEX, "hash_serialized_2", "The serialized hash"},
982+ {RPCResult::Type::NUM, "disk_size", "The estimated size of the chainstate on disk"},
983+ {RPCResult::Type::NUM, "total_amount", "The total amount"},
NUM_AMOUNT?
1031+ {RPCResult::Type::NUM, "confirmations", "The number of confirmations"},
1032+ {RPCResult::Type::STR_AMOUNT, "value", "The transaction value in " + CURRENCY_UNIT},
1033+ {RPCResult::Type::OBJ, "scriptPubKey", "",
1034+ {
1035+ {RPCResult::Type::STR_HEX, "asm", ""},
1036+ {RPCResult::Type::STR_HEX, "hex", ""},
I want to make this change as easy to review as possible. So I am for a minimal rendered diff, assuming --ignore-all-space.
So for example, the diff for this RPC is:
0$ git diff --ignore-all-space --word-diff ./gettxout
1diff --git a/gettxout b/gettxout
2index 1d186cd..90fa947 100644
3--- a/gettxout
4+++ b/gettxout
5@@ -8,21 +8,21 @@ Arguments:
63. include_mempool (boolean, optional, default=true) Whether to include the mempool. Note that an unspent output that is spent in the mempool won't appear.
7
8Result:
9{ {+(json object)+}
10 "bestblock" : [-"hash",-]{+"hex",+} (string) The hash of the block at the tip of the chain
11 "confirmations" : n, (numeric) The number of confirmations
12 "value" : [-x.xxx,-]{+n,+} (numeric) The transaction value in BTC
13 "scriptPubKey" : { (json object)
14 "asm" : [-"code",-]{+"hex",+} (string)
15 "hex" : "hex", (string)
16 "reqSigs" : [-n, (numeric)-]{+"hex", (string)+} Number of required signatures
17 "type" : [-"pubkeyhash",-]{+"hex",+} (string) The type, eg pubkeyhash
18 "addresses" : [ [-(array of string)-]{+(json array)+} array of bitcoin addresses
19 [-"address"-]{+"str",+} (string) bitcoin address
20 [-,...-]
21[- ]-]{+...+}
22{+ ],+}
23 },
24 "coinbase" : [-true|false (boolean)-]{+"hex", (string)+} Coinbase or not
25}
26
27Examples:
1240+ {RPCResult::Type::STR, "type", "one of \"buried\", \"bip9\""},
1241+ {RPCResult::Type::OBJ, "bip9", "status of bip9 softforks (only for \"bip9\" type)",
1242+ {
1243+ {RPCResult::Type::STR, "status", "one of \"defined\", \"started\", \"locked_in\", \"active\", \"failed\""},
1244+ {RPCResult::Type::NUM, "bit", "the bit (0-28) in the block version field used to signal this softfork"},
1245+ {RPCResult::Type::NUM, "start_time", "the minimum median time past of a block at which the bit gains its meaning"},
1739+ {RPCResult::Type::NUM, "75th_percentile_feerate", "The 75th percentile feerate"},
1740+ {RPCResult::Type::NUM, "90th_percentile_feerate", "The 90th percentile feerate"},
1741+ }},
1742+ {RPCResult::Type::NUM, "height", "The height of the block"},
1743+ {RPCResult::Type::NUM, "ins", "The number of inputs (excluding coinbase)"},
1744+ {RPCResult::Type::NUM, "maxfee", "Maximum fee in the block"},
This is a heroic PR. I left some comments but I didn’t finish going through all the code. I’ll take a look again after the comments are responded to - I don’t want to provide bad comments and I’ll calibrate based on how these are received.
However, concept ACK. This is awesome.
101+ * dictionaries can be nested in json. The top-level outer type is "NONE".
102+ */
103+enum class OuterType {
104+ ARR,
105+ OBJ,
106+ NONE, // Only set on first recursion
I just ran #17804 and this PR on:
0clang version 7.0.1-8 (tags/RELEASE_701/final)
1Target: x86_64-pc-linux-gnu
2Thread model: posix
3InstalledDir: /usr/bin
The base PR worked fine, however this PR gave me more than 20 errors. I post the error log here: https://pastebin.com/G5G6fNMJ
ACK a6db5efce95262671edba832eb2f8f2c95761b3c
A handy incantation that prints all RPC docs in a row (except the hidden ones):
0src/bitcoin-cli help | grep -Eo '^[^ ]+' | grep -v "=" | while read -r line ; do `echo src/bitcoin-cli help $line`; echo "\n\n\n\n\n\n\n\n\n\n\n\n\n"; done
Put that output in before.txt and after.txt and compare using vimdiff before.txt after.txt -c 'set diffopt+=iwhite' (install via Homebrew on macOS Catalina), and enjoy pretty diff:
I did not use a toothcomb while looking through that diff; smaller mistakes can be fixed in followups, or appended commits, imo.
This is needed so that it can be used by RPCResult
Also,
* rename NAMED_ARG to NONE for generalization.
* change RPCArg constructors to initialize the members by moving values
3918- "}\n"
3919+ RPCResult::Type::OBJ_DYN, "", "json object with addresses as keys",
3920+ {
3921+ {RPCResult::Type::OBJ, "address", "json object with information about address",
3922+ {
3923+ {RPCResult::Type::STR, "purpose", "Purpose of address (\"send\" for sending address, \"receive\" for receiving address)"},
2877+ {RPCResult::Type::BOOL, "spendable", "Whether we have the private keys to spend this output"},
2878+ {RPCResult::Type::BOOL, "solvable", "Whether we know how to spend this output, ignoring the lack of keys"},
2879+ {RPCResult::Type::BOOL, "reused", "(only present if avoid_reuse is set) Whether this output is reused/dirty (sent to an address that was previously spent from)"},
2880+ {RPCResult::Type::STR, "desc", "(only when solvable) A descriptor for spending this output"},
2881+ {RPCResult::Type::BOOL, "safe", "Whether this output is considered safe to spend. Unconfirmed transactions"
2882 " from outside keys and unconfirmed replacement transactions are considered unsafe\n"
git blame. Our style-guideline is to keep the current style unless the line is changed for other reasons.
\n character? I think I did. Someone should fix that.