listreceivedbyaddress Filter Address #9991

NicolasDorier · 2017-03-14T13:23:34Z

Supersede #9503 created by @JeremyRubin , I will maintain it.

NicolasDorier · 2017-03-14T16:56:53Z

Rebased and fixed conflicts
Addressed @kallewoof nits
Add the new parameter to client.cpp

kallewoof

utACK 8c373d5feedb428d234278d484e5d2a932010fae

NicolasDorier · 2017-03-16T05:18:39Z

@luke-jr @jnewbery @JeremyRubin nit addressed, can you re-ACK?

jnewbery · 2017-03-16T13:43:20Z

qa/rpc-tests/receivedby.py

+        #Test Address filtering
+        #Only on addr
+        expected = {"address":addr, "account":"", "amount":Decimal("0.1"), "confirmations":10, "txids":[txid,]}
+        res = self.nodes[1].listreceivedbyaddress(0, True, True, addr)


Nit: Consider using named arguments here, instead of positional arguments (since the meaning of the arguments is not clear without having to go back to the definition of listreceivedbyaddress).

Listreceivedbyaddress is not known at compile time, and thus has no definition in python. This is interpreted at runtime. So I can't really use named argument. ~~Would inline comment be good ?~~

EDIT: No mid line comments in python

You can use named arguments in the RPC calls. See #9983 for example.

jnewbery · 2017-03-16T13:52:10Z

qa/rpc-tests/receivedby.py

+                           {"address":empty_addr},
+                           {"address":empty_addr, "account":"", "amount":0, "confirmations":0, "txids":[]})
+
+        #Test Address filtering


I'd prefer this test if other_addr (currently declared on line 78) received some funds before you did your tests on listreceivedbyaddress() with a filter address. This isn't currently testing the case where the wallet contains multiple addresses with funds and you want to see the transactions received by one of those addresses. Ideally the test case would be:

define addr and send funds to it (already done for you in lines 42-43)

define other_addr and send funds to it

call listreceivedbyaddress() filtering on addr. Assert only one transaction is returned.

call listreceivedbyaddress() filtering on other_addr. Assert only one transaction is returned.

call listreceivedbyaddress() with no filtering. Assert both transactions are returned.

jnewbery · 2017-03-16T13:53:22Z

src/wallet/rpcwallet.cpp

            "\nList balances by receiving address.\n"
            "\nArguments:\n"
            "1. minconf           (numeric, optional, default=1) The minimum number of confirmations before payments are included.\n"
            "2. include_empty     (bool, optional, default=false) Whether to include addresses that haven't received any payments.\n"
            "3. include_watchonly (bool, optional, default=false) Whether to include watch-only addresses (see 'importaddress').\n"
-
+            "4. only_address   (string, optional) If present, only return information on this address. Otherwise, return all information.\n"


minor nit: align parentheses with line above.

jnewbery · 2017-03-16T14:30:04Z

Looks good. A couple of nits and a suggestion on improving the test case.

JeremyRubin · 2017-03-16T21:30:23Z

utAck

NicolasDorier · 2017-03-17T03:10:38Z

@jnewbery I improved the tests, and fixed the alignement. I can't use named parameter in the test though.

NicolasDorier · 2017-03-17T14:47:37Z

added named parameters to one call to listreceivedbyaddress in the tests for better readability.

jnewbery · 2017-03-17T15:23:17Z

line 73 minConf needs to be minconf.

with that change, tested ACK. Good stuff @NicolasDorier !

NicolasDorier · 2017-03-18T14:39:01Z

@jnewbery thanks done, @JeremyRubin did the hard work :)

EthanHeilman · 2017-03-21T20:07:32Z

utACK - the code looks good and this is very useful functionality.

NicolasDorier · 2017-03-22T00:43:16Z

@TheBlueMatt I fixed nits and added tests, can you re-ACK ?

nopara73 · 2017-05-25T06:18:39Z

ACK - works fine.
This could be (not sure if should be) combined with fixing this issue for correct GUI experience: #9192

