-
Notifications
You must be signed in to change notification settings - Fork 245
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
update account timestamp when login with keycard (#1613) #1614
Conversation
Hey @yenda, and thank you so much for making your first pull request in status-go! ❤️ Please help us make your experience better by filling out this brief questionnaire https://goo.gl/forms/uWqNcVpVz7OIopXg2 |
Pull Request Checklist
|
Jenkins Builds
|
Tested in status-react status-im/status-mobile#9014 What is the merging workflow around here? |
api/backend.go
Outdated
|
||
func (b *StatusBackend) StartNodeWithKey(acc multiaccounts.Account, password string, keyHex string) error { | ||
err := b.startNodeWithKey(acc, password, keyHex) | ||
signal.SendLoggedIn(err) |
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.
Not sure how significant it is but a potential scenario looks like this :
StartNodeWithKey
is called in one thread.startNodeWithKey
returns an error.- A signal is received in a different thread.
- status-react tries to restart node while
b.StopNode()
is not finished.
Should be move signal.SendLoggedIn(err)
behind b.StopNode()
?
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.
status-react only starts a node via login though, but we could still do that, it would have do be done in StartNodeWithAccount as well though, that is where I took this flow from
@adambabik I moved the login signal after stopNode is that what you had in mind? |
@yenda yes! |
@adambabik cool can you merge it? |
update account timestamp when login with keycard
Call b.multiaccountsDB.UpdateAccountTimestamp in startNodeWithKey as done in startNodeWithAccount
Closes #1613