Merge bitcoin/bitcoin#32517: rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response

060bb55508 rpc: add decoded tx details to gettransaction with extra wallet fields (Matthew Zipkin)
ad1c3bdba5 [move only] move DecodeTxDoc() to a common util file for sharing (Matthew Zipkin)
d633db5416 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](https://github.com/bitcoin/bitcoin/pull/32517/commits/060bb55508245776bb6a39c8b7849769ee588d69)
  maflcko:
    review ACK 060bb55508 🌛
  achow101:
    ACK 060bb55508
  rkrux:
    lgtm ACK 060bb55508

Tree-SHA512: aae4854d2bb4e9a7bc1152691ea90e594e8da8a63c9c7fda72a504fb6a7e54ae274ed5fa98d35d270e0829cc8f8d2fd35a5fc9735c252a10aa42cc22828930e7

src/core_io.h

@@ -21,6 +21,7 @@ class SigningProvider;
 class uint256;
 class UniValue;
 class CTxUndo;
+class CTxOut;
 
 /**
  * Verbose level for block's transaction
@@ -46,6 +47,6 @@ std::string FormatScript(const CScript& script);
 std::string EncodeHexTx(const CTransaction& tx);
 std::string SighashToStr(unsigned char sighash_type);
 void ScriptToUniv(const CScript& script, UniValue& out, bool include_hex = true, bool include_address = false, const SigningProvider* provider = nullptr);
-void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry, bool include_hex = true, const CTxUndo* txundo = nullptr, TxVerbosity verbosity = TxVerbosity::SHOW_DETAILS);
+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 = {});
 
 #endif // BITCOIN_CORE_IO_H

src/core_write.cpp

@@ -168,7 +168,7 @@ void ScriptToUniv(const CScript& script, UniValue& out, bool include_hex, bool i
     out.pushKV("type", GetTxnOutputType(type));
 }
 
-void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry, bool include_hex, const CTxUndo* txundo, TxVerbosity verbosity)
+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)
 {
     CHECK_NONFATAL(verbosity >= TxVerbosity::SHOW_DETAILS);
 
@@ -246,6 +246,11 @@ void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry
         UniValue o(UniValue::VOBJ);
         ScriptToUniv(txout.scriptPubKey, /*out=*/o, /*include_hex=*/true, /*include_address=*/true);
         out.pushKV("scriptPubKey", std::move(o));
+
+        if (is_change_func && is_change_func(txout)) {
+            out.pushKV("ischange", true);
+        }
+
         vout.push_back(std::move(out));
 
         if (have_undo) {

src/rpc/rawtransaction.cpp

@@ -84,47 +84,6 @@ static void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue&
     }
 }
 
