Merge #14411: [wallet] Restore ability to list incoming transactions by label

da427dbd48 Rename ListTransactions filter variable (Russell Yanofsky)
65b740f92b [wallet] Restore ability to list incoming transactions by label (Russell Yanofsky)

Pull request description:

  This change partially reverts #13075 and #14023.

  Fixes #14382

Tree-SHA512: 8c4e56104b3a45784cdc06bae8e5facdfff04fe3545b63a35e0ec2e440a41b79d84833ca4c4e728d8af7ebb8a519303a9eda7bee4bbfb92bd50c58587a33eb30
This commit is contained in:
MarcoFalke 2018-11-14 11:59:30 -05:00
commit e74649e951
No known key found for this signature in database
GPG key ID: D2EA4850E7528B25
3 changed files with 51 additions and 25 deletions

View file

@ -1261,25 +1261,26 @@ static void MaybePushAddress(UniValue & entry, const CTxDestination &dest)
/** /**
* List transactions based on the given criteria. * List transactions based on the given criteria.
* *
* @param pwallet The wallet. * @param pwallet The wallet.
* @param wtx The wallet transaction. * @param wtx The wallet transaction.
* @param nMinDepth The minimum confirmation depth. * @param nMinDepth The minimum confirmation depth.
* @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_ismine The "is mine" filter flags.
* @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_ismine, const std::string* filter_label)
{ {
CAmount nFee; CAmount nFee;
std::list<COutputEntry> listReceived; std::list<COutputEntry> listReceived;
std::list<COutputEntry> listSent; std::list<COutputEntry> listSent;
wtx.GetAmounts(listReceived, listSent, nFee, filter); wtx.GetAmounts(listReceived, listSent, nFee, filter_ismine);
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()