[Bug]: Unordered transaction fails because of sequence number mismatch.

[pakuula]

Is there an existing issue for this?

• I have searched the existing issues

What happened?

Summary:

Unordered transactions fail due to a sequence number mismatch. This behavior contradicts the intended functionality of unordered transactions as outlined in ADR 070.

Details:

ADR 070 "Unordered Transactions" introduces a mechanism to enable replay-attack protection without enforcing strict transaction order using sequence number as nonce. This is achieved by using a short-lived timeout stamp and a transaction cache, effectively bypassing the standard nonce rules.

However, the current implementation still checks the sequence number of unordered transactions. This results in transaction failure if the submitted sequence number doesn't align with the expected value.

Expected Behavior:

Unordered transactions should be processed successfully, irrespective of their sequence number, as long as they meet the specified timeout and the transaction number is unique in the cache of unordered transactions.

Observed Behavior:

An unordered transaction with a sequence number that doesn't match the expected sequence number results in an "account sequence mismatch" error, causing the transaction to fail.

Reproduce:

The following test demonstrates the issue:

set -e

which simd > /dev/null || { echo "No simd in the PATH"; exit 1; }

# Use ./zzz as the home of this test case.
dir=./zzz
rm -rf $dir > /dev/null || true
mkdir $dir

SIMD="simd --home $dir"

# Create a new testnet with chain-id `testnet`
$SIMD init --chain-id testnet node0 > /dev/null 2>&1

$SIMD config set client chain-id testnet
$SIMD config set client keyring-backend test
$SIMD config set client output json

# Create the test account
$SIMD keys add test --no-backup > /dev/null
address=$($SIMD keys show test -a)

# Finalize the genesis
$SIMD genesis add-genesis-account $address 1000000000stake > /dev/null 2>&1
$SIMD genesis gentx test 1000000stake > /dev/null 2>&1
$SIMD genesis collect-gentxs > /dev/null 2>&1

# Start the node
$SIMD start --log_level debug > $dir/log.log 2>&1 &
pid=$!
# Kill the node when the script exits
trap "kill -9 $pid" EXIT

sleep 2
# Send an ordered transactions: update the sequence number 
$SIMD tx bank send $address $address 10stake --yes --from $address --broadcast-mode sync 
# Unordered tx with timeout 100 seconds and sequence number 0
$SIMD tx bank send $address $address 10stake --fees 3stake --from $address --unordered --timeout-timestamp $(( $(date +%s) + 100)) --offline -a 0 -s 0 --yes 

Output:

error in json rpc client, with http response metadata: (Status: 200 OK, Protocol HTTP/1.1). RPC error -32603 - Internal error: broadcast error on transaction validation: tx 920E7FA01BE1C711691F21E5FB9ECCC85F893146284D2788CF85FEEA44E9C54C is invalid: code=32, data=, log='account sequence mismatch, expected higher than or equal to 2, got 0: incorrect account sequence', codespace='sdk'

Expected: The unordered transaction should be included in the block.
The error is: account sequence mismatch, expected higher than or equal to 2, got 0: incorrect account sequence.

Proposed Solution:

Modify the VerificationDecorator.verifySig method to check for unordered transactions. If a transaction is identified as unordered, the sequence number check should be bypassed.

Current version:

