fix(wallet): correctly deal with new coinbase transactions for the same height #3151
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hansieodendaal
previously approved these changes
Jul 30, 2021
A bug was observed that now and then a Coinbase Transaction would not conclude its monitoring with this error: `ValueNotFound(CompletedTransaction(15645503741743694020))` This occurred when due to a small network reorg the miner would request a block for the same height. The new request would often have a different amount of fees which would mean this transaction needs to be cancelled in favour of the new transaction. However, with the transaction being cancelled the coinbase monitoring task will come around to checking the DB if the transaction is still there so it can continue to poll the base node and will get the ValueNotFound error. This is correct behaviour so should not have been logged as an error. The error case also means that the TransactionCancelled event is never fired which resulted in the console wallet UI not updating the state of that transaction which left it in the transaction list erroneous. So this PR handles that “Error” properly and sends the event before ending the coinbase monitoring task.
philipr-za
force-pushed
the
philip-coinbase-cancellation
branch
from
a763360 to
98534b8
Compare
stringhandler
approved these changes
Aug 4, 2021
|
PR queued successfully. Your position in queue is: 3 |
stringhandler
changed the title
|
PR is on top of the queue now |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
A bug was observed that now and then a Coinbase Transaction would not conclude its monitoring with this error:
ValueNotFound(CompletedTransaction(15645503741743694020))This occurred when due to a small network reorg the miner would request a block for the same height. The new request would often have a different amount of fees which would mean this transaction needs to be cancelled in favour of the new transaction. However, with the transaction being cancelled the coinbase monitoring task will come around to checking the DB if the transaction is still there so it can continue to poll the base node and will get the ValueNotFound error. This is correct behaviour so should not have been logged as an error. The error case also means that the TransactionCancelled event is never fired which resulted in the console wallet UI not updating the state of that transaction which left it in the transaction list erroneous.
So this PR handles that “Error” properly and sends the event before ending the coinbase monitoring task.
How Has This Been Tested?
unit tests
Checklist:
developmentbranch.