-static std::vector<RPCResult> DecodeTxDoc(const std::string& txid_field_doc)
-{
-    return {
-        {RPCResult::Type::STR_HEX, "txid", txid_field_doc},
-        {RPCResult::Type::STR_HEX, "hash", "The transaction hash (differs from txid for witness transactions)"},
-        {RPCResult::Type::NUM, "size", "The serialized transaction size"},
-        {RPCResult::Type::NUM, "vsize", "The virtual transaction size (differs from size for witness transactions)"},
-        {RPCResult::Type::NUM, "weight", "The transaction's weight (between vsize*4-3 and vsize*4)"},
-        {RPCResult::Type::NUM, "version", "The version"},
-        {RPCResult::Type::NUM_TIME, "locktime", "The lock time"},
-        {RPCResult::Type::ARR, "vin", "",
-        {
-            {RPCResult::Type::OBJ, "", "",
-            {
-                {RPCResult::Type::STR_HEX, "coinbase", /*optional=*/true, "The coinbase value (only if coinbase transaction)"},
-                {RPCResult::Type::STR_HEX, "txid", /*optional=*/true, "The transaction id (if not coinbase transaction)"},
-                {RPCResult::Type::NUM, "vout", /*optional=*/true, "The output number (if not coinbase transaction)"},
-                {RPCResult::Type::OBJ, "scriptSig", /*optional=*/true, "The script (if not coinbase transaction)",
-                {
-                    {RPCResult::Type::STR, "asm", "Disassembly of the signature script"},
-                    {RPCResult::Type::STR_HEX, "hex", "The raw signature script bytes, hex-encoded"},
-                }},
-                {RPCResult::Type::ARR, "txinwitness", /*optional=*/true, "",
-                {
-                    {RPCResult::Type::STR_HEX, "hex", "hex-encoded witness data (if any)"},
-                }},
-                {RPCResult::Type::NUM, "sequence", "The script sequence number"},
-            }},
-        }},
-        {RPCResult::Type::ARR, "vout", "",
-        {
-            {RPCResult::Type::OBJ, "", "",
-            {
-                {RPCResult::Type::STR_AMOUNT, "value", "The value in " + CURRENCY_UNIT},
-                {RPCResult::Type::NUM, "n", "index"},
-                {RPCResult::Type::OBJ, "scriptPubKey", "", ScriptPubKeyDoc()},
-            }},
-        }},
-    };
-}
-
 static std::vector<RPCArg> CreateTxDoc()
 {
     return {
@@ -289,7 +248,7 @@ static RPCHelpMan getrawtransaction()
                              {RPCResult::Type::NUM, "time", /*optional=*/true, "Same as \"blocktime\""},
                              {RPCResult::Type::STR_HEX, "hex", "The serialized, hex-encoded data for 'txid'"},
                          },
-                         DecodeTxDoc(/*txid_field_doc=*/"The transaction id (same as provided)")),
+                         DecodeTxDoc(/*txid_field_doc=*/"The transaction id (same as provided)", /*wallet=*/false)),
                     },
                     RPCResult{"for verbosity = 2",
                         RPCResult::Type::OBJ, "", "",
@@ -463,7 +422,7 @@ static RPCHelpMan decoderawtransaction()
                 },
                 RPCResult{
                     RPCResult::Type::OBJ, "", "",
-                    DecodeTxDoc(/*txid_field_doc=*/"The transaction id"),
+                    DecodeTxDoc(/*txid_field_doc=*/"The transaction id", /*wallet=*/false),
                 },
                 RPCExamples{
                     HelpExampleCli("decoderawtransaction", "\"hexstring\"")

src/rpc/rawtransaction_util.cpp

@@ -343,3 +343,49 @@ void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const
         result.pushKV("errors", std::move(vErrors));
     }
 }
+
+std::vector<RPCResult> DecodeTxDoc(const std::string& txid_field_doc, bool wallet)
+{
+    return {
+        {RPCResult::Type::STR_HEX, "txid", txid_field_doc},
+        {RPCResult::Type::STR_HEX, "hash", "The transaction hash (differs from txid for witness transactions)"},
+        {RPCResult::Type::NUM, "size", "The serialized transaction size"},
+        {RPCResult::Type::NUM, "vsize", "The virtual transaction size (differs from size for witness transactions)"},
+        {RPCResult::Type::NUM, "weight", "The transaction's weight (between vsize*4-3 and vsize*4)"},
+        {RPCResult::Type::NUM, "version", "The version"},
+        {RPCResult::Type::NUM_TIME, "locktime", "The lock time"},
+        {RPCResult::Type::ARR, "vin", "",
+        {
+            {RPCResult::Type::OBJ, "", "",
+            {
+                {RPCResult::Type::STR_HEX, "coinbase", /*optional=*/true, "The coinbase value (only if coinbase transaction)"},
+                {RPCResult::Type::STR_HEX, "txid", /*optional=*/true, "The transaction id (if not coinbase transaction)"},
+                {RPCResult::Type::NUM, "vout", /*optional=*/true, "The output number (if not coinbase transaction)"},
+                {RPCResult::Type::OBJ, "scriptSig", /*optional=*/true, "The script (if not coinbase transaction)",
+                {
+                    {RPCResult::Type::STR, "asm", "Disassembly of the signature script"},
+                    {RPCResult::Type::STR_HEX, "hex", "The raw signature script bytes, hex-encoded"},
+                }},
+                {RPCResult::Type::ARR, "txinwitness", /*optional=*/true, "",
+                {
+                    {RPCResult::Type::STR_HEX, "hex", "hex-encoded witness data (if any)"},
+                }},
+                {RPCResult::Type::NUM, "sequence", "The script sequence number"},
+            }},
+        }},
+        {RPCResult::Type::ARR, "vout", "",
+        {
+            {RPCResult::Type::OBJ, "", "", Cat(
+                {
+                    {RPCResult::Type::STR_AMOUNT, "value", "The value in " + CURRENCY_UNIT},
+                    {RPCResult::Type::NUM, "n", "index"},
+                    {RPCResult::Type::OBJ, "scriptPubKey", "", ScriptPubKeyDoc()},
+                },
+                    wallet ?
+                    std::vector<RPCResult>{{RPCResult::Type::BOOL, "ischange", /*optional=*/true, "Output script is change (only present if true)"}} :
+                    std::vector<RPCResult>{}
+                )
+            },
+        }},
+    };
+}

