Merge #16873: rpc: fix regression in gettransaction

1b41c2c8a1 test: improve gettransaction test coverage (Jon Atack)
0f34f54888 rpc: fix regression in gettransaction (Jon Atack)

Pull request description:

  Closes #16872.

  PR #16866 renamed the `decode` argument in gettransaction to `verbose` to make it more consistent with other RPC calls like getrawtransaction. However, it inadvertently overloaded the "details" field when `verbose` is passed. The result is that the original "details" field is no longer returned correctly, which seems to be a breaking API change.

  This PR:

  - takes the simplest path to restoring the "details" field by renaming the decoded one back to "decoded" while leaving the `verbose` argument for API consistency, which was the main intent of #16866,

  - addresses [this comment](https://github.com/bitcoin/bitcoin/pull/16185#discussion_r320740413) by mentioning in the RPC help that the new decoded field is equivalent to decoderawtransaction, and

  - updates the help, functional test, and release note.

  Reviewers, to test this manually, build and run `bitcoin-cli help gettransaction` and `bitcoin-cli gettransaction <wallet txid> false true`, and verify that the command returns both `details` and `decoded` fields.

ACKs for top commit:
  jnewbery:
    tACK 1b41c2c8a1

Tree-SHA512: 287edd5db7ed58fe8b548975aba58628bd45ed708b28f40174f10a35a455d89f796fbf27430aa881fc376f47aabda8803f74d4d100683bd86577a02279091cf3
This commit is contained in:
Samuel Dobson 2019-09-16 09:44:06 +12:00
commit b0a7a76c9d
No known key found for this signature in database
GPG key ID: D300116E1C875A3D
3 changed files with 41 additions and 10 deletions

View file

@ -1,3 +1,6 @@
RPC changes
-----------
The `gettransaction` RPC now accepts a third (boolean) argument `verbose`. If set to `true`, a new `details` field will be added to the response containing additional transaction details.
The `gettransaction` RPC now accepts a third (boolean) argument `verbose`. If
set to `true`, a new `decoded` field will be added to the response containing
the decoded transaction. This field is equivalent to RPC `decoderawtransaction`,
or RPC `getrawtransaction` when `verbose` is passed.

View file

@ -1648,8 +1648,10 @@ static UniValue gettransaction(const JSONRPCRequest& request)
"\nGet detailed information about in-wallet transaction <txid>\n",
{
{"txid", RPCArg::Type::STR, RPCArg::Optional::NO, "The transaction id"},
{"include_watchonly", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Whether to include watch-only addresses in balance calculation and details[]"},
{"verbose", RPCArg::Type::BOOL, /* default */ "false", "Whether to add a field with additional transaction details"},
{"include_watchonly", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false",
"Whether to include watch-only addresses in balance calculation and details[]"},
{"verbose", RPCArg::Type::BOOL, /* default */ "false",
"Whether to include a `decoded` field containing the decoded transaction (equivalent to RPC decoderawtransaction)"},
},
RPCResult{
"{\n"
@ -1685,7 +1687,8 @@ static UniValue gettransaction(const JSONRPCRequest& request)
" ,...\n"
" ],\n"
" \"hex\" : \"data\" (string) Raw data for transaction\n"
" \"details\" : transaction (json object) Optional, additional transaction details. This object contains the same transaction details as the `getrawtransaction` RPC method\n"
" \"decoded\" : transaction (json object) Optional, the decoded transaction (only present when `verbose` is passed), equivalent to the\n"
" RPC decoderawtransaction method, or the RPC getrawtransaction method when `verbose` is passed.\n"
"}\n"
},
RPCExamples{
@ -1739,9 +1742,9 @@ static UniValue gettransaction(const JSONRPCRequest& request)
entry.pushKV("hex", strHex);
if (verbose) {
UniValue details(UniValue::VOBJ);
TxToUniv(*wtx.tx, uint256(), details, false);
entry.pushKV("details", details);
UniValue decoded(UniValue::VOBJ);
TxToUniv(*wtx.tx, uint256(), decoded, false);
entry.pushKV("decoded", decoded);
}
return entry;

View file

@ -499,10 +499,35 @@ class WalletTest(BitcoinTestFramework):
self.nodes[0].setlabel(change, 'foobar')
assert_equal(self.nodes[0].getaddressinfo(change)['ischange'], False)
# Test "verbose" field value in gettransaction response
self.log.info("Testing verbose gettransaction...")
# Test gettransaction response with different arguments.
self.log.info("Testing gettransaction response with different arguments...")
self.nodes[0].setlabel(change, 'baz')
baz = self.nodes[0].listtransactions(label="baz", count=1)[0]
expected_receive_vout = {"label": "baz",
"address": baz["address"],
"amount": baz["amount"],
"category": baz["category"],
"vout": baz["vout"]}
expected_fields = frozenset({'amount', 'bip125-replaceable', 'confirmations', 'details', 'fee',
'hex', 'time', 'timereceived', 'trusted', 'txid', 'walletconflicts'})
verbose_field = "decoded"
expected_verbose_fields = expected_fields | {verbose_field}
self.log.debug("Testing gettransaction response without verbose")
tx = self.nodes[0].gettransaction(txid=txid)
assert_equal(set([*tx]), expected_fields)
assert_array_result(tx["details"], {"category": "receive"}, expected_receive_vout)
self.log.debug("Testing gettransaction response with verbose set to False")
tx = self.nodes[0].gettransaction(txid=txid, verbose=False)
assert_equal(set([*tx]), expected_fields)
assert_array_result(tx["details"], {"category": "receive"}, expected_receive_vout)
self.log.debug("Testing gettransaction response with verbose set to True")
tx = self.nodes[0].gettransaction(txid=txid, verbose=True)
assert_equal(tx["details"], self.nodes[0].decoderawtransaction(tx["hex"]))
assert_equal(set([*tx]), expected_verbose_fields)
assert_array_result(tx["details"], {"category": "receive"}, expected_receive_vout)
assert_equal(tx[verbose_field], self.nodes[0].decoderawtransaction(tx["hex"]))
if __name__ == '__main__':