NicolasDorier · 2017-06-14T07:10:50Z

Closing. I am using a workaround now, I am caching the whole listtransaction, at every block. This call would have been super useful though.

jnewbery · 2017-06-14T14:27:34Z

😞 I'm happy to reACK if you decide to reopen this!

NicolasDorier · 2017-06-15T13:37:38Z

@jnewbery, if you have interest, I am reopening, I guess it does not hurt to keep it opens, it is not high maintenance PR. I was thinking to allow multiple addresses to be passed in the filter, instead of only one.

For NTumbleBit, I changed the design, I have a method called GetTransactions(address) which should send back the inputs AND outputs transactions of this address. listreceivedbyaddress would only show input transactions.

The way I ended up implemeting my need is by caching listtransaction then for each transaction in it, caching gettransaction in a internal database. I now have good performance, and I am not sure this RPC method would suit my case now.

EDIT: Documenting why I need also all transaction spent from this address: (not only transactions received)

In TumbleBit, there is a chain of transaction.

[Escrow] -> [Offer] -> [Fulfill]

Escrow is confirmed.
The user knows one of the ScriptPubKey inside Offer, but not the transaction ID.
His goal is to fetch Fulfill on the blockchain. (When the ScriptPubKey of Offer get spent, then important preimages are revealed)

So the way I am doing it now is adding Offer's ScriptPubKey as WatchOnly. Then fetching all the transactions from the wallet, caching them in internal database, and going through all those transactions to see if there is one input which is spent by the ScriptPubKey of Offer. If yes, then we have [Fulfill].

Watching all transactions related to one address is very useful, not only the received transaction of the address.

In Core, however it is not possible, because knowing if a transaction is "from an address" is considered bad practice.
At the root, this is the same reason as to why #10007 could not make it..

NicolasDorier · 2018-03-06T18:13:16Z

ok rebasing

NicolasDorier · 2018-03-06T23:10:15Z

Rebased, fixed the nits of @luke-jr and @promag .

I renamed only_address to address_filter so that if later we decide it is also possible to pass an array of address instead of a single address, we don't need to rename anything. It is more generic.

kallewoof

utACK

kallewoof · 2018-03-07T04:45:49Z

src/wallet/rpcwallet.cpp

@@ -1403,6 +1403,16 @@ UniValue ListReceived(CWallet * const pwallet, const UniValue& params, bool fByA
        if(params[2].get_bool())
            filter = filter | ISMINE_WATCH_ONLY;

+    bool fFilterAddress = false;


The code convention has changed a bit recently. Now, we don't put the type as prefix, and we try to use snake case. I would rename this has_filter_address and the below to filter_address. You can leave existing vars as is, if you don't want the diff to grow for no reason though (like fByAccounts below).

kallewoof · 2018-03-07T04:50:21Z

src/wallet/rpcwallet.cpp

@@ -3837,7 +3866,7 @@ static const CRPCCommand commands[] =
    { "wallet",             "listaddressgroupings",             &listaddressgroupings,          {} },
    { "wallet",             "listlockunspent",                  &listlockunspent,               {} },
    { "wallet",             "listreceivedbyaccount",            &listreceivedbyaccount,         {"minconf","include_empty","include_watchonly"} },
-    { "wallet",             "listreceivedbyaddress",            &listreceivedbyaddress,         {"minconf","include_empty","include_watchonly"} },
+    { "wallet",             "listreceivedbyaddress",            &listreceivedbyaddress,         {"minconf","include_empty","include_watchonly","address_filter" } },


No space after " (end of line) -- compare to other lines.

NicolasDorier · 2018-03-07T13:31:57Z

Addressed @kallewoof nits

kallewoof

utACK f087613

laanwj · 2018-03-07T15:09:23Z

utACK f087613

@JeremyRubin

f087613 Add tests of listreceivedbyaddress address filtering (Jeremy Rubin) 8ee0812 Add address filtering to listreceivedbyaddress (Jeremy Rubin) Pull request description: Supersede #9503 created by @JeremyRubin , I will maintain it. Tree-SHA512: 2accaed493b7e1c2eb5cb5270180f100f8c718b6585b9574f294191c318dc622a79e42ac185300f291f82d3b2a6f1c00850b6b17e4ff2dbab94d71df695acbfe

luke-jr · 2018-03-07T15:16:31Z

test/functional/wallet_listreceivedby.py

+                            {"address": empty_addr},
+                            {"address": empty_addr, "account": "", "amount": 0, "confirmations": 0, "txids": []})
+
+        #Test Address filtering


