Skip to content

Commit

Permalink
[wallet] Restore ability to list incoming transactions by label
Browse files Browse the repository at this point in the history 
This change partially reverts bitcoin#13075 and bitcoin#14023.

Fixes bitcoin#14382
  • Loading branch information
@ryanofsky
ryanofsky committed Nov 13, 2018
1 parent 8c59bb8 commit 65b740f
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 18 deletions.
30 changes: 20 additions & 10 deletions src/wallet/rpcwallet.cpp
View file
Expand Up @@ -1267,8 +1267,9 @@ static void MaybePushAddress(UniValue & entry, const CTxDestination &dest)
* @param fLong Whether to include the JSON version of the transaction.
* @param ret The UniValue into which the result is stored.
* @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;
std::list<COutputEntry> listReceived;
Expand All @@ -1279,7 +1280,7 @@ static void ListTransactions(interfaces::Chain::Lock& locked_chain, CWallet* con
bool involvesWatchonly = wtx.IsFromMe(ISMINE_WATCH_ONLY);

// Sent
if ((!listSent.empty() || nFee != 0))
if (!filter_label)
{
for (const COutputEntry& s : listSent)
{
Expand Down Expand Up @@ -1311,6 +1312,9 @@ static void ListTransactions(interfaces::Chain::Lock& locked_chain, CWallet* con
if (pwallet->mapAddressBook.count(r.destination)) {
label = pwallet->mapAddressBook[r.destination].name;
}
if (filter_label && label != *filter_label) {
continue;
}
UniValue entry(UniValue::VOBJ);
if (involvesWatchonly || (::IsMine(*pwallet, r.destination) & ISMINE_WATCH_ONLY)) {
entry.pushKV("involvesWatchonly", true);
Expand Down Expand Up @@ -1352,10 +1356,12 @@ UniValue listtransactions(const JSONRPCRequest& request)

if (request.fHelp || request.params.size() > 4)
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"
"\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"
"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"
Expand Down Expand Up @@ -1400,8 +1406,12 @@ UniValue listtransactions(const JSONRPCRequest& request)
// the user could have gotten from another RPC command prior to now
pwallet->BlockUntilSyncedToCurrentChain();

const std::string* filter_label = nullptr;
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;
if (!request.params[1].isNull())
Expand Down Expand Up @@ -1431,7 +1441,7 @@ UniValue listtransactions(const JSONRPCRequest& request)
for (CWallet::TxItems::const_reverse_iterator it = txOrdered.rbegin(); it != txOrdered.rend(); ++it)
{
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;
}
}
Expand Down Expand Up @@ -1568,7 +1578,7 @@ static UniValue listsinceblock(const JSONRPCRequest& request)
CWalletTx tx = pairWtx.second;

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 */);
}
}

Expand All @@ -1585,7 +1595,7 @@ static UniValue listsinceblock(const JSONRPCRequest& request)
if (it != pwallet->mapWallet.end()) {
// We want all transactions regardless of confirmation count to appear here,
// 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;
Expand Down Expand Up @@ -1688,7 +1698,7 @@ static UniValue gettransaction(const JSONRPCRequest& request)
WalletTxToJSON(pwallet->chain(), *locked_chain, wtx, entry);

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);

std::string strHex = EncodeHexTx(*wtx.tx, RPCSerializationFlags());
Expand Down Expand Up @@ -4143,7 +4153,7 @@ static const CRPCCommand commands[] =
{ "wallet", "listreceivedbyaddress", &listreceivedbyaddress, {"minconf","include_empty","include_watchonly","address_filter"} },
{ "wallet", "listreceivedbylabel", &listreceivedbylabel, {"minconf","include_empty","include_watchonly"} },
{ "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", "listwalletdir", &listwalletdir, {} },
{ "wallet", "listwallets", &listwallets, {} },
Expand Down
25 changes: 20 additions & 5 deletions test/functional/wallet_import_rescan.py
View file
Expand Up @@ -46,11 +46,11 @@ def do_import(self, timestamp):

if self.call == Call.single:
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:
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:
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)

elif self.call in (Call.multiaddress, Call.multiscript):
Expand All @@ -61,18 +61,32 @@ def do_import(self, timestamp):
"timestamp": timestamp + TIMESTAMP_WINDOW + (1 if self.rescan == Rescan.late_timestamp else 0),
"pubkeys": [self.address["pubkey"]] if self.data == Data.pub else [],
"keys": [self.key] if self.data == Data.priv else [],
"label": self.label,
"watchonly": self.data != Data.priv
}], {"rescan": self.rescan in (Rescan.yes, Rescan.late_timestamp)})
assert_equal(response, [{"success": True}])

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'])
if self.expected_txs:
assert_equal(len(addresses[0]["txids"]), self.expected_txs)

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"]]
assert_equal(address["address"], self.address["address"])
assert_equal(address["amount"], self.expected_balance)
Expand Down Expand Up @@ -134,7 +148,8 @@ def run_test(self):
# Create one transaction on node 0 with a unique amount for
# each possible type of wallet import RPC.
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.initial_amount = 1 - (i + 1) / 64
variant.initial_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.initial_amount)
Expand Down
7 changes: 4 additions & 3 deletions test/functional/wallet_listtransactions.py
View file
Expand Up @@ -97,9 +97,10 @@ def run_test(self):
txid = self.nodes[1].sendtoaddress(multisig["address"], 0.1)
self.nodes[1].generate(1)
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"]
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(txs, {"category": "receive", "amount": Decimal("0.1")}, {"txid": txid})
assert len(self.nodes[0].listtransactions(label="watchonly", count=100, include_watchonly=False)) == 0
assert_array_result(self.nodes[0].listtransactions(label="watchonly", count=100, include_watchonly=True),
{"category": "receive", "amount": Decimal("0.1")},
{"txid": txid, "label": "watchonly"})

self.run_rbf_opt_in_test()

Expand Down

0 comments on commit 65b740f

Please sign in to comment.