Merge pull request #186 from crescent-network/fix/maker-fee-rate

fix: constrain maker fee rate to non-negative value

app/testutil/amm.go

@@ -12,7 +12,7 @@ import (
 func (s *TestSuite) CreatePool(marketId uint64, price sdk.Dec) ammtypes.Pool {
 	s.T().Helper()
 	creatorAddr := utils.TestAddress(1000001)
-	creationFee := s.App.ExchangeKeeper.GetFees(s.Ctx).MarketCreationFee
+	creationFee := s.App.AMMKeeper.GetPoolCreationFee(s.Ctx)
 	if !s.GetAllBalances(creatorAddr).IsAllGTE(creationFee) {
 		s.FundAccount(creatorAddr, creationFee)
 	}

app/testutil/exchange.go

@@ -12,7 +12,7 @@ import (
 func (s *TestSuite) CreateMarket(baseDenom, quoteDenom string) exchangetypes.Market {
 	s.T().Helper()
 	creatorAddr := utils.TestAddress(1000000)
-	creationFee := s.App.ExchangeKeeper.GetFees(s.Ctx).MarketCreationFee
+	creationFee := s.App.ExchangeKeeper.GetMarketCreationFee(s.Ctx)
 	if !s.GetAllBalances(creatorAddr).IsAllGTE(creationFee) {
 		s.FundAccount(creatorAddr, creationFee)
 	}

client/docs/swagger-ui/swagger.yaml

@@ -12106,27 +12106,26 @@ paths:
               params:
                 type: object
                 properties:
-                  fees:
-                    type: object
-                    properties:
-                      market_creation_fee:
-                        type: array
-                        items:
-                          type: object
-                          properties:
-                            denom:
-                              type: string
-                            amount:
-                              type: string
-                          description: >-
-                            Coin defines a token with a denomination and an
-                            amount.
+                  market_creation_fee:
+                    type: array
+                    items:
+                      type: object
+                      properties:
+                        denom:
+                          type: string
+                        amount:
+                          type: string
+                      description: >-
+                        Coin defines a token with a denomination and an amount.
 
 
-                            NOTE: The amount field is an Int which implements
-                            the custom method
+                        NOTE: The amount field is an Int which implements the
+                        custom method
 
-                            signatures required by gogoproto.
+                        signatures required by gogoproto.
+                  fees:
+                    type: object
+                    properties:
                       default_maker_fee_rate:
                         type: string
                       default_taker_fee_rate:
@@ -20078,20 +20077,6 @@ definitions:
   crescent.exchange.v1beta1.Fees:
     type: object
     properties:
-      market_creation_fee:
-        type: array
-        items:
-          type: object
-          properties:
-            denom:
-              type: string
-            amount:
-              type: string
-          description: |-
-            Coin defines a token with a denomination and an amount.
-
-            NOTE: The amount field is an Int which implements the custom method
-            signatures required by gogoproto.
       default_maker_fee_rate:
         type: string
       default_taker_fee_rate:
@@ -20202,26 +20187,23 @@ definitions:
   crescent.exchange.v1beta1.Params:
     type: object
     properties:
+      market_creation_fee:
+        type: array
+        items:
+          type: object
+          properties:
+            denom:
+              type: string
+            amount:
+              type: string
+          description: |-
+            Coin defines a token with a denomination and an amount.
+
+            NOTE: The amount field is an Int which implements the custom method
+            signatures required by gogoproto.
       fees:
         type: object
         properties:
-          market_creation_fee:
-            type: array
-            items:
-              type: object
-              properties:
-                denom:
-                  type: string
-                amount:
-                  type: string
-              description: >-
-                Coin defines a token with a denomination and an amount.
-
-
-                NOTE: The amount field is an Int which implements the custom
-                method
-
-                signatures required by gogoproto.
           default_maker_fee_rate:
             type: string
           default_taker_fee_rate:
@@ -20537,26 +20519,26 @@ definitions:
       params:
         type: object
         properties:
-          fees:
-            type: object
-            properties:
-              market_creation_fee:
-                type: array
-                items:
-                  type: object
-                  properties:
-                    denom:
-                      type: string
-                    amount:
-                      type: string
-                  description: >-
-                    Coin defines a token with a denomination and an amount.
+          market_creation_fee:
+            type: array
+            items:
+              type: object
+              properties:
+                denom:
+                  type: string
+                amount:
+                  type: string
+              description: >-
+                Coin defines a token with a denomination and an amount.
 
 
-                    NOTE: The amount field is an Int which implements the custom
-                    method
+                NOTE: The amount field is an Int which implements the custom
+                method
 
-                    signatures required by gogoproto.
+                signatures required by gogoproto.
+          fees:
+            type: object
+            properties:
               default_maker_fee_rate:
                 type: string
               default_taker_fee_rate:

proto/crescent/exchange/v1beta1/params.proto

@@ -10,22 +10,22 @@ option go_package                      = "github.com/crescent-network/crescent/v
 option (gogoproto.goproto_getters_all) = false;
 
 message Params {
-  Fees                     fees               = 1 [(gogoproto.nullable) = false];
-  google.protobuf.Duration max_order_lifespan = 2 [(gogoproto.stdduration) = true, (gogoproto.nullable) = false];
+  repeated cosmos.base.v1beta1.Coin market_creation_fee = 1
+      [(gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins", (gogoproto.nullable) = false];
+  Fees                     fees               = 2 [(gogoproto.nullable) = false];
+  google.protobuf.Duration max_order_lifespan = 3 [(gogoproto.stdduration) = true, (gogoproto.nullable) = false];
   // max_order_price_ratio defines the ratio of the maximum possible order price compared to the market's last price
-  string max_order_price_ratio = 3
+  string max_order_price_ratio = 4
       [(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec", (gogoproto.nullable) = false];
-  uint32 max_swap_routes_len = 4;
-  uint32 max_num_mm_orders   = 5 [(gogoproto.customname) = "MaxNumMMOrders"];
+  uint32 max_swap_routes_len = 5;
+  uint32 max_num_mm_orders   = 6 [(gogoproto.customname) = "MaxNumMMOrders"];
 }
 
 message Fees {
-  repeated cosmos.base.v1beta1.Coin market_creation_fee = 1
-      [(gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins", (gogoproto.nullable) = false];
-  string default_maker_fee_rate = 2
+  string default_maker_fee_rate = 1
       [(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec", (gogoproto.nullable) = false];
-  string default_taker_fee_rate = 3
+  string default_taker_fee_rate = 2
       [(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec", (gogoproto.nullable) = false];
-  string default_order_source_fee_ratio = 4
+  string default_order_source_fee_ratio = 3
       [(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec", (gogoproto.nullable) = false];
 }

x/amm/keeper/grpc_query_test.go

@@ -103,8 +103,8 @@ func (s *KeeperTestSuite) TestQueryAllPools() {
 				pool := resp.Pools[0]
 				s.Require().EqualValues(1, pool.MarketId)
 				s.Require().EqualValues(2, pool.Id)
-				s.AssertEqual(utils.ParseCoin("153579683ucre"), pool.Balance0)
-				s.AssertEqual(utils.ParseCoin("257373895uusd"), pool.Balance1)
+				s.AssertEqual(utils.ParseCoin("153579684ucre"), pool.Balance0)
+				s.AssertEqual(utils.ParseCoin("257373896uusd"), pool.Balance1)
 			},
 		},
 		{
@@ -486,7 +486,7 @@ func (s *KeeperTestSuite) TestQueryCollectibleCoins() {
 			},
 			"",
 			func(resp *types.QueryCollectibleCoinsResponse) {
-				s.AssertEqual(utils.ParseCoins("26706uatom,72663ucre,1094754uusd"), resp.Fee)
+				s.AssertEqual(utils.ParseCoins("24181uatom,62704ucre,945127uusd"), resp.Fee)
 				s.AssertEqual(utils.ParseCoins("8578uatom,8467ucre"), resp.FarmingRewards)
 			},
 		},
@@ -497,7 +497,7 @@ func (s *KeeperTestSuite) TestQueryCollectibleCoins() {
 			},
 			"",
 			func(resp *types.QueryCollectibleCoinsResponse) {
-				s.AssertEqual(utils.ParseCoins("72663ucre,440713uusd"), resp.Fee)
+				s.AssertEqual(utils.ParseCoins("62704ucre,366086uusd"), resp.Fee)
 				s.AssertEqual(utils.ParseCoins("8467ucre"), resp.FarmingRewards)
 			},
 		},

x/amm/keeper/invariants.go

@@ -112,13 +112,13 @@ func CanCollectInvariant(k Keeper) sdk.Invariant {
 			fee, farmingRewards, err := k.CollectibleCoins(ctx, position.Id)
 			if err != nil {
 				msg += fmt.Sprintf(
-					"\tcannot calculate collectible coins for position %d\n", position.Id)
+					"\tcannot calculate collectible coins for position %d: %s\n", position.Id, err)
 				cnt++
 				return false
 			}
 			if err := k.Collect(ctx, ownerAddr, ownerAddr, position.Id, fee.Add(farmingRewards...)); err != nil {
 				msg += fmt.Sprintf(
-					"\tcannot collect rewards from position %d\n", position.Id)
+					"\tcannot collect rewards from position %d: %s\n", position.Id, err)
 				cnt++
 				return false
 			}

x/amm/keeper/pool.go

@@ -50,7 +50,7 @@ func (k Keeper) CreatePool(ctx sdk.Context, creatorAddr sdk.AccAddress, marketId
 	return pool, nil
 }
 
-func (k Keeper) IteratePoolOrders(ctx sdk.Context, pool types.Pool, isBuy bool, cb func(price, qty sdk.Dec) (stop bool)) {
+func (k Keeper) IteratePoolOrders(ctx sdk.Context, pool types.Pool, isBuy bool, cb func(price, qty, openQty sdk.Dec) (stop bool)) {
 	poolState := k.MustGetPoolState(ctx, pool.Id)
 	reserveBalance := k.bankKeeper.SpendableCoins(ctx, pool.MustGetReserveAddress()).
 		AmountOf(pool.DenomOut(isBuy)).ToDec()
@@ -83,18 +83,16 @@ func (k Keeper) IteratePoolOrders(ctx sdk.Context, pool types.Pool, isBuy bool,
 				orderPrice := exchangetypes.PriceAtTick(orderTick)
 				orderSqrtPrice := utils.DecApproxSqrt(orderPrice)
 				currentSqrtPrice := utils.DecApproxSqrt(currentPrice)
-				var qty sdk.Dec
+				var qty, openQty sdk.Dec
 				if isBuy {
-					qty = sdk.MinDec(
-						reserveBalance.QuoTruncate(orderPrice),
-						types.Amount1DeltaDec(currentSqrtPrice, orderSqrtPrice, orderLiquidity).QuoTruncate(orderPrice))
+					qty = types.Amount1DeltaDec(currentSqrtPrice, orderSqrtPrice, orderLiquidity).QuoTruncate(orderPrice)
+					openQty = sdk.MinDec(reserveBalance.QuoTruncate(orderPrice), qty)
 				} else {
-					qty = sdk.MinDec(
-						reserveBalance,
-						types.Amount0DeltaRoundingDec(currentSqrtPrice, orderSqrtPrice, orderLiquidity, false))
+					qty = types.Amount0DeltaRoundingDec(currentSqrtPrice, orderSqrtPrice, orderLiquidity, false)
+					openQty = sdk.MinDec(reserveBalance, qty)
 				}
-				if qty.GTE(pool.MinOrderQuantity) || orderTick == tick {
-					if cb(orderPrice, qty) {
+				if openQty.IsPositive() && (openQty.GTE(pool.MinOrderQuantity) || orderTick == tick) {
+					if cb(orderPrice, qty, openQty) {
 						return true
 					}
 					reserveBalance = reserveBalance.Sub(exchangetypes.DepositAmount(isBuy, orderPrice, qty))

x/amm/keeper/pool_test.go

@@ -140,12 +140,12 @@ func (s *KeeperTestSuite) TestPoolOrders() {
 			lpAddr := s.FundedAccount(1, utils.ParseCoins("10000_000000ucre,10000_000000uusd"))
 			tc.addLiquidity(pool, lpAddr)
 			var buyOrders, sellOrders []order
-			s.App.AMMKeeper.IteratePoolOrders(s.Ctx, pool, true, func(price, qty sdk.Dec) (stop bool) {
-				buyOrders = append(buyOrders, order{price, qty})
+			s.App.AMMKeeper.IteratePoolOrders(s.Ctx, pool, true, func(price, qty, openQty sdk.Dec) (stop bool) {
+				buyOrders = append(buyOrders, order{price, openQty})
 				return false
 			})
-			s.App.AMMKeeper.IteratePoolOrders(s.Ctx, pool, false, func(price, qty sdk.Dec) (stop bool) {
-				sellOrders = append(sellOrders, order{price, qty})
+			s.App.AMMKeeper.IteratePoolOrders(s.Ctx, pool, false, func(price, qty, openQty sdk.Dec) (stop bool) {
+				sellOrders = append(sellOrders, order{price, openQty})
 				return false
 			})
 			s.Require().Len(buyOrders, len(tc.buyOrders))

x/amm/keeper/source.go

@@ -28,34 +28,35 @@ func (k OrderSource) Name() string {
 func (k OrderSource) ConstructMemOrderBookSide(
 	ctx sdk.Context, market exchangetypes.Market,
 	createOrder exchangetypes.CreateOrderFunc,
-	opts exchangetypes.MemOrderBookSideOptions) {
+	opts exchangetypes.MemOrderBookSideOptions) error {
 	pool, found := k.GetPoolByMarket(ctx, market.Id)
 	if !found {
-		return // no pool found
+		return nil // no pool found
 	}
 
 	reserveAddr := pool.MustGetReserveAddress()
 	accQty := utils.ZeroDec
 	accQuote := utils.ZeroDec
 	numPriceLevels := 0
-	k.IteratePoolOrders(ctx, pool, opts.IsBuy, func(price, qty sdk.Dec) (stop bool) {
+	k.IteratePoolOrders(ctx, pool, opts.IsBuy, func(price, qty, openQty sdk.Dec) (stop bool) {
 		if opts.ReachedLimit(price, accQty, accQuote, numPriceLevels) {
 			return true
 		}
-		createOrder(reserveAddr, price, qty)
+		createOrder(reserveAddr, price, qty, openQty)
 		accQty = accQty.Add(qty)
 		accQuote = accQuote.Add(exchangetypes.QuoteAmount(!opts.IsBuy, price, qty))
 		numPriceLevels++
 		return false
 	})
+	return nil
 }
 
-func (k OrderSource) AfterOrdersExecuted(ctx sdk.Context, _ exchangetypes.Market, ordererAddr sdk.AccAddress, results []*exchangetypes.MemOrder) {
+func (k OrderSource) AfterOrdersExecuted(ctx sdk.Context, _ exchangetypes.Market, ordererAddr sdk.AccAddress, results []*exchangetypes.MemOrder) error {
 	pool := k.MustGetPoolByReserveAddress(ctx, ordererAddr)
-	k.AfterPoolOrdersExecuted(ctx, pool, results)
+	return k.AfterPoolOrdersExecuted(ctx, pool, results)
 }
 
-func (k Keeper) AfterPoolOrdersExecuted(ctx sdk.Context, pool types.Pool, results []*exchangetypes.MemOrder) {
+func (k Keeper) AfterPoolOrdersExecuted(ctx sdk.Context, pool types.Pool, results []*exchangetypes.MemOrder) error {
 	reserveAddr := pool.MustGetReserveAddress()
 	poolState := k.MustGetPoolState(ctx, pool.Id)
 	accruedRewards := sdk.NewCoins()
@@ -123,6 +124,7 @@ func (k Keeper) AfterPoolOrdersExecuted(ctx sdk.Context, pool types.Pool, result
 		orderTick := exchangetypes.TickAtPrice(result.Price())
 
 		if isBuy && max && poolState.CurrentTick == targetTick {
+			accrueFees()
 			netLiquidity := k.crossTick(ctx, pool.Id, targetTick, poolState)
 			poolState.CurrentLiquidity = poolState.CurrentLiquidity.Sub(netLiquidity)
 			foundTargetTick = false
@@ -165,7 +167,7 @@ func (k Keeper) AfterPoolOrdersExecuted(ctx sdk.Context, pool types.Pool, result
 		currentSqrtPrice := utils.DecApproxSqrt(poolState.CurrentPrice)
 		var nextSqrtPrice, nextPrice sdk.Dec
 		max = false
-		if i < len(results)-1 || result.ExecutableQuantity().LTE(utils.SmallestDec) {
+		if i < len(results)-1 || result.Quantity().Sub(result.ExecutedQuantity()).LTE(utils.SmallestDec) {
 			nextSqrtPrice = utils.DecApproxSqrt(result.Price())
 			nextPrice = result.Price()
 			max = true
@@ -219,6 +221,7 @@ func (k Keeper) AfterPoolOrdersExecuted(ctx sdk.Context, pool types.Pool, result
 
 	if err := k.bankKeeper.SendCoins(
 		ctx, reserveAddr, pool.MustGetRewardsPoolAddress(), accruedRewards); err != nil {
-		panic(err)
+		return err
 	}
+	return nil
 }

x/exchange/keeper/grpc_query_test.go

@@ -109,7 +109,7 @@ func (s *KeeperTestSuite) TestQueryMarket() {
 			"",
 			func(resp *types.QueryMarketResponse) {
 				s.Require().EqualValues(2, resp.Market.Id)
-				s.AssertEqual(utils.ParseDec("-0.0015"), resp.Market.MakerFeeRate)
+				s.AssertEqual(utils.ParseDec("0.0015"), resp.Market.MakerFeeRate)
 				s.AssertEqual(utils.ParseDec("0.003"), resp.Market.TakerFeeRate)
 				s.AssertEqual(utils.ParseDec("0.5"), resp.Market.OrderSourceFeeRatio)
 			},

x/exchange/keeper/market.go

@@ -21,13 +21,13 @@ func (k Keeper) CreateMarket(
 		return market, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "market already exists: %d", marketId)
 	}
 
-	fees := k.GetFees(ctx)
 	if err = k.bankKeeper.SendCoinsFromAccountToModule(
-		ctx, creatorAddr, types.ModuleName, fees.MarketCreationFee); err != nil {
+		ctx, creatorAddr, types.ModuleName, k.GetMarketCreationFee(ctx)); err != nil {
 		return market, sdkerrors.Wrap(err, "insufficient market creation fee")
 	}
 
 	marketId := k.GetNextMarketIdWithUpdate(ctx)
+	fees := k.GetFees(ctx)
 	market = types.NewMarket(
 		marketId, baseDenom, quoteDenom,
 		fees.DefaultMakerFeeRate, fees.DefaultTakerFeeRate, fees.DefaultOrderSourceFeeRatio)