Skip to content

Commit

Permalink
Duplicate validation checking for ProcessProposerMatchesEvents (#708)
Browse files Browse the repository at this point in the history
* minor fix

* dont need liquidation check

* basic validation ppme

* fixes to tests

* Fix tests again

* pr comments

* lint
  • Loading branch information
jonfung-dydx authored Oct 27, 2023
1 parent 1089ad2 commit da31d6e
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 25 deletions.
4 changes: 2 additions & 2 deletions protocol/x/clob/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1143,7 +1143,7 @@ func TestBeginBlocker_Success(t *testing.T) {
types.ProcessProposerMatchesEvents{
PlacedLongTermOrderIds: []types.OrderId{
constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT15.OrderId,
constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT20.OrderId,
constants.LongTermOrder_Carl_Num0_Id0_Clob0_Sell1BTC_Price50000_GTBT10.OrderId,
constants.LongTermOrder_Bob_Num0_Id0_Clob0_Buy25_Price30_GTBT10.OrderId,
},
ExpiredStatefulOrderIds: []types.OrderId{
Expand All @@ -1163,7 +1163,7 @@ func TestBeginBlocker_Success(t *testing.T) {
types.ProcessProposerMatchesEvents{
PlacedLongTermOrderIds: []types.OrderId{
constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT15.OrderId,
constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT20.OrderId,
constants.LongTermOrder_Carl_Num0_Id0_Clob0_Sell1BTC_Price50000_GTBT10.OrderId,
constants.LongTermOrder_Bob_Num0_Id0_Clob0_Buy25_Price30_GTBT10.OrderId,
},
ExpiredStatefulOrderIds: []types.OrderId{
Expand Down
9 changes: 4 additions & 5 deletions protocol/x/clob/keeper/process_proposer_matches_events.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package keeper

import (
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/dydxprotocol/v4-chain/protocol/lib"
"github.com/dydxprotocol/v4-chain/protocol/x/clob/types"
Expand All @@ -28,16 +26,17 @@ func (k Keeper) GetProcessProposerMatchesEvents(ctx sdk.Context) types.ProcessPr
// This function panics if:
// - the current block height does not match the block height of the ProcessProposerMatchesEvents
// - called outside of deliver TX mode
// - Any of the ProcessProposerMatchesEvents fields have duplicates.
//
// TODO(DEC-1281): add parameter validation.
func (k Keeper) MustSetProcessProposerMatchesEvents(
ctx sdk.Context,
processProposerMatchesEvents types.ProcessProposerMatchesEvents,
) {
lib.AssertDeliverTxMode(ctx)
if ctx.BlockHeight() != int64(processProposerMatchesEvents.BlockHeight) {
panic(fmt.Errorf("block height %d for ProcessProposerMatchesEvents does not equal current block height %d",
processProposerMatchesEvents.BlockHeight, ctx.BlockHeight()))

if err := processProposerMatchesEvents.ValidateProcessProposerMatchesEvents(ctx); err != nil {
panic(err)
}

// Retrieve an instance of the memory store.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func TestSetProcessProposerMatchesEvents(t *testing.T) {
types.ProcessProposerMatchesEvents{
PlacedLongTermOrderIds: []types.OrderId{
constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT15.OrderId,
constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT20.OrderId,
constants.LongTermOrder_Carl_Num0_Id0_Clob0_Sell1BTC_Price50000_GTBT10.OrderId,
constants.LongTermOrder_Bob_Num0_Id0_Clob0_Buy25_Price30_GTBT10.OrderId,
},
ExpiredStatefulOrderIds: []types.OrderId{
Expand All @@ -49,7 +49,7 @@ func TestSetProcessProposerMatchesEvents(t *testing.T) {
expectedProcessProposerMatchesEvents: types.ProcessProposerMatchesEvents{
PlacedLongTermOrderIds: []types.OrderId{
constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT15.OrderId,
constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT20.OrderId,
constants.LongTermOrder_Carl_Num0_Id0_Clob0_Sell1BTC_Price50000_GTBT10.OrderId,
constants.LongTermOrder_Bob_Num0_Id0_Clob0_Buy25_Price30_GTBT10.OrderId,
},
ExpiredStatefulOrderIds: []types.OrderId{
Expand Down Expand Up @@ -93,7 +93,7 @@ func TestSetProcessProposerMatchesEvents(t *testing.T) {
types.ProcessProposerMatchesEvents{
PlacedLongTermOrderIds: []types.OrderId{
constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT15.OrderId,
constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT20.OrderId,
constants.LongTermOrder_Carl_Num0_Id0_Clob0_Sell1BTC_Price50000_GTBT10.OrderId,
constants.LongTermOrder_Bob_Num0_Id0_Clob0_Buy25_Price30_GTBT10.OrderId,
},
ExpiredStatefulOrderIds: []types.OrderId{
Expand Down
27 changes: 12 additions & 15 deletions protocol/x/clob/memclob/memclob.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ func (m *MemClobPriceTimePriority) mustUpdateMemclobStateWithMatches(

// Update the total matched quantums for this matching loop stored in `subaccountTotalMatchedQuantums`.
for _, order := range []types.MatchableOrder{
matchedOrderHashToOrder[matchedMakerOrder.GetOrderHash()],
&matchedMakerOrder,
takerOrder,
} {
bigTotalMatchedQuantums, exists := subaccountTotalMatchedQuantums[order.GetSubaccountId()]
Expand Down Expand Up @@ -442,17 +442,15 @@ func (m *MemClobPriceTimePriority) PlaceOrder(

if m.generateOffchainUpdates {
// If this is a replacement order, then ensure we send the appropriate removal message.
if !order.IsLiquidation() {
orderId := order.OrderId
if _, found := m.openOrders.getOrder(ctx, orderId); found {
if message, success := off_chain_updates.CreateOrderRemoveMessageWithReason(
m.clobKeeper.Logger(ctx),
orderId,
indexershared.OrderRemovalReason_ORDER_REMOVAL_REASON_REPLACED,
off_chain_updates.OrderRemoveV1_ORDER_REMOVAL_STATUS_BEST_EFFORT_CANCELED,
); success {
offchainUpdates.AddRemoveMessage(orderId, message)
}
orderId := order.OrderId
if _, found := m.openOrders.getOrder(ctx, orderId); found {
if message, success := off_chain_updates.CreateOrderRemoveMessageWithReason(
m.clobKeeper.Logger(ctx),
orderId,
indexershared.OrderRemovalReason_ORDER_REMOVAL_REASON_REPLACED,
off_chain_updates.OrderRemoveV1_ORDER_REMOVAL_STATUS_BEST_EFFORT_CANCELED,
); success {
offchainUpdates.AddRemoveMessage(orderId, message)
}
}
if message, success := off_chain_updates.CreateOrderPlaceMessage(
Expand Down Expand Up @@ -735,7 +733,7 @@ func (m *MemClobPriceTimePriority) matchOrder(
) {
offchainUpdates = types.NewOffchainUpdates()

// // Branch the state. State will be wrote to only if matching does not return an error.
// Branch the state. State will be wrote to only if matching does not return an error.
branchedContext, writeCache := ctx.CacheContext()

// Attempt to match the order against the orderbook.
Expand Down Expand Up @@ -1917,8 +1915,7 @@ func (m *MemClobPriceTimePriority) mustRemoveOrder(
}

// mustUpdateOrderbookStateWithMatchedMakerOrder updates the orderbook with a matched maker order.
// If the maker order is fully filled, it removes it from the orderbook. Else, it updates the total number of quantums
// in the price level containing the maker order.
// If the maker order is fully filled, it removes it from the orderbook.
func (m *MemClobPriceTimePriority) mustUpdateOrderbookStateWithMatchedMakerOrder(
ctx sdk.Context,
makerOrder types.Order,
Expand Down
69 changes: 69 additions & 0 deletions protocol/x/clob/types/process_proposer_matches_events.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package types

import (
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/dydxprotocol/v4-chain/protocol/lib"
)

// ValidateProcessProposerMatchesEvents performs basic stateless validation on ProcessProposerMatchesEvents.
// It returns an error if:
// - Block height does not equal current block height.
// - Any of the fields have duplicate OrderIds. Note that this is currently invalid since
// stateful order replacements are not enabled.
func (ppme *ProcessProposerMatchesEvents) ValidateProcessProposerMatchesEvents(
ctx sdk.Context,
) error {
if ctx.BlockHeight() != int64(ppme.BlockHeight) {
return fmt.Errorf(
"block height %d for ProcessProposerMatchesEvents does not equal current block height %d",
ppme.BlockHeight,
ctx.BlockHeight(),
)
}

if lib.ContainsDuplicates(ppme.PlacedLongTermOrderIds) {
return fmt.Errorf(
"ProcessProposerMatchesEvents contains duplicate PlacedLongTermOrderIds: %+v",
ppme.PlacedLongTermOrderIds,
)
}
if lib.ContainsDuplicates(ppme.ExpiredStatefulOrderIds) {
return fmt.Errorf(
"ProcessProposerMatchesEvents contains duplicate ExpiredStatefulOrderIds: %+v",
ppme.ExpiredStatefulOrderIds,
)
}
if lib.ContainsDuplicates(ppme.OrderIdsFilledInLastBlock) {
return fmt.Errorf(
"ProcessProposerMatchesEvents contains duplicate OrderIdsFilledInLastBlock: %+v",
ppme.OrderIdsFilledInLastBlock,
)
}
if lib.ContainsDuplicates(ppme.PlacedStatefulCancellationOrderIds) {
return fmt.Errorf(
"ProcessProposerMatchesEvents contains duplicate PlacedStatefulCancellationOrderIds: %+v",
ppme.PlacedStatefulCancellationOrderIds,
)
}
if lib.ContainsDuplicates(ppme.RemovedStatefulOrderIds) {
return fmt.Errorf(
"ProcessProposerMatchesEvents contains duplicate RemovedStatefulOrderIds: %+v",
ppme.RemovedStatefulOrderIds,
)
}
if lib.ContainsDuplicates(ppme.ConditionalOrderIdsTriggeredInLastBlock) {
return fmt.Errorf(
"ProcessProposerMatchesEvents contains duplicate ConditionalOrderIdsTriggeredInLastBlock: %+v",
ppme.ConditionalOrderIdsTriggeredInLastBlock,
)
}
if lib.ContainsDuplicates(ppme.PlacedConditionalOrderIds) {
return fmt.Errorf(
"ProcessProposerMatchesEvents contains duplicate PlacedConditionalOrderIds: %+v",
ppme.PlacedConditionalOrderIds,
)
}
return nil
}

0 comments on commit da31d6e

Please sign in to comment.