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

Upgrade Client #7367

Merged
merged 34 commits into from
Oct 1, 2020
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
b2bee4c
implement upgrade module changes
AdityaSripal Sep 22, 2020
f999e7a
implement client changes
AdityaSripal Sep 22, 2020
6db562c
implement core tendermint logic
AdityaSripal Sep 22, 2020
888381f
remove assumption that new client state has same structure as old
AdityaSripal Sep 23, 2020
9d68fab
fix light client builds
AdityaSripal Sep 23, 2020
4f7de95
fix rest of build
AdityaSripal Sep 23, 2020
1b2a679
fix tendermint tests
AdityaSripal Sep 23, 2020
8b1afeb
fix all tests except MarshalJSON
AdityaSripal Sep 24, 2020
6536115
fix, marshalUpgrade fails
AdityaSripal Sep 24, 2020
020177e
Apply suggestions from code review
fedekunze Sep 24, 2020
60599af
minor updates
fedekunze Sep 24, 2020
3a84e74
update proto and validate path
fedekunze Sep 24, 2020
0abb1a0
fix MarshalJSON panic
AdityaSripal Sep 25, 2020
b5c1d71
hack my way to first passing test case
AdityaSripal Sep 28, 2020
5425dbc
add rest of upgrade test cases
AdityaSripal Sep 28, 2020
2d79550
fix plan tests
AdityaSripal Sep 28, 2020
aeb9152
add keeper tests
AdityaSripal Sep 29, 2020
ff1f0fa
fix upgrade tests
AdityaSripal Sep 29, 2020
36b9aca
add more tests
AdityaSripal Sep 29, 2020
8e22322
add upgrade path validation to ValidateSelfClient
AdityaSripal Sep 29, 2020
c164fe3
merge conflicts
AdityaSripal Sep 29, 2020
7d2bff3
validate upgradedClient
AdityaSripal Sep 29, 2020
a70b4a3
fix upgrade handler tests
AdityaSripal Sep 30, 2020
6b9a9b1
appease linter
AdityaSripal Sep 30, 2020
cdbb383
Apply suggestions from code review
AdityaSripal Sep 30, 2020
2fce2c6
change signer to string in proto
AdityaSripal Sep 30, 2020
4076d03
Merge branch 'aditya-upgrade-client' of github.com:cosmos/cosmos-sdk …
AdityaSripal Sep 30, 2020
3452f19
lint
AdityaSripal Sep 30, 2020
e29ae89
Merge branch 'master' into aditya-upgrade-client
AdityaSripal Sep 30, 2020
03bc5f1
start address @colin-axner review
AdityaSripal Sep 30, 2020
57a243c
Merge branch 'aditya-upgrade-client' of github.com:cosmos/cosmos-sdk …
AdityaSripal Sep 30, 2020
c19b808
improve test coverage
AdityaSripal Sep 30, 2020
787de74
fix abci stringer test
AdityaSripal Sep 30, 2020
17694c0
Merge branch 'master' into aditya-upgrade-client
AdityaSripal Sep 30, 2020
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ docs/modules
dist
tools-stamp
proto-tools-stamp
buf-stamp
artifacts

# Data - ideally these don't exist
Expand Down
8 changes: 8 additions & 0 deletions proto/cosmos/upgrade/v1beta1/upgrade.proto
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
syntax = "proto3";
package cosmos.upgrade.v1beta1;

import "google/protobuf/any.proto";
import "gogoproto/gogo.proto";
import "google/protobuf/timestamp.proto";

Expand Down Expand Up @@ -32,6 +33,13 @@ message Plan {
// Any application specific upgrade info to be included on-chain
// such as a git commit that validators could automatically upgrade to
string info = 4;

// IBC-enabled chains can opt-in to including the upgraded client state in its upgrade plan
// This will make the chain commit to the correct upgraded (self) client state before the upgrade occurs,
// so that connecting chains can verify that the new upgraded client is valid by verifying a proof on the
// previous version of the chain.
// This will allow IBC connections to persist smoothly across planned chain upgrades
google.protobuf.Any upgraded_client_state = 5;
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
}

// SoftwareUpgradeProposal is a gov Content type for initiating a software
Expand Down
12 changes: 12 additions & 0 deletions proto/ibc/client/client.proto
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,18 @@ message MsgUpdateClient {
string signer = 3;
}

