Skip to content

Commit

Permalink
resovle merge conflict from local branch
Browse files Browse the repository at this point in the history
  • Loading branch information
jerryfan01234 committed Jul 23, 2024
2 parents a6ae93c + 3fdb5b8 commit 9e4e31b
Show file tree
Hide file tree
Showing 12 changed files with 280 additions and 35 deletions.
4 changes: 2 additions & 2 deletions protocol/app/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) {
validateMemo: ante.NewValidateMemoDecorator(options.AccountKeeper),
validateBasic: ante.NewValidateBasicDecorator(),
validateSigCount: ante.NewValidateSigCountDecorator(options.AccountKeeper),
incrementSequence: ante.NewIncrementSequenceDecorator(options.AccountKeeper),
incrementSequence: customante.NewIncrementSequenceDecorator(options.AccountKeeper),
sigVerification: customante.NewSigVerificationDecorator(options.AccountKeeper, options.SignModeHandler),
consumeTxSizeGas: ante.NewConsumeGasForTxSizeDecorator(options.AccountKeeper),
deductFee: ante.NewDeductFeeDecorator(
Expand Down Expand Up @@ -140,7 +140,7 @@ type lockingAnteHandler struct {
validateMemo ante.ValidateMemoDecorator
validateBasic ante.ValidateBasicDecorator
validateSigCount ante.ValidateSigCountDecorator
incrementSequence ante.IncrementSequenceDecorator
incrementSequence customante.IncrementSequenceDecorator
sigVerification customante.SigVerificationDecorator
consumeTxSizeGas ante.ConsumeTxSizeGasDecorator
deductFee ante.DeductFeeDecorator
Expand Down
55 changes: 55 additions & 0 deletions protocol/app/ante/timestampnonce.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package ante

import (
errorsmod "cosmossdk.io/errors"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
ante "github.com/cosmos/cosmos-sdk/x/auth/ante"
authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"
accountpluskeeper "github.com/dydxprotocol/v4-chain/protocol/x/accountplus/keeper"
)

// TODO: combine increment sequence and sequence verification into one decorator
// https://github.com/cosmos/cosmos-sdk/pull/18817
type IncrementSequenceDecorator struct {
ak ante.AccountKeeper
}

func NewIncrementSequenceDecorator(ak ante.AccountKeeper) IncrementSequenceDecorator {
return IncrementSequenceDecorator{
ak: ak,
}
}

func (isd IncrementSequenceDecorator) AnteHandle(
ctx sdk.Context,
tx sdk.Tx,
simulate bool,
next sdk.AnteHandler,
) (sdk.Context, error) {
sigTx, ok := tx.(authsigning.SigVerifiableTx)
if !ok {
return ctx, errorsmod.Wrap(sdkerrors.ErrTxDecode, "invalid transaction type")
}

signatures, err := sigTx.GetSignaturesV2()
if err != nil {
return sdk.Context{}, err
}

for _, signature := range signatures {
if accountpluskeeper.IsTimestampNonce(signature.Sequence) {
// Skip increment for this signature
continue
}

acc := isd.ak.GetAccount(ctx, signature.PubKey.Address().Bytes())
if err := acc.SetSequence(acc.GetSequence() + 1); err != nil {
panic(err)
}

isd.ak.SetAccount(ctx, acc)
}

return next(ctx, tx, simulate)
}
75 changes: 75 additions & 0 deletions protocol/app/ante/timestampnonce_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package ante_test

import (
"testing"

testante "github.com/dydxprotocol/v4-chain/protocol/testutil/ante"
"github.com/stretchr/testify/require"

cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
"github.com/cosmos/cosmos-sdk/testutil/testdata"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/tx/signing"

customante "github.com/dydxprotocol/v4-chain/protocol/app/ante"
accountpluskeeper "github.com/dydxprotocol/v4-chain/protocol/x/accountplus/keeper"
)

// Modified from cosmossdk test for IncrementSequenceDecorator
func TestDydxIncrementSequenceDecorator(t *testing.T) {
suite := testante.SetupTestSuite(t, true)
suite.TxBuilder = suite.ClientCtx.TxConfig.NewTxBuilder()

priv, _, addr := testdata.KeyTestPubAddr()
acc := suite.AccountKeeper.NewAccountWithAddress(suite.Ctx, addr)
require.NoError(t, acc.SetAccountNumber(uint64(50)))
suite.AccountKeeper.SetAccount(suite.Ctx, acc)

msgs := []sdk.Msg{testdata.NewTestMsg(addr)}
require.NoError(t, suite.TxBuilder.SetMsgs(msgs...))
privs := []cryptotypes.PrivKey{priv}
accNums := []uint64{suite.AccountKeeper.GetAccount(suite.Ctx, addr).GetAccountNumber()}
feeAmount := testdata.NewTestFeeAmount()
gasLimit := testdata.NewTestGasLimit()
suite.TxBuilder.SetFeeAmount(feeAmount)
suite.TxBuilder.SetGasLimit(gasLimit)

isd := customante.NewIncrementSequenceDecorator(suite.AccountKeeper)
antehandler := sdk.ChainAnteDecorators(isd)

testCases := []struct {
ctx sdk.Context
simulate bool
// This value need not be valid (accountSeq + 1). Validity is handed in customante.NewSigVerificationDecorator
signatureSeq uint64
expectedSeq uint64
}{
// tests from cosmossdk checking incrementing seqence
{suite.Ctx.WithIsReCheckTx(true), false, 0, 1},
{suite.Ctx.WithIsCheckTx(true).WithIsReCheckTx(false), false, 0, 2},
{suite.Ctx.WithIsReCheckTx(true), false, 0, 3},
{suite.Ctx.WithIsReCheckTx(true), false, 0, 4},
{suite.Ctx.WithIsReCheckTx(true), true, 0, 5},

// tests checking that tx with timestamp nonces will not increment sequence
{suite.Ctx.WithIsReCheckTx(true), true, accountpluskeeper.TimestampNonceSequenceCutoff, 5},
{suite.Ctx.WithIsReCheckTx(true), true, accountpluskeeper.TimestampNonceSequenceCutoff + 100000, 5},
}

for i, tc := range testCases {
accSeqs := []uint64{tc.signatureSeq}
tx, err := suite.CreateTestTx(
suite.Ctx,
privs,
accNums,
accSeqs,
suite.Ctx.ChainID(),
signing.SignMode_SIGN_MODE_DIRECT,
)
require.NoError(t, err)

_, err = antehandler(tc.ctx, tx, tc.simulate)
require.NoError(t, err, "unexpected error; tc #%d, %v", i, tc)
require.Equal(t, tc.expectedSeq, suite.AccountKeeper.GetAccount(suite.Ctx, addr).GetSequence())
}
}
2 changes: 2 additions & 0 deletions protocol/x/accountplus/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ func (k Keeper) SetGenesisState(ctx sdk.Context, data types.GenesisState) error
return nil
}

// TODO: refactor this function -> InitializeWithTimestampNonceDetails
// Writing to store is expensive so directly write with ts-nonce instead of initializing an empty account then setting.
func (k Keeper) InitializeAccount(ctx sdk.Context, address sdk.AccAddress) error {
if _, found := k.GetAccountState(ctx, address); found {
return errors.New(
Expand Down
6 changes: 5 additions & 1 deletion protocol/x/accountplus/keeper/timestampnonce.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
package keeper

func Placeholder() {}
const TimestampNonceSequenceCutoff uint64 = 1 << 40 // 2^40

func IsTimestampNonce(ts uint64) bool {
return ts >= TimestampNonceSequenceCutoff
}
9 changes: 8 additions & 1 deletion protocol/x/accountplus/keeper/timestampnonce_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@ package keeper_test

import (
"testing"

"github.com/dydxprotocol/v4-chain/protocol/x/accountplus/keeper"
"github.com/stretchr/testify/require"
)

func Placeholder(t *testing.T) {}
func TestIsTimestampNonce(t *testing.T) {
require.True(t, keeper.IsTimestampNonce(keeper.TimestampNonceSequenceCutoff))
require.True(t, keeper.IsTimestampNonce(keeper.TimestampNonceSequenceCutoff+uint64(1)))
require.False(t, keeper.IsTimestampNonce(keeper.TimestampNonceSequenceCutoff-uint64(1)))
}
68 changes: 68 additions & 0 deletions protocol/x/clob/e2e/rate_limit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -805,3 +805,71 @@ func TestRateLimitingShortTermOrders_GuardedAgainstReplayAttacks(t *testing.T) {
})
}
}

func TestRateLimitingOrders_StatefulOrdersNotCountedDuringRecheck(t *testing.T) {
blockRateLimitConfig := clobtypes.BlockRateLimitConfiguration{
MaxStatefulOrdersPerNBlocks: []clobtypes.MaxPerNBlocksRateLimit{
{
NumBlocks: 2,
Limit: 2,
},
},
}
firstMsg := &LongTermPlaceOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5
secondMsg := &LongTermPlaceOrder_Alice_Num0_Id0_Clob1_Buy5_Price10_GTBT5

tApp := testapp.NewTestAppBuilder(t).
// Disable non-determinism checks since we mutate keeper state directly.
WithNonDeterminismChecksEnabled(false).
WithGenesisDocFn(func() (genesis types.GenesisDoc) {
genesis = testapp.DefaultGenesis()
testapp.UpdateGenesisDocWithAppStateForModule(
&genesis,
func(genesisState *clobtypes.GenesisState) {
genesisState.BlockRateLimitConfig = blockRateLimitConfig
},
)
testapp.UpdateGenesisDocWithAppStateForModule(
&genesis,
func(genesisState *satypes.GenesisState) {
genesisState.Subaccounts = []satypes.Subaccount{
constants.Alice_Num0_10_000USD,
constants.Alice_Num1_10_000USD,
}
})
return genesis
}).Build()
ctx := tApp.InitChain()

firstCheckTx := testapp.MustMakeCheckTx(
ctx,
tApp.App,
testapp.MustMakeCheckTxOptions{
AccAddressForSigning: testtx.MustGetOnlySignerAddress(tApp.App.AppCodec(), firstMsg),
},
firstMsg,
)
ctx = tApp.AdvanceToBlock(2, testapp.AdvanceToBlockOptions{})

// First transaction should be allowed.
resp := tApp.CheckTx(firstCheckTx)
require.Conditionf(t, resp.IsOK, "Expected CheckTx to succeed. Response: %+v", resp)

tApp.AdvanceToBlock(3, testapp.AdvanceToBlockOptions{
// First transaction did not get proposed in a block.
// Txn remains in the mempool and will get rechecked.
DeliverTxsOverride: [][]byte{},
})

// Rate limit is 2 over two block, second attempt should be allowed.
secondCheckTx := testapp.MustMakeCheckTx(
ctx,
tApp.App,
testapp.MustMakeCheckTxOptions{
AccAddressForSigning: testtx.MustGetOnlySignerAddress(tApp.App.AppCodec(), secondMsg),
},
secondMsg,
)
resp = tApp.CheckTx(secondCheckTx)
require.Conditionf(t, resp.IsOK, "Expected CheckTx to succeed. Response: %+v", resp)
}
19 changes: 12 additions & 7 deletions protocol/x/clob/keeper/rate_limit.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,16 @@ import (
"github.com/dydxprotocol/v4-chain/protocol/x/clob/types"
)

// The rate limiting is only performed during `CheckTx`.
// Rate limiting during `ReCheckTx` might result in over counting.
func (k *Keeper) ShouldRateLimit(ctx sdk.Context) bool {
return ctx.IsCheckTx() && !ctx.IsReCheckTx()
}

// RateLimitCancelOrder passes order cancellations with valid clob pairs to `cancelOrderRateLimiter`.
// The rate limiting is only performed during `CheckTx` and `ReCheckTx`.
func (k *Keeper) RateLimitCancelOrder(ctx sdk.Context, msg *types.MsgCancelOrder) error {
// Only rate limit during `CheckTx` and `ReCheckTx`.
if lib.IsDeliverTxMode(ctx) {
// Only rate limit during `CheckTx`.
if !k.ShouldRateLimit(ctx) {
return nil
}

Expand All @@ -37,8 +42,8 @@ func (k *Keeper) RateLimitCancelOrder(ctx sdk.Context, msg *types.MsgCancelOrder
// RateLimitPlaceOrder passes orders with valid clob pairs to `placeOrderRateLimiter`.
// The rate limiting is only performed during `CheckTx` and `ReCheckTx`.
func (k *Keeper) RateLimitPlaceOrder(ctx sdk.Context, msg *types.MsgPlaceOrder) error {
// Only rate limit during `CheckTx` and `ReCheckTx`.
if lib.IsDeliverTxMode(ctx) {
// Only rate limit during `CheckTx`.
if !k.ShouldRateLimit(ctx) {
return nil
}

Expand All @@ -65,8 +70,8 @@ func (k *Keeper) RateLimitPlaceOrder(ctx sdk.Context, msg *types.MsgPlaceOrder)
// RateLimitBatchCancel passes orders with valid clob pairs to `placeOrderRateLimiter`.
// The rate limiting is only performed during `CheckTx` and `ReCheckTx`.
func (k *Keeper) RateLimitBatchCancel(ctx sdk.Context, msg *types.MsgBatchCancel) error {
// Only rate limit during `CheckTx` and `ReCheckTx`.
if lib.IsDeliverTxMode(ctx) {
// Only rate limit during `CheckTx`.
if !k.ShouldRateLimit(ctx) {
return nil
}

Expand Down
47 changes: 23 additions & 24 deletions protocol/x/vault/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"runtime/debug"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/dydxprotocol/v4-chain/protocol/lib/abci"
"github.com/dydxprotocol/v4-chain/protocol/lib/log"
"github.com/dydxprotocol/v4-chain/protocol/x/vault/keeper"
)
Expand All @@ -14,18 +15,17 @@ func BeginBlocker(
) {
// Panic is not expected in BeginBlocker, but we should recover instead of
// halting the chain.
defer func() {
if r := recover(); r != nil {
log.ErrorLog(
ctx,
"panic in vault BeginBlocker",
"stack",
string(debug.Stack()),
)
}
}()

keeper.DecommissionNonPositiveEquityVaults(ctx)
if err := abci.RunCached(ctx, func(ctx sdk.Context) error {
keeper.DecommissionNonPositiveEquityVaults(ctx)
return nil
}); err != nil {
log.ErrorLog(
ctx,
"panic in vault BeginBlocker",
"stack",
string(debug.Stack()),
)
}
}

func EndBlocker(
Expand All @@ -34,16 +34,15 @@ func EndBlocker(
) {
// Panic is not expected in EndBlocker, but we should recover instead of
// halting the chain.
defer func() {
if r := recover(); r != nil {
log.ErrorLog(
ctx,
"panic in vault EndBlocker",
"stack",
string(debug.Stack()),
)
}
}()

keeper.RefreshAllVaultOrders(ctx)
if err := abci.RunCached(ctx, func(ctx sdk.Context) error {
keeper.RefreshAllVaultOrders(ctx)
return nil
}); err != nil {
log.ErrorLog(
ctx,
"panic in vault EndBlocker",
"stack",
string(debug.Stack()),
)
}
}
10 changes: 10 additions & 0 deletions protocol/x/vault/keeper/orders.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,12 @@ func (k Keeper) GetVaultClobOrders(
err,
fmt.Sprintf("VaultId: %v", vaultId),
)
} else if marketPrice.Price == 0 {
// Market price can be zero upon market initialization or due to invalid exchange config.
return orders, errorsmod.Wrap(
types.ErrZeroMarketPrice,
fmt.Sprintf("VaultId: %v", vaultId),
)
}

// Calculate leverage = open notional / equity.
Expand Down Expand Up @@ -215,6 +221,8 @@ func (k Keeper) GetVaultClobOrders(
orderSize.Quo(orderSize, lib.BigIntOneMillion())

// Round (towards-zero) order size to the nearest multiple of step size.
// Note: below division by StepBaseQuantums is safe as x/clob disallows
// a clob pair's StepBaseQuantums to be zero.
stepSize := lib.BigU(clobPair.StepBaseQuantums)
orderSize.Quo(orderSize, stepSize).Mul(orderSize, stepSize)

Expand Down Expand Up @@ -315,6 +323,8 @@ func (k Keeper) GetVaultClobOrders(
}

// Bound subticks between the minimum and maximum subticks.
// Note: below division by SubticksPerTick is safe as x/clob disallows
// a clob pair's SubticksPerTick to be zero.
subticksPerTick := lib.BigU(clobPair.SubticksPerTick)
subticks = lib.BigIntRoundToMultiple(
subticks,
Expand Down
Loading

0 comments on commit 9e4e31b

Please sign in to comment.