-
Notifications
You must be signed in to change notification settings - Fork 101
Keystore event fix #2311
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
base: master
Are you sure you want to change the base?
Keystore event fix #2311
Conversation
On keystore deregister the accounts linked to the keystore were closed and sent statusChanged events to the frontend + a final `accounts` event to notify the change in the accounts number. The lag between the status and the accounts events caused the execution of calls towards endpoint of already closed accounts, which generated unexpected errors. This send an accounts event immediately after each account close, fixing the lag.
`emitAccountsStatusChanged` is a misleading name, as it doesn't fire an event connected to the account status, but about the number of available accounts. As it is catched in the frontend by `syncAccountsList` subscription, now it is called `emitAccountsListChanged`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
n keystore deregister the accounts linked to the keystore were closed
and sent statusChanged events to the frontend + a finalaccounts
event
to notify the change in the accounts number. The lag between the status
and the accounts events caused the execution of calls towards endpoint
of already closed accounts, which generated unexpected errors.
I have a hard time understanding this. Which statusChanged
event exactly?
@@ -931,6 +931,7 @@ func (backend *Backend) uninitAccounts() { | |||
backend.onAccountUninit(account) | |||
} | |||
account.Close() | |||
backend.emitAccountsListChanged() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at this stage backend.accounts
has not changed at all though, so why fire an event here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I guess I'd have to remove the single accounts from the slice after each iteration and then firing the event. Agree?
This https://github.com/digitalbitbox/bitbox-wallet-app/blob/master/backend/coins/btc/account.go#L457 |
Fixes an event transmission lag that caused an unexpected error (discovered in #2307) and renames a misleading backend method. See commit messages for details.