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

pinheadmz commented at 3:40 PM on May 15, 2025: member

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
      }
    ]

in src/rpc/rawtransaction.cpp:121 in bef143cc28 outdated

 117 | @@ -118,6 +118,7 @@ static std::vector<RPCResult> DecodeTxDoc(const std::string& txid_field_doc)
 118 |                  {RPCResult::Type::STR_AMOUNT, "value", "The value in " + CURRENCY_UNIT},
 119 |                  {RPCResult::Type::NUM, "n", "index"},
 120 |                  {RPCResult::Type::OBJ, "scriptPubKey", "", ScriptPubKeyDoc()},
 121 | +                {RPCResult::Type::BOOL, "ischange", /*optional=*/true, "Output script is change (only if wallet transaction and true for selected rpcwallet)"},

maflcko commented at 4:12 PM on May 15, 2025:

nit: Not sure about making this optional out of convenience and then write the rule in the description string.

It would be better to just pass down a std::optionl<RPCResult> (or std::vector<RPCResult>) here, and then use Cat() to concatenate it?

pinheadmz commented at 4:39 PM on May 15, 2025:

Not sure what you mean here -- help gettransaction doesn't list this at all, it just tells the user to check decoderawtransaction

maflcko commented at 5:04 PM on May 15, 2025:

Hmm, looks like you are right. Though, that doesn't seem very machine readable. It is a bit unrelated, but I can try to fix it, if you don't want to include it here.

pinheadmz commented at 5:43 PM on May 15, 2025:

Ill give it a shot in this branch

pinheadmz commented at 2:16 PM on May 16, 2025:

done

maflcko commented at 11:36 AM on November 3, 2025:

Nice. Thanks for taking the suggestion.

in src/core_write.cpp:248 in bef143cc28 outdated

 242 | @@ -243,6 +243,11 @@ void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry
 243 |          UniValue o(UniValue::VOBJ);
 244 |          ScriptToUniv(txout.scriptPubKey, /*out=*/o, /*include_hex=*/true, /*include_address=*/true);
 245 |          out.pushKV("scriptPubKey", std::move(o));
 246 | +
 247 | +        if (is_change_func && is_change_func(txout)) {
 248 | +            out.pushKV("ischange", true);

maflcko commented at 4:13 PM on May 15, 2025:

        if (is_change_func  ) {
            out.pushKV("ischange", is_change_func(txout));

nit: Seems more user friendly to not make the field optional and instead return a boolean?

pinheadmz commented at 4:34 PM on May 15, 2025:

I started with this but then playing with the feature I made it optional because if you receive a payment you'd see a tx with a bunch of outputs that all say ischange: false. I felt like that was confusing because you didn't create this tx and that might imply that there is some way to detect a change output in someone else's tx.

achow101 commented at 10:47 PM on May 28, 2025: member

ACK f6517df210f5e940d87823c86358976743de2641

pinheadmz requested review from maflcko on May 28, 2025

in src/core_io.h:50 in bef143cc28 outdated

  46 | @@ -46,6 +47,6 @@ std::string FormatScript(const CScript& script);
  47 |  std::string EncodeHexTx(const CTransaction& tx);
  48 |  std::string SighashToStr(unsigned char sighash_type);
  49 |  void ScriptToUniv(const CScript& script, UniValue& out, bool include_hex = true, bool include_address = false, const SigningProvider* provider = nullptr);
  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);
  51 | +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(CTxOut)> is_change_func = {});

furszy commented at 3:25 PM on October 10, 2025:

Should be a const ref:

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 = {});

pinheadmz commented at 3:14 PM on October 15, 2025:

good catch thanks I'll update this arg with const here and in the lambda

furszy commented at 3:18 PM on October 15, 2025:

only const? why not passing the ref too. Otherwise you are copying the object.

pinheadmz commented at 3:22 PM on October 15, 2025:

oops thanks, updating...

furszy commented at 3:27 PM on October 10, 2025: member

utACK f6517df210f5e940d87823c86358976743de2641

left a small comment that would be nice to fix.

pinheadmz force-pushed on Oct 15, 2025