// MsgUpgradeClient defines an sdk.Msg to upgrade an IBC client to a new client state
message MsgUpgradeClient {
// client unique identifier
string client_id = 1 [(gogoproto.moretags) = "yaml:\"client_id\""];
// upgraded client state
google.protobuf.Any client_state = 2 [(gogoproto.moretags) = "yaml:\"client_state\""];
// proof that old chain committed to new client
bytes proof_upgrade = 3 [(gogoproto.moretags) = "yaml:\"proof_upgrade\""];
// signer address
bytes signer = 4 [(gogoproto.casttype) = "github.com/cosmos/cosmos-sdk/types.AccAddress"];
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
}

// MsgSubmitMisbehaviour defines an sdk.Msg type that submits Evidence for
// light client misbehaviour.
message MsgSubmitMisbehaviour {
Expand Down
5 changes: 2 additions & 3 deletions proto/ibc/commitment/commitment.proto
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,12 @@ message KeyPath {
message Key {
option (gogoproto.goproto_getters) = false;

bytes name = 1 [(gogoproto.customname) = "name"];
KeyEncoding enc = 2 [(gogoproto.customname) = "enc"];
bytes name = 1;
KeyEncoding enc = 2;
}

// KeyEncoding defines the encoding format of a key's bytes.
enum KeyEncoding {
option (gogoproto.goproto_enum_stringer) = false;
option (gogoproto.goproto_enum_prefix) = false;

// URL encoding
Expand Down
7 changes: 5 additions & 2 deletions proto/ibc/tendermint/tendermint.proto
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,15 @@ message ClientState {
// Proof specifications used in verifying counterparty state
repeated ics23.ProofSpec proof_specs = 8 [(gogoproto.moretags) = "yaml:\"proof_specs\""];

// Path at which next upgraded client will be committed
ibc.commitment.MerklePath upgrade_path = 9 [(gogoproto.moretags) = "yaml:\"upgrade_path\""];

// This flag, when set to true, will allow governance to recover a client
// which has expired
bool allow_update_after_expiry = 9 [(gogoproto.moretags) = "yaml:\"allow_update_after_expiry\""];
bool allow_update_after_expiry = 10 [(gogoproto.moretags) = "yaml:\"allow_update_after_expiry\""];
// This flag, when set to true, will allow governance to unfreeze a client
// whose chain has experienced a misbehaviour event
bool allow_update_after_misbehaviour = 10 [(gogoproto.moretags) = "yaml:\"allow_update_after_misbehaviour\""];
bool allow_update_after_misbehaviour = 11 [(gogoproto.moretags) = "yaml:\"allow_update_after_misbehaviour\""];
}

// ConsensusState defines the consensus state from Tendermint.
Expand Down
27 changes: 27 additions & 0 deletions x/ibc/02-client/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,33 @@ func HandleMsgUpdateClient(ctx sdk.Context, k keeper.Keeper, msg *types.MsgUpdat
}, nil
}

// HandleMsgUpgradeClient defines the sdk.Handler for MsgUpgradeClient
func HandleMsgUpgradeClient(ctx sdk.Context, k keeper.Keeper, msg *types.MsgUpgradeClient) (*sdk.Result, error) {
upgradedClient, err := types.UnpackClientState(msg.ClientState)
if err != nil {
return nil, err
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
}

if err := upgradedClient.Validate(); err != nil {
return nil, err
}

if err = k.UpgradeClient(ctx, msg.ClientId, upgradedClient, msg.ProofUpgrade); err != nil {
return nil, err
}

ctx.EventManager().EmitEvent(
sdk.NewEvent(
sdk.EventTypeMessage,
sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
),
)

return &sdk.Result{
Events: ctx.EventManager().Events().ToABCIEvents(),
}, nil
}

// HandleMsgSubmitMisbehaviour defines the Evidence module handler for submitting a
// light client misbehaviour.
func HandleMsgSubmitMisbehaviour(ctx sdk.Context, k keeper.Keeper, msg *types.MsgSubmitMisbehaviour) (*sdk.Result, error) {
Expand Down
137 changes: 137 additions & 0 deletions x/ibc/02-client/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ import (
client "github.com/cosmos/cosmos-sdk/x/ibc/02-client"
clienttypes "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types"
ibctmtypes "github.com/cosmos/cosmos-sdk/x/ibc/07-tendermint/types"
commitmenttypes "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment/types"
"github.com/cosmos/cosmos-sdk/x/ibc/exported"
solomachinetypes "github.com/cosmos/cosmos-sdk/x/ibc/light-clients/solomachine/types"
ibctesting "github.com/cosmos/cosmos-sdk/x/ibc/testing"
upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
)

func (suite *ClientTestSuite) TestNewClientUpdateProposalHandler() {
Expand Down Expand Up @@ -73,3 +77,136 @@ func (suite *ClientTestSuite) TestNewClientUpdateProposalHandler() {
}

}

func (suite *ClientTestSuite) TestUpgradeClient() {
var (
clientA string
upgradedClient exported.ClientState
msg *clienttypes.MsgUpgradeClient
)

upgradeHeight := clienttypes.NewHeight(1, 1)

cases := []struct {
name string
setup func()
expPass bool
}{
{
name: "successful upgrade",
setup: func() {

upgradedClient = ibctmtypes.NewClientState("newChainId", ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod+ibctesting.TrustingPeriod, ibctesting.MaxClockDrift, upgradeHeight, commitmenttypes.GetSDKSpecs(), &ibctesting.UpgradePath, false, false)
// zero custom fields and store in upgrade store
suite.chainB.App.UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), upgradedClient)

// commit upgrade store changes and update clients

suite.coordinator.CommitBlock(suite.chainB)
err := suite.coordinator.UpdateClient(suite.chainA, suite.chainB, clientA, ibctesting.Tendermint)
suite.Require().NoError(err)

cs, found := suite.chainA.App.IBCKeeper.ClientKeeper.GetClientState(suite.chainA.GetContext(), clientA)
suite.Require().True(found)

proofUpgrade, _ := suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(), cs.GetLatestHeight().GetEpochHeight())

msg, err = clienttypes.NewMsgUpgradeClient(clientA, upgradedClient, proofUpgrade, suite.chainA.SenderAccount.GetAddress())
suite.Require().NoError(err)
},
expPass: true,
},
{
name: "successful upgrade to different client type",
setup: func() {

// previous chain committed to the change
upgradedClient = ibctesting.NewSolomachine(suite.T(), suite.chainA.App.AppCodec(), clientA, "diversifier", 1).ClientState()
soloClient, _ := upgradedClient.(*solomachinetypes.ClientState)
// change sequence to be higher height than latest current client height
soloClient.Sequence = 100000000000
// zero custom fields and store in upgrade store
suite.chainB.App.UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), soloClient)

// commit upgrade store changes and update clients

suite.coordinator.CommitBlock(suite.chainB)
err := suite.coordinator.UpdateClient(suite.chainA, suite.chainB, clientA, ibctesting.Tendermint)
suite.Require().NoError(err)

cs, found := suite.chainA.App.IBCKeeper.ClientKeeper.GetClientState(suite.chainA.GetContext(), clientA)
suite.Require().True(found)

proofUpgrade, _ := suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(), cs.GetLatestHeight().GetEpochHeight())

msg, err = clienttypes.NewMsgUpgradeClient(clientA, upgradedClient, proofUpgrade, suite.chainA.SenderAccount.GetAddress())
suite.Require().NoError(err)
},
expPass: true,
},

