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