Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

importaddress when walletnotify is set, causes the notification of large number of unrelated transactions #6095

Closed
eriksank opened this issue May 2, 2015 · 16 comments

Comments

@eriksank
Copy link

eriksank commented May 2, 2015

When using importaddress, walletnotify will cause the notification of a large number of unrelated transactions, triggering hundreds of processes and even crash bitcoind by making the server run out of memory.

@eriksank eriksank changed the title importaddress when walletnotify is set, causes the notification of of large number of unrelated transactions importaddress when walletnotify is set, causes the notification of large number of unrelated transactions May 2, 2015
@laanwj
Copy link
Member

laanwj commented May 4, 2015

It should only notify for transactions for the address(es) that you importaddressed.

Can you add concrete steps to reproduce?

@eriksank
Copy link
Author

eriksank commented May 4, 2015

I did this in -regtest mode. So, run bitcoind -regtest -daemon. Generate a few blocks:

bitcoin-cli -regtest setgenerate true 50

Generate a random address and import it:

bitcoin-cli -regtest importaddress mo7W7FBZqg4frF5XtchSJUNVhjkbNr9aWB

And now you get a flurry of transactions being notified. Sometimes it exhausts all memory in my laptop and crashes it.

@wtogami
Copy link
Contributor

wtogami commented May 13, 2015

This is technically not a bug of bitcoind. The example you cite is entirely legitimate to rapid fire all the unique txid's as separate walletnotify events.

Exhausting all memory and crashing is the fault of your implementation. walletnotify is firing off many near-simultaneous processes, each of which is probably doing a RPC query in return to bitcoind. bitcoind services only 4 simultaneous RPC connections (almost true), certain things can cause all the queries to stall, and the responses can be fairly slow.

Increasing your resource limits wouldn't hurt but doing so won't fix the real problem.

Responding to every walletnotify in parallel might be a bad idea. Maybe you need whatever walletnotify is doing to finish instantly after adding the txid to an queue, and have a separate event loop that processes them one at a time.

@wtogami
Copy link
Contributor

wtogami commented May 13, 2015

Here's another way to understand it ... importaddress is not at fault. The very same failure of your system could happen if a lot of transactions hit your wallet fast enough.

@wtogami
Copy link
Contributor

wtogami commented May 13, 2015

Oh wait ... if #6084 is a real bug, then importaddress could be hitting the same problem.

If your system is running out of memory and crashing from your regtest example of only 50 transactions, it is entirely possible it could die during normal operation if hit quickly enough.

@wtogami
Copy link
Contributor

wtogami commented May 13, 2015

RPC might be stuck entirely during the importaddress rescan, thereby guaranteeing all the walletnotify processes will be stuck if they are waiting for RPC to respond. Queuing the RPC commands for one at a time execution would safely handle this.

@wtogami
Copy link
Contributor

wtogami commented May 13, 2015

If the complaint in #6084 for importprivkey and here for importaddress is the unrelated transactions causing walletnotify, then the real complaint is both commands causing a full rescan instead of looking at only a particular privkey or address.

@eriksank
Copy link
Author

I wonder how one address, which is actually an entirely new one (participating in no transactions whatsoever) leads to walletnotify calling my handler program repetitively -- a large number of times -- filling up the process table with I don't know how many new processes, and while eventually depriving every other process of CPU time. Furthermore, it sends me loads of transactions from the coinbase, because bitcoind may contain at that point -- in regtest -- only 1 or 2 non-coinbase transactions.Why does it need to send me all those coinbase transactions?

@wtogami
Copy link
Contributor

wtogami commented May 13, 2015

I explained above that your system failing is an indication that you implemented it wrong, as it is entirely possible for many transactions to appear on your node near simultaneously during normal operation. I suggested one way to make your system more robust against that possibility.

There may be room for improvement in the behavior of particularly importprivkey and importaddress, but please understand that your system as you described could fail even if the unrelated transactions no longer trigger walletnotify.

@eriksank
Copy link
Author

It concerns a newly-generated address that has not participated in any transaction. When I do importaddress on it, I get a flurry of walletnotify calls. How could that brand new address ever have been in a transaction that I should be notified of? By the way, this happens in -regtest on testnet. I haven't tried with mainnet addresses yet ...

@wtogami
Copy link
Contributor

wtogami commented May 14, 2015

https://en.bitcoin.it/wiki/How_to_import_private_keys#Import_Private_key.28s.29
If you are importing an address that you KNOW has never been used, you can use rescan=false. That type of import finishes instantly.

@paveljanik
Copy link
Contributor

@eriksank The walletnotify is not called only on newly imported address, but on every wallet transaction seen on full rescan (because this is the default - see help importaddress).

I do not think this is a bug. Maybe it could be explicitly mentioned in the walletnotify or importaddress help though.

@paveljanik
Copy link
Contributor

... or should we change the code to run walletnotify only on the newly added address when rescan (ie. not notify on old stuff)?
... or disable walletnotify when rescanning to prevent such issues at all?

To me, it looks pretty stupid to run external shell script on bulk scan anyway... But the only bug is that the user doesn't expect it to happen.

@eriksank
Copy link
Author

eriksank commented Jun 1, 2015

@paveljanik, I am actually quite ok with the workaround suggested by @wtogami in which I just call the importaddress function with the added parameter rescan=false. The function no longer crashes my laptop. In my impression, it would indeed make sense only to run walletnotify on the newly added address when rescanning. Indeed, I do not think that anybody expects walletnotify to notify transactions totally unrelated to the address being imported by the importaddress function.

@meshcollider
Copy link
Contributor

Is this still an issue? There has been no further discussion on it for nearly 3 years

@eriksank
Copy link
Author

eriksank commented Mar 7, 2018

Actually, no. Closing the issue. Thanks for inquiring.

@eriksank eriksank closed this as completed Mar 7, 2018
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants