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 10 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
8 changes: 8 additions & 0 deletions client/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ type Context struct {
AddressCodec address.Codec
ValidatorAddressCodec address.Codec
ConsensusAddressCodec address.Codec

AddressConfigs sdk.AddressConfig
bizk marked this conversation as resolved.
Show resolved Hide resolved
}

// WithCmdContext returns a copy of the context with an updated context.Context,
Expand Down Expand Up @@ -333,6 +335,12 @@ func (ctx Context) WithConsensusAddressCodec(consensusAddressCodec address.Codec
return ctx
}

// WithAddressConfig returns the context with the provided address config.
func (ctx Context) WithAddressConfig(addressConfig sdk.AddressConfig) Context {
ctx.AddressConfigs = addressConfig
return ctx
}

// PrintString prints the raw string to ctx.Output if it's defined, otherwise to os.Stdout
func (ctx Context) PrintString(str string) error {
return ctx.PrintBytes([]byte(str))
Expand Down
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.GetAddressConfig().GetCoinType(), "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
24 changes: 16 additions & 8 deletions client/keys/add_ledger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,13 @@ 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())
addressConfig := sdk.GetAddressConfig()
addressConfig.SetPurpose(44)
addressConfig.SetCoinType(330)

// Prepare a keybase
kbHome := t.TempDir()
Expand All @@ -50,10 +49,13 @@ func Test_runAddCmdLedgerWithCustomCoinType(t *testing.T) {
WithCodec(cdc).
WithAddressCodec(addresscodec.NewBech32Codec("cosmos")).
WithValidatorAddressCodec(addresscodec.NewBech32Codec("cosmosvaloper")).
WithConsensusAddressCodec(addresscodec.NewBech32Codec("cosmosvalcons"))
WithConsensusAddressCodec(addresscodec.NewBech32Codec("cosmosvalcons")).
WithAddressConfig(*addressConfig)
bizk marked this conversation as resolved.
Show resolved Hide resolved

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 +91,10 @@ func Test_runAddCmdLedgerWithCustomCoinType(t *testing.T) {
"PubKeySecp256k1{03028F0D5A9FD41600191CDEFDEA05E77A68DFBCE286241C0190805B9346667D07}",
pub.String())

config.SetPurpose(44)
config.SetCoinType(118)
addressConfig.SetPurpose(44)
bizk marked this conversation as resolved.
Show resolved Hide resolved
addressConfig.SetCoinType(330)
clientCtx = clientCtx.WithAddressConfig(*addressConfig)
bizk marked this conversation as resolved.
Show resolved Hide resolved

config.SetBech32PrefixForAccount(sdk.Bech32PrefixAccAddr, sdk.Bech32PrefixAccPub)
config.SetBech32PrefixForValidator(sdk.Bech32PrefixValAddr, sdk.Bech32PrefixValPub)
config.SetBech32PrefixForConsensusNode(sdk.Bech32PrefixConsAddr, sdk.Bech32PrefixConsPub)
Expand All @@ -104,12 +108,16 @@ func Test_runAddCmdLedger(t *testing.T) {
kbHome := t.TempDir()
cdc := moduletestutil.MakeTestEncodingConfig().Codec

addressConfig := sdk.GetAddressConfig()
addressConfig.SetCoinType(sdk.CoinType)

clientCtx := client.Context{}.
WithKeyringDir(kbHome).
WithCodec(cdc).
WithAddressCodec(addresscodec.NewBech32Codec("cosmos")).
WithValidatorAddressCodec(addresscodec.NewBech32Codec("cosmosvaloper")).
WithConsensusAddressCodec(addresscodec.NewBech32Codec("cosmosvalcons"))
WithConsensusAddressCodec(addresscodec.NewBech32Codec("cosmosvalcons")).
bizk marked this conversation as resolved.
Show resolved Hide resolved
WithAddressConfig(*addressConfig)
bizk marked this conversation as resolved.
Show resolved Hide resolved

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

Expand Down
5 changes: 4 additions & 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.GetAddressConfig().GetFullBIP44Path()
_, err = kb.NewAccount("subkey", testdata.TestMnemonic, "", path, hd.Secp256k1)
require.NoError(t, err)

bizk marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -261,6 +261,9 @@ func Test_runAddCmdDryRun(t *testing.T) {
}

func TestAddRecoverFileBackend(t *testing.T) {
clientContext := client.Context{}
addressConfig := sdk.NewAddressConfig()
clientContext.WithAddressConfig(*addressConfig)
bizk marked this conversation as resolved.
Show resolved Hide resolved
cmd := AddKeyCommand()
cmd.Flags().AddFlagSet(Commands().PersistentFlags())
cdc := moduletestutil.MakeTestEncodingConfig().Codec
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.GetAddressConfig().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.GetAddressConfig().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.GetAddressConfig().GetFullBIP44Path()

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

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

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
6 changes: 3 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.NewAddressConfig().GetFullBIP44Path(),
keyring.DefaultBIP39Passphrase,
algo,
)
Expand Down Expand Up @@ -63,9 +63,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, sdk.NewAddressConfig().GetFullBIP44Path(), algo)
} else {
record, secret, err = keybase.NewMnemonic(keyName, keyring.English, sdk.GetConfig().GetFullBIP44Path(), keyring.DefaultBIP39Passphrase, algo)
record, secret, err = keybase.NewMnemonic(keyName, keyring.English, sdk.NewAddressConfig().GetFullBIP44Path(), keyring.DefaultBIP39Passphrase, algo)
}
if err != nil {
return sdk.AccAddress{}, "", err
bizk marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
4 changes: 2 additions & 2 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.GetAddressConfig().GetCoinType(), 0).String(), hd.Secp256k1)
require.NoError(t, err)
addr1, err := k.GetAddress()
require.NoError(t, err)
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.GetAddressConfig().GetCoinType(), 0).String(), hd.Secp256k1)
require.NoError(t, err)
addr1, err = k.GetAddress()
require.NoError(t, err)
Expand Down
51 changes: 51 additions & 0 deletions types/address.go
Original file line number Diff line number Diff line change
Expand Up @@ -709,3 +709,54 @@ func cacheBech32Addr(prefix string, addr []byte, cache *simplelru.LRU, cacheKey
}
return bech32Addr
bizk marked this conversation as resolved.
Show resolved Hide resolved
}

