Merge pull request #206 from guggero/makefile

ci: add Makefile, use golangci-lint, parallelize CI actions

.github/workflows/go.yml

@@ -1,31 +1,64 @@
 name: Build and Test
 on: [push, pull_request]
+
+env:
+  # go needs absolute directories, using the $HOME variable doesn't work here.
+  GOCACHE: /home/runner/work/go/pkg/build
+  GOPATH: /home/runner/work/go
+  GO_VERSION: 1.16.8
+
 jobs:
   build:
-    name: Go CI
+    name: Build
     runs-on: ubuntu-latest
-    strategy:
-      matrix:
-        go: [1.14, 1.15]
     steps:
       - name: Set up Go
         uses: actions/setup-go@v2
         with:
-          go-version: ${{ matrix.go }}
+          go-version: ${{ env.GO_VERSION }}
+
       - name: Check out source
         uses: actions/checkout@v2
-      - name: Install Linters
-        run: "curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $(go env GOPATH)/bin v1.31.0"
+
       - name: Build
-        env:
-          GO111MODULE: "on"
-        run: go build ./...
+        run: make build
+
+  test-cover:
+    name: Unit coverage
+    runs-on: ubuntu-latest
+    steps:
+      - name: Set up Go
+        uses: actions/setup-go@v2
+        with:
+          go-version: ${{ env.GO_VERSION }}
+
+      - name: Check out source
+        uses: actions/checkout@v2
+
       - name: Test
-        env:
-          GO111MODULE: "on"
-        run: |
-          sh ./goclean.sh
+        run: make unit-cover
+
       - name: Send btcutil coverage
         uses: shogo82148/actions-goveralls@v1
         with:
-          path-to-profile: profile.cov
+          path-to-profile: coverage.txt
+
+      - name: Send btcutil coverage for psbt package
+        uses: shogo82148/actions-goveralls@v1
+        with:
+          path-to-profile: psbt/coverage.txt
+
+  test-race:
+    name: Unit race
+    runs-on: ubuntu-latest
+    steps:
+      - name: Set up Go
+        uses: actions/setup-go@v2
+        with:
+          go-version: ${{ env.GO_VERSION }}
+
+      - name: Check out source
+        uses: actions/checkout@v2
+
+      - name: Test
+        run: make unit-race

.gitignore

@@ -26,3 +26,5 @@ _cgo_export.*
 _testmain.go
 
 *.exe 
+coverage.txt
+psbt/coverage.txt

.golangci.yml

@@ -0,0 +1,54 @@
+run:
+  # timeout for analysis
+  deadline: 10m
+
+linters-settings:
+  govet:
+    # Don't report about shadowed variables
+    check-shadowing: false
+  gofmt:
+    # simplify code: gofmt with `-s` option, true by default
+    simplify: true
+
+linters:
+  enable-all: true
+  disable:
+    # Global variables are used in many places throughout the code base.
+    - gochecknoglobals
+
+    # Some lines are over 80 characters on purpose and we don't want to make them
+    # even longer by marking them as 'nolint'.
+    - lll
+
+    # We don't care (enough) about misaligned structs to lint that.
+    - maligned
+
+    # We have long functions, especially in tests. Moving or renaming those would
+    # trigger funlen problems that we may not want to solve at that time.
+    - funlen
+
+    # Disable for now as we haven't yet tuned the sensitivity to our codebase
+    # yet.  Enabling by default for example, would also force new contributors to
+    # potentially extensively refactor code, when they want to smaller change to
+    # land.
+    - gocyclo
+
+    # Instances of table driven tests that don't pre-allocate shouldn't trigger
+    # the linter.
+    - prealloc
+
+    # Init functions are used by loggers throughout the codebase.
+    - gochecknoinits
+
+    # Explicit types are okay.
+    - interfacer
+
+issues:
+  exclude-rules:
+    # Exclude gosec from running for tests so that tests with weak randomness
+    # (math/rand) will pass the linter.
+    - path: _test\.go
+      linters:
+        - gosec
+        #- errcheck
+        - dupl

Makefile

