Gaiacli generates the same address irrespective of the --account argument

[jleni]

Summary of Bug

When recovering addresses, gaiacli seems to ignore the account argument.

This issue is related to #3345

Steps to Reproduce

Using the same mnemonic, run the following commands:

gaiacli keys add gaia0 --recover --account 0
gaiacli keys add gaia1 --recover --account 1
gaiacli keys add gaia152 --recover --account 152

In all cases, the same address and pubkey will be generated.

---

For Admin Use

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

[jackzampolin]

It looks like the --account flag isn't being respected in the gaiacli keys add implementation

[jleni]

yes, I would say that having both --account and --index is a bit confusing

cosmos-sdk/client/keys/add.go

Lines 75 to 76 in b8843fc

	cmd.Flags().Uint32(flagAccount, 0, "Account number for HD derivation")
	cmd.Flags().Uint32(flagIndex, 0, "Index number for HD derivation")

[jleni]

@gamarin2 @alessio FYI

Flags:
      --account uint32            Account number for HD derivation
      --bip44-path string         BIP44 path from which to derive a private key (default "44'/118'/0'/0/0")
      --index uint32              Index number for HD derivation

Now, the confusing part is that:

• With Ledger:

  • USED: --account and --index ({44, 118, account, 0, index})
  • IGNORED: --bip44-path

• Without Ledger:

  • IGNORED: --account and --index
  • USED: --bip44-path

This is very confusing and not clear in the docs or flag description.

Reference:

cosmos-sdk/client/keys/add.go

Lines 171 to 190 in 24876aa

	bipFlag := cmd.Flags().Lookup(flagBIP44Path)
	bip44Params, err := getBIP44ParamsAndPath(bipFlag.Value.String(), bipFlag.Changed || !interactive)
	if err != nil {
	return err
	}
	// If we're using ledger, only thing we need is the path. So generate key and
	// we're done.
	if viper.GetBool(client.FlagUseLedger) {
	account := uint32(viper.GetInt(flagAccount))
	index := uint32(viper.GetInt(flagIndex))
	path := ccrypto.DerivationPath{44, 118, account, 0, index}
	info, err := kb.CreateLedger(name, path, keys.Secp256k1)
	if err != nil {
	return err
	}
	printCreate(info, "")
	return nil
	}

[cwgoes]

I think giving the option to specify the full BIP44 path may be a bit dangerous - changing the first two fields could result in conflicts with other coins. Is there any reason a user would need to do that? Unless there is, I would vote to standardize on --account and --index` - and document those better.

[alessio]

I think giving the option to specify the full BIP44 path may be a bit dangerous

I very much agree to that

[jackzampolin]

Closed by #3461