[R4R] add lint workflow (#858)

.github/workflows/golangci-lint.yml

@@ -0,0 +1,31 @@
+name: golangci-lint
+
+on:
+  push:
+  pull_request:
+  workflow_dispatch:
+jobs:
+  golangci:
+    name: lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/setup-go@v3
+        with:
+          go-version: 1.16
+      - uses: actions/checkout@v3
+      - uses: actions/cache@v2
+        with:
+          path: ~/go/bin/golangci-lint
+          key: golangci-lint-1.45.2
+      - uses: actions/cache@v2
+        with:
+          path: |
+            ~/go/pkg/mod
+            ~/.cache/go-build
+          key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
+          restore-keys: |
+            ${{ runner.os }}-go-
+      - run: git config --global url."https://${{ secrets.GH_ACCESS_TOKEN }}@github.com".insteadOf "https://github.com"
+      - run: go env -w GOPRIVATE="github.com/bnb-chain/*"
+      - name: golangci-lint
+        run: make lint

.github/workflows/integration.yml

@@ -15,6 +15,10 @@ jobs:
           go-version: 1.16
       - name: Checkout
         uses: actions/checkout@v2
+      - uses: actions/cache@v2
+        with:
+          path: ~/go/bin
+          key: tools-v0
       - uses: actions/cache@v2
         with:
           path: |

.golangci.yml

@@ -0,0 +1,57 @@
+# Options for analysis running.
+run:
+  # The default concurrency value is the number of available CPU.
+  # concurrency: 4
+  # Timeout for analysis, e.g. 30s, 5m.
+  # Default: 1m
+  timeout: 5m
+  # Exit code when at least one issue was found.
+  # Default: 1
+  # issues-exit-code: 2
+  # Include test files or not.
+  # Default: true
+  tests: false
+  # List of build tags, all linters use it.
+  # Default: [].
+#  build-tags:
+#    - mytag
+  # Which dirs to skip: issues from them won't be reported.
+  # Can use regexp here: `generated.*`, regexp is applied on full path.
+  # Default value is empty list,
+  # but default dirs are skipped independently of this option's value (see skip-dirs-use-default).
+  # "/" will be replaced by current OS file path separator to properly work on Windows.
+#  skip-dirs:
+#    - src/external_libs
+#    - autogenerated_by_my_lib
+  # Enables skipping of directories:
+  # - vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
+  # Default: true
+#  skip-dirs-use-default: false
+  # Which files to skip: they will be analyzed, but issues from them won't be reported.
+  # Default value is empty list,
+  # but there is no need to include all autogenerated files,
+  # we confidently recognize autogenerated files.
+  # If it's not please let us know.
+  # "/" will be replaced by current OS file path separator to properly work on Windows.
+#  skip-files:
+#    - ".*\\.my\\.go$"
+#    - lib/bad.go
+  # If set we pass it to "go list -mod={option}". From "go help modules":
+  # If invoked with -mod=readonly, the go command is disallowed from the implicit
+  # automatic updating of go.mod described above. Instead, it fails when any changes
+  # to go.mod are needed. This setting is most useful to check that go.mod does
+  # not need updates, such as in a continuous integration and testing system.
+  # If invoked with -mod=vendor, the go command assumes that the vendor
+  # directory holds the correct copies of dependencies and ignores
+  # the dependency descriptions in go.mod.
+  #
+  # Allowed values: readonly|vendor|mod
+  # By default, it isn't set.
+#  modules-download-mode: readonly
+  # Allow multiple parallel golangci-lint instances running.
+  # If false (default) - golangci-lint acquires file lock on start.
+#  allow-parallel-runners: false
+  # Define the Go version limit.
+  # Mainly related to generics support in go1.18.
+  # Default: use Go version from the go.mod file, fallback on the env var `GOVERSION`, fallback on 1.17
+#  go: '1.16'

Makefile

@@ -144,9 +144,13 @@ format:
 
 ########################################
 ### Lint
-lint:
+install_lint:
+	which golangci-lint || go install github.com/golangci/golangci-lint/cmd/[email protected]
+	golangci-lint --version
+
+lint: install_lint
 	@echo "-->Lint"
-	golint $(PACKAGES)
+	golangci-lint run
 
 ########################################
 ### Testing
@@ -167,13 +171,13 @@ set_with_deadlock:
 	cp go.sum go.sum_bak
 	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
+	find . -name "*.go" | grep -v "vendor/" | grep -v ".git/"  | 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" | grep -v "vendor/" | grep -v ".git/" | xargs -n 1 goimports -w
 	find . -name "*.go.mutex_bak" | grep -v "vendor/" | xargs rm
 	mv go.mod_bak go.mod
 	mv go.sum_bak go.sum
@@ -193,7 +197,7 @@ integration_test: build
 
 ########################################
 ### Pre Commit
-pre_commit: build test format
+pre_commit: build test format lint
 
 ########################################
 ### Local validator nodes using docker and docker-compose
@@ -227,3 +231,4 @@ localnet-stop:
 # unless there is a reason not to.
 # https://www.gnu.org/software/make/manual/html_node/Phony-Targets.html
 .PHONY: build install test test_unit build-linux build-docker-node localnet-start localnet-stop
+.PHONY: lint install_lint

app/app.go

@@ -135,7 +135,7 @@ func NewBinanceChain(logger log.Logger, db dbm.DB, traceStore io.Writer, baseApp
 
 	// create the applicationsimulate object
 	var app = &BinanceChain{
-		BaseApp:            baseapp.NewBaseApp(appName /*, cdc*/, logger, db, decoders, sdk.CollectConfig{ServerContext.PublishAccountBalance, ServerContext.PublishTransfer || ServerContext.PublishBlock}, baseAppOptions...),
+		BaseApp:            baseapp.NewBaseApp(appName /*, cdc*/, logger, db, decoders, sdk.CollectConfig{CollectAccountBalance: ServerContext.PublishAccountBalance, CollectTxs: ServerContext.PublishTransfer || ServerContext.PublishBlock}, baseAppOptions...),
 		Codec:              cdc,
 		queryHandlers:      make(map[string]types.AbciQueryHandler),
 		baseConfig:         ServerContext.BaseConfig,
@@ -839,8 +839,7 @@ func (app *BinanceChain) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) a
 
 	if app.publicationConfig.ShouldPublishAny() &&
 		pub.IsLive {
-		var stakeUpdates pub.StakeUpdates
-		stakeUpdates = pub.CollectStakeUpdatesForPublish(completedUbd)
+		stakeUpdates := pub.CollectStakeUpdatesForPublish(completedUbd)
 		if height >= app.publicationConfig.FromHeightInclusive {
 			app.publish(tradesToPublish, &proposals, &sideProposals, &stakeUpdates, blockFee, ctx, height, blockTime.UnixNano())
 
@@ -961,7 +960,7 @@ func (app *BinanceChain) AccountHandler(chainApp types.ChainApp, req abci.Reques
 				// let api server return 204 No Content
 				res = abci.ResponseQuery{
 					Code:  uint32(sdk.ABCICodeOK),
-					Value: make([]byte, 0, 0),
+					Value: make([]byte, 0),
 				}
 			}
 		} else {

app/fee_distribution.go

@@ -69,7 +69,7 @@ func distributeFee(ctx sdk.Context, am auth.AccountKeeper, valAddrCache *ValAddr
 	if fee.Type == sdk.FeeForProposer {
 		// The proposer's account must be initialized before it becomes a proposer.
 		proposerAcc := am.GetAccount(ctx, proposerAccAddr)
-		proposerAcc.SetCoins(proposerAcc.GetCoins().Plus(fee.Tokens))
+		_ = proposerAcc.SetCoins(proposerAcc.GetCoins().Plus(fee.Tokens))
 		am.SetAccount(ctx, proposerAcc)
 	} else if fee.Type == sdk.FeeForAll {
 		log.Info("Distributing the fees to all the validators",
@@ -91,7 +91,7 @@ func distributeFee(ctx sdk.Context, am auth.AccountKeeper, valAddrCache *ValAddr
 
 		if avgTokens.IsZero() {
 			proposerAcc := am.GetAccount(ctx, proposerAccAddr)
-			proposerAcc.SetCoins(proposerAcc.GetCoins().Plus(fee.Tokens))
+			_ = proposerAcc.SetCoins(proposerAcc.GetCoins().Plus(fee.Tokens))
 			am.SetAccount(ctx, proposerAcc)
 		} else {
 			for _, voteInfo := range voteInfos {
@@ -100,12 +100,12 @@ func distributeFee(ctx sdk.Context, am auth.AccountKeeper, valAddrCache *ValAddr
 				validatorAcc := am.GetAccount(ctx, accAddr)
 				if bytes.Equal(proposerValAddr, validator.Address) {
 					if !roundingTokens.IsZero() {
-						validatorAcc.SetCoins(validatorAcc.GetCoins().Plus(roundingTokens))
+						_ = validatorAcc.SetCoins(validatorAcc.GetCoins().Plus(roundingTokens))
 					}
 				} else if publishBlockFee {
 					validators = append(validators, string(accAddr))
 				}
-				validatorAcc.SetCoins(validatorAcc.GetCoins().Plus(avgTokens))
+				_ = validatorAcc.SetCoins(validatorAcc.GetCoins().Plus(avgTokens))
 				am.SetAccount(ctx, validatorAcc)
 			}
 		}

app/helpers.go

@@ -144,12 +144,12 @@ func (app *BinanceChain) processErrAbciResponseForPub(txBytes []byte) {
 			case order.NewOrderMsg:
 				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.UpdateOrderChangeSync(order.OrderChange{msg.Id, order.FailedBlocking, "", msg}, msg.Symbol)
+				app.DexKeeper.UpdateOrderChangeSync(order.OrderChange{Id: msg.Id, Tpe: order.FailedBlocking, MsgForFailedTx: msg}, msg.Symbol)
 			case order.CancelOrderMsg:
 				app.Logger.Info("failed to process CancelOrderMsg", "oid", msg.RefId)
 				// The error on deliver should be rare and only impact witness publisher's performance
 				// OrderInfo must has been in keeper.orderInfosForPub
-				app.DexKeeper.UpdateOrderChangeSync(order.OrderChange{msg.RefId, order.FailedBlocking, "", msg}, msg.Symbol)
+				app.DexKeeper.UpdateOrderChangeSync(order.OrderChange{Id: msg.RefId, Tpe: order.FailedBlocking, MsgForFailedTx: msg}, msg.Symbol)
 			default:
 				// deliberately do nothing for message other than NewOrderMsg
 				// in future, we may publish fail status of send msg

app/pub/helpers.go

@@ -55,7 +55,7 @@ func GetTradeAndOrdersRelatedAccounts(tradesToPublish []*Trade, orderChanges ord
 
 func GetBlockPublished(pool *sdk.Pool, header abci.Header, blockHash []byte) *Block {
 	txs := pool.GetTxs()
-	transactionsToPublish := make([]Transaction, 0, 0)
+	transactionsToPublish := make([]Transaction, 0)
 	timeStamp := header.GetTime().Format(time.RFC3339Nano)
 	txs.Range(func(key, value interface{}) bool {
 		txhash := key.(string)
@@ -181,7 +181,7 @@ func GetBlockPublished(pool *sdk.Pool, header abci.Header, blockHash []byte) *Bl
 }
 
 func GetTransferPublished(pool *sdk.Pool, height, blockTime int64) *Transfers {
-	transferToPublish := make([]Transfer, 0, 0)
+	transferToPublish := make([]Transfer, 0)
 	txs := pool.GetTxs()
 	txs.Range(func(key, value interface{}) bool {
 		txhash := key.(string)
@@ -281,9 +281,19 @@ func MatchAndAllocateAllForPublish(dexKeeper *orderPkg.DexKeeper, ctx sdk.Contex
 		if tran.IsExpire() {
 			if tran.IsExpiredWithFee() {
 				// we only got expire of Ioc here, gte orders expire is handled in breathe block
-				iocExpireFeeHolderCh <- orderPkg.ExpireHolder{tran.Oid, orderPkg.IocNoFill, tran.Fee.String(), tran.Symbol}
+				iocExpireFeeHolderCh <- orderPkg.ExpireHolder{
+					OrderId: tran.Oid,
+					Reason:  orderPkg.IocNoFill,
+					Fee:     tran.Fee.String(),
+					Symbol:  tran.Symbol,
+				}
 			} else {
-				iocExpireFeeHolderCh <- orderPkg.ExpireHolder{tran.Oid, orderPkg.IocExpire, tran.Fee.String(), tran.Symbol}
+				iocExpireFeeHolderCh <- orderPkg.ExpireHolder{
+					OrderId: tran.Oid,
+					Reason:  orderPkg.IocExpire,
+					Fee:     tran.Fee.String(),
+					Symbol:  tran.Symbol,
+				}
 			}
 		}
 	}
@@ -343,13 +353,12 @@ func ExpireOrdersForPublish(
 	go updateExpireFeeForPublish(dexKeeper, &wg, expireHolderCh)
 	var collectorForExpires = func(tran orderPkg.Transfer) {
 		if tran.IsExpire() {
-			expireHolderCh <- orderPkg.ExpireHolder{tran.Oid, orderPkg.Expired, tran.Fee.String(), tran.Symbol}
+			expireHolderCh <- orderPkg.ExpireHolder{OrderId: tran.Oid, Reason: orderPkg.Expired, Fee: tran.Fee.String(), Symbol: tran.Symbol}
 		}
 	}
 	dexKeeper.ExpireOrders(ctx, blockTime, collectorForExpires)
 	close(expireHolderCh)
 	wg.Wait()
-	return
 }
 
 func DelistTradingPairForPublish(ctx sdk.Context, dexKeeper *orderPkg.DexKeeper, symbol string) {
@@ -359,13 +368,17 @@ func DelistTradingPairForPublish(ctx sdk.Context, dexKeeper *orderPkg.DexKeeper,
 	go updateExpireFeeForPublish(dexKeeper, &wg, expireHolderCh)
 	var collectorForExpires = func(tran orderPkg.Transfer) {
 		if tran.IsExpire() {
-			expireHolderCh <- orderPkg.ExpireHolder{tran.Oid, orderPkg.Expired, tran.Fee.String(), tran.Symbol}
+			expireHolderCh <- orderPkg.ExpireHolder{
+				OrderId: tran.Oid,
+				Reason:  orderPkg.Expired,
+				Fee:     tran.Fee.String(),
+				Symbol:  tran.Symbol,
+			}
 		}
 	}
 	dexKeeper.DelistTradingPair(ctx, symbol, collectorForExpires)
 	close(expireHolderCh)
 	wg.Wait()
-	return
 }
 
 func CollectProposalsForPublish(passed, failed []gov.SimpleProposal) (Proposals, SideProposals) {
@@ -405,7 +418,7 @@ func updateExpireFeeForPublish(
 	defer wg.Done()
 	for expHolder := range expHolderCh {
 		Logger.Debug("transfer collector for order", "orderId", expHolder.OrderId)
-		change := orderPkg.OrderChange{expHolder.OrderId, expHolder.Reason, expHolder.Fee, nil}
+		change := orderPkg.OrderChange{Id: expHolder.OrderId, Tpe: expHolder.Reason, SingleFee: expHolder.Fee}
 		dexKeeper.UpdateOrderChangeSync(change, expHolder.Symbol)
 	}
 }
@@ -428,7 +441,7 @@ func filterChangedOrderBooksByOrders(
 		}
 		allSymbols[symbol] = struct{}{}
 		if _, ok := res[symbol]; !ok {
-			res[symbol] = orderPkg.ChangedPriceLevelsPerSymbol{make(map[int64]int64), make(map[int64]int64)}
+			res[symbol] = orderPkg.ChangedPriceLevelsPerSymbol{Buys: make(map[int64]int64), Sells: make(map[int64]int64)}
 			buyQtyDiff[symbol] = make(map[int64]int64)
 			sellQtyDiff[symbol] = make(map[int64]int64)
 		}