Skip to content
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

Injection of Signing algorithm in Keys Add CLI command. #8803

Closed
6 tasks
jgimeno opened this issue Mar 5, 2021 · 9 comments · Fixed by #8825
Closed
6 tasks

Injection of Signing algorithm in Keys Add CLI command. #8803

jgimeno opened this issue Mar 5, 2021 · 9 comments · Fixed by #8825
Assignees
Labels
C:CLI C:Keys Keybase, KMS and HSMs

Comments

@jgimeno
Copy link
Contributor

jgimeno commented Mar 5, 2021

Summary

When a user creates a new SigningAlgo in order to be used by the keyring the only way they can append it as an option is to rewrite the Add command. We can try to find a way that a custom signing algorithm can be injected in the keyring without having to rewrite the command.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@araskachoi
Copy link

we are particularly interested in this use case where we would like to pass our own custom "eth_secp256k1" signing algorithm to pass into account generation. However, when we use the sdk keys add cmd, we are unable to pass our own algorithm and it automatically defaults to secp256k1 signing algorithm that is native to cosmos-sdk. If there could be a way we can pass these as arguments and dynamically generate the keyring with the passed signing algorithm(s)?

thanks in advance for your help!

@amaury1093
Copy link
Contributor

Is this related/duplicate of #6500?

@alessio
Copy link
Contributor

alessio commented Mar 9, 2021

Is this related/duplicate of #6500?

Yes, I believe it is related

@clevinson clevinson added C:CLI C:Keys Keybase, KMS and HSMs labels Mar 9, 2021
@robert-zaremba
Copy link
Collaborator

In #8862 I'm extending the hd algos with:

  • generalizing ECDSA derivation
  • with above - adding support to sepc256r1 to keyring. Would be good if some folks with good knowledge of hardware wallets can chime in that PR.

@robert-zaremba
Copy link
Collaborator

I added subtasks to just start tackling this issue.

@alessio
Copy link
Contributor

alessio commented Mar 15, 2021

@robert-zaremba #8862 is not necessarily related to this. It does not look like a sub-task to me because this issue is not about supporting more algos in the SDK. This issue is about allowing applications to use their own algos and inject them in the keyring so that they could be supported by the keyring and the cli commands.

@clevinson
Copy link
Contributor

Re-opening this as it's not actually taken care of by #8886, and we should still be tracking the injection of new signing algos.

@JulianToledano
Copy link
Contributor

JulianToledano commented Sep 4, 2023

hey guys!
As far as I understand, the keyring allows you to configure your algos through options, as seen here:

for _, optionFn := range opts {
optionFn(&options)
}

Then, applications can set these options through their context:

// WithKeyringOptions returns a copy of the context with an updated keyring.
func (ctx Context) WithKeyringOptions(opts ...keyring.Option) Context {
ctx.KeyringOptions = opts
return ctx
}

// NewKeyringFromBackend gets a Keyring object from a backend
func NewKeyringFromBackend(ctx Context, backend string) (keyring.Keyring, error) {
if ctx.Simulate {
backend = keyring.BackendMemory
}
return keyring.New(sdk.KeyringServiceName(), backend, ctx.KeyringDir, ctx.Input, ctx.Codec, ctx.KeyringOptions...)
}

With this approach, it doesn't seem necessary to rewrite the add command.

what do you guys think? It is sufficient to close this?
@tac0turtle @clevinson @alessio

@JulianToledano
Copy link
Contributor

Closing this for now, as the context allows applications to pass their algorithms as an option. Feel free to reopen it if you believe this doesn't resolve the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:CLI C:Keys Keybase, KMS and HSMs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants