[R4R] Add delist feature (#575)

* add delist feature

* implement delist function

* update

* add unit test for list hooks

* add delist unit test

* add pair mapper unit tests

* update

* update test

* remove delayed days

* fix small issues

* refactor

* refactor

* check delist constraints before doing it

* remove panic

* change days back

* expand proposal search time range

* add pub test

* add is delisted param

* get max deposit period from store

* add upgrade config

* update dependency

* move methods in mapper to keeper

* fix imports

app/app.go

@@ -145,11 +145,6 @@ func NewBinanceChain(logger log.Logger, db dbm.DB, traceStore io.Writer, baseApp
 		app.RegisterCodespace(gov.DefaultCodespace),
 		app.Pool,
 	)
-	listHooks := list.NewListHooks(tradingPairMapper, app.TokenMapper)
-	feeChangeHooks := param.NewFeeChangeHooks(app.Codec)
-	app.govKeeper.AddHooks(gov.ProposalTypeListTradingPair, listHooks)
-	app.govKeeper.AddHooks(gov.ProposalTypeFeeChange, feeChangeHooks)
-
 	app.ParamHub.SetGovKeeper(app.govKeeper)
 
 	// legacy bank route (others moved to plugin init funcs)
@@ -232,6 +227,7 @@ func NewBinanceChain(logger log.Logger, db dbm.DB, traceStore io.Writer, baseApp
 
 	// remaining plugin init
 	app.initDex(tradingPairMapper)
+	app.initGovHooks()
 	app.initPlugins()
 	app.initParams()
 	app.initStateSyncManager(ServerContext.Config.StateSyncReactor)
@@ -240,7 +236,7 @@ func NewBinanceChain(logger log.Logger, db dbm.DB, traceStore io.Writer, baseApp
 
 // setUpgradeConfig will overwrite default upgrade config
 func (app *BinanceChain) setUpgradeConfig() {
-	// upgrade.Mgr.AddUpgradeHeight(,)
+	upgrade.Mgr.AddUpgradeHeight(upgrade.BEP6, app.upgradeConfig.BEP6Height)
 }
 
 func (app *BinanceChain) initRunningMode() {
@@ -277,6 +273,15 @@ func (app *BinanceChain) initPlugins() {
 	param.InitPlugin(app, app.ParamHub)
 }
 
+func (app *BinanceChain) initGovHooks() {
+	listHooks := list.NewListHooks(app.DexKeeper, app.TokenMapper)
+	feeChangeHooks := param.NewFeeChangeHooks(app.Codec)
+	delistHooks := list.NewDelistHooks(app.DexKeeper)
+	app.govKeeper.AddHooks(gov.ProposalTypeListTradingPair, listHooks)
+	app.govKeeper.AddHooks(gov.ProposalTypeFeeChange, feeChangeHooks)
+	app.govKeeper.AddHooks(gov.ProposalTypeDelistTradingPair, delistHooks)
+}
+
 func (app *BinanceChain) initParams() {
 	if app.CheckState == nil || app.CheckState.Ctx.BlockHeight() == 0 {
 		return
@@ -464,7 +469,7 @@ func (app *BinanceChain) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) a
 		bnclog.Info("Start Breathe Block Handling",
 			"height", height, "lastBlockTime", lastBlockTime, "newBlockTime", blockTime)
 		icoDone := ico.EndBlockAsync(ctx)
-		dex.EndBreatheBlock(ctx, app.DexKeeper, height, blockTime)
+		dex.EndBreatheBlock(ctx, app.DexKeeper, app.govKeeper, height, blockTime)
 		param.EndBreatheBlock(ctx, app.ParamHub)
 		// other end blockers
 		<-icoDone

app/config/config.go

@@ -6,6 +6,7 @@ import (
 	"text/template"
 
 	"github.com/spf13/viper"
+
 	"github.com/tendermint/tendermint/libs/cli"
 	"github.com/tendermint/tendermint/libs/common"
 
@@ -45,6 +46,8 @@ orderKeeperConcurrency = {{ .BaseConfig.OrderKeeperConcurrency }}
 breatheBlockDaysCountBack = {{ .BaseConfig.BreatheBlockDaysCountBack }}
 
 [upgrade]
+# Block height of BEP6 upgrade
+BEP6Height = {{ .UpgradeConfig.BEP6Height }}
 
 [addr]
 # Bech32PrefixAccAddr defines the Bech32 prefix of an account's address
@@ -283,10 +286,13 @@ func defaultBaseConfig() *BaseConfig {
 type UpgradeConfig struct {
 	// example
 	// FixXxxHeight int64 `mapstructure:"fixXxxHeight"`
+	BEP6Height int64 `mapstructure:"BEP6Height"`
 }
 
 func defaultUpgradeConfig() *UpgradeConfig {
-	return &UpgradeConfig{}
+	return &UpgradeConfig{
+		BEP6Height: 1,
+	}
 }
 
 func (context *BinanceChainContext) ParseAppConfigInPlace() error {

app/pub/helpers.go

@@ -177,6 +177,22 @@ func ExpireOrdersForPublish(
 	return
 }
 
+func DelistTradingPairForPublish(ctx sdk.Context, dexKeeper *orderPkg.Keeper, symbol string) {
+	expireHolderCh := make(chan orderPkg.ExpireHolder, TransferCollectionChannelSize)
+	wg := sync.WaitGroup{}
+	wg.Add(1)
+	go updateExpireFeeForPublish(dexKeeper, &wg, expireHolderCh)
+	var collectorForExpires = func(tran orderPkg.Transfer) {
+		if tran.IsExpire() {
+			expireHolderCh <- orderPkg.ExpireHolder{OrderId: tran.Oid, Reason: orderPkg.Expired}
+		}
+	}
+	dexKeeper.DelistTradingPair(ctx, symbol, collectorForExpires)
+	close(expireHolderCh)
+	wg.Wait()
+	return
+}
+
 // for partial and fully filled order fee
 func collectTradeForPublish(
 	tradesToPublish *[]*Trade,

app/pub/keeper_pub_test.go

@@ -162,6 +162,31 @@ func TestKeeper_ExpireWithFee(t *testing.T) {
 	assert.Equal(orderPkg.Expired, orderChange1.Tpe)
 }
 
+func TestKeeper_DelistWithFee(t *testing.T) {
+	assert, require := setupKeeperTest(t)
+
+	msg := orderPkg.NewOrderMsg{buyer, "1", "XYZ-000_BNB", orderPkg.OrderType.LIMIT, orderPkg.Side.BUY, 102000, 3000000, orderPkg.TimeInForce.GTE}
+	keeper.AddOrder(orderPkg.OrderInfo{msg, 42, 100, 42, 100, 0, "08E19B16880CF70D59DDD996E3D75C66CD0405DE"}, false)
+
+	require.Len(keeper.OrderChanges, 1)
+	require.Len(keeper.OrderInfosForPub, 1)
+
+	DelistTradingPairForPublish(ctx, keeper, "XYZ-000_BNB")
+
+	require.Len(keeper.OrderChanges, 2)
+	require.Len(keeper.OrderInfosForPub, 1)
+
+	orderChange0 := keeper.OrderChanges[0]
+	orderChange1 := keeper.OrderChanges[1]
+
+	// verify orderChange0 - Ack
+	assert.Equal("1", orderChange0.Id)
+	assert.Equal(orderPkg.Ack, orderChange0.Tpe)
+	// verify orderChange1 - ExpireNoFill
+	assert.Equal("1", orderChange1.Id)
+	assert.Equal(orderPkg.Expired, orderChange1.Tpe)
+}
+
 func Test_IOCPartialExpire(t *testing.T) {
 	assert, require := setupKeeperTest(t)
 

common/upgrade/upgrade.go

@@ -7,3 +7,5 @@ var Mgr = sdk.UpgradeMgr
 // prefix for the upgrade name
 // bugfix: fix
 // improvement: (maybe bep ?)
+
+const BEP6 = "BEP6"

integration_test.sh

@@ -138,7 +138,6 @@ check_operation "Place Order" "${result}" "${chain_operation_words}"
 result=$(./bnbcli dex show -l ${btc_symbol}_BNB  --trust-node true)
 check_operation "Order Book" "${result}" "${order_book_words}"
 
-
 ## ROUND 2 ##
 
 round="2"

plugins/dex/list/handler.go

@@ -10,9 +10,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/gov"
 
 	"github.com/binance-chain/node/common/log"
-	commonTypes "github.com/binance-chain/node/common/types"
 	"github.com/binance-chain/node/plugins/dex/order"
-	"github.com/binance-chain/node/plugins/dex/store"
 	"github.com/binance-chain/node/plugins/dex/types"
 	"github.com/binance-chain/node/plugins/tokens"
 )
@@ -30,7 +28,7 @@ func NewHandler(keeper *order.Keeper, tokenMapper tokens.Mapper, govKeeper gov.K
 	}
 }
 
-func checkProposal(ctx sdk.Context, govKeeper gov.Keeper, msg ListMsg) error {
+func checkListProposal(ctx sdk.Context, govKeeper gov.Keeper, msg ListMsg) error {
 	proposal := govKeeper.GetProposal(ctx, msg.ProposalId)
 	if proposal == nil {
 		return fmt.Errorf("proposal %d does not exist", msg.ProposalId)
@@ -74,42 +72,14 @@ func checkProposal(ctx sdk.Context, govKeeper gov.Keeper, msg ListMsg) error {
 	return nil
 }
 
-func checkPrerequisiteTradingPair(ctx sdk.Context, pairMapper store.TradingPairMapper, baseAssetSymbol, quoteAssetSymbol string) error {
-	// trading pair against native token should exist if quote token is not native token
-	baseAssetSymbol = strings.ToUpper(baseAssetSymbol)
-	quoteAssetSymbol = strings.ToUpper(quoteAssetSymbol)
-
-	if baseAssetSymbol != commonTypes.NativeTokenSymbol &&
-		quoteAssetSymbol != commonTypes.NativeTokenSymbol {
-
-		if !pairMapper.Exists(ctx, baseAssetSymbol, commonTypes.NativeTokenSymbol) &&
-			!pairMapper.Exists(ctx, commonTypes.NativeTokenSymbol, baseAssetSymbol) {
-			return fmt.Errorf("token %s should be listed against BNB before against %s",
-				baseAssetSymbol, quoteAssetSymbol)
-		}
-
-		if !pairMapper.Exists(ctx, quoteAssetSymbol, commonTypes.NativeTokenSymbol) &&
-			!pairMapper.Exists(ctx, commonTypes.NativeTokenSymbol, quoteAssetSymbol) {
-			return fmt.Errorf("token %s should be listed against BNB before listing %s against %s",
-				quoteAssetSymbol, baseAssetSymbol, quoteAssetSymbol)
-		}
-	}
-	return nil
-}
-
 func handleList(
 	ctx sdk.Context, keeper *order.Keeper, tokenMapper tokens.Mapper, govKeeper gov.Keeper, msg ListMsg,
 ) sdk.Result {
-	if err := checkProposal(ctx, govKeeper, msg); err != nil {
+	if err := checkListProposal(ctx, govKeeper, msg); err != nil {
 		return types.ErrInvalidProposal(err.Error()).Result()
 	}
 
-	if keeper.PairMapper.Exists(ctx, msg.BaseAssetSymbol, msg.QuoteAssetSymbol) ||
-		keeper.PairMapper.Exists(ctx, msg.QuoteAssetSymbol, msg.BaseAssetSymbol) {
-		return sdk.ErrInvalidCoins("trading pair exists").Result()
-	}
-
-	if err := checkPrerequisiteTradingPair(ctx, keeper.PairMapper, msg.BaseAssetSymbol, msg.QuoteAssetSymbol); err != nil {
+	if err := keeper.CanListTradingPair(ctx, msg.BaseAssetSymbol, msg.QuoteAssetSymbol); err != nil {
 		return sdk.ErrInvalidCoins(err.Error()).Result()
 	}
 

plugins/dex/list/hooks.go

@@ -8,18 +8,19 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/gov"
 
-	"github.com/binance-chain/node/plugins/dex/store"
+	"github.com/binance-chain/node/common/upgrade"
+	"github.com/binance-chain/node/plugins/dex/order"
 	"github.com/binance-chain/node/plugins/tokens"
 )
 
 type ListHooks struct {
-	pairMapper  store.TradingPairMapper
+	orderKeeper *order.Keeper
 	tokenMapper tokens.Mapper
 }
 
-func NewListHooks(pairMapper store.TradingPairMapper, tokenMapper tokens.Mapper) ListHooks {
+func NewListHooks(orderKeeper *order.Keeper, tokenMapper tokens.Mapper) ListHooks {
 	return ListHooks{
-		pairMapper:  pairMapper,
+		orderKeeper: orderKeeper,
 		tokenMapper: tokenMapper,
 	}
 }
@@ -65,12 +66,61 @@ func (hooks ListHooks) OnProposalSubmitted(ctx sdk.Context, proposal gov.Proposa
 		return errors.New("quote token does not exist")
 	}
 
-	if hooks.pairMapper.Exists(ctx, listParams.BaseAssetSymbol, listParams.QuoteAssetSymbol) ||
-		hooks.pairMapper.Exists(ctx, listParams.QuoteAssetSymbol, listParams.BaseAssetSymbol) {
-		return errors.New("trading pair exists")
+	if err := hooks.orderKeeper.CanListTradingPair(ctx, listParams.BaseAssetSymbol, listParams.QuoteAssetSymbol); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+type DelistHooks struct {
+	orderKeeper *order.Keeper
+}
+
+func NewDelistHooks(orderKeeper *order.Keeper) DelistHooks {
+	return DelistHooks{
+		orderKeeper: orderKeeper,
+	}
+}
+
+var _ gov.GovHooks = DelistHooks{}
+
+func (hooks DelistHooks) OnProposalSubmitted(ctx sdk.Context, proposal gov.Proposal) error {
+	if proposal.GetProposalType() != gov.ProposalTypeDelistTradingPair {
+		panic(fmt.Sprintf("received wrong type of proposal %x", proposal.GetProposalType()))
+	}
+
+	if !sdk.IsUpgrade(upgrade.BEP6) {
+		return fmt.Errorf("proposal type %s is not supported", gov.ProposalTypeDelistTradingPair)
+	}
+
+	delistParams := gov.DelistTradingPairParams{}
+	err := json.Unmarshal([]byte(proposal.GetDescription()), &delistParams)
+	if err != nil {
+		return fmt.Errorf("unmarshal list params error, err=%s", err.Error())
+	}
+
+	if delistParams.BaseAssetSymbol == "" {
+		return errors.New("base asset symbol should not be empty")
+	}
+
+	if delistParams.QuoteAssetSymbol == "" {
+		return errors.New("quote asset symbol should not be empty")
+	}
+
+	if delistParams.BaseAssetSymbol == delistParams.QuoteAssetSymbol {
+		return errors.New("base asset symbol and quote asset symbol should not be the same")
+	}
+
+	if delistParams.Justification == "" {
+		return errors.New("justification should not be empty")
+	}
+
+	if delistParams.IsExecuted {
+		return errors.New("is_executed should be false")
 	}
 
-	if err := checkPrerequisiteTradingPair(ctx, hooks.pairMapper, listParams.BaseAssetSymbol, listParams.QuoteAssetSymbol); err != nil {
+	if err := hooks.orderKeeper.CanDelistTradingPair(ctx, delistParams.BaseAssetSymbol, delistParams.QuoteAssetSymbol); err != nil {
 		return err
 	}