@@ -0,0 +1,116 @@
+PKG := github.com/btcsuite/btcutil
+
+LINT_PKG := github.com/golangci/golangci-lint/cmd/golangci-lint
+GOACC_PKG := github.com/ory/go-acc
+GOIMPORTS_PKG := golang.org/x/tools/cmd/goimports
+
+GO_BIN := ${GOPATH}/bin
+LINT_BIN := $(GO_BIN)/golangci-lint
+GOACC_BIN := $(GO_BIN)/go-acc
+
+LINT_COMMIT := v1.18.0
+GOACC_COMMIT := 80342ae2e0fcf265e99e76bcc4efd022c7c3811b
+
+DEPGET := cd /tmp && GO111MODULE=on go get -v
+GOBUILD := GO111MODULE=on go build -v
+GOINSTALL := GO111MODULE=on go install -v
+GOTEST := GO111MODULE=on go test 
+
+GOFILES_NOVENDOR = $(shell find . -type f -name '*.go' -not -path "./vendor/*")
+
+RM := rm -f
+CP := cp
+MAKE := make
+XARGS := xargs -L 1
+
+# Linting uses a lot of memory, so keep it under control by limiting the number
+# of workers if requested.
+ifneq ($(workers),)
+LINT_WORKERS = --concurrency=$(workers)
+endif
+
+LINT = $(LINT_BIN) run -v $(LINT_WORKERS)
+
+GREEN := "\\033[0;32m"
+NC := "\\033[0m"
+define print
+	echo $(GREEN)$1$(NC)
+endef
+
+default: build
+
+all: build check
+
+# ============
+# DEPENDENCIES
+# ============
+
+$(LINT_BIN):
+	@$(call print, "Fetching linter")
+	$(DEPGET) $(LINT_PKG)@$(LINT_COMMIT)
+
+$(GOACC_BIN):
+	@$(call print, "Fetching go-acc")
+	$(DEPGET) $(GOACC_PKG)@$(GOACC_COMMIT)
+
+goimports:
+	@$(call print, "Installing goimports.")
+	$(DEPGET) $(GOIMPORTS_PKG)
+
+# ============
+# INSTALLATION
+# ============
+
+build:
+	@$(call print, "Compiling btcutil.")
+	$(GOBUILD) $(PKG)/...
+
+# =======
+# TESTING
+# =======
+
+check: unit
+
+unit:
+	@$(call print, "Running unit tests.")
+	$(GOTEST) ./... -test.timeout=20m
+	cd psbt; $(GOTEST) ./... -test.timeout=20m
+
+unit-cover: $(GOACC_BIN)
+	@$(call print, "Running unit coverage tests.")
+	$(GOACC_BIN) ./...
+	cd psbt; $(GOACC_BIN) ./...
+
+unit-race:
+	@$(call print, "Running unit race tests.")
+	env CGO_ENABLED=1 GORACE="history_size=7 halt_on_errors=1" $(GOTEST) -race -test.timeout=20m ./...
+	cd psbt; env CGO_ENABLED=1 GORACE="history_size=7 halt_on_errors=1" $(GOTEST) -race -test.timeout=20m ./...
+
+# =========
+# UTILITIES
+# =========
+
+fmt: goimports
+	@$(call print, "Fixing imports.")
+	goimports -w $(GOFILES_NOVENDOR)
+	@$(call print, "Formatting source.")
+	gofmt -l -w -s $(GOFILES_NOVENDOR)
+
+lint: $(LINT_BIN)
+	@$(call print, "Linting source.")
+	$(LINT)
+
+clean:
+	@$(call print, "Cleaning source.$(NC)")
+	$(RM) coverage.txt psbt/coverage.txt
+
+.PHONY: all \
+	default \
+	build \
+	check \
+	unit \
+	unit-cover \
+	unit-race \
+	fmt \
+	lint \
+	clean

address.go

