Skip to content

Commit

Permalink
add protos, msgs, keeper handler to upgrade clients using v1 governan…
Browse files Browse the repository at this point in the history
…ce proposals (#4436)

* add protos and keeper function placeholder

* add keeper functions/tests, msgs and tests
  • Loading branch information
charleenfei authored Aug 29, 2023
1 parent 51417ee commit 35909f6
Show file tree
Hide file tree
Showing 9 changed files with 766 additions and 12 deletions.
25 changes: 25 additions & 0 deletions modules/core/02-client/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -435,3 +435,28 @@ func (k Keeper) SetParams(ctx sdk.Context, params types.Params) {
bz := k.cdc.MustMarshal(&params)
store.Set([]byte(types.ParamsKey), bz)
}

// ScheduleIBCSoftwareUpgrade schedules an upgrade for the IBC client.
func (k Keeper) ScheduleIBCSoftwareUpgrade(ctx sdk.Context, plan upgradetypes.Plan, upgradedClientState exported.ClientState) error {
// zero out any custom fields before setting
cs := upgradedClientState.ZeroCustomFields()
bz, err := types.MarshalClientState(k.cdc, cs)
if err != nil {
return errorsmod.Wrap(err, "could not marshal UpgradedClientState")
}

if err := k.upgradeKeeper.ScheduleUpgrade(ctx, plan); err != nil {
return err
}

// sets the new upgraded client last height committed on this chain at plan.Height,
// since the chain will panic at plan.Height and new chain will resume at plan.Height
if err = k.upgradeKeeper.SetUpgradedClient(ctx, plan.Height, bz); err != nil {
return err
}

// emitting an event for handling client upgrade proposal
emitUpgradeClientProposalEvent(ctx, plan.Name, plan.Height)

return nil
}
112 changes: 112 additions & 0 deletions modules/core/02-client/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import (
"github.com/cosmos/cosmos-sdk/codec"
cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"

tmbytes "github.com/cometbft/cometbft/libs/bytes"
tmproto "github.com/cometbft/cometbft/proto/tendermint/types"
Expand Down Expand Up @@ -483,3 +485,113 @@ func (suite *KeeperTestSuite) TestUnsetParams() {
suite.chainA.GetSimApp().IBCKeeper.ClientKeeper.GetParams(ctx)
})
}

// TestIBCSoftwareUpgrade tests that an IBC client upgrade has been properly scheduled
func (suite *KeeperTestSuite) TestIBCSoftwareUpgrade() {
var (
upgradedClientState *ibctm.ClientState
oldPlan, plan upgradetypes.Plan
)

testCases := []struct {
name string
malleate func()
expError error
}{
{
"valid upgrade proposal",
func() {},
nil,
},
{
"valid upgrade proposal with previous IBC state", func() {
oldPlan = upgradetypes.Plan{
Name: "upgrade IBC clients",
Height: 100,
}
},
nil,
},
{
"fail: scheduling upgrade with plan height 0",
func() {
plan.Height = 0
},
sdkerrors.ErrInvalidRequest,
},
}

for _, tc := range testCases {
tc := tc

suite.Run(tc.name, func() {
suite.SetupTest() // reset
oldPlan.Height = 0 // reset

path := ibctesting.NewPath(suite.chainA, suite.chainB)
suite.coordinator.SetupClients(path)
upgradedClientState = suite.chainA.GetClientState(path.EndpointA.ClientID).ZeroCustomFields().(*ibctm.ClientState)

// use height 1000 to distinguish from old plan
plan = upgradetypes.Plan{
Name: "upgrade IBC clients",
Height: 1000,
}

tc.malleate()

// set the old plan if it is not empty
if oldPlan.Height != 0 {
// set upgrade plan in the upgrade store
store := suite.chainA.GetContext().KVStore(suite.chainA.GetSimApp().GetKey(upgradetypes.StoreKey))
bz := suite.chainA.App.AppCodec().MustMarshal(&oldPlan)
store.Set(upgradetypes.PlanKey(), bz)

bz, err := types.MarshalClientState(suite.chainA.App.AppCodec(), upgradedClientState)
suite.Require().NoError(err)

suite.Require().NoError(suite.chainA.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainA.GetContext(), oldPlan.Height, bz))
}

err := suite.chainA.App.GetIBCKeeper().ClientKeeper.ScheduleIBCSoftwareUpgrade(suite.chainA.GetContext(), plan, upgradedClientState)

if tc.expError == nil {
suite.Require().NoError(err)

// check that the correct plan is returned
storedPlan, found := suite.chainA.GetSimApp().UpgradeKeeper.GetUpgradePlan(suite.chainA.GetContext())
suite.Require().True(found)
suite.Require().Equal(plan, storedPlan)

// check that old upgraded client state is cleared
cs, found := suite.chainA.GetSimApp().UpgradeKeeper.GetUpgradedClient(suite.chainA.GetContext(), oldPlan.Height)
suite.Require().False(found)
suite.Require().Empty(cs)

// check that client state was set
storedClientState, found := suite.chainA.GetSimApp().UpgradeKeeper.GetUpgradedClient(suite.chainA.GetContext(), plan.Height)
suite.Require().True(found)
clientState, err := types.UnmarshalClientState(suite.chainA.App.AppCodec(), storedClientState)
suite.Require().NoError(err)
suite.Require().Equal(upgradedClientState, clientState)
} else {
// check that the new plan wasn't stored
storedPlan, found := suite.chainA.GetSimApp().UpgradeKeeper.GetUpgradePlan(suite.chainA.GetContext())
if oldPlan.Height != 0 {
// NOTE: this is only true if the ScheduleUpgrade function
// returns an error before clearing the old plan
suite.Require().True(found)
suite.Require().Equal(oldPlan, storedPlan)
} else {
suite.Require().False(found)
suite.Require().Empty(storedPlan)
}

// check that client state was not set
cs, found := suite.chainA.GetSimApp().UpgradeKeeper.GetUpgradedClient(suite.chainA.GetContext(), plan.Height)
suite.Require().Empty(cs)
suite.Require().False(found)
}
})
}
}
1 change: 1 addition & 0 deletions modules/core/02-client/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
&MsgUpgradeClient{},
&MsgSubmitMisbehaviour{},
&MsgUpdateParams{},
&MsgIBCSoftwareUpgrade{},
&MsgRecoverClient{},
)

