crypto: fix Bip44 derivation path (#577)

* change derivationpath to geth's const

* fix lint

* change to var

* add hd path and derivation tests

* replace method

* add to tests

* fix test; pr to sdk

* Update types/config.go

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

* Update types/config.go

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

* add wrong path/acct test

* use BytesToAddress

* add to changelog

* update tests

* update changelog

* minor comment change

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-authored-by: Federico Kunze <federico.kunze94@gmail.com>

CHANGELOG.md

@@ -35,12 +35,18 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 # Changelog
 
+## Unreleased
+
 ### API Breaking
 
 * (crypto) [\#559](https://github.com/cosmos/ethermint/pull/559) Refactored crypto package in preparation for the SDK's Stargate release:
   * `crypto.PubKeySecp256k1` and `crypto.PrivKeySecp256k1` are now `ethsecp256k1.PubKey` and `ethsecp256k1.PrivKey`, respectively
   * Moved SDK `SigningAlgo` implementation for Ethermint's Secp256k1 key to `crypto/hd` package.
 
+### Bug Fixes
+
+* (crypto) [\#577](https://github.com/cosmos/ethermint/pull/577) Fix `BIP44HDPath` that did not prepend `m/` to the path. This now uses the `DefaultBaseDerivationPath` variable from go-ethereum to ensure addresses are consistent.
+
 ## [v0.2.1] - 2020-09-30
 
 ### Features

crypto/hd/algorithm_test.go

@@ -122,3 +122,55 @@ func TestKeyring(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, addr.String(), account.Address.String())
 }
+
+func TestDerivation(t *testing.T) {
+	mnemonic := "picnic rent average infant boat squirrel federal assault mercy purity very motor fossil wheel verify upset box fresh horse vivid copy predict square regret"
+
+	bz, err := DeriveSecp256k1(mnemonic, keys.DefaultBIP39Passphrase, ethermint.BIP44HDPath)
+	require.NoError(t, err)
+	require.NotEmpty(t, bz)
+
+	badBz, err := DeriveSecp256k1(mnemonic, keys.DefaultBIP39Passphrase, "44'/60'/0'/0/0")
+	require.NoError(t, err)
+	require.NotEmpty(t, badBz)
+
+	require.NotEqual(t, bz, badBz)
+
+	privkey, err := EthermintKeygenFunc(bz, EthSecp256k1)
+	require.NoError(t, err)
+	require.NotEmpty(t, privkey)
+
+	badPrivKey, err := EthermintKeygenFunc(badBz, EthSecp256k1)
+	require.NoError(t, err)
+	require.NotEmpty(t, badPrivKey)
+
+	require.NotEqual(t, privkey, badPrivKey)
+
+	wallet, err := hdwallet.NewFromMnemonic(mnemonic)
+	require.NoError(t, err)
+
+	path := hdwallet.MustParseDerivationPath(ethermint.BIP44HDPath)
+	account, err := wallet.Derive(path, false)
+	require.NoError(t, err)
+
+	badPath := hdwallet.MustParseDerivationPath("44'/60'/0'/0/0")
+	badAccount, err := wallet.Derive(badPath, false)
+	require.NoError(t, err)
+
+	// Equality of Address BIP44
+	require.Equal(t, account.Address.String(), "0xA588C66983a81e800Db4dF74564F09f91c026351")
+	require.Equal(t, badAccount.Address.String(), "0xF8D6FDf2B8b488ea37e54903750dcd13F67E71cb")
+	// Inequality of wrong derivation path address
+	require.NotEqual(t, account.Address.String(), badAccount.Address.String())
+	// Equality of Ethermint implementation
+	require.Equal(t, common.BytesToAddress(privkey.PubKey().Address().Bytes()).String(), "0xA588C66983a81e800Db4dF74564F09f91c026351")
+	require.Equal(t, common.BytesToAddress(badPrivKey.PubKey().Address().Bytes()).String(), "0xF8D6FDf2B8b488ea37e54903750dcd13F67E71cb")
+
+	// Equality of Eth and Ethermint implementation
+	require.Equal(t, common.BytesToAddress(privkey.PubKey().Address()).String(), account.Address.String())
+	require.Equal(t, common.BytesToAddress(badPrivKey.PubKey().Address()).String(), badAccount.Address.String())
+
+	// Inequality of wrong derivation path of Eth and Ethermint implementation
+	require.NotEqual(t, common.BytesToAddress(privkey.PubKey().Address()).String(), badAccount.Address.String())
+	require.NotEqual(t, common.BytesToAddress(badPrivKey.PubKey().Address()).String(), account.Address.Hex())
+}

types/config.go

@@ -2,6 +2,7 @@ package types
 
 import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	ethaccounts "github.com/ethereum/go-ethereum/accounts"
 )
 
 const (
@@ -23,9 +24,11 @@ const (
 
 	// Bip44CoinType satisfies EIP84. See https://github.com/ethereum/EIPs/issues/84 for more info.
 	Bip44CoinType = 60
+)
 
+var (
 	// BIP44HDPath is the BIP44 HD path used on Ethereum.
-	BIP44HDPath = "44'/60'/0'/0/0"
+	BIP44HDPath = ethaccounts.DefaultBaseDerivationPath.String()
 )
 
 // SetBech32Prefixes sets the global prefixes to be used when serializing addresses and public keys to Bech32 strings.

types/config_test.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 
+	"github.com/cosmos/cosmos-sdk/crypto/keys/hd"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
@@ -44,3 +45,11 @@ func TestSetCoinType(t *testing.T) {
 	require.Equal(t, sdk.GetConfig().GetCoinType(), config.GetCoinType())
 	require.Equal(t, sdk.GetConfig().GetFullFundraiserPath(), config.GetFullFundraiserPath())
 }
+
+func TestHDPath(t *testing.T) {
+	params := *hd.NewFundraiserParams(0, Bip44CoinType, 0)
+	// need to prepend "m/" because the below method provided by the sdk does not add the proper prepending
+	hdPath := "m/" + params.String()
+	require.Equal(t, "m/44'/60'/0'/0/0", hdPath)
+	require.Equal(t, hdPath, BIP44HDPath)
+}