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
This commit is contained in:
MarcoFalke 2018-11-04 17:22:35 -05:00
commit b74078868b
No known key found for this signature in database
GPG key ID: D2EA4850E7528B25
5 changed files with 30 additions and 3 deletions

View file

@ -4,3 +4,6 @@ Low-level RPC changes
`-usehd` was removed in version 0.16. From that version onwards, all new `-usehd` was removed in version 0.16. From that version onwards, all new
wallets created are hierarchical deterministic wallets. Version 0.18 makes wallets created are hierarchical deterministic wallets. Version 0.18 makes
specifying `-usehd` invalid config. specifying `-usehd` invalid config.
`ischange` field of boolean type that shows if an address was used for change
output was added to `getaddressinfo` method response.

View file

@ -3458,6 +3458,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request)
" \"solvable\" : true|false, (boolean) If the address is solvable by the wallet\n" " \"solvable\" : true|false, (boolean) If the address is solvable by the wallet\n"
" \"iswatchonly\" : true|false, (boolean) If the address is watchonly\n" " \"iswatchonly\" : true|false, (boolean) If the address is watchonly\n"
" \"isscript\" : true|false, (boolean) If the key is a script\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"
" \"iswitness\" : true|false, (boolean) If the address is a witness address\n" " \"iswitness\" : true|false, (boolean) If the address is a witness address\n"
" \"witness_version\" : version (numeric, optional) The version number of the witness program\n" " \"witness_version\" : version (numeric, optional) The version number of the witness program\n"
" \"witness_program\" : \"hex\" (string, optional) The hex value of the witness program\n" " \"witness_program\" : \"hex\" (string, optional) The hex value of the witness program\n"
@ -3516,6 +3517,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request)
if (pwallet->mapAddressBook.count(dest)) { if (pwallet->mapAddressBook.count(dest)) {
ret.pushKV("label", pwallet->mapAddressBook[dest].name); ret.pushKV("label", pwallet->mapAddressBook[dest].name);
} }
ret.pushKV("ischange", pwallet->IsChange(scriptPubKey));
const CKeyMetadata* meta = nullptr; const CKeyMetadata* meta = nullptr;
CKeyID key_id = GetKeyForDestination(*pwallet, dest); CKeyID key_id = GetKeyForDestination(*pwallet, dest);
if (!key_id.IsNull()) { if (!key_id.IsNull()) {

View file

@ -1261,6 +1261,11 @@ CAmount CWallet::GetCredit(const CTxOut& txout, const isminefilter& filter) cons
} }
bool CWallet::IsChange(const CTxOut& txout) const 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 // 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 // payment to a script that is ours, but is not in the address book
@ -1269,10 +1274,10 @@ bool CWallet::IsChange(const CTxOut& txout) const
// a better way of identifying which outputs are 'the send' and which are // 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 // 'the change' will need to be implemented (maybe extend CWalletTx to remember
// which output, if any, was change). // which output, if any, was change).
if (::IsMine(*this, txout.scriptPubKey)) if (::IsMine(*this, script))
{ {
CTxDestination address; CTxDestination address;
if (!ExtractDestination(txout.scriptPubKey, address)) if (!ExtractDestination(script, address))
return true; return true;
LOCK(cs_wallet); LOCK(cs_wallet);

View file

@ -971,6 +971,7 @@ public:
isminetype IsMine(const CTxOut& txout) const; isminetype IsMine(const CTxOut& txout) const;
CAmount GetCredit(const CTxOut& txout, const isminefilter& filter) const; CAmount GetCredit(const CTxOut& txout, const isminefilter& filter) const;
bool IsChange(const CTxOut& txout) const; bool IsChange(const CTxOut& txout) const;
bool IsChange(const CScript& script) const;
CAmount GetChange(const CTxOut& txout) const; CAmount GetChange(const CTxOut& txout) const;
bool IsMine(const CTransaction& tx) const; bool IsMine(const CTransaction& tx) const;
/** should probably be renamed to IsRelevantToMe */ /** should probably be renamed to IsRelevantToMe */

View file

@ -479,7 +479,7 @@ class WalletTest(BitcoinTestFramework):
# Verify nothing new in wallet # Verify nothing new in wallet
assert_equal(total_txs, len(self.nodes[0].listtransactions("*", 99999))) 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") assert_raises_rpc_error(-5, "Invalid address", self.nodes[0].getaddressinfo, "3J98t1WpEZ73CNmQviecrnyiWrnqRhWNLy")
address_info = self.nodes[0].getaddressinfo("mneYUmWYsuk7kySiURxCi3AGxrAqZxLgPZ") address_info = self.nodes[0].getaddressinfo("mneYUmWYsuk7kySiURxCi3AGxrAqZxLgPZ")
assert_equal(address_info['address'], "mneYUmWYsuk7kySiURxCi3AGxrAqZxLgPZ") assert_equal(address_info['address'], "mneYUmWYsuk7kySiURxCi3AGxrAqZxLgPZ")
@ -487,6 +487,22 @@ class WalletTest(BitcoinTestFramework):
assert not address_info["ismine"] assert not address_info["ismine"]
assert not address_info["iswatchonly"] assert not address_info["iswatchonly"]
assert not address_info["isscript"] 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__': if __name__ == '__main__':
WalletTest().main() WalletTest().main()