refactor(client): change flag for unordered tx timeouts (#24561)

Author: technicallyty

CHANGELOG.md

@@ -66,6 +66,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Improvements
 
+* (client) [#24561](https://github.com/cosmos/cosmos-sdk/pull/24561) TimeoutTimestamp flag has been changed to TimeoutDuration, which now sets the timeout timestamp of unordered transactions to the current time + duration passed.
 * (telemetry) [#24541](https://github.com/cosmos/cosmos-sdk/pull/24541) Telemetry now includes a pre_blocker metric key. x/upgrade should migrate to this key in v0.54.0.
 * (x/auth) [#24541](https://github.com/cosmos/cosmos-sdk/pull/24541) x/auth's PreBlocker now emits telemetry under the pre_blocker metric key.
 * (x/bank) [#24431](https://github.com/cosmos/cosmos-sdk/pull/24431) Reduce the number of `ValidateDenom` calls in `bank.SendCoins` and `Coin`.

client/flags/flags.go

@@ -74,7 +74,7 @@ const (
 	FlagOffset           = "offset"
 	FlagCountTotal       = "count-total"
 	FlagTimeoutHeight    = "timeout-height"
-	FlagTimeoutTimestamp = "timeout-timestamp"
+	TimeoutDuration      = "timeout-duration"
 	FlagUnordered        = "unordered"
 	FlagKeyAlgorithm     = "algo"
 	FlagKeyType          = "key-type"
@@ -137,9 +137,9 @@ func AddTxFlagsToCmd(cmd *cobra.Command) {
 	f.Bool(FlagOffline, false, "Offline mode (does not allow any online functionality)")
 	f.BoolP(FlagSkipConfirmation, "y", false, "Skip tx broadcasting prompt confirmation")
 	f.String(FlagSignMode, "", "Choose sign mode (direct|amino-json|direct-aux|textual), this is an advanced feature")
-	f.Uint64(FlagTimeoutHeight, 0, "DEPRECATED: Please use --timeout-timestamp instead. Set a block timeout height to prevent the tx from being committed past a certain height")
-	f.Int64(FlagTimeoutTimestamp, 0, "Set a block timeout timestamp to prevent the tx from being committed past a certain time")
-	f.Bool(FlagUnordered, false, "Enable unordered transaction delivery; must be used in conjunction with --timeout-timestamp")
+	f.Uint64(FlagTimeoutHeight, 0, "DEPRECATED: Please use --timeout-duration instead. Set a block timeout height to prevent the tx from being committed past a certain height")
+	f.Duration(TimeoutDuration, 0, "TimeoutDuration is the duration the transaction will be considered valid in the mempool. The transaction's unordered nonce will be set to the time of transaction creation + the duration value passed. If the transaction is still in the mempool, and the block time has passed the time of submission + TimeoutTimestamp, the transaction will be rejected.")
+	f.Bool(FlagUnordered, false, "Enable unordered transaction delivery; must be used in conjunction with --timeout-duration")
 	f.String(FlagFeePayer, "", "Fee payer pays fees for the transaction instead of deducting from the signer")
 	f.String(FlagFeeGranter, "", "Fee granter grants fees for the transaction")
 	f.String(FlagTip, "", "Tip is the amount that is going to be transferred to the fee payer on the target chain. This flag is only valid when used with --aux, and is ignored if the target chain didn't enable the TipDecorator")
@@ -149,8 +149,8 @@ func AddTxFlagsToCmd(cmd *cobra.Command) {
 	f.String(FlagGas, "", fmt.Sprintf("gas limit to set per-transaction; set to %q to calculate sufficient gas automatically. Note: %q option doesn't always report accurate results. Set a valid coin value to adjust the result. Can be used instead of %q. (default %d)",
 		GasFlagAuto, GasFlagAuto, FlagFees, DefaultGasLimit))
 
-	cmd.MarkFlagsMutuallyExclusive(FlagTimeoutHeight, FlagTimeoutTimestamp)
-	cmd.MarkFlagsRequiredTogether(FlagUnordered, FlagTimeoutTimestamp)
+	cmd.MarkFlagsMutuallyExclusive(FlagTimeoutHeight, TimeoutDuration)
+	cmd.MarkFlagsRequiredTogether(FlagUnordered, TimeoutDuration)
 
 	AddKeyringFlags(f)
 }

client/tx/factory.go

@@ -88,8 +88,11 @@ func NewFactoryCLI(clientCtx client.Context, flagSet *pflag.FlagSet) (Factory, e
 
 	gasAdj := clientCtx.Viper.GetFloat64(flags.FlagGasAdjustment)
 	memo := clientCtx.Viper.GetString(flags.FlagNote)
-	timestampUnix := clientCtx.Viper.GetInt64(flags.FlagTimeoutTimestamp)
-	timeoutTimestamp := time.Unix(timestampUnix, 0)
+	timeout := clientCtx.Viper.GetDuration(flags.TimeoutDuration)
+	var timeoutTimestamp time.Time
+	if timeout > 0 {
+		timeoutTimestamp = time.Now().Add(timeout)
+	}
 	timeoutHeight := clientCtx.Viper.GetUint64(flags.FlagTimeoutHeight)
 	unordered := clientCtx.Viper.GetBool(flags.FlagUnordered)
 

docs/docs/learn/beginner/01-tx-lifecycle.md

@@ -39,7 +39,15 @@ Additionally, there are several [flags](../advanced/07-cli.md) users can use to
 
 The ultimate value of the fees paid is equal to the gas multiplied by the gas prices. In other words, `fees = ceil(gas * gasPrices)`. Thus, since fees can be calculated using gas prices and vice versa, the users specify only one of the two.
 
-Later, validators decide whether or not to include the transaction in their block by comparing the given or calculated `gas-prices` to their local `min-gas-prices`. `Tx` is rejected if its `gas-prices` is not high enough, so users are incentivized to pay more.
+Later, validators decide whether to include the transaction in their block by comparing the given or calculated `gas-prices` to their local `min-gas-prices`. `Tx` is rejected if its `gas-prices` is not high enough, so users are incentivized to pay more.
+
+#### Unordered Transactions
+
+With Cosmos SDK v0.53.0, users may send unordered transactions to chains that have this feature enabled.
+The following flags allow a user to build an unordered transaction from the CLI.
+
+* `--unordered` specifies that this transaction should be unordered.
+* `--timeout-duration` specifies the amount of time the unordered transaction should be valid in the mempool. The transaction's unordered nonce will be set to the time of transaction creation + timeout duration.
 
 #### CLI Example
 

systemtests/cli.go

@@ -195,6 +195,7 @@ func (c CLIWrapper) RunAndWait(args ...string) string {
 // RunCommandWithArgs use for run cli command, not tx
 func (c CLIWrapper) RunCommandWithArgs(args ...string) string {
 	c.t.Helper()
+	args = c.WithKeyringFlags(args...)
 	execOutput, _ := c.run(args)
 	return execOutput
 }

tests/systemtests/unordered_tx_test.go

@@ -7,18 +7,19 @@ import (
 	"testing"
 	"time"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/tidwall/gjson"
 
 	systest "cosmossdk.io/systemtests"
+
+	"github.com/cosmos/cosmos-sdk/testutil"
 )
 
 func TestUnorderedTXDuplicate(t *testing.T) {
 	// scenario: test unordered tx duplicate
 	// given a running chain with a tx in the unordered tx pool
-	// when a new tx with the same hash is broadcasted
-	// then the new tx should be rejected
+	// when a new tx with the same unordered nonce is broadcasted,
+	// then the new tx should be rejected.
 
 	systest.Sut.ResetChain(t)
 	cli := systest.NewCLIWrapper(t, systest.Sut, systest.Verbose)
@@ -31,22 +32,29 @@ func TestUnorderedTXDuplicate(t *testing.T) {
 
 	systest.Sut.StartChain(t)
 
-	timeoutTimestamp := time.Now().Add(time.Minute)
 	// send tokens
-	cmd := []string{"tx", "bank", "send", account1Addr, account2Addr, "5000stake", "--from=" + account1Addr, "--fees=1stake", fmt.Sprintf("--timeout-timestamp=%v", timeoutTimestamp.Unix()), "--unordered", "--sequence=1", "--note=1"}
-	rsp1 := cli.Run(cmd...)
+	cmd := []string{"tx", "bank", "send", account1Addr, account2Addr, "5000stake", "--from=" + account1Addr, "--fees=1stake", "--timeout-duration=5m", "--unordered", "--note=1", "--chain-id=testing", "--generate-only"}
+	rsp1 := cli.RunCommandWithArgs(cmd...)
+	txFile := testutil.TempFile(t)
+	_, err := txFile.WriteString(rsp1)
+	require.NoError(t, err)
+
+	signCmd := []string{"tx", "sign", txFile.Name(), "--from=" + account1Addr, "--chain-id=testing"}
+	rsp1 = cli.RunCommandWithArgs(signCmd...)
+	signedFile := testutil.TempFile(t)
+	_, err = signedFile.WriteString(rsp1)
+	require.NoError(t, err)
+
+	cmd = []string{"tx", "broadcast", signedFile.Name(), "--chain-id=testing"}
+	rsp1 = cli.RunCommandWithArgs(cmd...)
 	systest.RequireTxSuccess(t, rsp1)
 
-	assertDuplicateErr := func(xt assert.TestingT, gotErr error, gotOutputs ...interface{}) bool {
-		require.Len(t, gotOutputs, 1)
-		output := gotOutputs[0].(string)
-		code := gjson.Get(output, "code")
-		require.True(t, code.Exists())
-		require.Equal(t, int64(19), code.Int()) // 19 == already in mempool.
-		return false                            // always abort
-	}
-	rsp2 := cli.WithRunErrorMatcher(assertDuplicateErr).Run(cmd...)
+	cmd = []string{"tx", "broadcast", signedFile.Name(), "--chain-id=testing"}
+	rsp2, _ := cli.RunOnly(cmd...)
 	systest.RequireTxFailure(t, rsp2)
+	code := gjson.Get(rsp2, "code")
+	require.True(t, code.Exists())
+	require.Equal(t, int64(19), code.Int())
 
 	require.Eventually(t, func() bool {
 		return cli.QueryBalance(account2Addr, "stake") == 5000
@@ -86,7 +94,7 @@ func TestTxBackwardsCompatability(t *testing.T) {
 	code := gjson.Get(response, "code").Int()
 	require.Equal(t, int64(0), code)
 
-	bankSendCmdArgs = []string{"tx", "bank", "send", senderAddr, valAddr, fmt.Sprintf("%d%s", transferAmount, denom), "--chain-id=" + v50CLI.ChainID(), "--fees=10stake", "--sign-mode=direct", "--unordered", "--timeout-timestamp=10000"}
+	bankSendCmdArgs = []string{"tx", "bank", "send", senderAddr, valAddr, fmt.Sprintf("%d%s", transferAmount, denom), "--chain-id=" + v50CLI.ChainID(), "--fees=10stake", "--sign-mode=direct", "--unordered", "--timeout-duration=8m"}
 	res, ok = v53CLI.RunOnly(bankSendCmdArgs...)
 	require.True(t, ok)