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(x/genutil)!: remove Address.String() #19926

Merged
merged 3 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i

### API Breaking Changes

* (x/genutil) []() Removes the use of Address String method:
* `CollectTxs`, `GenAppStateFromConfig` and `AddGenesisAccount` add an address codec as argument.
* `CollectGenTxsCmd` removed ValidatorAddressCodec argument. Now it takes it from the context.
* `ValidateAccountInGenesis` now takes a string instead of an `AccAddress`.
* (server) [#19854](https://github.com/cosmos/cosmos-sdk/pull/19854) Remove `servertypes.ModuleInitFlags` types and from `server.AddCommands` as `StartCmdOptions` already achieves the same goal.
* (types) [#19792](https://github.com/cosmos/cosmos-sdk/pull/19792) In `MsgSimulatorFn` `sdk.Context` argument is replaced for an `address.Codec`. It also returns an error.
* (types) [#19742](https://github.com/cosmos/cosmos-sdk/pull/19742) Removes the use of `Accounts.String`
Expand Down
12 changes: 5 additions & 7 deletions simapp/simd/cmd/testnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"github.com/spf13/cobra"
"github.com/spf13/pflag"

"cosmossdk.io/core/address"
"cosmossdk.io/math"
"cosmossdk.io/math/unsafe"
"cosmossdk.io/simapp"
Expand Down Expand Up @@ -148,7 +147,7 @@ Example:
args.numValidators, _ = cmd.Flags().GetInt(flagNumValidators)
args.algo, _ = cmd.Flags().GetString(flags.FlagKeyType)

return initTestnetFiles(clientCtx, cmd, config, mm, genBalIterator, clientCtx.TxConfig.SigningContext().ValidatorAddressCodec(), args)
return initTestnetFiles(clientCtx, cmd, config, mm, genBalIterator, args)
},
}

Expand Down Expand Up @@ -209,7 +208,6 @@ func initTestnetFiles(
nodeConfig *cmtconfig.Config,
mm *module.Manager,
genBalIterator banktypes.GenesisBalancesIterator,
valAddrCodec address.ValidatorAddressCodec,
args initArgs,
) error {
if args.chainID == "" {
Expand Down Expand Up @@ -301,7 +299,7 @@ func initTestnetFiles(
genBalances = append(genBalances, banktypes.Balance{Address: addr.String(), Coins: coins.Sort()})
genAccounts = append(genAccounts, authtypes.NewBaseAccount(addr, nil, 0, 0))

valStr, err := valAddrCodec.BytesToString(sdk.ValAddress(addr))
valStr, err := clientCtx.ValidatorAddressCodec.BytesToString(sdk.ValAddress(addr))
if err != nil {
return err
}
Expand Down Expand Up @@ -360,7 +358,7 @@ func initTestnetFiles(

err := collectGenFiles(
clientCtx, nodeConfig, args.chainID, nodeIDs, valPubKeys, args.numValidators,
args.outputDir, args.nodeDirPrefix, args.nodeDaemonHome, genBalIterator, valAddrCodec,
args.outputDir, args.nodeDirPrefix, args.nodeDaemonHome, genBalIterator,
)
if err != nil {
return err
Expand Down Expand Up @@ -417,7 +415,7 @@ func initGenFiles(
func collectGenFiles(
clientCtx client.Context, nodeConfig *cmtconfig.Config, chainID string,
nodeIDs []string, valPubKeys []cryptotypes.PubKey, numValidators int,
outputDir, nodeDirPrefix, nodeDaemonHome string, genBalIterator banktypes.GenesisBalancesIterator, valAddrCodec address.ValidatorAddressCodec,
outputDir, nodeDirPrefix, nodeDaemonHome string, genBalIterator banktypes.GenesisBalancesIterator,
) error {
var appState json.RawMessage
genTime := cmttime.Now()
Expand All @@ -439,7 +437,7 @@ func collectGenFiles(
}

nodeAppState, err := genutil.GenAppStateFromConfig(clientCtx.Codec, clientCtx.TxConfig, nodeConfig, initCfg, appGenesis, genBalIterator, genutiltypes.DefaultMessageValidator,
valAddrCodec)
clientCtx.ValidatorAddressCodec, clientCtx.AddressCodec)
if err != nil {
return err
}
Expand Down
3 changes: 2 additions & 1 deletion testutil/network/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ func collectGenFiles(cfg Config, vals []*Validator, outputDir string) error {
}

appState, err := genutil.GenAppStateFromConfig(cfg.Codec, cfg.TxConfig,
cmtCfg, initCfg, appGenesis, banktypes.GenesisBalancesIterator{}, genutiltypes.DefaultMessageValidator, cfg.TxConfig.SigningContext().ValidatorAddressCodec())
cmtCfg, initCfg, appGenesis, banktypes.GenesisBalancesIterator{}, genutiltypes.DefaultMessageValidator,
cfg.ValidatorAddressCodec, cfg.AddressCodec)
if err != nil {
return err
}
Expand Down
5 changes: 2 additions & 3 deletions x/genutil/client/cli/collect.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (

"github.com/spf13/cobra"

"cosmossdk.io/core/address"
"cosmossdk.io/errors"

"github.com/cosmos/cosmos-sdk/client"
Expand All @@ -18,7 +17,7 @@ import (
const flagGenTxDir = "gentx-dir"

// CollectGenTxsCmd - return the cobra command to collect genesis transactions
func CollectGenTxsCmd(genBalIterator types.GenesisBalancesIterator, validator types.MessageValidator, valAddrCodec address.ValidatorAddressCodec) *cobra.Command {
func CollectGenTxsCmd(genBalIterator types.GenesisBalancesIterator, validator types.MessageValidator) *cobra.Command {
cmd := &cobra.Command{
Use: "collect-gentxs",
Short: "Collect genesis txs and output a genesis.json file",
Expand Down Expand Up @@ -48,7 +47,7 @@ func CollectGenTxsCmd(genBalIterator types.GenesisBalancesIterator, validator ty
toPrint := newPrintInfo(config.Moniker, appGenesis.ChainID, nodeID, genTxsDir, json.RawMessage(""))
initCfg := types.NewInitConfig(appGenesis.ChainID, genTxsDir, nodeID, valPubKey)

appMessage, err := genutil.GenAppStateFromConfig(cdc, clientCtx.TxConfig, config, initCfg, appGenesis, genBalIterator, validator, valAddrCodec)
appMessage, err := genutil.GenAppStateFromConfig(cdc, clientCtx.TxConfig, config, initCfg, appGenesis, genBalIterator, validator, clientCtx.ValidatorAddressCodec, clientCtx.AddressCodec)
if err != nil {
return errors.Wrap(err, "failed to get genesis app state from config")
}
Expand Down
2 changes: 1 addition & 1 deletion x/genutil/client/cli/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func CommandsWithCustomMigrationMap(txConfig client.TxConfig, mm *module.Manager
cmd.AddCommand(
GenTxCmd(mm, txConfig, banktypes.GenesisBalancesIterator{}, txConfig.SigningContext().ValidatorAddressCodec()),
MigrateGenesisCmd(migrationMap),
CollectGenTxsCmd(banktypes.GenesisBalancesIterator{}, gentxModule.GenTxValidator(), txConfig.SigningContext().ValidatorAddressCodec()),
CollectGenTxsCmd(banktypes.GenesisBalancesIterator{}, gentxModule.GenTxValidator()),
ValidateGenesisCmd(mm),
AddGenesisAccountCmd(txConfig.SigningContext().AddressCodec()),
ExportCmd(appExport),
Expand Down
2 changes: 1 addition & 1 deletion x/genutil/client/cli/genaccount.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ contain valid denominations. Accounts may optionally be supplied with vesting pa
vestingAmtStr, _ := cmd.Flags().GetString(flagVestingAmt)
moduleNameStr, _ := cmd.Flags().GetString(flagModuleName)

return genutil.AddGenesisAccount(clientCtx.Codec, addr, appendflag, config.GenesisFile(), args[1], vestingAmtStr, vestingStart, vestingEnd, moduleNameStr)
return genutil.AddGenesisAccount(clientCtx.Codec, clientCtx.AddressCodec, addr, appendflag, config.GenesisFile(), args[1], vestingAmtStr, vestingStart, vestingEnd, moduleNameStr)
},
}

Expand Down
10 changes: 7 additions & 3 deletions x/genutil/client/cli/genaccount_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ import (

func TestAddGenesisAccountCmd(t *testing.T) {
_, _, addr1 := testdata.KeyTestPubAddr()
ac := codectestutil.CodecOptions{}.GetAddressCodec()
addr1Str, err := ac.BytesToString(addr1)
require.NoError(t, err)

tests := []struct {
name string
addr string
Expand All @@ -41,14 +45,14 @@ func TestAddGenesisAccountCmd(t *testing.T) {
},
{
name: "valid address",
addr: addr1.String(),
addr: addr1Str,
denom: "1000atom",
withKeyring: false,
expectErr: false,
},
{
name: "multiple denoms",
addr: addr1.String(),
addr: addr1Str,
denom: "1000atom, 2000stake",
withKeyring: false,
expectErr: false,
Expand All @@ -75,7 +79,7 @@ func TestAddGenesisAccountCmd(t *testing.T) {
require.NoError(t, err)

serverCtx := server.NewContext(viper.New(), cfg, logger)
clientCtx := client.Context{}.WithCodec(appCodec).WithHomeDir(home)
clientCtx := client.Context{}.WithCodec(appCodec).WithHomeDir(home).WithAddressCodec(ac)

if tc.withKeyring {
path := hd.CreateHDPath(118, 0, 0).String()
Expand Down
6 changes: 5 additions & 1 deletion x/genutil/client/cli/gentx.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,11 @@ $ %s gentx my-key-name 1000000stake --home=/path/to/home/dir --keyring-backend=o
if err != nil {
return err
}
err = genutil.ValidateAccountInGenesis(genesisState, genBalIterator, addr, coins, cdc)
strAddr, err := clientCtx.AddressCodec.BytesToString(addr)
if err != nil {
return err
}
err = genutil.ValidateAccountInGenesis(genesisState, genBalIterator, strAddr, coins, cdc)
if err != nil {
return errors.Wrap(err, "failed to validate account in genesis")
}
Expand Down
13 changes: 8 additions & 5 deletions x/genutil/collect.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ import (
// GenAppStateFromConfig gets the genesis app state from the config
func GenAppStateFromConfig(cdc codec.JSONCodec, txEncodingConfig client.TxEncodingConfig,
config *cfg.Config, initCfg types.InitConfig, genesis *types.AppGenesis, genBalIterator types.GenesisBalancesIterator,
validator types.MessageValidator, valAddrCodec address.ValidatorAddressCodec,
validator types.MessageValidator, valAddrCodec address.ValidatorAddressCodec, addressCodec address.Codec,
) (appState json.RawMessage, err error) {
// process genesis transactions, else create default genesis.json
appGenTxs, persistentPeers, err := CollectTxs(
cdc, txEncodingConfig.TxJSONDecoder(), config.Moniker, initCfg.GenTxsDir, genesis, genBalIterator, validator, valAddrCodec)
cdc, txEncodingConfig.TxJSONDecoder(), config.Moniker, initCfg.GenTxsDir, genesis, genBalIterator, validator, valAddrCodec, addressCodec)
if err != nil {
return appState, err
}
Expand Down Expand Up @@ -68,7 +68,7 @@ func GenAppStateFromConfig(cdc codec.JSONCodec, txEncodingConfig client.TxEncodi
// the list of appGenTxs, and persistent peers required to generate genesis.json.
func CollectTxs(cdc codec.JSONCodec, txJSONDecoder sdk.TxDecoder, moniker, genTxsDir string,
genesis *types.AppGenesis, genBalIterator types.GenesisBalancesIterator,
validator types.MessageValidator, valAddrCodec address.ValidatorAddressCodec,
validator types.MessageValidator, valAddrCodec address.ValidatorAddressCodec, addressCodec address.Codec,
) (appGenTxs []sdk.Tx, persistentPeers string, err error) {
// prepare a map of all balances in genesis state to then validate
// against the validators addresses
Expand Down Expand Up @@ -142,7 +142,10 @@ func CollectTxs(cdc codec.JSONCodec, txJSONDecoder sdk.TxDecoder, moniker, genTx
return appGenTxs, persistentPeers, err
}

valAccAddr := sdk.AccAddress(valAddr).String()
valAccAddr, err := addressCodec.BytesToString(valAddr)
if err != nil {
return appGenTxs, persistentPeers, err
}
Comment on lines +145 to +148
Copy link
Contributor

Choose a reason for hiding this comment

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

Convert the error message to use fmt.Errorf with %w to wrap the original error, enhancing error handling by preserving the original error context.

- return appGenTxs, persistentPeers, err
+ return appGenTxs, persistentPeers, fmt.Errorf("error converting bytes to string: %w", err)

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
valAccAddr, err := addressCodec.BytesToString(valAddr)
if err != nil {
return appGenTxs, persistentPeers, err
}
valAccAddr, err := addressCodec.BytesToString(valAddr)
if err != nil {
return appGenTxs, persistentPeers, fmt.Errorf("error converting bytes to string: %w", err)
}


delBal, delOk := balancesMap[valAccAddr]
if !delOk {
Expand All @@ -158,7 +161,7 @@ func CollectTxs(cdc codec.JSONCodec, txJSONDecoder sdk.TxDecoder, moniker, genTx
if !valOk {
_, file, no, ok := runtime.Caller(1)
if ok {
fmt.Printf("CollectTxs-2, called from %s#%d - %s\n", file, no, sdk.AccAddress(msg.ValidatorAddress).String())
fmt.Printf("CollectTxs-2, called from %s#%d - %s\n", file, no, valAccAddr)
}
return appGenTxs, persistentPeers, fmt.Errorf("account %s balance not in genesis state: %+v", valAddr, balancesMap)
}
Expand Down
2 changes: 1 addition & 1 deletion x/genutil/collect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func TestCollectTxsHandlesDirectories(t *testing.T) {

dnc := &doNothingUnmarshalJSON{cdc}
if _, _, err := genutil.CollectTxs(dnc, txDecoder, "foo", testDir, genesis, balItr, types.DefaultMessageValidator,
addresscodec.NewBech32Codec("cosmosvaloper")); err != nil {
addresscodec.NewBech32Codec("cosmosvaloper"), addresscodec.NewBech32Codec("cosmos")); err != nil {
t.Fatal(err)
}
}
13 changes: 10 additions & 3 deletions x/genutil/genaccounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"

"cosmossdk.io/core/address"
authtypes "cosmossdk.io/x/auth/types"
authvesting "cosmossdk.io/x/auth/vesting/types"
banktypes "cosmossdk.io/x/bank/types"
Expand All @@ -23,12 +24,18 @@ import (
// and coins to be appended to the account already in the genesis.json file.
func AddGenesisAccount(
cdc codec.Codec,
addressCodec address.Codec,
accAddr sdk.AccAddress,
appendAcct bool,
genesisFileURL, amountStr, vestingAmtStr string,
vestingStart, vestingEnd int64,
moduleName string,
) error {
addr, err := addressCodec.BytesToString(accAddr)
if err != nil {
return err
}

coins, err := sdk.ParseCoinsNormalized(amountStr)
if err != nil {
return fmt.Errorf("failed to parse coins: %w", err)
Expand All @@ -42,7 +49,7 @@ func AddGenesisAccount(
// create concrete account type based on input parameters
var genAccount authtypes.GenesisAccount

balances := banktypes.Balance{Address: accAddr.String(), Coins: coins.Sort()}
balances := banktypes.Balance{Address: addr, Coins: coins.Sort()}
baseAccount := authtypes.NewBaseAccount(accAddr, nil, 0, 0)

if !vestingAmt.IsZero() {
Expand Down Expand Up @@ -96,12 +103,12 @@ func AddGenesisAccount(

genesisB := banktypes.GetGenesisStateFromAppState(cdc, appState)
for idx, acc := range genesisB.Balances {
if acc.Address != accAddr.String() {
if acc.Address != addr {
continue
}

updatedCoins := acc.Coins.Add(coins...)
bankGenState.Balances[idx] = banktypes.Balance{Address: accAddr.String(), Coins: updatedCoins.Sort()}
bankGenState.Balances[idx] = banktypes.Balance{Address: addr, Coins: updatedCoins.Sort()}
break
}
} else {
Expand Down
4 changes: 2 additions & 2 deletions x/genutil/gentx.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func SetGenTxsInAppGenesisState(
// balance in the set of genesis accounts.
func ValidateAccountInGenesis(
appGenesisState map[string]json.RawMessage, genBalIterator types.GenesisBalancesIterator,
addr sdk.Address, coins sdk.Coins, cdc codec.JSONCodec,
addr string, coins sdk.Coins, cdc codec.JSONCodec,
) error {
var stakingData stakingtypes.GenesisState
cdc.MustUnmarshalJSON(appGenesisState[stakingtypes.ModuleName], &stakingData)
Expand All @@ -56,7 +56,7 @@ func ValidateAccountInGenesis(
accAddress := bal.GetAddress()
accCoins := bal.GetCoins()
// ensure that account is in genesis
if strings.EqualFold(accAddress, addr.String()) {
if strings.EqualFold(accAddress, addr) {
// ensure account contains enough funds of default bond denom
if coins.AmountOf(bondDenom).GT(accCoins.AmountOf(bondDenom)) {
err = fmt.Errorf(
Expand Down
28 changes: 20 additions & 8 deletions x/genutil/gentx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ type GenTxTestSuite struct {
}

func (suite *GenTxTestSuite) SetupTest() {
valAc := codectestutil.CodecOptions{}.GetValidatorCodec()
suite.encodingConfig = moduletestutil.MakeTestEncodingConfig(codectestutil.CodecOptions{}, genutil.AppModule{})
key := storetypes.NewKVStoreKey("a_Store_Key")
tkey := storetypes.NewTransientStoreKey("a_transient_store")
Expand All @@ -67,11 +68,13 @@ func (suite *GenTxTestSuite) SetupTest() {
var err error
amount := sdk.NewInt64Coin(sdk.DefaultBondDenom, 50)
one := math.OneInt()
suite.msg1, err = stakingtypes.NewMsgCreateValidator(
sdk.ValAddress(pk1.Address()).String(), pk1, amount, desc, comm, one)
pk1Addr, err := valAc.BytesToString(pk1.Address())
suite.NoError(err)
suite.msg2, err = stakingtypes.NewMsgCreateValidator(
sdk.ValAddress(pk2.Address()).String(), pk1, amount, desc, comm, one)
suite.msg1, err = stakingtypes.NewMsgCreateValidator(pk1Addr, pk1, amount, desc, comm, one)
suite.NoError(err)
pk2Addr, err := valAc.BytesToString(pk2.Address())
suite.NoError(err)
suite.msg2, err = stakingtypes.NewMsgCreateValidator(pk2Addr, pk1, amount, desc, comm, one)
suite.NoError(err)
}

Expand Down Expand Up @@ -161,8 +164,14 @@ func (suite *GenTxTestSuite) TestValidateAccountInGenesis() {
var (
appGenesisState = make(map[string]json.RawMessage)
coins sdk.Coins
ac = codectestutil.CodecOptions{}.GetAddressCodec()
)

addr1Str, err := ac.BytesToString(addr1)
suite.Require().NoError(err)
addr2Str, err := ac.BytesToString(addr2)
suite.Require().NoError(err)

testCases := []struct {
msg string
malleate func()
Expand All @@ -180,7 +189,7 @@ func (suite *GenTxTestSuite) TestValidateAccountInGenesis() {
func() {
coins = sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 0)}
balances := banktypes.Balance{
Address: addr2.String(),
Address: addr2Str,
Coins: sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 50)},
}
appGenesisState[banktypes.ModuleName] = suite.setAccountBalance([]banktypes.Balance{balances})
Expand All @@ -192,7 +201,7 @@ func (suite *GenTxTestSuite) TestValidateAccountInGenesis() {
func() {
coins = sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 50)}
balances := banktypes.Balance{
Address: addr1.String(),
Address: addr1Str,
Coins: sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 25)},
}
appGenesisState[banktypes.ModuleName] = suite.setAccountBalance([]banktypes.Balance{balances})
Expand All @@ -204,7 +213,7 @@ func (suite *GenTxTestSuite) TestValidateAccountInGenesis() {
func() {
coins = sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 10)}
balances := banktypes.Balance{
Address: addr1.String(),
Address: addr1Str,
Coins: sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 25)},
}
appGenesisState[banktypes.ModuleName] = suite.setAccountBalance([]banktypes.Balance{balances})
Expand All @@ -221,10 +230,13 @@ func (suite *GenTxTestSuite) TestValidateAccountInGenesis() {
suite.Require().NoError(err)
appGenesisState[stakingtypes.ModuleName] = stakingGenesis

addr, err := addresscodec.NewBech32Codec("cosmos").BytesToString(addr1)
suite.Require().NoError(err)

Comment on lines +233 to +235
Copy link
Contributor

Choose a reason for hiding this comment

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

The conversion of addr1 to a string using addresscodec.NewBech32Codec("cosmos").BytesToString(addr1) seems redundant since addr1Str is already available. Consider using addr1Str directly to avoid unnecessary conversions.

- addr, err := addresscodec.NewBech32Codec("cosmos").BytesToString(addr1)
- suite.Require().NoError(err)
+ addr := addr1Str

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
addr, err := addresscodec.NewBech32Codec("cosmos").BytesToString(addr1)
suite.Require().NoError(err)
addr := addr1Str

tc.malleate()
err = genutil.ValidateAccountInGenesis(
appGenesisState, banktypes.GenesisBalancesIterator{},
addr1, coins, cdc,
addr, coins, cdc,
)

if tc.expPass {
Expand Down
Loading
Loading