refactor: use local clock time only for transfer CLI relative timeouts

modules/apps/27-interchain-accounts/controller/client/cli/tx.go

@@ -1,9 +1,11 @@
 package cli
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"strings"
+	"time"
 
 	"github.com/spf13/cobra"
 
@@ -22,10 +24,17 @@ const (
 	// The controller chain channel version
 	flagVersion = "version"
 	// The channel ordering
-	flagOrdering              = "ordering"
-	flagRelativePacketTimeout = "relative-packet-timeout"
+	flagOrdering               = "ordering"
+	flagPacketTimeoutTimestamp = "packet-timeout-timestamp"
+	flagAbsoluteTimeouts       = "absolute-timeouts"
 )
 
+// defaultRelativePacketTimeoutTimestamp is the default packet timeout timestamp (in nanoseconds)
+// relative to the current block timestamp of the counterparty chain provided by the client
+// state. The timeout is disabled when set to 0. The default is currently set to a 10 minute
+// timeout.
+var defaultRelativePacketTimeoutTimestamp = uint64((time.Duration(10) * time.Minute).Nanoseconds())
+
 func newRegisterInterchainAccountCmd() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "register [connection-id]",
@@ -73,8 +82,8 @@ func newSendTxCmd() *cobra.Command {
 		Use:   "send-tx [connection-id] [path/to/packet_msg.json]",
 		Short: "Send an interchain account tx on the provided connection.",
 		Long: strings.TrimSpace(`Submits pre-built packet data containing messages to be executed on the host chain 
-and attempts to send the packet. Packet data is provided as json, file or string. An 
-appropriate relative timeoutTimestamp must be provided with flag {relative-packet-timeout}`),
+and attempts to send the packet. Packet data is provided as json, file or string. A relative timeout timestamp can be provided with flag {packet-timeout-timestamp}.
+If no timeout value is set then a default relative timeout value of 10 minutes is used.`),
 		Args: cobra.ExactArgs(2),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			clientCtx, err := client.GetClientTxContext(cmd)
@@ -101,18 +110,37 @@ appropriate relative timeoutTimestamp must be provided with flag {relative-packe
 				}
 			}
 
-			relativeTimeoutTimestamp, err := cmd.Flags().GetUint64(flagRelativePacketTimeout)
+			timeoutTimestamp, err := cmd.Flags().GetUint64(flagPacketTimeoutTimestamp)
+			if err != nil {
+				return err
+			}
+
+			absoluteTimeouts, err := cmd.Flags().GetBool(flagAbsoluteTimeouts)
 			if err != nil {
 				return err
 			}
 
-			msg := types.NewMsgSendTx(owner, connectionID, relativeTimeoutTimestamp, icaMsgData)
+			// NOTE: relative timeouts using block height are not supported.
+			if !absoluteTimeouts && timeoutTimestamp == 0 {
+				return errors.New("relative timeouts must provide a non zero value timestamp")
+			}
+
+			// use local clock time as reference time for calculating timeout timestamp.
+			now := time.Now().UnixNano()
+			if now <= 0 {
+				return errors.New("local clock time is not greater than Jan 1st, 1970 12:00 AM")
+			}
+
+			timeoutTimestamp = uint64(now) + timeoutTimestamp
+
+			msg := types.NewMsgSendTx(owner, connectionID, timeoutTimestamp, icaMsgData)
 
 			return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg)
 		},
 	}
 
-	cmd.Flags().Uint64(flagRelativePacketTimeout, icatypes.DefaultRelativePacketTimeoutTimestamp, "Relative packet timeout in nanoseconds from now. Default is 10 minutes.")
+	cmd.Flags().Uint64(flagPacketTimeoutTimestamp, defaultRelativePacketTimeoutTimestamp, "Packet timeout timestamp in nanoseconds from now. Default is 10 minutes.")
+	cmd.Flags().Bool(flagAbsoluteTimeouts, false, "Timeout flags are used as absolute timeouts.")
 	flags.AddTxFlagsToCmd(cmd)
 
 	return cmd

modules/apps/27-interchain-accounts/types/packet.go

@@ -3,7 +3,6 @@ package types
 import (
 	"encoding/json"
 	"strings"
-	"time"
 
 	errorsmod "cosmossdk.io/errors"
 
@@ -21,19 +20,6 @@ var (
 // MaxMemoCharLength defines the maximum length for the InterchainAccountPacketData memo field
 const MaxMemoCharLength = 32768
 
-var (
-	// DefaultRelativePacketTimeoutHeight is the default packet timeout height (in blocks) relative
-	// to the current block height of the counterparty chain provided by the client state. The
-	// timeout is disabled when set to 0.
-	DefaultRelativePacketTimeoutHeight = "0-1000"
-
-	// DefaultRelativePacketTimeoutTimestamp is the default packet timeout timestamp (in nanoseconds)
-	// relative to the current block timestamp of the counterparty chain provided by the client
-	// state. The timeout is disabled when set to 0. The default is currently set to a 10 minute
-	// timeout.
-	DefaultRelativePacketTimeoutTimestamp = uint64((time.Duration(10) * time.Minute).Nanoseconds())
-)
-
 // ValidateBasic performs basic validation of the interchain account packet data.
 // The memo may be empty.
 func (iapd InterchainAccountPacketData) ValidateBasic() error {

modules/apps/transfer/client/cli/tx.go

@@ -15,10 +15,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/version"
 
 	"github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
-	clientutils "github.com/cosmos/ibc-go/v8/modules/core/02-client/client/utils"
 	clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
-	channelutils "github.com/cosmos/ibc-go/v8/modules/core/04-channel/client/utils"
-	"github.com/cosmos/ibc-go/v8/modules/core/exported"
 )
 
 const (
@@ -28,17 +25,20 @@ const (
 	flagMemo                   = "memo"
 )
 
+// defaultRelativePacketTimeoutTimestamp is the default packet timeout timestamp (in nanoseconds)
+// relative to the current block timestamp of the counterparty chain provided by the client
+// state. The timeout is disabled when set to 0. The default is currently set to a 10 minute
+// timeout.
+var defaultRelativePacketTimeoutTimestamp = uint64((time.Duration(10) * time.Minute).Nanoseconds())
+
 // NewTransferTxCmd returns the command to create a NewMsgTransfer transaction
 func NewTransferTxCmd() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "transfer [src-port] [src-channel] [receiver] [amount]",
 		Short: "Transfer a fungible token through IBC",
-		Long: strings.TrimSpace(`Transfer a fungible token through IBC. Timeouts can be specified
-as absolute or relative using the "absolute-timeouts" flag. Timeout height can be set by passing in the height string
-in the form {revision}-{height} using the "packet-timeout-height" flag. Relative timeout height is added to the block
-height queried from the latest consensus state corresponding to the counterparty channel. Relative timeout timestamp 
-is added to the greater value of the local clock time and the block timestamp queried from the latest consensus state 
-corresponding to the counterparty channel. Any timeout set to 0 is disabled.`),
+		Long: strings.TrimSpace(`Transfer a fungible token through IBC. Timeouts can be specified as absolute using the {absolute-timeouts} flag. 
+Timeout height can be set by passing in the height string in the form {revision}-{height} using the {packet-timeout-height} flag. Note, relative timeout height is not supported. 
+Relative timeout timestamp is added to the value of the user's local system clock time using the {packet-timeout-timestamp} flag. If no timeout value is set then a default relative timeout value of 10 minutes is used.`),
 		Example: fmt.Sprintf("%s tx ibc-transfer transfer [src-port] [src-channel] [receiver] [amount]", version.AppName),
 		Args:    cobra.ExactArgs(4),
 		RunE: func(cmd *cobra.Command, args []string) error {
@@ -65,6 +65,7 @@ corresponding to the counterparty channel. Any timeout set to 0 is disabled.`),
 			if err != nil {
 				return err
 			}
+
 			timeoutHeight, err := clienttypes.ParseHeight(timeoutHeightStr)
 			if err != nil {
 				return err
@@ -85,66 +86,24 @@ corresponding to the counterparty channel. Any timeout set to 0 is disabled.`),
 				return err
 			}
 
