From 5be42d95b8655320d1d40edd849a76ad3fbb3de3 Mon Sep 17 00:00:00 2001 From: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Date: Fri, 20 Nov 2020 12:18:03 +0100 Subject: [PATCH] crypto/hd: add 'm/' prefix to hd path (#7977) * launchpad: backport 7970 * cherry pick 7628 * fix tests * Update CHANGELOG.md * revert tiny portion of #7970 (#7984) Testable examples were accidentally converted into tests. * adapt test to launchpad * fix golangci-lint warnings Co-authored-by: Emmanuel T Odeke Co-authored-by: Alessio Treglia --- CHANGELOG.md | 1 + client/keys/delete_test.go | 4 +- client/keys/show_test.go | 4 +- client/keys/update_test.go | 5 +- crypto/keys/hd/fundraiser_test.go | 6 +- crypto/keys/hd/hdpath.go | 62 ++++++++---- crypto/keys/hd/hdpath_test.go | 161 +++++++++++++++++++++--------- crypto/keys/keybase_test.go | 27 ++++- crypto/keys/types_test.go | 26 ++--- crypto/ledger_test.go | 24 ++--- types/address.go | 10 +- 11 files changed, 226 insertions(+), 104 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 83e0e9c323..d23048413a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes +* (crypto) [\#7966](https://github.com/cosmos/cosmos-sdk/issues/7966) `BIP44Params` `String()` method now correctly returns the absolute HD path by adding the `m/` prefix. * (client) [\#7588](https://github.com/cosmos/cosmos-sdk/issues/7588) Fix gov votes querier to use proper query params. * (types) [\#7038](https://github.com/cosmos/cosmos-sdk/issues/7038) Fix infinite looping of `ApproxRoot` by including a hard-coded maximum iterations limit of 100. * (types) [\#7084](https://github.com/cosmos/cosmos-sdk/pull/7084) Fix panic when calling `BigInt()` on an uninitialized `Int`. diff --git a/client/keys/delete_test.go b/client/keys/delete_test.go index b2df8684df..891e338c76 100644 --- a/client/keys/delete_test.go +++ b/client/keys/delete_test.go @@ -46,13 +46,13 @@ func Test_runDeleteCmd(t *testing.T) { if runningUnattended { mockIn.Reset("testpass1\ntestpass1\n") } - _, err = kb.CreateAccount(fakeKeyName1, tests.TestMnemonic, "", "", "0", keys.Secp256k1) + _, err = kb.CreateAccount(fakeKeyName1, tests.TestMnemonic, "", "", sdk.FullFundraiserPath, keys.Secp256k1) require.NoError(t, err) if runningUnattended { mockIn.Reset("testpass1\ntestpass1\n") } - _, err = kb.CreateAccount(fakeKeyName2, tests.TestMnemonic, "", "", "1", keys.Secp256k1) + _, err = kb.CreateAccount(fakeKeyName2, tests.TestMnemonic, "", "", "m/44'/118'/0'/0/1", keys.Secp256k1) require.NoError(t, err) if runningUnattended { diff --git a/client/keys/show_test.go b/client/keys/show_test.go index ec84eea49e..8c68f9d2c0 100644 --- a/client/keys/show_test.go +++ b/client/keys/show_test.go @@ -58,13 +58,13 @@ func Test_runShowCmd(t *testing.T) { if runningUnattended { mockIn.Reset("testpass1\ntestpass1\n") } - _, err = kb.CreateAccount(fakeKeyName1, tests.TestMnemonic, "", "", "0", keys.Secp256k1) + _, err = kb.CreateAccount(fakeKeyName1, tests.TestMnemonic, "", "", sdk.FullFundraiserPath, keys.Secp256k1) require.NoError(t, err) if runningUnattended { mockIn.Reset("testpass1\n") } - _, err = kb.CreateAccount(fakeKeyName2, tests.TestMnemonic, "", "", "1", keys.Secp256k1) + _, err = kb.CreateAccount(fakeKeyName2, tests.TestMnemonic, "", "", "m/44'/118'/0'/0/1", keys.Secp256k1) require.NoError(t, err) // Now try single key diff --git a/client/keys/update_test.go b/client/keys/update_test.go index 351cedd578..75a3b5042f 100644 --- a/client/keys/update_test.go +++ b/client/keys/update_test.go @@ -9,6 +9,7 @@ import ( "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/crypto/keys" "github.com/cosmos/cosmos-sdk/tests" + sdk "github.com/cosmos/cosmos-sdk/types" ) func Test_updateKeyCommand(t *testing.T) { @@ -39,9 +40,9 @@ func Test_runUpdateCmd(t *testing.T) { kb, err := NewKeyBaseFromDir(viper.GetString(flags.FlagHome)) assert.NoError(t, err) - _, err = kb.CreateAccount(fakeKeyName1, tests.TestMnemonic, "", "", "0", keys.Secp256k1) + _, err = kb.CreateAccount(fakeKeyName1, tests.TestMnemonic, "", "", sdk.FullFundraiserPath, keys.Secp256k1) assert.NoError(t, err) - _, err = kb.CreateAccount(fakeKeyName2, tests.TestMnemonic, "", "", "1", keys.Secp256k1) + _, err = kb.CreateAccount(fakeKeyName2, tests.TestMnemonic, "", "", "m/44'/118'/0'/0/1", keys.Secp256k1) assert.NoError(t, err) // Try again now that we have keys diff --git a/crypto/keys/hd/fundraiser_test.go b/crypto/keys/hd/fundraiser_test.go index 6fa4ca725f..52e8284de6 100644 --- a/crypto/keys/hd/fundraiser_test.go +++ b/crypto/keys/hd/fundraiser_test.go @@ -24,6 +24,10 @@ type addrData struct { Addr string } +func TestFullFundraiserPath(t *testing.T) { + require.Equal(t, "m/44'/118'/0'/0/0", NewFundraiserParams(0, 118, 0).String()) +} + func initFundraiserTestVectors(t *testing.T) []addrData { // NOTE: atom fundraiser address // var hdPath string = "m/44'/118'/0'/0/0" @@ -57,7 +61,7 @@ func TestFundraiserCompatibility(t *testing.T) { t.Logf("ROUND: %d MNEMONIC: %s", i, d.Mnemonic) master, ch := ComputeMastersFromSeed(seed) - priv, err := DerivePrivateKeyForPath(master, ch, "44'/118'/0'/0/0") + priv, err := DerivePrivateKeyForPath(master, ch, "m/44'/118'/0'/0/0") require.NoError(t, err) pub := secp256k1.PrivKeySecp256k1(priv).PubKey() diff --git a/crypto/keys/hd/hdpath.go b/crypto/keys/hd/hdpath.go index a92d79be1f..39651f1c4b 100644 --- a/crypto/keys/hd/hdpath.go +++ b/crypto/keys/hd/hdpath.go @@ -16,7 +16,6 @@ import ( "crypto/sha512" "encoding/binary" - "errors" "fmt" "math/big" "strconv" @@ -49,48 +48,62 @@ func NewParams(purpose, coinType, account uint32, change bool, addressIdx uint32 } } -// Parse the BIP44 path and unmarshal into the struct. +// NewParamsFromPath parses the BIP44 path and unmarshals it into a Bip44Params. It supports both +// absolute and relative paths. func NewParamsFromPath(path string) (*BIP44Params, error) { spl := strings.Split(path, "/") + + // Handle absolute or relative paths + switch { + case spl[0] == path: + return nil, fmt.Errorf("path %s doesn't contain '/' separators", path) + + case strings.TrimSpace(spl[0]) == "": + return nil, fmt.Errorf("ambiguous path %s: use 'm/' prefix for absolute paths, or no leading '/' for relative ones", path) + + case strings.TrimSpace(spl[0]) == "m": + spl = spl[1:] + } + if len(spl) != 5 { - return nil, fmt.Errorf("path length is wrong. Expected 5, got %d", len(spl)) + return nil, fmt.Errorf("invalid path length %s", path) } // Check items can be parsed purpose, err := hardenedInt(spl[0]) if err != nil { - return nil, err + return nil, fmt.Errorf("invalid HD path purpose %s: %w", spl[0], err) } coinType, err := hardenedInt(spl[1]) if err != nil { - return nil, err + return nil, fmt.Errorf("invalid HD path coin type %s: %w", spl[1], err) } account, err := hardenedInt(spl[2]) if err != nil { - return nil, err + return nil, fmt.Errorf("invalid HD path account %s: %w", spl[2], err) } change, err := hardenedInt(spl[3]) if err != nil { - return nil, err + return nil, fmt.Errorf("invalid HD path change %s: %w", spl[3], err) } addressIdx, err := hardenedInt(spl[4]) if err != nil { - return nil, err + return nil, fmt.Errorf("invalid HD path address index %s: %w", spl[4], err) } // Confirm valid values if spl[0] != "44'" { - return nil, fmt.Errorf("first field in path must be 44', got %v", spl[0]) + return nil, fmt.Errorf("first field in path must be 44', got %s", spl[0]) } if !isHardened(spl[1]) || !isHardened(spl[2]) { return nil, - fmt.Errorf("second and third field in path must be hardened (ie. contain the suffix ', got %v and %v", spl[1], spl[2]) + fmt.Errorf("second and third field in path must be hardened (ie. contain the suffix ', got %s and %s", spl[1], spl[2]) } if isHardened(spl[3]) || isHardened(spl[4]) { return nil, - fmt.Errorf("fourth and fifth field in path must not be hardened (ie. not contain the suffix ', got %v and %v", spl[3], spl[4]) + fmt.Errorf("fourth and fifth field in path must not be hardened (ie. not contain the suffix ', got %s and %s", spl[3], spl[4]) } if !(change == 0 || change == 1) { @@ -144,6 +157,8 @@ func (p BIP44Params) DerivationPath() []uint32 { } } +// String returns the full absolute HD path of the BIP44 (https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki) params: +// m / purpose' / coin_type' / account' / change / address_index func (p BIP44Params) String() string { var changeStr string if p.Change { @@ -151,8 +166,7 @@ func (p BIP44Params) String() string { } else { changeStr = "0" } - // m / Purpose' / coin_type' / Account' / Change / address_index - return fmt.Sprintf("%d'/%d'/%d'/%s/%d", + return fmt.Sprintf("m/%d'/%d'/%d'/%s/%d", p.Purpose, p.CoinType, p.Account, @@ -173,6 +187,14 @@ func ComputeMastersFromSeed(seed []byte) (secret [32]byte, chainCode [32]byte) { func DerivePrivateKeyForPath(privKeyBytes [32]byte, chainCode [32]byte, path string) ([32]byte, error) { data := privKeyBytes parts := strings.Split(path, "/") + + switch { + case parts[0] == path: + return [32]byte{}, fmt.Errorf("path '%s' doesn't contain '/' separators", path) + case strings.TrimSpace(parts[0]) == "m": + parts = parts[1:] + } + for _, part := range parts { // do we have an apostrophe? harden := part[len(part)-1:] == "'" @@ -180,19 +202,21 @@ func DerivePrivateKeyForPath(privKeyBytes [32]byte, chainCode [32]byte, path str if harden { part = part[:len(part)-1] } - idx, err := strconv.Atoi(part) + + // As per the extended keys specification in + // https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki#extended-keys + // index values are in the range [0, 1<<31-1] aka [0, max(int32)] + idx, err := strconv.ParseUint(part, 10, 31) if err != nil { - return [32]byte{}, fmt.Errorf("invalid BIP 32 path: %s", err) - } - if idx < 0 { - return [32]byte{}, errors.New("invalid BIP 32 path: index negative ot too large") + return [32]byte{}, fmt.Errorf("invalid BIP 32 path %s: %w", path, err) } + data, chainCode = derivePrivateKey(data, chainCode, uint32(idx), harden) } var derivedKey [32]byte n := copy(derivedKey[:], data[:]) if n != 32 || len(data) != 32 { - return [32]byte{}, fmt.Errorf("expected a (secp256k1) key of length 32, got length: %v", len(data)) + return [32]byte{}, fmt.Errorf("expected a key of length 32, got length: %d", len(data)) } return derivedKey, nil diff --git a/crypto/keys/hd/hdpath_test.go b/crypto/keys/hd/hdpath_test.go index a5110e2cbd..f2102ae692 100644 --- a/crypto/keys/hd/hdpath_test.go +++ b/crypto/keys/hd/hdpath_test.go @@ -5,11 +5,11 @@ import ( "fmt" "testing" + "github.com/stretchr/testify/require" + "github.com/cosmos/cosmos-sdk/types" bip39 "github.com/cosmos/go-bip39" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) var defaultBIP39Passphrase = "" @@ -19,26 +19,15 @@ func mnemonicToSeed(mnemonic string) []byte { return bip39.NewSeed(mnemonic, defaultBIP39Passphrase) } -// nolint:govet -func ExampleStringifyPathParams() { - path := NewParams(44, 0, 0, false, 0) - fmt.Println(path.String()) - path = NewParams(44, 33, 7, true, 9) - fmt.Println(path.String()) - // Output: - // 44'/0'/0'/0/0 - // 44'/33'/7'/1/9 -} - func TestStringifyFundraiserPathParams(t *testing.T) { path := NewFundraiserParams(4, types.CoinType, 22) - require.Equal(t, "44'/118'/4'/0/22", path.String()) + require.Equal(t, "m/44'/118'/4'/0/22", path.String()) path = NewFundraiserParams(4, types.CoinType, 57) - require.Equal(t, "44'/118'/4'/0/57", path.String()) + require.Equal(t, "m/44'/118'/4'/0/57", path.String()) path = NewFundraiserParams(4, 12345, 57) - require.Equal(t, "44'/12345'/4'/0/57", path.String()) + require.Equal(t, "m/44'/12345'/4'/0/57", path.String()) } func TestPathToArray(t *testing.T) { @@ -54,56 +43,136 @@ func TestParamsFromPath(t *testing.T) { params *BIP44Params path string }{ - {&BIP44Params{44, 0, 0, false, 0}, "44'/0'/0'/0/0"}, - {&BIP44Params{44, 1, 0, false, 0}, "44'/1'/0'/0/0"}, - {&BIP44Params{44, 0, 1, false, 0}, "44'/0'/1'/0/0"}, - {&BIP44Params{44, 0, 0, true, 0}, "44'/0'/0'/1/0"}, - {&BIP44Params{44, 0, 0, false, 1}, "44'/0'/0'/0/1"}, - {&BIP44Params{44, 1, 1, true, 1}, "44'/1'/1'/1/1"}, - {&BIP44Params{44, 118, 52, true, 41}, "44'/118'/52'/1/41"}, + {&BIP44Params{44, 0, 0, false, 0}, "m/44'/0'/0'/0/0"}, + {&BIP44Params{44, 1, 0, false, 0}, "m/44'/1'/0'/0/0"}, + {&BIP44Params{44, 0, 1, false, 0}, "m/44'/0'/1'/0/0"}, + {&BIP44Params{44, 0, 0, true, 0}, "m/44'/0'/0'/1/0"}, + {&BIP44Params{44, 0, 0, false, 1}, "m/44'/0'/0'/0/1"}, + {&BIP44Params{44, 1, 1, true, 1}, "m/44'/1'/1'/1/1"}, + {&BIP44Params{44, 118, 52, true, 41}, "m/44'/118'/52'/1/41"}, } for i, c := range goodCases { params, err := NewParamsFromPath(c.path) errStr := fmt.Sprintf("%d %v", i, c) - assert.NoError(t, err, errStr) - assert.EqualValues(t, c.params, params, errStr) - assert.Equal(t, c.path, c.params.String()) + require.NoError(t, err, errStr) + require.EqualValues(t, c.params, params, errStr) + require.Equal(t, c.path, c.params.String()) } badCases := []struct { path string }{ - {"43'/0'/0'/0/0"}, // doesnt start with 44 - {"44'/1'/0'/0/0/5"}, // too many fields - {"44'/0'/1'/0"}, // too few fields - {"44'/0'/0'/2/0"}, // change field can only be 0/1 - {"44/0'/0'/0/0"}, // first field needs ' - {"44'/0/0'/0/0"}, // second field needs ' - {"44'/0'/0/0/0"}, // third field needs ' - {"44'/0'/0'/0'/0"}, // fourth field must not have ' - {"44'/0'/0'/0/0'"}, // fifth field must not have ' - {"44'/-1'/0'/0/0"}, // no negatives - {"44'/0'/0'/-1/0"}, // no negatives - {"a'/0'/0'/-1/0"}, // valid values - {"0/X/0'/-1/0"}, // valid values - {"44'/0'/X/-1/0"}, // valid values - {"44'/0'/0'/%/0"}, // valid values - {"44'/0'/0'/0/%"}, // valid values + {"m/43'/0'/0'/0/0"}, // doesn't start with 44 + {"m/44'/1'/0'/0/0/5"}, // too many fields + {"m/44'/0'/1'/0"}, // too few fields + {"m/44'/0'/0'/2/0"}, // change field can only be 0/1 + {"m/44/0'/0'/0/0"}, // first field needs ' + {"m/44'/0/0'/0/0"}, // second field needs ' + {"m/44'/0'/0/0/0"}, // third field needs ' + {"m/44'/0'/0'/0'/0"}, // fourth field must not have ' + {"m/44'/0'/0'/0/0'"}, // fifth field must not have ' + {"m/44'/-1'/0'/0/0"}, // no negatives + {"m/44'/0'/0'/-1/0"}, // no negatives + {"m/a'/0'/0'/-1/0"}, // invalid values + {"m/0/X/0'/-1/0"}, // invalid values + {"m/44'/0'/X/-1/0"}, // invalid values + {"m/44'/0'/0'/%/0"}, // invalid values + {"m/44'/0'/0'/0/%"}, // invalid values + {"m44'0'0'00"}, // no separators + {" /44'/0'/0'/0/0"}, // blank first component } for i, c := range badCases { params, err := NewParamsFromPath(c.path) errStr := fmt.Sprintf("%d %v", i, c) - assert.Nil(t, params, errStr) - assert.Error(t, err, errStr) + require.Nil(t, params, errStr) + require.Error(t, err, errStr) + } + +} + +// Tests to ensure that any index value is in the range [0, max(int32)] as per +// the extended keys specification. If the index belongs to that of a hardened key, +// its 0x80000000 bit will be set, so we can still accept values in [0, max(int32)] and then +// increase its value as deriveKeyPath already augments. +// See issue https://github.com/cosmos/cosmos-sdk/issues/7627. +func TestDeriveHDPathRange(t *testing.T) { + seed := mnemonicToSeed("I am become Death, the destroyer of worlds!") + + tests := []struct { + path string + wantErr string + }{ + { + path: "m/1'/2147483648/0'/0/0", + wantErr: "out of range", + }, + { + path: "m/2147483648'/1/0/0", + wantErr: "out of range", + }, + { + path: "m/2147483648'/2147483648/0'/0/0", + wantErr: "out of range", + }, + { + path: "m/1'/-5/0'/0/0", + wantErr: "invalid syntax", + }, + { + path: "m/-2147483646'/1/0/0", + wantErr: "invalid syntax", + }, + { + path: "m/-2147483648'/-2147483648/0'/0/0", + wantErr: "invalid syntax", + }, + { + path: "m44'118'0'00", + wantErr: "path 'm44'118'0'00' doesn't contain '/' separators", + }, + { + path: "", + wantErr: "path '' doesn't contain '/' separators", + }, + { + // Should pass. + path: "m/1'/2147483647'/1/0'/0/0", + }, + { + // Should pass. + path: "1'/2147483647'/1/0'/0/0", + }, } + for _, tt := range tests { + tt := tt + t.Run(tt.path, func(t *testing.T) { + master, ch := ComputeMastersFromSeed(seed) + _, err := DerivePrivateKeyForPath(master, ch, tt.path) + + if tt.wantErr == "" { + require.NoError(t, err, "unexpected error") + } else { + require.Error(t, err, "expected a report of an int overflow") + require.Contains(t, err.Error(), tt.wantErr) + } + }) + } } -// nolint:govet -func ExampleSomeBIP32TestVecs() { +func ExampleNewParams() { + path := NewParams(44, 0, 0, false, 0) + fmt.Println(path.String()) + path = NewParams(44, 33, 7, true, 9) + fmt.Println(path.String()) + // Output: + // m/44'/0'/0'/0/0 + // m/44'/33'/7'/1/9 +} +func ExampleDerivePrivateKeyForPath() { seed := mnemonicToSeed("barrel original fuel morning among eternal " + "filter ball stove pluck matrix mechanic") master, ch := ComputeMastersFromSeed(seed) diff --git a/crypto/keys/keybase_test.go b/crypto/keys/keybase_test.go index 0c2396afbf..21a8a0bb2d 100644 --- a/crypto/keys/keybase_test.go +++ b/crypto/keys/keybase_test.go @@ -104,7 +104,7 @@ func TestCreateLedger(t *testing.T) { path, err := restoredKey.GetPath() assert.NoError(t, err) - assert.Equal(t, "44'/118'/3'/0/1", path.String()) + assert.Equal(t, "m/44'/118'/3'/0/1", path.String()) } // TestKeyManagement makes sure we can manipulate these keys well @@ -225,7 +225,7 @@ func TestSignVerify(t *testing.T) { // Import a public key armor, err := cstore.ExportPubKey(n2) require.Nil(t, err) - cstore.ImportPubKey(n3, armor) + require.NoError(t, cstore.ImportPubKey(n3, armor)) i3, err := cstore.Get(n3) require.NoError(t, err) require.Equal(t, i3.GetName(), n3) @@ -435,6 +435,29 @@ func TestSeedPhrase(t *testing.T) { require.Equal(t, info.GetPubKey(), newInfo.GetPubKey()) } +func TestCreateHDPath(t *testing.T) { + type args struct { + account uint32 + index uint32 + } + tests := []struct { + name string + args args + want hd.BIP44Params + }{ + {"m/44'/0'/0'/0/0", args{0, 0}, hd.BIP44Params{CoinType: 118, Purpose: 44}}, + {"m/44'/114'/0'/0/0", args{0, 0}, hd.BIP44Params{Purpose: 44, CoinType: 118, Account: 0, AddressIndex: 0}}, + {"m/44'/114'/1'/1/0", args{1, 1}, hd.BIP44Params{Purpose: 44, CoinType: 118, Account: 1, AddressIndex: 1}}, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + tt := tt + require.Equal(t, tt.want, *CreateHDPath(tt.args.account, tt.args.index)) + }) + } +} + func ExampleNew() { // Select the encryption and storage for your cryptostore customKeyGenFunc := func(bz []byte, algo SigningAlgo) (crypto.PrivKey, error) { diff --git a/crypto/keys/types_test.go b/crypto/keys/types_test.go index 3c18f4cf10..63e1f091f0 100644 --- a/crypto/keys/types_test.go +++ b/crypto/keys/types_test.go @@ -4,7 +4,8 @@ import ( "encoding/hex" "testing" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/crypto/secp256k1" "github.com/cosmos/cosmos-sdk/crypto/keys/hd" @@ -17,28 +18,27 @@ func Test_writeReadLedgerInfo(t *testing.T) { copy(tmpKey[:], bz) lInfo := newLedgerInfo("some_name", tmpKey, *hd.NewFundraiserParams(5, sdk.CoinType, 1), Secp256k1) - assert.Equal(t, TypeLedger, lInfo.GetType()) + require.Equal(t, TypeLedger, lInfo.GetType()) path, err := lInfo.GetPath() - assert.NoError(t, err) - assert.Equal(t, "44'/118'/5'/0/1", path.String()) - assert.Equal(t, + require.NoError(t, err) + require.Equal(t, "m/44'/118'/5'/0/1", path.String()) + require.Equal(t, "cosmospub1addwnpepqddddqg2glc8x4fl7vxjlnr7p5a3czm5kcdp4239sg6yqdc4rc2r5wmxv8p", sdk.MustBech32ifyPubKey(sdk.Bech32PubKeyTypeAccPub, lInfo.GetPubKey())) // Serialize and restore serialized := marshalInfo(lInfo) restoredInfo, err := unmarshalInfo(serialized) - assert.NoError(t, err) - assert.NotNil(t, restoredInfo) + require.NoError(t, err) + require.NotNil(t, restoredInfo) // Check both keys match - assert.Equal(t, lInfo.GetName(), restoredInfo.GetName()) - assert.Equal(t, lInfo.GetType(), restoredInfo.GetType()) - assert.Equal(t, lInfo.GetPubKey(), restoredInfo.GetPubKey()) + require.Equal(t, lInfo.GetName(), restoredInfo.GetName()) + require.Equal(t, lInfo.GetType(), restoredInfo.GetType()) + require.Equal(t, lInfo.GetPubKey(), restoredInfo.GetPubKey()) restoredPath, err := restoredInfo.GetPath() - assert.NoError(t, err) - - assert.Equal(t, path, restoredPath) + require.NoError(t, err) + require.Equal(t, path, restoredPath) } diff --git a/crypto/ledger_test.go b/crypto/ledger_test.go index 0f6c4572ad..0a2e4751fc 100644 --- a/crypto/ledger_test.go +++ b/crypto/ledger_test.go @@ -25,7 +25,7 @@ func TestLedgerErrorHandling(t *testing.T) { func TestPublicKeyUnsafe(t *testing.T) { path := *hd.NewFundraiserParams(0, sdk.CoinType, 0) priv, err := NewPrivKeyLedgerSecp256k1Unsafe(path) - require.Nil(t, err, "%s", err) + require.NoError(t, err) require.NotNil(t, priv) require.Equal(t, "eb5ae98721034fef9cd7c4c63588d3b03feb5281b9d232cba34d6f3d71aee59211ffbfe1fe87", @@ -63,10 +63,10 @@ func TestPublicKeyUnsafeHDPath(t *testing.T) { // Check with device for i := uint32(0); i < 10; i++ { path := *hd.NewFundraiserParams(0, sdk.CoinType, i) - fmt.Printf("Checking keys at %v\n", path) + t.Logf("Checking keys at %v\n", path) priv, err := NewPrivKeyLedgerSecp256k1Unsafe(path) - require.Nil(t, err, "%s", err) + require.NoError(t, err) require.NotNil(t, priv) // Check other methods @@ -101,7 +101,7 @@ func TestPublicKeySafe(t *testing.T) { path := *hd.NewFundraiserParams(0, sdk.CoinType, 0) priv, addr, err := NewPrivKeyLedgerSecp256k1(path, "cosmos") - require.Nil(t, err, "%s", err) + require.NoError(t, err) require.NotNil(t, priv) require.Equal(t, "eb5ae98721034fef9cd7c4c63588d3b03feb5281b9d232cba34d6f3d71aee59211ffbfe1fe87", @@ -154,10 +154,10 @@ func TestPublicKeyHDPath(t *testing.T) { // Check with device for i := uint32(0); i < 10; i++ { path := *hd.NewFundraiserParams(0, sdk.CoinType, i) - fmt.Printf("Checking keys at %v\n", path) + t.Logf("Checking keys at %s\n", path) priv, addr, err := NewPrivKeyLedgerSecp256k1(path, "cosmos") - require.Nil(t, err, "%s", err) + require.NoError(t, err) require.NotNil(t, addr) require.NotNil(t, priv) @@ -208,14 +208,14 @@ func TestSignaturesHD(t *testing.T) { msg := getFakeTx(account) path := *hd.NewFundraiserParams(account, sdk.CoinType, account/5) - fmt.Printf("Checking signature at %v --- PLEASE REVIEW AND ACCEPT IN THE DEVICE\n", path) + t.Logf("Checking signature at %v --- PLEASE REVIEW AND ACCEPT IN THE DEVICE\n", path) priv, err := NewPrivKeyLedgerSecp256k1Unsafe(path) - require.Nil(t, err, "%s", err) + require.NoError(t, err) pub := priv.PubKey() sig, err := priv.Sign(msg) - require.Nil(t, err) + require.NoError(t, err) valid := pub.VerifyBytes(msg, sig) require.True(t, valid, "Is your device using test mnemonic: %s ?", tests.TestMnemonic) @@ -226,11 +226,11 @@ func TestRealLedgerSecp256k1(t *testing.T) { msg := getFakeTx(50) path := *hd.NewFundraiserParams(0, sdk.CoinType, 0) priv, err := NewPrivKeyLedgerSecp256k1Unsafe(path) - require.Nil(t, err, "%s", err) + require.NoError(t, err) pub := priv.PubKey() sig, err := priv.Sign(msg) - require.Nil(t, err) + require.NoError(t, err) valid := pub.VerifyBytes(msg, sig) require.True(t, valid) @@ -245,7 +245,7 @@ func TestRealLedgerSecp256k1(t *testing.T) { // signing with the loaded key should match the original pubkey sig, err = priv.Sign(msg) - require.Nil(t, err) + require.NoError(t, err) valid = pub.VerifyBytes(msg, sig) require.True(t, valid) diff --git a/types/address.go b/types/address.go index dce02ec9c0..45b2a8a7f6 100644 --- a/types/address.go +++ b/types/address.go @@ -30,15 +30,15 @@ const ( // AddrLen defines a valid address length AddrLen = 20 - // Bech32PrefixAccAddr defines the Bech32 prefix of an account's address + // Bech32MainPrefix defines the main SDK Bech32 prefix of an account's address Bech32MainPrefix = "cosmos" - // Atom in https://github.com/satoshilabs/slips/blob/master/slip-0044.md + // CoinType is the ATOM coin type as defined in SLIP44 (https://github.com/satoshilabs/slips/blob/master/slip-0044.md) CoinType = 118 - // BIP44Prefix is the parts of the BIP44 HD path that are fixed by - // what we used during the fundraiser. - FullFundraiserPath = "44'/118'/0'/0/0" + // FullFundraiserPath is the parts of the BIP44 HD path that are fixed by + // what we used during the ATOM fundraiser. + FullFundraiserPath = "m/44'/118'/0'/0/0" // PrefixAccount is the prefix for account keys PrefixAccount = "acc"