Skip to content

Commit 5be42d9

Browse files
fedekunzeodeke-emAlessio Treglia
authored
crypto/hd: add 'm/' prefix to hd path (cosmos#7977)
* launchpad: backport 7970 * cherry pick 7628 * fix tests * Update CHANGELOG.md * revert tiny portion of cosmos#7970 (cosmos#7984) Testable examples were accidentally converted into tests. * adapt test to launchpad * fix golangci-lint warnings Co-authored-by: Emmanuel T Odeke <emmanuel@orijtech.com> Co-authored-by: Alessio Treglia <alessio@tendermint.com>
1 parent 69f6ec2 commit 5be42d9

File tree

11 files changed

+226
-104
lines changed

11 files changed

+226
-104
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
4848

4949
### Bug Fixes
5050

51+
* (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.
5152
* (client) [\#7588](https://github.com/cosmos/cosmos-sdk/issues/7588) Fix gov votes querier to use proper query params.
5253
* (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.
5354
* (types) [\#7084](https://github.com/cosmos/cosmos-sdk/pull/7084) Fix panic when calling `BigInt()` on an uninitialized `Int`.

client/keys/delete_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,13 @@ func Test_runDeleteCmd(t *testing.T) {
4646
if runningUnattended {
4747
mockIn.Reset("testpass1\ntestpass1\n")
4848
}
49-
_, err = kb.CreateAccount(fakeKeyName1, tests.TestMnemonic, "", "", "0", keys.Secp256k1)
49+
_, err = kb.CreateAccount(fakeKeyName1, tests.TestMnemonic, "", "", sdk.FullFundraiserPath, keys.Secp256k1)
5050
require.NoError(t, err)
5151

5252
if runningUnattended {
5353
mockIn.Reset("testpass1\ntestpass1\n")
5454
}
55-
_, err = kb.CreateAccount(fakeKeyName2, tests.TestMnemonic, "", "", "1", keys.Secp256k1)
55+
_, err = kb.CreateAccount(fakeKeyName2, tests.TestMnemonic, "", "", "m/44'/118'/0'/0/1", keys.Secp256k1)
5656
require.NoError(t, err)
5757

5858
if runningUnattended {

client/keys/show_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,13 @@ func Test_runShowCmd(t *testing.T) {
5858
if runningUnattended {
5959
mockIn.Reset("testpass1\ntestpass1\n")
6060
}
61-
_, err = kb.CreateAccount(fakeKeyName1, tests.TestMnemonic, "", "", "0", keys.Secp256k1)
61+
_, err = kb.CreateAccount(fakeKeyName1, tests.TestMnemonic, "", "", sdk.FullFundraiserPath, keys.Secp256k1)
6262
require.NoError(t, err)
6363

6464
if runningUnattended {
6565
mockIn.Reset("testpass1\n")
6666
}
67-
_, err = kb.CreateAccount(fakeKeyName2, tests.TestMnemonic, "", "", "1", keys.Secp256k1)
67+
_, err = kb.CreateAccount(fakeKeyName2, tests.TestMnemonic, "", "", "m/44'/118'/0'/0/1", keys.Secp256k1)
6868
require.NoError(t, err)
6969

7070
// Now try single key

client/keys/update_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/cosmos/cosmos-sdk/client/flags"
1010
"github.com/cosmos/cosmos-sdk/crypto/keys"
1111
"github.com/cosmos/cosmos-sdk/tests"
12+
sdk "github.com/cosmos/cosmos-sdk/types"
1213
)
1314

1415
func Test_updateKeyCommand(t *testing.T) {
@@ -39,9 +40,9 @@ func Test_runUpdateCmd(t *testing.T) {
3940

4041
kb, err := NewKeyBaseFromDir(viper.GetString(flags.FlagHome))
4142
assert.NoError(t, err)
42-
_, err = kb.CreateAccount(fakeKeyName1, tests.TestMnemonic, "", "", "0", keys.Secp256k1)
43+
_, err = kb.CreateAccount(fakeKeyName1, tests.TestMnemonic, "", "", sdk.FullFundraiserPath, keys.Secp256k1)
4344
assert.NoError(t, err)
44-
_, err = kb.CreateAccount(fakeKeyName2, tests.TestMnemonic, "", "", "1", keys.Secp256k1)
45+
_, err = kb.CreateAccount(fakeKeyName2, tests.TestMnemonic, "", "", "m/44'/118'/0'/0/1", keys.Secp256k1)
4546
assert.NoError(t, err)
4647

4748
// Try again now that we have keys

crypto/keys/hd/fundraiser_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ type addrData struct {
2424
Addr string
2525
}
2626

27+
func TestFullFundraiserPath(t *testing.T) {
28+
require.Equal(t, "m/44'/118'/0'/0/0", NewFundraiserParams(0, 118, 0).String())
29+
}
30+
2731
func initFundraiserTestVectors(t *testing.T) []addrData {
2832
// NOTE: atom fundraiser address
2933
// var hdPath string = "m/44'/118'/0'/0/0"
@@ -57,7 +61,7 @@ func TestFundraiserCompatibility(t *testing.T) {
5761
t.Logf("ROUND: %d MNEMONIC: %s", i, d.Mnemonic)
5862

5963
master, ch := ComputeMastersFromSeed(seed)
60-
priv, err := DerivePrivateKeyForPath(master, ch, "44'/118'/0'/0/0")
64+
priv, err := DerivePrivateKeyForPath(master, ch, "m/44'/118'/0'/0/0")
6165
require.NoError(t, err)
6266
pub := secp256k1.PrivKeySecp256k1(priv).PubKey()
6367

crypto/keys/hd/hdpath.go

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616
"crypto/sha512"
1717

1818
"encoding/binary"
19-
"errors"
2019
"fmt"
2120
"math/big"
2221
"strconv"
@@ -49,48 +48,62 @@ func NewParams(purpose, coinType, account uint32, change bool, addressIdx uint32
4948
}
5049
}
5150

52-
// Parse the BIP44 path and unmarshal into the struct.
51+
// NewParamsFromPath parses the BIP44 path and unmarshals it into a Bip44Params. It supports both
52+
// absolute and relative paths.
5353
func NewParamsFromPath(path string) (*BIP44Params, error) {
5454
spl := strings.Split(path, "/")
55+
56+
// Handle absolute or relative paths
57+
switch {
58+
case spl[0] == path:
59+
return nil, fmt.Errorf("path %s doesn't contain '/' separators", path)
60+
61+
case strings.TrimSpace(spl[0]) == "":
62+
return nil, fmt.Errorf("ambiguous path %s: use 'm/' prefix for absolute paths, or no leading '/' for relative ones", path)
63+
64+
case strings.TrimSpace(spl[0]) == "m":
65+
spl = spl[1:]
66+
}
67+
5568
if len(spl) != 5 {
56-
return nil, fmt.Errorf("path length is wrong. Expected 5, got %d", len(spl))
69+
return nil, fmt.Errorf("invalid path length %s", path)
5770
}
5871

5972
// Check items can be parsed
6073
purpose, err := hardenedInt(spl[0])
6174
if err != nil {
62-
return nil, err
75+
return nil, fmt.Errorf("invalid HD path purpose %s: %w", spl[0], err)
6376
}
6477
coinType, err := hardenedInt(spl[1])
6578
if err != nil {
66-
return nil, err
79+
return nil, fmt.Errorf("invalid HD path coin type %s: %w", spl[1], err)
6780
}
6881
account, err := hardenedInt(spl[2])
6982
if err != nil {
70-
return nil, err
83+
return nil, fmt.Errorf("invalid HD path account %s: %w", spl[2], err)
7184
}
7285
change, err := hardenedInt(spl[3])
7386
if err != nil {
74-
return nil, err
87+
return nil, fmt.Errorf("invalid HD path change %s: %w", spl[3], err)
7588
}
7689

7790
addressIdx, err := hardenedInt(spl[4])
7891
if err != nil {
79-
return nil, err
92+
return nil, fmt.Errorf("invalid HD path address index %s: %w", spl[4], err)
8093
}
8194

8295
// Confirm valid values
8396
if spl[0] != "44'" {
84-
return nil, fmt.Errorf("first field in path must be 44', got %v", spl[0])
97+
return nil, fmt.Errorf("first field in path must be 44', got %s", spl[0])
8598
}
8699

87100
if !isHardened(spl[1]) || !isHardened(spl[2]) {
88101
return nil,
89-
fmt.Errorf("second and third field in path must be hardened (ie. contain the suffix ', got %v and %v", spl[1], spl[2])
102+
fmt.Errorf("second and third field in path must be hardened (ie. contain the suffix ', got %s and %s", spl[1], spl[2])
90103
}
91104
if isHardened(spl[3]) || isHardened(spl[4]) {
92105
return nil,
93-
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])
106+
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])
94107
}
95108

96109
if !(change == 0 || change == 1) {
@@ -144,15 +157,16 @@ func (p BIP44Params) DerivationPath() []uint32 {
144157
}
145158
}
146159

160+
// String returns the full absolute HD path of the BIP44 (https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki) params:
161+
// m / purpose' / coin_type' / account' / change / address_index
147162
func (p BIP44Params) String() string {
148163
var changeStr string
149164
if p.Change {
150165
changeStr = "1"
151166
} else {
152167
changeStr = "0"
153168
}
154-
// m / Purpose' / coin_type' / Account' / Change / address_index
155-
return fmt.Sprintf("%d'/%d'/%d'/%s/%d",
169+
return fmt.Sprintf("m/%d'/%d'/%d'/%s/%d",
156170
p.Purpose,
157171
p.CoinType,
158172
p.Account,
@@ -173,26 +187,36 @@ func ComputeMastersFromSeed(seed []byte) (secret [32]byte, chainCode [32]byte) {
173187
func DerivePrivateKeyForPath(privKeyBytes [32]byte, chainCode [32]byte, path string) ([32]byte, error) {
174188
data := privKeyBytes
175189
parts := strings.Split(path, "/")
190+
191+
switch {
192+
case parts[0] == path:
193+
return [32]byte{}, fmt.Errorf("path '%s' doesn't contain '/' separators", path)
194+
case strings.TrimSpace(parts[0]) == "m":
195+
parts = parts[1:]
196+
}
197+
176198
for _, part := range parts {
177199
// do we have an apostrophe?
178200
harden := part[len(part)-1:] == "'"
179201
// harden == private derivation, else public derivation:
180202
if harden {
181203
part = part[:len(part)-1]
182204
}
183-
idx, err := strconv.Atoi(part)
205+
206+
// As per the extended keys specification in
207+
// https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki#extended-keys
208+
// index values are in the range [0, 1<<31-1] aka [0, max(int32)]
209+
idx, err := strconv.ParseUint(part, 10, 31)
184210
if err != nil {
185-
return [32]byte{}, fmt.Errorf("invalid BIP 32 path: %s", err)
186-
}
187-
if idx < 0 {
188-
return [32]byte{}, errors.New("invalid BIP 32 path: index negative ot too large")
211+
return [32]byte{}, fmt.Errorf("invalid BIP 32 path %s: %w", path, err)
189212
}
213+
190214
data, chainCode = derivePrivateKey(data, chainCode, uint32(idx), harden)
191215
}
192216
var derivedKey [32]byte
193217
n := copy(derivedKey[:], data[:])
194218
if n != 32 || len(data) != 32 {
195-
return [32]byte{}, fmt.Errorf("expected a (secp256k1) key of length 32, got length: %v", len(data))
219+
return [32]byte{}, fmt.Errorf("expected a key of length 32, got length: %d", len(data))
196220
}
197221

198222
return derivedKey, nil

0 commit comments

Comments
 (0)