Skip to content

Commit 8c2710b

Browse files
achow101achow101
committed
Merge bitcoin/bitcoin#32517: rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response
060bb55 rpc: add decoded tx details to gettransaction with extra wallet fields (Matthew Zipkin) ad1c3bd [move only] move DecodeTxDoc() to a common util file for sharing (Matthew Zipkin) d633db5 rpc: add "ischange: true" in wallet gettransaction decoded tx output (Matthew Zipkin) Pull request description: This change is motivated by external RBF clients like https://github.com/CardCoins/additive-rbf-batcher/. It saves the user a redundant re-looping of tx outputs, calling `getaddressinfo` on each one, looking for the change output in order to adjust the fee. The field `"ischange"` only appears when `gettransaction` is called on a wallet, and is either `true` or not present at all. I chose not to include `ischange: false` because it is confusing to see that on *received* transactions. Example of the new field: ``` "vout": [ { "value": 1.00000000, "n": 0, "scriptPubKey": { "asm": "0 5483235e05c76273b3b50af62519738781aff021", "desc": "addr(bcrt1q2jpjxhs9ca388va4ptmz2xtns7q6lupppkw7wu)#d42g84j6", "hex": "00145483235e05c76273b3b50af62519738781aff021", "address": "bcrt1q2jpjxhs9ca388va4ptmz2xtns7q6lupppkw7wu", "type": "witness_v0_keyhash" } }, { "value": 198.99859000, "n": 1, "scriptPubKey": { "asm": "0 870ab1ab58632b05a417d5295f4038500e407592", "desc": "addr(bcrt1qsu9tr26cvv4stfqh65547spc2q8yqavj7fnlju)#tgapemkv", "hex": "0014870ab1ab58632b05a417d5295f4038500e407592", "address": "bcrt1qsu9tr26cvv4stfqh65547spc2q8yqavj7fnlju", "type": "witness_v0_keyhash" }, "ischange": true } ] ``` ACKs for top commit: furszy: ACK [060bb55](bitcoin/bitcoin@060bb55) maflcko: review ACK 060bb55 🌛 achow101: ACK 060bb55 rkrux: lgtm ACK 060bb55 Tree-SHA512: aae4854d2bb4e9a7bc1152691ea90e594e8da8a63c9c7fda72a504fb6a7e54ae274ed5fa98d35d270e0829cc8f8d2fd35a5fc9735c252a10aa42cc22828930e7
2 parents 1fe851a + 060bb55 commit 8c2710b

File tree

7 files changed

+78
-50
lines changed

7 files changed

+78
-50
lines changed

‎src/core_io.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ class SigningProvider;
2121
class uint256;
2222
class UniValue;
2323
class CTxUndo;
24+
class CTxOut;
2425

2526
/**
2627
* Verbose level for block's transaction
@@ -46,6 +47,6 @@ std::string FormatScript(const CScript& script);
4647
std::string EncodeHexTx(const CTransaction& tx);
4748
std::string SighashToStr(unsigned char sighash_type);
4849
void ScriptToUniv(const CScript& script, UniValue& out, bool include_hex = true, bool include_address = false, const SigningProvider* provider = nullptr);
49-
void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry, bool include_hex = true, const CTxUndo* txundo = nullptr, TxVerbosity verbosity = TxVerbosity::SHOW_DETAILS);
50+
void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry, bool include_hex = true, const CTxUndo* txundo = nullptr, TxVerbosity verbosity = TxVerbosity::SHOW_DETAILS, std::function<bool(const CTxOut&)> is_change_func = {});
5051

5152
#endif // BITCOIN_CORE_IO_H

‎src/core_write.cpp‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ void ScriptToUniv(const CScript& script, UniValue& out, bool include_hex, bool i
168168
out.pushKV("type", GetTxnOutputType(type));
169169
}
170170

171-
void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry, bool include_hex, const CTxUndo* txundo, TxVerbosity verbosity)
171+
void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry, bool include_hex, const CTxUndo* txundo, TxVerbosity verbosity, std::function<bool(const CTxOut&)> is_change_func)
172172
{
173173
CHECK_NONFATAL(verbosity >= TxVerbosity::SHOW_DETAILS);
174174

@@ -246,6 +246,11 @@ void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry
246246
UniValue o(UniValue::VOBJ);
247247
ScriptToUniv(txout.scriptPubKey, /*out=*/o, /*include_hex=*/true, /*include_address=*/true);
248248
out.pushKV("scriptPubKey", std::move(o));
249+
250+
if (is_change_func && is_change_func(txout)) {
251+
out.pushKV("ischange", true);
252+
}
253+
249254
vout.push_back(std::move(out));
250255

251256
if (have_undo) {

‎src/rpc/rawtransaction.cpp‎

Lines changed: 2 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -84,47 +84,6 @@ static void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue&
8484
}
8585
}
8686

