From 27898c5fc04db06186b9a4f9dd78fb50036191db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 19 May 2021 18:35:25 +0200 Subject: [PATCH 1/9] simplify UpdateClient proposal to copy over latest consensus state --- docs/ibc/proto-docs.md | 9 +- modules/core/02-client/client/cli/tx.go | 11 +- modules/core/02-client/keeper/proposal.go | 21 ++- .../core/02-client/keeper/proposal_test.go | 23 ++- modules/core/02-client/types/client.pb.go | 152 ++++++------------ modules/core/02-client/types/proposal.go | 11 +- modules/core/exported/client.go | 2 +- .../06-solomachine/types/proposal_handle.go | 1 - .../types/proposal_handle_test.go | 2 +- .../07-tendermint/types/proposal_handle.go | 51 ++---- .../types/proposal_handle_test.go | 61 +------ .../09-localhost/types/client_state.go | 2 +- .../09-localhost/types/client_state_test.go | 2 +- proto/ibc/core/client/v1/client.proto | 13 +- 14 files changed, 110 insertions(+), 251 deletions(-) diff --git a/docs/ibc/proto-docs.md b/docs/ibc/proto-docs.md index 67c1741efed..8cbc75f8de7 100644 --- a/docs/ibc/proto-docs.md +++ b/docs/ibc/proto-docs.md @@ -476,11 +476,9 @@ client. ### ClientUpdateProposal ClientUpdateProposal is a governance proposal. If it passes, the substitute -client's consensus states starting from the 'initial height' are copied over -to the subjects client state. The proposal handler may fail if the subject -and the substitute do not match in client and chain parameters (with -exception to latest height, frozen height, and chain-id). The updated client -must also be valid (cannot be expired). +client's latest consensus state is copied over to the subject client. The proposal +handler may fail if the subject and the substitute do not match in client and +chain parameters (with exception to latest height, frozen height, and chain-id). | Field | Type | Label | Description | @@ -489,7 +487,6 @@ must also be valid (cannot be expired). | `description` | [string](#string) | | the description of the proposal | | `subject_client_id` | [string](#string) | | the client identifier for the client to be updated if the proposal passes | | `substitute_client_id` | [string](#string) | | the substitute client identifier for the client standing in for the subject client | -| `initial_height` | [Height](#ibc.core.client.v1.Height) | | the intital height to copy consensus states from the substitute to the subject | diff --git a/modules/core/02-client/client/cli/tx.go b/modules/core/02-client/client/cli/tx.go index f195f0fa3fc..1e6c21450f6 100644 --- a/modules/core/02-client/client/cli/tx.go +++ b/modules/core/02-client/client/cli/tx.go @@ -237,12 +237,12 @@ func NewUpgradeClientCmd() *cobra.Command { // NewCmdSubmitUpdateClientProposal implements a command handler for submitting an update IBC client proposal transaction. func NewCmdSubmitUpdateClientProposal() *cobra.Command { cmd := &cobra.Command{ - Use: "update-client [subject-client-id] [substitute-client-id] [initial-height] [flags]", + Use: "update-client [subject-client-id] [substitute-client-id] [flags]", Args: cobra.ExactArgs(3), Short: "Submit an update IBC client proposal", Long: "Submit an update IBC client proposal along with an initial deposit.\n" + "Please specify a subject client identifier you want to update..\n" + - "Please specify the substitute client the subject client will use and the initial height to reference the substitute client's state.", + "Please specify the substitute client the subject client will be updated to.", RunE: func(cmd *cobra.Command, args []string) error { clientCtx, err := client.GetClientTxContext(cmd) if err != nil { @@ -262,12 +262,7 @@ func NewCmdSubmitUpdateClientProposal() *cobra.Command { subjectClientID := args[0] substituteClientID := args[1] - initialHeight, err := types.ParseHeight(args[2]) - if err != nil { - return err - } - - content := types.NewClientUpdateProposal(title, description, subjectClientID, substituteClientID, initialHeight) + content := types.NewClientUpdateProposal(title, description, subjectClientID, substituteClientID) from := clientCtx.GetFromAddress() diff --git a/modules/core/02-client/keeper/proposal.go b/modules/core/02-client/keeper/proposal.go index b381b26ebd7..62a0494d7ef 100644 --- a/modules/core/02-client/keeper/proposal.go +++ b/modules/core/02-client/keeper/proposal.go @@ -11,8 +11,7 @@ import ( ) // ClientUpdateProposal will retrieve the subject and substitute client. -// The initial height must be greater than the latest height of the subject -// client. A callback will occur to the subject client state with the client +// A callback will occur to the subject client state with the client // prefixed store being provided for both the subject and the substitute client. // The localhost client is not allowed to be modified with a proposal. The IBC // client implementations are responsible for validating the parameters of the @@ -29,8 +28,10 @@ func (k Keeper) ClientUpdateProposal(ctx sdk.Context, p *types.ClientUpdatePropo return sdkerrors.Wrapf(types.ErrClientNotFound, "subject client with ID %s", p.SubjectClientId) } - if subjectClientState.GetLatestHeight().GTE(p.InitialHeight) { - return sdkerrors.Wrapf(types.ErrInvalidHeight, "subject client state latest height is greater or equal to initial height (%s >= %s)", subjectClientState.GetLatestHeight(), p.InitialHeight) + subjectClientStore := k.ClientStore(ctx, p.SubjectClientId) + + if status := subjectClientState.Status(ctx, subjectClientStore, k.cdc); status == exported.Active { + return sdkerrors.Wrap(types.ErrInvalidUpdateClientProposal, "cannot update Active subject client") } substituteClientState, found := k.GetClientState(ctx, p.SubstituteClientId) @@ -38,7 +39,17 @@ func (k Keeper) ClientUpdateProposal(ctx sdk.Context, p *types.ClientUpdatePropo return sdkerrors.Wrapf(types.ErrClientNotFound, "substitute client with ID %s", p.SubstituteClientId) } - clientState, err := subjectClientState.CheckSubstituteAndUpdateState(ctx, k.cdc, k.ClientStore(ctx, p.SubjectClientId), k.ClientStore(ctx, p.SubstituteClientId), substituteClientState, p.InitialHeight) + if subjectClientState.GetLatestHeight().GTE(substituteClientState.GetLatestHeight()) { + return sdkerrors.Wrapf(types.ErrInvalidHeight, "subject client state latest height is greater or equal to substitute client state latest height (%s >= %s)", subjectClientState.GetLatestHeight(), substituteClientState.GetLatestHeight()) + } + + substituteClientStore := k.ClientStore(ctx, p.SubstituteClientId) + + if status := substituteClientState.Status(ctx, substituteClientStore, k.cdc); status != exported.Active { + return sdkerrors.Wrapf(types.ErrClientNotActive, "substitute client is not Active, status is %s", status) + } + + clientState, err := subjectClientState.CheckSubstituteAndUpdateState(ctx, k.cdc, subjectClientStore, substituteClientStore, substituteClientState) if err != nil { return err } diff --git a/modules/core/02-client/keeper/proposal_test.go b/modules/core/02-client/keeper/proposal_test.go index cee39c3cdcc..30579447fbe 100644 --- a/modules/core/02-client/keeper/proposal_test.go +++ b/modules/core/02-client/keeper/proposal_test.go @@ -13,7 +13,6 @@ func (suite *KeeperTestSuite) TestClientUpdateProposal() { var ( subject, substitute string subjectClientState, substituteClientState exported.ClientState - initialHeight types.Height content govtypes.Content err error ) @@ -25,7 +24,7 @@ func (suite *KeeperTestSuite) TestClientUpdateProposal() { }{ { "valid update client proposal", func() { - content = types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, substitute, initialHeight) + content = types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, substitute) }, true, }, { @@ -37,31 +36,33 @@ func (suite *KeeperTestSuite) TestClientUpdateProposal() { newRevisionNumber := tmClientState.GetLatestHeight().GetRevisionNumber() + 1 tmClientState.LatestHeight = types.NewHeight(newRevisionNumber, tmClientState.GetLatestHeight().GetRevisionHeight()) - initialHeight = types.NewHeight(newRevisionNumber, initialHeight.GetRevisionHeight()) + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), substitute, tmClientState.LatestHeight, consState) + clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), substitute) + ibctmtypes.SetProcessedTime(clientStore, tmClientState.LatestHeight, 100) suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), substitute, tmClientState) - content = types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, substitute, initialHeight) + content = types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, substitute) }, true, }, { "cannot use localhost as subject", func() { - content = types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, exported.Localhost, substitute, initialHeight) + content = types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, exported.Localhost, substitute) }, false, }, { "cannot use localhost as substitute", func() { - content = types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, exported.Localhost, initialHeight) + content = types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, exported.Localhost) }, false, }, { "subject client does not exist", func() { - content = types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, ibctesting.InvalidID, substitute, initialHeight) + content = types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, ibctesting.InvalidID, substitute) }, false, }, { "substitute client does not exist", func() { - content = types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, ibctesting.InvalidID, initialHeight) + content = types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, ibctesting.InvalidID) }, false, }, { @@ -71,7 +72,7 @@ func (suite *KeeperTestSuite) TestClientUpdateProposal() { tmClientState.LatestHeight = substituteClientState.GetLatestHeight().(types.Height) suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), subject, tmClientState) - content = types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, substitute, initialHeight) + content = types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, substitute) }, false, }, { @@ -81,7 +82,7 @@ func (suite *KeeperTestSuite) TestClientUpdateProposal() { tmClientState.FrozenHeight = types.ZeroHeight() suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), subject, tmClientState) - content = types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, substitute, initialHeight) + content = types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, substitute) }, false, }, } @@ -100,7 +101,6 @@ func (suite *KeeperTestSuite) TestClientUpdateProposal() { substitutePath := ibctesting.NewPath(suite.chainA, suite.chainB) suite.coordinator.SetupClients(substitutePath) substitute = substitutePath.EndpointA.ClientID - initialHeight = types.NewHeight(subjectClientState.GetLatestHeight().GetRevisionNumber(), subjectClientState.GetLatestHeight().GetRevisionHeight()+1) // update substitute twice substitutePath.EndpointA.UpdateClient() @@ -118,7 +118,6 @@ func (suite *KeeperTestSuite) TestClientUpdateProposal() { suite.Require().True(ok) tmClientState.AllowUpdateAfterMisbehaviour = true tmClientState.AllowUpdateAfterExpiry = true - tmClientState.FrozenHeight = tmClientState.LatestHeight suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), substitute, tmClientState) tc.malleate() diff --git a/modules/core/02-client/types/client.pb.go b/modules/core/02-client/types/client.pb.go index 99f5fe17fc3..bb251b6ea80 100644 --- a/modules/core/02-client/types/client.pb.go +++ b/modules/core/02-client/types/client.pb.go @@ -194,11 +194,9 @@ func (m *ClientConsensusStates) GetConsensusStates() []ConsensusStateWithHeight } // ClientUpdateProposal is a governance proposal. If it passes, the substitute -// client's consensus states starting from the 'initial height' are copied over -// to the subjects client state. The proposal handler may fail if the subject -// and the substitute do not match in client and chain parameters (with -// exception to latest height, frozen height, and chain-id). The updated client -// must also be valid (cannot be expired). +// client's latest consensus state is copied over to the subject client. The proposal +// handler may fail if the subject and the substitute do not match in client and +// chain parameters (with exception to latest height, frozen height, and chain-id). type ClientUpdateProposal struct { // the title of the update proposal Title string `protobuf:"bytes,1,opt,name=title,proto3" json:"title,omitempty"` @@ -208,10 +206,7 @@ type ClientUpdateProposal struct { SubjectClientId string `protobuf:"bytes,3,opt,name=subject_client_id,json=subjectClientId,proto3" json:"subject_client_id,omitempty" yaml:"subject_client_id"` // the substitute client identifier for the client standing in for the subject // client - SubstituteClientId string `protobuf:"bytes,4,opt,name=substitute_client_id,json=substituteClientId,proto3" json:"substitute_client_id,omitempty" yaml:"susbtitute_client_id"` - // the intital height to copy consensus states from the substitute to the - // subject - InitialHeight Height `protobuf:"bytes,5,opt,name=initial_height,json=initialHeight,proto3" json:"initial_height" yaml:"initial_height"` + SubstituteClientId string `protobuf:"bytes,4,opt,name=substitute_client_id,json=substituteClientId,proto3" json:"substitute_client_id,omitempty" yaml:"substitute_client_id"` } func (m *ClientUpdateProposal) Reset() { *m = ClientUpdateProposal{} } @@ -402,54 +397,52 @@ func init() { func init() { proto.RegisterFile("ibc/core/client/v1/client.proto", fileDescriptor_b6bc4c8185546947) } var fileDescriptor_b6bc4c8185546947 = []byte{ - // 744 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xa4, 0x55, 0x3d, 0x73, 0xd3, 0x4a, - 0x14, 0xb5, 0x1c, 0xc7, 0x13, 0xaf, 0xf3, 0xec, 0x3c, 0xc5, 0x7e, 0x71, 0xfc, 0xf2, 0x2c, 0xcf, - 0xce, 0x2b, 0x5c, 0x10, 0x09, 0x9b, 0x81, 0x61, 0xd2, 0x61, 0x37, 0x49, 0x01, 0x18, 0x31, 0x19, - 0x18, 0x1a, 0xa3, 0x8f, 0x8d, 0xbc, 0x19, 0x59, 0xeb, 0xd1, 0xae, 0x0c, 0xfe, 0x07, 0x94, 0x94, - 0x14, 0x14, 0xf9, 0x05, 0xfc, 0x0a, 0x8a, 0x94, 0x99, 0xa1, 0xa1, 0xd2, 0x30, 0x49, 0x43, 0xad, - 0x96, 0x86, 0x91, 0x76, 0xe5, 0xd8, 0x4e, 0x02, 0x0c, 0x74, 0xab, 0xbb, 0xe7, 0x9e, 0x7b, 0xcf, - 0xd9, 0xbd, 0x2b, 0xa0, 0x60, 0xd3, 0xd2, 0x2c, 0xe2, 0x23, 0xcd, 0x72, 0x31, 0xf2, 0x98, 0x36, - 0x69, 0x8b, 0x95, 0x3a, 0xf6, 0x09, 0x23, 0xb2, 0x8c, 0x4d, 0x4b, 0x8d, 0x01, 0xaa, 0x08, 0x4f, - 0xda, 0xf5, 0x8a, 0x43, 0x1c, 0x92, 0x6c, 0x6b, 0xf1, 0x8a, 0x23, 0xeb, 0xdb, 0x0e, 0x21, 0x8e, - 0x8b, 0xb4, 0xe4, 0xcb, 0x0c, 0x8e, 0x34, 0xc3, 0x9b, 0x8a, 0xad, 0xff, 0x2d, 0x42, 0x47, 0x84, - 0x6a, 0xc1, 0xd8, 0xf1, 0x0d, 0x1b, 0x69, 0x93, 0xb6, 0x89, 0x98, 0xd1, 0x4e, 0xbf, 0x39, 0x0a, - 0xbe, 0x97, 0x40, 0xf5, 0xc0, 0x46, 0x1e, 0xc3, 0x47, 0x18, 0xd9, 0xbd, 0xa4, 0xdc, 0x53, 0x66, - 0x30, 0x24, 0xb7, 0x41, 0x81, 0x57, 0x1f, 0x60, 0xbb, 0x26, 0x35, 0xa5, 0x56, 0xa1, 0x5b, 0x89, - 0x42, 0x65, 0x63, 0x6a, 0x8c, 0xdc, 0x3d, 0x38, 0xdb, 0x82, 0xfa, 0x1a, 0x5f, 0x1f, 0xd8, 0x72, - 0x1f, 0xac, 0x8b, 0x38, 0x8d, 0x29, 0x6a, 0xd9, 0xa6, 0xd4, 0x2a, 0x76, 0x2a, 0x2a, 0x6f, 0x52, - 0x4d, 0x9b, 0x54, 0x1f, 0x78, 0xd3, 0xee, 0x56, 0x14, 0x2a, 0x9b, 0x0b, 0x5c, 0x49, 0x0e, 0xd4, - 0x8b, 0xd6, 0x65, 0x13, 0xf0, 0x83, 0x04, 0x6a, 0x3d, 0xe2, 0x51, 0xe4, 0xd1, 0x80, 0x26, 0xa1, - 0x67, 0x98, 0x0d, 0xf7, 0x11, 0x76, 0x86, 0x4c, 0xbe, 0x0f, 0xf2, 0xc3, 0x64, 0x95, 0xb4, 0x57, - 0xec, 0xd4, 0xd5, 0xab, 0xbe, 0xa9, 0x1c, 0xdb, 0xcd, 0x9d, 0x86, 0x4a, 0x46, 0x17, 0x78, 0xf9, - 0x39, 0x28, 0x5b, 0x29, 0xeb, 0x2f, 0xf4, 0xba, 0x1d, 0x85, 0x4a, 0x35, 0xee, 0x15, 0x2e, 0x65, - 0x41, 0xbd, 0x64, 0x2d, 0x74, 0x07, 0x3f, 0x4a, 0xa0, 0xca, 0x5d, 0x5c, 0x6c, 0x9b, 0xfe, 0x8e, - 0x9f, 0xaf, 0xc1, 0xc6, 0x52, 0x41, 0x5a, 0xcb, 0x36, 0x57, 0x5a, 0xc5, 0xce, 0xad, 0xeb, 0xa4, - 0xde, 0x64, 0x54, 0x57, 0x89, 0xc5, 0x47, 0xa1, 0xb2, 0x25, 0x6a, 0x2d, 0x71, 0x42, 0xbd, 0xbc, - 0xa8, 0x82, 0xc2, 0x4f, 0x59, 0x50, 0xe1, 0x32, 0x0e, 0xc7, 0xb6, 0xc1, 0x50, 0xdf, 0x27, 0x63, - 0x42, 0x0d, 0x57, 0xae, 0x80, 0x55, 0x86, 0x99, 0x8b, 0xb8, 0x02, 0x9d, 0x7f, 0xc8, 0x4d, 0x50, - 0xb4, 0x11, 0xb5, 0x7c, 0x3c, 0x66, 0x98, 0x78, 0x89, 0x97, 0x05, 0x7d, 0x3e, 0x24, 0xef, 0x83, - 0xbf, 0x69, 0x60, 0x1e, 0x23, 0x8b, 0x0d, 0x2e, 0x5d, 0x58, 0x49, 0x5c, 0xd8, 0x89, 0x42, 0xa5, - 0xc6, 0x3b, 0xbb, 0x02, 0x81, 0x7a, 0x59, 0xc4, 0x7a, 0xa9, 0x29, 0x4f, 0x40, 0x85, 0x06, 0x26, - 0x65, 0x98, 0x05, 0x0c, 0xcd, 0x91, 0xe5, 0x12, 0x32, 0x25, 0x0a, 0x95, 0x7f, 0x53, 0x32, 0x6a, - 0x2e, 0xa3, 0xa0, 0x2e, 0x5f, 0x26, 0xcf, 0x28, 0x5f, 0x82, 0x12, 0xf6, 0x30, 0xc3, 0x86, 0x3b, - 0x10, 0x17, 0x6a, 0xf5, 0xa7, 0x17, 0xea, 0x3f, 0xe1, 0x69, 0x95, 0x17, 0x5b, 0xcc, 0x87, 0xfa, - 0x5f, 0x22, 0xc0, 0xd1, 0x7b, 0xb9, 0x37, 0x27, 0x4a, 0x06, 0x7e, 0x93, 0x40, 0xf9, 0x90, 0x8f, - 0xdf, 0x1f, 0x1b, 0x7a, 0x0f, 0xe4, 0xc6, 0xae, 0xe1, 0x25, 0x1e, 0x16, 0x3b, 0x3b, 0x2a, 0x9f, - 0x76, 0x35, 0x9d, 0x6e, 0x31, 0xed, 0x6a, 0xdf, 0x35, 0x3c, 0x71, 0xf9, 0x13, 0xbc, 0x7c, 0x0c, - 0xaa, 0x02, 0x63, 0x0f, 0x16, 0x86, 0x35, 0xf7, 0x83, 0x01, 0x68, 0x46, 0xa1, 0xb2, 0xc3, 0x85, - 0x5e, 0x9b, 0x0c, 0xf5, 0xcd, 0x34, 0x3e, 0xf7, 0x84, 0xec, 0xad, 0xc7, 0xaa, 0xdf, 0x9d, 0x28, - 0x99, 0xaf, 0x27, 0x8a, 0x14, 0x3f, 0x35, 0x79, 0x31, 0xb9, 0x3d, 0x50, 0xf6, 0xd1, 0x04, 0x53, - 0x4c, 0xbc, 0x81, 0x17, 0x8c, 0x4c, 0xe4, 0x27, 0xf2, 0x73, 0xdd, 0x7a, 0x14, 0x2a, 0xff, 0xf0, - 0x42, 0x4b, 0x00, 0xa8, 0x97, 0xd2, 0xc8, 0xa3, 0x24, 0xb0, 0x40, 0x22, 0x8e, 0x2d, 0x7b, 0x23, - 0x49, 0x7a, 0x2e, 0x33, 0x12, 0x71, 0x30, 0x6b, 0x69, 0x8b, 0xf0, 0x21, 0xc8, 0xf7, 0x0d, 0xdf, - 0x18, 0xd1, 0x98, 0xd8, 0x70, 0x5d, 0xf2, 0x6a, 0x26, 0x92, 0xd6, 0xa4, 0xe6, 0x4a, 0xab, 0x30, - 0x4f, 0xbc, 0x04, 0x80, 0x7a, 0x49, 0x44, 0xb8, 0x7e, 0xda, 0x7d, 0x7c, 0x7a, 0xde, 0x90, 0xce, - 0xce, 0x1b, 0xd2, 0x97, 0xf3, 0x86, 0xf4, 0xf6, 0xa2, 0x91, 0x39, 0xbb, 0x68, 0x64, 0x3e, 0x5f, - 0x34, 0x32, 0x2f, 0xee, 0x3a, 0x98, 0x0d, 0x03, 0x53, 0xb5, 0xc8, 0x48, 0x13, 0x6f, 0x34, 0x36, - 0xad, 0x5d, 0x87, 0x68, 0x23, 0x62, 0x07, 0x2e, 0xa2, 0xfc, 0xdf, 0x70, 0xbb, 0xb3, 0x2b, 0x7e, - 0x0f, 0x6c, 0x3a, 0x46, 0xd4, 0xcc, 0x27, 0x27, 0x72, 0xe7, 0x7b, 0x00, 0x00, 0x00, 0xff, 0xff, - 0xca, 0x6e, 0xea, 0xc6, 0x3e, 0x06, 0x00, 0x00, + // 710 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xa4, 0x54, 0x3f, 0x6f, 0xd3, 0x4c, + 0x1c, 0x8e, 0xdb, 0xbc, 0x51, 0x73, 0xa9, 0x9a, 0xbe, 0x6e, 0xf2, 0x36, 0xcd, 0x5b, 0xc5, 0xd1, + 0x89, 0x21, 0x03, 0xb5, 0x49, 0x10, 0x08, 0x65, 0x23, 0x59, 0xda, 0x01, 0x08, 0x46, 0x15, 0x88, + 0x25, 0xf2, 0x9f, 0xab, 0x73, 0x95, 0xe3, 0x8b, 0x7c, 0xe7, 0x40, 0xbe, 0x01, 0x23, 0x23, 0x03, + 0x43, 0x3f, 0x01, 0x9f, 0x82, 0xa1, 0x63, 0x47, 0x26, 0x0b, 0xb5, 0x0b, 0x2b, 0x5e, 0x59, 0x90, + 0xef, 0xce, 0x6d, 0x92, 0xb6, 0x08, 0xc1, 0x76, 0xf7, 0xdc, 0x73, 0xcf, 0xef, 0x79, 0x7e, 0xf6, + 0xef, 0x80, 0x86, 0x6d, 0xc7, 0x70, 0x48, 0x88, 0x0c, 0xc7, 0xc7, 0x28, 0x60, 0xc6, 0xb4, 0x2d, + 0x57, 0xfa, 0x24, 0x24, 0x8c, 0xa8, 0x2a, 0xb6, 0x1d, 0x3d, 0x25, 0xe8, 0x12, 0x9e, 0xb6, 0xeb, + 0x15, 0x8f, 0x78, 0x84, 0x1f, 0x1b, 0xe9, 0x4a, 0x30, 0xeb, 0x3b, 0x1e, 0x21, 0x9e, 0x8f, 0x0c, + 0xbe, 0xb3, 0xa3, 0x23, 0xc3, 0x0a, 0x66, 0xf2, 0xe8, 0x8e, 0x43, 0xe8, 0x98, 0x50, 0x23, 0x9a, + 0x78, 0xa1, 0xe5, 0x22, 0x63, 0xda, 0xb6, 0x11, 0xb3, 0xda, 0xd9, 0x5e, 0xb0, 0xe0, 0x47, 0x05, + 0x54, 0x0f, 0x5c, 0x14, 0x30, 0x7c, 0x84, 0x91, 0xdb, 0xe7, 0xe5, 0x5e, 0x30, 0x8b, 0x21, 0xb5, + 0x0d, 0x8a, 0xa2, 0xfa, 0x10, 0xbb, 0x35, 0xa5, 0xa9, 0xb4, 0x8a, 0xbd, 0x4a, 0x12, 0x6b, 0x9b, + 0x33, 0x6b, 0xec, 0x77, 0xe1, 0xe5, 0x11, 0x34, 0xd7, 0xc4, 0xfa, 0xc0, 0x55, 0x07, 0x60, 0x5d, + 0xe2, 0x34, 0x95, 0xa8, 0xad, 0x34, 0x95, 0x56, 0xa9, 0x53, 0xd1, 0x85, 0x49, 0x3d, 0x33, 0xa9, + 0x3f, 0x0e, 0x66, 0xbd, 0xed, 0x24, 0xd6, 0xb6, 0x16, 0xb4, 0xf8, 0x1d, 0x68, 0x96, 0x9c, 0x2b, + 0x13, 0xf0, 0x93, 0x02, 0x6a, 0x7d, 0x12, 0x50, 0x14, 0xd0, 0x88, 0x72, 0xe8, 0x25, 0x66, 0xa3, + 0x7d, 0x84, 0xbd, 0x11, 0x53, 0x1f, 0x81, 0xc2, 0x88, 0xaf, 0xb8, 0xbd, 0x52, 0xa7, 0xae, 0x5f, + 0xef, 0x9b, 0x2e, 0xb8, 0xbd, 0xfc, 0x69, 0xac, 0xe5, 0x4c, 0xc9, 0x57, 0x5f, 0x81, 0xb2, 0x93, + 0xa9, 0xfe, 0x86, 0xd7, 0x9d, 0x24, 0xd6, 0xaa, 0xa9, 0x57, 0xb8, 0x74, 0x0b, 0x9a, 0x1b, 0xce, + 0x82, 0x3b, 0xf8, 0x59, 0x01, 0x55, 0xd1, 0xc5, 0x45, 0xdb, 0xf4, 0x4f, 0xfa, 0xf9, 0x16, 0x6c, + 0x2e, 0x15, 0xa4, 0xb5, 0x95, 0xe6, 0x6a, 0xab, 0xd4, 0xb9, 0x7b, 0x53, 0xd4, 0xdb, 0x1a, 0xd5, + 0xd3, 0xd2, 0xf0, 0x49, 0xac, 0x6d, 0xcb, 0x5a, 0x4b, 0x9a, 0xd0, 0x2c, 0x2f, 0xa6, 0xa0, 0xf0, + 0xbb, 0x02, 0x2a, 0x22, 0xc6, 0xe1, 0xc4, 0xb5, 0x18, 0x1a, 0x84, 0x64, 0x42, 0xa8, 0xe5, 0xab, + 0x15, 0xf0, 0x0f, 0xc3, 0xcc, 0x47, 0x22, 0x81, 0x29, 0x36, 0x6a, 0x13, 0x94, 0x5c, 0x44, 0x9d, + 0x10, 0x4f, 0x18, 0x26, 0x01, 0xef, 0x65, 0xd1, 0x9c, 0x87, 0xd4, 0x7d, 0xf0, 0x2f, 0x8d, 0xec, + 0x63, 0xe4, 0xb0, 0xe1, 0x55, 0x17, 0x56, 0x79, 0x17, 0x76, 0x93, 0x58, 0xab, 0x09, 0x67, 0xd7, + 0x28, 0xd0, 0x2c, 0x4b, 0xac, 0x9f, 0x35, 0xe5, 0x39, 0xa8, 0xd0, 0xc8, 0xa6, 0x0c, 0xb3, 0x88, + 0xa1, 0x39, 0xb1, 0x3c, 0x17, 0xd3, 0x92, 0x58, 0xfb, 0xff, 0x52, 0xec, 0x1a, 0x0b, 0x9a, 0xea, + 0x15, 0x9c, 0x49, 0x76, 0xf3, 0xef, 0x4e, 0xb4, 0x1c, 0xfc, 0xa1, 0x80, 0xf2, 0xa1, 0x18, 0x8e, + 0xbf, 0x8e, 0xfb, 0x10, 0xe4, 0x27, 0xbe, 0x15, 0xf0, 0x84, 0xa5, 0xce, 0xae, 0x2e, 0x66, 0x51, + 0xcf, 0x66, 0x4f, 0xce, 0xa2, 0x3e, 0xf0, 0xad, 0x40, 0xfe, 0x9a, 0x9c, 0xaf, 0x1e, 0x83, 0xaa, + 0xe4, 0xb8, 0xc3, 0x85, 0x51, 0xca, 0xff, 0xe2, 0xf7, 0x6c, 0x26, 0xb1, 0xb6, 0x2b, 0x32, 0xdf, + 0x78, 0x19, 0x9a, 0x5b, 0x19, 0x3e, 0x37, 0xe0, 0xdd, 0xf5, 0x34, 0xf5, 0x87, 0x13, 0x2d, 0xf7, + 0xed, 0x44, 0x53, 0xd2, 0x87, 0xa0, 0x20, 0xe7, 0xaa, 0x0f, 0xca, 0x21, 0x9a, 0x62, 0x8a, 0x49, + 0x30, 0x0c, 0xa2, 0xb1, 0x8d, 0x42, 0x1e, 0x3f, 0xdf, 0xab, 0x27, 0xb1, 0xf6, 0x9f, 0x28, 0xb4, + 0x44, 0x80, 0xe6, 0x46, 0x86, 0x3c, 0xe5, 0xc0, 0x82, 0x88, 0x9c, 0xd2, 0x95, 0x5b, 0x45, 0x04, + 0x61, 0x4e, 0x44, 0x38, 0xe9, 0xae, 0x65, 0x16, 0xe1, 0x13, 0x50, 0x18, 0x58, 0xa1, 0x35, 0xa6, + 0xa9, 0xb0, 0xe5, 0xfb, 0xe4, 0xcd, 0x65, 0x48, 0x5a, 0x53, 0x9a, 0xab, 0xad, 0xe2, 0xbc, 0xf0, + 0x12, 0x01, 0x9a, 0x1b, 0x12, 0x11, 0xf9, 0x69, 0xef, 0xd9, 0xe9, 0x79, 0x43, 0x39, 0x3b, 0x6f, + 0x28, 0x5f, 0xcf, 0x1b, 0xca, 0xfb, 0x8b, 0x46, 0xee, 0xec, 0xa2, 0x91, 0xfb, 0x72, 0xd1, 0xc8, + 0xbd, 0x7e, 0xe0, 0x61, 0x36, 0x8a, 0x6c, 0xdd, 0x21, 0x63, 0x43, 0xbe, 0xa0, 0xd8, 0x76, 0xf6, + 0x3c, 0x62, 0x8c, 0x89, 0x1b, 0xf9, 0x88, 0x8a, 0x97, 0xfb, 0x5e, 0x67, 0x4f, 0x3e, 0xde, 0x6c, + 0x36, 0x41, 0xd4, 0x2e, 0xf0, 0x2f, 0x72, 0xff, 0x67, 0x00, 0x00, 0x00, 0xff, 0xff, 0x67, 0xcd, + 0xb5, 0xb7, 0xdc, 0x05, 0x00, 0x00, } func (this *UpgradeProposal) Equal(that interface{}) bool { @@ -636,16 +629,6 @@ func (m *ClientUpdateProposal) MarshalToSizedBuffer(dAtA []byte) (int, error) { _ = i var l int _ = l - { - size, err := m.InitialHeight.MarshalToSizedBuffer(dAtA[:i]) - if err != nil { - return 0, err - } - i -= size - i = encodeVarintClient(dAtA, i, uint64(size)) - } - i-- - dAtA[i] = 0x2a if len(m.SubstituteClientId) > 0 { i -= len(m.SubstituteClientId) copy(dAtA[i:], m.SubstituteClientId) @@ -885,8 +868,6 @@ func (m *ClientUpdateProposal) Size() (n int) { if l > 0 { n += 1 + l + sovClient(uint64(l)) } - l = m.InitialHeight.Size() - n += 1 + l + sovClient(uint64(l)) return n } @@ -1459,39 +1440,6 @@ func (m *ClientUpdateProposal) Unmarshal(dAtA []byte) error { } m.SubstituteClientId = string(dAtA[iNdEx:postIndex]) iNdEx = postIndex - case 5: - if wireType != 2 { - return fmt.Errorf("proto: wrong wireType = %d for field InitialHeight", wireType) - } - var msglen int - for shift := uint(0); ; shift += 7 { - if shift >= 64 { - return ErrIntOverflowClient - } - if iNdEx >= l { - return io.ErrUnexpectedEOF - } - b := dAtA[iNdEx] - iNdEx++ - msglen |= int(b&0x7F) << shift - if b < 0x80 { - break - } - } - if msglen < 0 { - return ErrInvalidLengthClient - } - postIndex := iNdEx + msglen - if postIndex < 0 { - return ErrInvalidLengthClient - } - if postIndex > l { - return io.ErrUnexpectedEOF - } - if err := m.InitialHeight.Unmarshal(dAtA[iNdEx:postIndex]); err != nil { - return err - } - iNdEx = postIndex default: iNdEx = preIndex skippy, err := skipClient(dAtA[iNdEx:]) diff --git a/modules/core/02-client/types/proposal.go b/modules/core/02-client/types/proposal.go index 3d10a925f5b..bf5ac73e772 100644 --- a/modules/core/02-client/types/proposal.go +++ b/modules/core/02-client/types/proposal.go @@ -28,13 +28,12 @@ func init() { } // NewClientUpdateProposal creates a new client update proposal. -func NewClientUpdateProposal(title, description, subjectClientID, substituteClientID string, initialHeight Height) govtypes.Content { +func NewClientUpdateProposal(title, description, subjectClientID, substituteClientID string) govtypes.Content { return &ClientUpdateProposal{ Title: title, Description: description, SubjectClientId: subjectClientID, SubstituteClientId: substituteClientID, - InitialHeight: initialHeight, } } @@ -67,10 +66,6 @@ func (cup *ClientUpdateProposal) ValidateBasic() error { return err } - if cup.InitialHeight.IsZero() { - return sdkerrors.Wrap(ErrInvalidHeight, "initial height cannot be zero height") - } - return nil } @@ -111,10 +106,6 @@ func (up *UpgradeProposal) ValidateBasic() error { return err } - if up.Plan.Height <= 0 { - return sdkerrors.Wrap(ErrInvalidUpgradeProposal, "IBC chain upgrades must set a positive height") - } - if up.UpgradedClientState == nil { return sdkerrors.Wrap(ErrInvalidUpgradeProposal, "upgraded client state cannot be nil") } diff --git a/modules/core/exported/client.go b/modules/core/exported/client.go index 77268b1b350..1e4b815ac70 100644 --- a/modules/core/exported/client.go +++ b/modules/core/exported/client.go @@ -63,7 +63,7 @@ type ClientState interface { CheckHeaderAndUpdateState(sdk.Context, codec.BinaryCodec, sdk.KVStore, Header) (ClientState, ConsensusState, error) CheckMisbehaviourAndUpdateState(sdk.Context, codec.BinaryCodec, sdk.KVStore, Misbehaviour) (ClientState, error) - CheckSubstituteAndUpdateState(ctx sdk.Context, cdc codec.BinaryCodec, subjectClientStore, substituteClientStore sdk.KVStore, substituteClient ClientState, height Height) (ClientState, error) + CheckSubstituteAndUpdateState(ctx sdk.Context, cdc codec.BinaryCodec, subjectClientStore, substituteClientStore sdk.KVStore, substituteClient ClientState) (ClientState, error) // Upgrade functions // NOTE: proof heights are not included as upgrade to a new revision is expected to pass only on the last diff --git a/modules/light-clients/06-solomachine/types/proposal_handle.go b/modules/light-clients/06-solomachine/types/proposal_handle.go index a4a89006894..b4dab1e6b00 100644 --- a/modules/light-clients/06-solomachine/types/proposal_handle.go +++ b/modules/light-clients/06-solomachine/types/proposal_handle.go @@ -20,7 +20,6 @@ import ( func (cs ClientState) CheckSubstituteAndUpdateState( ctx sdk.Context, cdc codec.BinaryCodec, subjectClientStore, _ sdk.KVStore, substituteClient exported.ClientState, - _ exported.Height, ) (exported.ClientState, error) { if !cs.AllowUpdateAfterProposal { diff --git a/modules/light-clients/06-solomachine/types/proposal_handle_test.go b/modules/light-clients/06-solomachine/types/proposal_handle_test.go index bc8e69acbcb..822a1c1032f 100644 --- a/modules/light-clients/06-solomachine/types/proposal_handle_test.go +++ b/modules/light-clients/06-solomachine/types/proposal_handle_test.go @@ -70,7 +70,7 @@ func (suite *SoloMachineTestSuite) TestCheckSubstituteAndUpdateState() { subjectClientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), solomachine.ClientID) substituteClientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), substitute.ClientID) - updatedClient, err := subjectClientState.CheckSubstituteAndUpdateState(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), subjectClientStore, substituteClientStore, substituteClientState, nil) + updatedClient, err := subjectClientState.CheckSubstituteAndUpdateState(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), subjectClientStore, substituteClientStore, substituteClientState) if tc.expPass { suite.Require().NoError(err) diff --git a/modules/light-clients/07-tendermint/types/proposal_handle.go b/modules/light-clients/07-tendermint/types/proposal_handle.go index 55c00d0f9d5..9963949d34d 100644 --- a/modules/light-clients/07-tendermint/types/proposal_handle.go +++ b/modules/light-clients/07-tendermint/types/proposal_handle.go @@ -28,7 +28,6 @@ import ( func (cs ClientState) CheckSubstituteAndUpdateState( ctx sdk.Context, cdc codec.BinaryCodec, subjectClientStore, substituteClientStore sdk.KVStore, substituteClient exported.ClientState, - initialHeight exported.Height, ) (exported.ClientState, error) { substituteClientState, ok := substituteClient.(*ClientState) if !ok { @@ -37,16 +36,6 @@ func (cs ClientState) CheckSubstituteAndUpdateState( ) } - // substitute clients are not allowed to be upgraded during the voting period - // If an upgrade passes before the subject client has been updated, a new proposal must be created - // with an initial height that contains the new revision number. - if substituteClientState.GetLatestHeight().GetRevisionNumber() != initialHeight.GetRevisionNumber() { - return nil, sdkerrors.Wrapf( - clienttypes.ErrInvalidHeight, "substitute client revision number must equal initial height revision number (%d != %d)", - substituteClientState.GetLatestHeight().GetRevisionNumber(), initialHeight.GetRevisionNumber(), - ) - } - if !IsMatchingClientState(cs, *substituteClientState) { return nil, sdkerrors.Wrap(clienttypes.ErrInvalidSubstitute, "subject client state does not match substitute client state") } @@ -80,41 +69,21 @@ func (cs ClientState) CheckSubstituteAndUpdateState( // copy consensus states and processed time from substitute to subject // starting from initial height and ending on the latest height (inclusive) - for i := initialHeight.GetRevisionHeight(); i <= substituteClientState.GetLatestHeight().GetRevisionHeight(); i++ { - height := clienttypes.NewHeight(substituteClientState.GetLatestHeight().GetRevisionNumber(), i) - - consensusState, err := GetConsensusState(substituteClientStore, cdc, height) - if err != nil { - // not all consensus states will be filled in - continue - } - SetConsensusState(subjectClientStore, cdc, consensusState, height) - - processedTime, found := GetProcessedTime(substituteClientStore, height) - if !found { - continue - } - SetProcessedTime(subjectClientStore, height, processedTime) - - } - - cs.LatestHeight = substituteClientState.LatestHeight - - // validate the updated client and ensure it isn't expired - if err := cs.Validate(); err != nil { - return nil, sdkerrors.Wrap(err, "unexpected error: updated subject client state is invalid") - } + height := substituteClientState.GetLatestHeight() - latestConsensusState, err := GetConsensusState(subjectClientStore, cdc, cs.GetLatestHeight()) + consensusState, err = GetConsensusState(substituteClientStore, cdc, height) if err != nil { - return nil, sdkerrors.Wrapf( - err, "unexpected error: could not get consensus state for updated subject client from clientstore at height: %d", cs.GetLatestHeight(), - ) + return nil, sdkerrors.Wrap(err, "unable to retrieve latest consensus state for substitute client") } + SetConsensusState(subjectClientStore, cdc, consensusState, height) - if cs.IsExpired(latestConsensusState.Timestamp, ctx.BlockTime()) { - return nil, sdkerrors.Wrap(clienttypes.ErrInvalidClient, "updated subject client is expired") + processedTime, found := GetProcessedTime(substituteClientStore, height) + if !found { + return nil, sdkerrors.Wrap(clienttypes.ErrUpdateClientFailed, "unable to retrieve processed time for substitute client latest height") } + SetProcessedTime(subjectClientStore, height, processedTime) + + cs.LatestHeight = substituteClientState.LatestHeight return &cs, nil } diff --git a/modules/light-clients/07-tendermint/types/proposal_handle_test.go b/modules/light-clients/07-tendermint/types/proposal_handle_test.go index ce09917853e..aa607f6d65a 100644 --- a/modules/light-clients/07-tendermint/types/proposal_handle_test.go +++ b/modules/light-clients/07-tendermint/types/proposal_handle_test.go @@ -16,7 +16,6 @@ var ( func (suite *TendermintTestSuite) TestCheckSubstituteUpdateStateBasic() { var ( substituteClientState exported.ClientState - initialHeight clienttypes.Height substitutePath *ibctesting.Path ) testCases := []struct { @@ -28,11 +27,6 @@ func (suite *TendermintTestSuite) TestCheckSubstituteUpdateStateBasic() { substituteClientState = ibctesting.NewSolomachine(suite.T(), suite.cdc, "solo machine", "", 1).ClientState() }, }, - { - "initial height and substitute revision numbers do not match", func() { - initialHeight = clienttypes.NewHeight(substituteClientState.GetLatestHeight().GetRevisionNumber()+1, 1) - }, - }, { "non-matching substitute", func() { suite.coordinator.SetupClients(substitutePath) @@ -43,49 +37,6 @@ func (suite *TendermintTestSuite) TestCheckSubstituteUpdateStateBasic() { tmClientState.ChainId = tmClientState.ChainId + "different chain" }, }, - { - "updated client is invalid - revision height is zero", func() { - suite.coordinator.SetupClients(substitutePath) - substituteClientState = suite.chainA.GetClientState(substitutePath.EndpointA.ClientID).(*types.ClientState) - tmClientState, ok := substituteClientState.(*types.ClientState) - suite.Require().True(ok) - // match subject - tmClientState.AllowUpdateAfterMisbehaviour = true - tmClientState.AllowUpdateAfterExpiry = true - - // will occur. This case should never occur (caught by upstream checks) - initialHeight = clienttypes.NewHeight(5, 0) - tmClientState.LatestHeight = clienttypes.NewHeight(5, 0) - }, - }, - { - "updated client is expired", func() { - suite.coordinator.SetupClients(substitutePath) - substituteClientState = suite.chainA.GetClientState(substitutePath.EndpointA.ClientID).(*types.ClientState) - tmClientState, ok := substituteClientState.(*types.ClientState) - suite.Require().True(ok) - initialHeight = tmClientState.LatestHeight - - // match subject - tmClientState.AllowUpdateAfterMisbehaviour = true - tmClientState.AllowUpdateAfterExpiry = true - suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), substitutePath.EndpointA.ClientID, tmClientState) - - // update substitute a few times - err := substitutePath.EndpointA.UpdateClient() - suite.Require().NoError(err) - substituteClientState = suite.chainA.GetClientState(substitutePath.EndpointA.ClientID) - - err = substitutePath.EndpointA.UpdateClient() - suite.Require().NoError(err) - - // expire client - suite.coordinator.IncrementTimeBy(tmClientState.TrustingPeriod) - suite.coordinator.CommitBlock(suite.chainA, suite.chainB) - - substituteClientState = suite.chainA.GetClientState(substitutePath.EndpointA.ClientID) - }, - }, } for _, tc := range testCases { @@ -111,7 +62,7 @@ func (suite *TendermintTestSuite) TestCheckSubstituteUpdateStateBasic() { subjectClientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID) substituteClientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), substitutePath.EndpointA.ClientID) - updatedClient, err := subjectClientState.CheckSubstituteAndUpdateState(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), subjectClientStore, substituteClientStore, substituteClientState, initialHeight) + updatedClient, err := subjectClientState.CheckSubstituteAndUpdateState(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), subjectClientStore, substituteClientStore, substituteClientState) suite.Require().Error(err) suite.Require().Nil(updatedClient) }) @@ -300,8 +251,6 @@ func (suite *TendermintTestSuite) TestCheckSubstituteAndUpdateState() { substituteClientState.AllowUpdateAfterMisbehaviour = tc.AllowUpdateAfterMisbehaviour suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), substitutePath.EndpointA.ClientID, substituteClientState) - initialHeight := substituteClientState.GetLatestHeight() - // update substitute a few times for i := 0; i < 3; i++ { err := substitutePath.EndpointA.UpdateClient() @@ -315,11 +264,17 @@ func (suite *TendermintTestSuite) TestCheckSubstituteAndUpdateState() { subjectClientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID) substituteClientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), substitutePath.EndpointA.ClientID) - updatedClient, err := subjectClientState.CheckSubstituteAndUpdateState(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), subjectClientStore, substituteClientStore, substituteClientState, initialHeight) + updatedClient, err := subjectClientState.CheckSubstituteAndUpdateState(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), subjectClientStore, substituteClientStore, substituteClientState) if tc.expPass { suite.Require().NoError(err) suite.Require().Equal(clienttypes.ZeroHeight(), updatedClient.(*types.ClientState).FrozenHeight) + + // check that the consensus state was copied over + suite.Require().Equal(substituteClientState.GetLatestHeight(), updatedClient.GetLatestHeight()) + subjectConsState := subjectPath.EndpointA.GetConsensusState(updatedClient.GetLatestHeight()) + substituteConsState := substitutePath.EndpointA.GetConsensusState(substituteClientState.GetLatestHeight()) + suite.Require().Equal(substituteConsState, subjectConsState) } else { suite.Require().Error(err) suite.Require().Nil(updatedClient) diff --git a/modules/light-clients/09-localhost/types/client_state.go b/modules/light-clients/09-localhost/types/client_state.go index 294ee4d9d48..1b4fd86fab8 100644 --- a/modules/light-clients/09-localhost/types/client_state.go +++ b/modules/light-clients/09-localhost/types/client_state.go @@ -107,7 +107,7 @@ func (cs ClientState) CheckMisbehaviourAndUpdateState( // proposals. func (cs ClientState) CheckSubstituteAndUpdateState( ctx sdk.Context, _ codec.BinaryCodec, _, _ sdk.KVStore, - _ exported.ClientState, _ exported.Height, + _ exported.ClientState, ) (exported.ClientState, error) { return nil, sdkerrors.Wrap(clienttypes.ErrUpdateClientFailed, "cannot update localhost client with a proposal") } diff --git a/modules/light-clients/09-localhost/types/client_state_test.go b/modules/light-clients/09-localhost/types/client_state_test.go index 358ac294170..6d95996b84f 100644 --- a/modules/light-clients/09-localhost/types/client_state_test.go +++ b/modules/light-clients/09-localhost/types/client_state_test.go @@ -178,7 +178,7 @@ func (suite *LocalhostTestSuite) TestMisbehaviourAndUpdateState() { func (suite *LocalhostTestSuite) TestProposedHeaderAndUpdateState() { clientState := types.NewClientState("chainID", clientHeight) - cs, err := clientState.CheckSubstituteAndUpdateState(suite.ctx, nil, nil, nil, nil, nil) + cs, err := clientState.CheckSubstituteAndUpdateState(suite.ctx, nil, nil, nil, nil) suite.Require().Error(err) suite.Require().Nil(cs) } diff --git a/proto/ibc/core/client/v1/client.proto b/proto/ibc/core/client/v1/client.proto index a4a2cc85def..88a6c343adb 100644 --- a/proto/ibc/core/client/v1/client.proto +++ b/proto/ibc/core/client/v1/client.proto @@ -37,11 +37,9 @@ message ClientConsensusStates { } // ClientUpdateProposal is a governance proposal. If it passes, the substitute -// client's consensus states starting from the 'initial height' are copied over -// to the subjects client state. The proposal handler may fail if the subject -// and the substitute do not match in client and chain parameters (with -// exception to latest height, frozen height, and chain-id). The updated client -// must also be valid (cannot be expired). +// client's latest consensus state is copied over to the subject client. The proposal +// handler may fail if the subject and the substitute do not match in client and +// chain parameters (with exception to latest height, frozen height, and chain-id). message ClientUpdateProposal { option (gogoproto.goproto_getters) = false; // the title of the update proposal @@ -52,10 +50,7 @@ message ClientUpdateProposal { string subject_client_id = 3 [(gogoproto.moretags) = "yaml:\"subject_client_id\""]; // the substitute client identifier for the client standing in for the subject // client - string substitute_client_id = 4 [(gogoproto.moretags) = "yaml:\"susbtitute_client_id\""]; - // the intital height to copy consensus states from the substitute to the - // subject - Height initial_height = 5 [(gogoproto.moretags) = "yaml:\"initial_height\"", (gogoproto.nullable) = false]; + string substitute_client_id = 4 [(gogoproto.moretags) = "yaml:\"substitute_client_id\""]; } // UpgradeProposal is a gov Content type for initiating an IBC breaking From f2c4dd4ac11732c4ff45352492342f851ebbd9cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 20 May 2021 12:28:43 +0200 Subject: [PATCH 2/9] fix tests and update godoc --- modules/core/02-client/keeper/proposal.go | 2 +- .../core/02-client/proposal_handler_test.go | 3 +-- modules/core/02-client/types/proposal_test.go | 20 ++++++------------- .../07-tendermint/types/proposal_handle.go | 15 +++++++------- .../types/proposal_handle_test.go | 6 +++--- 5 files changed, 19 insertions(+), 27 deletions(-) diff --git a/modules/core/02-client/keeper/proposal.go b/modules/core/02-client/keeper/proposal.go index 62a0494d7ef..4ab66792703 100644 --- a/modules/core/02-client/keeper/proposal.go +++ b/modules/core/02-client/keeper/proposal.go @@ -17,7 +17,7 @@ import ( // client implementations are responsible for validating the parameters of the // subtitute (enusring they match the subject's parameters) as well as copying // the necessary consensus states from the subtitute to the subject client -// store. +// store. The substitute must be Active and the subject must no be Active. func (k Keeper) ClientUpdateProposal(ctx sdk.Context, p *types.ClientUpdateProposal) error { if p.SubjectClientId == exported.Localhost || p.SubstituteClientId == exported.Localhost { return sdkerrors.Wrap(types.ErrInvalidUpdateClientProposal, "cannot update localhost client with proposal") diff --git a/modules/core/02-client/proposal_handler_test.go b/modules/core/02-client/proposal_handler_test.go index 2c83b95b8cf..ea4eb31b260 100644 --- a/modules/core/02-client/proposal_handler_test.go +++ b/modules/core/02-client/proposal_handler_test.go @@ -29,7 +29,6 @@ func (suite *ClientTestSuite) TestNewClientUpdateProposalHandler() { substitutePath := ibctesting.NewPath(suite.chainA, suite.chainB) suite.coordinator.SetupClients(substitutePath) - initialHeight := clienttypes.NewHeight(subjectClientState.GetLatestHeight().GetRevisionNumber(), subjectClientState.GetLatestHeight().GetRevisionHeight()+1) // update substitute twice err = substitutePath.EndpointA.UpdateClient() @@ -50,7 +49,7 @@ func (suite *ClientTestSuite) TestNewClientUpdateProposalHandler() { tmClientState.AllowUpdateAfterMisbehaviour = true suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), substitutePath.EndpointA.ClientID, tmClientState) - content = clienttypes.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subjectPath.EndpointA.ClientID, substitutePath.EndpointA.ClientID, initialHeight) + content = clienttypes.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subjectPath.EndpointA.ClientID, substitutePath.EndpointA.ClientID) }, true, }, { diff --git a/modules/core/02-client/types/proposal_test.go b/modules/core/02-client/types/proposal_test.go index 56d6103cca2..32521d8c168 100644 --- a/modules/core/02-client/types/proposal_test.go +++ b/modules/core/02-client/types/proposal_test.go @@ -17,14 +17,11 @@ func (suite *TypesTestSuite) TestValidateBasic() { subjectPath := ibctesting.NewPath(suite.chainA, suite.chainB) suite.coordinator.SetupClients(subjectPath) subject := subjectPath.EndpointA.ClientID - subjectClientState := suite.chainA.GetClientState(subject) substitutePath := ibctesting.NewPath(suite.chainA, suite.chainB) suite.coordinator.SetupClients(substitutePath) substitute := substitutePath.EndpointA.ClientID - initialHeight := types.NewHeight(subjectClientState.GetLatestHeight().GetRevisionNumber(), subjectClientState.GetLatestHeight().GetRevisionHeight()+1) - testCases := []struct { name string proposal govtypes.Content @@ -32,32 +29,27 @@ func (suite *TypesTestSuite) TestValidateBasic() { }{ { "success", - types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, substitute, initialHeight), + types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, substitute), true, }, { "fails validate abstract - empty title", - types.NewClientUpdateProposal("", ibctesting.Description, subject, substitute, initialHeight), + types.NewClientUpdateProposal("", ibctesting.Description, subject, substitute), false, }, { "subject and substitute use the same identifier", - types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, subject, initialHeight), + types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, subject), false, }, { "invalid subject clientID", - types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, ibctesting.InvalidID, substitute, initialHeight), + types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, ibctesting.InvalidID, substitute), false, }, { "invalid substitute clientID", - types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, ibctesting.InvalidID, initialHeight), - false, - }, - { - "initial height is zero", - types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, substitute, types.ZeroHeight()), + types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, ibctesting.InvalidID), false, }, } @@ -77,7 +69,7 @@ func (suite *TypesTestSuite) TestValidateBasic() { // tests a client update proposal can be marshaled and unmarshaled func (suite *TypesTestSuite) TestMarshalClientUpdateProposalProposal() { // create proposal - proposal := types.NewClientUpdateProposal("update IBC client", "description", "subject", "substitute", types.NewHeight(1, 0)) + proposal := types.NewClientUpdateProposal("update IBC client", "description", "subject", "substitute") // create codec ir := codectypes.NewInterfaceRegistry() diff --git a/modules/light-clients/07-tendermint/types/proposal_handle.go b/modules/light-clients/07-tendermint/types/proposal_handle.go index 9963949d34d..3fa096bf765 100644 --- a/modules/light-clients/07-tendermint/types/proposal_handle.go +++ b/modules/light-clients/07-tendermint/types/proposal_handle.go @@ -13,8 +13,8 @@ import ( // CheckSubstituteAndUpdateState will try to update the client with the state of the // substitute if and only if the proposal passes and one of the following conditions are // satisfied: -// 1) AllowUpdateAfterMisbehaviour and IsFrozen() = true -// 2) AllowUpdateAfterExpiry=true and Expire(ctx.BlockTime) = true +// 1) AllowUpdateAfterMisbehaviour and Status() == Frozen +// 2) AllowUpdateAfterExpiry=true and Status() == Expired // // The following must always be true: // - The substitute client is the same type as the subject client @@ -23,8 +23,6 @@ import ( // In case 1) before updating the client, the client will be unfrozen by resetting // the FrozenHeight to the zero Height. If a client is frozen and AllowUpdateAfterMisbehaviour // is set to true, the client will be unexpired even if AllowUpdateAfterExpiry is set to false. -// Note, that even if the subject is updated to the state of the substitute, an error may be -// returned if the updated client state is invalid or the client is expired. func (cs ClientState) CheckSubstituteAndUpdateState( ctx sdk.Context, cdc codec.BinaryCodec, subjectClientStore, substituteClientStore sdk.KVStore, substituteClient exported.ClientState, @@ -48,9 +46,9 @@ func (cs ClientState) CheckSubstituteAndUpdateState( ) } - switch { + switch cs.Status(ctx, subjectClientStore, cdc) { - case !cs.FrozenHeight.IsZero(): + case exported.Frozen: if !cs.AllowUpdateAfterMisbehaviour { return nil, sdkerrors.Wrap(clienttypes.ErrUpdateClientFailed, "client is not allowed to be unfrozen") } @@ -58,7 +56,7 @@ func (cs ClientState) CheckSubstituteAndUpdateState( // unfreeze the client cs.FrozenHeight = clienttypes.ZeroHeight() - case cs.IsExpired(consensusState.Timestamp, ctx.BlockTime()): + case exported.Expired: if !cs.AllowUpdateAfterExpiry { return nil, sdkerrors.Wrap(clienttypes.ErrUpdateClientFailed, "client is not allowed to be unexpired") } @@ -85,6 +83,9 @@ func (cs ClientState) CheckSubstituteAndUpdateState( cs.LatestHeight = substituteClientState.LatestHeight + // no validation is necessary since the substitute is verified to be Active + // in 02-client. + return &cs, nil } diff --git a/modules/light-clients/07-tendermint/types/proposal_handle_test.go b/modules/light-clients/07-tendermint/types/proposal_handle_test.go index aa607f6d65a..daa3486211c 100644 --- a/modules/light-clients/07-tendermint/types/proposal_handle_test.go +++ b/modules/light-clients/07-tendermint/types/proposal_handle_test.go @@ -261,6 +261,7 @@ func (suite *TendermintTestSuite) TestCheckSubstituteAndUpdateState() { // get updated substitute substituteClientState = suite.chainA.GetClientState(substitutePath.EndpointA.ClientID).(*types.ClientState) + expectedConsState := substitutePath.EndpointA.GetConsensusState(substituteClientState.GetLatestHeight()) subjectClientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID) substituteClientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), substitutePath.EndpointA.ClientID) @@ -270,11 +271,10 @@ func (suite *TendermintTestSuite) TestCheckSubstituteAndUpdateState() { suite.Require().NoError(err) suite.Require().Equal(clienttypes.ZeroHeight(), updatedClient.(*types.ClientState).FrozenHeight) - // check that the consensus state was copied over + // check that the correct consensus state was copied over suite.Require().Equal(substituteClientState.GetLatestHeight(), updatedClient.GetLatestHeight()) subjectConsState := subjectPath.EndpointA.GetConsensusState(updatedClient.GetLatestHeight()) - substituteConsState := substitutePath.EndpointA.GetConsensusState(substituteClientState.GetLatestHeight()) - suite.Require().Equal(substituteConsState, subjectConsState) + suite.Require().Equal(expectedConsState, subjectConsState) } else { suite.Require().Error(err) suite.Require().Nil(updatedClient) From 05d7e67849fdbea1ee0bbe76a395b41340d468bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 20 May 2021 13:01:02 +0200 Subject: [PATCH 3/9] update docs --- .../adr-026-ibc-client-recovery-mechanisms.md | 6 +++--- docs/ibc/proposals.md | 15 +++++++-------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/docs/architecture/adr-026-ibc-client-recovery-mechanisms.md b/docs/architecture/adr-026-ibc-client-recovery-mechanisms.md index 2e33bf58684..c40e3b08cd5 100644 --- a/docs/architecture/adr-026-ibc-client-recovery-mechanisms.md +++ b/docs/architecture/adr-026-ibc-client-recovery-mechanisms.md @@ -5,6 +5,7 @@ - 2020/06/23: Initial version - 2020/08/06: Revisions per review & to reference version - 2021/01/15: Revision to support substitute clients for unfreezing +- 2021/05/20: Revision to simplify consensus state copying, remove initial height ## Status @@ -42,11 +43,10 @@ We elect not to deal with chains which have actually halted, which is necessaril 1. Require Tendermint light clients (ICS 07) to expose the following additional state mutation functions 1. `Unfreeze()`, which unfreezes a light client after misbehaviour and clears any frozen height previously set 1. Add a new governance proposal type, `ClientUpdateProposal`, in the `x/ibc` module - 1. Extend the base `Proposal` with two client identifiers (`string`) and an initial height ('exported.Height'). + 1. Extend the base `Proposal` with two client identifiers (`string`). 1. The first client identifier is the proposed client to be updated. This client must be either frozen or expired. 1. The second client is a substitute client. It carries all the state for the client which may be updated. It must have identitical client and chain parameters to the client which may be updated (except for latest height, frozen height, and chain-id). It should be continually updated during the voting period. - 1. The initial height represents the starting height consensus states which will be copied from the substitute client to the frozen/expired client. - 1. If this governance proposal passes, the client on trial will be updated with all the state of the substitute, if and only if: + 1. If this governance proposal passes, the client on trial will be updated to the latest state of the substitute, if and only if: 1. `allow_governance_override_after_expiry` is true and the client has expired (`Expired()` returns true) 1. `allow_governance_override_after_misbehaviour` is true and the client has been frozen (`Frozen()` returns true) 1. In this case, additionally, the client is unfrozen by calling `Unfreeze()` diff --git a/docs/ibc/proposals.md b/docs/ibc/proposals.md index 6bdf9f7051f..b3ce139c2a0 100644 --- a/docs/ibc/proposals.md +++ b/docs/ibc/proposals.md @@ -32,11 +32,10 @@ ultimately lead the on-chain light client to become expired. In the case that a highly valued light client is frozen, expired, or rendered non-updateable, a governance proposal may be submitted to update this client, known as the subject client. The -proposal includes the client identifier for the subject, the client identifier for a substitute -client, and an initial height to reference the substitute client from. Light client implementations -may implement custom updating logic, but in most cases, the subject will be updated with information -from the substitute client, if the proposal passes. The substitute client is used as a "stand in" -while the subject is on trial. It is best practice to create a substitute client *after* the subject -has become frozen to avoid the substitute from also becoming frozen. An active substitute client -allows headers to be submitted during the voting period to prevent accidental expiry once the proposal -passes. +proposal includes the client identifier for the subject and the client identifier for a substitute +client. Light client implementations may implement custom updating logic, but in most cases, +the subject will be updated to the latest consensus state of the substitute client, if the proposal passes. +The substitute client is used as a "stand in" while the subject is on trial. It is best practice to create +a substitute client *after* the subject has become frozen to avoid the substitute from also becoming frozen. +An active substitute client allows headers to be submitted during the voting period to prevent accidental expiry +once the proposal passes. From bf7a6c0c1047c4c1fb3bf0fbaae8953fe4e809d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 20 May 2021 13:18:41 +0200 Subject: [PATCH 4/9] code coverage --- .../core/02-client/keeper/proposal_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/modules/core/02-client/keeper/proposal_test.go b/modules/core/02-client/keeper/proposal_test.go index 30579447fbe..5a31b7e0945 100644 --- a/modules/core/02-client/keeper/proposal_test.go +++ b/modules/core/02-client/keeper/proposal_test.go @@ -55,6 +55,15 @@ func (suite *KeeperTestSuite) TestClientUpdateProposal() { content = types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, exported.Localhost) }, false, }, + { + "cannot use solomachine as substitute for tendermint client", func() { + solomachine := ibctesting.NewSolomachine(suite.T(), suite.cdc, "solo machine", "", 1) + solomachine.Sequence = subjectClientState.GetLatestHeight().GetRevisionHeight() + 1 + substituteClientState = solomachine.ClientState() + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), substitute, substituteClientState) + content = types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, substitute) + }, false, + }, { "subject client does not exist", func() { content = types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, ibctesting.InvalidID, substitute) @@ -82,6 +91,16 @@ func (suite *KeeperTestSuite) TestClientUpdateProposal() { tmClientState.FrozenHeight = types.ZeroHeight() suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), subject, tmClientState) + content = types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, substitute) + }, false, + }, + { + "substitute is frozen", func() { + tmClientState, ok := substituteClientState.(*ibctmtypes.ClientState) + suite.Require().True(ok) + tmClientState.FrozenHeight = types.NewHeight(0, 1) + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), substitute, tmClientState) + content = types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, substitute) }, false, }, From ac4f784380521fca97a3dd36ff9072be9d1321ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 20 May 2021 13:20:13 +0200 Subject: [PATCH 5/9] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8119e1019a7..0015bd3c0d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking +* (02-client) [\#181](https://github.com/cosmos/ibc-go/pull/181) Remove 'InitialHeight' from UpdateClient Proposal. Only copy over latest consensus state from substitute client. * (06-solomachine) [\#169](https://github.com/cosmos/ibc-go/pull/169) Change FrozenSequence to boolean in solomachine ClientState. The solo machine proto package has been bumped from `v1` to `v2`. * (module/core/02-client) [\#165](https://github.com/cosmos/ibc-go/pull/165) Remove GetFrozenHeight from the ClientState interface. * (modules) [\#166](https://github.com/cosmos/ibc-go/pull/166) Remove GetHeight from the misbehaviour interface. The `consensus_height` attribute has been removed from Misbehaviour events. From 16dde8374c6c7af426d12ac86824bc4f0091ca0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 20 May 2021 13:21:50 +0200 Subject: [PATCH 6/9] remove unused code --- .../07-tendermint/types/proposal_handle.go | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/modules/light-clients/07-tendermint/types/proposal_handle.go b/modules/light-clients/07-tendermint/types/proposal_handle.go index 3fa096bf765..cc2f6f65b7e 100644 --- a/modules/light-clients/07-tendermint/types/proposal_handle.go +++ b/modules/light-clients/07-tendermint/types/proposal_handle.go @@ -38,14 +38,6 @@ func (cs ClientState) CheckSubstituteAndUpdateState( return nil, sdkerrors.Wrap(clienttypes.ErrInvalidSubstitute, "subject client state does not match substitute client state") } - // get consensus state corresponding to client state to check if the client is expired - consensusState, err := GetConsensusState(subjectClientStore, cdc, cs.GetLatestHeight()) - if err != nil { - return nil, sdkerrors.Wrapf( - err, "unexpected error: could not get consensus state from clientstore at height: %d", cs.GetLatestHeight(), - ) - } - switch cs.Status(ctx, subjectClientStore, cdc) { case exported.Frozen: @@ -69,7 +61,7 @@ func (cs ClientState) CheckSubstituteAndUpdateState( // starting from initial height and ending on the latest height (inclusive) height := substituteClientState.GetLatestHeight() - consensusState, err = GetConsensusState(substituteClientStore, cdc, height) + consensusState, err := GetConsensusState(substituteClientStore, cdc, height) if err != nil { return nil, sdkerrors.Wrap(err, "unable to retrieve latest consensus state for substitute client") } From ef254c07ff4e245354937b975f1b7d8d1852d60f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Fri, 21 May 2021 17:36:34 +0200 Subject: [PATCH 7/9] set subject chain-id using substitute --- .../light-clients/07-tendermint/types/proposal_handle.go | 1 + .../07-tendermint/types/proposal_handle_test.go | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/modules/light-clients/07-tendermint/types/proposal_handle.go b/modules/light-clients/07-tendermint/types/proposal_handle.go index cc2f6f65b7e..8ce18c7074c 100644 --- a/modules/light-clients/07-tendermint/types/proposal_handle.go +++ b/modules/light-clients/07-tendermint/types/proposal_handle.go @@ -74,6 +74,7 @@ func (cs ClientState) CheckSubstituteAndUpdateState( SetProcessedTime(subjectClientStore, height, processedTime) cs.LatestHeight = substituteClientState.LatestHeight + cs.ChainId = substituteClientState.ChainId // no validation is necessary since the substitute is verified to be Active // in 02-client. diff --git a/modules/light-clients/07-tendermint/types/proposal_handle_test.go b/modules/light-clients/07-tendermint/types/proposal_handle_test.go index daa3486211c..9a15f4ec242 100644 --- a/modules/light-clients/07-tendermint/types/proposal_handle_test.go +++ b/modules/light-clients/07-tendermint/types/proposal_handle_test.go @@ -261,6 +261,11 @@ func (suite *TendermintTestSuite) TestCheckSubstituteAndUpdateState() { // get updated substitute substituteClientState = suite.chainA.GetClientState(substitutePath.EndpointA.ClientID).(*types.ClientState) + + // test that subject gets updated chain-id + newChainID := "new-chain-id" + substituteClientState.ChainId = newChainID + expectedConsState := substitutePath.EndpointA.GetConsensusState(substituteClientState.GetLatestHeight()) subjectClientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID) @@ -275,6 +280,7 @@ func (suite *TendermintTestSuite) TestCheckSubstituteAndUpdateState() { suite.Require().Equal(substituteClientState.GetLatestHeight(), updatedClient.GetLatestHeight()) subjectConsState := subjectPath.EndpointA.GetConsensusState(updatedClient.GetLatestHeight()) suite.Require().Equal(expectedConsState, subjectConsState) + suite.Require().Equal(newChainID, updatedClient.(*types.ClientState).ChainId) } else { suite.Require().Error(err) suite.Require().Nil(updatedClient) From c394e41fb00ed70d9f837492492ee9ced2e30fe0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Mon, 24 May 2021 12:39:38 +0200 Subject: [PATCH 8/9] fix tests --- .../core/02-client/keeper/proposal_test.go | 1 + .../types/proposal_handle_test.go | 22 +++++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/modules/core/02-client/keeper/proposal_test.go b/modules/core/02-client/keeper/proposal_test.go index 5a31b7e0945..8a4270653e9 100644 --- a/modules/core/02-client/keeper/proposal_test.go +++ b/modules/core/02-client/keeper/proposal_test.go @@ -40,6 +40,7 @@ func (suite *KeeperTestSuite) TestClientUpdateProposal() { suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), substitute, tmClientState.LatestHeight, consState) clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), substitute) ibctmtypes.SetProcessedTime(clientStore, tmClientState.LatestHeight, 100) + ibctmtypes.SetProcessedHeight(clientStore, tmClientState.LatestHeight, types.NewHeight(0, 1)) suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), substitute, tmClientState) content = types.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subject, substitute) diff --git a/modules/light-clients/07-tendermint/types/proposal_handle_test.go b/modules/light-clients/07-tendermint/types/proposal_handle_test.go index 9a15f4ec242..e4236424bf0 100644 --- a/modules/light-clients/07-tendermint/types/proposal_handle_test.go +++ b/modules/light-clients/07-tendermint/types/proposal_handle_test.go @@ -266,20 +266,38 @@ func (suite *TendermintTestSuite) TestCheckSubstituteAndUpdateState() { newChainID := "new-chain-id" substituteClientState.ChainId = newChainID - expectedConsState := substitutePath.EndpointA.GetConsensusState(substituteClientState.GetLatestHeight()) - subjectClientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID) substituteClientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), substitutePath.EndpointA.ClientID) + + expectedConsState := substitutePath.EndpointA.GetConsensusState(substituteClientState.GetLatestHeight()) + expectedProcessedTime, found := types.GetProcessedTime(substituteClientStore, substituteClientState.GetLatestHeight()) + suite.Require().True(found) + expectedProcessedHeight, found := types.GetProcessedTime(substituteClientStore, substituteClientState.GetLatestHeight()) + suite.Require().True(found) + expectedIterationKey := types.GetIterationKey(substituteClientStore, substituteClientState.GetLatestHeight()) + updatedClient, err := subjectClientState.CheckSubstituteAndUpdateState(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), subjectClientStore, substituteClientStore, substituteClientState) if tc.expPass { suite.Require().NoError(err) suite.Require().Equal(clienttypes.ZeroHeight(), updatedClient.(*types.ClientState).FrozenHeight) + subjectClientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID) + // check that the correct consensus state was copied over suite.Require().Equal(substituteClientState.GetLatestHeight(), updatedClient.GetLatestHeight()) subjectConsState := subjectPath.EndpointA.GetConsensusState(updatedClient.GetLatestHeight()) + subjectProcessedTime, found := types.GetProcessedTime(subjectClientStore, updatedClient.GetLatestHeight()) + suite.Require().True(found) + subjectProcessedHeight, found := types.GetProcessedTime(substituteClientStore, updatedClient.GetLatestHeight()) + suite.Require().True(found) + subjectIterationKey := types.GetIterationKey(substituteClientStore, updatedClient.GetLatestHeight()) + suite.Require().Equal(expectedConsState, subjectConsState) + suite.Require().Equal(expectedProcessedTime, subjectProcessedTime) + suite.Require().Equal(expectedProcessedHeight, subjectProcessedHeight) + suite.Require().Equal(expectedIterationKey, subjectIterationKey) + suite.Require().Equal(newChainID, updatedClient.(*types.ClientState).ChainId) } else { suite.Require().Error(err) From 250ae7629a2ff4769e27e774b5f2c2ea80d9e791 Mon Sep 17 00:00:00 2001 From: Aditya Date: Mon, 24 May 2021 11:51:23 -0400 Subject: [PATCH 9/9] Update modules/core/02-client/keeper/proposal.go --- modules/core/02-client/keeper/proposal.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/core/02-client/keeper/proposal.go b/modules/core/02-client/keeper/proposal.go index 4ab66792703..1880e2cde58 100644 --- a/modules/core/02-client/keeper/proposal.go +++ b/modules/core/02-client/keeper/proposal.go @@ -17,7 +17,7 @@ import ( // client implementations are responsible for validating the parameters of the // subtitute (enusring they match the subject's parameters) as well as copying // the necessary consensus states from the subtitute to the subject client -// store. The substitute must be Active and the subject must no be Active. +// store. The substitute must be Active and the subject must not be Active. func (k Keeper) ClientUpdateProposal(ctx sdk.Context, p *types.ClientUpdateProposal) error { if p.SubjectClientId == exported.Localhost || p.SubstituteClientId == exported.Localhost { return sdkerrors.Wrap(types.ErrInvalidUpdateClientProposal, "cannot update localhost client with proposal")