@@ -41,7 +41,7 @@ var (
 
 	// ErrUnknownAddressType describes an error where an address can not
 	// decoded as a specific address type due to the string encoding
-	// begining with an identifier byte unknown to any standard or
+	// beginning with an identifier byte unknown to any standard or
 	// registered (via chaincfg.Register) network.
 	ErrUnknownAddressType = errors.New("unknown address type")
 
@@ -294,7 +294,7 @@ func NewAddressPubKeyHash(pkHash []byte, net *chaincfg.Params) (*AddressPubKeyHa
 // newAddressPubKeyHash is the internal API to create a pubkey hash address
 // with a known leading identifier byte for a network, rather than looking
 // it up through its parameters.  This is useful when creating a new address
-// structure from a string encoding where the identifer byte is already
+// structure from a string encoding where the identifier byte is already
 // known.
 func newAddressPubKeyHash(pkHash []byte, netID byte) (*AddressPubKeyHash, error) {
 	// Check for a valid pubkey hash length.
@@ -333,7 +333,7 @@ func (a *AddressPubKeyHash) String() string {
 }
 
 // Hash160 returns the underlying array of the pubkey hash.  This can be useful
-// when an array is more appropiate than a slice (for example, when used as map
+// when an array is more appropriate than a slice (for example, when used as map
 // keys).
 func (a *AddressPubKeyHash) Hash160() *[ripemd160.Size]byte {
 	return &a.hash
@@ -361,7 +361,7 @@ func NewAddressScriptHashFromHash(scriptHash []byte, net *chaincfg.Params) (*Add
 // newAddressScriptHashFromHash is the internal API to create a script hash
 // address with a known leading identifier byte for a network, rather than
 // looking it up through its parameters.  This is useful when creating a new
-// address structure from a string encoding where the identifer byte is already
+// address structure from a string encoding where the identifier byte is already
 // known.
 func newAddressScriptHashFromHash(scriptHash []byte, netID byte) (*AddressScriptHash, error) {
 	// Check for a valid script hash length.
@@ -400,7 +400,7 @@ func (a *AddressScriptHash) String() string {
 }
 
 // Hash160 returns the underlying array of the script hash.  This can be useful
-// when an array is more appropiate than a slice (for example, when used as map
+// when an array is more appropriate than a slice (for example, when used as map
 // keys).
 func (a *AddressScriptHash) Hash160() *[ripemd160.Size]byte {
 	return &a.hash

address_test.go

@@ -876,7 +876,7 @@ func TestAddresses(t *testing.T) {
 			// Encode again and compare against the original.
 			encoded := decoded.EncodeAddress()
 			if test.encoded != encoded {
-				t.Errorf("%v: decoding and encoding produced different addressess: %v != %v",
+				t.Errorf("%v: decoding and encoding produced different addresses: %v != %v",
 					test.name, test.encoded, encoded)
 				return
 			}

base58/base58check.go

@@ -28,7 +28,7 @@ func checksum(input []byte) (cksum [4]byte) {
 func CheckEncode(input []byte, version byte) string {
 	b := make([]byte, 0, 1+len(input)+4)
 	b = append(b, version)
-	b = append(b, input[:]...)
+	b = append(b, input...)
 	cksum := checksum(b)
 	b = append(b, cksum[:]...)
 	return Encode(b)

base58/base58check_test.go

@@ -37,11 +37,14 @@ func TestBase58Check(t *testing.T) {
 
 		// test decoding
 		res, version, err := base58.CheckDecode(test.out)
-		if err != nil {
+		switch {
+		case err != nil:
 			t.Errorf("CheckDecode test #%d failed with err: %v", x, err)
-		} else if version != test.version {
+
+		case version != test.version:
 			t.Errorf("CheckDecode test #%d failed: got version: %d want: %d", x, version, test.version)
-		} else if string(res) != test.in {
+
+		case string(res) != test.in:
 			t.Errorf("CheckDecode test #%d failed: got: %s want: %s", x, res, test.in)
 		}
 	}
@@ -56,7 +59,7 @@ func TestBase58Check(t *testing.T) {
 	// bytes are missing).
 	testString := ""
 	for len := 0; len < 4; len++ {
-		testString = testString + "x"
+		testString += "x"
 		_, _, err = base58.CheckDecode(testString)
 		if err != base58.ErrInvalidFormat {
 			t.Error("Checkdecode test failed, expected ErrInvalidFormat")

bech32/bech32.go

@@ -362,7 +362,7 @@ func ConvertBits(data []byte, fromBits, toBits uint8, pad bool) ([]byte, error)
 	for _, b := range data {
 
 		// Discard unused bits.
-		b = b << (8 - fromBits)
+		b <<= 8 - fromBits
 
 		// How many bits remaining to extract from the input data.
 		remFromBits := fromBits
@@ -383,7 +383,7 @@ func ConvertBits(data []byte, fromBits, toBits uint8, pad bool) ([]byte, error)
 
 			// Discard the bits we just extracted and get ready for
 			// next iteration.
-			b = b << toExtract
+			b <<= toExtract
 			remFromBits -= toExtract
 			filledBits += toExtract
 
@@ -399,7 +399,7 @@ func ConvertBits(data []byte, fromBits, toBits uint8, pad bool) ([]byte, error)
 
 	// We pad any unfinished group if specified.
 	if pad && filledBits > 0 {
-		nextByte = nextByte << (toBits - filledBits)
+		nextByte <<= toBits - filledBits
 		regrouped = append(regrouped, nextByte)
 		filledBits = 0
 		nextByte = 0

bloom/merkleblock.go

@@ -116,7 +116,7 @@ func NewMerkleBlock(block *btcutil.Block, filter *Filter) (*wire.MsgMerkleBlock,
 		Flags:        make([]byte, (len(mBlock.bits)+7)/8),
 	}
 	for _, hash := range mBlock.finalHashes {
-		msgMerkleBlock.AddTxHash(hash)
+		_ = msgMerkleBlock.AddTxHash(hash)
 	}
 	for i := uint32(0); i < uint32(len(mBlock.bits)); i++ {
 		msgMerkleBlock.Flags[i/8] |= mBlock.bits[i] << (i % 8)

certgen.go

@@ -61,7 +61,7 @@ func NewTLSCertPair(organization string, validUntil time.Time, extraHosts []stri
 
 	addIP := func(ipAddr net.IP) {
 		for _, ip := range ipAddresses {
-			if bytes.Equal(ip, ipAddr) {
+			if ip.Equal(ipAddr) {
 				return
 			}
 		}