Space after # is expected now it seems.

luke-jr · 2018-03-07T15:16:43Z

test/functional/wallet_listreceivedby.py

+
+        #Test Address filtering
+        #Only on addr
+        expected = {"address":addr, "account":"", "amount":Decimal("0.1"), "confirmations":10, "txids":[txid,]}


Space after : is expected now it seems.

@JeremyRubin

f087613 Add tests of listreceivedbyaddress address filtering (Jeremy Rubin) 8ee0812 Add address filtering to listreceivedbyaddress (Jeremy Rubin) Pull request description: Supersede bitcoin#9503 created by @JeremyRubin , I will maintain it. Tree-SHA512: 2accaed493b7e1c2eb5cb5270180f100f8c718b6585b9574f294191c318dc622a79e42ac185300f291f82d3b2a6f1c00850b6b17e4ff2dbab94d71df695acbfe

@JeremyRubin

f087613 Add tests of listreceivedbyaddress address filtering (Jeremy Rubin) 8ee0812 Add address filtering to listreceivedbyaddress (Jeremy Rubin) Pull request description: Supersede bitcoin#9503 created by @JeremyRubin , I will maintain it. Tree-SHA512: 2accaed493b7e1c2eb5cb5270180f100f8c718b6585b9574f294191c318dc622a79e42ac185300f291f82d3b2a6f1c00850b6b17e4ff2dbab94d71df695acbfe

@JeremyRubin

f087613 Add tests of listreceivedbyaddress address filtering (Jeremy Rubin) 8ee0812 Add address filtering to listreceivedbyaddress (Jeremy Rubin) Pull request description: Supersede bitcoin#9503 created by @JeremyRubin , I will maintain it. Tree-SHA512: 2accaed493b7e1c2eb5cb5270180f100f8c718b6585b9574f294191c318dc622a79e42ac185300f291f82d3b2a6f1c00850b6b17e4ff2dbab94d71df695acbfe

@JeremyRubin

f087613 Add tests of listreceivedbyaddress address filtering (Jeremy Rubin) 8ee0812 Add address filtering to listreceivedbyaddress (Jeremy Rubin) Pull request description: Supersede bitcoin#9503 created by @JeremyRubin , I will maintain it. Tree-SHA512: 2accaed493b7e1c2eb5cb5270180f100f8c718b6585b9574f294191c318dc622a79e42ac185300f291f82d3b2a6f1c00850b6b17e4ff2dbab94d71df695acbfe

maflcko mentioned this pull request Mar 14, 2017

listreceivedbyaddress Filter Address #9503

Closed

NicolasDorier force-pushed the listreceivedbyaddress-filtered branch from 45c69fa to 8c373d5 Compare March 14, 2017 16:52

kallewoof approved these changes Mar 14, 2017

View reviewed changes

fanquake added RPC/REST/ZMQ Wallet labels Mar 14, 2017

jnewbery reviewed Mar 16, 2017

View reviewed changes

NicolasDorier force-pushed the listreceivedbyaddress-filtered branch from 8c373d5 to 2f435f4 Compare March 17, 2017 03:09

NicolasDorier force-pushed the listreceivedbyaddress-filtered branch from 2f435f4 to 7d9a0f1 Compare March 17, 2017 14:47

NicolasDorier force-pushed the listreceivedbyaddress-filtered branch from 7d9a0f1 to eef18e8 Compare March 18, 2017 14:04

