Add removeaddress RPC call (remove watch-only address) #5525

pull paveljanik wants to merge 2 commits into bitcoin:master from paveljanik:rpc_removeaddress changing 4 files +49 −1

paveljanik commented at 10:17 PM on December 21, 2014: contributor
The current master has the feature to add watch-only addresses (RPC importaddress). There is no way to remove watch-only addresses (looks like the unwritten law to not remove/erase anything from the wallet ;-). This PR adds it. The RPC call is named removeaddress. It has only one argument, the address/script to be removed.

You can't remove the address you don't watch in the wallet, you can't remove the address with a privkey.

Problems/questions:
1. After the call, your walletdb contains transactions added by previous (optional) rescan of the call importaddress. I do not touch them, because you can always run bitcoind -zapwallettxes=1 to get rid of them.
2. Qt GUI needs a small fix in TransactionFilterProxy::filterAcceptsRow():
```
if(address.isEmpty())
    return false;
```
(or similar...) to not display these transactions after watch-only addresses are removed. 3. Qt GUI needs to auto-refresh the list of Recent transactions (the right tab on the main screen) because after removeaddress the list is not updated (watch-only transactions are still shown). Or you can restart GUI... 4. I do not remove the address from addressbook, because it could have been added even before adding it in importaddress.
jonasschnelli commented at 8:34 AM on December 22, 2014: contributor

Would it not be recommended to have some tests in (a new rpc test script in qa/pull-tester/and maybe a test in rpc_wallet_tests.cpp)?
paveljanik commented at 8:50 AM on December 22, 2014: contributor

@jonasschnelli Definitely yes. I'll also add missing importaddress tests.
paveljanik force-pushed on Dec 22, 2014
paveljanik force-pushed on Dec 22, 2014
jgarzik added the label RPC on Dec 31, 2014
luke-jr commented at 11:22 AM on January 1, 2015: member

What is the use case for this? Since watchonly really only works with complete wallets, I'm not sure it makes sense to remove addresses?
laanwj added the label Wallet on Jan 5, 2015
paveljanik commented at 10:39 AM on January 10, 2015: contributor

@luke-jr Imagine you are interested in watching some addresses. You importaddress them. After some time, you change your interest and want to stop watching some of them...
luke-jr commented at 1:32 PM on January 10, 2015: member

@paveljanik importaddress is designed for (and only works sanely for) watching an entire wallet by importing all of its addresses. It shouldn't be mixed in a normal wallet or used with just a single address or subset of a wallet, because that causes erratic results.
Diapolo commented at 2:07 PM on January 10, 2015: none

@luke-jr What you say causes problems is the thing I'mn doing, I imported a watch-only address into a "normal use" testing mainnet wallet...
luke-jr commented at 2:10 PM on January 10, 2015: member

@Diapolo Then you should be seeing arbitrary "send" transactions which have nothing to do with the address in question.
paveljanik commented at 9:36 AM on January 11, 2015: contributor

@luke-jr well, OK, but this is what we expect :-) We want to watch the added address... Such transactions should be marked as "watched" though.
jonasschnelli commented at 12:56 PM on March 11, 2015: contributor

needs rebase.
laanwj commented at 2:41 PM on May 6, 2015: member

Still needs rebase
paveljanik force-pushed on May 6, 2015
paveljanik force-pushed on May 6, 2015
paveljanik commented at 3:26 PM on May 6, 2015: contributor

Rebased.
jgarzik commented at 6:44 PM on July 23, 2015: contributor

Bug: Needs to call EnsureWalletIsAvailable()

Otherwise, looks good to me, ut ACK modulo that issue.
paveljanik commented at 8:32 PM on July 23, 2015: contributor
... and LOCK needed IMO
```
LOCK2(cs_main, pwalletMain->cs_wallet);
```
Will rebase/test tomorrow.
Add removeaddress RPC call (remove watch-only address) c6844c901a
Add importaddress and removeaddress tests to wallet tests 621a12233a
paveljanik force-pushed on Jul 23, 2015
TheBlueMatt commented at 6:15 PM on August 21, 2015: member

Needs rebase (again). Maybe needs a bit more of an interactive rebase because of the importaddress/importscript stuff related to the fundrawtransaction watch-only modifications. Sorry about that :/
laanwj commented at 9:39 AM on October 27, 2015: member

The rebase seems to be trivial. See here: https://github.com/laanwj/bitcoin/tree/2015_10_paveljanik_remove_watchonly

Would it not be recommended to have some tests in (a new rpc test script in qa/pull-tester/and maybe a test in rpc_wallet_tests.cpp)?

This does still need tests. Probably best option is to adapt one of the wallet RPC tests to exercise the call.
paveljanik commented at 10:02 AM on October 27, 2015: contributor

I'm sorry but I'm a bit distracted by other project these days :-(
laanwj commented at 10:07 AM on October 27, 2015: member

Are you still planning to do this, say, within the next weeks? Or would it be better to close for now until you can pick it up again?
paveljanik commented at 10:09 AM on October 27, 2015: contributor

I think we can close this and reopen later or anyone can pick up the work with your rebase. Closing.
paveljanik closed this on Oct 27, 2015
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels