[wallet] Restore ability to list incoming transactions by label

This change partially reverts #13075 and #14023.

Fixes #14382
This commit is contained in:
Russell Yanofsky 2018-10-06 00:48:23 -04:00
parent 8c59bb85f9
commit 65b740f92b
3 changed files with 44 additions and 18 deletions

View file

@ -1267,8 +1267,9 @@ static void MaybePushAddress(UniValue & entry, const CTxDestination &dest)
* @param fLong Whether to include the JSON version of the transaction. * @param fLong Whether to include the JSON version of the transaction.
* @param ret The UniValue into which the result is stored. * @param ret The UniValue into which the result is stored.
* @param filter The "is mine" filter bool. * @param filter The "is mine" filter bool.
* @param filter_label Optional label string to filter incoming transactions.
*/ */
static void ListTransactions(interfaces::Chain::Lock& locked_chain, CWallet* const pwallet, const CWalletTx& wtx, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter) static void ListTransactions(interfaces::Chain::Lock& locked_chain, CWallet* const pwallet, const CWalletTx& wtx, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter, const std::string* filter_label)
{ {
CAmount nFee; CAmount nFee;
std::list<COutputEntry> listReceived; std::list<COutputEntry> listReceived;
@ -1279,7 +1280,7 @@ static void ListTransactions(interfaces::Chain::Lock& locked_chain, CWallet* con
bool involvesWatchonly = wtx.IsFromMe(ISMINE_WATCH_ONLY); bool involvesWatchonly = wtx.IsFromMe(ISMINE_WATCH_ONLY);
// Sent // Sent
if ((!listSent.empty() || nFee != 0)) if (!filter_label)
{ {
for (const COutputEntry& s : listSent) for (const COutputEntry& s : listSent)
{ {
@ -1311,6 +1312,9 @@ static void ListTransactions(interfaces::Chain::Lock& locked_chain, CWallet* con
if (pwallet->mapAddressBook.count(r.destination)) { if (pwallet->mapAddressBook.count(r.destination)) {
label = pwallet->mapAddressBook[r.destination].name; label = pwallet->mapAddressBook[r.destination].name;
} }
if (filter_label && label != *filter_label) {
continue;
}
UniValue entry(UniValue::VOBJ); UniValue entry(UniValue::VOBJ);
if (involvesWatchonly || (::IsMine(*pwallet, r.destination) & ISMINE_WATCH_ONLY)) { if (involvesWatchonly || (::IsMine(*pwallet, r.destination) & ISMINE_WATCH_ONLY)) {
entry.pushKV("involvesWatchonly", true); entry.pushKV("involvesWatchonly", true);
@ -1352,10 +1356,12 @@ UniValue listtransactions(const JSONRPCRequest& request)
if (request.fHelp || request.params.size() > 4) if (request.fHelp || request.params.size() > 4)
throw std::runtime_error( throw std::runtime_error(
"listtransactions ( \"dummy\" count skip include_watchonly)\n" "listtransactions ( \"label\" count skip include_watchonly )\n"
"\nIf a label name is provided, this will return only incoming transactions paying to addresses with the specified label.\n"
"\nReturns up to 'count' most recent transactions skipping the first 'from' transactions.\n" "\nReturns up to 'count' most recent transactions skipping the first 'from' transactions.\n"
"\nArguments:\n" "\nArguments:\n"
"1. \"dummy\" (string, optional) If set, should be \"*\" for backwards compatibility.\n" "1. \"label\" (string, optional) If set, should be a valid label name to return only incoming transactions\n"
" with the specified label, or \"*\" to disable filtering and return all transactions.\n"
"2. count (numeric, optional, default=10) The number of transactions to return\n" "2. count (numeric, optional, default=10) The number of transactions to return\n"
"3. skip (numeric, optional, default=0) The number of transactions to skip\n" "3. skip (numeric, optional, default=0) The number of transactions to skip\n"
"4. include_watchonly (bool, optional, default=false) Include transactions to watch-only addresses (see 'importaddress')\n" "4. include_watchonly (bool, optional, default=false) Include transactions to watch-only addresses (see 'importaddress')\n"
@ -1400,8 +1406,12 @@ UniValue listtransactions(const JSONRPCRequest& request)
// the user could have gotten from another RPC command prior to now // the user could have gotten from another RPC command prior to now
pwallet->BlockUntilSyncedToCurrentChain(); pwallet->BlockUntilSyncedToCurrentChain();
const std::string* filter_label = nullptr;
if (!request.params[0].isNull() && request.params[0].get_str() != "*") { if (!request.params[0].isNull() && request.params[0].get_str() != "*") {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Dummy value must be set to \"*\""); filter_label = &request.params[0].get_str();
if (filter_label->empty()) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Label argument must be a valid label name or \"*\".");
}
} }
int nCount = 10; int nCount = 10;
if (!request.params[1].isNull()) if (!request.params[1].isNull())
@ -1431,7 +1441,7 @@ UniValue listtransactions(const JSONRPCRequest& request)
for (CWallet::TxItems::const_reverse_iterator it = txOrdered.rbegin(); it != txOrdered.rend(); ++it) for (CWallet::TxItems::const_reverse_iterator it = txOrdered.rbegin(); it != txOrdered.rend(); ++it)
{ {
CWalletTx *const pwtx = (*it).second; CWalletTx *const pwtx = (*it).second;
ListTransactions(*locked_chain, pwallet, *pwtx, 0, true, ret, filter); ListTransactions(*locked_chain, pwallet, *pwtx, 0, true, ret, filter, filter_label);
if ((int)ret.size() >= (nCount+nFrom)) break; if ((int)ret.size() >= (nCount+nFrom)) break;
} }
} }
@ -1568,7 +1578,7 @@ static UniValue listsinceblock(const JSONRPCRequest& request)
CWalletTx tx = pairWtx.second; CWalletTx tx = pairWtx.second;
if (depth == -1 || tx.GetDepthInMainChain(*locked_chain) < depth) { if (depth == -1 || tx.GetDepthInMainChain(*locked_chain) < depth) {
ListTransactions(*locked_chain, pwallet, tx, 0, true, transactions, filter); ListTransactions(*locked_chain, pwallet, tx, 0, true, transactions, filter, nullptr /* filter_label */);
} }
} }
@ -1585,7 +1595,7 @@ static UniValue listsinceblock(const JSONRPCRequest& request)
if (it != pwallet->mapWallet.end()) { if (it != pwallet->mapWallet.end()) {
// We want all transactions regardless of confirmation count to appear here, // We want all transactions regardless of confirmation count to appear here,
// even negative confirmation ones, hence the big negative. // even negative confirmation ones, hence the big negative.
ListTransactions(*locked_chain, pwallet, it->second, -100000000, true, removed, filter); ListTransactions(*locked_chain, pwallet, it->second, -100000000, true, removed, filter, nullptr /* filter_label */);
} }
} }
paltindex = paltindex->pprev; paltindex = paltindex->pprev;
@ -1688,7 +1698,7 @@ static UniValue gettransaction(const JSONRPCRequest& request)
WalletTxToJSON(pwallet->chain(), *locked_chain, wtx, entry); WalletTxToJSON(pwallet->chain(), *locked_chain, wtx, entry);
UniValue details(UniValue::VARR); UniValue details(UniValue::VARR);
ListTransactions(*locked_chain, pwallet, wtx, 0, false, details, filter); ListTransactions(*locked_chain, pwallet, wtx, 0, false, details, filter, nullptr /* filter_label */);
entry.pushKV("details", details); entry.pushKV("details", details);
std::string strHex = EncodeHexTx(*wtx.tx, RPCSerializationFlags()); std::string strHex = EncodeHexTx(*wtx.tx, RPCSerializationFlags());
@ -4143,7 +4153,7 @@ static const CRPCCommand commands[] =
{ "wallet", "listreceivedbyaddress", &listreceivedbyaddress, {"minconf","include_empty","include_watchonly","address_filter"} }, { "wallet", "listreceivedbyaddress", &listreceivedbyaddress, {"minconf","include_empty","include_watchonly","address_filter"} },
{ "wallet", "listreceivedbylabel", &listreceivedbylabel, {"minconf","include_empty","include_watchonly"} }, { "wallet", "listreceivedbylabel", &listreceivedbylabel, {"minconf","include_empty","include_watchonly"} },
{ "wallet", "listsinceblock", &listsinceblock, {"blockhash","target_confirmations","include_watchonly","include_removed"} }, { "wallet", "listsinceblock", &listsinceblock, {"blockhash","target_confirmations","include_watchonly","include_removed"} },
{ "wallet", "listtransactions", &listtransactions, {"dummy","count","skip","include_watchonly"} }, { "wallet", "listtransactions", &listtransactions, {"label|dummy","count","skip","include_watchonly"} },
{ "wallet", "listunspent", &listunspent, {"minconf","maxconf","addresses","include_unsafe","query_options"} }, { "wallet", "listunspent", &listunspent, {"minconf","maxconf","addresses","include_unsafe","query_options"} },
{ "wallet", "listwalletdir", &listwalletdir, {} }, { "wallet", "listwalletdir", &listwalletdir, {} },
{ "wallet", "listwallets", &listwallets, {} }, { "wallet", "listwallets", &listwallets, {} },

View file

@ -46,11 +46,11 @@ class Variant(collections.namedtuple("Variant", "call data rescan prune")):
if self.call == Call.single: if self.call == Call.single:
if self.data == Data.address: if self.data == Data.address:
response = self.try_rpc(self.node.importaddress, address=self.address["address"], rescan=rescan) response = self.try_rpc(self.node.importaddress, address=self.address["address"], label=self.label, rescan=rescan)
elif self.data == Data.pub: elif self.data == Data.pub:
response = self.try_rpc(self.node.importpubkey, pubkey=self.address["pubkey"], rescan=rescan) response = self.try_rpc(self.node.importpubkey, pubkey=self.address["pubkey"], label=self.label, rescan=rescan)
elif self.data == Data.priv: elif self.data == Data.priv:
response = self.try_rpc(self.node.importprivkey, privkey=self.key, rescan=rescan) response = self.try_rpc(self.node.importprivkey, privkey=self.key, label=self.label, rescan=rescan)
assert_equal(response, None) assert_equal(response, None)
elif self.call in (Call.multiaddress, Call.multiscript): elif self.call in (Call.multiaddress, Call.multiscript):
@ -61,18 +61,32 @@ class Variant(collections.namedtuple("Variant", "call data rescan prune")):
"timestamp": timestamp + TIMESTAMP_WINDOW + (1 if self.rescan == Rescan.late_timestamp else 0), "timestamp": timestamp + TIMESTAMP_WINDOW + (1 if self.rescan == Rescan.late_timestamp else 0),
"pubkeys": [self.address["pubkey"]] if self.data == Data.pub else [], "pubkeys": [self.address["pubkey"]] if self.data == Data.pub else [],
"keys": [self.key] if self.data == Data.priv else [], "keys": [self.key] if self.data == Data.priv else [],
"label": self.label,
"watchonly": self.data != Data.priv "watchonly": self.data != Data.priv
}], {"rescan": self.rescan in (Rescan.yes, Rescan.late_timestamp)}) }], {"rescan": self.rescan in (Rescan.yes, Rescan.late_timestamp)})
assert_equal(response, [{"success": True}]) assert_equal(response, [{"success": True}])
def check(self, txid=None, amount=None, confirmations=None): def check(self, txid=None, amount=None, confirmations=None):
"""Verify that listreceivedbyaddress returns expected values.""" """Verify that listtransactions/listreceivedbyaddress return expected values."""
txs = self.node.listtransactions(label=self.label, count=10000, include_watchonly=True)
assert_equal(len(txs), self.expected_txs)
addresses = self.node.listreceivedbyaddress(minconf=0, include_watchonly=True, address_filter=self.address['address']) addresses = self.node.listreceivedbyaddress(minconf=0, include_watchonly=True, address_filter=self.address['address'])
if self.expected_txs: if self.expected_txs:
assert_equal(len(addresses[0]["txids"]), self.expected_txs) assert_equal(len(addresses[0]["txids"]), self.expected_txs)
if txid is not None: if txid is not None:
tx, = [tx for tx in txs if tx["txid"] == txid]
assert_equal(tx["label"], self.label)
assert_equal(tx["address"], self.address["address"])
assert_equal(tx["amount"], amount)
assert_equal(tx["category"], "receive")
assert_equal(tx["label"], self.label)
assert_equal(tx["txid"], txid)
assert_equal(tx["confirmations"], confirmations)
assert_equal("trusted" not in tx, True)
address, = [ad for ad in addresses if txid in ad["txids"]] address, = [ad for ad in addresses if txid in ad["txids"]]
assert_equal(address["address"], self.address["address"]) assert_equal(address["address"], self.address["address"])
assert_equal(address["amount"], self.expected_balance) assert_equal(address["amount"], self.expected_balance)
@ -134,7 +148,8 @@ class ImportRescanTest(BitcoinTestFramework):
# Create one transaction on node 0 with a unique amount for # Create one transaction on node 0 with a unique amount for
# each possible type of wallet import RPC. # each possible type of wallet import RPC.
for i, variant in enumerate(IMPORT_VARIANTS): for i, variant in enumerate(IMPORT_VARIANTS):
variant.address = self.nodes[1].getaddressinfo(self.nodes[1].getnewaddress()) variant.label = "label {} {}".format(i, variant)
variant.address = self.nodes[1].getaddressinfo(self.nodes[1].getnewaddress(variant.label))
variant.key = self.nodes[1].dumpprivkey(variant.address["address"]) variant.key = self.nodes[1].dumpprivkey(variant.address["address"])
variant.initial_amount = 1 - (i + 1) / 64 variant.initial_amount = 1 - (i + 1) / 64
variant.initial_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.initial_amount) variant.initial_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.initial_amount)

View file

@ -97,9 +97,10 @@ class ListTransactionsTest(BitcoinTestFramework):
txid = self.nodes[1].sendtoaddress(multisig["address"], 0.1) txid = self.nodes[1].sendtoaddress(multisig["address"], 0.1)
self.nodes[1].generate(1) self.nodes[1].generate(1)
self.sync_all() self.sync_all()
assert not [tx for tx in self.nodes[0].listtransactions(dummy="*", count=100, skip=0, include_watchonly=False) if "label" in tx and tx["label"] == "watchonly"] assert len(self.nodes[0].listtransactions(label="watchonly", count=100, include_watchonly=False)) == 0
txs = [tx for tx in self.nodes[0].listtransactions(dummy="*", count=100, skip=0, include_watchonly=True) if "label" in tx and tx['label'] == 'watchonly'] assert_array_result(self.nodes[0].listtransactions(label="watchonly", count=100, include_watchonly=True),
assert_array_result(txs, {"category": "receive", "amount": Decimal("0.1")}, {"txid": txid}) {"category": "receive", "amount": Decimal("0.1")},
{"txid": txid, "label": "watchonly"})
self.run_rbf_opt_in_test() self.run_rbf_opt_in_test()