Fix active_transactions missed notifications #4505
Merged
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
…nfirmation. Previously the account_balance observers would not be notified if a block was indirectly confirmed without an election
… active_transactions. This data can now be easily obtained directly from block objects.
This is an interaction between the ledger when a block is confirmed and the wallets that might need to generate receive blocks. Previously this functionality was unnecessarily being handled by the active_transactions class. It could also miss notifications if the block was indirectly confirmed without an active election, or if the election had already been removed from active_transactions.
pwojcikdev
approved these changes
Mar 19, 2024
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.
This fixes two instances where notifications could be missed after a block is confirmed.
Blocks confirmed indirectly through confirmation height wouldn't receive balance change notifications because it was directly invoking node.observers.blocks.notify rather than using active_transactions::notify_obserners. This is currently only used by the qt wallet.
The wallet may not have been notified of a block that could be received because of the implementation of active_transactions::block_cemented_callback. The wallet was only notified within active_transactions::handle_confirmation which itself was only called if a transaction was active when active_transactions::block_cemented_callback -> active_transactions::election_status -> active_transactions::confirm_block was invoked. The need for this wallet notification only depends on the source block being confirmed regardless of details about elections. This callback was moved directly on to the node class rather than active_transactions.
This also removes node::process_confirmed_data which was only a utility function for nano::active_transactions and was retrieving data that can be directly obtained from the block itself.