Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: remove GetHeight from ClientMessage interface #1285

Merged
21 changes: 9 additions & 12 deletions modules/core/02-client/keeper/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
damiannolan marked this conversation as resolved.
Show resolved Hide resolved

// 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)
Expand All @@ -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(
Expand All @@ -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
}
Expand Down
32 changes: 17 additions & 15 deletions modules/core/02-client/keeper/events.go
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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(
Expand Down Expand Up @@ -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),
),
)
}
2 changes: 1 addition & 1 deletion modules/core/02-client/legacy/v100/solomachine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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!")
}

Expand Down
11 changes: 6 additions & 5 deletions modules/core/02-client/types/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
damiannolan marked this conversation as resolved.
Show resolved Hide resolved
AttributeKeyConsensusHeights = "consensus_heights"
AttributeKeyHeader = "header"
)

// IBC client events vars
Expand Down
5 changes: 2 additions & 3 deletions modules/core/exported/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,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)
Expand Down Expand Up @@ -203,7 +203,6 @@ type ConsensusState interface {
type ClientMessage interface {
proto.Message

GetHeight() Height
ClientType() string
ValidateBasic() error
}
Expand Down
7 changes: 0 additions & 7 deletions modules/light-clients/06-solomachine/types/header.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 6 additions & 3 deletions modules/light-clients/06-solomachine/types/update.go
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
12 changes: 8 additions & 4 deletions modules/light-clients/06-solomachine/types/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}

})
Expand Down
11 changes: 7 additions & 4 deletions modules/light-clients/07-tendermint/types/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package types

import (
"bytes"
"fmt"
"reflect"

"github.com/cosmos/cosmos-sdk/codec"
Expand Down Expand Up @@ -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)
Expand All @@ -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()),
Expand All @@ -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,
Expand Down
Loading