Fix multiwallet transaction notifications#120
Merged
jonasschnelli merged 3 commits intoNov 12, 2020
Merged
Conversation
This is needed because next commit moves vQueueNotifications to TransactionTablePriv member.
Drop global vQueueNotifications and make one for each wallet.
This was referenced Oct 28, 2020
promag
force-pushed
the
2020-10-fix-transaction-notifications
branch
from
278ad5f to
2414342
Compare
Contributor
Author
|
@jonasschnelli @fanquake this is a bugfix. |
Contributor
|
utACK 2414342 |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Nov 12, 2020
2414342 refactor: qt: Use vQueueNotifications.clear() (João Barbosa) 989e579 qt: Make transaction notification queue wallet specific (João Barbosa) 7b3b230 move-only: Define TransactionNotification before TransactionTablePriv (João Barbosa) Pull request description: Currently `vQueueNotifications` holds transactions of any wallet, but the queue is dispatched on a given wallet and it assumes notifications are of that wallet. This means that some transactions can be missed if multiple wallets are loaded. Fix this by having a queue for each wallet. ACKs for top commit: jonasschnelli: utACK 2414342 hebasto: ACK 2414342, I have reviewed the code and it looks OK, I agree it can be merged. ryanofsky: Code review ACK 2414342. Only change is dropping one commit Tree-SHA512: 61beac5a16ed659e3a25ad145dbceafcef963aaf8f9838355298949ec2324e2bd760f59353cd251d30cf0334d8dc1642a1f3821d8a9eec092533b581f6ce86db
willyko
pushed a commit
to syscoin-core/syscoin-trimmed
that referenced
this pull request
Nov 12, 2020
241434200ec2067673d8522fee4f1228abfd8247 refactor: qt: Use vQueueNotifications.clear() (João Barbosa)
989e579d07bb5031639060b717f7a0be15d10e29 qt: Make transaction notification queue wallet specific (João Barbosa)
7b3b2303f44031c3545651858f697a495c3ea37a move-only: Define TransactionNotification before TransactionTablePriv (João Barbosa)
Pull request description:
Currently `vQueueNotifications` holds transactions of any wallet, but the queue is dispatched on a given wallet and it assumes notifications are of that wallet.
This means that some transactions can be missed if multiple wallets are loaded.
Fix this by having a queue for each wallet.
ACKs for top commit:
jonasschnelli:
utACK 241434200ec2067673d8522fee4f1228abfd8247
hebasto:
ACK 241434200ec2067673d8522fee4f1228abfd8247, I have reviewed the code and it looks OK, I agree it can be merged.
ryanofsky:
Code review ACK 241434200ec2067673d8522fee4f1228abfd8247. Only change is dropping one commit
Tree-SHA512: 61beac5a16ed659e3a25ad145dbceafcef963aaf8f9838355298949ec2324e2bd760f59353cd251d30cf0334d8dc1642a1f3821d8a9eec092533b581f6ce86db
Former-commit-id: 340957c6beffff2f444a8077254b9bd02afe4b2d
apoelstra
added a commit
to apoelstra/elements
that referenced
this pull request
Dec 3, 2020
gwillen
pushed a commit
to ElementsProject/elements
that referenced
this pull request
Mar 23, 2021
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Dec 23, 2021
Summary: This is needed because next commit moves `vQueueNotifications` to `TransactionTablePriv` member. This is a backport of [[bitcoin-core/gui#120 | core-gui#120]] [1/2] bitcoin-core/gui@7b3b230 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D10730
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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.
Currently
vQueueNotificationsholds transactions of any wallet, but the queue is dispatched on a given wallet and it assumes notifications are of that wallet.This means that some transactions can be missed if multiple wallets are loaded.
Fix this by having a queue for each wallet.