-			// if the timeouts are not absolute, retrieve latest block height and block timestamp
-			// for the consensus state connected to the destination port/channel.
-			// localhost clients must rely solely on local clock time in order to use relative timestamps.
+			// NOTE: relative timeouts using block height are not supported.
+			// if the timeouts are not absolute, CLI users rely solely on local clock time in order to calculate relative timestamps.
 			if !absoluteTimeouts {
-				clientRes, err := channelutils.QueryChannelClientState(clientCtx, srcPort, srcChannel, false)
-				if err != nil {
-					return err
-				}
-
-				var clientState exported.ClientState
-				if err := clientCtx.InterfaceRegistry.UnpackAny(clientRes.IdentifiedClientState.ClientState, &clientState); err != nil {
-					return err
-				}
-
-				clientHeight, ok := clientState.GetLatestHeight().(clienttypes.Height)
-				if !ok {
-					return fmt.Errorf("invalid height type. expected type: %T, got: %T", clienttypes.Height{}, clientState.GetLatestHeight())
+				if !timeoutHeight.IsZero() {
+					return errors.New("relative timeouts using block height is not supported")
 				}
 
-				var consensusState exported.ConsensusState
-				if clientState.ClientType() != exported.Localhost {
-					consensusStateRes, err := clientutils.QueryConsensusState(clientCtx, clientRes.IdentifiedClientState.ClientId, clientHeight, false, true)
-					if err != nil {
-						return err
-					}
-
-					if err := clientCtx.InterfaceRegistry.UnpackAny(consensusStateRes.ConsensusState, &consensusState); err != nil {
-						return err
-					}
+				if timeoutTimestamp == 0 {
+					return errors.New("relative timeouts must provide a non zero value timestamp")
 				}
 
-				if !timeoutHeight.IsZero() {
-					absoluteHeight := clientHeight
-					absoluteHeight.RevisionNumber += timeoutHeight.RevisionNumber
-					absoluteHeight.RevisionHeight += timeoutHeight.RevisionHeight
-					timeoutHeight = absoluteHeight
+				// use local clock time as reference time for calculating timeout timestamp.
+				now := time.Now().UnixNano()
+				if now <= 0 {
+					return errors.New("local clock time is not greater than Jan 1st, 1970 12:00 AM")
 				}
 
-				// use local clock time as reference time if it is later than the
-				// consensus state timestamp of the counterparty chain, otherwise
-				// still use consensus state timestamp as reference.
-				// for localhost clients local clock time is always used.
-				if timeoutTimestamp != 0 {
-					var consensusStateTimestamp uint64
-					if consensusState != nil {
-						consensusStateTimestamp = consensusState.GetTimestamp()
-					}
-
-					now := time.Now().UnixNano()
-					if now > 0 {
-						now := uint64(now)
-						if now > consensusStateTimestamp {
-							timeoutTimestamp = now + timeoutTimestamp
-						} else {
-							timeoutTimestamp = consensusStateTimestamp + timeoutTimestamp
-						}
-					} else {
-						return errors.New("local clock time is not greater than Jan 1st, 1970 12:00 AM")
-					}
-				}
+				timeoutTimestamp = uint64(now) + timeoutTimestamp
 			}
 
 			msg := types.NewMsgTransfer(
@@ -154,8 +113,8 @@ corresponding to the counterparty channel. Any timeout set to 0 is disabled.`),
 		},
 	}
 
-	cmd.Flags().String(flagPacketTimeoutHeight, types.DefaultRelativePacketTimeoutHeight, "Packet timeout block height. The timeout is disabled when set to 0-0.")
-	cmd.Flags().Uint64(flagPacketTimeoutTimestamp, types.DefaultRelativePacketTimeoutTimestamp, "Packet timeout timestamp in nanoseconds from now. Default is 10 minutes. The timeout is disabled when set to 0.")
+	cmd.Flags().String(flagPacketTimeoutHeight, "0-0", "Packet timeout block height in the format {revision}-{height}. The timeout is disabled when set to 0-0.")
+	cmd.Flags().Uint64(flagPacketTimeoutTimestamp, defaultRelativePacketTimeoutTimestamp, "Packet timeout timestamp in nanoseconds from now. Default is 10 minutes. The timeout is disabled when set to 0.")
 	cmd.Flags().Bool(flagAbsoluteTimeouts, false, "Timeout flags are used as absolute timeouts.")
 	cmd.Flags().String(flagMemo, "", "Memo to be sent along with the packet.")
 	flags.AddTxFlagsToCmd(cmd)

modules/apps/transfer/types/packet.go

@@ -4,7 +4,6 @@ import (
 	"encoding/json"
 	"errors"
 	"strings"
-	"time"
 
 	errorsmod "cosmossdk.io/errors"
 	sdkmath "cosmossdk.io/math"
@@ -18,19 +17,6 @@ var (
 	_ ibcexported.PacketDataProvider = (*FungibleTokenPacketData)(nil)
 )
 
-var (
-	// DefaultRelativePacketTimeoutHeight is the default packet timeout height (in blocks) relative
-	// to the current block height of the counterparty chain provided by the client state. The
-	// timeout is disabled when set to 0.
-	DefaultRelativePacketTimeoutHeight = "0-1000"
-
-	// DefaultRelativePacketTimeoutTimestamp is the default packet timeout timestamp (in nanoseconds)
-	// relative to the current block timestamp of the counterparty chain provided by the client
-	// state. The timeout is disabled when set to 0. The default is currently set to a 10 minute
-	// timeout.
-	DefaultRelativePacketTimeoutTimestamp = uint64((time.Duration(10) * time.Minute).Nanoseconds())
-)
-
 // NewFungibleTokenPacketData constructs a new FungibleTokenPacketData instance
 func NewFungibleTokenPacketData(
 	denom string, amount string,