Skip to content

Commit

Permalink
[OTE-553] Timestamp nonce (#1970)
Browse files Browse the repository at this point in the history
minor refactor on timestamp nonce ante handler and keeper
  • Loading branch information
jerryfan01234 authored Jul 26, 2024
1 parent 749dff9 commit 7542acc
Show file tree
Hide file tree
Showing 11 changed files with 188 additions and 242 deletions.
13 changes: 10 additions & 3 deletions protocol/app/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -112,7 +113,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,
Expand Down Expand Up @@ -153,7 +154,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
Expand Down Expand Up @@ -251,7 +252,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
}
Expand Down
43 changes: 16 additions & 27 deletions protocol/app/ante/sigverify.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ 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.
// The ordering of the sigs and signers have matching ordering (sigs[i] belongs to signers[i]).
if len(sigs) != len(signers) {
err := errorsmod.Wrapf(
sdkerrors.ErrUnauthorized,
Expand All @@ -78,6 +79,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 {
Expand All @@ -95,35 +97,22 @@ 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()
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,
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() {
Expand Down
57 changes: 0 additions & 57 deletions protocol/app/ante/timestampnonce.go

This file was deleted.

75 changes: 0 additions & 75 deletions protocol/app/ante/timestampnonce_test.go

This file was deleted.

4 changes: 4 additions & 0 deletions protocol/lib/metrics/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
28 changes: 28 additions & 0 deletions protocol/x/accountplus/ante/timestampnonce.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
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")
}

return accountpluskeeper.IsTimestampNonce(signatures[0].Sequence), nil
}
85 changes: 85 additions & 0 deletions protocol/x/accountplus/ante/timestampnonce_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package ante_test

import (
"testing"

"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/testutil/testdata"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/tx/signing"
authtx "github.com/cosmos/cosmos-sdk/x/auth/tx"
"github.com/dydxprotocol/v4-chain/protocol/testutil/constants"
sdktest "github.com/dydxprotocol/v4-chain/protocol/testutil/sdk"
"github.com/dydxprotocol/v4-chain/protocol/x/accountplus/ante"
"github.com/dydxprotocol/v4-chain/protocol/x/accountplus/keeper"
"github.com/stretchr/testify/require"
)

func TestIsTimestampNonceTx(t *testing.T) {
tests := map[string]struct {
seqs []uint64
expectedResult bool
expectedErr bool
}{
"Returns false for non-ts nonce": {
seqs: []uint64{0},
expectedResult: false,
expectedErr: false,
},
"Returns true for ts nonce": {
seqs: []uint64{keeper.TimestampNonceSequenceCutoff},
expectedResult: true,
expectedErr: false,
},
"Returns error for more than one signature": {
seqs: []uint64{keeper.TimestampNonceSequenceCutoff, keeper.TimestampNonceSequenceCutoff},
expectedResult: false,
expectedErr: true,
},
}

// Run tests.
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
// Initialize some test setup which builds a test transaction from a slice of messages.
var reg codectypes.InterfaceRegistry
protoCfg := authtx.NewTxConfig(codec.NewProtoCodec(reg), authtx.DefaultSignModes)
builder := protoCfg.NewTxBuilder()
err := builder.SetMsgs([]sdk.Msg{constants.Msg_Send}...)
require.NoError(t, err)

// Create signatures
var signatures []signing.SignatureV2
for _, seq := range tc.seqs {
signatures = append(signatures, getSignature(seq))
}
err = builder.SetSignatures(signatures...)

require.NoError(t, err)
tx := builder.GetTx()
ctx, _, _ := sdktest.NewSdkContextWithMultistore()

// Invoke the function under test.
result, err := ante.IsTimestampNonceTx(ctx, tx)

// Assert the results.
if tc.expectedErr {
require.NotNil(t, err)
}
require.Equal(t, tc.expectedResult, result)
})
}
}

func getSignature(seq uint64) signing.SignatureV2 {
_, pubKey, _ := testdata.KeyTestPubAddr()
return signing.SignatureV2{
PubKey: pubKey,
Data: &signing.SingleSignatureData{
SignMode: signing.SignMode_SIGN_MODE_DIRECT,
Signature: nil,
},
Sequence: seq,
}
}
Loading

0 comments on commit 7542acc

Please sign in to comment.