src/rpc/rawtransaction_util.h

@@ -7,6 +7,7 @@
 
 #include <addresstype.h>
 #include <consensus/amount.h>
+#include <rpc/util.h>
 #include <map>
 #include <string>
 #include <optional>
@@ -55,4 +56,7 @@ void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in);
 /** Create a transaction from univalue parameters */
 CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime, std::optional<bool> rbf, const uint32_t version);
 
+/** Explain the UniValue "decoded" transaction object, may include extra fields if processed by wallet **/
+std::vector<RPCResult> DecodeTxDoc(const std::string& txid_field_doc, bool wallet);
+
 #endif // BITCOIN_RPC_RAWTRANSACTION_UTIL_H

src/wallet/rpc/transactions.cpp

@@ -7,6 +7,7 @@
 #include <policy/rbf.h>
 #include <primitives/transaction_identifier.h>
 #include <rpc/util.h>
+#include <rpc/rawtransaction_util.h>
 #include <rpc/blockchain.h>
 #include <util/vector.h>
 #include <wallet/receive.h>
@@ -700,7 +701,7 @@ RPCHelpMan gettransaction()
                         {RPCResult::Type::STR_HEX, "hex", "Raw data for transaction"},
                         {RPCResult::Type::OBJ, "decoded", /*optional=*/true, "The decoded transaction (only present when `verbose` is passed)",
                         {
-                            {RPCResult::Type::ELISION, "", "Equivalent to the RPC decoderawtransaction method, or the RPC getrawtransaction method when `verbose` is passed."},
+                            DecodeTxDoc(/*txid_field_doc=*/"The transaction id", /*wallet=*/true),
                         }},
                         RESULT_LAST_PROCESSED_BLOCK,
                     })
@@ -752,7 +753,16 @@ RPCHelpMan gettransaction()
 
     if (verbose) {
         UniValue decoded(UniValue::VOBJ);
-        TxToUniv(*wtx.tx, /*block_hash=*/uint256(), /*entry=*/decoded, /*include_hex=*/false);
+        TxToUniv(*wtx.tx,
+                /*block_hash=*/uint256(),
+                /*entry=*/decoded,
+                /*include_hex=*/false,
+                /*txundo=*/nullptr,
+                /*verbosity=*/TxVerbosity::SHOW_DETAILS,
+                /*is_change_func=*/[&pwallet](const CTxOut& txout) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) {
+                                        AssertLockHeld(pwallet->cs_wallet);
+                                        return OutputIsChange(*pwallet, txout);
+                                    });
         entry.pushKV("decoded", std::move(decoded));
     }
 

test/functional/wallet_basic.py

@@ -540,13 +540,16 @@ class WalletTest(BitcoinTestFramework):
         destination = self.nodes[1].getnewaddress()
         txid = self.nodes[0].sendtoaddress(destination, 0.123)
         tx = self.nodes[0].gettransaction(txid=txid, verbose=True)['decoded']
-        output_addresses = [vout['scriptPubKey']['address'] for vout in tx["vout"]]
+        assert len(tx["vout"]) > 1
-        assert len(output_addresses) > 1
+        for vout in tx["vout"]:
-        for address in output_addresses:
+            address = vout['scriptPubKey']['address']
             ischange = self.nodes[0].getaddressinfo(address)['ischange']
             assert_equal(ischange, address != destination)
             if ischange:
                 change = address
+                assert vout["ischange"]
+            else:
+                assert "ischange" not in vout
         self.nodes[0].setlabel(change, 'foobar')
         assert_equal(self.nodes[0].getaddressinfo(change)['ischange'], False)