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

BEP70 - Support busd pair listing and trading #710

Merged
merged 13 commits into from
Apr 27, 2020
Merged

BEP70 - Support busd pair listing and trading #710

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
57b3caa
add busd pair support
Mar 17, 2020
fc1ffd4
support busd pairs for listing and trading
Mar 18, 2020
ddbeca3
add more test cases
Mar 27, 2020
8ed1e06
change SupportedListSymbols[] to single BUSDSymbol
Apr 7, 2020
0e46d38
extract methods; update BEP number
Apr 9, 2020
018598e
refactor
Apr 12, 2020
65ae54a
refactor keeper
Apr 24, 2020
53e2c6e
busd fee refactor
rickyyangz Apr 26, 2020
7b9e938
Merge pull request #722 from binance-chain/busd
forcodedancing Apr 27, 2020
ba142c2
add check
Apr 27, 2020
c1fa20a
Merge branch 'develop' into support_busd
forcodedancing Apr 27, 2020
72f09d9
order and group upgrade config
Apr 27, 2020
5d9010b
order and group upgrade config
Apr 27, 2020
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
6 changes: 6 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,9 @@ vendor
*.swp
.vscode/*.json

# Test generate tmp files
app/data
app/apptest/data
app_test/data
plugins/param/data
plugins/tokens/data
7 changes: 6 additions & 1 deletion app/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ type BinanceChain struct {
publicationConfig *config.PublicationConfig
publisher pub.MarketDataPublisher

govConfig *config.GovConfig

// Unlike tendermint, we don't need implement a no-op metrics, usage of this field should
// check nil-ness to know whether metrics collection is turn on
// TODO(#246): make it an aggregated wrapper of all component metrics (i.e. DexKeeper, StakeKeeper)
Expand All @@ -119,6 +121,7 @@ func NewBinanceChain(logger log.Logger, db dbm.DB, traceStore io.Writer, baseApp
upgradeConfig: ServerContext.UpgradeConfig,
abciQueryBlackList: getABCIQueryBlackList(ServerContext.QueryConfig),
publicationConfig: ServerContext.PublicationConfig,
govConfig: ServerContext.GovConfig,
}
// set upgrade config
SetUpgradeConfig(app.upgradeConfig)
Expand Down Expand Up @@ -263,6 +266,7 @@ func SetUpgradeConfig(upgradeConfig *config.UpgradeConfig) {
upgrade.Mgr.AddUpgradeHeight(upgrade.LotSizeOptimization, upgradeConfig.LotSizeUpgradeHeight)
upgrade.Mgr.AddUpgradeHeight(upgrade.ListingRuleUpgrade, upgradeConfig.ListingRuleUpgradeHeight)
upgrade.Mgr.AddUpgradeHeight(upgrade.FixZeroBalance, upgradeConfig.FixZeroBalanceHeight)
upgrade.Mgr.AddUpgradeHeight(upgrade.BEP_BUSD, upgradeConfig.BEP_BUSDHeight)

// register store keys of upgrade
upgrade.Mgr.RegisterStoreKeys(upgrade.BEP9, common.TimeLockStoreKey.Name())
Expand Down Expand Up @@ -302,7 +306,8 @@ func (app *BinanceChain) initRunningMode() {

func (app *BinanceChain) initDex(pairMapper dex.TradingPairMapper) {
app.DexKeeper = dex.NewOrderKeeper(common.DexStoreKey, app.AccountKeeper, pairMapper,
app.RegisterCodespace(dex.DefaultCodespace), app.baseConfig.OrderKeeperConcurrency, app.Codec,
app.RegisterCodespace(dex.DefaultCodespace), app.baseConfig.OrderKeeperConcurrency,
app.govConfig.SupportedListAgainstSymbols, app.Codec,
app.publicationConfig.ShouldPublishAny())
app.DexKeeper.SubscribeParamChange(app.ParamHub)

Expand Down
19 changes: 18 additions & 1 deletion app/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ LotSizeUpgradeHeight = {{ .UpgradeConfig.LotSizeUpgradeHeight }}
ListingRuleUpgradeHeight = {{ .UpgradeConfig.ListingRuleUpgradeHeight }}
# Block height of FixZeroBalanceHeight upgrade
FixZeroBalanceHeight = {{ .UpgradeConfig.FixZeroBalanceHeight }}
# Block height to BEP_BUSD upgrade
BEP_BUSDHeight = {{ .UpgradeConfig.BEP_BUSDHeight }}

[query]
# ABCI query interface black list, suggested value: ["custom/gov/proposals", "custom/timelock/timelocks", "custom/atomicSwap/swapcreator", "custom/atomicSwap/swaprecipient"]
Expand Down Expand Up @@ -178,6 +180,7 @@ type BinanceChainConfig struct {
*BaseConfig `mapstructure:"base"`
*UpgradeConfig `mapstructure:"upgrade"`
*QueryConfig `mapstructure:"query"`
*GovConfig `mapstructure:"gov"`
}

func DefaultBinanceChainConfig() *BinanceChainConfig {
Expand All @@ -188,6 +191,7 @@ func DefaultBinanceChainConfig() *BinanceChainConfig {
BaseConfig: defaultBaseConfig(),
UpgradeConfig: defaultUpgradeConfig(),
QueryConfig: defaultQueryConfig(),
GovConfig: defaultGovConfig(),
}
}

Expand Down Expand Up @@ -358,7 +362,9 @@ type UpgradeConfig struct {
// Hubble Upgrade
BEP12Height int64 `mapstructure:"BEP12Height"`
// Archimedes Upgrade
BEP3Height int64 `mapstructure:"BEP3Height"`
BEP3Height int64 `mapstructure:"BEP3Height"`
BEP_BUSDHeight int64 `mapstructure:"BEP_BUSDHeight"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please follow the naming convension

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, will rename it after we have the BEP finalized. Or, can we have the BEP NO. now?


// TODO: add upgrade name
FixSignBytesOverflowHeight int64 `mapstructure:"FixSignBytesOverflowHeight"`
LotSizeUpgradeHeight int64 `mapstructure:"LotSizeUpgradeHeight"`
Expand All @@ -375,6 +381,7 @@ func defaultUpgradeConfig() *UpgradeConfig {
BEP19Height: 1,
BEP12Height: 1,
BEP3Height: 1,
BEP_BUSDHeight: 1,
FixSignBytesOverflowHeight: math.MaxInt64,
LotSizeUpgradeHeight: math.MaxInt64,
ListingRuleUpgradeHeight: math.MaxInt64,
Expand All @@ -392,6 +399,16 @@ func defaultQueryConfig() *QueryConfig {
}
}

type GovConfig struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's not a gov related config. can you create a DexConfig instead and remember the update the config template above.

SupportedListAgainstSymbols []string `mapstructure:"SupportedListAgainstSymbols"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest we just simplify the config to BusdSymbol. I don't think we would support multiple assets, or if we want to support multiple assets, we need to have a more reasonable solution instead of the config way.

}

func defaultGovConfig() *GovConfig {
return &GovConfig{
SupportedListAgainstSymbols: nil,
}
}

func (context *BinanceChainContext) ParseAppConfigInPlace() error {
// this piece of code should be consistent with bindFlagsLoadViper
// vendor/github.com/tendermint/tendermint/libs/cli/setup.go:125
Expand Down
2 changes: 1 addition & 1 deletion app/pub/keeper_pub_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func setupKeeperTest(t *testing.T) (*assert.Assertions, *require.Assertions) {
ctx = sdk.NewContext(ms, abci.Header{ChainID: "mychainid"}, sdk.RunTxModeDeliver, logger).WithAccountCache(accountCache)

pairMapper := store.NewTradingPairMapper(cdc, common.PairStoreKey)
keeper = orderPkg.NewKeeper(capKey2, am, pairMapper, sdk.NewCodespacer().RegisterNext(dextypes.DefaultCodespace), 2, cdc, true)
keeper = orderPkg.NewKeeper(capKey2, am, pairMapper, sdk.NewCodespacer().RegisterNext(dextypes.DefaultCodespace), 2, []string{}, cdc, true)
tradingPair := dextypes.NewTradingPair("XYZ-000", types.NativeTokenSymbol, 1e8)
keeper.PairMapper.AddTradingPair(ctx, tradingPair)
keeper.AddEngine(tradingPair)
Expand Down
3 changes: 3 additions & 0 deletions common/upgrade/upgrade.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ const (
BEP12 = "BEP12" // https://github.com/binance-chain/BEPs/pull/17
// Archimedes Upgrade
BEP3 = "BEP3" // https://github.com/binance-chain/BEPs/pull/30
//TODO: change the correct BEP number
BEP_BUSD = "BEP_BUSD" // supporting listing and trading BUSD pairs

// TODO: add upgrade name
FixSignBytesOverflow = sdk.FixSignBytesOverflow
LotSizeOptimization = "LotSizeOptimization"
Expand Down
2 changes: 1 addition & 1 deletion plugins/dex/list/handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func MakeKeepers(cdc *codec.Codec) (ms sdkStore.CommitMultiStore, orderKeeper *o
codespacer := sdk.NewCodespacer()
pairMapper := store.NewTradingPairMapper(cdc, pairKey)
orderKeeper = order.NewKeeper(common.DexStoreKey, accKeeper, pairMapper,
codespacer.RegisterNext(dexTypes.DefaultCodespace), 2, cdc, false)
codespacer.RegisterNext(dexTypes.DefaultCodespace), 2, []string{}, cdc, false)

tokenMapper = tokenStore.NewMapper(cdc, tokenKey)

Expand Down
117 changes: 104 additions & 13 deletions plugins/dex/order/fee.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"math"
"math/big"

"github.com/binance-chain/node/common/upgrade"

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

tmlog "github.com/tendermint/tendermint/libs/log"
Expand Down Expand Up @@ -42,16 +44,18 @@ var (
)

type FeeManager struct {
cdc *wire.Codec
logger tmlog.Logger
FeeConfig FeeConfig
cdc *wire.Codec
logger tmlog.Logger
FeeConfig FeeConfig
GovSupportedSymbols []string
}

func NewFeeManager(cdc *wire.Codec, storeKey sdk.StoreKey, logger tmlog.Logger) *FeeManager {
func NewFeeManager(cdc *wire.Codec, storeKey sdk.StoreKey, sybmols []string, logger tmlog.Logger) *FeeManager {
return &FeeManager{
cdc: cdc,
logger: logger,
FeeConfig: NewFeeConfig(),
cdc: cdc,
logger: logger,
FeeConfig: NewFeeConfig(),
GovSupportedSymbols: sybmols,
}
}

Expand Down Expand Up @@ -134,7 +138,7 @@ func (m *FeeManager) calcTradeFeeForSingleTransfer(balances sdk.Coins, tran *Tra
}

func (m *FeeManager) calcNativeFee(inSymbol string, inQty int64, engines map[string]*matcheng.MatchEng) (fee int64, isOverflow bool) {
var nativeNotional *big.Int
var nativeNotional = big.NewInt(0)
if isNativeToken(inSymbol) {
nativeNotional = big.NewInt(inQty)
} else {
Expand All @@ -143,15 +147,58 @@ func (m *FeeManager) calcNativeFee(inSymbol string, inQty int64, engines map[str
if engine, ok := engines[utils.Assets2TradingPair(inSymbol, types.NativeTokenSymbol)]; ok {
// XYZ_BNB
nativeNotional = utils.CalBigNotional(engine.LastTradePrice, inQty)
} else {
} else if engine, ok := engines[utils.Assets2TradingPair(types.NativeTokenSymbol, inSymbol)]; ok {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you help extract such code into a method, like getEngine, I see we have so many places using that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure

// BNB_XYZ
engine := engines[utils.Assets2TradingPair(types.NativeTokenSymbol, inSymbol)]
var amount big.Int
nativeNotional = amount.Div(
amount.Mul(
big.NewInt(inQty),
big.NewInt(cmnUtils.Fixed8One.ToInt64())),
big.NewInt(engine.LastTradePrice))
} else {
// for BUSD pairs, it is possible that there is no trading pair between BNB and inAsset, e.g., BUSD -> XYZ
if sdk.IsUpgrade(upgrade.BEP_BUSD) {
for _, symbol := range m.GovSupportedSymbols {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to check all the symbols. Each TradeTransfer has both in and out asset symbol and quantity, you just need to use the opposite quantity to calc the native fee. we can talk offline

var foundFirstPair, foundSecondPair bool
var intermediateAmount = big.NewInt(0)

if market, ok := engines[utils.Assets2TradingPair(symbol, inSymbol)]; ok {
foundFirstPair = true
var tmp big.Int
intermediateAmount = tmp.Div(tmp.Mul(
big.NewInt(inQty),
big.NewInt(cmnUtils.Fixed8One.ToInt64())),
big.NewInt(market.LastTradePrice))
} else if market, ok := engines[utils.Assets2TradingPair(inSymbol, symbol)]; ok {
foundFirstPair = true
intermediateAmount = utils.CalBigNotional(market.LastTradePrice, inQty)
}

if foundFirstPair {
var intermediateTmp int64
if intermediateAmount.IsInt64() {
intermediateTmp = intermediateAmount.Int64()
} else {
intermediateTmp = math.MaxInt64
}
if market, ok := engines[utils.Assets2TradingPair(types.NativeTokenSymbol, symbol)]; ok {
foundSecondPair = true
var tmp big.Int
nativeNotional = tmp.Div(tmp.Mul(
big.NewInt(intermediateTmp),
big.NewInt(cmnUtils.Fixed8One.ToInt64())),
big.NewInt(market.LastTradePrice))
} else if market, ok := engines[utils.Assets2TradingPair(symbol, types.NativeTokenSymbol)]; ok {
foundSecondPair = true
nativeNotional = utils.CalBigNotional(market.LastTradePrice, intermediateTmp)
}
}

if foundFirstPair && foundSecondPair {
break
}
}
}
}
}

Expand Down Expand Up @@ -238,18 +285,62 @@ func (m *FeeManager) CalcFixedFee(balances sdk.Coins, eventType transferEventTyp
} else {
// the amount may overflow int64, so use big.Int instead.
// TODO: (perf) may remove the big.Int use to improve the performance
var amount *big.Int
var amount = big.NewInt(0)
if market, ok := engines[utils.Assets2TradingPair(inAsset, types.NativeTokenSymbol)]; ok {
// XYZ_BNB
var tmp big.Int
amount = tmp.Div(tmp.Mul(
big.NewInt(feeAmount),
big.NewInt(cmnUtils.Fixed8One.ToInt64())),
big.NewInt(market.LastTradePrice))
} else {
} else if market, ok := engines[utils.Assets2TradingPair(types.NativeTokenSymbol, inAsset)]; ok {
// BNB_XYZ
market = engines[utils.Assets2TradingPair(types.NativeTokenSymbol, inAsset)]
amount = utils.CalBigNotional(market.LastTradePrice, feeAmount)
} else {
// for BUSD pairs, it is possible that there is no trading pair between BNB and inAsset, e.g., BUSD -> XYZ
if sdk.IsUpgrade(upgrade.BEP_BUSD) {
for _, symbol := range m.GovSupportedSymbols {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

var foundFirstPair, foundSecondPair bool
var intermediateAmount = big.NewInt(0)

if market, ok := engines[utils.Assets2TradingPair(symbol, types.NativeTokenSymbol)]; ok {
foundFirstPair = true
var tmp big.Int
intermediateAmount = tmp.Div(tmp.Mul(
big.NewInt(feeAmount),
big.NewInt(cmnUtils.Fixed8One.ToInt64())),
big.NewInt(market.LastTradePrice))
} else if market, ok := engines[utils.Assets2TradingPair(types.NativeTokenSymbol, symbol)]; ok {
foundFirstPair = true
intermediateAmount = utils.CalBigNotional(market.LastTradePrice, feeAmount)
}

if foundFirstPair {
var intermediateTmp int64
if intermediateAmount.IsInt64() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This conversion may be omitted since big.Int can be passed to below calculations

Copy link
Contributor Author

@forcodedancing forcodedancing Mar 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, i don't get your point. This sentence "amount = utils.CalBigNotional(market.LastTradePrice, intermediateTmp)" needs an int64.

intermediateTmp = intermediateAmount.Int64()
} else {
intermediateTmp = math.MaxInt64
}

if market, ok := engines[utils.Assets2TradingPair(inAsset, symbol)]; ok {
foundSecondPair = true
var tmp big.Int
amount = tmp.Div(tmp.Mul(
big.NewInt(intermediateTmp),
big.NewInt(cmnUtils.Fixed8One.ToInt64())),
big.NewInt(market.LastTradePrice))
} else if market, ok := engines[utils.Assets2TradingPair(symbol, inAsset)]; ok {
foundSecondPair = true
amount = utils.CalBigNotional(market.LastTradePrice, intermediateTmp)
}
}

if foundFirstPair && foundSecondPair {
break
}
}
}
}

if amount.IsInt64() {
Expand Down
70 changes: 70 additions & 0 deletions plugins/dex/order/fee_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package order
import (
"testing"

"github.com/binance-chain/node/common/upgrade"

"github.com/stretchr/testify/require"

sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down Expand Up @@ -286,3 +288,71 @@ func TestFeeManager_CalcFixedFee(t *testing.T) {
fee = keeper.FeeManager.CalcFixedFee(acc.GetCoins(), eventFullyExpire, "BTC-000", keeper.engines)
require.Equal(t, sdk.Coins{sdk.NewCoin("BTC-000", 1e13)}, fee.Tokens)
}

func TestFeeManager_calcTradeFeeForSingleTransfer_SupportBUSD(t *testing.T) {
upgrade.Mgr.AddUpgradeHeight(upgrade.BEP_BUSD, -1)

ctx, am, keeper := setup()
keeper.FeeManager.UpdateConfig(NewTestFeeConfig())
keeper.AddEngine(dextype.NewTradingPair("BNB", "BUSD-BD1", 1e5))
keeper.AddEngine(dextype.NewTradingPair("ABC-000", "BUSD-BD1", 1e7))
keeper.AddEngine(dextype.NewTradingPair("BUSD-BD1", "XYZ-999", 1e6))

// enough BNB, BNB will be collected
_, acc := testutils.NewAccount(ctx, am, 1e5)
tran := Transfer{
inAsset: "BUSD-BD1",
in: 1000,
outAsset: "ABC-000",
out: 100,
}
fee := keeper.FeeManager.calcTradeFeeForSingleTransfer(acc.GetCoins(), &tran, keeper.engines)
require.Equal(t, sdk.Coins{{"BNB", 5e2}}, fee.Tokens)

// transferred in ABC-000
tran = Transfer{
inAsset: "ABC-000",
in: 1000,
outAsset: "BUSD-BD1",
out: 100,
}
fee = keeper.FeeManager.calcTradeFeeForSingleTransfer(acc.GetCoins(), &tran, keeper.engines)
require.Equal(t, sdk.Coins{{"BNB", 5e1}}, fee.Tokens)

// transferred in XYZ-999
tran = Transfer{
inAsset: "XYZ-999",
in: 1000,
outAsset: "BUSD-BD1",
out: 100,
}
fee = keeper.FeeManager.calcTradeFeeForSingleTransfer(acc.GetCoins(), &tran, keeper.engines)
require.Equal(t, sdk.Coins{{"BNB", 5e4}}, fee.Tokens)
}

func TestFeeManager_CalcFixedFee_SupportBUSD(t *testing.T) {
upgrade.Mgr.AddUpgradeHeight(upgrade.BEP_BUSD, -1)

ctx, am, keeper := setup()
keeper.FeeManager.UpdateConfig(NewTestFeeConfig())
_, acc := testutils.NewAccount(ctx, am, 0)
keeper.AddEngine(dextype.NewTradingPair("BNB", "BUSD-BD1", 1e5))
keeper.AddEngine(dextype.NewTradingPair("ABC-000", "BUSD-BD1", 1e7))
keeper.AddEngine(dextype.NewTradingPair("BUSD-BD1", "XYZ-999", 1e6))

// no enough BNB, the transferred-in asset will be collected
// buy BUSD-BD1
acc.SetCoins(sdk.Coins{{Denom: "BUSD-BD1", Amount: 1e4}})
fee := keeper.FeeManager.CalcFixedFee(acc.GetCoins(), eventFullyExpire, "BUSD-BD1", keeper.engines)
require.Equal(t, sdk.Coins{sdk.NewCoin("BUSD-BD1", 1e2)}, fee.Tokens)

// buy ABC-000
acc.SetCoins(sdk.Coins{{Denom: "ABC-000", Amount: 1e4}})
fee = keeper.FeeManager.CalcFixedFee(acc.GetCoins(), eventFullyExpire, "ABC-000", keeper.engines)
require.Equal(t, sdk.Coins{sdk.NewCoin("ABC-000", 1e3)}, fee.Tokens)

// buy XYZ-999
acc.SetCoins(sdk.Coins{{Denom: "XYZ-999", Amount: 1e4}})
fee = keeper.FeeManager.CalcFixedFee(acc.GetCoins(), eventFullyExpire, "XYZ-999", keeper.engines)
require.Equal(t, sdk.Coins{sdk.NewCoin("XYZ-999", 1)}, fee.Tokens)
}
Loading