{
name: "invalid clientstate",
setup: func() {

upgradedClient = ibctmtypes.NewClientState("", ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod+ibctesting.TrustingPeriod, ibctesting.MaxClockDrift, upgradeHeight, commitmenttypes.GetSDKSpecs(), &ibctesting.UpgradePath, false, false)
// zero custom fields and store in upgrade store
suite.chainB.App.UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), upgradedClient)

// commit upgrade store changes and update clients

suite.coordinator.CommitBlock(suite.chainB)
err := suite.coordinator.UpdateClient(suite.chainA, suite.chainB, clientA, ibctesting.Tendermint)
suite.Require().NoError(err)

cs, found := suite.chainA.App.IBCKeeper.ClientKeeper.GetClientState(suite.chainA.GetContext(), clientA)
suite.Require().True(found)

proofUpgrade, _ := suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(), cs.GetLatestHeight().GetEpochHeight())

msg, err = clienttypes.NewMsgUpgradeClient(clientA, upgradedClient, proofUpgrade, suite.chainA.SenderAccount.GetAddress())
suite.Require().NoError(err)
},
expPass: false,
},
{
name: "VerifyUpgrade fails",
setup: func() {

upgradedClient = ibctmtypes.NewClientState("newChainId", ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod+ibctesting.TrustingPeriod, ibctesting.MaxClockDrift, upgradeHeight, commitmenttypes.GetSDKSpecs(), &ibctesting.UpgradePath, false, false)
// zero custom fields and store in upgrade store
suite.chainB.App.UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), upgradedClient)

