Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: optimize CancelAllOrders gas usage, fix offer coin checking #296 #299 #29

Merged
merged 4 commits into from
May 3, 2022
Merged

fix: optimize CancelAllOrders gas usage, fix offer coin checking #296 #299 #29

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b5ef143
fix: optimize CancelAllOrders gas usage
hallazzang Apr 19, 2022
64f9a96
fix: cache pairs when iterating pair ids from msg
hallazzang Apr 22, 2022
4777e90
fix: check if the orderer has sufficient balance to make an order
hallazzang Apr 19, 2022
fb5c13d
fix: fix offer_coin event attribute
hallazzang Apr 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 39 additions & 30 deletions x/liquidity/keeper/swap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@ import (
// ValidateMsgLimitOrder validates types.MsgLimitOrder with state and returns
// calculated offer coin and price that is fit into ticks.
func (k Keeper) ValidateMsgLimitOrder(ctx sdk.Context, msg *types.MsgLimitOrder) (offerCoin sdk.Coin, price sdk.Dec, err error) {
spendable := k.bankKeeper.SpendableCoins(ctx, msg.GetOrderer())
if spendableAmt := spendable.AmountOf(msg.OfferCoin.Denom); spendableAmt.LT(msg.OfferCoin.Amount) {
return sdk.Coin{}, sdk.Dec{}, sdkerrors.Wrapf(
sdkerrors.ErrInsufficientFunds, "%s is smaller than %s",
sdk.NewCoin(msg.OfferCoin.Denom, spendableAmt), msg.OfferCoin)
}

params := k.GetParams(ctx)

if msg.OrderLifespan > params.MaxOrderLifespan {
Expand Down Expand Up @@ -122,6 +129,13 @@ func (k Keeper) LimitOrder(ctx sdk.Context, msg *types.MsgLimitOrder) (types.Ord
// ValidateMsgMarketOrder validates types.MsgMarketOrder with state and returns
// calculated offer coin and price.
func (k Keeper) ValidateMsgMarketOrder(ctx sdk.Context, msg *types.MsgMarketOrder) (offerCoin sdk.Coin, price sdk.Dec, err error) {
spendable := k.bankKeeper.SpendableCoins(ctx, msg.GetOrderer())
if spendableAmt := spendable.AmountOf(msg.OfferCoin.Denom); spendableAmt.LT(msg.OfferCoin.Amount) {
return sdk.Coin{}, sdk.Dec{}, sdkerrors.Wrapf(
sdkerrors.ErrInsufficientFunds, "%s is smaller than %s",
sdk.NewCoin(msg.OfferCoin.Denom, spendableAmt), msg.OfferCoin)
}

params := k.GetParams(ctx)

if msg.OrderLifespan > params.MaxOrderLifespan {
Expand Down Expand Up @@ -200,7 +214,7 @@ func (k Keeper) MarketOrder(ctx sdk.Context, msg *types.MsgMarketOrder) (types.O
sdk.NewAttribute(types.AttributeKeyOrderer, msg.Orderer),
sdk.NewAttribute(types.AttributeKeyPairId, strconv.FormatUint(msg.PairId, 10)),
sdk.NewAttribute(types.AttributeKeyOrderDirection, msg.Direction.String()),
sdk.NewAttribute(types.AttributeKeyOfferCoin, msg.OfferCoin.String()),
sdk.NewAttribute(types.AttributeKeyOfferCoin, offerCoin.String()),
sdk.NewAttribute(types.AttributeKeyDemandCoinDenom, msg.DemandCoinDenom),
sdk.NewAttribute(types.AttributeKeyPrice, price.String()),
sdk.NewAttribute(types.AttributeKeyAmount, msg.Amount.String()),
Expand Down Expand Up @@ -260,43 +274,38 @@ func (k Keeper) CancelOrder(ctx sdk.Context, msg *types.MsgCancelOrder) error {

// CancelAllOrders handles types.MsgCancelAllOrders and cancels all orders.
func (k Keeper) CancelAllOrders(ctx sdk.Context, msg *types.MsgCancelAllOrders) error {
var canceledOrderIds []string
cb := func(pair types.Pair, order types.Order) (stop bool, err error) {
if order.Orderer == msg.Orderer && order.Status != types.OrderStatusCanceled && order.BatchId < pair.CurrentBatchId {
if err := k.FinishOrder(ctx, order, types.OrderStatusCanceled); err != nil {
return false, err
}
canceledOrderIds = append(canceledOrderIds, strconv.FormatUint(order.Id, 10))
orderPairCache := map[uint64]types.Pair{} // maps order's pair id to pair, to cache the result
pairIdSet := map[uint64]struct{}{} // set of pairs where to cancel orders
var pairIds []string // needed to emit an event
for _, pairId := range msg.PairIds {
pair, found := k.GetPair(ctx, pairId)
if !found { // check if the pair exists
return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "pair %d not found", pairId)
}
return false, nil
pairIdSet[pairId] = struct{}{} // add pair id to the set
pairIds = append(pairIds, strconv.FormatUint(pairId, 10))
orderPairCache[pairId] = pair // also cache the pair to use at below
}

var pairIds []string
if len(msg.PairIds) == 0 {
pairMap := map[uint64]types.Pair{}
if err := k.IterateAllOrders(ctx, func(order types.Order) (stop bool, err error) {
pair, ok := pairMap[order.PairId]
var canceledOrderIds []string
if err := k.IterateOrdersByOrderer(ctx, msg.GetOrderer(), func(order types.Order) (stop bool, err error) {
_, ok := pairIdSet[order.PairId] // is the pair included in the pair set?
if len(pairIdSet) == 0 || ok { // pair ids not specified(cancel all), or the pair is in the set
pair, ok := orderPairCache[order.PairId]
if !ok {
pair, _ = k.GetPair(ctx, order.PairId)
pairMap[order.PairId] = pair
orderPairCache[order.PairId] = pair
}
return cb(pair, order)
}); err != nil {
return err
}
} else {
for _, pairId := range msg.PairIds {
pairIds = append(pairIds, strconv.FormatUint(pairId, 10))
pair, found := k.GetPair(ctx, pairId)
if !found {
return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "pair %d not found", pairId)
}
if err := k.IterateOrdersByPair(ctx, pairId, func(req types.Order) (stop bool, err error) {
return cb(pair, req)
}); err != nil {
return err
if order.Status != types.OrderStatusCanceled && order.BatchId < pair.CurrentBatchId {
if err := k.FinishOrder(ctx, order, types.OrderStatusCanceled); err != nil {
return false, err
}
canceledOrderIds = append(canceledOrderIds, strconv.FormatUint(order.Id, 10))
}
}
return false, nil
}); err != nil {
return err
}

ctx.EventManager().EmitEvents(sdk.Events{
Expand Down
60 changes: 60 additions & 0 deletions x/liquidity/keeper/swap_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"time"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"

utils "github.com/crescent-network/crescent/types"
"github.com/crescent-network/crescent/x/liquidity"
Expand Down Expand Up @@ -113,6 +114,23 @@ func (s *KeeperTestSuite) TestLimitOrder() {
}
}

func (s *KeeperTestSuite) TestLimitOrderInsufficientOfferCoin() {
pair := s.createPair(s.addr(0), "denom1", "denom2", true)

orderer := s.addr(1)
s.fundAddr(orderer, utils.ParseCoins("1000000denom2"))
_, err := s.keeper.LimitOrder(s.ctx, types.NewMsgLimitOrder(
orderer, pair.Id, types.OrderDirectionBuy, utils.ParseCoin("1000001denom2"), "denom1",
utils.ParseDec("1.0"), sdk.NewInt(1000000), 0))
s.Require().ErrorIs(err, sdkerrors.ErrInsufficientFunds)

s.fundAddr(orderer, utils.ParseCoins("1000000denom1"))
_, err = s.keeper.LimitOrder(s.ctx, types.NewMsgLimitOrder(
orderer, pair.Id, types.OrderDirectionSell, utils.ParseCoin("1000001denom1"), "denom2",
utils.ParseDec("1.0"), sdk.NewInt(1000000), 0))
s.Require().ErrorIs(err, sdkerrors.ErrInsufficientFunds)
}

func (s *KeeperTestSuite) TestLimitOrderRefund() {
pair := s.createPair(s.addr(0), "denom1", "denom2", true)
orderer := s.addr(1)
Expand Down Expand Up @@ -196,6 +214,23 @@ func (s *KeeperTestSuite) TestMarketOrder() {
s.Require().True(coinsEq(utils.ParseCoins("10800denom2"), s.getBalances(s.addr(4))))
}

func (s *KeeperTestSuite) TestMarketOrderInsufficientOfferCoin() {
pair := s.createPair(s.addr(0), "denom1", "denom2", true)

orderer := s.addr(1)
s.fundAddr(orderer, utils.ParseCoins("1000000denom2"))
_, err := s.keeper.MarketOrder(s.ctx, types.NewMsgMarketOrder(
orderer, pair.Id, types.OrderDirectionBuy, utils.ParseCoin("1000001denom2"), "denom1",
sdk.NewInt(1000000), 0))
s.Require().ErrorIs(err, sdkerrors.ErrInsufficientFunds)

s.fundAddr(orderer, utils.ParseCoins("1000000denom1"))
_, err = s.keeper.MarketOrder(s.ctx, types.NewMsgMarketOrder(
orderer, pair.Id, types.OrderDirectionSell, utils.ParseCoin("1000001denom1"), "denom2",
sdk.NewInt(1000000), 0))
s.Require().ErrorIs(err, sdkerrors.ErrInsufficientFunds)
}

func (s *KeeperTestSuite) TestMarketOrderRefund() {
pair := s.createPair(s.addr(0), "denom1", "denom2", true)
p := utils.ParseDec("1.0")
Expand Down Expand Up @@ -399,6 +434,31 @@ func (s *KeeperTestSuite) TestCancelAllOrders() {
s.Require().True(coinsEq(utils.ParseCoins("10000denom2,10000denom1"), s.getBalances(s.addr(2))))
}

func (s *KeeperTestSuite) TestCancelAllOrdersGasUsage() {
// Ensure that the number of other orders in pairs doesn't affect
// the msg's gas usage.

pair := s.createPair(s.addr(0), "denom1", "denom2", true)

// 1000 other users make orders.
for i := 1; i <= 1000; i++ {
s.buyLimitOrder(s.addr(i), pair.Id, utils.ParseDec("0.9"), sdk.NewInt(10000), time.Minute, true)
s.sellLimitOrder(s.addr(i), pair.Id, utils.ParseDec("1.1"), sdk.NewInt(10000), time.Minute, true)
}

// The orderer makes an order.
orderer := s.addr(1001)
s.sellLimitOrder(orderer, pair.Id, utils.ParseDec("1.1"), sdk.NewInt(10000), time.Minute, true)

// New batch begins, now the orderer can cancel his/her order.
liquidity.EndBlocker(s.ctx, s.keeper)
liquidity.BeginBlocker(s.ctx, s.keeper)

s.ctx = s.ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) // to record gas consumption
s.cancelAllOrders(orderer, nil) // cancel all orders in all pairs
s.Require().Less(s.ctx.GasMeter().GasConsumed(), sdk.Gas(50000))
}

func (s *KeeperTestSuite) TestDustCollector() {
pair := s.createPair(s.addr(0), "denom1", "denom2", true)

Expand Down