Enable more linters (#861)

* Enable all default linters and fixed all errors

* Enable two more linters

* Added linter for `gofmt`

* Enables the `unparam` linter

* Added `make check`

* Fixed `deadcode` errors after rebase

* Fixed `misspell` errors after rebase

* Fixed `ineffassign` errors after rebase

* Fixed `unparam` errors after rebase

* Fixed `gosimple` errors after rebase

* Fixed `staticcheck` errors after rebase

* Addressed review feedback from @recmo

.golangci.yml

@@ -1,5 +1,11 @@
 linters:
-  disable-all: true
+  disable-all: false
   enable:
-    - errcheck
-    - govet
+      # Finds repeated strings that could be made constants
+      - goconst
+      # Ensures that the code was formatted with `gofmt -s`
+      - gofmt
+      # Identifies commonly misspelled words
+      - misspell
+      # Identifies unused function parameters
+      - unparam

Makefile

@@ -36,6 +36,11 @@ deps-ts-no-lockfile:
 	yarn install --frozen-lockfile
 
 
+# Provides a pre-commit convenience command that runs all of the tests and the linters
+.PHONY: check
+check: test-all lint
+
+
 .PHONY: test-all
 test-all: test-go test-wasm-browser test-ts test-browser-conversion test-browser-integration
 

cmd/cut-release/main.go

@@ -92,6 +92,12 @@ func generateTypescriptDocs() {
 	}
 }
 
