Keyring --ledger only supports default key secp256k1 even when Option is provided

[fedekunze]

Summary of Bug

Key command with --ledger flag only supports the secp256k1 algorithm for adding and signing transactions, even though the algorithm is supported by the keyring Options.

Version

main, v0.43.0

Steps to Reproduce

git clone github.com/tharsis/ethermint
cd ethermint
make install
ethermintd keys add <name> --ledger

Source:

cosmos-sdk/crypto/keyring/keyring.go

Lines 391 to 404 in 085ab06

	func (ks keystore) SaveLedgerKey(uid string, algo SignatureAlgo, hrp string, coinType, account, index uint32) (Info, error) {
	if !ks.options.SupportedAlgosLedger.Contains(algo) {
	return nil, ErrUnsupportedSigningAlgo
	}
	hdPath := hd.NewFundraiserParams(account, coinType, index)
	priv, _, err := ledger.NewPrivKeySecp256k1(*hdPath, hrp)
	if err != nil {
	return nil, err
	}
	return ks.writeLedgerKey(uid, priv.PubKey(), *hdPath, algo.Name())
	}

cosmos-sdk/crypto/keyring/keyring.go

Lines 613 to 639 in 085ab06

	// SignWithLedger signs a binary message with the ledger device referenced by an Info object
	// and returns the signed bytes and the public key. It returns an error if the device could
	// not be queried or it returned an error.
	func SignWithLedger(info Info, msg []byte) (sig []byte, pub types.PubKey, err error) {
	switch info.(type) {
	case *ledgerInfo, ledgerInfo:
	default:
	return nil, nil, errors.New("not a ledger object")
	}
	path, err := info.GetPath()
	if err != nil {
	return
	}
	priv, err := ledger.NewPrivKeySecp256k1Unsafe(*path)
	if err != nil {
	return
	}
	sig, err = priv.Sign(msg)
	if err != nil {
	return nil, nil, err
	}
	return sig, priv.PubKey(), nil
	}

cc: @alessio

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[fedekunze]

@jleni do we need to provide the interface instead? We want to use the Ethereum secp256k1 keys for the ledger using the default key.

[jleni]

Hi Fede, I am not sure I follow the question.
can you explain this?

only supports the secp256k1 algorithm for adding and signing transactions, even though the algorithm is supported by the keyring Options

at the moment, the app only support secp256k1. That's correct.

I am not sure what you mean is being set in options that is different to that. Are you trying to use a different algorithm?

This bug seems ethermint related. I cannot see how the code in ethermint is calling this differently.

[fedekunze]

@jleni the keyring supports providing multiple types of signing algorithms by providing them as options:

cosmos-sdk/crypto/keyring/keyring.go

Lines 154 to 160 in 085ab06

	// Options define the options of the Keyring.
	type Options struct {
	// supported signing algorithms for keyring
	SupportedAlgos SigningAlgoList
	// supported signing algorithms for Ledger
	SupportedAlgosLedger SigningAlgoList
	}

I thought that meant that you could generate keys for those provided options in Ledger too. But it seems it's hardcoded to only support secp256k1.

This bug seems ethermint related. I cannot see how the code in ethermint is calling this differently.

We are using the keyring implementation from the SDK. I could fork the keyring and use the SECP256K1 interface to support the ethereum secp256k1 key but ideally, we would like to push for a solution that works for everyone on the SDK. We would love to help implement this functionality here

[jleni]

At the moment, I can also see:

cosmos-sdk/crypto/keyring/keyring.go

Lines 211 to 214 in 085ab06

	options := Options{
	SupportedAlgos: SigningAlgoList{hd.Secp256k1},
	SupportedAlgosLedger: SigningAlgoList{hd.Secp256k1},
	}

But if there is a need for more, we need to extend the app, etc.
Could you create an issue about what is missing or what you want to get support for?

[fedekunze]

I added wrapped the error and now it shows this:

ethermintd keys add test --ledger
Error: address '' rejected: [APDU_CODE_DATA_INVALID] Referenced data reversibly blocked (invalidated)

seems to be coming from the ledger-go lib:

https://github.com/cosmos/ledger-go/blob/1c99b41f1877b58e3517ef93012cc76fe5cd335b/ledger.go#L104-L105

[fedekunze]

seems like a standard APDU response error? @jleni

[jleni]

ok, but that is a completely different error code and unrelated to the signature scheme,
it seems that you are sending an empty address?

address '' rejected

I don't think these things are actually related to the app itself, sounds like the payload of what you are trying to sign is invalid.

[fedekunze]

the keys add <name> --ledger command doesn't work with coinType 60 and only works with 118. Will open a PR on the SDK for better error messages

ethermintd keys add test --algo eth_secp256k1 --ledger
test eth_secp256k1 eth 60 0 0
Error: failed to generate ledger key: failed to retrieve pubkey: [APDU_CODE_DATA_INVALID] Referenced data reversibly blocked (invalidated): address rejected for path m/44'/60'/0'/0/0
Usage:
  ethermintd keys add <name> [flags]

https://github.com/cosmos/cosmos-sdk/blob/v0.43.0-rc2/crypto/ledger/ledger_secp256k1.go#L261-L265

it seems that you are sending an empty address?

the address will always be empty because of the error above

[robert-zaremba]

BTW - the keyring options can be customized when creating the keyring. This is currently done through a context If I remember correctly.