Expand Down
60 changes: 52 additions & 8 deletions modules/core/02-client/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

codectypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"

host "github.com/cosmos/ibc-go/v7/modules/core/24-host"
ibcerrors "github.com/cosmos/ibc-go/v7/modules/core/errors"
Expand All @@ -17,6 +18,7 @@ var (
_ sdk.Msg = (*MsgSubmitMisbehaviour)(nil)
_ sdk.Msg = (*MsgUpgradeClient)(nil)
_ sdk.Msg = (*MsgUpdateParams)(nil)
_ sdk.Msg = (*MsgIBCSoftwareUpgrade)(nil)
_ sdk.Msg = (*MsgRecoverClient)(nil)

_ codectypes.UnpackInterfacesMessage = (*MsgCreateClient)(nil)
Expand Down Expand Up @@ -285,24 +287,57 @@ func (msg *MsgUpdateParams) ValidateBasic() error {
return msg.Params.Validate()
}

// NewMsgRecoverClient creates a new MsgRecoverClient instance
func NewMsgRecoverClient(signer, subjectClientID, substituteClientID string) *MsgRecoverClient {
return &MsgRecoverClient{
Signer: signer,
SubjectClientId: subjectClientID,
SubstituteClientId: substituteClientID,
// NewMsgIBCSoftwareUpgrade creates a new MsgIBCSoftwareUpgrade instance
func NewMsgIBCSoftwareUpgrade(signer string, plan upgradetypes.Plan, upgradedClientState exported.ClientState) (*MsgIBCSoftwareUpgrade, error) {
anyClient, err := PackClientState(upgradedClientState)
if err != nil {
return nil, err
}

return &MsgIBCSoftwareUpgrade{
Signer: signer,
Plan: plan,
UpgradedClientState: anyClient,
}, nil
}

// GetSigners returns the expected signers for a MsgRecoverClient message.
func (msg *MsgRecoverClient) GetSigners() []sdk.AccAddress {
// ValidateBasic performs basic checks on a MsgIBCSoftwareUpgrade.
func (msg *MsgIBCSoftwareUpgrade) ValidateBasic() error {
if _, err := sdk.AccAddressFromBech32(msg.Signer); err != nil {
return errorsmod.Wrapf(ibcerrors.ErrInvalidAddress, "string could not be parsed as address: %v", err)
}

clientState, err := UnpackClientState(msg.UpgradedClientState)
if err != nil {
return err
}

// for the time being, we should implicitly be on tendermint when using ibc-go
if clientState.ClientType() != exported.Tendermint {
return errorsmod.Wrapf(ErrInvalidUpgradeClient, "upgraded client state must be a Tendermint client")
}

return msg.Plan.ValidateBasic()
}

// GetSigners returns the expected signers for a MsgIBCSoftwareUpgrade message.
func (msg *MsgIBCSoftwareUpgrade) GetSigners() []sdk.AccAddress {
accAddr, err := sdk.AccAddressFromBech32(msg.Signer)
if err != nil {
panic(err)
}
return []sdk.AccAddress{accAddr}
}

// NewMsgRecoverClient creates a new MsgRecoverClient instance
func NewMsgRecoverClient(signer, subjectClientID, substituteClientID string) *MsgRecoverClient {
return &MsgRecoverClient{
Signer: signer,
SubjectClientId: subjectClientID,
SubstituteClientId: substituteClientID,
}
}

// ValidateBasic performs basic checks on a MsgRecoverClient.
func (msg *MsgRecoverClient) ValidateBasic() error {
if _, err := sdk.AccAddressFromBech32(msg.Signer); err != nil {
Expand All @@ -315,3 +350,12 @@ func (msg *MsgRecoverClient) ValidateBasic() error {

return host.ClientIdentifierValidator(msg.SubstituteClientId)
}

// GetSigners returns the expected signers for a MsgRecoverClient message.
func (msg *MsgRecoverClient) GetSigners() []sdk.AccAddress {
accAddr, err := sdk.AccAddressFromBech32(msg.Signer)
if err != nil {
panic(err)
}
return []sdk.AccAddress{accAddr}
}
86 changes: 86 additions & 0 deletions modules/core/02-client/types/msgs_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package types_test

import (
"errors"
"testing"
"time"

Expand All @@ -9,11 +10,13 @@ import (
testifysuite "github.com/stretchr/testify/suite"

sdk "github.com/cosmos/cosmos-sdk/types"
upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"

"github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
commitmenttypes "github.com/cosmos/ibc-go/v7/modules/core/23-commitment/types"
host "github.com/cosmos/ibc-go/v7/modules/core/24-host"
ibcerrors "github.com/cosmos/ibc-go/v7/modules/core/errors"
"github.com/cosmos/ibc-go/v7/modules/core/exported"
solomachine "github.com/cosmos/ibc-go/v7/modules/light-clients/06-solomachine"
ibctm "github.com/cosmos/ibc-go/v7/modules/light-clients/07-tendermint"
ibctesting "github.com/cosmos/ibc-go/v7/testing"
Expand Down Expand Up @@ -763,3 +766,86 @@ func TestMsgRecoverClientGetSigners(t *testing.T) {
}
}
}

// TestMsgIBCSoftwareUpgrade_NewMsgIBCSoftwareUpgrade tests NewMsgIBCSoftwareUpgrade
func (suite *TypesTestSuite) TestMsgIBCSoftwareUpgrade_NewMsgIBCSoftwareUpgrade() {
testCases := []struct {
name string
upgradedClientState exported.ClientState
expPass bool
}{
{
"success",
ibctm.NewClientState(suite.chainA.ChainID, ibctesting.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath),
true,
},
{
"fail: failed to pack ClientState",
nil,
false,
},
}

for _, tc := range testCases {
plan := upgradetypes.Plan{
Name: "upgrade IBC clients",
Height: 1000,
}
msg, err := types.NewMsgIBCSoftwareUpgrade(
ibctesting.TestAccAddress,
plan,
tc.upgradedClientState,
)

if tc.expPass {
suite.Require().NoError(err)
suite.Assert().Equal(ibctesting.TestAccAddress, msg.Signer)
suite.Assert().Equal(plan, msg.Plan)
unpackedClientState, err := types.UnpackClientState(msg.UpgradedClientState)
suite.Require().NoError(err)
suite.Assert().Equal(tc.upgradedClientState, unpackedClientState)
} else {
suite.Require().True(errors.Is(err, ibcerrors.ErrPackAny))
}
}
}

// TestMsgIBCSoftwareUpgrade_GetSigners tests GetSigners for MsgIBCSoftwareUpgrade
func (suite *TypesTestSuite) TestMsgIBCSoftwareUpgrade_GetSigners() {
testCases := []struct {
name string
address sdk.AccAddress
expPass bool
}{
{
"success: valid address",
sdk.AccAddress(ibctesting.TestAccAddress),
true,
},
{
"failure: nil address",
nil,
false,
},
}

for _, tc := range testCases {
clientState := ibctm.NewClientState(suite.chainA.ChainID, ibctesting.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath)
plan := upgradetypes.Plan{
Name: "upgrade IBC clients",
Height: 1000,
}
msg, err := types.NewMsgIBCSoftwareUpgrade(
tc.address.String(),
plan,
clientState,
)
suite.Require().NoError(err)

if tc.expPass {
suite.Require().Equal([]sdk.AccAddress{tc.address}, msg.GetSigners())
} else {
suite.Require().Panics(func() { msg.GetSigners() })
}
}
}
Loading

0 comments on commit 35909f6

Please sign in to comment.