+const (
+	captureVersionString                = `"version": "(.*)"`
+	captureMeshBrowserVersionString     = `"@0x/mesh-browser": "(.*)"`
+	captureMeshBrowserLiteVersionString = `"@0x/mesh-browser-lite": "(.*)"`
+)
+
 // Update the version string in all files that must be updated for a new release
 func updateHardCodedVersions(version string) {
 	newVersionString := fmt.Sprintf(`"version": "%s"`, version)
@@ -100,68 +106,56 @@ func updateHardCodedVersions(version string) {
 
 	// Update `packages/mesh-graphql-client/package.json`
 	tsClientPackageJSONPath := "packages/mesh-graphql-client/package.json"
-	regex := `"version": "(.*)"`
-	updateFileWithRegex(tsClientPackageJSONPath, regex, newVersionString)
+	updateFileWithRegex(tsClientPackageJSONPath, captureVersionString, newVersionString)
 
 	// Update `packages/mesh-browser-lite/package.json`
 	browserLitePackageJSONPath := "packages/mesh-browser-lite/package.json"
-	regex = `"version": "(.*)"`
-	updateFileWithRegex(browserLitePackageJSONPath, regex, newVersionString)
+	updateFileWithRegex(browserLitePackageJSONPath, captureVersionString, newVersionString)
 
 	// Update `packages/mesh-browser/package.json`
 	browserPackageJSONPath := "packages/mesh-browser/package.json"
-	regex = `"version": "(.*)"`
-	updateFileWithRegex(browserPackageJSONPath, regex, newVersionString)
+	updateFileWithRegex(browserPackageJSONPath, captureVersionString, newVersionString)
 	// NOTE(jalextowle): `@0x/mesh-browser` uses the local version of `@0x/mesh-browser-lite`
 	// on the `development` branch. Once the `@0x/mesh-browser-lite` package has been published,
 	// we need to update dependency in `@0x/mesh-browser` to published version.
-	regex = `"@0x/mesh-browser-lite": "(.*)"`
-	updateFileWithRegex(browserPackageJSONPath, regex, newBrowserLiteDependencyString)
+	updateFileWithRegex(browserPackageJSONPath, captureMeshBrowserLiteVersionString, newBrowserLiteDependencyString)
 
 	// Update `packages/mesh-webpack-example-lite/package.json`
 	webpackExampleLitePackageJSONPath := "packages/mesh-webpack-example-lite/package.json"
-	regex = `"@0x/mesh-browser-lite": "(.*)"`
-	updateFileWithRegex(webpackExampleLitePackageJSONPath, regex, newBrowserLiteDependencyString)
+	updateFileWithRegex(webpackExampleLitePackageJSONPath, captureMeshBrowserLiteVersionString, newBrowserLiteDependencyString)
 
 	// Update `packages/mesh-webpack-example/package.json`
 	webpackExamplePackageJSONPath := "packages/mesh-webpack-example/package.json"
-	regex = `"@0x/mesh-browser": "(.*)"`
-	updateFileWithRegex(webpackExamplePackageJSONPath, regex, newBrowserDependencyString)
+	updateFileWithRegex(webpackExamplePackageJSONPath, captureMeshBrowserVersionString, newBrowserDependencyString)
 
 	// Update `packages/mesh-integration-tests/package.json`
 	integrationTestsPackageJSONPath := "packages/mesh-integration-tests/package.json"
-	regex = `"@0x/mesh-browser": "(.*)"`
-	updateFileWithRegex(integrationTestsPackageJSONPath, regex, newBrowserDependencyString)
+	updateFileWithRegex(integrationTestsPackageJSONPath, captureMeshBrowserVersionString, newBrowserDependencyString)
 
 	// Update `packages/mesh-browser-shim/package.json`
 	testWasmPackageJSONPath := "packages/mesh-browser-shim/package.json"
-	regex = `"@0x/mesh-browser-lite": "(.*)"`
-	updateFileWithRegex(testWasmPackageJSONPath, regex, newBrowserLiteDependencyString)
+	updateFileWithRegex(testWasmPackageJSONPath, captureMeshBrowserLiteVersionString, newBrowserLiteDependencyString)
 
 	// Update `core.go`
 	corePath := "core/core.go"
 	newVersionString = fmt.Sprintf(`version$1= "%s"`, version)
-	regex = `version(.*)= "(.*)"`
-	updateFileWithRegex(corePath, regex, newVersionString)
+	updateFileWithRegex(corePath, `version(.*)= "(.*)"`, newVersionString)
 
 	// Update `docs/deployment_with_telemetry.md`
 	newVersionString = fmt.Sprintf(`image: 0xorg/mesh:%s`, version)
-	regex = `image: 0xorg/mesh:[0-9.]+.*`
-	updateFileWithRegex("docs/deployment_with_telemetry.md", regex, newVersionString)
+	updateFileWithRegex("docs/deployment_with_telemetry.md", `image: 0xorg/mesh:[0-9.]+.*`, newVersionString)
 
 	// Update `CHANGELOG.md`
 	changelog := "CHANGELOG.md"
 	newChangelogSection := fmt.Sprintf(`## v%s`, version)
-	regex = `(## Upcoming release)`
-	updateFileWithRegex(changelog, regex, newChangelogSection)
+	updateFileWithRegex(changelog, `(## Upcoming release)`, newChangelogSection)
 
 	// Update badge in README.md
 	pathToMDFilesWithBadges := []string{"README.md", "docs/graphql_api.md", "docs/deployment.md", "docs/deployment_with_telemetry.md"}
 	doubleDashVersion := strings.Replace(version, "-", "--", -1)
 	newSvgName := fmt.Sprintf("version-%s-orange.svg", doubleDashVersion)
-	regex = `version-(.*)-orange.svg`
 	for _, path := range pathToMDFilesWithBadges {
-		updateFileWithRegex(path, regex, newSvgName)
+		updateFileWithRegex(path, `version-(.*)-orange.svg`, newSvgName)
 	}
 }
 
@@ -194,19 +188,3 @@ func getDocsCommitHash(docsPath string) (string, error) {
 	}
 	return matches[1], nil
 }
-
-func getFileContentsWithRegex(filePath string, regex string) (string, error) {
-	dat, err := ioutil.ReadFile(filePath)
-	if err != nil {
-		log.Fatal(err)
-	}
-
-	var re = regexp.MustCompile(regex)
-	matches := re.FindAllStringSubmatch(string(dat), -1)
-
-	if len(matches) < 1 || len(matches[0]) < 3 {
-		return "", errors.New("No contents found")
-	}
-
-	return matches[0][2], nil
-}

cmd/mesh-bootstrap/main.go

