Skip to content

Commit

Permalink
Ignore short-term clob messages with non-zero tx timeout. (backport #…
Browse files Browse the repository at this point in the history
…1849) (#1850)

Co-authored-by: vincentwschau <[email protected]>
  • Loading branch information
mergify[bot] and vincentwschau authored Jul 4, 2024
1 parent a3ea114 commit dac2239
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 1 deletion.
3 changes: 3 additions & 0 deletions protocol/testutil/sdk/tx/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package tx

import (
"context"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/tx"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
Expand All @@ -18,10 +19,12 @@ func CreateTestTx(
accSeqs []uint64,
chainID string,
msgs []sdk.Msg,
timeoutHeight uint64,
) (xauthsigning.Tx, error) {
encodingConfig := app.GetEncodingConfig()
clientCtx := client.Context{}.WithTxConfig(encodingConfig.TxConfig)
txBuilder := clientCtx.TxConfig.NewTxBuilder()
txBuilder.SetTimeoutHeight(timeoutHeight)
err := txBuilder.SetMsgs(msgs...)
if err != nil {
return nil, err
Expand Down
30 changes: 30 additions & 0 deletions protocol/x/clob/ante/clob.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ import (
satypes "github.com/dydxprotocol/v4-chain/protocol/x/subaccounts/types"
)

var (
timeoutHeightLogKey = "TimeoutHeight"
)

// ClobDecorator is an AnteDecorator which is responsible for:
// - adding short term order placements and cancelations to the in-memory orderbook (`CheckTx` only).
// - adding stateful order placements and cancelations to state (`CheckTx` and `RecheckTx` only).
Expand Down Expand Up @@ -106,6 +110,20 @@ func (cd ClobDecorator) AnteHandle(
return next(ctx, tx, simulate)
}

// HOTFIX: Ignore any short-term place orders in a transaction with a timeout height.
if timeoutHeight := GetTimeoutHeight(tx); timeoutHeight > 0 && ctx.IsCheckTx() {
log.InfoLog(
ctx,
"Rejected short-term place order with non-zero timeout height",
timeoutHeightLogKey,
timeoutHeight,
)
return ctx, errorsmod.Wrap(
sdkerrors.ErrInvalidRequest,
"a short term place order message may not have a non-zero timeout height, use goodTilBlock instead",
)
}

var orderSizeOptimisticallyFilledFromMatchingQuantums satypes.BaseQuantums
var status types.OrderStatus
// Note that `msg.ValidateBasic` is called before all AnteHandlers.
Expand Down Expand Up @@ -239,3 +257,15 @@ func IsShortTermClobMsgTx(ctx sdk.Context, tx sdk.Tx) (bool, error) {

return true, nil
}

// GetTimeoutHeight returns the timeout height of a transaction. If the transaction does not have
// a timeout height, return 0.
func GetTimeoutHeight(tx sdk.Tx) uint64 {
timeoutTx, ok := tx.(sdk.TxWithTimeoutHeight)
if !ok {
return 0
}

timeoutHeight := timeoutTx.GetTimeoutHeight()
return timeoutHeight
}
29 changes: 28 additions & 1 deletion protocol/x/clob/ante/clob_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package ante_test
import (
"testing"

errorsmod "cosmossdk.io/errors"
"cosmossdk.io/log"
"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
Expand All @@ -28,7 +29,9 @@ type TestCase struct {
useWithIsCheckTxContext bool
useWithIsRecheckTxContext bool
isSimulate bool
timeoutHeight uint64
expectedErr error
additionalAssertions func(ctx sdk.Context, mck *mocks.ClobKeeper)
}

func runTestCase(t *testing.T, tc TestCase) {
Expand Down Expand Up @@ -60,7 +63,7 @@ func runTestCase(t *testing.T, tc TestCase) {
// Create Test Transaction.
priv1, _, _ := testdata.KeyTestPubAddr()
privs, accNums, accSeqs := []cryptotypes.PrivKey{priv1}, []uint64{0}, []uint64{0}
tx, err := txtest.CreateTestTx(privs, accNums, accSeqs, "dydx", tc.msgs)
tx, err := txtest.CreateTestTx(privs, accNums, accSeqs, "dydx", tc.msgs, tc.timeoutHeight)
require.NoError(t, err)

// Call Antehandler.
Expand All @@ -76,6 +79,10 @@ func runTestCase(t *testing.T, tc TestCase) {
// Assert mock expectations.
result := mockClobKeeper.AssertExpectations(t)
require.True(t, result)

if tc.additionalAssertions != nil {
tc.additionalAssertions(ctx, mockClobKeeper)
}
}

func TestClobDecorator_MsgPlaceOrder(t *testing.T) {
Expand Down Expand Up @@ -215,6 +222,26 @@ func TestClobDecorator_MsgPlaceOrder(t *testing.T) {
useWithIsCheckTxContext: true,
expectedErr: sdkerrors.ErrInvalidRequest,
},
// Test for hotfix.
"PlaceShortTermOrder is not called on keeper CheckTx if transaction timeout height is non-zero": {
msgs: []sdk.Msg{constants.Msg_PlaceOrder},
useWithIsCheckTxContext: true,
useWithIsRecheckTxContext: false,
isSimulate: false,
expectedErr: errorsmod.Wrap(
sdkerrors.ErrInvalidRequest,
"a short term place order message may not have a non-zero timeout height, use goodTilBlock instead",
),
timeoutHeight: 1,
additionalAssertions: func(ctx sdk.Context, mck *mocks.ClobKeeper) {
mck.AssertNotCalled(
t,
"PlaceShortTermOrder",
ctx,
constants.Msg_PlaceOrder,
)
},
},
}

// Run tests.
Expand Down

0 comments on commit dac2239

Please sign in to comment.