diff --git a/modules/core/02-client/keeper/client.go b/modules/core/02-client/keeper/client.go index a1646da9c6b..9fc10fe3d1b 100644 --- a/modules/core/02-client/keeper/client.go +++ b/modules/core/02-client/keeper/client.go @@ -59,7 +59,7 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, clientMsg exporte clientStore := k.ClientStore(ctx, clientID) if status := clientState.Status(ctx, clientStore, k.cdc); status != exported.Active { - return sdkerrors.Wrapf(types.ErrClientNotActive, "cannot update client (%s) with status %s", clientID, status) + return sdkerrors.Wrapf(types.ErrClientStateNotFound, "cannot update client (%s) with status %s", clientID, status) } if err := clientState.VerifyClientMessage(ctx, k.cdc, clientStore, clientMsg); err != nil { @@ -119,7 +119,7 @@ func (k Keeper) UpgradeClient(ctx sdk.Context, clientID string, upgradedClient e clientStore := k.ClientStore(ctx, clientID) if status := clientState.Status(ctx, clientStore, k.cdc); status != exported.Active { - return sdkerrors.Wrapf(types.ErrClientNotActive, "cannot upgrade client (%s) with status %s", clientID, status) + return sdkerrors.Wrapf(types.ErrClientStateNotFound, "cannot upgrade client (%s) with status %s", clientID, status) } if err := clientState.VerifyUpgradeAndUpdateState(ctx, k.cdc, clientStore, diff --git a/modules/core/02-client/keeper/proposal.go b/modules/core/02-client/keeper/proposal.go index ffe29f688cb..115707777e5 100644 --- a/modules/core/02-client/keeper/proposal.go +++ b/modules/core/02-client/keeper/proposal.go @@ -41,7 +41,7 @@ func (k Keeper) ClientUpdateProposal(ctx sdk.Context, p *types.ClientUpdatePropo substituteClientStore := k.ClientStore(ctx, p.SubstituteClientId) if status := substituteClientState.Status(ctx, substituteClientStore, k.cdc); status != exported.Active { - return sdkerrors.Wrapf(types.ErrClientNotActive, "substitute client is not Active, status is %s", status) + return sdkerrors.Wrapf(types.ErrClientStateNotFound, "substitute client is not Active, status is %s", status) } clientState, err := subjectClientState.CheckSubstituteAndUpdateState(ctx, k.cdc, subjectClientStore, substituteClientStore, substituteClientState) diff --git a/modules/core/02-client/types/errors.go b/modules/core/02-client/types/errors.go index c40bae878cc..f97ab0014cb 100644 --- a/modules/core/02-client/types/errors.go +++ b/modules/core/02-client/types/errors.go @@ -33,5 +33,5 @@ var ( ErrInvalidHeight = sdkerrors.Register(SubModuleName, 26, "invalid height") ErrInvalidSubstitute = sdkerrors.Register(SubModuleName, 27, "invalid client state substitute") ErrInvalidUpgradeProposal = sdkerrors.Register(SubModuleName, 28, "invalid upgrade proposal") - ErrClientNotActive = sdkerrors.Register(SubModuleName, 29, "client is not active") + ErrClientStateNotFound = sdkerrors.Register(SubModuleName, 29, "client state not found") ) diff --git a/modules/core/03-connection/keeper/keeper.go b/modules/core/03-connection/keeper/keeper.go index 22954ca7943..8221dd597bc 100644 --- a/modules/core/03-connection/keeper/keeper.go +++ b/modules/core/03-connection/keeper/keeper.go @@ -86,18 +86,19 @@ func (k Keeper) SetConnection(ctx sdk.Context, connectionID string, connection t // GetTimestampAtHeight returns the timestamp in nanoseconds of the consensus state at the // given height. func (k Keeper) GetTimestampAtHeight(ctx sdk.Context, connection types.ConnectionEnd, height exported.Height) (uint64, error) { - consensusState, found := k.clientKeeper.GetClientConsensusState( - ctx, connection.GetClientID(), height, - ) - + clientState, found := k.clientKeeper.GetClientState(ctx, connection.GetClientID()) if !found { return 0, sdkerrors.Wrapf( - clienttypes.ErrConsensusStateNotFound, - "clientID (%s), height (%s)", connection.GetClientID(), height, + clienttypes.ErrClientStateNotFound, "clientID (%s)", connection.GetClientID(), ) } - return consensusState.GetTimestamp(), nil + timestamp, err := clientState.GetTimestampAtHeight(ctx, k.clientKeeper.ClientStore(ctx, connection.GetClientID()), k.cdc, height) + if err != nil { + return 0, err + } + + return timestamp, nil } // GetClientConnectionPaths returns all the connection paths stored under a diff --git a/modules/core/03-connection/keeper/keeper_test.go b/modules/core/03-connection/keeper/keeper_test.go index b4dfa839028..0c25d51c452 100644 --- a/modules/core/03-connection/keeper/keeper_test.go +++ b/modules/core/03-connection/keeper/keeper_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/suite" "github.com/cosmos/ibc-go/v3/modules/core/03-connection/types" + "github.com/cosmos/ibc-go/v3/modules/core/exported" ibctesting "github.com/cosmos/ibc-go/v3/testing" ) @@ -112,7 +113,10 @@ func (suite KeeperTestSuite) TestGetAllClientConnectionPaths() { // TestGetTimestampAtHeight verifies if the clients on each chain return the // correct timestamp for the other chain. func (suite *KeeperTestSuite) TestGetTimestampAtHeight() { - var connection types.ConnectionEnd + var ( + connection types.ConnectionEnd + height exported.Height + ) cases := []struct { msg string @@ -123,10 +127,14 @@ func (suite *KeeperTestSuite) TestGetTimestampAtHeight() { path := ibctesting.NewPath(suite.chainA, suite.chainB) suite.coordinator.SetupConnections(path) connection = path.EndpointA.GetConnection() + height = suite.chainB.LastHeader.GetHeight() }, true}, + {"client state not found", func() {}, false}, {"consensus state not found", func() { - // any non-nil value of connection is valid - suite.Require().NotNil(connection) + path := ibctesting.NewPath(suite.chainA, suite.chainB) + suite.coordinator.SetupConnections(path) + connection = path.EndpointA.GetConnection() + height = suite.chainB.LastHeader.GetHeight().Increment() }, false}, } @@ -137,7 +145,7 @@ func (suite *KeeperTestSuite) TestGetTimestampAtHeight() { tc.malleate() actualTimestamp, err := suite.chainA.App.GetIBCKeeper().ConnectionKeeper.GetTimestampAtHeight( - suite.chainA.GetContext(), connection, suite.chainB.LastHeader.GetHeight(), + suite.chainA.GetContext(), connection, height, ) if tc.expPass { diff --git a/modules/core/03-connection/keeper/verify.go b/modules/core/03-connection/keeper/verify.go index 1059cf3444e..abdad3fd002 100644 --- a/modules/core/03-connection/keeper/verify.go +++ b/modules/core/03-connection/keeper/verify.go @@ -28,7 +28,7 @@ func (k Keeper) VerifyClientState( } if status := targetClient.Status(ctx, clientStore, k.cdc); status != exported.Active { - return sdkerrors.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status) + return sdkerrors.Wrapf(clienttypes.ErrClientStateNotFound, "client (%s) status is %s", clientID, status) } if err := targetClient.VerifyClientState( @@ -59,7 +59,7 @@ func (k Keeper) VerifyClientConsensusState( } if status := clientState.Status(ctx, clientStore, k.cdc); status != exported.Active { - return sdkerrors.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status) + return sdkerrors.Wrapf(clienttypes.ErrClientStateNotFound, "client (%s) status is %s", clientID, status) } if err := clientState.VerifyClientConsensusState( @@ -91,7 +91,7 @@ func (k Keeper) VerifyConnectionState( } if status := clientState.Status(ctx, clientStore, k.cdc); status != exported.Active { - return sdkerrors.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status) + return sdkerrors.Wrapf(clienttypes.ErrClientStateNotFound, "client (%s) status is %s", clientID, status) } if err := clientState.VerifyConnectionState( @@ -124,7 +124,7 @@ func (k Keeper) VerifyChannelState( } if status := clientState.Status(ctx, clientStore, k.cdc); status != exported.Active { - return sdkerrors.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status) + return sdkerrors.Wrapf(clienttypes.ErrClientStateNotFound, "client (%s) status is %s", clientID, status) } if err := clientState.VerifyChannelState( @@ -159,7 +159,7 @@ func (k Keeper) VerifyPacketCommitment( } if status := clientState.Status(ctx, clientStore, k.cdc); status != exported.Active { - return sdkerrors.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status) + return sdkerrors.Wrapf(clienttypes.ErrClientStateNotFound, "client (%s) status is %s", clientID, status) } // get time and block delays @@ -199,7 +199,7 @@ func (k Keeper) VerifyPacketAcknowledgement( } if status := clientState.Status(ctx, clientStore, k.cdc); status != exported.Active { - return sdkerrors.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status) + return sdkerrors.Wrapf(clienttypes.ErrClientStateNotFound, "client (%s) status is %s", clientID, status) } // get time and block delays @@ -239,7 +239,7 @@ func (k Keeper) VerifyPacketReceiptAbsence( } if status := clientState.Status(ctx, clientStore, k.cdc); status != exported.Active { - return sdkerrors.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status) + return sdkerrors.Wrapf(clienttypes.ErrClientStateNotFound, "client (%s) status is %s", clientID, status) } // get time and block delays @@ -278,7 +278,7 @@ func (k Keeper) VerifyNextSequenceRecv( } if status := clientState.Status(ctx, clientStore, k.cdc); status != exported.Active { - return sdkerrors.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status) + return sdkerrors.Wrapf(clienttypes.ErrClientStateNotFound, "client (%s) status is %s", clientID, status) } // get time and block delays diff --git a/modules/core/04-channel/keeper/packet.go b/modules/core/04-channel/keeper/packet.go index c3e8e45b89c..05be506077a 100644 --- a/modules/core/04-channel/keeper/packet.go +++ b/modules/core/04-channel/keeper/packet.go @@ -71,7 +71,7 @@ func (k Keeper) SendPacket( // prevent accidental sends with clients that cannot be updated clientStore := k.clientKeeper.ClientStore(ctx, connectionEnd.GetClientID()) if status := clientState.Status(ctx, clientStore, k.cdc); status != exported.Active { - return sdkerrors.Wrapf(clienttypes.ErrClientNotActive, "cannot send packet using client (%s) with status %s", connectionEnd.GetClientID(), status) + return sdkerrors.Wrapf(clienttypes.ErrClientStateNotFound, "cannot send packet using client (%s) with status %s", connectionEnd.GetClientID(), status) } // check if packet is timed out on the receiving chain @@ -84,25 +84,16 @@ func (k Keeper) SendPacket( ) } - clientType, _, err := clienttypes.ParseClientIdentifier(connectionEnd.GetClientID()) + latestTimestamp, err := k.connectionKeeper.GetTimestampAtHeight(ctx, connectionEnd, latestHeight) if err != nil { return err } - // NOTE: this is a temporary fix. Solo machine does not support usage of 'GetTimestampAtHeight' - // A future change should move this function to be a ClientState callback. - if clientType != exported.Solomachine { - latestTimestamp, err := k.connectionKeeper.GetTimestampAtHeight(ctx, connectionEnd, latestHeight) - if err != nil { - return err - } - - if packet.GetTimeoutTimestamp() != 0 && latestTimestamp >= packet.GetTimeoutTimestamp() { - return sdkerrors.Wrapf( - types.ErrPacketTimeout, - "receiving chain block timestamp >= packet timeout timestamp (%s >= %s)", time.Unix(0, int64(latestTimestamp)), time.Unix(0, int64(packet.GetTimeoutTimestamp())), - ) - } + if packet.GetTimeoutTimestamp() != 0 && latestTimestamp >= packet.GetTimeoutTimestamp() { + return sdkerrors.Wrapf( + types.ErrPacketTimeout, + "receiving chain block timestamp >= packet timeout timestamp (%s >= %s)", time.Unix(0, int64(latestTimestamp)), time.Unix(0, int64(packet.GetTimeoutTimestamp())), + ) } nextSequenceSend, found := k.GetNextSequenceSend(ctx, packet.GetSourcePort(), packet.GetSourceChannel()) diff --git a/modules/core/04-channel/keeper/packet_test.go b/modules/core/04-channel/keeper/packet_test.go index 1f7e5546c70..66bc6846450 100644 --- a/modules/core/04-channel/keeper/packet_test.go +++ b/modules/core/04-channel/keeper/packet_test.go @@ -177,6 +177,23 @@ func (suite *KeeperTestSuite) TestSendPacket() { packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, disabledTimeoutHeight, timestamp) channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) }, false}, + {"timeout timestamp passed with solomachine", func() { + suite.coordinator.Setup(path) + // swap client with solomachine + solomachine := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "solomachinesingle", "testing", 1) + path.EndpointA.ClientID = clienttypes.FormatClientIdentifier(exported.Solomachine, 10) + path.EndpointA.SetClientState(solomachine.ClientState()) + connection := path.EndpointA.GetConnection() + connection.ClientId = path.EndpointA.ClientID + path.EndpointA.SetConnection(connection) + + clientState := path.EndpointA.GetClientState() + timestamp, err := suite.chainA.App.GetIBCKeeper().ConnectionKeeper.GetTimestampAtHeight(suite.chainA.GetContext(), connection, clientState.GetLatestHeight()) + suite.Require().NoError(err) + + packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, disabledTimeoutHeight, timestamp) + channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + }, false}, {"next sequence send not found", func() { path := ibctesting.NewPath(suite.chainA, suite.chainB) suite.coordinator.SetupConnections(path)