crypto/hd: add 'm/' prefix to hd path (#7970)

* crypto/hd: add 'm/' prefix to hd path

* update fundraiser path

* fix some tests

* tests

* fix test case

* changelog

* fix ledger tests

CHANGELOG.md

@@ -42,6 +42,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
   * Updated iavl dependency to v0.15-rc2
 * (version) [\#7848](https://github.com/cosmos/cosmos-sdk/pull/7848) [\#7941](https://github.com/cosmos/cosmos-sdk/pull/7941) `version --long` output now shows the list of build dependencies and replaced build dependencies.
 
+### Bug Fixes
+
+* (crypto) [\#7966](https://github.com/cosmos/cosmos-sdk/issues/7966) `Bip44Params` `String()` function now correctly returns the absolute HD path by adding the `m/` prefix.
+
 ## [v0.40.0-rc3](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.40.0-rc3) - 2020-11-06
 
 ### Client Breaking

crypto/hd/fundraiser_test.go

@@ -27,7 +27,7 @@ type addrData struct {
 }
 
 func TestFullFundraiserPath(t *testing.T) {
-	require.Equal(t, "44'/118'/0'/0/0", hd.NewFundraiserParams(0, 118, 0).String())
+	require.Equal(t, "m/44'/118'/0'/0/0", hd.NewFundraiserParams(0, 118, 0).String())
 }
 
 func initFundraiserTestVectors(t *testing.T) []addrData {
@@ -63,8 +63,9 @@ func TestFundraiserCompatibility(t *testing.T) {
 		t.Logf("ROUND: %d MNEMONIC: %s", i, d.Mnemonic)
 
 		master, ch := hd.ComputeMastersFromSeed(seed)
-		priv, err := hd.DerivePrivateKeyForPath(master, ch, "44'/118'/0'/0/0")
+		priv, err := hd.DerivePrivateKeyForPath(master, ch, "m/44'/118'/0'/0/0")
 		require.NoError(t, err)
+
 		privKey := &secp256k1.PrivKey{Key: priv}
 		pub := privKey.PubKey()
 

crypto/hd/hdpath.go

@@ -36,52 +36,66 @@ 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) {
@@ -135,15 +149,16 @@ 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 {
 		changeStr = "1"
 	} 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,
@@ -165,6 +180,13 @@ func DerivePrivateKeyForPath(privKeyBytes, chainCode [32]byte, path string) ([]b
 	data := privKeyBytes
 	parts := strings.Split(path, "/")
 
+	switch {
+	case parts[0] == path:
+		return nil, 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:] == "'"
@@ -178,7 +200,7 @@ func DerivePrivateKeyForPath(privKeyBytes, chainCode [32]byte, path string) ([]b
 		// 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 []byte{}, fmt.Errorf("invalid BIP 32 path: %s", err)
+			return []byte{}, fmt.Errorf("invalid BIP 32 path %s: %w", path, err)
 		}
 
 		data, chainCode = derivePrivateKey(data, chainCode, uint32(idx), harden)
@@ -188,7 +210,7 @@ func DerivePrivateKeyForPath(privKeyBytes, chainCode [32]byte, path string) ([]b
 	n := copy(derivedKey, data[:])
 
 	if n != 32 || len(data) != 32 {
-		return []byte{}, fmt.Errorf("expected a (secp256k1) key of length 32, got length: %v", len(data))
+		return []byte{}, fmt.Errorf("expected a key of length 32, got length: %d", len(data))
 	}
 
 	return derivedKey, nil