Skip to content

Commit

Permalink
Remove unneeded memstores (#1631)
Browse files Browse the repository at this point in the history
  • Loading branch information
roy-dydx authored and tqin7 committed Jul 11, 2024
1 parent 6e9f27d commit 3784bbb
Show file tree
Hide file tree
Showing 9 changed files with 86 additions and 350 deletions.
8 changes: 4 additions & 4 deletions protocol/x/clob/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -807,11 +807,11 @@ func TestEndBlocker_Success(t *testing.T) {
for _, triggeredConditionalOrderId := range actualProcessProposerMatchesEvents.
ConditionalOrderIdsTriggeredInLastBlock {
// TODO(CLOB-746) Once R/W methods are created, substitute those methods here.
triggeredConditionalOrderMemstore := ks.ClobKeeper.GetTriggeredConditionalOrderPlacementMemStore(ctx)
untriggeredConditionalOrderMemstore := ks.ClobKeeper.GetUntriggeredConditionalOrderPlacementMemStore(ctx)
exists := triggeredConditionalOrderMemstore.Has(triggeredConditionalOrderId.ToStateKey())
triggeredConditionalOrderStore := ks.ClobKeeper.GetTriggeredConditionalOrderPlacementStore(ctx)
untriggeredConditionalOrderStore := ks.ClobKeeper.GetUntriggeredConditionalOrderPlacementStore(ctx)
exists := triggeredConditionalOrderStore.Has(triggeredConditionalOrderId.ToStateKey())
require.True(t, exists)
exists = untriggeredConditionalOrderMemstore.Has(triggeredConditionalOrderId.ToStateKey())
exists = untriggeredConditionalOrderStore.Has(triggeredConditionalOrderId.ToStateKey())
require.False(t, exists)
}

Expand Down
1 change: 0 additions & 1 deletion protocol/x/clob/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,6 @@ func (k Keeper) InitMemStore(ctx sdk.Context) {

// Initialize all the necessary memory stores.
for _, keyPrefix := range []string{
types.OrderAmountFilledKeyPrefix,
types.StatefulOrderKeyPrefix,
} {
// Retrieve an instance of the memstore.
Expand Down
34 changes: 7 additions & 27 deletions protocol/x/clob/keeper/order_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,21 +81,9 @@ func (k Keeper) SetOrderFillAmount(
orderId.ToStateKey(),
orderFillStateBytes,
)

// Retrieve an instance of the memStore.
memStore := prefix.NewStore(
ctx.KVStore(k.memKey),
[]byte(types.OrderAmountFilledKeyPrefix),
)

// Write `orderFillStateBytes` to memStore.
memStore.Set(
orderId.ToStateKey(),
orderFillStateBytes,
)
}

// GetOrderFillAmount returns the total `fillAmount` and `prunableBlockHeight` from the memStore.
// GetOrderFillAmount returns the total `fillAmount` and `prunableBlockHeight` from state.
func (k Keeper) GetOrderFillAmount(
ctx sdk.Context,
orderId types.OrderId,
Expand All @@ -104,16 +92,15 @@ func (k Keeper) GetOrderFillAmount(
fillAmount satypes.BaseQuantums,
prunableBlockHeight uint32,
) {
memStore := ctx.KVStore(k.memKey)
store := ctx.KVStore(k.storeKey)

// Retrieve an instance of the memStore.
memPrefixStore := prefix.NewStore(
memStore,
prefixStore := prefix.NewStore(
store,
[]byte(types.OrderAmountFilledKeyPrefix),
)

// Retrieve the `OrderFillState` bytes from the store.
orderFillStateBytes := memPrefixStore.Get(
orderFillStateBytes := prefixStore.Get(
orderId.ToStateKey(),
)

Expand Down Expand Up @@ -259,8 +246,8 @@ func (k Keeper) MigratePruneableOrders(ctx sdk.Context) {
}
}

// RemoveOrderFillAmount removes the fill amount of an Order from state and the memstore.
// This function is a no-op if no order fill amount exists in state and the mem store with `orderId`.
// RemoveOrderFillAmount removes the fill amount of an Order from state.
// This function is a no-op if no order fill amount exists in state with `orderId`.
func (k Keeper) RemoveOrderFillAmount(ctx sdk.Context, orderId types.OrderId) {
// Delete the fill amount from the state store.
orderAmountFilledStore := prefix.NewStore(
Expand All @@ -270,13 +257,6 @@ func (k Keeper) RemoveOrderFillAmount(ctx sdk.Context, orderId types.OrderId) {

orderAmountFilledStore.Delete(orderId.ToStateKey())

// Delete the fill amount from the mem store.
memStore := prefix.NewStore(
ctx.KVStore(k.memKey),
[]byte(types.OrderAmountFilledKeyPrefix),
)
memStore.Delete(orderId.ToStateKey())

// If grpc stream is on, zero out the fill amount.
if k.GetGrpcStreamingManager().Enabled() {
allUpdates := types.NewOffchainUpdates()
Expand Down
24 changes: 0 additions & 24 deletions protocol/x/clob/keeper/order_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -582,10 +582,6 @@ func TestRemoveOrderFillAmount(t *testing.T) {
string(constants.Order_Alice_Num1_Clob0_Id4_Buy10_Price45_GTB20.OrderId.ToStateKey()),
types.OrderAmountFilledKeyPrefix +
string(constants.Order_Alice_Num1_Clob0_Id4_Buy10_Price45_GTB20.OrderId.ToStateKey()),
types.OrderAmountFilledKeyPrefix +
string(constants.Order_Alice_Num1_Clob0_Id4_Buy10_Price45_GTB20.OrderId.ToStateKey()),
types.OrderAmountFilledKeyPrefix +
string(constants.Order_Alice_Num1_Clob0_Id4_Buy10_Price45_GTB20.OrderId.ToStateKey()),
},
},
"SetOrderFillAmount twice and then RemoveOrderFillAmount removes the fill amount": {
Expand Down Expand Up @@ -617,12 +613,6 @@ func TestRemoveOrderFillAmount(t *testing.T) {
string(constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB15.OrderId.ToStateKey()),
types.OrderAmountFilledKeyPrefix +
string(constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB15.OrderId.ToStateKey()),
types.OrderAmountFilledKeyPrefix +
string(constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB15.OrderId.ToStateKey()),
types.OrderAmountFilledKeyPrefix +
string(constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB15.OrderId.ToStateKey()),
types.OrderAmountFilledKeyPrefix +
string(constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB15.OrderId.ToStateKey()),
},
},
"RemoveOrderFillAmount with non-existent order": {
Expand All @@ -634,8 +624,6 @@ func TestRemoveOrderFillAmount(t *testing.T) {
expectedMultiStoreWrites: []string{
types.OrderAmountFilledKeyPrefix +
string(constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB15.OrderId.ToStateKey()),
types.OrderAmountFilledKeyPrefix +
string(constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB15.OrderId.ToStateKey()),
},
},
"SetOrderFillAmount, RemoveOrderFillAmount, SetOrderFillAmount re-creates the fill amount": {
Expand Down Expand Up @@ -668,12 +656,6 @@ func TestRemoveOrderFillAmount(t *testing.T) {
string(constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB15.OrderId.ToStateKey()),
types.OrderAmountFilledKeyPrefix +
string(constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB15.OrderId.ToStateKey()),
types.OrderAmountFilledKeyPrefix +
string(constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB15.OrderId.ToStateKey()),
types.OrderAmountFilledKeyPrefix +
string(constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB15.OrderId.ToStateKey()),
types.OrderAmountFilledKeyPrefix +
string(constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB15.OrderId.ToStateKey()),
},
},
"RemoveOrderFillAmount does not delete fill amounts for other orders": {
Expand Down Expand Up @@ -704,12 +686,6 @@ func TestRemoveOrderFillAmount(t *testing.T) {
expectedMultiStoreWrites: []string{
types.OrderAmountFilledKeyPrefix +
string(constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB15.OrderId.ToStateKey()),
types.OrderAmountFilledKeyPrefix +
string(constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB15.OrderId.ToStateKey()),
types.OrderAmountFilledKeyPrefix +
string(constants.Order_Bob_Num0_Id0_Clob1_Sell10_Price15_GTB20.OrderId.ToStateKey()),
types.OrderAmountFilledKeyPrefix +
string(constants.Order_Bob_Num0_Id0_Clob1_Sell10_Price15_GTB20.OrderId.ToStateKey()),
types.OrderAmountFilledKeyPrefix +
string(constants.Order_Bob_Num0_Id0_Clob1_Sell10_Price15_GTB20.OrderId.ToStateKey()),
types.OrderAmountFilledKeyPrefix +
Expand Down
20 changes: 2 additions & 18 deletions protocol/x/clob/keeper/orders_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,14 +144,10 @@ func TestPlaceShortTermOrder(t *testing.T) {
types.PrunableOrdersKeyPrefix,
// Update taker order fill amount
types.OrderAmountFilledKeyPrefix,
// Update taker order fill amount in memStore
types.OrderAmountFilledKeyPrefix,
// Update prunable block height for maker fill amount
types.PrunableOrdersKeyPrefix,
// Update maker order fill amount
types.OrderAmountFilledKeyPrefix,
// Update maker order fill amount in memStore
types.OrderAmountFilledKeyPrefix,
},
expectedOpenInterests: map[uint32]*big.Int{
// positions fully closed
Expand Down Expand Up @@ -396,14 +392,10 @@ func TestPlaceShortTermOrder(t *testing.T) {
types.PrunableOrdersKeyPrefix,
// Update taker order fill amount
types.OrderAmountFilledKeyPrefix,
// Update taker order fill amount in memStore
types.OrderAmountFilledKeyPrefix,
// Update prunable block height for maker fill amount
types.PrunableOrdersKeyPrefix,
// Update maker order fill amount
types.OrderAmountFilledKeyPrefix,
// Update maker order fill amount in memStore
types.OrderAmountFilledKeyPrefix,
},
expectedOpenInterests: map[uint32]*big.Int{
// 1 BTC + 0.01 BTC + 0.01 BTC filled
Expand Down Expand Up @@ -754,14 +746,10 @@ func TestAddPreexistingStatefulOrder(t *testing.T) {
indexer_manager.IndexerEventsCountKey,
// Update block stats
statstypes.BlockStatsKey,
// Update taker order fill amount to state and memStore.
// Update taker order fill amount to state.
types.OrderAmountFilledKeyPrefix +
string(constants.LongTermOrder_Carl_Num0_Id0_Clob0_Buy1BTC_Price50000_GTBT10.OrderId.ToStateKey()),
types.OrderAmountFilledKeyPrefix +
string(constants.LongTermOrder_Carl_Num0_Id0_Clob0_Buy1BTC_Price50000_GTBT10.OrderId.ToStateKey()),
// Update maker order fill amount to state and memStore.
types.OrderAmountFilledKeyPrefix +
string(constants.LongTermOrder_Dave_Num0_Id0_Clob0_Sell1BTC_Price50000_GTBT10.OrderId.ToStateKey()),
// Update maker order fill amount to state.
types.OrderAmountFilledKeyPrefix +
string(constants.LongTermOrder_Dave_Num0_Id0_Clob0_Sell1BTC_Price50000_GTBT10.OrderId.ToStateKey()),
},
Expand Down Expand Up @@ -2434,11 +2422,9 @@ func TestPlaceConditionalOrdersTriggeredInLastBlock(t *testing.T) {
longTermOrderPlacementBytes := ks.Cdc.MustMarshal(&longTermOrderPlacement)

store := ks.ClobKeeper.GetTriggeredConditionalOrderPlacementStore(ctx)
memstore := ks.ClobKeeper.GetTriggeredConditionalOrderPlacementMemStore(ctx)

orderKey := order.OrderId.ToStateKey()
store.Set(orderKey, longTermOrderPlacementBytes)
memstore.Set(orderKey, longTermOrderPlacementBytes)
}

// Write to untriggered orders state
Expand All @@ -2449,11 +2435,9 @@ func TestPlaceConditionalOrdersTriggeredInLastBlock(t *testing.T) {
longTermOrderPlacementBytes := ks.Cdc.MustMarshal(&longTermOrderPlacement)

store := ks.ClobKeeper.GetUntriggeredConditionalOrderPlacementStore(ctx)
memstore := ks.ClobKeeper.GetUntriggeredConditionalOrderPlacementMemStore(ctx)

orderKey := order.OrderId.ToStateKey()
store.Set(orderKey, longTermOrderPlacementBytes)
memstore.Set(orderKey, longTermOrderPlacementBytes)
}

// Assert expected order placement memclob calls.
Expand Down
50 changes: 19 additions & 31 deletions protocol/x/clob/keeper/stateful_order_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,12 @@ func (k Keeper) SetLongTermOrderPlacement(
longTermOrderPlacementBytes := k.cdc.MustMarshal(&longTermOrderPlacement)

// For setting long term order placements, always set conditional orders to the untriggered state store.
store, memStore := k.fetchStateStoresForOrder(ctx, order.OrderId)
store := k.fetchStateStoresForOrder(ctx, order.OrderId)
orderKey := order.OrderId.ToStateKey()

// Write the `LongTermOrderPlacement` to state.
store.Set(orderKey, longTermOrderPlacementBytes)

// Write the `LongTermOrderPlacement` to memstore.
memStore.Set(orderKey, longTermOrderPlacementBytes)

if !found {
// Increment the stateful order count.
k.SetStatefulOrderCount(
Expand All @@ -83,15 +80,15 @@ func (k Keeper) SetLongTermOrderPlacement(
}
}

// GetTriggeredConditionalOrderPlacement gets an triggered conditional order placement from the memstore.
// Returns false if no triggered conditional order exists in memstore with `orderId`.
// GetTriggeredConditionalOrderPlacement gets an triggered conditional order placement from the store.
// Returns false if no triggered conditional order exists in store with `orderId`.
func (k Keeper) GetTriggeredConditionalOrderPlacement(
ctx sdk.Context,
orderId types.OrderId,
) (val types.LongTermOrderPlacement, found bool) {
memStore := k.GetTriggeredConditionalOrderPlacementMemStore(ctx)
store := k.GetTriggeredConditionalOrderPlacementStore(ctx)

b := memStore.Get(orderId.ToStateKey())
b := store.Get(orderId.ToStateKey())
if b == nil {
return val, false
}
Expand All @@ -100,15 +97,15 @@ func (k Keeper) GetTriggeredConditionalOrderPlacement(
return val, true
}

// GetUntriggeredConditionalOrderPlacement gets an untriggered conditional order placement from the memstore.
// Returns false if no untriggered conditional order exists in memstore with `orderId`.
// GetUntriggeredConditionalOrderPlacement gets an untriggered conditional order placement from the store.
// Returns false if no untriggered conditional order exists in store with `orderId`.
func (k Keeper) GetUntriggeredConditionalOrderPlacement(
ctx sdk.Context,
orderId types.OrderId,
) (val types.LongTermOrderPlacement, found bool) {
memStore := k.GetUntriggeredConditionalOrderPlacementMemStore(ctx)
store := k.GetUntriggeredConditionalOrderPlacementStore(ctx)

b := memStore.Get(orderId.ToStateKey())
b := store.Get(orderId.ToStateKey())
if b == nil {
return val, false
}
Expand All @@ -127,9 +124,9 @@ func (k Keeper) GetLongTermOrderPlacement(
// If this is a Short-Term order, panic.
orderId.MustBeStatefulOrder()

_, memStore := k.fetchStateStoresForOrder(ctx, orderId)
store := k.fetchStateStoresForOrder(ctx, orderId)

b := memStore.Get(orderId.ToStateKey())
b := store.Get(orderId.ToStateKey())
if b == nil {
return val, false
}
Expand All @@ -148,15 +145,13 @@ func (k Keeper) DeleteLongTermOrderPlacement(
// If this is a Short-Term order, panic.
orderId.MustBeStatefulOrder()

store, memStore := k.fetchStateStoresForOrder(ctx, orderId)
store := k.fetchStateStoresForOrder(ctx, orderId)

// Note that since store reads/writes can cost gas we need to ensure that the number of operations is the
// same regardless of whether the memstore has the order or not.
count := k.GetStatefulOrderCount(ctx, orderId.SubaccountId)
orderKey := orderId.ToStateKey()
if memStore.Has(orderKey) {
if store.Has(orderKey) {
if count == 0 {
log.ErrorLog(ctx, "Stateful order count is zero but order is in the memstore. Underflow",
log.ErrorLog(ctx, "Stateful order count is zero but order is in the store. Underflow",
"orderId", cometbftlog.NewLazySprintf("%+v", orderId),
)
} else {
Expand All @@ -167,9 +162,6 @@ func (k Keeper) DeleteLongTermOrderPlacement(
// Delete the `StatefulOrderPlacement` from state.
store.Delete(orderKey)

// Delete the `StatefulOrderPlacement` from memstore.
memStore.Delete(orderKey)

// Set the count.
k.SetStatefulOrderCount(ctx, orderId.SubaccountId, count)

Expand Down Expand Up @@ -239,10 +231,9 @@ func (k Keeper) MustTriggerConditionalOrder(

blockHeight := lib.MustConvertIntegerToUint32(ctx.BlockHeight())

untriggeredConditionalOrderMemStore := k.GetUntriggeredConditionalOrderPlacementMemStore(ctx)
untriggeredConditionalOrderStore := k.GetUntriggeredConditionalOrderPlacementStore(ctx)

bytes := untriggeredConditionalOrderMemStore.Get(orderId.ToStateKey())
bytes := untriggeredConditionalOrderStore.Get(orderId.ToStateKey())
if bytes == nil {
panic(
fmt.Sprintf(
Expand All @@ -262,17 +253,14 @@ func (k Keeper) MustTriggerConditionalOrder(
TransactionIndex: nextStatefulOrderTransactionIndex,
}

// Write the StatefulOrderPlacement to the Triggered state store/memstore.
// Write the StatefulOrderPlacement to the Triggered state store.
longTermOrderPlacementBytes := k.cdc.MustMarshal(&longTermOrderPlacement)
triggeredConditionalOrderMemStore := k.GetTriggeredConditionalOrderPlacementMemStore(ctx)
triggeredConditionalOrderStore := k.GetTriggeredConditionalOrderPlacementStore(ctx)
orderKey := orderId.ToStateKey()
triggeredConditionalOrderStore.Set(orderKey, longTermOrderPlacementBytes)
triggeredConditionalOrderMemStore.Set(orderKey, longTermOrderPlacementBytes)

// Delete the `StatefulOrderPlacement` from Untriggered state store/memstore.
// Delete the `StatefulOrderPlacement` from Untriggered state store.
untriggeredConditionalOrderStore.Delete(orderKey)
untriggeredConditionalOrderMemStore.Delete(orderKey)
}

// MustAddOrderToStatefulOrdersTimeSlice adds a new `OrderId` to an existing time slice, or creates a new time slice
Expand Down Expand Up @@ -367,9 +355,9 @@ func (k Keeper) IsConditionalOrderTriggered(
) (triggered bool) {
// If this is not a conditional order, panic.
orderId.MustBeConditionalOrder()
triggeredMemstore := k.GetTriggeredConditionalOrderPlacementMemStore(ctx)
store := k.GetTriggeredConditionalOrderPlacementStore(ctx)
orderKey := orderId.ToStateKey()
return triggeredMemstore.Has(orderKey)
return store.Has(orderKey)
}

// RemoveExpiredStatefulOrdersTimeSlices iterates all time slices from 0 until the time specified by
Expand Down
Loading

0 comments on commit 3784bbb

Please sign in to comment.