BEP67 Price-based Order Expiration #704

EnderCrypto · 2020-03-09T15:39:46Z

Description

expiration time unchanged (3 days), but keep orders in the best 500 price levels on both ask and bid for at most 1 month.

Rationale

tell us why we need these changes...

Some users want to keep their open orders more than 3 days.
add a way user can extend the order life to stay longer than 72 hours

add an example CLI or API response...

Changes

Notable changes:

Add RemoveOrdersBiasedly in Orderbook for the new expire order approach

Preflight checks

build passed (make build)
tests passed (make test)
integration tests passed (make integration_test)
manual transaction test passed (cli invoke)

Already reviewed by

...

Related issues

... reference related issue #'s here ...

rickyyangz · 2020-03-10T04:26:10Z

plugins/dex/matcheng/unrolledlinkedlist.go

@@ -433,3 +433,51 @@ func (ull *ULList) UpdateForEach(updater LevelIter) {
 		b = oldNext
 	}
 }
+
+func (ull *ULList) UpdateForEachBiasedly(preferenceSize int, preferenceUpdater LevelIter, updater LevelIter) {


can we reuse the original UpdateForEach, I think we can change the updater function itself.

rickyyangz · 2020-04-06T15:16:47Z

plugins/dex/matcheng/types.go

@@ -190,7 +190,7 @@ func (overlapped *OverLappedLevel) HasSellTaker() bool {
 	return overlapped.SellTakerStartIdx < len(overlapped.SellOrders)
 }

-type LevelIter func(*PriceLevel)
+type LevelIter func(priceLevel *PriceLevel, currentLevel int)


I suggest renaming the currentLevel to levelIndex

rickyyangz · 2020-04-06T15:24:08Z

plugins/dex/matcheng/orderbook.go

+	return nil
+}
+
+func (ob *OrderBookOnULList) UpdateForEachPriceLevel(side int8, updater func(priceLevel *PriceLevel, currentLevel int)) {


just use LevelIter as the updater's type

rickyyangz · 2020-04-06T15:53:48Z

plugins/dex/order/keeper.go

@@ -831,6 +832,17 @@ func (kp *Keeper) expireOrders(ctx sdk.Context, blockTime time.Time) []chan Tran
 		return nil
 	}

+	var forceExpireHeight int64
+	var err2 error


err2 is not a good name, actually you can just reuse the err

rickyyangz · 2020-04-06T15:55:33Z

plugins/dex/order/keeper.go

+	numPricesStored          = 2000
+	pricesStoreEvery         = 1000
+	minimalNumPrices         = 500
+	bep2PreferencePriceLevel = 500


why add a bep2 prefix

Mini-token may have less price levels for orders living longer than 3 days

remove bep2 prefix first. You can change that when you really need it in mini token PR

rickyyangz · 2020-04-06T15:59:27Z

plugins/dex/matcheng/orderbook.go

