From a9b034b5f3e6086d71d6f89f668e292f48df5ad1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 3 Mar 2021 17:40:05 +0100 Subject: [PATCH] Emit header in MsgUpdateClient events (#8624) * emit header in update client msg * update CHANGELOG * update spec * fix nil header bug * use JSON encoding for emitting header * Update x/ibc/core/spec/06_events.md * use proto for encoding * add tests * encode to hex before emitting header in event * add comment addressing reasoning for hex encoding * Update CHANGELOG.md Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Co-authored-by: Alessio Treglia Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> --- CHANGELOG.md | 1 + x/ibc/core/02-client/keeper/client.go | 13 ++++++++ x/ibc/core/02-client/keeper/client_test.go | 37 +++++++++++++++++++++ x/ibc/core/02-client/types/encoding.go | 33 ++++++++++++++++-- x/ibc/core/02-client/types/encoding_test.go | 30 +++++++++++++++++ x/ibc/core/02-client/types/events.go | 1 + x/ibc/core/spec/06_events.md | 2 +- 7 files changed, 113 insertions(+), 4 deletions(-) create mode 100644 x/ibc/core/02-client/types/encoding_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c9dc44c7..188ff0095 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,6 +70,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/ibc) [\#7949](https://github.com/cosmos/cosmos-sdk/issues/7949) Standardized channel `Acknowledgement` moved to its own file. Codec registration redundancy removed. * (crypto/types) [\#8600](https://github.com/cosmos/cosmos-sdk/pull/8600) `CompactBitArray`: optimize the `NumTrueBitsBefore` method and add an `Equal` method. * (store/cachekv), (x/bank/types) [\#8719](https://github.com/cosmos/cosmos-sdk/pull/8719) algorithmically fix pathologically slow code +* (x/ibc) [\#8624](https://github.com/cosmos/cosmos-sdk/pull/8624) Emit full header in IBC UpdateClient message. ### Bug Fixes diff --git a/x/ibc/core/02-client/keeper/client.go b/x/ibc/core/02-client/keeper/client.go index 672dcf5d7..e822402bf 100644 --- a/x/ibc/core/02-client/keeper/client.go +++ b/x/ibc/core/02-client/keeper/client.go @@ -1,6 +1,8 @@ package keeper import ( + "encoding/hex" + "github.com/armon/go-metrics" "github.com/cosmos/cosmos-sdk/telemetry" @@ -95,6 +97,16 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.H ) }() + // emit the full header in events + var headerStr string + if header != nil { + // Marshal the Header 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. + headerStr = hex.EncodeToString(types.MustMarshalHeader(k.cdc, header)) + + } + // emitting events in the keeper emits for both begin block and handler client updates ctx.EventManager().EmitEvent( sdk.NewEvent( @@ -102,6 +114,7 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.H sdk.NewAttribute(types.AttributeKeyClientID, clientID), sdk.NewAttribute(types.AttributeKeyClientType, clientState.ClientType()), sdk.NewAttribute(types.AttributeKeyConsensusHeight, consensusHeight.String()), + sdk.NewAttribute(types.AttributeKeyHeader, headerStr), ), ) diff --git a/x/ibc/core/02-client/keeper/client_test.go b/x/ibc/core/02-client/keeper/client_test.go index 9eb816adf..320837a92 100644 --- a/x/ibc/core/02-client/keeper/client_test.go +++ b/x/ibc/core/02-client/keeper/client_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + "encoding/hex" "fmt" "time" @@ -589,3 +590,39 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { }) } } + +func (suite *KeeperTestSuite) TestUpdateClientEventEmission() { + clientID, _ := suite.coordinator.SetupClients(suite.chainA, suite.chainB, exported.Tendermint) + header, err := suite.chainA.ConstructUpdateTMClientHeader(suite.chainB, clientID) + suite.Require().NoError(err) + + msg, err := clienttypes.NewMsgUpdateClient( + clientID, header, + suite.chainA.SenderAccount.GetAddress(), + ) + + result, err := suite.chainA.SendMsgs(msg) + suite.Require().NoError(err) + // first event type is "message" + updateEvent := result.Events[1] + + suite.Require().Equal(clienttypes.EventTypeUpdateClient, updateEvent.Type) + + // use a boolean to ensure the update event contains the header + contains := false + for _, attr := range updateEvent.Attributes { + if string(attr.Key) == clienttypes.AttributeKeyHeader { + contains = true + + bz, err := hex.DecodeString(string(attr.Value)) + suite.Require().NoError(err) + + emittedHeader, err := types.UnmarshalHeader(suite.chainA.App.AppCodec(), bz) + suite.Require().NoError(err) + suite.Require().Equal(header, emittedHeader) + } + + } + suite.Require().True(contains) + +} diff --git a/x/ibc/core/02-client/types/encoding.go b/x/ibc/core/02-client/types/encoding.go index a912b13ab..862148456 100644 --- a/x/ibc/core/02-client/types/encoding.go +++ b/x/ibc/core/02-client/types/encoding.go @@ -57,7 +57,7 @@ func MustUnmarshalConsensusState(cdc codec.BinaryMarshaler, bz []byte) exported. return consensusState } -// MustMarshalConsensusState attempts to encode an ConsensusState object and returns the +// MustMarshalConsensusState attempts to encode a ConsensusState object and returns the // raw encoded bytes. It panics on error. func MustMarshalConsensusState(cdc codec.BinaryMarshaler, consensusState exported.ConsensusState) []byte { bz, err := MarshalConsensusState(cdc, consensusState) @@ -68,12 +68,12 @@ func MustMarshalConsensusState(cdc codec.BinaryMarshaler, consensusState exporte return bz } -// MarshalConsensusState protobuf serializes an ConsensusState interface +// MarshalConsensusState protobuf serializes a ConsensusState interface func MarshalConsensusState(cdc codec.BinaryMarshaler, cs exported.ConsensusState) ([]byte, error) { return cdc.MarshalInterface(cs) } -// UnmarshalConsensusState returns an ConsensusState interface from raw encoded clientState +// UnmarshalConsensusState returns a ConsensusState interface from raw encoded consensus state // bytes of a Proto-based ConsensusState type. An error is returned upon decoding // failure. func UnmarshalConsensusState(cdc codec.BinaryMarshaler, bz []byte) (exported.ConsensusState, error) { @@ -84,3 +84,30 @@ func UnmarshalConsensusState(cdc codec.BinaryMarshaler, bz []byte) (exported.Con return consensusState, nil } + +// MarshalHeader protobuf serializes a Header interface +func MarshalHeader(cdc codec.BinaryMarshaler, h exported.Header) ([]byte, error) { + return cdc.MarshalInterface(h) +} + +// MustMarshalHeader attempts to encode a Header object and returns the +// raw encoded bytes. It panics on error. +func MustMarshalHeader(cdc codec.BinaryMarshaler, header exported.Header) []byte { + bz, err := MarshalHeader(cdc, header) + if err != nil { + panic(fmt.Errorf("failed to encode header: %w", err)) + } + + return bz +} + +// UnmarshalHeader returns a Header interface from raw proto encoded header bytes. +// An error is returned upon decoding failure. +func UnmarshalHeader(cdc codec.BinaryMarshaler, bz []byte) (exported.Header, error) { + var header exported.Header + if err := cdc.UnmarshalInterface(bz, &header); err != nil { + return nil, err + } + + return header, nil +} diff --git a/x/ibc/core/02-client/types/encoding_test.go b/x/ibc/core/02-client/types/encoding_test.go new file mode 100644 index 000000000..89953bc9f --- /dev/null +++ b/x/ibc/core/02-client/types/encoding_test.go @@ -0,0 +1,30 @@ +package types_test + +import ( + "github.com/cosmos/cosmos-sdk/x/ibc/core/02-client/types" + ibctmtypes "github.com/cosmos/cosmos-sdk/x/ibc/light-clients/07-tendermint/types" +) + +func (suite *TypesTestSuite) TestMarshalHeader() { + + cdc := suite.chainA.App.AppCodec() + h := &ibctmtypes.Header{ + TrustedHeight: types.NewHeight(4, 100), + } + + // marshal header + bz, err := types.MarshalHeader(cdc, h) + suite.Require().NoError(err) + + // unmarshal header + newHeader, err := types.UnmarshalHeader(cdc, bz) + suite.Require().NoError(err) + + suite.Require().Equal(h, newHeader) + + // use invalid bytes + invalidHeader, err := types.UnmarshalHeader(cdc, []byte("invalid bytes")) + suite.Require().Error(err) + suite.Require().Nil(invalidHeader) + +} diff --git a/x/ibc/core/02-client/types/events.go b/x/ibc/core/02-client/types/events.go index d0760ba89..f8812e651 100644 --- a/x/ibc/core/02-client/types/events.go +++ b/x/ibc/core/02-client/types/events.go @@ -12,6 +12,7 @@ const ( AttributeKeySubjectClientID = "subject_client_id" AttributeKeyClientType = "client_type" AttributeKeyConsensusHeight = "consensus_height" + AttributeKeyHeader = "header" ) // IBC client events vars diff --git a/x/ibc/core/spec/06_events.md b/x/ibc/core/spec/06_events.md index 528a30cff..8a416217e 100644 --- a/x/ibc/core/spec/06_events.md +++ b/x/ibc/core/spec/06_events.md @@ -32,6 +32,7 @@ callbacks to IBC applications. | update_client | client_id | {clientId} | | update_client | client_type | {clientType} | | update_client | consensus_height | {consensusHeight} | +| update_client | header | {header} | | message | action | update_client | | message | module | ibc_client | @@ -238,4 +239,3 @@ callbacks to IBC applications. | message | action | timeout_packet | | message | module | ibc-channel | -