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 <[email protected]> * Update types/config.go Co-authored-by: Federico Kunze <[email protected]> * add wrong path/acct test * use BytesToAddress * add to changelog * update tests * update changelog * minor comment change Co-authored-by: Federico Kunze <[email protected]> Co-authored-by: Federico Kunze <[email protected]>
cosmos · Oct 16, 2020 · 0870a27 · 0870a27
1 parent 13d0d52
commit 0870a27
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 1 deletion.
diff --git a/CHANGELOG.md b/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

diff --git a/crypto/hd/algorithm_test.go b/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())
+}
diff --git a/types/config.go b/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.

diff --git a/types/config_test.go b/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)
+}