@@ -20,7 +20,8 @@ type OrderBookInterface interface {
 	GetOrder(id string, side int8, price int64) (OrderPart, error)
 	RemoveOrder(id string, side int8, price int64) (OrderPart, error)
 	RemoveOrders(beforeTime int64, side int8, cb func(OrderPart)) error
-	UpdateForEachPriceLevel(side int8, updater func(*PriceLevel))
+	RemoveOrdersBiasedly(beforeTime int64, forceTime int64, preferenceSize int, side int8, removeCallback func(ord OrderPart)) error


the method name is confusing, so are the parameter names.

rickyyangz · 2020-04-06T16:01:13Z

common/upgrade/upgrade.go

@@ -8,6 +8,7 @@ var Mgr = sdk.UpgradeMgr
 // bugfix: fix
 // improvement: (maybe bep ?)
 const (
+	BEPX = "BEPX" //placeholder for expire order extension


since we do not have a corresponding BEP, just give it a meaningful name.
Also, you need to register the upgrade with an upgrade height

rickyyangz · 2020-04-19T13:50:57Z

app/app.go

@@ -253,6 +253,7 @@ func NewBinanceChain(logger log.Logger, db dbm.DB, traceStore io.Writer, baseApp
 // setUpgradeConfig will overwrite default upgrade config
 func SetUpgradeConfig(upgradeConfig *config.UpgradeConfig) {
 	// register upgrade height
+	upgrade.Mgr.AddUpgradeHeight(upgrade.BEP67, upgradeConfig.BEP6Height)


put the upgrade config in the right order.

rickyyangz · 2020-04-19T13:51:09Z

app/config/config.go

@@ -47,6 +47,8 @@ orderKeeperConcurrency = {{ .BaseConfig.OrderKeeperConcurrency }}
 breatheBlockDaysCountBack = {{ .BaseConfig.BreatheBlockDaysCountBack }}

 [upgrade]
+# Block height of BEP67 upgrade
+BEP67Height = {{ .UpgradeConfig.BEP67Height }}


rickyyangz · 2020-04-19T14:06:17Z

plugins/dex/order/keeper.go

+		const forceExpireDays = 30
+		forceExpireHeight, err = kp.GetBreatheBlockHeight(ctx, blockTime, forceExpireDays)
+		if err != nil {
+			kp.logger.Info(err.Error())


let's use kp.logger.Error. Also for the #831

rickyyangz · 2020-04-19T14:08:55Z

plugins/dex/order/keeper.go

+			kp.logger.Info(err.Error())
+			forceExpireHeight = -1
+		}
+	}


it's better to extract a method get the expireHeight and forceExpireHeight.
e.g. func getExpireHeight(...) (expireHeight, forceExpireHeight int64)

ForceExpireHeight is only needed after BEP67

rickyyangz · 2020-04-19T14:09:55Z

plugins/dex/matcheng/unrolledlinkedlist.go

@@ -395,6 +395,7 @@ func (ull *ULList) GetPriceLevel(p int64) *PriceLevel {
 func (ull *ULList) UpdateForEach(updater LevelIter) {
 	b := ull.begin
 	var last *bucket
+	iteratedCount := 0


levelIndex may be better

rickyyangz · 2020-04-19T14:13:59Z

plugins/dex/matcheng/types.go

@@ -190,7 +190,7 @@ func (overlapped *OverLappedLevel) HasSellTaker() bool {
 	return overlapped.SellTakerStartIdx < len(overlapped.SellOrders)
 }

-type LevelIter func(*PriceLevel)
+type LevelIter func(priceLevel *PriceLevel, levelIndex int)


the type of levelIndex should be int64.

Max level Index is int
func (ob *OrderBookOnULList) ShowDepth(maxLevels int, iterBuy LevelIter, iterSell LevelIter) {
ob.buyQueue.Iterate(maxLevels, iterBuy)
ob.sellQueue.Iterate(maxLevels, iterSell)
}

fix & test format extract preference size as param format: refactor updator fix format rename arg refactor

EnderCrypto requested review from rickyyangz, ackratos, fletcher142 and forcodedancing March 9, 2020 15:40

EnderCrypto changed the base branch from master to develop March 9, 2020 15:44

rickyyangz reviewed Mar 10, 2020

View reviewed changes

EnderCrypto force-pushed the expire_order_opt branch from 6dec50f to f68f997 Compare March 15, 2020 10:17

EnderCrypto requested a review from FitzLu March 15, 2020 10:27

rickyyangz reviewed Apr 6, 2020

View reviewed changes

EnderCrypto force-pushed the expire_order_opt branch from 3f809f0 to acd72cb Compare April 7, 2020 10:57

EnderCrypto requested a review from rickyyangz April 7, 2020 10:59

rickyyangz reviewed Apr 19, 2020

View reviewed changes

EnderCrypto force-pushed the expire_order_opt branch 3 times, most recently from 2435876 to 8f86d0e Compare April 21, 2020 14:08

expire orders by price level

81875bd

fix & test format extract preference size as param format: refactor updator fix format rename arg refactor

EnderCrypto force-pushed the expire_order_opt branch from 8f86d0e to 81875bd Compare April 21, 2020 14:23

EnderCrypto merged commit 4ab7320 into develop Apr 22, 2020

EnderCrypto changed the title ~~Expire order opt~~ BEP67 Price-based Order Expiration Jun 2, 2020

EnderCrypto mentioned this pull request Jun 2, 2020

0.7 Upgrade #744

Merged

unclezoro deleted the expire_order_opt branch May 10, 2022 06:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BEP67 Price-based Order Expiration #704

BEP67 Price-based Order Expiration #704

EnderCrypto commented Mar 9, 2020 •

edited by forcodedancing

rickyyangz Mar 10, 2020 •

edited

rickyyangz Apr 6, 2020

rickyyangz Apr 6, 2020

rickyyangz Apr 6, 2020

rickyyangz Apr 6, 2020

EnderCrypto Apr 7, 2020

rickyyangz Apr 19, 2020

rickyyangz Apr 6, 2020

rickyyangz Apr 6, 2020 •

edited

rickyyangz Apr 19, 2020

rickyyangz Apr 19, 2020

rickyyangz Apr 19, 2020 •

edited

rickyyangz Apr 19, 2020

EnderCrypto Apr 21, 2020

rickyyangz Apr 19, 2020

rickyyangz Apr 19, 2020

EnderCrypto Apr 21, 2020

BEP67 Price-based Order Expiration #704

BEP67 Price-based Order Expiration #704

Conversation

EnderCrypto commented Mar 9, 2020 • edited by forcodedancing

Description

Rationale

Changes

Preflight checks

Already reviewed by

Related issues

rickyyangz Mar 10, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rickyyangz Apr 6, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rickyyangz Apr 19, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EnderCrypto commented Mar 9, 2020 •

edited by forcodedancing

rickyyangz Mar 10, 2020 •

edited

rickyyangz Apr 6, 2020 •

edited

rickyyangz Apr 19, 2020 •

edited