Skip to content

Commit

Permalink
fix: add unit test to reproduce e2e test failure for ScheduleIBCUpgra…
Browse files Browse the repository at this point in the history
…de, update code to expect tendermint client temporarily
  • Loading branch information
damiannolan committed Dec 5, 2024
1 parent a0f28fc commit 785dc47
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 5 deletions.
57 changes: 57 additions & 0 deletions modules/core/02-client/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
testifysuite "github.com/stretchr/testify/suite"

sdkmath "cosmossdk.io/math"
govtypesv1 "cosmossdk.io/x/gov/types/v1"
stakingtypes "cosmossdk.io/x/staking/types"
upgradetypes "cosmossdk.io/x/upgrade/types"

Expand Down Expand Up @@ -642,3 +643,59 @@ func (suite *KeeperTestSuite) TestIBCSoftwareUpgrade() {
})
}
}

func (suite *KeeperTestSuite) TestIBCScheduledUpgradeProposal() {
suite.SetupTest()

path := ibctesting.NewPath(suite.chainA, suite.chainB)
path.SetupClients()

tmClientState, ok := path.EndpointB.GetClientState().(*ibctm.ClientState)
suite.Require().True(ok)

// bump the chain id revision number for the ibc client upgrade
chainID := tmClientState.ChainId
revisionNumber := types.ParseChainID(chainID)

newChainID, err := types.SetRevisionNumber(chainID, revisionNumber+1)
suite.Require().NoError(err)
suite.Require().NotEqual(chainID, newChainID)

tmClientState.ChainId = newChainID
upgradedClientState := tmClientState.ZeroCustomFields()

msg, err := types.NewMsgIBCSoftwareUpgrade(
suite.chainA.GetSimApp().IBCKeeper.GetAuthority(),
upgradetypes.Plan{
Name: "upgrade-client",
Height: 1000,
},
upgradedClientState,
)
suite.Require().NoError(err)

proposal, err := govtypesv1.NewMsgSubmitProposal(
[]sdk.Msg{msg},
sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, govtypesv1.DefaultMinDepositTokens)),
path.EndpointA.Chain.SenderAccount.GetAddress().String(),
"metadata",
"ibc client upgrade",
"gov proposal for initialising ibc client upgrade",
govtypesv1.ProposalType_PROPOSAL_TYPE_STANDARD,
)
suite.Require().NoError(err)

res, err := suite.chainA.SendMsgs(proposal)
suite.Require().NoError(err)

proposalID, err := ibctesting.ParseProposalIDFromEvents(res.Events)
suite.Require().NoError(err)

// vote and pass proposal, trigger msg execution
err = ibctesting.VoteAndCheckProposalStatus(path.EndpointA, proposalID)
suite.Require().NoError(err)

storedPlan, err := suite.chainA.GetSimApp().UpgradeKeeper.GetUpgradePlan(suite.chainA.GetContext())
suite.Require().NoError(err)
suite.Require().Equal(msg.Plan, storedPlan)
}
17 changes: 12 additions & 5 deletions modules/core/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors"
"github.com/cosmos/ibc-go/v9/modules/core/internal/telemetry"
coretypes "github.com/cosmos/ibc-go/v9/modules/core/types"
ibctm "github.com/cosmos/ibc-go/v9/modules/light-clients/07-tendermint"
)

var (
Expand Down Expand Up @@ -122,13 +123,19 @@ func (k *Keeper) IBCSoftwareUpgrade(goCtx context.Context, msg *clienttypes.MsgI
return nil, errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "expected %s, got %s", k.GetAuthority(), msg.Signer)
}

ctx := sdk.UnwrapSDKContext(goCtx)
upgradedClientState, err := clienttypes.UnpackClientState(msg.UpgradedClientState)
if err != nil {
return nil, errorsmod.Wrapf(clienttypes.ErrInvalidClientType, "cannot unpack client state: %s", err)
// TODO: Make decision on this. Accessing GetCachedValue returns nil due to loss of data in internal sdk msg handler.
// For now we can assume tendermint client state here as that's what we expect, but this may not always be the case for other msg handlers
// which use pb.Any encoding of msg types going through msg router service hybrid handler.
// upgradedClientState, err := clienttypes.UnpackClientState(msg.UpgradedClientState)
// if err != nil {
// return nil, errorsmod.Wrapf(clienttypes.ErrInvalidClientType, "cannot unpack client state: %s", err)
// }
var upgradedClientState ibctm.ClientState
if err := k.cdc.Unmarshal(msg.UpgradedClientState.Value, &upgradedClientState); err != nil {
return nil, err
}

if err = k.ClientKeeper.ScheduleIBCSoftwareUpgrade(ctx, msg.Plan, upgradedClientState); err != nil {
if err := k.ClientKeeper.ScheduleIBCSoftwareUpgrade(goCtx, msg.Plan, &upgradedClientState); err != nil {
return nil, errorsmod.Wrap(err, "failed to schedule upgrade")
}

Expand Down

0 comments on commit 785dc47

Please sign in to comment.