@@ -8,14 +8,12 @@ package main
 import (
 	"context"
 	"fmt"
-	mathrand "math/rand"
 	"strings"
 	"time"
 
 	"github.com/0xProject/0x-mesh/loghooks"
 	"github.com/0xProject/0x-mesh/p2p"
 	"github.com/0xProject/0x-mesh/p2p/banner"
-	"github.com/ipfs/go-datastore"
 	leveldbStore "github.com/ipfs/go-ds-leveldb"
 	libp2p "github.com/libp2p/go-libp2p"
 	autonat "github.com/libp2p/go-libp2p-autonat-svc"
@@ -40,23 +38,6 @@ const (
 	// peerGraceDuration is the amount of time a newly opened connection is given
 	// before it becomes subject to pruning.
 	peerGraceDuration = 10 * time.Second
-	// defaultNetworkTimeout is the default timeout for network requests (e.g.
-	// connecting to a new peer).
-	defaultNetworkTimeout = 30 * time.Second
-	// checkBandwidthLoopInterval is how often to potentially check bandwidth usage
-	// for peers.
-	checkBandwidthLoopInterval = 5 * time.Second
-	// chanceToCheckBandwidthUsage is the approximate ratio of (number of check
-	// bandwidth loop iterations in which we check bandwidth usage) to (total
-	// number of check bandwidth loop iterations). We check bandwidth
-	// non-deterministically in order to prevent spammers from avoiding detection
-	// by carefully timing their bandwidth usage. So on each iteration of the
-	// check bandwidth loop we generate a number between 0 and 1. If its less than
-	// chanceToCheckBandiwdthUsage, we perform a bandwidth check.
-	chanceToCheckBandwidthUsage = 0.1
-	// DataStoreType constants
-	leveldbDataStore = "leveldb"
-	sqlDataStore     = "sqldb"
 )
 
 // Config contains configuration options for a Node.
@@ -135,9 +116,7 @@ func main() {
 	// We need to declare the newDHT function ahead of time so we can use it in
 	// the libp2p.Routing option.
 	var kadDHT *dht.IpfsDHT
-	var newDHT func(h host.Host) (routing.PeerRouting, error)
-
-	newDHT = func(h host.Host) (routing.PeerRouting, error) {
+	newDHT := func(h host.Host) (routing.PeerRouting, error) {
 		var err error
 		dhtDir := getDHTDir(config)
 		// Set up the DHT to use LevelDB.
@@ -309,24 +288,3 @@ func parseAddrs(commaSeparatedAddrs string) ([]ma.Multiaddr, error) {
 	}
 	return maddrs, nil
 }
-
-func continuoslyCheckBandwidth(ctx context.Context, banner *banner.Banner) error {
-	ticker := time.NewTicker(checkBandwidthLoopInterval)
-	for {
-		select {
-		case <-ctx.Done():
-			return nil
-		case <-ticker.C:
-			// Check bandwidth usage non-deterministically
-			if mathrand.Float64() <= chanceToCheckBandwidthUsage {
-				banner.CheckBandwidthUsage()
-			}
-		}
-	}
-}
-
-// NewDHTWithDatastore returns a new Kademlia DHT instance configured with store
-// as the persistant storage interface.
-func NewDHTWithDatastore(ctx context.Context, store datastore.Batching, host host.Host) (*dht.IpfsDHT, error) {
-	return dht.New(ctx, host, dhtopts.Datastore(store), dhtopts.Protocols(p2p.DHTProtocolID))
-}

cmd/mesh-bootstrap/storage.go

@@ -13,11 +13,6 @@ import (
 	_ "github.com/lib/pq" // postgres driver
 )
 
-const (
-	dhtTableName       = "dhtkv"
-	peerStoreTableName = "peerStore"
-)
-
 func getPrivateKeyPath(config Config) string {
 	return filepath.Join(config.LevelDBDataDir, "keys", "privkey")
 }

cmd/mesh/graphql_server.go

@@ -35,12 +35,10 @@ func serveGraphQL(ctx context.Context, app *core.App, addr string, enableGraphiQ
 	// Start the server
 	server := &http.Server{Addr: addr, Handler: handler}
 	go func() {
-		select {
-		case <-ctx.Done():
-			shutdownContext, cancel := context.WithTimeout(context.Background(), gracefulShutdownTimeout)
-			defer cancel()
-			_ = server.Shutdown(shutdownContext)
-		}
+		<-ctx.Done()
+		shutdownContext, cancel := context.WithTimeout(context.Background(), gracefulShutdownTimeout)
+		defer cancel()
+		_ = server.Shutdown(shutdownContext)
 	}()
 	return server.ListenAndServe()
 }

cmd/peer-id-to-pub-key/main.go

@@ -14,7 +14,7 @@ func main() {
 		log.Fatal("expects exactly one argument")
 	}
 	peerIDString := os.Args[1]
-	peerID, err := peer.IDB58Decode(peerIDString)
+	peerID, err := peer.Decode(peerIDString)
 	if err != nil {
 		log.Fatal(err)
 	}

common/types/types.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/0xProject/0x-mesh/zeroex"
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/common/math"
 	"github.com/ethereum/go-ethereum/core/types"
 )
@@ -197,5 +198,5 @@ func BytesToHex(b []byte) string {
 	if len(b) == 0 {
 		return "0x"
 	}
-	return common.ToHex(b)
+	return hexutil.Encode(b)
 }

core/core.go

@@ -37,8 +37,7 @@ import (
 	"github.com/ethereum/go-ethereum/event"
 	"github.com/ethereum/go-ethereum/rpc"
 	p2pcrypto "github.com/libp2p/go-libp2p-core/crypto"
-	peer "github.com/libp2p/go-libp2p-core/peer"
-	peerstore "github.com/libp2p/go-libp2p-peerstore"
+	"github.com/libp2p/go-libp2p-core/peer"
 	ma "github.com/multiformats/go-multiaddr"
 	log "github.com/sirupsen/logrus"
 )
@@ -48,7 +47,6 @@ const (
 	ethereumRPCRequestTimeout     = 30 * time.Second
 	peerConnectTimeout            = 60 * time.Second
 	checkNewAddrInterval          = 20 * time.Second
-	expirationPollingInterval     = 50 * time.Millisecond
 	rateLimiterCheckpointInterval = 1 * time.Minute
 	// estimatedNonPollingEthereumRPCRequestsPer24Hrs is an estimate of the
 	// minimum number of RPC requests Mesh needs to send (not including block
@@ -285,14 +283,14 @@ func newWithPrivateConfig(ctx context.Context, config Config, pConfig privateCon
 	}
 
 	// Initialize metadata and check stored chain id (if any).
-	_, err = initMetadata(config.EthereumChainID, database)
+	err = initMetadata(config.EthereumChainID, database)
 	if err != nil {
 		return nil, err
 	}
 
 	// Initialize ETH JSON-RPC RateLimiter
 	var ethRPCRateLimiter ratelimit.RateLimiter
-	if config.EnableEthereumRPCRateLimiting == false {
+	if !config.EnableEthereumRPCRateLimiting {
 		ethRPCRateLimiter = ratelimit.NewUnlimited()
 	} else {
 		clock := clock.New()
@@ -460,7 +458,7 @@ func initPrivateKey(path string) (p2pcrypto.PrivKey, error) {
 	return nil, err
 }
 
-func initMetadata(chainID int, database *db.DB) (*types.Metadata, error) {
+func initMetadata(chainID int, database *db.DB) error {
 	metadata, err := database.GetMetadata()
 	if err != nil {
 		if err == db.ErrNotFound {
@@ -469,20 +467,20 @@ func initMetadata(chainID int, database *db.DB) (*types.Metadata, error) {
 				EthereumChainID: chainID,
 			}
 			if err := database.SaveMetadata(metadata); err != nil {
-				return nil, err
+				return err
 			}
-			return metadata, nil
+			return nil
 		}
-		return nil, err
+		return err
 	}
 
 	// on subsequent startups, verify we are on the same chain
 	if metadata.EthereumChainID != chainID {
 		err := fmt.Errorf("expected chainID to be %d but got %d", metadata.EthereumChainID, chainID)
 		log.WithError(err).Error("Mesh previously started on different Ethereum chain; switch chainId or remove DB")
-		return nil, err
+		return err
 	}
-	return metadata, nil
+	return nil
 }
 
 func (app *App) Start() error {
@@ -951,12 +949,8 @@ func (app *App) AddOrdersRaw(ctx context.Context, signedOrdersRaw []*json.RawMes
 		return nil, err
 	}
 
-	for _, orderInfo := range validationResults.Accepted {
-		allValidationResults.Accepted = append(allValidationResults.Accepted, orderInfo)
-	}
-	for _, orderInfo := range validationResults.Rejected {
-		allValidationResults.Rejected = append(allValidationResults.Rejected, orderInfo)
-	}
+	allValidationResults.Accepted = append(allValidationResults.Accepted, validationResults.Accepted...)
+	allValidationResults.Rejected = append(allValidationResults.Rejected, validationResults.Rejected...)
 
 	for _, acceptedOrderInfo := range allValidationResults.Accepted {
 		// If the order isn't new, we don't add to OrderWatcher, log it's receipt
@@ -990,7 +984,7 @@ func (app *App) shareOrder(order *zeroex.SignedOrder) error {
 }
 
 // AddPeer can be used to manually connect to a new peer.
-func (app *App) AddPeer(peerInfo peerstore.PeerInfo) error {
+func (app *App) AddPeer(peerInfo peer.AddrInfo) error {
 	<-app.started
 
 	return app.node.Connect(peerInfo, peerConnectTimeout)