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

txdb: fix lockedUnconfirmed if FINALIZE is inserted with block #512

Merged
merged 1 commit into from Nov 22, 2020

Conversation

pinheadmz
Copy link
Member

This is an edge case we should have addressed in #464 especially after dealing with basically the same behavior in #387. And actually if I'd written better test coverage in #387 to actually include TRANSFER and FINALIZE, we probably would've fixed all the bugs six months ago 🤷‍♂️ 😬

The issue is that in bcoin and hsd, unconfirmed includes all confirmed amounts in addition to unconfirmed amounts, they are not mutually exclusive. Normally when we send a TX, it is inserted into the database as unconfirmed. Then when we see it in a block, we update the confirmed balance but we leave the unconfirmed balance alone (it was already updated when the TX was first seen in the mempool).

During a rescan (or more explicitly, an import-from-seed or deep rescan) we never see the TX as unconfirmed -- it is inserted into the database with the block that confirms it. So to keep the unconfirmed balance in sync, we need to update that value first, and then continue with the confirmation process that updates the confirmed balance.

@pinheadmz pinheadmz requested review from tynes and chjj September 8, 2020 17:33
@coveralls
Copy link

coveralls commented Sep 8, 2020

Pull Request Test Coverage Report for Build 250320504

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 3 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.001%) to 59.231%

Files with Coverage Reduction New Missed Lines %
lib/net/pool.js 1 32.63%
lib/net/peer.js 2 35.28%
Totals Coverage Status
Change from base Build 238010293: 0.001%
Covered Lines: 19388
Relevant Lines: 30472

💛 - Coveralls

@pinheadmz pinheadmz mentioned this pull request Sep 8, 2020
2 tasks
test/wallet-test.js Outdated Show resolved Hide resolved
@pinheadmz
Copy link
Member Author

rebase to 0e7865c:
clean up test re: #512 (comment)

@pinheadmz pinheadmz added this to the v2.3.0 milestone Sep 17, 2020
@chjj chjj merged commit dd2cf3c into handshake-org:master Nov 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants