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 16 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.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
19 changes: 11 additions & 8 deletions client/keys/add_ledger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,11 @@ 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()

// Prepare a keybase
kbHome := t.TempDir()
Expand All @@ -50,10 +47,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 +89,8 @@ func Test_runAddCmdLedgerWithCustomCoinType(t *testing.T) {
"PubKeySecp256k1{03028F0D5A9FD41600191CDEFDEA05E77A68DFBCE286241C0190805B9346667D07}",
pub.String())

config.SetPurpose(44)
config.SetCoinType(118)
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 +104,15 @@ func Test_runAddCmdLedger(t *testing.T) {
kbHome := t.TempDir()
cdc := moduletestutil.MakeTestEncodingConfig().Codec

addressConfig := sdk.GetAddressConfig()

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)

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

Expand Down
7 changes: 5 additions & 2 deletions client/keys/add_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,16 +221,19 @@ func Test_runAddCmdDryRun(t *testing.T) {
kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn, cdc)
require.NoError(t, err)

addressConfig := sdk.NewAddressConfig()

clientCtx := client.Context{}.
WithCodec(cdc).
WithKeyringDir(kbHome).
WithKeyring(kb).
WithAddressCodec(addresscodec.NewBech32Codec("cosmos")).
WithValidatorAddressCodec(addresscodec.NewBech32Codec("cosmosvaloper")).
WithConsensusAddressCodec(addresscodec.NewBech32Codec("cosmosvalcons"))
WithConsensusAddressCodec(addresscodec.NewBech32Codec("cosmosvalcons")).
WithAddressConfig(*addressConfig)
ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)

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

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

path := sdk.GetConfig().GetFullBIP44Path()
path := sdk.GetAddressConfig().GetFullBIP44Path()
fmt.Println(sdk.GetAddressConfig().GetFullBIP44Path())
bizk marked this conversation as resolved.
Show resolved Hide resolved
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
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.NewAddressConfig().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.NewAddressConfig().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.NewAddressConfig().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.NewAddressConfig().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.NewAddressConfig().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.NewAddressConfig().GetFullBIP44Path())
require.NoError(t, err)

require.NotEqual(t, addr1, addr2)
Expand Down
5 changes: 4 additions & 1 deletion testutil/network/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ type Config struct {
AddressCodec address.Codec // address codec
ValidatorAddressCodec runtime.ValidatorAddressCodec // validator address codec
ConsensusAddressCodec runtime.ConsensusAddressCodec // consensus address codec

AddressConfig sdk.AddressConfig
}

// DefaultConfig returns a sane default configuration suitable for nearly all
Expand Down Expand Up @@ -162,6 +164,7 @@ func DefaultConfig(factory TestFixtureFactory) Config {
AddressCodec: addresscodec.NewBech32Codec("cosmos"),
ValidatorAddressCodec: addresscodec.NewBech32Codec("cosmosvaloper"),
ConsensusAddressCodec: addresscodec.NewBech32Codec("cosmosvalcons"),
AddressConfig: *sdk.NewAddressConfig(),
}
}

Expand Down Expand Up @@ -455,7 +458,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, cfg.AddressConfig.GetFullBIP44Path())
if err != nil {
return nil, err
}
Expand Down
32 changes: 31 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,34 @@ 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,
}
}

// GetFullBIP44Path returns the BIP44Prefix.
func (config *AddressConfig) GetFullBIP44Path() string {
return fmt.Sprintf("m/%d'/%d'/0'/0/0", config.purpose, config.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
Loading
Loading