-
Notifications
You must be signed in to change notification settings - Fork 249
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
api
- Refactor removal / replacement of deprecated and obsolete funcs
#4976
base: develop
Are you sure you want to change the base?
Conversation
Jenkins Builds
|
|
||
GetNodeConfig() (*params.NodeConfig, error) | ||
UpdateRootDataDir(datadir string) | ||
|
||
// SelectAccount(loginParams account.LoginParams) error | ||
SelectAccount(loginParams account.LoginParams) error |
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.
Just wondering... if this function calls func (m *DefaultManager) SelectAccount(loginParams LoginParams) error
and it expects loginParams.Password
to be provided, won't that return an error in case of a keycard migrated profile?
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.
Interesting questions @saledjenic. I don't think that this change will break any existing flows.
This change only reintroduces the function signature to the StatusBackend
interface. The implementation details of this function (whether loginParams.Password
is expected etc) are a concern for the implementing struct.
However existing code, unaltered by this PR, that implements this function signature already uses the loginParams.Password
property.
func (m *DefaultManager) SelectAccount(loginParams LoginParams) error {
m.mu.Lock()
defer m.mu.Unlock()
m.accountsGenerator.Reset()
selectedChatAccount, err := m.unlockExtendedKey(loginParams.ChatAddress.String(), loginParams.Password)
if err != nil {
return err
}
m.watchAddresses = loginParams.WatchAddresses
m.mainAccountAddress = loginParams.MainAccount
m.selectedChatAccount = selectedChatAccount
return nil
}
This code is called via the LoginAccount
API method. So perhaps a different API method is called when logging in via Keycard, because the failure would already be an issue without the changes in this PR.
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.
@Samyoul yes, I see what you've changed, no problem with your change at all, I just used the opportunity to ask does that somehow affect the keycard users, nothing else, sorry about confusion. :)
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.
🚢 it, thanks @Samyoul !
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.
Samyoul, just bumping this. The poor PR is feeling forgotten 😅
Oh no, Poor little PR . Thank you @osmaczko , I'll resolve the conflicts on merge |
I've resolved a number of refactor issues in the
api
package.backend
: Updated theStatusBackend
interface to match usagebackend_test
: Resolved unhandled errors, import collisions and upgraded deprecated funcseth_backend
: resolved import collisions