diff --git a/modules/core/02-client/keeper/client.go b/modules/core/02-client/keeper/client.go index e702c31fe5c..a0ab6348dcf 100644 --- a/modules/core/02-client/keeper/client.go +++ b/modules/core/02-client/keeper/client.go @@ -70,14 +70,6 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, clientMsg exporte return err } - // Marshal the ClientMessage as an Any and encode the resulting bytes to hex. - // This prevents the event value from containing invalid UTF-8 characters - // which may cause data to be lost when JSON encoding/decoding. - clientMsgStr := hex.EncodeToString(types.MustMarshalClientMessage(k.cdc, clientMsg)) - - // set default consensus height with header height - consensusHeight := clientMsg.GetHeight() - foundMisbehaviour := clientState.CheckForMisbehaviour(ctx, k.cdc, clientStore, clientMsg) if foundMisbehaviour { clientState.UpdateStateOnMisbehaviour(ctx, k.cdc, clientStore, clientMsg) @@ -96,14 +88,14 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, clientMsg exporte ) }() - EmitSubmitMisbehaviourEventOnUpdate(ctx, clientID, clientState.ClientType(), consensusHeight, clientMsgStr) + EmitSubmitMisbehaviourEvent(ctx, clientID, clientState) return nil } - clientState.UpdateState(ctx, k.cdc, clientStore, clientMsg) + consensusHeights := clientState.UpdateState(ctx, k.cdc, clientStore, clientMsg) - k.Logger(ctx).Info("client state updated", "client-id", clientID, "height", consensusHeight.String()) + k.Logger(ctx).Info("client state updated", "client-id", clientID, "heights", consensusHeights) defer func() { telemetry.IncrCounterWithLabels( @@ -117,8 +109,13 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, clientMsg exporte ) }() + // Marshal the ClientMessage as an Any and encode the resulting bytes to hex. + // This prevents the event value from containing invalid UTF-8 characters + // which may cause data to be lost when JSON encoding/decoding. + clientMsgStr := hex.EncodeToString(types.MustMarshalClientMessage(k.cdc, clientMsg)) + // emitting events in the keeper emits for both begin block and handler client updates - EmitUpdateClientEvent(ctx, clientID, clientState.ClientType(), consensusHeight, clientMsgStr) + EmitUpdateClientEvent(ctx, clientID, clientState.ClientType(), consensusHeights, clientMsgStr) return nil } diff --git a/modules/core/02-client/keeper/events.go b/modules/core/02-client/keeper/events.go index 4e2f38941a0..d49d41cb1cb 100644 --- a/modules/core/02-client/keeper/events.go +++ b/modules/core/02-client/keeper/events.go @@ -1,6 +1,8 @@ package keeper import ( + "strings" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/ibc-go/v3/modules/core/02-client/types" @@ -24,13 +26,26 @@ func EmitCreateClientEvent(ctx sdk.Context, clientID string, clientState exporte } // EmitUpdateClientEvent emits an update client event -func EmitUpdateClientEvent(ctx sdk.Context, clientID string, clientType string, consensusHeight exported.Height, clientMsgStr string) { +func EmitUpdateClientEvent(ctx sdk.Context, clientID string, clientType string, consensusHeights []exported.Height, clientMsgStr string) { + var consensusHeightAttr string + if len(consensusHeights) != 0 { + consensusHeightAttr = consensusHeights[0].String() + } + + var consensusHeightsAttr []string + for _, height := range consensusHeights { + consensusHeightsAttr = append(consensusHeightsAttr, height.String()) + } + ctx.EventManager().EmitEvents(sdk.Events{ sdk.NewEvent( types.EventTypeUpdateClient, sdk.NewAttribute(types.AttributeKeyClientID, clientID), sdk.NewAttribute(types.AttributeKeyClientType, clientType), - sdk.NewAttribute(types.AttributeKeyConsensusHeight, consensusHeight.String()), + // Deprecated: AttributeKeyConsensusHeight is deprecated and will be removed in a future release. + // Please use AttributeKeyConsensusHeights instead. + sdk.NewAttribute(types.AttributeKeyConsensusHeight, consensusHeightAttr), + sdk.NewAttribute(types.AttributeKeyConsensusHeights, strings.Join(consensusHeightsAttr, ",")), sdk.NewAttribute(types.AttributeKeyHeader, clientMsgStr), ), sdk.NewEvent( @@ -78,16 +93,3 @@ func EmitSubmitMisbehaviourEvent(ctx sdk.Context, clientID string, clientState e ), ) } - -// EmitSubmitMisbehaviourEventOnUpdate emits a client misbehaviour event on a client update event -func EmitSubmitMisbehaviourEventOnUpdate(ctx sdk.Context, clientID string, clientType string, consensusHeight exported.Height, headerStr string) { - ctx.EventManager().EmitEvent( - sdk.NewEvent( - types.EventTypeSubmitMisbehaviour, - sdk.NewAttribute(types.AttributeKeyClientID, clientID), - sdk.NewAttribute(types.AttributeKeyClientType, clientType), - sdk.NewAttribute(types.AttributeKeyConsensusHeight, consensusHeight.String()), - sdk.NewAttribute(types.AttributeKeyHeader, headerStr), - ), - ) -} diff --git a/modules/core/02-client/legacy/v100/solomachine.go b/modules/core/02-client/legacy/v100/solomachine.go index 19e5459206c..3080c5c8ce4 100644 --- a/modules/core/02-client/legacy/v100/solomachine.go +++ b/modules/core/02-client/legacy/v100/solomachine.go @@ -108,7 +108,7 @@ func (cs *ClientState) VerifyClientMessage( } // UpdateState panis! -func (cs *ClientState) UpdateState(_ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore, _ exported.ClientMessage) error { +func (cs *ClientState) UpdateState(_ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore, _ exported.ClientMessage) []exported.Height { panic("legacy solo machine is deprecated!") } diff --git a/modules/core/02-client/types/events.go b/modules/core/02-client/types/events.go index 391e1e37080..05708a84859 100644 --- a/modules/core/02-client/types/events.go +++ b/modules/core/02-client/types/events.go @@ -8,11 +8,12 @@ import ( // IBC client events const ( - AttributeKeyClientID = "client_id" - AttributeKeySubjectClientID = "subject_client_id" - AttributeKeyClientType = "client_type" - AttributeKeyConsensusHeight = "consensus_height" - AttributeKeyHeader = "header" + AttributeKeyClientID = "client_id" + AttributeKeySubjectClientID = "subject_client_id" + AttributeKeyClientType = "client_type" + AttributeKeyConsensusHeight = "consensus_height" + AttributeKeyConsensusHeights = "consensus_heights" + AttributeKeyHeader = "header" ) // IBC client events vars diff --git a/modules/core/exported/client.go b/modules/core/exported/client.go index 70ca542199d..2521fe5fd42 100644 --- a/modules/core/exported/client.go +++ b/modules/core/exported/client.go @@ -65,8 +65,8 @@ type ClientState interface { VerifyClientMessage(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, clientMsg ClientMessage) error // UpdateState updates and stores as necessary any associated information for an IBC client, such as the ClientState and corresponding ConsensusState. - // An error is returned if ClientMessage is of type Misbehaviour - UpdateState(sdk.Context, codec.BinaryCodec, sdk.KVStore, ClientMessage) error + // Upon successful update, a list of consensus heights is returned. It assumes the ClientMessage has already been verified. + UpdateState(sdk.Context, codec.BinaryCodec, sdk.KVStore, ClientMessage) []Height // Update and Misbehaviour functions CheckSubstituteAndUpdateState(ctx sdk.Context, cdc codec.BinaryCodec, subjectClientStore, substituteClientStore sdk.KVStore, substituteClient ClientState) (ClientState, error) @@ -206,7 +206,6 @@ type ConsensusState interface { type ClientMessage interface { proto.Message - GetHeight() Height ClientType() string ValidateBasic() error } diff --git a/modules/light-clients/06-solomachine/types/header.go b/modules/light-clients/06-solomachine/types/header.go index eb2cbe5f8b3..a6639a0f0c9 100644 --- a/modules/light-clients/06-solomachine/types/header.go +++ b/modules/light-clients/06-solomachine/types/header.go @@ -17,13 +17,6 @@ func (Header) ClientType() string { return exported.Solomachine } -// GetHeight returns the current sequence number as the height. -// Return clientexported.Height to satisfy interface -// Revision number is always 0 for a solo-machine -func (h Header) GetHeight() exported.Height { - return clienttypes.NewHeight(0, h.Sequence) -} - // GetPubKey unmarshals the new public key into a cryptotypes.PubKey type. // An error is returned if the new public key is nil or the cached value // is not a PubKey. diff --git a/modules/light-clients/06-solomachine/types/update.go b/modules/light-clients/06-solomachine/types/update.go index 0fc5c76544f..e8c464772f7 100644 --- a/modules/light-clients/06-solomachine/types/update.go +++ b/modules/light-clients/06-solomachine/types/update.go @@ -1,6 +1,8 @@ package types import ( + "fmt" + "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -81,10 +83,11 @@ func (cs ClientState) verifyMisbehaviour(ctx sdk.Context, cdc codec.BinaryCodec, } // UpdateState updates the consensus state to the new public key and an incremented sequence. -func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, clientMsg exported.ClientMessage) error { +// A list containing the updated consensus height is returned. +func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, clientMsg exported.ClientMessage) []exported.Height { smHeader, ok := clientMsg.(*Header) if !ok { - return sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "expected %T got %T", Header{}, clientMsg) + panic(fmt.Errorf("unsupported ClientMessage: %T", clientMsg)) } // create new solomachine ConsensusState @@ -99,7 +102,7 @@ func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, client clientStore.Set(host.ClientStateKey(), clienttypes.MustMarshalClientState(cdc, &cs)) - return nil + return []exported.Height{clienttypes.NewHeight(0, cs.Sequence)} } // CheckForMisbehaviour returns true for type Misbehaviour (passed VerifyClientMessage check), otherwise returns false diff --git a/modules/light-clients/06-solomachine/types/update_test.go b/modules/light-clients/06-solomachine/types/update_test.go index 7e3acaf0637..223e468e87f 100644 --- a/modules/light-clients/06-solomachine/types/update_test.go +++ b/modules/light-clients/06-solomachine/types/update_test.go @@ -441,23 +441,27 @@ func (suite *SoloMachineTestSuite) TestUpdateState() { suite.Run(tc.name, func() { tc.setup() // setup test - err := clientState.UpdateState(suite.chainA.GetContext(), suite.chainA.Codec, suite.store, clientMsg) - if tc.expPass { - suite.Require().NoError(err) + consensusHeights := clientState.UpdateState(suite.chainA.GetContext(), suite.chainA.Codec, suite.store, clientMsg) clientStateBz := suite.store.Get(host.ClientStateKey()) suite.Require().NotEmpty(clientStateBz) newClientState := clienttypes.MustUnmarshalClientState(suite.chainA.Codec, clientStateBz) + suite.Require().Len(consensusHeights, 1) + suite.Require().Equal(uint64(0), consensusHeights[0].GetRevisionNumber()) + suite.Require().Equal(newClientState.(*types.ClientState).Sequence, consensusHeights[0].GetRevisionHeight()) + suite.Require().False(newClientState.(*types.ClientState).IsFrozen) suite.Require().Equal(clientMsg.(*types.Header).Sequence+1, newClientState.(*types.ClientState).Sequence) suite.Require().Equal(clientMsg.(*types.Header).NewPublicKey, newClientState.(*types.ClientState).ConsensusState.PublicKey) suite.Require().Equal(clientMsg.(*types.Header).NewDiversifier, newClientState.(*types.ClientState).ConsensusState.Diversifier) suite.Require().Equal(clientMsg.(*types.Header).Timestamp, newClientState.(*types.ClientState).ConsensusState.Timestamp) } else { - suite.Require().Error(err) + suite.Require().Panics(func() { + clientState.UpdateState(suite.chainA.GetContext(), suite.chainA.Codec, suite.store, clientMsg) + }) } }) diff --git a/modules/light-clients/07-tendermint/types/update.go b/modules/light-clients/07-tendermint/types/update.go index 18849096bba..b862e80fd30 100644 --- a/modules/light-clients/07-tendermint/types/update.go +++ b/modules/light-clients/07-tendermint/types/update.go @@ -2,6 +2,7 @@ package types import ( "bytes" + "fmt" "reflect" "github.com/cosmos/cosmos-sdk/codec" @@ -154,19 +155,20 @@ func (cs *ClientState) verifyHeader( // If we are updating to a past height, a consensus state is created for that height to be persisted in client store // If we are updating to a future height, the consensus state is created and the client state is updated to reflect // the new latest height +// A list containing the updated consensus height is returned. // UpdateState must only be used to update within a single revision, thus header revision number and trusted height's revision // number must be the same. To update to a new revision, use a separate upgrade path // UpdateState will prune the oldest consensus state if it is expired. -func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, clientMsg exported.ClientMessage) error { +func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, clientMsg exported.ClientMessage) []exported.Height { header, ok := clientMsg.(*Header) if !ok { - return sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "expected type %T, got %T", &Header{}, header) + panic(fmt.Errorf("expected type %T, got %T", &Header{}, clientMsg)) } // check for duplicate update if consensusState, _ := GetConsensusState(clientStore, cdc, header.GetHeight()); consensusState != nil { // perform no-op - return nil + return []exported.Height{header.GetHeight()} } cs.pruneOldestConsensusState(ctx, cdc, clientStore) @@ -175,6 +177,7 @@ func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, client if height.GT(cs.LatestHeight) { cs.LatestHeight = height } + consensusState := &ConsensusState{ Timestamp: header.GetTime(), Root: commitmenttypes.NewMerkleRoot(header.Header.GetAppHash()), @@ -186,7 +189,7 @@ func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, client setConsensusState(clientStore, cdc, consensusState, header.GetHeight()) setConsensusMetadata(ctx, clientStore, header.GetHeight()) - return nil + return []exported.Height{height} } // pruneOldestConsensusState will retrieve the earliest consensus state for this clientID and check if it is expired. If it is, diff --git a/modules/light-clients/07-tendermint/types/update_test.go b/modules/light-clients/07-tendermint/types/update_test.go index 022e6d1ba36..59326645248 100644 --- a/modules/light-clients/07-tendermint/types/update_test.go +++ b/modules/light-clients/07-tendermint/types/update_test.go @@ -305,6 +305,7 @@ func (suite *TendermintTestSuite) TestUpdateState() { path *ibctesting.Path clientMessage exported.ClientMessage clientStore sdk.KVStore + consensusHeights []exported.Height pruneHeight clienttypes.Height prevClientState exported.ClientState prevConsensusState exported.ConsensusState @@ -318,10 +319,17 @@ func (suite *TendermintTestSuite) TestUpdateState() { }{ { "success with height later than latest height", func() { - suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().LT(clientMessage.GetHeight())) + tmHeader, ok := clientMessage.(*types.Header) + suite.Require().True(ok) + suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().LT(tmHeader.GetHeight())) }, func() { - suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().EQ(clientMessage.GetHeight())) // new update, updated client state should have changed + tmHeader, ok := clientMessage.(*types.Header) + suite.Require().True(ok) + + clientState := path.EndpointA.GetClientState() + suite.Require().True(clientState.GetLatestHeight().EQ(tmHeader.GetHeight())) // new update, updated client state should have changed + suite.Require().True(clientState.GetLatestHeight().EQ(consensusHeights[0])) }, true, }, { @@ -332,12 +340,16 @@ func (suite *TendermintTestSuite) TestUpdateState() { err := path.EndpointA.UpdateClient() suite.Require().NoError(err) - suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().GT(clientMessage.GetHeight())) + tmHeader, ok := clientMessage.(*types.Header) + suite.Require().True(ok) + suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().GT(tmHeader.GetHeight())) prevClientState = path.EndpointA.GetClientState() }, func() { - suite.Require().Equal(path.EndpointA.GetClientState(), prevClientState) // fill in height, no change to client state + clientState := path.EndpointA.GetClientState() + suite.Require().Equal(clientState, prevClientState) // fill in height, no change to client state + suite.Require().True(clientState.GetLatestHeight().GT(consensusHeights[0])) }, true, }, { @@ -349,14 +361,22 @@ func (suite *TendermintTestSuite) TestUpdateState() { // use the same header which just updated the client clientMessage, err = path.EndpointA.Chain.ConstructUpdateTMClientHeader(path.EndpointA.Counterparty.Chain, path.EndpointA.ClientID) suite.Require().NoError(err) - suite.Require().Equal(path.EndpointA.GetClientState().GetLatestHeight(), clientMessage.GetHeight()) + + tmHeader, ok := clientMessage.(*types.Header) + suite.Require().True(ok) + suite.Require().Equal(path.EndpointA.GetClientState().GetLatestHeight(), tmHeader.GetHeight()) prevClientState = path.EndpointA.GetClientState() - prevConsensusState = path.EndpointA.GetConsensusState(clientMessage.GetHeight()) + prevConsensusState = path.EndpointA.GetConsensusState(tmHeader.GetHeight()) }, func() { - suite.Require().Equal(path.EndpointA.GetClientState(), prevClientState) - suite.Require().Equal(path.EndpointA.GetConsensusState(clientMessage.GetHeight()), prevConsensusState) + clientState := path.EndpointA.GetClientState() + suite.Require().Equal(clientState, prevClientState) + suite.Require().True(clientState.GetLatestHeight().EQ(consensusHeights[0])) + + tmHeader, ok := clientMessage.(*types.Header) + suite.Require().True(ok) + suite.Require().Equal(path.EndpointA.GetConsensusState(tmHeader.GetHeight()), prevConsensusState) }, true, }, { @@ -385,7 +405,12 @@ func (suite *TendermintTestSuite) TestUpdateState() { suite.Require().NoError(err) }, func() { - suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().EQ(clientMessage.GetHeight())) // new update, updated client state should have changed + tmHeader, ok := clientMessage.(*types.Header) + suite.Require().True(ok) + + clientState := path.EndpointA.GetClientState() + suite.Require().True(clientState.GetLatestHeight().EQ(tmHeader.GetHeight())) // new update, updated client state should have changed + suite.Require().True(clientState.GetLatestHeight().EQ(consensusHeights[0])) // ensure consensus state was pruned _, found := path.EndpointA.Chain.GetConsensusState(path.EndpointA.ClientID, pruneHeight) @@ -396,7 +421,8 @@ func (suite *TendermintTestSuite) TestUpdateState() { "invalid ClientMessage type", func() { clientMessage = &types.Misbehaviour{} }, - func() {}, false, + func() {}, + false, }, } for _, tc := range testCases { @@ -417,12 +443,10 @@ func (suite *TendermintTestSuite) TestUpdateState() { tc.malleate() clientState := path.EndpointA.GetClientState() - clientStore = suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID) - err = clientState.UpdateState(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), clientStore, clientMessage) if tc.expPass { - suite.Require().NoError(err) + consensusHeights = clientState.UpdateState(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), clientStore, clientMessage) header := clientMessage.(*types.Header) expConsensusState := &types.ConsensusState{ @@ -437,7 +461,9 @@ func (suite *TendermintTestSuite) TestUpdateState() { suite.Require().Equal(expConsensusState, updatedConsensusState) } else { - suite.Require().Error(err) + suite.Require().Panics(func() { + clientState.UpdateState(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), clientStore, clientMessage) + }) } // perform custom checks @@ -556,7 +582,9 @@ func (suite *TendermintTestSuite) TestCheckForMisbehaviour() { NextValidatorsHash: header.Header.NextValidatorsHash, } - suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), path.EndpointA.ClientID, clientMessage.GetHeight(), consensusState) + tmHeader, ok := clientMessage.(*types.Header) + suite.Require().True(ok) + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), path.EndpointA.ClientID, tmHeader.GetHeight(), consensusState) }, false, }, @@ -572,7 +600,9 @@ func (suite *TendermintTestSuite) TestCheckForMisbehaviour() { NextValidatorsHash: header.Header.NextValidatorsHash, } - suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), path.EndpointA.ClientID, clientMessage.GetHeight(), consensusState) + tmHeader, ok := clientMessage.(*types.Header) + suite.Require().True(ok) + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), path.EndpointA.ClientID, tmHeader.GetHeight(), consensusState) }, true, },