type AddressConfig struct {
coinType uint32
purpose uint32
}
bizk marked this conversation as resolved.
Show resolved Hide resolved

var (
sdkAddressConfig *AddressConfig
initAddressConfig sync.Once
)

// GetAddressConfig returns theaddres instance for the SDK.
func GetAddressConfig() *AddressConfig {
bizk marked this conversation as resolved.
Show resolved Hide resolved
initAddressConfig.Do(func() {
sdkAddressConfig = NewAddressConfig()
})
return sdkAddressConfig
bizk marked this conversation as resolved.
Show resolved Hide resolved
}

// New returns a new Config with default values.
func NewAddressConfig() *AddressConfig {
bizk marked this conversation as resolved.
Show resolved Hide resolved
return &AddressConfig{
coinType: CoinType,
purpose: Purpose,
}
}

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

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

// Set the BIP-0044 Purpose code on the config
func (config *AddressConfig) SetPurpose(purpose uint32) {
config.purpose = purpose
}

// GetPurpose returns the BIP-0044 Purpose code on the config.
func (config *AddressConfig) GetPurpose() uint32 {
return config.purpose
}

// GetFullBIP44Path returns the BIP44Prefix.
func (config *AddressConfig) GetFullBIP44Path() string {
return fmt.Sprintf("m/%d'/%d'/0'/0/0", config.purpose, config.coinType)
}
bizk marked this conversation as resolved.
Show resolved Hide resolved
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
10 changes: 2 additions & 8 deletions types/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,20 @@ func TestConfigTestSuite(t *testing.T) {
}

func (s *contextTestSuite) TestConfig_SetPurpose() {
config := sdk.NewConfig()
config := sdk.NewAddressConfig()
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 := sdk.NewAddressConfig()
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
Expand Down
Loading