in src/wallet/rpc/transactions.cpp:805 in 9059a71e97 outdated

 801 | +                /*block_hash=*/uint256(),
 802 | +                /*entry=*/decoded,
 803 | +                /*include_hex=*/false,
 804 | +                /*txundo=*/nullptr,
 805 | +                /*verbosity=*/TxVerbosity::SHOW_DETAILS,
 806 | +                /*is_change_func=*/[&pwallet](const CTxOut txout) NO_THREAD_SAFETY_ANALYSIS {

furszy commented at 2:26 PM on October 29, 2025:

nit: Missing ref here const CTxOut& txout

                /*is_change_func=*/[&pwallet](const CTxOut& txout) NO_THREAD_SAFETY_ANALYSIS {

Also, why not EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) instead of NO_THREAD_SAFETY_ANALYSIS?

pinheadmz commented at 4:12 PM on October 29, 2025:

Thanks, taken

furszy commented at 2:27 PM on October 29, 2025: member

ACK ab06c3f16063701cfa25504a0d3cd4d5a5eb3a97

rpc: add "ischange: true" in wallet gettransaction decoded tx output d633db5416

[move only] move DecodeTxDoc() to a common util file for sharing ad1c3bdba5

rpc: add decoded tx details to gettransaction with extra wallet fields 060bb55508

pinheadmz force-pushed on Oct 29, 2025

furszy commented at 5:19 PM on October 29, 2025: member

ACK 060bb55

in src/core_io.h:50 in d633db5416 outdated

  46 | @@ -46,6 +47,6 @@ std::string FormatScript(const CScript& script);
  47 |  std::string EncodeHexTx(const CTransaction& tx);
  48 |  std::string SighashToStr(unsigned char sighash_type);
  49 |  void ScriptToUniv(const CScript& script, UniValue& out, bool include_hex = true, bool include_address = false, const SigningProvider* provider = nullptr);
  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);
  51 | +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 = {});

rkrux commented at 10:34 AM on November 3, 2025:

In d633db54166497685b80a12c51db6772982e01fe "rpc: add "ischange: true" in wallet gettransaction decoded tx output"

Nit: #IWYU

diff --git a/src/core_io.h b/src/core_io.h
index 1874c93a05..57ae6a2af7 100644
--- a/src/core_io.h
+++ b/src/core_io.h
@@ -11,6 +11,7 @@
 #include <string>
 #include <vector>
 #include <optional>
+#include <functional>
 
 class CBlock;
 class CBlockHeader;

rkrux commented at 10:35 AM on November 3, 2025:

In d633db54166497685b80a12c51db6772982e01fe "rpc: add "ischange: true" in wallet gettransaction decoded tx output"

Nit: s/is_change_func/is_output_change_func

@@ -47,6 +48,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, std::function<bool(const CTxOut&)> is_change_func = {});
+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_output_change_func = {});
 
 #endif // BITCOIN_CORE_IO_H

in src/rpc/rawtransaction_util.cpp:376 in 060bb55508

 378 | +                    {RPCResult::Type::STR_AMOUNT, "value", "The value in " + CURRENCY_UNIT},
 379 | +                    {RPCResult::Type::NUM, "n", "index"},
 380 | +                    {RPCResult::Type::OBJ, "scriptPubKey", "", ScriptPubKeyDoc()},
 381 | +                },
 382 | +                    wallet ?
 383 | +                    std::vector<RPCResult>{{RPCResult::Type::BOOL, "ischange", /*optional=*/true, "Output script is change (only present if true)"}} :

rkrux commented at 10:51 AM on November 3, 2025:

In 060bb55508245776bb6a39c8b7849769ee588d69 "rpc: add decoded tx details to gettransaction with extra wallet fields"

Nit: Can do away with the word script

@@ -373,7 +373,7 @@ std::vector<RPCResult> DecodeTxDoc(const std::string& txid_field_doc, bool walle
                     {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>{{RPCResult::Type::BOOL, "ischange", /*optional=*/true, "Output is change (only present if true)"}} :
                     std::vector<RPCResult>{}
                 )
             },

rkrux approved

rkrux commented at 10:53 AM on November 3, 2025: contributor

lgtm ACK 060bb55508245776bb6a39c8b7849769ee588d69

Only minor nits if retouched.

maflcko commented at 11:35 AM on November 3, 2025: member

Sorry for missing the review request earlier.

review ACK 060bb55508245776bb6a39c8b7849769ee588d69 🌛

<details><summary>Show signature</summary>

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 060bb55508245776bb6a39c8b7849769ee588d69 🌛
oFaD8aXwgNJESDCDkPUK36fans9bFYO48Z+kJQ23jaQZBYjSlHReFMS0DTWGUZ53/9KXGQYvRo3vNbWJscPFAA==

</details>

achow101 commented at 4:50 PM on November 10, 2025: member

ACK 060bb55508245776bb6a39c8b7849769ee588d69

achow101 merged this on Nov 10, 2025

achow101 closed this on Nov 10, 2025

in test/functional/wallet_basic.py:552 in 060bb55508

 550 | +            address = vout['scriptPubKey']['address']
 551 |              ischange = self.nodes[0].getaddressinfo(address)['ischange']
 552 |              assert_equal(ischange, address != destination)
 553 |              if ischange:
 554 |                  change = address
 555 | +                assert vout["ischange"]

maflcko commented at 9:21 AM on November 11, 2025:

https://github.com/bitcoin/bitcoin/actions/runs/19238750700/job/54998849583?pr=33842#step:12:208:

  File "D:\a\bitcoin\bitcoin/test/functional/wallet_basic.py", line 550, in run_test

    assert vout["ischange"]

           ~~~~^^^^^^^^^^^^

KeyError: 'ischange'