From 0870a270183d445745b1933310a6796d319e52b0 Mon Sep 17 00:00:00 2001 From: Daniel Choi Date: Fri, 16 Oct 2020 06:51:00 -0700 Subject: [PATCH] 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 --- CHANGELOG.md | 6 +++++ crypto/hd/algorithm_test.go | 52 +++++++++++++++++++++++++++++++++++++ types/config.go | 5 +++- types/config_test.go | 9 +++++++ 4 files changed, 71 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cccee304..f0a50c33 100644 --- 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 index 90a0fb25..f8f0a2c8 100644 --- 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 index 3595558d..4b07b873 100644 --- 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 index 40ce3c07..d9789e40 100644 --- 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) +}