87-
static std::vector<RPCResult> DecodeTxDoc(const std::string& txid_field_doc)
88-
{
89-
return {
90-
{RPCResult::Type::STR_HEX, "txid", txid_field_doc},
91-
{RPCResult::Type::STR_HEX, "hash", "The transaction hash (differs from txid for witness transactions)"},
92-
{RPCResult::Type::NUM, "size", "The serialized transaction size"},
93-
{RPCResult::Type::NUM, "vsize", "The virtual transaction size (differs from size for witness transactions)"},
94-
{RPCResult::Type::NUM, "weight", "The transaction's weight (between vsize*4-3 and vsize*4)"},
95-
{RPCResult::Type::NUM, "version", "The version"},
96-
{RPCResult::Type::NUM_TIME, "locktime", "The lock time"},
97-
{RPCResult::Type::ARR, "vin", "",
98-
{
99-
{RPCResult::Type::OBJ, "", "",
100-
{
101-
{RPCResult::Type::STR_HEX, "coinbase", /*optional=*/true, "The coinbase value (only if coinbase transaction)"},
102-
{RPCResult::Type::STR_HEX, "txid", /*optional=*/true, "The transaction id (if not coinbase transaction)"},
103-
{RPCResult::Type::NUM, "vout", /*optional=*/true, "The output number (if not coinbase transaction)"},
104-
{RPCResult::Type::OBJ, "scriptSig", /*optional=*/true, "The script (if not coinbase transaction)",
105-
{
106-
{RPCResult::Type::STR, "asm", "Disassembly of the signature script"},
107-
{RPCResult::Type::STR_HEX, "hex", "The raw signature script bytes, hex-encoded"},
108-
}},
109-
{RPCResult::Type::ARR, "txinwitness", /*optional=*/true, "",
110-
{
111-
{RPCResult::Type::STR_HEX, "hex", "hex-encoded witness data (if any)"},
112-
}},
113-
{RPCResult::Type::NUM, "sequence", "The script sequence number"},
114-
}},
115-
}},
116-
{RPCResult::Type::ARR, "vout", "",
117-
{
118-
{RPCResult::Type::OBJ, "", "",
119-
{
120-
{RPCResult::Type::STR_AMOUNT, "value", "The value in " + CURRENCY_UNIT},
121-
{RPCResult::Type::NUM, "n", "index"},
122-
{RPCResult::Type::OBJ, "scriptPubKey", "", ScriptPubKeyDoc()},
123-
}},
124-
}},
125-
};
126-
}
127-
12887
static std::vector<RPCArg> CreateTxDoc()
12988
{
13089
return {
@@ -289,7 +248,7 @@ static RPCHelpMan getrawtransaction()
289248
{RPCResult::Type::NUM, "time", /*optional=*/true, "Same as \"blocktime\""},
290249
{RPCResult::Type::STR_HEX, "hex", "The serialized, hex-encoded data for 'txid'"},
291250
},
292-
DecodeTxDoc(/*txid_field_doc=*/"The transaction id (same as provided)")),
251+
DecodeTxDoc(/*txid_field_doc=*/"The transaction id (same as provided)", /*wallet=*/false)),
293252
},
294253
RPCResult{"for verbosity = 2",
295254
RPCResult::Type::OBJ, "", "",
@@ -463,7 +422,7 @@ static RPCHelpMan decoderawtransaction()
463422
},
464423
RPCResult{
465424
RPCResult::Type::OBJ, "", "",
466-
DecodeTxDoc(/*txid_field_doc=*/"The transaction id"),
425+
DecodeTxDoc(/*txid_field_doc=*/"The transaction id", /*wallet=*/false),
467426
},
468427
RPCExamples{
469428
HelpExampleCli("decoderawtransaction", "\"hexstring\"")

‎src/rpc/rawtransaction_util.cpp‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,3 +343,49 @@ void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const
343343
result.pushKV("errors", std::move(vErrors));
344344
}
345345
}
346+
347+
std::vector<RPCResult> DecodeTxDoc(const std::string& txid_field_doc, bool wallet)
348+
{
349+
return {
350+
{RPCResult::Type::STR_HEX, "txid", txid_field_doc},
351+
{RPCResult::Type::STR_HEX, "hash", "The transaction hash (differs from txid for witness transactions)"},
352+
{RPCResult::Type::NUM, "size", "The serialized transaction size"},
353+
{RPCResult::Type::NUM, "vsize", "The virtual transaction size (differs from size for witness transactions)"},
354+
{RPCResult::Type::NUM, "weight", "The transaction's weight (between vsize*4-3 and vsize*4)"},
355+
{RPCResult::Type::NUM, "version", "The version"},
356+
{RPCResult::Type::NUM_TIME, "locktime", "The lock time"},
357+
{RPCResult::Type::ARR, "vin", "",
358+
{
359+
{RPCResult::Type::OBJ, "", "",
360+
{
361+
{RPCResult::Type::STR_HEX, "coinbase", /*optional=*/true, "The coinbase value (only if coinbase transaction)"},
362+
{RPCResult::Type::STR_HEX, "txid", /*optional=*/true, "The transaction id (if not coinbase transaction)"},
363+
{RPCResult::Type::NUM, "vout", /*optional=*/true, "The output number (if not coinbase transaction)"},
364+
{RPCResult::Type::OBJ, "scriptSig", /*optional=*/true, "The script (if not coinbase transaction)",
365+
{
366+
{RPCResult::Type::STR, "asm", "Disassembly of the signature script"},
367+
{RPCResult::Type::STR_HEX, "hex", "The raw signature script bytes, hex-encoded"},
368+
}},
369+
{RPCResult::Type::ARR, "txinwitness", /*optional=*/true, "",
370+
{
371+
{RPCResult::Type::STR_HEX, "hex", "hex-encoded witness data (if any)"},
372+
}},
373+
{RPCResult::Type::NUM, "sequence", "The script sequence number"},
374+
}},
375+
}},
376+
{RPCResult::Type::ARR, "vout", "",
377+
{
378+
{RPCResult::Type::OBJ, "", "", Cat(
379+
{
380+
{RPCResult::Type::STR_AMOUNT, "value", "The value in " + CURRENCY_UNIT},
381+
{RPCResult::Type::NUM, "n", "index"},
382+
{RPCResult::Type::OBJ, "scriptPubKey", "", ScriptPubKeyDoc()},
383+
},
384+
wallet ?
385+
std::vector<RPCResult>{{RPCResult::Type::BOOL, "ischange", /*optional=*/true, "Output script is change (only present if true)"}} :
386+
std::vector<RPCResult>{}
387+
)
388+
},
389+
}},
390+
};
391+
}

