Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(types,client)!: refactor global address sdk.Config #18372

Merged
merged 21 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/keys/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ Example:
f.Bool(flagNoBackup, false, "Don't print out seed phrase (if others are watching the terminal)")
f.Bool(flags.FlagDryRun, false, "Perform action, but don't add key to local keystore")
f.String(flagHDPath, "", "Manual HD Path derivation (overrides BIP44 config)")
f.Uint32(flagCoinType, sdk.GetConfig().GetCoinType(), "coin type number for HD derivation")
f.Uint32(flagCoinType, sdk.CoinType, "coin type number for HD derivation")
f.Uint32(flagAccount, 0, "Account number for HD derivation (less than equal 2147483647)")
f.Uint32(flagIndex, 0, "Address index number for HD derivation (less than equal 2147483647)")
f.String(flags.FlagKeyType, string(hd.Secp256k1Type), "Key signing algorithm to generate keys for")
Expand Down
9 changes: 2 additions & 7 deletions client/keys/add_ledger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,10 @@ func Test_runAddCmdLedgerWithCustomCoinType(t *testing.T) {
bech32PrefixConsAddr := "terravalcons"
bech32PrefixConsPub := "terravalconspub"

config.SetPurpose(44)
config.SetCoinType(330)
config.SetBech32PrefixForAccount(bech32PrefixAccAddr, bech32PrefixAccPub)
config.SetBech32PrefixForValidator(bech32PrefixValAddr, bech32PrefixValPub)
config.SetBech32PrefixForConsensusNode(bech32PrefixConsAddr, bech32PrefixConsPub)

cmd := AddKeyCommand()
cmd.Flags().AddFlagSet(Commands().PersistentFlags())

// Prepare a keybase
kbHome := t.TempDir()

Expand All @@ -54,6 +49,8 @@ func Test_runAddCmdLedgerWithCustomCoinType(t *testing.T) {

ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)

cmd := AddKeyCommand()
cmd.Flags().AddFlagSet(Commands().PersistentFlags())
cmd.SetArgs([]string{
"keyname1",
fmt.Sprintf("--%s=true", flags.FlagUseLedger),
Expand Down Expand Up @@ -89,8 +86,6 @@ func Test_runAddCmdLedgerWithCustomCoinType(t *testing.T) {
"PubKeySecp256k1{03028F0D5A9FD41600191CDEFDEA05E77A68DFBCE286241C0190805B9346667D07}",
pub.String())

config.SetPurpose(44)
config.SetCoinType(118)
config.SetBech32PrefixForAccount(sdk.Bech32PrefixAccAddr, sdk.Bech32PrefixAccPub)
config.SetBech32PrefixForValidator(sdk.Bech32PrefixValAddr, sdk.Bech32PrefixValPub)
config.SetBech32PrefixForConsensusNode(sdk.Bech32PrefixConsAddr, sdk.Bech32PrefixConsPub)
bizk marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
2 changes: 1 addition & 1 deletion client/keys/add_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ func Test_runAddCmdDryRun(t *testing.T) {
WithConsensusAddressCodec(addresscodec.NewBech32Codec("cosmosvalcons"))
ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)

path := sdk.GetConfig().GetFullBIP44Path()
path := sdk.GetFullBIP44Path()
_, err = kb.NewAccount("subkey", testdata.TestMnemonic, "", path, hd.Secp256k1)
require.NoError(t, err)

Expand Down
2 changes: 1 addition & 1 deletion client/keys/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func Test_runDeleteCmd(t *testing.T) {
fakeKeyName1 := "runDeleteCmd_Key1"
fakeKeyName2 := "runDeleteCmd_Key2"

path := sdk.GetConfig().GetFullBIP44Path()
path := sdk.GetFullBIP44Path()
cdc := moduletestutil.MakeTestEncodingConfig().Codec

cmd.SetArgs([]string{"blah", fmt.Sprintf("--%s=%s", flags.FlagKeyringDir, kbHome)})
Expand Down
2 changes: 1 addition & 1 deletion client/keys/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func Test_runExportCmd(t *testing.T) {
require.NoError(t, err)
t.Cleanup(cleanupKeys(t, kb, "keyname1"))

path := sdk.GetConfig().GetFullBIP44Path()
path := sdk.GetFullBIP44Path()
_, err = kb.NewAccount("keyname1", testdata.TestMnemonic, "", path, hd.Secp256k1)
require.NoError(t, err)

Expand Down
2 changes: 1 addition & 1 deletion client/keys/rename_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func Test_runRenameCmd(t *testing.T) {
fakeKeyName1 := "runRenameCmd_Key1"
fakeKeyName2 := "runRenameCmd_Key2"

path := sdk.GetConfig().GetFullBIP44Path()
path := sdk.GetFullBIP44Path()

cdc := moduletestutil.MakeTestEncodingConfig().Codec
kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn, cdc)
Expand Down
4 changes: 0 additions & 4 deletions crypto/ledger/ledger_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,6 @@ func (mock LedgerSECP256K1Mock) GetPublicKeySECP256K1(derivationPath []uint32) (
return nil, errors.New("invalid derivation path")
}

if derivationPath[1] != sdk.GetConfig().GetCoinType() {
return nil, errors.New("invalid derivation path")
}

seed, err := bip39.NewSeedWithErrorChecking(testdata.TestMnemonic, "")
if err != nil {
return nil, err
Expand Down
8 changes: 0 additions & 8 deletions crypto/ledger/ledger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,6 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
)

func TestErrorHandling(t *testing.T) {
// first, try to generate a key, must return an error
// (no panic)
path := *hd.NewParams(44, 555, 0, false, 0)
_, err := NewPrivKeySecp256k1Unsafe(path)
require.Error(t, err)
}

func TestPublicKeyUnsafe(t *testing.T) {
path := *hd.NewFundraiserParams(0, sdk.CoinType, 0)
priv, err := NewPrivKeySecp256k1Unsafe(path)
Expand Down
2 changes: 1 addition & 1 deletion simapp/simd/cmd/testnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ func initTestnetFiles(
return err
}

addr, secret, err := testutil.GenerateSaveCoinKey(kb, nodeDirName, "", true, algo)
addr, secret, err := testutil.GenerateSaveCoinKey(kb, nodeDirName, "", true, algo, sdk.GetFullBIP44Path())
if err != nil {
_ = os.RemoveAll(args.outputDir)
return err
Expand Down
4 changes: 1 addition & 3 deletions tests/integration/runtime/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,7 @@ func TestQueryAppConfig(t *testing.T) {
// has all expected modules
for _, modName := range []string{"auth", "bank", "tx", "consensus", "runtime", "staking"} {
modConfig := moduleConfigs[modName]
if modConfig == nil {
t.Fatalf("missing %s", modName)
}
assert.Assert(t, modConfig != nil)
assert.Assert(t, modConfig.Config != nil)
}
}
Expand Down
7 changes: 4 additions & 3 deletions testutil/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ func GenerateCoinKey(algo keyring.SignatureAlgo, cdc codec.Codec) (sdk.AccAddres
info, secret, err := keyring.NewInMemory(cdc).NewMnemonic(
"name",
keyring.English,
sdk.GetConfig().GetFullBIP44Path(),
sdk.GetFullBIP44Path(),
keyring.DefaultBIP39Passphrase,
algo,
)
Expand All @@ -37,6 +37,7 @@ func GenerateSaveCoinKey(
keyName, mnemonic string,
overwrite bool,
algo keyring.SignatureAlgo,
hdPath string,
) (sdk.AccAddress, string, error) {
exists := false
_, err := keybase.Key(keyName)
bizk marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -63,9 +64,9 @@ func GenerateSaveCoinKey(
// generate or recover a new account
if mnemonic != "" {
secret = mnemonic
record, err = keybase.NewAccount(keyName, mnemonic, keyring.DefaultBIP39Passphrase, sdk.GetConfig().GetFullBIP44Path(), algo)
record, err = keybase.NewAccount(keyName, mnemonic, keyring.DefaultBIP39Passphrase, hdPath, algo)
} else {
record, secret, err = keybase.NewMnemonic(keyName, keyring.English, sdk.GetConfig().GetFullBIP44Path(), keyring.DefaultBIP39Passphrase, algo)
record, secret, err = keybase.NewMnemonic(keyName, keyring.English, hdPath, keyring.DefaultBIP39Passphrase, algo)
}
if err != nil {
return sdk.AccAddress{}, "", err
bizk marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
12 changes: 6 additions & 6 deletions testutil/key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func TestGenerateCoinKey(t *testing.T) {
require.NoError(t, err)

// Test creation
k, err := keyring.NewInMemory(cdc).NewAccount("xxx", mnemonic, "", hd.NewFundraiserParams(0, types.GetConfig().GetCoinType(), 0).String(), hd.Secp256k1)
k, err := keyring.NewInMemory(cdc).NewAccount("xxx", mnemonic, "", hd.NewFundraiserParams(0, types.CoinType, 0).String(), hd.Secp256k1)
require.NoError(t, err)
addr1, err := k.GetAddress()
require.NoError(t, err)
Expand All @@ -32,7 +32,7 @@ func TestGenerateSaveCoinKey(t *testing.T) {
kb, err := keyring.New(t.Name(), "test", t.TempDir(), nil, encCfg.Codec)
require.NoError(t, err)

addr, mnemonic, err := GenerateSaveCoinKey(kb, "keyname", "", false, hd.Secp256k1)
addr, mnemonic, err := GenerateSaveCoinKey(kb, "keyname", "", false, hd.Secp256k1, types.GetFullBIP44Path())
require.NoError(t, err)

// Test key was actually saved
Expand All @@ -43,7 +43,7 @@ func TestGenerateSaveCoinKey(t *testing.T) {
require.Equal(t, addr, addr1)

// Test in-memory recovery
k, err = keyring.NewInMemory(encCfg.Codec).NewAccount("xxx", mnemonic, "", hd.NewFundraiserParams(0, types.GetConfig().GetCoinType(), 0).String(), hd.Secp256k1)
k, err = keyring.NewInMemory(encCfg.Codec).NewAccount("xxx", mnemonic, "", hd.NewFundraiserParams(0, types.CoinType, 0).String(), hd.Secp256k1)
require.NoError(t, err)
addr1, err = k.GetAddress()
require.NoError(t, err)
Expand All @@ -58,15 +58,15 @@ func TestGenerateSaveCoinKeyOverwriteFlag(t *testing.T) {
require.NoError(t, err)

keyname := "justakey"
addr1, _, err := GenerateSaveCoinKey(kb, keyname, "", false, hd.Secp256k1)
addr1, _, err := GenerateSaveCoinKey(kb, keyname, "", false, hd.Secp256k1, types.GetFullBIP44Path())
require.NoError(t, err)

// Test overwrite with overwrite=false
_, _, err = GenerateSaveCoinKey(kb, keyname, "", false, hd.Secp256k1)
_, _, err = GenerateSaveCoinKey(kb, keyname, "", false, hd.Secp256k1, types.GetFullBIP44Path())
require.Error(t, err)

// Test overwrite with overwrite=true
addr2, _, err := GenerateSaveCoinKey(kb, keyname, "", true, hd.Secp256k1)
addr2, _, err := GenerateSaveCoinKey(kb, keyname, "", true, hd.Secp256k1, types.GetFullBIP44Path())
require.NoError(t, err)

require.NotEqual(t, addr1, addr2)
Expand Down
2 changes: 1 addition & 1 deletion testutil/network/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ func New(l Logger, baseDir string, cfg Config) (NetworkI, error) {
return nil, err
}

addr, secret, err := testutil.GenerateSaveCoinKey(kb, nodeDirName, mnemonic, true, algo)
addr, secret, err := testutil.GenerateSaveCoinKey(kb, nodeDirName, mnemonic, true, algo, sdk.GetFullBIP44Path())
if err != nil {
return nil, err
}
Expand Down
6 changes: 5 additions & 1 deletion types/address.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ const (
// config.SetBech32PrefixForValidator(yourBech32PrefixValAddr, yourBech32PrefixValPub)
// config.SetBech32PrefixForConsensusNode(yourBech32PrefixConsAddr, yourBech32PrefixConsPub)
// config.SetPurpose(yourPurpose)
// config.SetCoinType(yourCoinType)
// config.Seal()

// Bech32MainPrefix defines the main SDK Bech32 prefix of an account's address
Expand Down Expand Up @@ -709,3 +708,8 @@ func cacheBech32Addr(prefix string, addr []byte, cache *simplelru.LRU, cacheKey
}
return bech32Addr
bizk marked this conversation as resolved.
Show resolved Hide resolved
}

// GetFullBIP44Path returns the BIP44Prefix.
func GetFullBIP44Path() string {
return fmt.Sprintf("m/%d'/%d'/0'/0/0", Purpose, CoinType)
}
28 changes: 0 additions & 28 deletions types/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package types

import (
"context"
"fmt"
"sync"

"github.com/cosmos/cosmos-sdk/version"
Expand Down Expand Up @@ -128,18 +127,6 @@ func (config *Config) SetFullFundraiserPath(fullFundraiserPath string) {
config.fullFundraiserPath = fullFundraiserPath
}

// Set the BIP-0044 Purpose code on the config
func (config *Config) SetPurpose(purpose uint32) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you checked on sourcegraph if those were used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is being used in some chains, but as far as I can see, all of them use types.Purpose directly. GetPurpose is only used on tests

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So Get/SetPurpose is redundant, since it is never really used

config.assertNotSealed()
config.purpose = purpose
}

// Set the BIP-0044 CoinType code on the config
func (config *Config) SetCoinType(coinType uint32) {
config.assertNotSealed()
config.coinType = coinType
}

// Seal seals the config such that the config state could not be modified further
func (config *Config) Seal() *Config {
config.mtx.Lock()
bizk marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -197,28 +184,13 @@ func (config *Config) GetAddressVerifier() func([]byte) error {
return config.addressVerifier
}

// GetPurpose returns the BIP-0044 Purpose code on the config.
func (config *Config) GetPurpose() uint32 {
bizk marked this conversation as resolved.
Show resolved Hide resolved
return config.purpose
}

// GetCoinType returns the BIP-0044 CoinType code on the config.
func (config *Config) GetCoinType() uint32 {
return config.coinType
}

// GetFullFundraiserPath returns the BIP44Prefix.
//
// Deprecated: This method is supported for backward compatibility only and will be removed in a future release. Use GetFullBIP44Path instead.
func (config *Config) GetFullFundraiserPath() string {
return config.fullFundraiserPath
}

// GetFullBIP44Path returns the BIP44Prefix.
func (config *Config) GetFullBIP44Path() string {
return fmt.Sprintf("m/%d'/%d'/0'/0/0", config.purpose, config.coinType)
}

func KeyringServiceName() string {
if len(version.Name) == 0 {
return DefaultKeyringServiceName
bizk marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
23 changes: 0 additions & 23 deletions types/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,29 +17,6 @@ func TestConfigTestSuite(t *testing.T) {
suite.Run(t, new(configTestSuite))
}

func (s *contextTestSuite) TestConfig_SetPurpose() {
config := sdk.NewConfig()
config.SetPurpose(44)
s.Require().Equal(uint32(44), config.GetPurpose())

config.SetPurpose(0)
s.Require().Equal(uint32(0), config.GetPurpose())

config.Seal()
s.Require().Panics(func() { config.SetPurpose(10) })
}

func (s *configTestSuite) TestConfig_SetCoinType() {
config := sdk.NewConfig()
config.SetCoinType(1)
s.Require().Equal(uint32(1), config.GetCoinType())
config.SetCoinType(99)
s.Require().Equal(uint32(99), config.GetCoinType())

config.Seal()
s.Require().Panics(func() { config.SetCoinType(99) })
}

func (s *configTestSuite) TestConfig_SetTxEncoder() {
bizk marked this conversation as resolved.
Show resolved Hide resolved
mockErr := errors.New("test")
config := sdk.NewConfig()
bizk marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
Loading