[wallet] [rpc] getreceivedbyaddress should return error if called with address not owned by the wallet #11055

pull jnewbery wants to merge 2 commits into bitcoin:master from jnewbery:getreceivedbyaddress_error changing 2 files +53 −73

jnewbery commented at 3:56 PM on August 15, 2017: member
Two commits:
- First commit tidies up the receivedby.py test (and speeds it up by factor of two)
- Second commit changes getreceivedbyaddress to return error if the address is not found in wallet, and adds test to receivedby.py
jnewbery commented at 5:38 PM on August 15, 2017: member

Should be included in v0.16 release notes
jonasschnelli commented at 6:38 PM on August 15, 2017: contributor

utACK 672fbde065df795d2d23080f2eec651c25875cb2.
jonasschnelli added the label Wallet on Aug 15, 2017
laanwj commented at 10:55 AM on August 16, 2017: member

Concept ACK, this makes sense, there is no reason to ever call this on an unknown (neither directly owned nor watch-only) address.
meshcollider commented at 11:56 AM on August 16, 2017: contributor

utACK https://github.com/bitcoin/bitcoin/commit/672fbde065df795d2d23080f2eec651c25875cb2

in src/wallet/rpcwallet.cpp:645 in 672fbde065 outdated

 641 | @@ -642,7 +642,7 @@ UniValue getreceivedbyaddress(const JSONRPCRequest& request)
 642 |          throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address");
 643 |      CScript scriptPubKey = GetScriptForDestination(address.Get());
 644 |      if (!IsMine(*pwallet, scriptPubKey)) {
 645 | -        return ValueFromAmount(0);
 646 | +        throw JSONRPCError(RPC_WALLET_ERROR, "Address not owned by this wallet");

promag commented at 2:15 PM on August 17, 2017:

throw JSONRPCError(RPC_WALLET_ERROR, "Address not found in wallet");

promag commented at 2:30 PM on August 17, 2017: member
IMO these 2 commits would do:
- Cleanup receivedby.py
- Throw error in getreceivedbyaddress if address is not found in wallet
Where the 2nd commit adds the new error and also the corresponding test.

Otherwise tested ACK 672fbde.
jnewbery commented at 2:54 PM on August 17, 2017: member

Thanks @promag - both good suggestions.

I've squashed this down to 2 commits and changed the error text to "Address not found in wallet". No other changes.
jnewbery force-pushed on Aug 17, 2017
promag commented at 12:49 AM on August 18, 2017: member

reACK 828c454.
jnewbery renamed this:
~~getreceivedbyaddress should return error if called with address not owned by the wallet~~
[wallet] [rpc] getreceivedbyaddress should return error if called with address not owned by the wallet
on Sep 1, 2017
luke-jr commented at 6:47 AM on September 2, 2017: member

Needs rebase
MarcoFalke added the label Needs release notes on Sep 2, 2017
jnewbery force-pushed on Sep 4, 2017
jnewbery commented at 3:56 PM on September 4, 2017: member

Rebased
MarcoFalke commented at 8:02 PM on November 10, 2017: member

@jnewbery Needs rebase

Ping me back, so we can get this merged.

[tests] Tidy up receivedby.py

- Fix flake8 warnings
- Remove the useless get_sub_array_from_array() function
- Reduce runtime for receivedby.py by about half by only using two nodes

ea0cd24f7d

[wallet] getreceivedbyaddress should return error if address is not mine 5e0ba8f8cd
jnewbery force-pushed on Nov 10, 2017
jnewbery commented at 9:08 PM on November 10, 2017: member

@MarcoFalke - apologies. I didn't realise there was a silent merge conflict. Rebased.

MarcoFalke commented at 5:36 PM on November 11, 2017: member

MarcoFalke merged this on Nov 11, 2017
MarcoFalke closed this on Nov 11, 2017
MarcoFalke referenced this in commit 95e14dc81d on Nov 11, 2017
MarcoFalke removed the label Needs release notes on Nov 11, 2017
jnewbery deleted the branch on Nov 11, 2017
MarcoFalke referenced this in commit 41aa9c4a80 on Nov 13, 2017
PastaPastaPasta referenced this in commit 1f88bb9556 on Jan 17, 2020
PastaPastaPasta referenced this in commit 83d475ae34 on Jan 22, 2020
PastaPastaPasta referenced this in commit cc5b2f163c on Jan 22, 2020
PastaPastaPasta referenced this in commit 4dfa4001c7 on Jan 29, 2020
ckti referenced this in commit 13f6469bd7 on Mar 28, 2021
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#11663 [trivial] doc: Add getreceivedbyaddress release notes