diff --git a/CHANGELOG.md b/CHANGELOG.md index 99b12c62a23..5f7d0054e1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,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. 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. diff --git a/docs/ibc/proto-docs.md b/docs/ibc/proto-docs.md index b098c732179..701d4684940 100644 --- a/docs/ibc/proto-docs.md +++ b/docs/ibc/proto-docs.md @@ -477,11 +477,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 | @@ -490,7 +488,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..1880e2cde58 100644 --- a/modules/core/02-client/keeper/proposal.go +++ b/modules/core/02-client/keeper/proposal.go @@ -11,14 +11,13 @@ 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 // 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 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") @@ -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..8a4270653e9 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,43 @@ 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) + 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, 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, + }, + { + "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, 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 +82,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 +92,17 @@ 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, + }, + { + "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, }, } @@ -100,7 +121,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 +138,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/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/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 612342287a4..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 } 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/core/exported/client.go b/modules/core/exported/client.go index 8de7976c14c..1578900af21 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 780ffdf9ff4..6364aa8ff88 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,12 +23,9 @@ 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, - initialHeight exported.Height, ) (exported.ClientState, error) { substituteClientState, ok := substituteClient.(*ClientState) if !ok { @@ -37,31 +34,13 @@ 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") } - // 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 { + 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") } @@ -69,7 +48,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") } @@ -80,37 +59,33 @@ 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) + height := substituteClientState.GetLatestHeight() - // set metadata for this consensus state - setConsensusMetadata(ctx, subjectClientStore, height) + consensusState, err := GetConsensusState(substituteClientStore, cdc, height) + if err != nil { + return nil, sdkerrors.Wrap(err, "unable to retrieve latest consensus state for substitute client") } - cs.LatestHeight = substituteClientState.LatestHeight + SetConsensusState(subjectClientStore, cdc, consensusState, height) - // 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") + // set metadata stored for the substitute consensus state + processedHeight, found := GetProcessedHeight(substituteClientStore, height) + if !found { + return nil, sdkerrors.Wrap(clienttypes.ErrUpdateClientFailed, "unable to retrieve processed height for substitute client latest height") } - latestConsensusState, err := GetConsensusState(subjectClientStore, cdc, cs.GetLatestHeight()) - 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(), - ) + processedTime, found := GetProcessedTime(substituteClientStore, height) + if !found { + return nil, sdkerrors.Wrap(clienttypes.ErrUpdateClientFailed, "unable to retrieve processed time for substitute client latest height") } - if cs.IsExpired(latestConsensusState.Timestamp, ctx.BlockTime()) { - return nil, sdkerrors.Wrap(clienttypes.ErrInvalidClient, "updated subject client is expired") - } + setConsensusMetadataWithValues(subjectClientStore, height, processedHeight, processedTime) + + cs.LatestHeight = substituteClientState.LatestHeight + cs.ChainId = substituteClientState.ChainId + + // 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 ce09917853e..e4236424bf0 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() @@ -313,13 +262,43 @@ 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 + 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) + + 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) suite.Require().Nil(updatedClient) diff --git a/modules/light-clients/07-tendermint/types/store.go b/modules/light-clients/07-tendermint/types/store.go index ba21e81ba80..e86c8144931 100644 --- a/modules/light-clients/07-tendermint/types/store.go +++ b/modules/light-clients/07-tendermint/types/store.go @@ -295,8 +295,17 @@ func bigEndianHeightBytes(height exported.Height) []byte { // client state and consensus state will be set by client keeper // set iteration key to provide ability for efficient ordered iteration of consensus states. func setConsensusMetadata(ctx sdk.Context, clientStore sdk.KVStore, height exported.Height) { - SetProcessedTime(clientStore, height, uint64(ctx.BlockTime().UnixNano())) - SetProcessedHeight(clientStore, height, clienttypes.GetSelfHeight(ctx)) + setConsensusMetadataWithValues(clientStore, height, clienttypes.GetSelfHeight(ctx), uint64(ctx.BlockTime().UnixNano())) +} + +// setConsensusMetadataWithValues sets the consensus metadata with the provided values +func setConsensusMetadataWithValues( + clientStore sdk.KVStore, height, + processedHeight exported.Height, + processedTime uint64, +) { + SetProcessedTime(clientStore, height, processedTime) + SetProcessedHeight(clientStore, height, processedHeight) SetIterationKey(clientStore, height) } diff --git a/modules/light-clients/09-localhost/types/client_state.go b/modules/light-clients/09-localhost/types/client_state.go index a0615b8fda9..4fe00390cc6 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 e2dbe89b78b..46691a52391 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