Merge pull request #504 from binance-chain/release/0.5.4

Release v0.5.4

CHANGELOG.md

@@ -1,5 +1,16 @@
 # Changelog
 
+## 0.5.4
+
+IMPROVEMENTS
+
+BUG FIXES
+
+* [\#502](https://github.com/binance-chain/node/pull/502) [MatchEngine] Fix order sequence in price level
+* [\#500](https://github.com/binance-chain/node/pull/500) [Publish] Failed blocking should not be regarded as closed order
+* [\#495](https://github.com/binance-chain/node/pull/500) [MatchEngine] Fully fill order might not be correctly removed in orderbook when two continuous orders fully filled.
+
+
 ## 0.5.1
 
 BREAKING CHANGES

Gopkg.toml

@@ -42,7 +42,7 @@
 [[constraint]]
   name = "github.com/cosmos/cosmos-sdk"
   source = "github.com/binance-chain/bnc-cosmos-sdk"
-  version = "=v0.25.0-binance.11"
+  version = "=v0.25.0-binance.13"
 
 [[constraint]]
   name = "github.com/btcsuite/btcd"

Makefile

@@ -1,3 +1,11 @@
+GOTOOLS = \
+	github.com/mitchellh/gox \
+	github.com/golang/dep/cmd/dep \
+	github.com/golangci/golangci-lint/cmd/golangci-lint \
+	github.com/gogo/protobuf/protoc-gen-gogo \
+	github.com/square/certstrap
+GOBIN?=${GOPATH}/bin
+
 PACKAGES=$(shell go list ./... | grep -v '/vendor/')
 COMMIT_HASH := $(shell git rev-parse --short HEAD)
 
@@ -30,7 +38,7 @@ ci: get_vendor_deps build
 ########################################
 ### Build
 
-build:
+build: get_tools
 ifeq ($(OS),Windows_NT)
 	go build $(BUILD_FLAGS) -o build/bnbcli.exe ./cmd/bnbcli
 	go build $(BUILD_TESTNET_FLAGS) -o build/tbnbcli.exe ./cmd/bnbcli
@@ -111,7 +119,29 @@ lint:
 ########################################
 ### Testing
 
-test: test_unit test_race
+get_tools:
+	@echo "--> Installing tools"
+	./scripts/get_tools.sh
+
+test:
+	make set_with_deadlock
+	make test_unit
+	make test_race
+	make cleanup_after_test_with_deadlock
+
+# uses https://github.com/sasha-s/go-deadlock/ to detect potential deadlocks
+set_with_deadlock:
+	find . -name "*.go" | grep -v "vendor/" | xargs -n 1 sed -i.mutex_bak 's/sync.RWMutex/deadlock.RWMutex/'
+	find . -name "*.go" | grep -v "vendor/" | xargs -n 1 sed -i.mutex_bak 's/sync.Mutex/deadlock.Mutex/'
+	find . -name "*.go" | grep -v "vendor/" | xargs -n 1 goimports -w
+
+# cleanes up after you ran test_with_deadlock
+cleanup_after_test_with_deadlock:
+	find . -name "*.go" | grep -v "vendor/" | xargs -n 1 sed -i.mutex_bak 's/deadlock.RWMutex/sync.RWMutex/'
+	find . -name "*.go" | grep -v "vendor/" | xargs -n 1 sed -i.mutex_bak 's/deadlock.Mutex/sync.Mutex/'
+	find . -name "*.go" | grep -v "vendor/" | xargs -n 1 goimports -w
+	find . -name "*.go.mutex_bak" | grep -v "vendor/" | xargs rm
+
 
 test_race:
 	@go test -race $(PACKAGES)

app/app_test.go

@@ -10,7 +10,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/auth"
 	"github.com/cosmos/cosmos-sdk/x/mock"
 	"github.com/stretchr/testify/require"
-	"github.com/tendermint/tendermint/abci/client"
+	abcicli "github.com/tendermint/tendermint/abci/client"
 	"github.com/tendermint/tendermint/abci/types"
 	abci "github.com/tendermint/tendermint/abci/types"
 	cfg "github.com/tendermint/tendermint/config"

app/helpers.go

@@ -159,26 +159,25 @@ func (app *BinanceChain) processErrAbciResponseForPub(txBytes []byte) {
 	tx, err := app.TxDecoder(txBytes)
 	txHash := cmn.HexBytes(tmhash.Sum(txBytes)).String()
 	if err != nil {
-		app.Logger.Error("failed to process invalid tx", "tx", txHash)
+		app.Logger.Info("failed to process invalid tx", "tx", txHash)
 	} else {
 		if msgs := tx.GetMsgs(); len(msgs) != 1 {
 			// The error message here should be consistent with vendor/github.com/cosmos/cosmos-sdk/baseapp/baseapp.go:537
 			app.Logger.Error("Tx.GetMsgs() must return exactly one message")
 		} else {
 			switch msg := msgs[0].(type) {
 			case order.NewOrderMsg:
-				app.Logger.Error("failed to process NewOrderMsg", "oid", msg.Id)
+				app.Logger.Info("failed to process NewOrderMsg", "oid", msg.Id)
 				// The error on deliver should be rare and only impact witness publisher's performance
 				app.DexKeeper.OrderChangesMtx.Lock()
-				app.DexKeeper.OrderInfosForPub[msg.Id] = &order.OrderInfo{NewOrderMsg: msg, TxHash: txHash}
-				app.DexKeeper.OrderChanges = append(app.DexKeeper.OrderChanges, order.OrderChange{msg.Id, order.FailedBlocking})
+				app.DexKeeper.OrderChanges = append(app.DexKeeper.OrderChanges, order.OrderChange{msg.Id, order.FailedBlocking, msg})
 				app.DexKeeper.OrderChangesMtx.Unlock()
 			case order.CancelOrderMsg:
-				app.Logger.Error("failed to process CancelOrderMsg", "oid", msg.RefId)
+				app.Logger.Info("failed to process CancelOrderMsg", "oid", msg.RefId)
 				// The error on deliver should be rare and only impact witness publisher's performance
 				app.DexKeeper.OrderChangesMtx.Lock()
 				// OrderInfo must has been in keeper.OrderInfosForPub
-				app.DexKeeper.OrderChanges = append(app.DexKeeper.OrderChanges, order.OrderChange{msg.RefId, order.FailedBlocking})
+				app.DexKeeper.OrderChanges = append(app.DexKeeper.OrderChanges, order.OrderChange{msg.RefId, order.FailedBlocking, msg})
 				app.DexKeeper.OrderChangesMtx.Unlock()
 			default:
 				// deliberately do nothing for message other than NewOrderMsg

app/pub/helpers.go

@@ -218,7 +218,7 @@ func updateExpireFeeForPublish(
 	defer wg.Done()
 	for tranHolder := range tranHolderCh {
 		Logger.Debug("transfer collector for order", "orderId", tranHolder.OrderId)
-		change := orderPkg.OrderChange{tranHolder.OrderId, reason}
+		change := orderPkg.OrderChange{tranHolder.OrderId, reason, nil}
 		dexKeeper.OrderChanges = append(dexKeeper.OrderChanges, change)
 	}
 }
@@ -333,9 +333,9 @@ func collectOrdersToPublish(
 	orderChanges orderPkg.OrderChanges,
 	orderInfos orderPkg.OrderInfoForPublish,
 	feeHolder orderPkg.FeeHolder,
-	timestamp int64) (opensToPublish []*Order, canceledToPublish []*Order, feeToPublish map[string]string) {
+	timestamp int64) (opensToPublish []*Order, closedToPublish []*Order, feeToPublish map[string]string) {
 	opensToPublish = make([]*Order, 0)
-	canceledToPublish = make([]*Order, 0)
+	closedToPublish = make([]*Order, 0)
 	// serve as a cache to avoid fee's serialization several times for one address
 	feeToPublish = make(map[string]string)
 
@@ -347,87 +347,87 @@ func collectOrdersToPublish(
 
 	// collect orders (new, cancel, ioc-no-fill, expire, failed-blocking and failed-matching) from orderChanges
 	for _, o := range orderChanges {
-		if orderInfo := orderInfos[o.Id]; orderInfo != nil {
+		if orderInfo := o.ResolveOrderInfo(orderInfos); orderInfo != nil {
 			orderToPublish := Order{
-				orderInfo.Symbol,
-				o.Tpe,
-				o.Id,
-				"",
-				orderInfo.Sender.String(),
-				orderInfo.Side,
-				orderPkg.OrderType.LIMIT,
-				orderInfo.Price,
-				orderInfo.Quantity,
-				0,
-				0,
-				orderInfo.CumQty,
-				"",
-				orderInfo.CreatedTimestamp,
-				timestamp,
-				orderInfo.TimeInForce,
-				orderPkg.NEW,
-				orderInfo.TxHash,
+				orderInfo.Symbol, o.Tpe, o.Id,
+				"", orderInfo.Sender.String(), orderInfo.Side,
+				orderPkg.OrderType.LIMIT, orderInfo.Price, orderInfo.Quantity,
+				0, 0, orderInfo.CumQty, "",
+				orderInfo.CreatedTimestamp, timestamp, orderInfo.TimeInForce,
+				orderPkg.NEW, orderInfo.TxHash,
 			}
 
-			if o.Tpe == orderPkg.Ack {
+			if o.Tpe.IsOpen() {
 				opensToPublish = append(opensToPublish, &orderToPublish)
 			} else {
-				if orderInfo.CumQty == 0 {
-					if o.Tpe == orderPkg.Canceled {
-						if _, ok := chargedCancels[string(orderInfo.Sender)]; ok {
-							chargedCancels[string(orderInfo.Sender)] += 1
-						} else {
-							chargedCancels[string(orderInfo.Sender)] = 1
-						}
-					} else {
-						if _, ok := chargedExpires[string(orderInfo.Sender)]; ok {
-							chargedExpires[string(orderInfo.Sender)] += 1
-						} else {
-							chargedExpires[string(orderInfo.Sender)] = 1
-						}
-					}
+				closedToPublish = append(closedToPublish, &orderToPublish)
+			}
+
+			// fee field handling
+			if orderToPublish.isChargedCancel() {
+				if _, ok := chargedCancels[string(orderInfo.Sender)]; ok {
+					chargedCancels[string(orderInfo.Sender)] += 1
+				} else {
+					chargedCancels[string(orderInfo.Sender)] = 1
+				}
+			} else if orderToPublish.isChargedExpire() {
+				if _, ok := chargedExpires[string(orderInfo.Sender)]; ok {
+					chargedExpires[string(orderInfo.Sender)] += 1
+				} else {
+					chargedExpires[string(orderInfo.Sender)] = 1
 				}
-				canceledToPublish = append(canceledToPublish, &orderToPublish)
 			}
 		} else {
 			Logger.Error("failed to locate order change in OrderChangesMap", "orderChange", o.String())
 		}
 	}
 
 	// update C and E fields in serialized fee string
-	for _, order := range canceledToPublish {
-		senderStr := string(orderInfos[order.OrderId].Sender)
-		if _, ok := feeToPublish[senderStr]; !ok {
-			numOfChargedCanceled := chargedCancels[senderStr]
-			numOfExpiredCanceled := chargedExpires[senderStr]
-			if raw, ok := feeHolder[senderStr]; ok {
-				fee := raw.SerializeForPub(numOfChargedCanceled, numOfExpiredCanceled)
-				feeToPublish[senderStr] = fee
-				order.Fee = fee
-			} else {
-				// TODO(#192): handle cancel fee is not included within feeHolder
+	for _, order := range closedToPublish {
+		if orderInfo, ok := orderInfos[order.OrderId]; ok {
+			senderBytesStr := string(orderInfo.Sender)
+			if _, ok := feeToPublish[senderBytesStr]; !ok {
+				numOfChargedCanceled := chargedCancels[senderBytesStr]
+				numOfExpiredCanceled := chargedExpires[senderBytesStr]
+				if raw, ok := feeHolder[senderBytesStr]; ok {
+					fee := raw.SerializeForPub(numOfChargedCanceled, numOfExpiredCanceled)
+					feeToPublish[senderBytesStr] = fee
+					order.Fee = fee
+				} else {
+					Logger.Error("cannot find fee for cancel/expire", "sender", order.Owner)
+				}
 			}
+		} else {
+			Logger.Error("should not to locate order in OrderChangesMap", "oid", order.OrderId)
 		}
 	}
 
 	// update fee and collect orders from trades
 	for _, t := range trades {
 		if o, exists := orderInfos[t.Bid]; exists {
 			orderToPublish := tradeToOrder(t, o, timestamp, feeHolder, feeToPublish)
-			opensToPublish = append(opensToPublish, &orderToPublish)
+			if orderToPublish.Status.IsOpen() {
+				opensToPublish = append(opensToPublish, &orderToPublish)
+			} else {
+				closedToPublish = append(closedToPublish, &orderToPublish)
+			}
 		} else {
 			Logger.Error("failed to resolve order information from orderInfos", "orderId", t.Bid)
 		}
 
 		if o, exists := orderInfos[t.Sid]; exists {
 			orderToPublish := tradeToOrder(t, o, timestamp, feeHolder, feeToPublish)
-			opensToPublish = append(opensToPublish, &orderToPublish)
+			if orderToPublish.Status.IsOpen() {
+				opensToPublish = append(opensToPublish, &orderToPublish)
+			} else {
+				closedToPublish = append(closedToPublish, &orderToPublish)
+			}
 		} else {
 			Logger.Error("failed to resolve order information from orderInfos", "orderId", t.Sid)
 		}
 	}
 
-	return opensToPublish, canceledToPublish, feeToPublish
+	return opensToPublish, closedToPublish, feeToPublish
 }
 
 func getSerializedFeeForOrder(orderInfo *orderPkg.OrderInfo, status orderPkg.ChangeType, feeHolder orderPkg.FeeHolder, feeToPublish map[string]string) string {

app/pub/msgs.go

@@ -189,8 +189,10 @@ func (msg *Order) effectQtyToOrderBook() int64 {
 		return msg.Qty
 	case orderPkg.FullyFill, orderPkg.PartialFill:
 		return -msg.LastExecutedQty
-	case orderPkg.Expired, orderPkg.IocNoFill, orderPkg.Canceled:
+	case orderPkg.Expired, orderPkg.IocNoFill, orderPkg.Canceled, orderPkg.FailedMatching:
 		return msg.CumQty - msg.Qty // deliberated be negative value
+	case orderPkg.FailedBlocking:
+		return 0
 	default:
 		Logger.Error("does not supported order status", "order", msg.String())
 		return 0
@@ -220,6 +222,14 @@ func (msg *Order) toNativeMap() map[string]interface{} {
 	return native
 }
 
+func (msg Order) isChargedCancel() bool {
+	return msg.CumQty == 0 && msg.Status == orderPkg.Canceled
+}
+
+func (msg Order) isChargedExpire() bool {
+	return msg.CumQty == 0 && (msg.Status == orderPkg.IocNoFill || msg.Status == orderPkg.Expired)
+}
+
 type Proposals struct {
 	NumOfMsgs int
 	Proposals []*Proposal

app/pub/publisher.go

@@ -49,26 +49,16 @@ func Publish(
 			// DEX query service team relies on the fact that we publish orders before trades so that
 			// they can assign buyer/seller address into trade before persist into DB
 			var opensToPublish []*Order
-			var canceledToPublish []*Order
+			var closedToPublish []*Order
 			var feeToPublish map[string]string
 			if cfg.PublishOrderUpdates || cfg.PublishOrderBook {
-				opensToPublish, canceledToPublish, feeToPublish = collectOrdersToPublish(
+				opensToPublish, closedToPublish, feeToPublish = collectOrdersToPublish(
 					marketData.tradesToPublish,
 					marketData.orderChanges,
 					marketData.orderInfos,
 					marketData.feeHolder,
 					marketData.timestamp)
-				for _, o := range opensToPublish {
-					if o.Status == orderPkg.FullyFill {
-						if ToRemoveOrderIdCh != nil {
-							Logger.Debug(
-								"going to delete fully filled order from order changes map",
-								"orderId", o.OrderId)
-							ToRemoveOrderIdCh <- o.OrderId
-						}
-					}
-				}
-				for _, o := range canceledToPublish {
+				for _, o := range closedToPublish {
 					if ToRemoveOrderIdCh != nil {
 						Logger.Debug(
 							"going to delete order from order changes map",
@@ -84,7 +74,7 @@ func Publish(
 				close(ToRemoveOrderIdCh)
 			}
 
-			ordersToPublish := append(opensToPublish, canceledToPublish...)
+			ordersToPublish := append(opensToPublish, closedToPublish...)
 			if cfg.PublishOrderUpdates {
 				duration := Timer(Logger, "publish all orders", func() {
 					publishExecutionResult(

app/pub/publisher_kafka.go

@@ -8,7 +8,7 @@ import (
 	"time"
 
 	"github.com/Shopify/sarama"
-	"github.com/deathowl/go-metrics-prometheus"
+	prometheusmetrics "github.com/deathowl/go-metrics-prometheus"
 	"github.com/eapache/go-resiliency/breaker"
 	"github.com/linkedin/goavro"
 	"github.com/prometheus/client_golang/prometheus"

cmd/bnbchaind/init/init.go

@@ -133,7 +133,7 @@ enabled, and the genesis file will not be generated.
 func prepareCreateValidatorTx(cdc *codec.Codec, chainId, name, memo string,
 	valOperAddr sdk.ValAddress, valPubKey crypto.PubKey) json.RawMessage {
 	msg := stake.MsgCreateValidatorProposal{
-		MsgCreateValidator:stake.NewMsgCreateValidator(
+		MsgCreateValidator: stake.NewMsgCreateValidator(
 			valOperAddr,
 			valPubKey,
 			app.DefaultSelfDelegationToken,

cmd/dexperf/main.go

@@ -7,7 +7,6 @@ import (
 	"encoding/json"
 	"flag"
 	"fmt"
-	"go.uber.org/ratelimit"
 	"io/ioutil"
 	"math/rand"
 	"os"
@@ -21,6 +20,8 @@ import (
 	"sync"
 	"time"
 
+	"go.uber.org/ratelimit"
+
 	"github.com/cosmos/cosmos-sdk/client/context"
 	"github.com/cosmos/cosmos-sdk/client/keys"
 	sdk "github.com/cosmos/cosmos-sdk/types"