‎src/rpc/rawtransaction_util.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#include <addresstype.h>
99
#include <consensus/amount.h>
10+
#include <rpc/util.h>
1011
#include <map>
1112
#include <string>
1213
#include <optional>
@@ -55,4 +56,7 @@ void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in);
5556
/** Create a transaction from univalue parameters */
5657
CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime, std::optional<bool> rbf, const uint32_t version);
5758

59+
/** Explain the UniValue "decoded" transaction object, may include extra fields if processed by wallet **/
60+
std::vector<RPCResult> DecodeTxDoc(const std::string& txid_field_doc, bool wallet);
61+
5862
#endif // BITCOIN_RPC_RAWTRANSACTION_UTIL_H

‎src/wallet/rpc/transactions.cpp‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include <policy/rbf.h>
88
#include <primitives/transaction_identifier.h>
99
#include <rpc/util.h>
10+
#include <rpc/rawtransaction_util.h>
1011
#include <rpc/blockchain.h>
1112
#include <util/vector.h>
1213
#include <wallet/receive.h>
@@ -700,7 +701,7 @@ RPCHelpMan gettransaction()
700701
{RPCResult::Type::STR_HEX, "hex", "Raw data for transaction"},
701702
{RPCResult::Type::OBJ, "decoded", /*optional=*/true, "The decoded transaction (only present when `verbose` is passed)",
702703
{
703-
{RPCResult::Type::ELISION, "", "Equivalent to the RPC decoderawtransaction method, or the RPC getrawtransaction method when `verbose` is passed."},
704+
DecodeTxDoc(/*txid_field_doc=*/"The transaction id", /*wallet=*/true),
704705
}},
705706
RESULT_LAST_PROCESSED_BLOCK,
706707
})
@@ -752,7 +753,16 @@ RPCHelpMan gettransaction()
752753

753754
if (verbose) {
754755
UniValue decoded(UniValue::VOBJ);
755-
TxToUniv(*wtx.tx, /*block_hash=*/uint256(), /*entry=*/decoded, /*include_hex=*/false);
756+
TxToUniv(*wtx.tx,
757+
/*block_hash=*/uint256(),
758+
/*entry=*/decoded,
759+
/*include_hex=*/false,
760+
/*txundo=*/nullptr,
761+
/*verbosity=*/TxVerbosity::SHOW_DETAILS,
762+
/*is_change_func=*/[&pwallet](const CTxOut& txout) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) {
763+
AssertLockHeld(pwallet->cs_wallet);
764+
return OutputIsChange(*pwallet, txout);
765+
});
756766
entry.pushKV("decoded", std::move(decoded));
757767
}
758768

‎test/functional/wallet_basic.py‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -540,13 +540,16 @@ def run_test(self):
540540
destination = self.nodes[1].getnewaddress()
541541
txid = self.nodes[0].sendtoaddress(destination, 0.123)
542542
tx = self.nodes[0].gettransaction(txid=txid, verbose=True)['decoded']
543-
output_addresses = [vout['scriptPubKey']['address'] for vout in tx["vout"]]
544-
assert len(output_addresses) > 1
545-
for address in output_addresses:
543+
assert len(tx["vout"]) > 1
544+
for vout in tx["vout"]:
545+
address = vout['scriptPubKey']['address']
546546
ischange = self.nodes[0].getaddressinfo(address)['ischange']
547547
assert_equal(ischange, address != destination)
548548
if ischange:
549549
change = address
550+
assert vout["ischange"]
551+
else:
552+
assert "ischange" not in vout
550553
self.nodes[0].setlabel(change, 'foobar')
551554
assert_equal(self.nodes[0].getaddressinfo(change)['ischange'], False)
552555

0 commit comments

Comments
 (0)