func (svd SigVerificationDecorator) verifySig(ctx context.Context, tx sdk.Tx, acc sdk.AccountI, sig signing.SignatureV2, newlyCreated bool) error {
	execMode := svd.ak.GetEnvironment().TransactionService.ExecMode(ctx)
	if execMode == transaction.ExecModeCheck {
		if sig.Sequence < acc.GetSequence() {
			return errorsmod.Wrapf(
				sdkerrors.ErrWrongSequence,
				"account sequence mismatch, expected higher than or equal to %d, got %d", acc.GetSequence(), sig.Sequence,
			)
		}
	} else if sig.Sequence != acc.GetSequence() {
		return errorsmod.Wrapf(
			sdkerrors.ErrWrongSequence,
			"account sequence mismatch: expected %d, got %d", acc.GetSequence(), sig.Sequence,
		)
	}
...

Proposed modification:

func (svd SigVerificationDecorator) verifySig(ctx context.Context, tx sdk.Tx, acc sdk.AccountI, sig signing.SignatureV2, newlyCreated bool) error {
	execMode := svd.ak.GetEnvironment().TransactionService.ExecMode(ctx)
	unorderedTx, ok := tx.(sdk.TxWithUnordered)
	isUnordered := ok && unorderedTx.GetUnordered()
	if !isUnordered {
		if execMode == transaction.ExecModeCheck {
			if sig.Sequence < acc.GetSequence() {
				return errorsmod.Wrapf(
					sdkerrors.ErrWrongSequence,
					"account sequence mismatch, expected higher than or equal to %d, got %d", acc.GetSequence(), sig.Sequence,
				)
			}
		} else if sig.Sequence != acc.GetSequence() {
			return errorsmod.Wrapf(
				sdkerrors.ErrWrongSequence,
				"account sequence mismatch: expected %d, got %d", acc.GetSequence(), sig.Sequence,
			)
		}
	}
...

Patch:

diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go
index 0e52f349a3..fcac40edbc 100644
--- a/x/auth/ante/sigverify.go
+++ b/x/auth/ante/sigverify.go
@@ -320,18 +320,22 @@ func (svd SigVerificationDecorator) consumeSignatureGas(
 // verifySig will verify the signature of the provided signer account.
 func (svd SigVerificationDecorator) verifySig(ctx context.Context, tx sdk.Tx, acc sdk.AccountI, sig signing.SignatureV2, newlyCreated bool) error {
 	execMode := svd.ak.GetEnvironment().TransactionService.ExecMode(ctx)
-	if execMode == transaction.ExecModeCheck {
-		if sig.Sequence < acc.GetSequence() {
+	unorderedTx, ok := tx.(sdk.TxWithUnordered)
+	isUnordered := ok && unorderedTx.GetUnordered()
+	if !isUnordered {
+		if execMode == transaction.ExecModeCheck {
+			if sig.Sequence < acc.GetSequence() {
+				return errorsmod.Wrapf(
+					sdkerrors.ErrWrongSequence,
+					"account sequence mismatch, expected higher than or equal to %d, got %d", acc.GetSequence(), sig.Sequence,
+				)
+			}
+		} else if sig.Sequence != acc.GetSequence() {
 			return errorsmod.Wrapf(
 				sdkerrors.ErrWrongSequence,
-				"account sequence mismatch, expected higher than or equal to %d, got %d", acc.GetSequence(), sig.Sequence,
+				"account sequence mismatch: expected %d, got %d", acc.GetSequence(), sig.Sequence,
 			)
 		}
-	} else if sig.Sequence != acc.GetSequence() {
-		return errorsmod.Wrapf(
-			sdkerrors.ErrWrongSequence,
-			"account sequence mismatch: expected %d, got %d", acc.GetSequence(), sig.Sequence,
-		)
 	}
 
 	// we're in simulation mode, or in ReCheckTx, or context is not

Cosmos SDK Version

0.52

How to reproduce?

Run the bash script.

set -e

which simd > /dev/null || { echo "No simd in the PATH"; exit 1; }

# Use ./zzz as the home of this test case.
dir=./zzz
rm -rf $dir > /dev/null || true
mkdir $dir

SIMD="simd --home $dir"

# Create a new testnet with chain-id `testnet`
$SIMD init --chain-id testnet node0 > /dev/null 2>&1

$SIMD config set client chain-id testnet
$SIMD config set client keyring-backend test
$SIMD config set client output json

# Create the test account
$SIMD keys add test --no-backup > /dev/null
address=$($SIMD keys show test -a)

# Finalize the genesis
$SIMD genesis add-genesis-account $address 1000000000stake > /dev/null 2>&1
$SIMD genesis gentx test 1000000stake > /dev/null 2>&1
$SIMD genesis collect-gentxs > /dev/null 2>&1

# Start the node
$SIMD start --log_level debug > $dir/log.log 2>&1 &
pid=$!
# Kill the node when the script exits
trap "kill -9 $pid" EXIT

sleep 2
# Send an ordered transactions: update the sequence number 
$SIMD tx bank send $address $address 10stake --yes --from $address --broadcast-mode sync 
# Unordered tx with timeout 100 seconds and sequence number 0
$SIMD tx bank send $address $address 10stake --fees 3stake --from $address --unordered --timeout-timestamp $(( $(date +%s) + 100)) --offline -a 0 -s 0 --yes 

Output:

error in json rpc client, with http response metadata: (Status: 200 OK, Protocol HTTP/1.1). RPC error -32603 - Internal error: broadcast error on transaction validation: tx 920E7FA01BE1C711691F21E5FB9ECCC85F893146284D2788CF85FEEA44E9C54C is invalid: code=32, data=, log='account sequence mismatch, expected higher than or equal to 2, got 0: incorrect account sequence', codespace='sdk'

Expected:

{"height":"0","txhash":"920E7FA01BE1C711691F21E5FB9ECCC85F893146284D2788CF85FEEA44E9C54C","codespace":"","code":0,"data":"","raw_log":"","logs":[],"info":"","gas_wanted":"0","gas_used":"0","tx":null,"timestamp":"","events":[]}

[julienrbrt]

You are right, this is already being addressed in this issue #23392.

[pakuula]

You are right, this is already being addressed in this issue #23392.

Thank you for the link. I tested the branch facu/fix-unordered-part2 and it works.

@julienrbrt should I close this issue, or wait until #23392 is merged and ported to release/v0.52x branch?

[julienrbrt]

It just got merged, we can close #23392.
Thanks again for submitting the issue still :)

[pakuula]

@julienrbrt do you plan to merge the port the fix from #23571 to release/v0.52.x?

[julienrbrt]

@julienrbrt do you plan to merge the port the fix from #23571 to release/v0.52.x?

Yes, there is a backport already ready to be merged.