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 <emmanuel@orijtech.com>
Co-authored-by: Alessio Treglia <alessio@tendermint.com>

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`.

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 {

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

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

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()
 

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,15 +157,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,
@@ -173,26 +187,36 @@ 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:] == "'"
 		// harden == private derivation, else public derivation:
 		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