Merge #16873: rpc: fix regression in gettransaction

1b41c2c8a126ef4be183e1d800a17d85cab8837b test: improve gettransaction test coverage (Jon Atack)
0f34f54888f680bfbe7a29ac278636d7178a99bb 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 1b41c2c8a126ef4be183e1d800a17d85cab8837b

Tree-SHA512: 287edd5db7ed58fe8b548975aba58628bd45ed708b28f40174f10a35a455d89f796fbf27430aa881fc376f47aabda8803f74d4d100683bd86577a02279091cf3
This commit is contained in:
Samuel Dobson 2019-09-16 09:44:06 +12:00 committed by UdjinM6
parent 234d472944
commit e4fcb170cf
3 changed files with 40 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

@ -1655,7 +1655,7 @@ static UniValue gettransaction(const JSONRPCRequest& request)
{
{"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"},
{"verbose", RPCArg::Type::BOOL, /* default */ "false", "Whether to include a `decoded` field containing the decoded transaction (equivalent to RPC decoderawtransaction)"},
},
RPCResult{
RPCResult::Type::OBJ, "", "", Cat(Cat<std::vector<RPCResult>>(
@ -1689,9 +1689,9 @@ static UniValue gettransaction(const JSONRPCRequest& request)
}},
}},
{RPCResult::Type::STR_HEX, "hex", "Raw data for transaction"},
{RPCResult::Type::OBJ, "details", /*optional=*/true, "Additional transaction details",
{RPCResult::Type::OBJ, "decoded", /*optional=*/true, "the decoded transaction (only present when `verbose` is passed), equivalent to the",
{
{RPCResult::Type::ELISION, "", "Equivalent to the RPC decoderawtransaction method, or the RPC getrawtransaction method when `verbose` is passed."},
{RPCResult::Type::ELISION, "", "RPC decoderawtransaction method, or the RPC getrawtransaction method when `verbose` is passed."},
}},
}),
},
@ -1748,9 +1748,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

@ -496,10 +496,37 @@ 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', 'chainlock', 'confirmations', 'details', 'fee',
'instantlock', 'instantlock_internal',
'hex', 'timereceived', 'time', '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__':