// commit upgrade store changes and update clients

suite.coordinator.CommitBlock(suite.chainB)
err := suite.coordinator.UpdateClient(suite.chainA, suite.chainB, clientA, ibctesting.Tendermint)
suite.Require().NoError(err)

msg, err = clienttypes.NewMsgUpgradeClient(clientA, upgradedClient, nil, suite.chainA.SenderAccount.GetAddress())
suite.Require().NoError(err)
},
expPass: false,
},
}

for _, tc := range cases {
tc := tc
clientA, _ = suite.coordinator.SetupClients(suite.chainA, suite.chainB, ibctesting.Tendermint)

tc.setup()

_, err := client.HandleMsgUpgradeClient(
suite.chainA.GetContext(), suite.chainA.App.IBCKeeper.ClientKeeper, msg,
)

if tc.expPass {
suite.Require().NoError(err, "upgrade handler failed on valid case: %s", tc.name)
newClient, ok := suite.chainA.App.IBCKeeper.ClientKeeper.GetClientState(suite.chainA.GetContext(), clientA)
suite.Require().True(ok)
suite.Require().Equal(upgradedClient, newClient)
} else {
suite.Require().Error(err, "upgrade handler passed on invalid case: %s", tc.name)
}
}
}
37 changes: 36 additions & 1 deletion x/ibc/02-client/keeper/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.H
consensusHeight = types.GetSelfHeight(ctx)
}

k.Logger(ctx).Info(fmt.Sprintf("client %s updated height %d", clientID, consensusHeight))
k.Logger(ctx).Info("client state updated", "client-id", clientID, "height", consensusHeight)

// emitting events in the keeper emits for both begin block and handler client updates
ctx.EventManager().EmitEvent(
Expand All @@ -80,6 +80,41 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.H
return nil
}

// UpgradeClient upgrades the client to a new client state if this new client was committed to
// by the old client at the specified upgrade height
func (k Keeper) UpgradeClient(ctx sdk.Context, clientID string, upgradedClient exported.ClientState, proofUpgrade []byte) error {
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
clientState, found := k.GetClientState(ctx, clientID)
if !found {
return sdkerrors.Wrapf(types.ErrClientNotFound, "cannot update client with ID %s", clientID)
}

// prevent upgrade if current client is frozen
if clientState.IsFrozen() {
return sdkerrors.Wrapf(types.ErrClientFrozen, "cannot update client with ID %s", clientID)
}

err := clientState.VerifyUpgrade(ctx, k.cdc, k.ClientStore(ctx, clientID), upgradedClient, proofUpgrade)
if err != nil {
return sdkerrors.Wrapf(err, "cannot upgrade client with ID: %s", clientID)
}

k.SetClientState(ctx, clientID, upgradedClient)

k.Logger(ctx).Info("client state upgraded", "client-id", clientID, "height", upgradedClient.GetLatestHeight())

// emitting events in the keeper emits for both begin block and handler client updates
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
ctx.EventManager().EmitEvent(
sdk.NewEvent(
types.EventTypeUpgradeClient,
sdk.NewAttribute(types.AttributeKeyClientID, clientID),
sdk.NewAttribute(types.AttributeKeyClientType, clientState.ClientType()),
sdk.NewAttribute(types.AttributeKeyConsensusHeight, upgradedClient.GetLatestHeight().String()),
),
)

return nil
}

// CheckMisbehaviourAndUpdateState checks for client misbehaviour and freezes the
// client if so.
func (k Keeper) CheckMisbehaviourAndUpdateState(ctx sdk.Context, misbehaviour exported.Misbehaviour) error {
Expand Down
Loading