Refactor timestamp nonce ante handler and sigverify

protocol/app/ante.go

@@ -16,6 +16,7 @@ import (
 	"github.com/dydxprotocol/v4-chain/protocol/lib"
 	libante "github.com/dydxprotocol/v4-chain/protocol/lib/ante"
 	"github.com/dydxprotocol/v4-chain/protocol/lib/log"
+	accountplusante "github.com/dydxprotocol/v4-chain/protocol/x/accountplus/ante"
 	accountpluskeeper "github.com/dydxprotocol/v4-chain/protocol/x/accountplus/keeper"
 	clobante "github.com/dydxprotocol/v4-chain/protocol/x/clob/ante"
 	clobtypes "github.com/dydxprotocol/v4-chain/protocol/x/clob/types"
@@ -28,8 +29,10 @@ import (
 // struct embedding to include the normal cosmos-sdk `HandlerOptions`.
 type HandlerOptions struct {
 	ante.HandlerOptions
-	Codec             codec.Codec
-	AuthStoreKey      storetypes.StoreKey
+	Codec        codec.Codec
+	AuthStoreKey storetypes.StoreKey
+	// TODO: create interface for accountplus keeper when mocking need arises
+	// https://github.com/dydxprotocol/v4-chain/pull/1963#discussion_r1691904072
 	AccountplusKeeper *accountpluskeeper.Keeper
 	ClobKeeper        clobtypes.ClobKeeper
 	PerpetualsKeeper  perpetualstypes.PerpetualsKeeper
@@ -111,7 +114,7 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) {
 		validateMemo:             ante.NewValidateMemoDecorator(options.AccountKeeper),
 		validateBasic:            ante.NewValidateBasicDecorator(),
 		validateSigCount:         ante.NewValidateSigCountDecorator(options.AccountKeeper),
-		incrementSequence:        customante.NewIncrementSequenceDecorator(options.AccountKeeper),
+		incrementSequence:        ante.NewIncrementSequenceDecorator(options.AccountKeeper),
 		sigVerification: customante.NewSigVerificationDecorator(
 			options.AccountKeeper,
 			*options.AccountplusKeeper,
@@ -150,7 +153,7 @@ type lockingAnteHandler struct {
 	validateMemo             ante.ValidateMemoDecorator
 	validateBasic            ante.ValidateBasicDecorator
 	validateSigCount         ante.ValidateSigCountDecorator
-	incrementSequence        customante.IncrementSequenceDecorator
+	incrementSequence        ante.IncrementSequenceDecorator
 	sigVerification          customante.SigVerificationDecorator
 	consumeTxSizeGas         ante.ConsumeTxSizeGasDecorator
 	deductFee                ante.DeductFeeDecorator
@@ -248,7 +251,13 @@ func (h *lockingAnteHandler) clobAnteHandle(ctx sdk.Context, tx sdk.Tx, simulate
 	if isShortTerm, err = clobante.IsShortTermClobMsgTx(ctx, tx); err != nil {
 		return ctx, err
 	}
-	if !isShortTerm {
+
+	var isTimestampNonce bool
+	if isTimestampNonce, err = accountplusante.IsTimestampNonceTx(ctx, tx); err != nil {
+		return ctx, err
+	}
+
+	if !isShortTerm && !isTimestampNonce {
 		if ctx, err = h.incrementSequence.AnteHandle(ctx, tx, simulate, noOpAnteHandle); err != nil {
 			return ctx, err
 		}

protocol/app/ante/sigverify.go

@@ -2,6 +2,7 @@ package ante
 
 import (
 	"fmt"
+	"time"
 
 	errorsmod "cosmossdk.io/errors"
 	txsigning "cosmossdk.io/x/tx/signing"
@@ -63,7 +64,10 @@ func (svd SigVerificationDecorator) AnteHandle(
 		return ctx, err
 	}
 
-	// check that signer length and signature length are the same
+	// Check that signer length and signature length are the same.
+	// If the lengths are the same, then sigs and signers are ordered correctly. This means that each expected signer
+	// provided a signature. In cosmos original sigverify, the ordering is explicity checked.
+	//https://github.com/cosmos/cosmos-sdk/blob/502450cd1ef0b6bd77807922091893611c370c5d/x/auth/ante/sigverify.go#L189
 	if len(sigs) != len(signers) {
 		err := errorsmod.Wrapf(
 			sdkerrors.ErrUnauthorized,
@@ -78,6 +82,7 @@ func (svd SigVerificationDecorator) AnteHandle(
 	// only messages that use `GoodTilBlock` for replay protection.
 	skipSequenceValidation := ShouldSkipSequenceValidation(tx.GetMsgs())
 
+	// Iterate on sig and signer pairs.
 	for i, sig := range sigs {
 		acc, err := sdkante.GetSignerAcc(ctx, svd.ak, signers[i])
 		if err != nil {
@@ -95,35 +100,24 @@ func (svd SigVerificationDecorator) AnteHandle(
 		// `GoodTilBlock` for replay protection.
 		if !skipSequenceValidation {
 			if accountpluskeeper.IsTimestampNonce(sig.Sequence) {
-				tsNonce := sig.Sequence
-				blockTs := uint64(ctx.BlockTime().UnixMilli())
-				address := acc.GetAddress()
+				start := time.Now()
+				err = svd.akp.ProcessTimestampNonce(ctx, acc, sig.Sequence)
 
-				if !accountpluskeeper.IsValidTimestampNonce(tsNonce, blockTs) {
-					return ctx, errorsmod.Wrapf(
-						sdkerrors.ErrWrongSequence,
-						"timestamp nonce %d not within valid time window", tsNonce,
+				telemetry.MeasureSince(start, []string{metrics.TimestampNonce, metrics.Latency}...)
+				if err == nil {
+					telemetry.IncrCounterWithLabels(
+						[]string{metrics.TimestampNonce, metrics.Valid, metrics.Count},
+						1,
+						[]gometrics.Label{metrics.GetLabelForIntValue(metrics.ExecMode, int(ctx.ExecMode()))},
 					)
-				}
-				accountState, found := svd.akp.GetAccountState(ctx, address)
-				if !found {
-					err := svd.akp.InitializeAccountWithTimestampNonceDetails(ctx, address, tsNonce)
-					if err != nil {
-						return ctx, errorsmod.Wrapf(
-							sdkerrors.ErrLogic,
-							fmt.Sprintf("failed to initialize AccountState for address %d", address),
-						)
-					}
+					return ctx, nil
 				} else {
-					accountpluskeeper.EjectStaleTimestampNonces(&accountState, blockTs)
-					tsNonceAccepted := accountpluskeeper.AttemptTimestampNonceUpdate(tsNonce, &accountState)
-					if !tsNonceAccepted {
-						return ctx, errorsmod.Wrapf(
-							sdkerrors.ErrWrongSequence,
-							"timestamp nonce %d rejected", tsNonce,
-						)
-					}
-					svd.akp.SetAccountState(ctx, address, accountState)
+					telemetry.IncrCounterWithLabels(
+						[]string{metrics.TimestampNonce, metrics.Invalid, metrics.Count},
+						1,
+						[]gometrics.Label{metrics.GetLabelForIntValue(metrics.ExecMode, int(ctx.ExecMode()))},
+					)
+					return ctx, errorsmod.Wrapf(sdkerrors.ErrWrongSequence, err.Error())
 				}
 			} else {
 				if sig.Sequence != acc.GetSequence() {

protocol/lib/metrics/constants.go

@@ -11,6 +11,7 @@ const (
 	Deterministic    = "deterministic"
 	Distribution     = "distribution"
 	Error            = "error"
+	ExecMode         = "exec_mode"
 	GitCommit        = "git_commit"
 	HttpGet5xx       = "http_get_5xx"
 	HttpGetHangup    = "http_get_hangup"
@@ -407,6 +408,9 @@ const (
 	ValidatorNumMatchedTakerOrders = "validator_num_matched_taker_orders"
 	ValidatorVolumeQuoteQuantums   = "validator_volume_quote_quantums"
 
+	// x/acocuntplus
+	TimestampNonce = "timestamp_nonce"
+
 	// x/ratelimit
 	Capacity           = "capacity"
 	RateLimitDenom     = "rate_limit_denom"

protocol/x/accountplus/ante/timestampnonce.go

@@ -0,0 +1,32 @@
+package ante
+
+import (
+	errorsmod "cosmossdk.io/errors"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"
+	accountpluskeeper "github.com/dydxprotocol/v4-chain/protocol/x/accountplus/keeper"
+)
+
+// IsTimestampNonceTx returns `true` if the supplied `tx` consist of a single signature that uses a timestamp nonce
+// value for sequence
+func IsTimestampNonceTx(ctx sdk.Context, tx sdk.Tx) (bool, error) {
+	sigTx, ok := tx.(authsigning.SigVerifiableTx)
+	if !ok {
+		return false, errorsmod.Wrap(sdkerrors.ErrTxDecode, "invalid transaction type")
+	}
+	signatures, err := sigTx.GetSignaturesV2()
+	if err != nil {
+		return false, err
+	}
+
+	if len(signatures) != 1 {
+		return false, errorsmod.Wrap(sdkerrors.ErrTxDecode, "more than one signature")
+	}
+
+	if accountpluskeeper.IsTimestampNonce(signatures[0].Sequence) {
+		return true, nil
+	} else {
+		return false, nil
+	}
+}