Skip to content

Commit

Permalink
chore(upgrade): remove address.string calls (#17862)
Browse files Browse the repository at this point in the history
  • Loading branch information
tac0turtle authored Sep 25, 2023
1 parent 2a29c7e commit fb3f61b
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 37 deletions.
18 changes: 14 additions & 4 deletions x/upgrade/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,10 @@ func setupTest(t *testing.T, height int64, skip map[int64]bool) *TestSuite {

s.baseApp.SetParamStore(&paramStore{params: cmtproto.ConsensusParams{Version: &cmtproto.VersionParams{App: 1}}})

s.keeper = keeper.NewKeeper(skip, storeService, s.encCfg.Codec, t.TempDir(), s.baseApp, authtypes.NewModuleAddress(govtypes.ModuleName).String())
authority, err := addresscodec.NewBech32Codec("cosmos").BytesToString(authtypes.NewModuleAddress(govtypes.ModuleName))
require.NoError(t, err)

s.keeper = keeper.NewKeeper(skip, storeService, s.encCfg.Codec, t.TempDir(), s.baseApp, authority)

s.ctx = testCtx.Ctx.WithHeaderInfo(header.Info{Time: time.Now(), Height: height})

Expand Down Expand Up @@ -460,12 +463,16 @@ func TestDowngradeVerification(t *testing.T) {
ctx := testCtx.Ctx.WithHeaderInfo(header.Info{Time: time.Now(), Height: 10})

skip := map[int64]bool{}
k := keeper.NewKeeper(skip, storeService, encCfg.Codec, t.TempDir(), nil, authtypes.NewModuleAddress(govtypes.ModuleName).String())

authority, err := addresscodec.NewBech32Codec("cosmos").BytesToString(authtypes.NewModuleAddress(govtypes.ModuleName))
require.NoError(t, err)

k := keeper.NewKeeper(skip, storeService, encCfg.Codec, t.TempDir(), nil, authority)
m := upgrade.NewAppModule(k, addresscodec.NewBech32Codec("cosmos"))

// submit a plan.
planName := "downgrade"
err := k.ScheduleUpgrade(ctx, types.Plan{Name: planName, Height: ctx.HeaderInfo().Height + 1})
err = k.ScheduleUpgrade(ctx, types.Plan{Name: planName, Height: ctx.HeaderInfo().Height + 1})
require.NoError(t, err)
ctx = ctx.WithHeaderInfo(header.Info{Height: ctx.HeaderInfo().Height + 1})

Expand Down Expand Up @@ -505,8 +512,11 @@ func TestDowngradeVerification(t *testing.T) {
for name, tc := range testCases {
ctx, _ := ctx.CacheContext()

authority, err := addresscodec.NewBech32Codec("cosmos").BytesToString(authtypes.NewModuleAddress(govtypes.ModuleName))
require.NoError(t, err)

// downgrade. now keeper does not have the handler.
k := keeper.NewKeeper(skip, storeService, encCfg.Codec, t.TempDir(), nil, authtypes.NewModuleAddress(govtypes.ModuleName).String())
k := keeper.NewKeeper(skip, storeService, encCfg.Codec, t.TempDir(), nil, authority)
m := upgrade.NewAppModule(k, addresscodec.NewBech32Codec("cosmos"))

// assertions
Expand Down
8 changes: 6 additions & 2 deletions x/upgrade/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ func NewCmdSubmitUpgradeProposal(ac addresscodec.Codec) *cobra.Command {
return fmt.Errorf("invalid authority address: %w", err)
}
} else {
authority = sdk.AccAddress(address.Module("gov")).String()
if authority, err = ac.BytesToString(address.Module("gov")); err != nil {
return fmt.Errorf("failed to convert authority address to string: %w", err)
}
}

if err := proposal.SetMsgs([]sdk.Msg{
Expand Down Expand Up @@ -159,7 +161,9 @@ func NewCmdSubmitCancelUpgradeProposal(ac addresscodec.Codec) *cobra.Command {
return fmt.Errorf("invalid authority address: %w", err)
}
} else {
authority = sdk.AccAddress(address.Module("gov")).String()
if authority, err = ac.BytesToString(address.Module("gov")); err != nil {
return fmt.Errorf("failed to convert authority address to string: %w", err)
}
}

if err := proposal.SetMsgs([]sdk.Msg{
Expand Down
20 changes: 12 additions & 8 deletions x/upgrade/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"cosmossdk.io/x/upgrade/types"

"github.com/cosmos/cosmos-sdk/baseapp"
addresscodec "github.com/cosmos/cosmos-sdk/codec/address"
"github.com/cosmos/cosmos-sdk/runtime"
"github.com/cosmos/cosmos-sdk/testutil"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand All @@ -26,10 +27,11 @@ import (
type UpgradeTestSuite struct {
suite.Suite

upgradeKeeper *keeper.Keeper
ctx sdk.Context
queryClient types.QueryClient
encCfg moduletestutil.TestEncodingConfig
upgradeKeeper *keeper.Keeper
ctx sdk.Context
queryClient types.QueryClient
encCfg moduletestutil.TestEncodingConfig
encodedAuthority string
}

func (suite *UpgradeTestSuite) SetupTest() {
Expand All @@ -40,9 +42,11 @@ func (suite *UpgradeTestSuite) SetupTest() {
suite.ctx = testCtx.Ctx

skipUpgradeHeights := make(map[int64]bool)

suite.upgradeKeeper = keeper.NewKeeper(skipUpgradeHeights, storeService, suite.encCfg.Codec, suite.T().TempDir(), nil, authtypes.NewModuleAddress(govtypes.ModuleName).String())
err := suite.upgradeKeeper.SetModuleVersionMap(suite.ctx, module.VersionMap{
authority, err := addresscodec.NewBech32Codec("cosmos").BytesToString(authtypes.NewModuleAddress(govtypes.ModuleName))
suite.Require().NoError(err)
suite.encodedAuthority = authority
suite.upgradeKeeper = keeper.NewKeeper(skipUpgradeHeights, storeService, suite.encCfg.Codec, suite.T().TempDir(), nil, authority)
err = suite.upgradeKeeper.SetModuleVersionMap(suite.ctx, module.VersionMap{
"bank": 0,
})
suite.Require().NoError(err)
Expand Down Expand Up @@ -230,7 +234,7 @@ func (suite *UpgradeTestSuite) TestModuleVersions() {
func (suite *UpgradeTestSuite) TestAuthority() {
res, err := suite.queryClient.Authority(context.Background(), &types.QueryAuthorityRequest{})
suite.Require().NoError(err)
suite.Require().Equal(authtypes.NewModuleAddress(govtypes.ModuleName).String(), res.Address)
suite.Require().Equal(suite.encodedAuthority, res.Address)
}

func TestUpgradeTestSuite(t *testing.T) {
Expand Down
30 changes: 20 additions & 10 deletions x/upgrade/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"cosmossdk.io/x/upgrade/types"

"github.com/cosmos/cosmos-sdk/baseapp"
addresscodec "github.com/cosmos/cosmos-sdk/codec/address"
"github.com/cosmos/cosmos-sdk/runtime"
"github.com/cosmos/cosmos-sdk/testutil"
simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
Expand All @@ -30,14 +31,16 @@ import (
type KeeperTestSuite struct {
suite.Suite

key *storetypes.KVStoreKey
baseApp *baseapp.BaseApp
upgradeKeeper *keeper.Keeper
homeDir string
ctx sdk.Context
msgSrvr types.MsgServer
addrs []sdk.AccAddress
encCfg moduletestutil.TestEncodingConfig
key *storetypes.KVStoreKey
baseApp *baseapp.BaseApp
upgradeKeeper *keeper.Keeper
homeDir string
ctx sdk.Context
msgSrvr types.MsgServer
addrs []sdk.AccAddress
encodedAddrs []string
encodedAuthority string
encCfg moduletestutil.TestEncodingConfig
}

func (s *KeeperTestSuite) SetupTest() {
Expand All @@ -62,14 +65,21 @@ func (s *KeeperTestSuite) SetupTest() {
skipUpgradeHeights := make(map[int64]bool)

homeDir := filepath.Join(s.T().TempDir(), "x_upgrade_keeper_test")
s.upgradeKeeper = keeper.NewKeeper(skipUpgradeHeights, storeService, s.encCfg.Codec, homeDir, s.baseApp, authtypes.NewModuleAddress(govtypes.ModuleName).String())
ac := addresscodec.NewBech32Codec("cosmos")
authority, err := ac.BytesToString(authtypes.NewModuleAddress(govtypes.ModuleName))
s.Require().NoError(err)
s.encodedAuthority = authority
s.upgradeKeeper = keeper.NewKeeper(skipUpgradeHeights, storeService, s.encCfg.Codec, homeDir, s.baseApp, authority)

s.Require().Equal(testCtx.Ctx.Logger().With("module", "x/"+types.ModuleName), s.upgradeKeeper.Logger(testCtx.Ctx))
s.T().Log("home dir:", homeDir)
s.homeDir = homeDir

s.msgSrvr = keeper.NewMsgServerImpl(s.upgradeKeeper)
s.addrs = simtestutil.CreateIncrementalAccounts(1)
encodedAddr, err := ac.BytesToString(s.addrs[0].Bytes())
s.Require().NoError(err)
s.encodedAddrs = []string{encodedAddr}
}

func (s *KeeperTestSuite) TestReadUpgradeInfoFromDisk() {
Expand Down Expand Up @@ -237,7 +247,7 @@ func (s *KeeperTestSuite) TestIsSkipHeight() {
s.Require().False(ok)
skip := map[int64]bool{skipOne: true}
storeService := runtime.NewKVStoreService(s.key)
upgradeKeeper := keeper.NewKeeper(skip, storeService, s.encCfg.Codec, s.T().TempDir(), s.baseApp, authtypes.NewModuleAddress(govtypes.ModuleName).String())
upgradeKeeper := keeper.NewKeeper(skip, storeService, s.encCfg.Codec, s.T().TempDir(), s.baseApp, s.encodedAuthority)
s.Require().True(upgradeKeeper.IsSkipHeight(9))
s.Require().False(upgradeKeeper.IsSkipHeight(10))
}
Expand Down
17 changes: 5 additions & 12 deletions x/upgrade/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,9 @@ package keeper_test

import (
"cosmossdk.io/x/upgrade/types"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/address"
)

func (s *KeeperTestSuite) TestSoftwareUpgrade() {
govAccAddr := sdk.AccAddress(address.Module("gov")).String()

testCases := []struct {
name string
req *types.MsgSoftwareUpgrade
Expand All @@ -31,7 +26,7 @@ func (s *KeeperTestSuite) TestSoftwareUpgrade() {
{
"unauthorized authority address",
&types.MsgSoftwareUpgrade{
Authority: s.addrs[0].String(),
Authority: s.encodedAddrs[0],
Plan: types.Plan{
Name: "all-good",
Info: "some text here",
Expand All @@ -44,7 +39,7 @@ func (s *KeeperTestSuite) TestSoftwareUpgrade() {
{
"invalid plan",
&types.MsgSoftwareUpgrade{
Authority: govAccAddr,
Authority: s.encodedAuthority,
Plan: types.Plan{
Height: 123450000,
},
Expand All @@ -55,7 +50,7 @@ func (s *KeeperTestSuite) TestSoftwareUpgrade() {
{
"successful upgrade scheduled",
&types.MsgSoftwareUpgrade{
Authority: govAccAddr,
Authority: s.encodedAuthority,
Plan: types.Plan{
Name: "all-good",
Info: "some text here",
Expand Down Expand Up @@ -83,8 +78,6 @@ func (s *KeeperTestSuite) TestSoftwareUpgrade() {
}

func (s *KeeperTestSuite) TestCancelUpgrade() {
govAccAddr := "cosmos10d07y265gmmuvt4z0w9aw880jnsr700j6zn9kn" // TODO
// govAccAddr := s.govKeeper.GetGovernanceAccount(s.ctx).GetAddress().String()
err := s.upgradeKeeper.ScheduleUpgrade(s.ctx, types.Plan{
Name: "some name",
Info: "some info",
Expand All @@ -109,15 +102,15 @@ func (s *KeeperTestSuite) TestCancelUpgrade() {
{
"unauthorized authority address",
&types.MsgCancelUpgrade{
Authority: s.addrs[0].String(),
Authority: s.encodedAddrs[0],
},
true,
"expected authority account as only signer for proposal message",
},
{
"upgrade canceled successfully",
&types.MsgCancelUpgrade{
Authority: govAccAddr,
Authority: s.encodedAuthority,
},
false,
"",
Expand Down
7 changes: 6 additions & 1 deletion x/upgrade/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,13 @@ func ProvideModule(in ModuleInputs) ModuleOutputs {
authority = authtypes.NewModuleAddressOrBech32Address(in.Config.Authority)
}

auth, err := in.AddressCodec.BytesToString(authority)
if err != nil {
panic(err)
}

// set the governance module account as the authority for conducting upgrades
k := keeper.NewKeeper(skipUpgradeHeights, in.StoreService, in.Cdc, homePath, in.AppVersionModifier, authority.String())
k := keeper.NewKeeper(skipUpgradeHeights, in.StoreService, in.Cdc, homePath, in.AppVersionModifier, auth)
m := NewAppModule(k, in.AddressCodec)

return ModuleOutputs{UpgradeKeeper: k, Module: m}
Expand Down

0 comments on commit fb3f61b

Please sign in to comment.