Block Wallet RPCs until wallet is synced to our current chain #9570
Conversation
cbf8fb9 to
4ec00df
Compare
This simplifies fixing bitcoin#9148 as we can now hold cs_wallet across an entire block instead of only per-tx. This change also removes the NOT_IN_BLOCK constant in favor of only passing the CBlockIndex* parameter to SyncTransactions when a new block is being connected, instead of also when a block is being disconnected.
3ccf3d4 to
9010f29
Compare
This removes another callback from block connection logic, one more step towards fixing bitcoin#9148. Note that this does a full mapWallet loop after the first block which is connected after restart. This is due to a previous bug where a coinbase transaction from the current best tip which existed in wallet immediately prior to restart would not be shown in the GUI except because on-load -checkblocks would call DisconnectBlock and ConnectBlock on the current tip. To avoid making it worse (ie that such transactions would never be displayed until restart), a scan to find such transactions was added.
9010f29 to
593a8f1
Compare
This blocks until the wallet has synced up to the current height.
593a8f1 to
d632a99
Compare
|
@morcos points out that the cs_main holding at for the block's transaction's processing loop might interfere with all the fancy new fast block-response networking stuff we have going in 0.14. Sadly, because cs_main must come before cs_wallet, this isnt trivial to fix. One easy fix is to add another cs_wallet - cs_wallet_before_main which is locked here to emulate the behavior prior to the lock split in #7946. |
|
Thinking about it more, most of those things will not be running at that time anyway, because we only have one message processing thread. This may have adverse effects on miners who run with a wallet, but aside from that I dont think there is really all that much cs_main contention here (and certainly wouldn't be a regression from 0.13). We can add a cs_wallet_before_main when we split message processing thread. |
…t we've moved passed the initial block without cs_main
887291d to
da28613
Compare
|
utACK? I think the code is right, but I wish there were a bit more straightforward way of doing this. |
This fixes #9148, though with a bit more code than I intended.
There were two issues: a) wallet could return data based on partial block processing, because there were no locks between SyncTransaction calls, which is fixed relatively easily. and b) you could be busy-waiting on getbestblockheight , and call getbalance immediately after that changed, getting your balance as of the previous block instead of the new one, which is a regression in master from 0.13.
To resolve the second, we introduce a primitive to block until the wallet has caught up either with the current chain, or the best chain (to resolve a simple race condition).
This should probably get a test case, because I know you can see a in simple tests, but I'm not 100% sure about b.