chore: modify connection keeper GetTimestampAtHeight to use the clien…

…t state interface function (#1666)

* updated connection keeper to use clientstate to get timestamp, updated tests

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,

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)

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")
 )

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

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 {

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

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())

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)