From 8942d3ce94fa44cd8896709855d576b54946fc5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 16 Mar 2022 16:42:13 +0100 Subject: [PATCH] refactor: modify VerifyUpgradeAndUpdateState to set upgraded client and consensus state (#598) * modify VerifyUpgradeAndUpdateState interface function to remove returned client and consensus state removes the client and consensus state from the return values in VerifyUpgradeAndUpdateState client state interface function Updates light client implementations to set client and consensus state in client store Fixes and updates tests * add changelog entry * add migration docs * use upgraded client to emit height in events --- CHANGELOG.md | 2 + docs/migrations/v3-to-v4.md | 112 +--- modules/core/02-client/keeper/client.go | 16 +- modules/core/exported/client.go | 3 +- .../06-solomachine/types/client_state.go | 491 ++++++++++++++++++ modules/light-clients/07-tendermint/store.go | 58 ++- .../light-clients/07-tendermint/upgrade.go | 30 +- .../07-tendermint/upgrade_test.go | 47 +- .../09-localhost/types/client_state.go | 4 +- 9 files changed, 586 insertions(+), 177 deletions(-) create mode 100644 modules/light-clients/06-solomachine/types/client_state.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 929183fb2fe..02777a689bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (core) [\#709](https://github.com/cosmos/ibc-go/pull/709) Replace github.com/pkg/errors with stdlib errors ### API Breaking + +* (02-client) [\#598](https://github.com/cosmos/ibc-go/pull/598) The client state and consensus state return value has been removed from `VerifyUpgradeAndUpdateState`. Light client implementations must update the client state and consensus state after verifying a valid client upgrade. * (testing) [\#939](https://github.com/cosmos/ibc-go/pull/939) Support custom power reduction for testing. * (modules/core/05-port) [\#1086](https://github.com/cosmos/ibc-go/pull/1086) Added `counterpartyChannelID` argument to IBCModule.OnChanOpenAck * (06-solomachine) [\#1100](https://github.com/cosmos/ibc-go/pull/1100) Remove `GetClientID` function from 06-solomachine `Misbehaviour` type. diff --git a/docs/migrations/v3-to-v4.md b/docs/migrations/v3-to-v4.md index 1526b1642e0..f933a7380f7 100644 --- a/docs/migrations/v3-to-v4.md +++ b/docs/migrations/v3-to-v4.md @@ -1,4 +1,4 @@ -# Migrating from ibc-go v3 to v4 +# Migrating from ibc-go v2 to v3 This document is intended to highlight significant changes which may require more information than presented in the CHANGELOG. Any changes that must be done by a user of ibc-go should be documented here. @@ -18,115 +18,13 @@ No genesis or in-place migrations required when upgrading from v1 or v2 of ibc-g ## Chains -### Migration to fix support for base denoms with slashes - -As part of [v1.5.0](https://github.com/cosmos/ibc-go/releases/tag/v1.5.0), [v2.3.0](https://github.com/cosmos/ibc-go/releases/tag/v2.3.0) and [v3.1.0](https://github.com/cosmos/ibc-go/releases/tag/v3.1.0) some [migration handler code sample was documented](https://github.com/cosmos/ibc-go/blob/main/docs/migrations/support-denoms-with-slashes.md#upgrade-proposal) that needs to run in order to correct the trace information of coins transferred using ICS20 whose base denom contains slashes. - -Based on feedback from the community we add now an improved solution to run the same migration that does not require copying a large piece of code over from the migration document, but instead requires only adding a one-line upgrade handler. - -If the chain will migrate to supporting base denoms with slashes, it must set the appropriate params during the execution of the upgrade handler in `app.go`: -```go -app.UpgradeKeeper.SetUpgradeHandler("MigrateTraces", - func(ctx sdk.Context, _ upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { - // transfer module consensus version has been bumped to 2 - return app.mm.RunMigrations(ctx, app.configurator, fromVM) - }) - -``` - -If a chain receives coins of a base denom with slashes before it upgrades to supporting it, the receive may pass however the trace information will be incorrect. - -E.g. If a base denom of `testcoin/testcoin/testcoin` is sent to a chain that does not support slashes in the base denom, the receive will be successful. However, the trace information stored on the receiving chain will be: `Trace: "transfer/{channel-id}/testcoin/testcoin", BaseDenom: "testcoin"`. - -This incorrect trace information must be corrected when the chain does upgrade to fully supporting denominations with slashes. - -## IBC Apps - -### ICS03 - Connection - -Crossing hellos have been removed from 03-connection handshake negotiation. -`PreviousConnectionId` in `MsgConnectionOpenTry` has been deprecated and is no longer used by core IBC. - -`NewMsgConnectionOpenTry` no longer takes in the `PreviousConnectionId` as crossing hellos are no longer supported. A non-empty `PreviousConnectionId` will fail basic validation for this message. - -### ICS04 - Channel +### IS04 - Channel The `WriteAcknowledgement` API now takes the `exported.Acknowledgement` type instead of passing in the acknowledgement byte array directly. This is an API breaking change and as such IBC application developers will have to update any calls to `WriteAcknowledgement`. -The `OnChanOpenInit` application callback has been modified. -The return signature now includes the application version as detailed in the latest IBC [spec changes](https://github.com/cosmos/ibc/pull/629). - -The `NewErrorAcknowledgement` method signature has changed. -It now accepts an `error` rather than a `string`. This was done in order to prevent accidental state changes. -All error acknowledgements now contain a deterministic ABCI code and error message. It is the responsibility of the application developer to emit error details in events. - -Crossing hellos have been removed from 04-channel handshake negotiation. -IBC Applications no longer need to account from already claimed capabilities in the `OnChanOpenTry` callback. The capability provided by core IBC must be able to be claimed with error. -`PreviousChannelId` in `MsgChannelOpenTry` has been deprecated and is no longer used by core IBC. - -`NewMsgChannelOpenTry` no longer takes in the `PreviousChannelId` as crossing hellos are no longer supported. A non-empty `PreviousChannelId` will fail basic validation for this message. - -### ICS27 - Interchain Accounts - -The `RegisterInterchainAccount` API has been modified to include an additional `version` argument. This change has been made in order to support ICS29 fee middleware, for relayer incentivization of ICS27 packets. -Consumers of the `RegisterInterchainAccount` are now expected to build the appropriate JSON encoded version string themselves and pass it accordingly. -This should be constructed within the interchain accounts authentication module which leverages the APIs exposed via the interchain accounts `controllerKeeper`. If an empty string is passed in the `version` argument, then the version will be initialized to a default value in the `OnChanOpenInit` callback of the controller's handler, so that channel handshake can proceed. - -The following code snippet illustrates how to construct an appropriate interchain accounts `Metadata` and encode it as a JSON bytestring: - -```go -icaMetadata := icatypes.Metadata{ - Version: icatypes.Version, - ControllerConnectionId: controllerConnectionID, - HostConnectionId: hostConnectionID, - Encoding: icatypes.EncodingProtobuf, - TxType: icatypes.TxTypeSDKMultiMsg, -} - -appVersion, err := icatypes.ModuleCdc.MarshalJSON(&icaMetadata) -if err != nil { - return err -} - -if err := k.icaControllerKeeper.RegisterInterchainAccount(ctx, msg.ConnectionId, msg.Owner, string(appVersion)); err != nil { - return err -} -``` - -Similarly, if the application stack is configured to route through ICS29 fee middleware and a fee enabled channel is desired, construct the appropriate ICS29 `Metadata` type: - -```go -icaMetadata := icatypes.Metadata{ - Version: icatypes.Version, - ControllerConnectionId: controllerConnectionID, - HostConnectionId: hostConnectionID, - Encoding: icatypes.EncodingProtobuf, - TxType: icatypes.TxTypeSDKMultiMsg, -} - -appVersion, err := icatypes.ModuleCdc.MarshalJSON(&icaMetadata) -if err != nil { - return err -} - -feeMetadata := feetypes.Metadata{ - AppVersion: string(appVersion), - FeeVersion: feetypes.Version, -} - -feeEnabledVersion, err := feetypes.ModuleCdc.MarshalJSON(&feeMetadata) -if err != nil { - return err -} - -if err := k.icaControllerKeeper.RegisterInterchainAccount(ctx, msg.ConnectionId, msg.Owner, string(feeEnabledVersion)); err != nil { - return err -} -``` - -## Relayers +## IBC Light Clients -When using the `DenomTrace` gRPC, the full IBC denomination with the `ibc/` prefix may now be passed in. +The `VerifyUpgradeAndUpdateState` function has been modified. The client state and consensus state return value has been removed. -Crossing hellos are no longer supported by core IBC for 03-connection and 04-channel. The handshake should be completed in the logical 4 step process (INIT, TRY, ACK, CONFIRM). +Light clients **must** set the updated client state and consensus state in the client store after verifying a valid client upgrade. diff --git a/modules/core/02-client/keeper/client.go b/modules/core/02-client/keeper/client.go index 78086ddb4d6..bfadcda026e 100644 --- a/modules/core/02-client/keeper/client.go +++ b/modules/core/02-client/keeper/client.go @@ -158,30 +158,26 @@ func (k Keeper) UpgradeClient(ctx sdk.Context, clientID string, upgradedClient e return sdkerrors.Wrapf(types.ErrClientNotActive, "cannot upgrade client (%s) with status %s", clientID, status) } - updatedClientState, updatedConsState, err := clientState.VerifyUpgradeAndUpdateState(ctx, k.cdc, clientStore, - upgradedClient, upgradedConsState, proofUpgradeClient, proofUpgradeConsState) - if err != nil { + if err := clientState.VerifyUpgradeAndUpdateState(ctx, k.cdc, clientStore, + upgradedClient, upgradedConsState, proofUpgradeClient, proofUpgradeConsState, + ); err != nil { return sdkerrors.Wrapf(err, "cannot upgrade client with ID %s", clientID) } - k.SetClientState(ctx, clientID, updatedClientState) - k.SetClientConsensusState(ctx, clientID, updatedClientState.GetLatestHeight(), updatedConsState) - - k.Logger(ctx).Info("client state upgraded", "client-id", clientID, "height", updatedClientState.GetLatestHeight().String()) + k.Logger(ctx).Info("client state upgraded", "client-id", clientID, "height", upgradedClient.GetLatestHeight().String()) defer func() { telemetry.IncrCounterWithLabels( []string{"ibc", "client", "upgrade"}, 1, []metrics.Label{ - telemetry.NewLabel(types.LabelClientType, updatedClientState.ClientType()), + telemetry.NewLabel(types.LabelClientType, upgradedClient.ClientType()), telemetry.NewLabel(types.LabelClientID, clientID), }, ) }() - // emitting events in the keeper emits for client upgrades - EmitUpgradeClientEvent(ctx, clientID, updatedClientState) + EmitUpgradeClientEvent(ctx, clientID, upgradedClient) return nil } diff --git a/modules/core/exported/client.go b/modules/core/exported/client.go index bb473468041..39095aff28f 100644 --- a/modules/core/exported/client.go +++ b/modules/core/exported/client.go @@ -68,6 +68,7 @@ type ClientState interface { // height of the current revision is somehow encoded in the proof verification process. // This is to ensure that no premature upgrades occur, since upgrade plans committed to by the counterparty // may be cancelled or modified before the last planned height. + // If the upgrade is verified, the upgraded client and consensus states must be set in the client store. VerifyUpgradeAndUpdateState( ctx sdk.Context, cdc codec.BinaryCodec, @@ -76,7 +77,7 @@ type ClientState interface { newConsState ConsensusState, proofUpgradeClient, proofUpgradeConsState []byte, - ) (ClientState, ConsensusState, error) + ) error // Utility function that zeroes out any client customizable fields in client state // Ledger enforced fields are maintained while all custom fields are zero values // Used to verify upgrades diff --git a/modules/light-clients/06-solomachine/types/client_state.go b/modules/light-clients/06-solomachine/types/client_state.go new file mode 100644 index 00000000000..ef3088b314f --- /dev/null +++ b/modules/light-clients/06-solomachine/types/client_state.go @@ -0,0 +1,491 @@ +package types + +import ( + "reflect" + + "github.com/cosmos/cosmos-sdk/codec" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/cosmos/cosmos-sdk/types/tx/signing" + + clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types" + commitmenttypes "github.com/cosmos/ibc-go/v3/modules/core/23-commitment/types" + host "github.com/cosmos/ibc-go/v3/modules/core/24-host" + "github.com/cosmos/ibc-go/v3/modules/core/exported" +) + +var _ exported.ClientState = (*ClientState)(nil) + +// NewClientState creates a new ClientState instance. +func NewClientState(latestSequence uint64, consensusState *ConsensusState, allowUpdateAfterProposal bool) *ClientState { + return &ClientState{ + Sequence: latestSequence, + IsFrozen: false, + ConsensusState: consensusState, + AllowUpdateAfterProposal: allowUpdateAfterProposal, + } +} + +// ClientType is Solo Machine. +func (cs ClientState) ClientType() string { + return exported.Solomachine +} + +// GetLatestHeight returns the latest sequence number. +// Return exported.Height to satisfy ClientState interface +// Revision number is always 0 for a solo-machine. +func (cs ClientState) GetLatestHeight() exported.Height { + return clienttypes.NewHeight(0, cs.Sequence) +} + +// Status returns the status of the solo machine client. +// The client may be: +// - Active: if frozen sequence is 0 +// - Frozen: otherwise solo machine is frozen +func (cs ClientState) Status(_ sdk.Context, _ sdk.KVStore, _ codec.BinaryCodec) exported.Status { + if cs.IsFrozen { + return exported.Frozen + } + + return exported.Active +} + +// Validate performs basic validation of the client state fields. +func (cs ClientState) Validate() error { + if cs.Sequence == 0 { + return sdkerrors.Wrap(clienttypes.ErrInvalidClient, "sequence cannot be 0") + } + if cs.ConsensusState == nil { + return sdkerrors.Wrap(clienttypes.ErrInvalidConsensus, "consensus state cannot be nil") + } + return cs.ConsensusState.ValidateBasic() +} + +// ZeroCustomFields returns solomachine client state with client-specific fields FrozenSequence, +// and AllowUpdateAfterProposal zeroed out +func (cs ClientState) ZeroCustomFields() exported.ClientState { + return NewClientState( + cs.Sequence, cs.ConsensusState, false, + ) +} + +// Initialize will check that initial consensus state is equal to the latest consensus state of the initial client. +func (cs ClientState) Initialize(_ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore, consState exported.ConsensusState) error { + if !reflect.DeepEqual(cs.ConsensusState, consState) { + return sdkerrors.Wrapf(clienttypes.ErrInvalidConsensus, "consensus state in initial client does not equal initial consensus state. expected: %s, got: %s", + cs.ConsensusState, consState) + } + return nil +} + +// ExportMetadata is a no-op since solomachine does not store any metadata in client store +func (cs ClientState) ExportMetadata(_ sdk.KVStore) []exported.GenesisMetadata { + return nil +} + +// VerifyUpgradeAndUpdateState returns an error since solomachine client does not support upgrades +func (cs ClientState) VerifyUpgradeAndUpdateState( + _ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore, + _ exported.ClientState, _ exported.ConsensusState, _, _ []byte, +) error { + return sdkerrors.Wrap(clienttypes.ErrInvalidUpgradeClient, "cannot upgrade solomachine client") +} + +// VerifyClientState verifies a proof of the client state of the running chain +// stored on the solo machine. +func (cs *ClientState) VerifyClientState( + store sdk.KVStore, + cdc codec.BinaryCodec, + height exported.Height, + prefix exported.Prefix, + counterpartyClientIdentifier string, + proof []byte, + clientState exported.ClientState, +) error { + // NOTE: the proof height sequence is incremented by one due to the connection handshake verification ordering + height = clienttypes.NewHeight(height.GetRevisionNumber(), height.GetRevisionHeight()+1) + + publicKey, sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof) + if err != nil { + return err + } + + clientPrefixedPath := commitmenttypes.NewMerklePath(host.FullClientStatePath(counterpartyClientIdentifier)) + path, err := commitmenttypes.ApplyPrefix(prefix, clientPrefixedPath) + if err != nil { + return err + } + + signBz, err := ClientStateSignBytes(cdc, sequence, timestamp, cs.ConsensusState.Diversifier, path, clientState) + if err != nil { + return err + } + + if err := VerifySignature(publicKey, signBz, sigData); err != nil { + return err + } + + cs.Sequence++ + cs.ConsensusState.Timestamp = timestamp + setClientState(store, cdc, cs) + return nil +} + +// VerifyClientConsensusState verifies a proof of the consensus state of the +// running chain stored on the solo machine. +func (cs *ClientState) VerifyClientConsensusState( + store sdk.KVStore, + cdc codec.BinaryCodec, + height exported.Height, + counterpartyClientIdentifier string, + consensusHeight exported.Height, + prefix exported.Prefix, + proof []byte, + consensusState exported.ConsensusState, +) error { + // NOTE: the proof height sequence is incremented by two due to the connection handshake verification ordering + height = clienttypes.NewHeight(height.GetRevisionNumber(), height.GetRevisionHeight()+2) + + publicKey, sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof) + if err != nil { + return err + } + + clientPrefixedPath := commitmenttypes.NewMerklePath(host.FullConsensusStatePath(counterpartyClientIdentifier, consensusHeight)) + path, err := commitmenttypes.ApplyPrefix(prefix, clientPrefixedPath) + if err != nil { + return err + } + + signBz, err := ConsensusStateSignBytes(cdc, sequence, timestamp, cs.ConsensusState.Diversifier, path, consensusState) + if err != nil { + return err + } + + if err := VerifySignature(publicKey, signBz, sigData); err != nil { + return err + } + + cs.Sequence++ + cs.ConsensusState.Timestamp = timestamp + setClientState(store, cdc, cs) + return nil +} + +// VerifyConnectionState verifies a proof of the connection state of the +// specified connection end stored on the target machine. +func (cs *ClientState) VerifyConnectionState( + store sdk.KVStore, + cdc codec.BinaryCodec, + height exported.Height, + prefix exported.Prefix, + proof []byte, + connectionID string, + connectionEnd exported.ConnectionI, +) error { + publicKey, sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof) + if err != nil { + return err + } + + connectionPath := commitmenttypes.NewMerklePath(host.ConnectionPath(connectionID)) + path, err := commitmenttypes.ApplyPrefix(prefix, connectionPath) + if err != nil { + return err + } + + signBz, err := ConnectionStateSignBytes(cdc, sequence, timestamp, cs.ConsensusState.Diversifier, path, connectionEnd) + if err != nil { + return err + } + + if err := VerifySignature(publicKey, signBz, sigData); err != nil { + return err + } + + cs.Sequence++ + cs.ConsensusState.Timestamp = timestamp + setClientState(store, cdc, cs) + return nil +} + +// VerifyChannelState verifies a proof of the channel state of the specified +// channel end, under the specified port, stored on the target machine. +func (cs *ClientState) VerifyChannelState( + store sdk.KVStore, + cdc codec.BinaryCodec, + height exported.Height, + prefix exported.Prefix, + proof []byte, + portID, + channelID string, + channel exported.ChannelI, +) error { + publicKey, sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof) + if err != nil { + return err + } + + channelPath := commitmenttypes.NewMerklePath(host.ChannelPath(portID, channelID)) + path, err := commitmenttypes.ApplyPrefix(prefix, channelPath) + if err != nil { + return err + } + + signBz, err := ChannelStateSignBytes(cdc, sequence, timestamp, cs.ConsensusState.Diversifier, path, channel) + if err != nil { + return err + } + + if err := VerifySignature(publicKey, signBz, sigData); err != nil { + return err + } + + cs.Sequence++ + cs.ConsensusState.Timestamp = timestamp + setClientState(store, cdc, cs) + return nil +} + +// VerifyPacketCommitment verifies a proof of an outgoing packet commitment at +// the specified port, specified channel, and specified sequence. +func (cs *ClientState) VerifyPacketCommitment( + ctx sdk.Context, + store sdk.KVStore, + cdc codec.BinaryCodec, + height exported.Height, + _ uint64, + _ uint64, + prefix exported.Prefix, + proof []byte, + portID, + channelID string, + packetSequence uint64, + commitmentBytes []byte, +) error { + publicKey, sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof) + if err != nil { + return err + } + + commitmentPath := commitmenttypes.NewMerklePath(host.PacketCommitmentPath(portID, channelID, packetSequence)) + path, err := commitmenttypes.ApplyPrefix(prefix, commitmentPath) + if err != nil { + return err + } + + signBz, err := PacketCommitmentSignBytes(cdc, sequence, timestamp, cs.ConsensusState.Diversifier, path, commitmentBytes) + if err != nil { + return err + } + + if err := VerifySignature(publicKey, signBz, sigData); err != nil { + return err + } + + cs.Sequence++ + cs.ConsensusState.Timestamp = timestamp + setClientState(store, cdc, cs) + return nil +} + +// VerifyPacketAcknowledgement verifies a proof of an incoming packet +// acknowledgement at the specified port, specified channel, and specified sequence. +func (cs *ClientState) VerifyPacketAcknowledgement( + ctx sdk.Context, + store sdk.KVStore, + cdc codec.BinaryCodec, + height exported.Height, + _ uint64, + _ uint64, + prefix exported.Prefix, + proof []byte, + portID, + channelID string, + packetSequence uint64, + acknowledgement []byte, +) error { + publicKey, sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof) + if err != nil { + return err + } + + ackPath := commitmenttypes.NewMerklePath(host.PacketAcknowledgementPath(portID, channelID, packetSequence)) + path, err := commitmenttypes.ApplyPrefix(prefix, ackPath) + if err != nil { + return err + } + + signBz, err := PacketAcknowledgementSignBytes(cdc, sequence, timestamp, cs.ConsensusState.Diversifier, path, acknowledgement) + if err != nil { + return err + } + + if err := VerifySignature(publicKey, signBz, sigData); err != nil { + return err + } + + cs.Sequence++ + cs.ConsensusState.Timestamp = timestamp + setClientState(store, cdc, cs) + return nil +} + +// VerifyPacketReceiptAbsence verifies a proof of the absence of an +// incoming packet receipt at the specified port, specified channel, and +// specified sequence. +func (cs *ClientState) VerifyPacketReceiptAbsence( + ctx sdk.Context, + store sdk.KVStore, + cdc codec.BinaryCodec, + height exported.Height, + _ uint64, + _ uint64, + prefix exported.Prefix, + proof []byte, + portID, + channelID string, + packetSequence uint64, +) error { + publicKey, sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof) + if err != nil { + return err + } + + receiptPath := commitmenttypes.NewMerklePath(host.PacketReceiptPath(portID, channelID, packetSequence)) + path, err := commitmenttypes.ApplyPrefix(prefix, receiptPath) + if err != nil { + return err + } + + signBz, err := PacketReceiptAbsenceSignBytes(cdc, sequence, timestamp, cs.ConsensusState.Diversifier, path) + if err != nil { + return err + } + + if err := VerifySignature(publicKey, signBz, sigData); err != nil { + return err + } + + cs.Sequence++ + cs.ConsensusState.Timestamp = timestamp + setClientState(store, cdc, cs) + return nil +} + +// VerifyNextSequenceRecv verifies a proof of the next sequence number to be +// received of the specified channel at the specified port. +func (cs *ClientState) VerifyNextSequenceRecv( + ctx sdk.Context, + store sdk.KVStore, + cdc codec.BinaryCodec, + height exported.Height, + _ uint64, + _ uint64, + prefix exported.Prefix, + proof []byte, + portID, + channelID string, + nextSequenceRecv uint64, +) error { + publicKey, sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof) + if err != nil { + return err + } + + nextSequenceRecvPath := commitmenttypes.NewMerklePath(host.NextSequenceRecvPath(portID, channelID)) + path, err := commitmenttypes.ApplyPrefix(prefix, nextSequenceRecvPath) + if err != nil { + return err + } + + signBz, err := NextSequenceRecvSignBytes(cdc, sequence, timestamp, cs.ConsensusState.Diversifier, path, nextSequenceRecv) + if err != nil { + return err + } + + if err := VerifySignature(publicKey, signBz, sigData); err != nil { + return err + } + + cs.Sequence++ + cs.ConsensusState.Timestamp = timestamp + setClientState(store, cdc, cs) + return nil +} + +// produceVerificationArgs perfoms the basic checks on the arguments that are +// shared between the verification functions and returns the public key of the +// consensus state, the unmarshalled proof representing the signature and timestamp +// along with the solo-machine sequence encoded in the proofHeight. +func produceVerificationArgs( + cdc codec.BinaryCodec, + cs *ClientState, + height exported.Height, + prefix exported.Prefix, + proof []byte, +) (cryptotypes.PubKey, signing.SignatureData, uint64, uint64, error) { + if revision := height.GetRevisionNumber(); revision != 0 { + return nil, nil, 0, 0, sdkerrors.Wrapf(sdkerrors.ErrInvalidHeight, "revision must be 0 for solomachine, got revision-number: %d", revision) + } + // sequence is encoded in the revision height of height struct + sequence := height.GetRevisionHeight() + if prefix == nil { + return nil, nil, 0, 0, sdkerrors.Wrap(commitmenttypes.ErrInvalidPrefix, "prefix cannot be empty") + } + + _, ok := prefix.(*commitmenttypes.MerklePrefix) + if !ok { + return nil, nil, 0, 0, sdkerrors.Wrapf(commitmenttypes.ErrInvalidPrefix, "invalid prefix type %T, expected MerklePrefix", prefix) + } + + if proof == nil { + return nil, nil, 0, 0, sdkerrors.Wrap(ErrInvalidProof, "proof cannot be empty") + } + + timestampedSigData := &TimestampedSignatureData{} + if err := cdc.Unmarshal(proof, timestampedSigData); err != nil { + return nil, nil, 0, 0, sdkerrors.Wrapf(err, "failed to unmarshal proof into type %T", timestampedSigData) + } + + timestamp := timestampedSigData.Timestamp + + if len(timestampedSigData.SignatureData) == 0 { + return nil, nil, 0, 0, sdkerrors.Wrap(ErrInvalidProof, "signature data cannot be empty") + } + + sigData, err := UnmarshalSignatureData(cdc, timestampedSigData.SignatureData) + if err != nil { + return nil, nil, 0, 0, err + } + + if cs.ConsensusState == nil { + return nil, nil, 0, 0, sdkerrors.Wrap(clienttypes.ErrInvalidConsensus, "consensus state cannot be empty") + } + + latestSequence := cs.GetLatestHeight().GetRevisionHeight() + if latestSequence != sequence { + return nil, nil, 0, 0, sdkerrors.Wrapf( + sdkerrors.ErrInvalidHeight, + "client state sequence != proof sequence (%d != %d)", latestSequence, sequence, + ) + } + + if cs.ConsensusState.GetTimestamp() > timestamp { + return nil, nil, 0, 0, sdkerrors.Wrapf(ErrInvalidProof, "the consensus state timestamp is greater than the signature timestamp (%d >= %d)", cs.ConsensusState.GetTimestamp(), timestamp) + } + + publicKey, err := cs.ConsensusState.GetPubKey() + if err != nil { + return nil, nil, 0, 0, err + } + + return publicKey, sigData, timestamp, sequence, nil +} + +// sets the client state to the store +func setClientState(store sdk.KVStore, cdc codec.BinaryCodec, clientState exported.ClientState) { + bz := clienttypes.MustMarshalClientState(cdc, clientState) + store.Set([]byte(host.KeyClientState), bz) +} diff --git a/modules/light-clients/07-tendermint/store.go b/modules/light-clients/07-tendermint/store.go index e8824d06e1d..33c2386bb78 100644 --- a/modules/light-clients/07-tendermint/store.go +++ b/modules/light-clients/07-tendermint/store.go @@ -1,4 +1,4 @@ -package tendermint +package types import ( "bytes" @@ -8,10 +8,11 @@ import ( "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/store/prefix" sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - clienttypes "github.com/cosmos/ibc-go/v5/modules/core/02-client/types" - host "github.com/cosmos/ibc-go/v5/modules/core/24-host" - "github.com/cosmos/ibc-go/v5/modules/core/exported" + clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types" + host "github.com/cosmos/ibc-go/v3/modules/core/24-host" + "github.com/cosmos/ibc-go/v3/modules/core/exported" ) /* @@ -49,23 +50,38 @@ func setClientState(clientStore sdk.KVStore, cdc codec.BinaryCodec, clientState clientStore.Set(key, val) } -// setConsensusState stores the consensus state at the given height. -func setConsensusState(clientStore sdk.KVStore, cdc codec.BinaryCodec, consensusState *ConsensusState, height exported.Height) { +// SetConsensusState stores the consensus state at the given height. +func SetConsensusState(clientStore sdk.KVStore, cdc codec.BinaryCodec, consensusState *ConsensusState, height exported.Height) { key := host.ConsensusStateKey(height) val := clienttypes.MustMarshalConsensusState(cdc, consensusState) clientStore.Set(key, val) } -// GetConsensusState retrieves the consensus state from the client prefixed store. -// If the ConsensusState does not exist in state for the provided height a nil value and false boolean flag is returned -func GetConsensusState(store sdk.KVStore, cdc codec.BinaryCodec, height exported.Height) (*ConsensusState, bool) { +// GetConsensusState retrieves the consensus state from the client prefixed +// store. An error is returned if the consensus state does not exist. +func GetConsensusState(store sdk.KVStore, cdc codec.BinaryCodec, height exported.Height) (*ConsensusState, error) { bz := store.Get(host.ConsensusStateKey(height)) if bz == nil { - return nil, false + return nil, sdkerrors.Wrapf( + clienttypes.ErrConsensusStateNotFound, + "consensus state does not exist for height %s", height, + ) + } + + consensusStateI, err := clienttypes.UnmarshalConsensusState(cdc, bz) + if err != nil { + return nil, sdkerrors.Wrapf(clienttypes.ErrInvalidConsensus, "unmarshal error: %v", err) } - consensusStateI := clienttypes.MustUnmarshalConsensusState(cdc, bz) - return consensusStateI.(*ConsensusState), true + consensusState, ok := consensusStateI.(*ConsensusState) + if !ok { + return nil, sdkerrors.Wrapf( + clienttypes.ErrInvalidConsensus, + "invalid consensus type %T, expected %T", consensusState, &ConsensusState{}, + ) + } + + return consensusState, nil } // deleteConsensusState deletes the consensus state at the given height @@ -87,6 +103,7 @@ func IterateConsensusMetadata(store sdk.KVStore, cb func(key, val []byte) bool) if len(keySplit) != 3 { // ignore all consensus state keys continue + } if keySplit[2] != "processedTime" && keySplit[2] != "processedHeight" { @@ -215,7 +232,7 @@ func GetHeightFromIterationKey(iterKey []byte) exported.Height { // IterateConsensusStateAscending iterates through the consensus states in ascending order. It calls the provided // callback on each height, until stop=true is returned. -func IterateConsensusStateAscending(clientStore sdk.KVStore, cb func(height exported.Height) (stop bool)) { +func IterateConsensusStateAscending(clientStore sdk.KVStore, cb func(height exported.Height) (stop bool)) error { iterator := sdk.KVStorePrefixIterator(clientStore, []byte(KeyIterateConsensusStatePrefix)) defer iterator.Close() @@ -223,9 +240,10 @@ func IterateConsensusStateAscending(clientStore sdk.KVStore, cb func(height expo iterKey := iterator.Key() height := GetHeightFromIterationKey(iterKey) if cb(height) { - break + return nil } } + return nil } // GetNextConsensusState returns the lowest consensus state that is larger than the given height. @@ -277,12 +295,13 @@ func GetPreviousConsensusState(clientStore sdk.KVStore, cdc codec.BinaryCodec, h func PruneAllExpiredConsensusStates( ctx sdk.Context, clientStore sdk.KVStore, cdc codec.BinaryCodec, clientState *ClientState, -) { +) (err error) { var heights []exported.Height pruneCb := func(height exported.Height) bool { - consState, found := GetConsensusState(clientStore, cdc, height) - if !found { // consensus state should always be found + consState, err := GetConsensusState(clientStore, cdc, height) + // this error should never occur + if err != nil { return true } @@ -294,11 +313,16 @@ func PruneAllExpiredConsensusStates( } IterateConsensusStateAscending(clientStore, pruneCb) + if err != nil { + return err + } for _, height := range heights { deleteConsensusState(clientStore, height) deleteConsensusMetadata(clientStore, height) } + + return nil } // Helper function for GetNextConsensusState and GetPreviousConsensusState diff --git a/modules/light-clients/07-tendermint/upgrade.go b/modules/light-clients/07-tendermint/upgrade.go index 277ced61273..471769f0610 100644 --- a/modules/light-clients/07-tendermint/upgrade.go +++ b/modules/light-clients/07-tendermint/upgrade.go @@ -1,4 +1,4 @@ -package tendermint +package types import ( "fmt" @@ -8,22 +8,22 @@ import ( sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" - clienttypes "github.com/cosmos/ibc-go/v5/modules/core/02-client/types" - commitmenttypes "github.com/cosmos/ibc-go/v5/modules/core/23-commitment/types" - "github.com/cosmos/ibc-go/v5/modules/core/exported" + clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types" + commitmenttypes "github.com/cosmos/ibc-go/v3/modules/core/23-commitment/types" + "github.com/cosmos/ibc-go/v3/modules/core/exported" ) // VerifyUpgradeAndUpdateState checks if the upgraded client has been committed by the current client -// It will zero out all client-specific fields (e.g. TrustingPeriod) and verify all data +// It will zero out all client-specific fields (e.g. TrustingPeriod and verify all data // in client state that must be the same across all valid Tendermint clients for the new chain. // VerifyUpgrade will return an error if: // - the upgradedClient is not a Tendermint ClientState -// - the latest height of the client state does not have the same revision number or has a greater +// - the lastest height of the client state does not have the same revision number or has a greater // height than the committed client. -// - the height of upgraded client is not greater than that of current client -// - the latest height of the new client does not match or is greater than the height in committed client -// - any Tendermint chain specified parameter in upgraded client such as ChainID, UnbondingPeriod, -// and ProofSpecs do not match parameters set by committed client +// - the height of upgraded client is not greater than that of current client +// - the latest height of the new client does not match or is greater than the height in committed client +// - any Tendermint chain specified parameter in upgraded client such as ChainID, UnbondingPeriod, +// and ProofSpecs do not match parameters set by committed client func (cs ClientState) VerifyUpgradeAndUpdateState( ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, upgradedClient exported.ClientState, upgradedConsState exported.ConsensusState, @@ -67,13 +67,13 @@ func (cs ClientState) VerifyUpgradeAndUpdateState( // Must prove against latest consensus state to ensure we are verifying against latest upgrade plan // This verifies that upgrade is intended for the provided revision, since committed client must exist // at this consensus state - consState, found := GetConsensusState(clientStore, cdc, lastHeight) - if !found { - return sdkerrors.Wrap(clienttypes.ErrConsensusStateNotFound, "could not retrieve consensus state for lastHeight") + consState, err := GetConsensusState(clientStore, cdc, lastHeight) + if err != nil { + return sdkerrors.Wrap(err, "could not retrieve consensus state for lastHeight") } // Verify client proof - bz, err := cdc.MarshalInterface(upgradedClient.ZeroCustomFields()) + bz, err := cdc.MarshalInterface(upgradedClient) if err != nil { return sdkerrors.Wrapf(clienttypes.ErrInvalidClient, "could not marshal client state: %v", err) } @@ -120,7 +120,7 @@ func (cs ClientState) VerifyUpgradeAndUpdateState( ) setClientState(clientStore, cdc, newClientState) - setConsensusState(clientStore, cdc, newConsState, newClientState.LatestHeight) + SetConsensusState(clientStore, cdc, newConsState, newClientState.LatestHeight) setConsensusMetadata(ctx, clientStore, tmUpgradeClient.LatestHeight) return nil diff --git a/modules/light-clients/07-tendermint/upgrade_test.go b/modules/light-clients/07-tendermint/upgrade_test.go index 301b64288c9..112d3366cda 100644 --- a/modules/light-clients/07-tendermint/upgrade_test.go +++ b/modules/light-clients/07-tendermint/upgrade_test.go @@ -1,18 +1,21 @@ -package tendermint_test +package types_test import ( upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" - clienttypes "github.com/cosmos/ibc-go/v5/modules/core/02-client/types" - commitmenttypes "github.com/cosmos/ibc-go/v5/modules/core/23-commitment/types" - "github.com/cosmos/ibc-go/v5/modules/core/exported" - ibctm "github.com/cosmos/ibc-go/v5/modules/light-clients/07-tendermint" - ibctesting "github.com/cosmos/ibc-go/v5/testing" + clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types" + commitmenttypes "github.com/cosmos/ibc-go/v3/modules/core/23-commitment/types" + "github.com/cosmos/ibc-go/v3/modules/core/exported" + "github.com/cosmos/ibc-go/v3/modules/light-clients/07-tendermint/types" + ibctesting "github.com/cosmos/ibc-go/v3/testing" +) + +var ( + newChainId = "newChainId-1" ) func (suite *TendermintTestSuite) TestVerifyUpgrade() { var ( - newChainID string upgradedClient exported.ClientState upgradedConsState exported.ConsensusState lastHeight clienttypes.Height @@ -54,7 +57,9 @@ func (suite *TendermintTestSuite) TestVerifyUpgrade() { { name: "successful upgrade to same revision", setup: func() { - upgradedClient = ibctm.NewClientState(suite.chainB.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod+trustingPeriod, maxClockDrift, clienttypes.NewHeight(clienttypes.ParseChainID(suite.chainB.ChainID), upgradedClient.GetLatestHeight().GetRevisionHeight()+10), commitmenttypes.GetSDKSpecs(), upgradePath, false, false) + upgradedHeight := clienttypes.NewHeight(0, uint64(suite.chainB.GetContext().BlockHeight()+2)) + // don't use -1 suffix in chain id + upgradedClient = types.NewClientState("newChainId", types.DefaultTrustLevel, trustingPeriod, ubdPeriod+trustingPeriod, maxClockDrift, upgradedHeight, commitmenttypes.GetSDKSpecs(), upgradePath, false, false) upgradedClient = upgradedClient.ZeroCustomFields() upgradedClientBz, err = clienttypes.MarshalClientState(suite.chainA.App.AppCodec(), upgradedClient) suite.Require().NoError(err) @@ -109,7 +114,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgrade() { name: "unsuccessful upgrade: committed client does not have zeroed custom fields", setup: func() { // non-zeroed upgrade client - upgradedClient = ibctm.NewClientState(newChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod+trustingPeriod, maxClockDrift, newClientHeight, commitmenttypes.GetSDKSpecs(), upgradePath, false, false) + upgradedClient = types.NewClientState(newChainId, types.DefaultTrustLevel, trustingPeriod, ubdPeriod+trustingPeriod, maxClockDrift, newClientHeight, commitmenttypes.GetSDKSpecs(), upgradePath, false, false) upgradedClientBz, err = clienttypes.MarshalClientState(suite.chainA.App.AppCodec(), upgradedClient) suite.Require().NoError(err) @@ -145,7 +150,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgrade() { suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsStateBz) // change upgradedClient client-specified parameters - upgradedClient = ibctm.NewClientState("wrongchainID", ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, newClientHeight, commitmenttypes.GetSDKSpecs(), upgradePath, true, true) + upgradedClient = types.NewClientState("wrongchainID", types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, newClientHeight, commitmenttypes.GetSDKSpecs(), upgradePath, true, true) suite.coordinator.CommitBlock(suite.chainB) err := path.EndpointA.UpdateClient() @@ -167,7 +172,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgrade() { suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsStateBz) // change upgradedClient client-specified parameters - upgradedClient = ibctm.NewClientState(newChainID, ibctm.DefaultTrustLevel, ubdPeriod, ubdPeriod+trustingPeriod, maxClockDrift+5, lastHeight, commitmenttypes.GetSDKSpecs(), upgradePath, true, false) + upgradedClient = types.NewClientState(newChainId, types.DefaultTrustLevel, ubdPeriod, ubdPeriod+trustingPeriod, maxClockDrift+5, lastHeight, commitmenttypes.GetSDKSpecs(), upgradePath, true, false) suite.coordinator.CommitBlock(suite.chainB) err := path.EndpointA.UpdateClient() @@ -192,7 +197,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgrade() { suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsStateBz) // change submitted upgradedConsensusState - upgradedConsState = &ibctm.ConsensusState{ + upgradedConsState = &types.ConsensusState{ NextValidatorsHash: []byte("maliciousValidators"), } @@ -296,7 +301,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgrade() { proofUpgradedConsState, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedConsStateKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) // SetClientState with empty upgrade path - tmClient, _ := cs.(*ibctm.ClientState) + tmClient, _ := cs.(*types.ClientState) tmClient.UpgradePath = []string{""} suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID, tmClient) }, @@ -398,7 +403,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgrade() { name: "unsuccessful upgrade: final client is not valid", setup: func() { // new client has smaller unbonding period such that old trusting period is no longer valid - upgradedClient = ibctm.NewClientState(newChainID, ibctm.DefaultTrustLevel, trustingPeriod, trustingPeriod, maxClockDrift, newClientHeight, commitmenttypes.GetSDKSpecs(), upgradePath, false, false) + upgradedClient = types.NewClientState(newChainId, types.DefaultTrustLevel, trustingPeriod, trustingPeriod, maxClockDrift, newClientHeight, commitmenttypes.GetSDKSpecs(), upgradePath, false, false) upgradedClientBz, err = clienttypes.MarshalClientState(suite.chainA.App.AppCodec(), upgradedClient) suite.Require().NoError(err) @@ -433,20 +438,12 @@ func (suite *TendermintTestSuite) TestVerifyUpgrade() { path = ibctesting.NewPath(suite.chainA, suite.chainB) suite.coordinator.SetupClients(path) - - clientState := path.EndpointA.GetClientState().(*ibctm.ClientState) - revisionNumber := clienttypes.ParseChainID(clientState.ChainId) - - var err error - newChainID, err = clienttypes.SetRevisionNumber(clientState.ChainId, revisionNumber+1) - suite.Require().NoError(err) - - upgradedClient = ibctm.NewClientState(newChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod+trustingPeriod, maxClockDrift, clienttypes.NewHeight(revisionNumber+1, clientState.GetLatestHeight().GetRevisionHeight()+1), commitmenttypes.GetSDKSpecs(), upgradePath, false, false) + upgradedClient = types.NewClientState(newChainId, types.DefaultTrustLevel, trustingPeriod, ubdPeriod+trustingPeriod, maxClockDrift, newClientHeight, commitmenttypes.GetSDKSpecs(), upgradePath, false, false) upgradedClient = upgradedClient.ZeroCustomFields() upgradedClientBz, err = clienttypes.MarshalClientState(suite.chainA.App.AppCodec(), upgradedClient) suite.Require().NoError(err) - upgradedConsState = &ibctm.ConsensusState{ + upgradedConsState = &types.ConsensusState{ NextValidatorsHash: []byte("nextValsHash"), } upgradedConsStateBz, err = clienttypes.MarshalConsensusState(suite.chainA.App.AppCodec(), upgradedConsState) @@ -460,7 +457,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgrade() { // Call ZeroCustomFields on upgraded clients to clear any client-chosen parameters in test-case upgradedClient upgradedClient = upgradedClient.ZeroCustomFields() - err = cs.VerifyUpgradeAndUpdateState( + err := cs.VerifyUpgradeAndUpdateState( suite.chainA.GetContext(), suite.cdc, clientStore, diff --git a/modules/light-clients/09-localhost/types/client_state.go b/modules/light-clients/09-localhost/types/client_state.go index e818f9c8d7e..728e4ec5f12 100644 --- a/modules/light-clients/09-localhost/types/client_state.go +++ b/modules/light-clients/09-localhost/types/client_state.go @@ -138,8 +138,8 @@ func (cs ClientState) CheckSubstituteAndUpdateState( func (cs ClientState) VerifyUpgradeAndUpdateState( _ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore, _ exported.ClientState, _ exported.ConsensusState, _, _ []byte, -) (exported.ClientState, exported.ConsensusState, error) { - return nil, nil, sdkerrors.Wrap(clienttypes.ErrInvalidUpgradeClient, "cannot upgrade localhost client") +) error { + return sdkerrors.Wrap(clienttypes.ErrInvalidUpgradeClient, "cannot upgrade localhost client") } // VerifyClientState verifies that the localhost client state is stored locally