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.

Sign up for GitHub

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

Jump to bottom

refactor(x/genutil)!: remove Address.String() #19926

Merged
merged 3 commits into from
Apr 3, 2024
Merged

refactor(x/genutil)!: remove Address.String() #19926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1e24967
update: remove Address String()
JulianToledano Apr 3, 2024
2a905a7
fix: tests
JulianToledano Apr 3, 2024
5f35a02
Update CHANGELOG.md
JulianToledano Apr 3, 2024
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
View file
Open in desktop
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) [#19926](https://github.com/cosmos/cosmos-sdk/pull/19926) Removal of the Address.String() method and related changes:
* Added an address codec as an argument to `CollectTxs`, `GenAppStateFromConfig`, and `AddGenesisAccount`.
* Removed the `ValidatorAddressCodec` argument from `CollectGenTxsCmd`, now utilizing the context for this purpose.
* Changed `ValidateAccountInGenesis` to accept 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
View file
Open in desktop
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
7 changes: 5 additions & 2 deletions simapp/simd/cmd/testnet_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ func Test_TestnetCmd(t *testing.T) {
require.Len(t, moduleManager.Modules, 7)

home := t.TempDir()
encodingConfig := moduletestutil.MakeTestEncodingConfig(codectestutil.CodecOptions{}, auth.AppModule{}, staking.AppModule{})
cdcOpts := codectestutil.CodecOptions{}
encodingConfig := moduletestutil.MakeTestEncodingConfig(cdcOpts, auth.AppModule{}, staking.AppModule{})
logger := log.NewNopLogger()
cfg, err := genutiltest.CreateDefaultCometConfig(home)
require.NoError(t, err)
Expand All @@ -59,7 +60,9 @@ func Test_TestnetCmd(t *testing.T) {
clientCtx := client.Context{}.
WithCodec(encodingConfig.Codec).
WithHomeDir(home).
WithTxConfig(encodingConfig.TxConfig)
WithTxConfig(encodingConfig.TxConfig).
WithAddressCodec(cdcOpts.GetAddressCodec()).
WithValidatorAddressCodec(cdcOpts.GetValidatorCodec())

ctx := context.Background()
ctx = context.WithValue(ctx, server.ServerContextKey, serverCtx)
Expand Down
3 changes: 2 additions & 1 deletion testutil/network/util.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading
Loading