NicolasDorier force-pushed the listreceivedbyaddress-filtered branch from eef18e8 to c262be5 Compare March 21, 2017 01:58

nopara73 mentioned this pull request May 25, 2017

Tumbling is very slow, lot's of Client's redeem broadcasted NTumbleBit/NTumbleBit#34

Closed

NicolasDorier closed this Jun 14, 2017

NicolasDorier reopened this Jun 15, 2017

jnewbery mentioned this pull request Jun 16, 2017

RPC: Add parameter to addmultisigaddress / createmultisig to sort public keys #8751

Closed

NicolasDorier reopened this Mar 6, 2018

NicolasDorier force-pushed the listreceivedbyaddress-filtered branch 5 times, most recently from 22ebb2a to b3f2540 Compare March 6, 2018 23:09

NicolasDorier force-pushed the listreceivedbyaddress-filtered branch from b3f2540 to 522c119 Compare March 6, 2018 23:12

kallewoof approved these changes Mar 7, 2018

View reviewed changes

NicolasDorier force-pushed the listreceivedbyaddress-filtered branch 2 times, most recently from 9f63a88 to 4211124 Compare March 7, 2018 13:29

JeremyRubin added 2 commits March 7, 2018 08:31

Add address filtering to listreceivedbyaddress

8ee0812

Add tests of listreceivedbyaddress address filtering

f087613

NicolasDorier force-pushed the listreceivedbyaddress-filtered branch from 4211124 to f087613 Compare March 7, 2018 13:31

kallewoof approved these changes Mar 7, 2018

View reviewed changes

laanwj merged commit f087613 into bitcoin:master Mar 7, 2018

luke-jr reviewed Mar 7, 2018

View reviewed changes

ryanofsky mentioned this pull request Mar 7, 2018

Rename account to label where appropriate #11536

Merged

NicolasDorier mentioned this pull request Mar 8, 2018

[Trivial] Fix formatting issue on wallet_listreceivedby #12644

Closed

Fuzzbawls mentioned this pull request Jul 5, 2020

[Tracking] Remove wallet 'account' API PIVX-Project/PIVX#1706

Closed

11 tasks

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

listreceivedbyaddress Filter Address #9991

listreceivedbyaddress Filter Address #9991

Uh oh!

Conversation

NicolasDorier commented Mar 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NicolasDorier commented Mar 14, 2017

Uh oh!

kallewoof left a comment

Choose a reason for hiding this comment

Uh oh!

NicolasDorier commented Mar 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NicolasDorier Mar 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jnewbery commented Mar 16, 2017

Uh oh!

JeremyRubin commented Mar 16, 2017

Uh oh!

NicolasDorier commented Mar 17, 2017

Uh oh!

NicolasDorier commented Mar 17, 2017

Uh oh!

jnewbery commented Mar 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NicolasDorier commented Mar 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EthanHeilman commented Mar 21, 2017

Uh oh!

NicolasDorier commented Mar 22, 2017

Uh oh!

nopara73 commented May 25, 2017

Uh oh!

NicolasDorier commented Jun 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jnewbery commented Jun 14, 2017

Uh oh!

NicolasDorier commented Jun 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NicolasDorier commented Mar 6, 2018

Uh oh!

NicolasDorier commented Mar 6, 2018

Uh oh!

kallewoof left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NicolasDorier commented Mar 7, 2018

Uh oh!

kallewoof left a comment

Choose a reason for hiding this comment

Uh oh!

laanwj commented Mar 7, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

NicolasDorier commented Mar 14, 2017 •

edited

Loading

NicolasDorier commented Mar 16, 2017 •

edited

Loading

NicolasDorier Mar 17, 2017 •

edited

Loading

jnewbery commented Mar 17, 2017 •

edited

Loading

NicolasDorier commented Mar 18, 2017 •

edited

Loading

NicolasDorier commented Jun 14, 2017 •

edited

Loading

NicolasDorier commented Jun 15, 2017 •

edited

Loading