Avoid conflicting chain locks when rescan is triggered by new block #533
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix for bcoin-org/bcoin#1006 from upstream in bcoin.
Issue summary:
Bug fix summary:
Emit the
'block connect'
event synchronously so the original'connect'
event from chain can resolve. The chain'connect'
event is emitted asynchronously meaning it waits for all its listeners to resolve before program flow continues. The chain event is proxied bynodeclient
to the wallet as'block connect'
and this is what ultimately triggers the reorg.Note that this is only an issue when the wallet is run as a plugin. If the wallet is being run as a separate node, it gets the
'block connect'
event from the node's HTTP websocket:hsd/lib/node/http.js
Lines 666 to 680 in a1409dc
The websocket event is obviously synchronous because it is just a one-way firing, it doesn't wait for a response at all. Indeed! The fact that one mode of operation uses synchronous events should help justify this pull request.
The side effect of this change is that in a few tests we always just expect the walletDB to be up to date with the chain when we add new blocks. This is no longer the case and probably we should always consider wallet and node operations to be asynchronous anyway. For those tests I've added a wait-er to ensure wallet is synced to chain before continuing.
Reviewers can run the new test
test/wallet-rescan-test.js
after reverting the one-line change inlib/wallet/nodeclient.js
and observe that the test will timeout -- this is because the two conflicting locks will wait for each other forever.