-
Notifications
You must be signed in to change notification settings - Fork 10
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
Refactor/keyset topup #498
Conversation
Quality Gate passedIssues Measures |
keys, err := helper.GetPrivateKeys() | ||
if err != nil { | ||
return err | ||
} | ||
|
||
var keySets []*keyset.KeySet | ||
|
||
if len(keys) > 0 { | ||
if len(selectedKeys) > 0 { | ||
for _, selectedKey := range selectedKeys { | ||
key, found := keys[selectedKey] | ||
if !found { | ||
return fmt.Errorf("key not found: %s", selectedKey) | ||
} | ||
fundingKeySet, _, err := helper.GetKeySetsXpriv(key) | ||
if err != nil { | ||
return fmt.Errorf("failed to get key sets: %v", err) | ||
} | ||
keySets = append(keySets, fundingKeySet) | ||
} | ||
} else { | ||
for _, key := range keys { | ||
fundingKeySet, _, err := helper.GetKeySetsXpriv(key) | ||
if err != nil { | ||
return fmt.Errorf("failed to get key sets: %v", err) | ||
} | ||
keySets = append(keySets, fundingKeySet) | ||
} | ||
} | ||
} else { | ||
return errors.New("no keys given in configuration") | ||
keySets, err := helper.GetKeySets(keys, selectedKeys) | ||
if err != nil { | ||
return 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.
Just loud thinking: maybe it could be one function too? I mean we could have helper functions like:
func GetPrivateKeys()
func GetKeySetsFor(keys map[string]string (...))
func GetKeySets()
The first two would be for future flexibility, and the last one would be used in the currently existing implementation. What do you think?
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.
I have them as getKeySets and GetPrivateKeys , yeah if need be i can fragment and add one more function GetKeySetsFor(keys map[string]string (...))
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.
we also have this function GetKeySetsKeyFile() in the helper which will resolve the issue ?
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.
I agree with @arkadiuszos4chain that we could move both in to yet another function which does both func GetKeySets()
We can also do this in a separate PR. Thus I'll approve now
Refactor/keyset topup
Refactor for keyset topup