Summary of Changes
Rationale
The outputs from the help command for getblocktemplate are outdated and don’t reflect the actual results from getblocktemplate (see #19625 for more details)
Fixes #19625.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
No conflicts as of last run.
ACK c91b241b48d7f97b3e6b39d84ec780f2a3e3a0a7
coinbasetxn was removed (only matches for it are in the doc and a RPC test that checks it is gone)
default_witness_commitment
517@@ -517,15 +518,15 @@ static UniValue getblocktemplate(const JSONRPCRequest& request)
518 {RPCResult::Type::NUM, "version", "The preferred block version"},
519 {RPCResult::Type::ARR, "rules", "specific block rules that are to be enforced",
520 {
521- {RPCResult::Type::STR, "", "rulename"},
522+ {RPCResult::Type::STR, "", "name of a rule the client must understand to some extend; see BIP 9 for format"},
551 {RPCResult::Type::NUM, "coinbasevalue", "maximum allowable input to coinbase transaction, including the generation award and transaction fees (in satoshis)"},
552- {RPCResult::Type::OBJ, "coinbasetxn", "information for coinbase transaction",
553- {
554- {RPCResult::Type::ELISION, "", ""},
555- }},
556+ {RPCResult::Type::STR, "longpollid", "the id of the block template longpoll"},
560@@ -563,6 +561,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request)
561 {RPCResult::Type::NUM_TIME, "curtime", "current timestamp in " + UNIX_EPOCH_TIME},
562 {RPCResult::Type::STR, "bits", "compressed target of next block"},
563 {RPCResult::Type::NUM, "height", "The height of the next block"},
564+ {RPCResult::Type::STR, "default_witness_commitment", /* optional */ true, "the default witness commitment of the block template"}
@jakeleventhal for later, you can also try this as a nice way to credit a co-author in the commit message, if you like:
https://stackoverflow.com/questions/7442112/how-to-attribute-a-single-commit-to-multiple-developers
499@@ -500,12 +500,13 @@ static UniValue getblocktemplate(const JSONRPCRequest& request)
500 {"mode", RPCArg::Type::STR, /* treat as named arg */ RPCArg::Optional::OMITTED_NAMED_ARG, "This must be set to \"template\", \"proposal\" (see BIP 23), or omitted"},
501 {"capabilities", RPCArg::Type::ARR, /* treat as named arg */ RPCArg::Optional::OMITTED_NAMED_ARG, "A list of strings",
502 {
503- {"support", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "client side supported feature, 'longpoll', 'coinbasetxn', 'coinbasevalue', 'proposal', 'serverlist', 'workid'"},
504+ {"str", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "client side supported feature, 'longpoll', 'coinbasevalue', 'proposal', 'serverlist', 'workid'"},
feature to be more expressive
git grep "RPCArg::Type::STR" | grep "\"str\"" gives me no results.
506 },
507 {"rules", RPCArg::Type::ARR, RPCArg::Optional::NO, "A list of strings",
508 {
509- {"support", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "client side supported softfork deployment"},
510+ {"segwit", RPCArg::Type::STR, RPCArg::Optional::NO, "(literal) indicates client side segwit support"},
511+ {"str", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "other client side supported softfork deployment"},
rule or softfork
utACK 1c028ff5831b5688a571bed93b008917f1c542c0
Feel free to ignore my nits but I think you should credit @luke-jr properly as indicated by @jonatack. Here is a link to the official docs: https://docs.github.com/en/github/committing-changes-to-your-project/creating-a-commit-with-multiple-authors
"str" was the type used in other places
@luke-jr reverted nits. I thought the same too - it seemed like
"str"was the type used in other places
Which other places?
0$ git grep "\"str\""
1src/univalue/test/object.cpp: std::string correctValue("str");
I seem to be missing something fundamental and I am curious what it is…
541@@ -541,15 +542,12 @@ static UniValue getblocktemplate(const JSONRPCRequest& request)
542 {RPCResult::Type::NUM, "weight", "total transaction weight, as counted for purposes of block limits"},
543 }},
544 }},
545- {RPCResult::Type::OBJ, "coinbaseaux", "data that should be included in the coinbase's scriptSig content",
546+ {RPCResult::Type::OBJ_DYN, "coinbaseaux", "data that should be included in the coinbase's scriptSig content",
547 {
548- {RPCResult::Type::ELISION, "", ""},
549+ {RPCResult::Type::STR_HEX, "str", "values must be in the coinbase (keys may be ignored)"},
Looking at other RPCResult::Type::OBJ_DYN instances, the keys are usually named "key" unless there is a better name.
0 {RPCResult::Type::STR_HEX, "key", "values must be in the coinbase (keys may be ignored)"},
Hm, I don’t really understand your comment. I didn’t say anything about changing the hex string type which is the type of the value. "key" or "str" is just the placeholder for the key.
This is generated by the current code:
0 "coinbaseaux" : { (json object) data that should be included in the coinbase's scriptSig content
1 "str" : "hex", (string) values must be in the coinbase (keys may be ignored)
2 ...
3 },
And this is with the suggested change:
0 "coinbaseaux" : { (json object) data that should be included in the coinbase's scriptSig content
1 "key" : "hex", (string) values must be in the coinbase (keys may be ignored)
2 ...
3 },
I think I have realized what you meant while reviewing another PR: "str" gets automatically inserted by RPCHelpMan in various places of the help depending on the context of a string type in args and results. For example as the value of a key-value pair with value type string, see the mode arg in this RPC as well as the rules array in the results. But "str" is never explicitly given as a name in the RPC code as you are doing here.
Still this was a nit so feel free to ignore, I was just scratching my head where the discrepancy came from.
* Removed coinbasetxn from the getblocktemplate help outputs
* Added the missing name for transactions in the help outputs
* Added getblocktemplate help outputs for longpollid and default_witness_commitment
* Added more clarity to capabilities, rules, and coinbaseaux for getblocktemplate help (credit to luke-jr)
Co-authored-by: Luke Dashjr <luke+github_public@dashjr.org>
@jakeleventhal: This isn’t the right location for general questions. Please try bitcoin-core-pr-reviews channel on IRC instead.
Jon Atack discussed the Core review process in this Stephan Livera episode. But please no more discussion here. Thanks