From 2f59f766d6a8341e8ab5533333e355d5f3efcb66 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sun, 4 Nov 2018 17:22:35 -0500 Subject: [PATCH] Merge #14410: rpcwallet: 'ischange' field for 'getaddressinfo' RPC 14a06525b2 tests: add test for 'getaddressinfo' RPC result 'ischange' field (whythat) 93d1aa9abc rpcwallet: add 'ischange' field to 'getaddressinfo' response (whythat) Pull request description: Implementation of proposal in #14396. This introduces `CWallet::IsChange(CScript&)` method and replaces original `CWallet::IsChange(CTxOut&)` method with overloaded version that delegates to the new method with *txout*'s `scriptPubKey`. In this way `TODO` note from the original method can still be addressed in a single place. Tree-SHA512: ef5dbc82d76b4b9b2fa6a70abc3385a677c55021f79e187ee2f392ee32bc6b406191f4129acae5c17b0206e72b6712e7e0cad574a4bbd966871c2e656c45e041 # Conflicts: # doc/release-notes-14282.md # src/wallet/rpcwallet.cpp --- src/wallet/rpcwallet.cpp | 2 ++ src/wallet/wallet.cpp | 9 +++++++-- src/wallet/wallet.h | 1 + test/functional/wallet_basic.py | 18 +++++++++++++++++- 4 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 7dd1249971..aab81c1037 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4165,6 +4165,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request) " \"ismine\" : true|false, (boolean) If the address is yours or not\n" " \"iswatchonly\" : true|false, (boolean) If the address is watchonly\n" " \"isscript\" : true|false, (boolean) If the key is a script\n" + " \"ischange\" : true|false, (boolean) If the address was used for change output\n" " \"script\" : \"type\" (string, optional) The output script type. Only if \"isscript\" is true and the redeemscript is known. Possible types: nonstandard, pubkey, pubkeyhash, scripthash, multisig, nulldata\n" " \"hex\" : \"hex\", (string, optional) The redeemscript for the p2sh address\n" " \"pubkeys\" (string, optional) Array of pubkeys associated with the known redeemscript (only if \"script\" is \"multisig\")\n" @@ -4222,6 +4223,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request) ret.pushKV("account", pwallet->mapAddressBook[dest].name); } } + ret.pushKV("ischange", pwallet->IsChange(scriptPubKey)); const CKeyMetadata* meta = nullptr; const CKeyID *key_id = boost::get(&dest); if (key_id != nullptr && !key_id->IsNull()) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e37e2ddc9a..62919f41d1 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1717,6 +1717,11 @@ CAmount CWallet::GetCredit(const CTxOut& txout, const isminefilter& filter) cons } bool CWallet::IsChange(const CTxOut& txout) const +{ + return IsChange(txout.scriptPubKey); +} + +bool CWallet::IsChange(const CScript& script) const { // TODO: fix handling of 'change' outputs. The assumption is that any // payment to a script that is ours, but is not in the address book @@ -1725,10 +1730,10 @@ bool CWallet::IsChange(const CTxOut& txout) const // a better way of identifying which outputs are 'the send' and which are // 'the change' will need to be implemented (maybe extend CWalletTx to remember // which output, if any, was change). - if (::IsMine(*this, txout.scriptPubKey)) + if (::IsMine(*this, script)) { CTxDestination address; - if (!ExtractDestination(txout.scriptPubKey, address)) + if (!ExtractDestination(script, address)) return true; LOCK(cs_wallet); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 20af192b6e..6d775d02fe 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1164,6 +1164,7 @@ public: isminetype IsMine(const CTxOut& txout) const; CAmount GetCredit(const CTxOut& txout, const isminefilter& filter) const; bool IsChange(const CTxOut& txout) const; + bool IsChange(const CScript& script) const; CAmount GetChange(const CTxOut& txout) const; bool IsMine(const CTransaction& tx) const; /** should probably be renamed to IsRelevantToMe */ diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 8f9515f97f..844c0fb5e0 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -496,7 +496,7 @@ class WalletTest(BitcoinTestFramework): # Verify nothing new in wallet assert_equal(total_txs, len(self.nodes[0].listtransactions("*", 99999))) - # Test getaddressinfo. Note that these addresses are taken from disablewallet.py + # Test getaddressinfo on external address. Note that these addresses are taken from disablewallet.py assert_raises_rpc_error(-5, "Invalid address", self.nodes[0].getaddressinfo, "3J98t1WpEZ73CNmQviecrnyiWrnqRhWNLy") address_info = self.nodes[0].getaddressinfo("yjQ5gLvGRtmq1cwc4kePLCrzQ8GVCh9Gaz") assert_equal(address_info['address'], "yjQ5gLvGRtmq1cwc4kePLCrzQ8GVCh9Gaz") @@ -504,6 +504,22 @@ class WalletTest(BitcoinTestFramework): assert not address_info["ismine"] assert not address_info["iswatchonly"] assert not address_info["isscript"] + assert not address_info["ischange"] + + # Test getaddressinfo 'ischange' field on change address. + self.nodes[0].generate(1) + destination = self.nodes[1].getnewaddress() + txid = self.nodes[0].sendtoaddress(destination, 0.123) + tx = self.nodes[0].decoderawtransaction(self.nodes[0].getrawtransaction(txid)) + output_addresses = [vout['scriptPubKey']['addresses'][0] for vout in tx["vout"]] + assert len(output_addresses) > 1 + for address in output_addresses: + ischange = self.nodes[0].getaddressinfo(address)['ischange'] + assert_equal(ischange, address != destination) + if ischange: + change = address + self.nodes[0].setlabel(change, 'foobar') + assert_equal(self.nodes[0].getaddressinfo(change)['ischange'], False) if __name__ == '__main__': WalletTest().main()