Merge pull request #94 from guggero/lint

Add linter to makefile, lint on Travis build

.golangci.yml

@@ -0,0 +1,43 @@
+run:
+  # timeout for analysis
+  deadline: 4m
+
+  # Linting uses a lot of memory. Keep it under control by only running a single
+  # worker.
+  concurrency: 1
+
+linters-settings:
+  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
+
+issues:
+  # Only show newly introduced problems.
+  new-from-rev: 36838cf7f464cf73b0201798063b2caffeae4250

.travis.yml

@@ -18,7 +18,7 @@ sudo: required
 
 script:
   - export GO111MODULE=on
-  - make unit
+  - make lint unit
 
 after_script:
   - echo "Uploading to termbin.com..." && find *.log | xargs -I{} sh -c "cat {} | nc termbin.com 9999 | xargs -r0 printf '{} uploaded to %s'"

Makefile

@@ -2,14 +2,35 @@ PKG := github.com/lightninglabs/loop
 
 GOTEST := GO111MODULE=on go test -v
 
+GO_BIN := ${GOPATH}/bin
+
+GOFILES_NOVENDOR = $(shell find . -type f -name '*.go' -not -path "./vendor/*")
 GOLIST := go list $(PKG)/... | grep -v '/vendor/'
 
+LINT_BIN := $(GO_BIN)/golangci-lint
+LINT_PKG := github.com/golangci/golangci-lint/cmd/golangci-lint
+LINT_COMMIT := v1.18.0
+LINT = $(LINT_BIN) run -v
+
+DEPGET := cd /tmp && GO111MODULE=on go get -v
 XARGS := xargs -L 1
 
 TEST_FLAGS = -test.timeout=20m
 
 UNIT := $(GOLIST) | $(XARGS) env $(GOTEST) $(TEST_FLAGS)
 
+$(LINT_BIN):
+	@$(call print, "Fetching linter")
+	$(DEPGET) $(LINT_PKG)@$(LINT_COMMIT)
+
 unit: 
 	@$(call print, "Running unit tests.")
 	$(UNIT)
+
+fmt:
+	@$(call print, "Formatting source.")
+	gofmt -l -w -s $(GOFILES_NOVENDOR)
+
+lint: $(LINT_BIN)
+	@$(call print, "Linting source.")
+	$(LINT)

client.go

@@ -384,7 +384,7 @@ func (s *Client) LoopOutQuote(ctx context.Context,
 	return &LoopOutQuote{
 		SwapFee:         swapFee,
 		MinerFee:        minerFee,
-		PrepayAmount:    btcutil.Amount(quote.PrepayAmount),
+		PrepayAmount:    quote.PrepayAmount,
 		SwapPaymentDest: quote.SwapPaymentDest,
 		CltvDelta:       quote.CltvDelta,
 	}, nil

cmd/loop/loopin.go

@@ -41,8 +41,7 @@ func loopIn(ctx *cli.Context) error {
 		args = args.Tail()
 	default:
 		// Show command help if no arguments and flags were provided.
-		cli.ShowCommandHelp(ctx, "in")
-		return nil
+		return cli.ShowCommandHelp(ctx, "in")
 	}
 
 	amt, err := parseAmt(amtStr)

cmd/loop/loopout.go

@@ -59,8 +59,7 @@ func loopOut(ctx *cli.Context) error {
 		args = args.Tail()
 	default:
 		// Show command help if no arguments and flags were provided.
-		cli.ShowCommandHelp(ctx, "out")
-		return nil
+		return cli.ShowCommandHelp(ctx, "out")
 	}
 
 	amt, err := parseAmt(amtStr)

cmd/loop/main.go

@@ -97,7 +97,7 @@ type limits struct {
 }
 
 func getLimits(amt btcutil.Amount, quote *looprpc.QuoteResponse) *limits {
-	maxSwapRoutingFee := getMaxRoutingFee(btcutil.Amount(amt))
+	maxSwapRoutingFee := getMaxRoutingFee(amt)
 	maxPrepayRoutingFee := getMaxRoutingFee(btcutil.Amount(
 		quote.PrepayAmt,
 	))
@@ -126,7 +126,7 @@ func displayLimits(swapType loop.Type, amt btcutil.Amount, l *limits,
 	if l.maxPrepayRoutingFee != nil {
 		totalSuccessMax += *l.maxPrepayRoutingFee
 	}
-	
+
 	if swapType == loop.TypeIn && externalHtlc {
 		fmt.Printf("On-chain fee for external loop in is not " +
 			"included.\nSufficient fees will need to be paid " +
@@ -135,7 +135,7 @@ func displayLimits(swapType loop.Type, amt btcutil.Amount, l *limits,
 	}
 
 	fmt.Printf("Max swap fees for %d Loop %v: %d\n",
-		btcutil.Amount(amt), swapType, totalSuccessMax,
+		amt, swapType, totalSuccessMax,
 	)
 
 	fmt.Printf("CONTINUE SWAP? (y/n), expand fee detail (x): ")

cmd/loop/quote.go

@@ -28,8 +28,7 @@ func quote(ctx *cli.Context) error {
 	// Show command help if the incorrect number arguments and/or flags were
 	// provided.
 	if ctx.NArg() != 1 || ctx.NumFlags() > 1 {
-		cli.ShowCommandHelp(ctx, "quote")
-		return nil
+		return cli.ShowCommandHelp(ctx, "quote")
 	}
 
 	args := ctx.Args()

executor.go

@@ -69,7 +69,7 @@ func (s *executor) run(mainCtx context.Context,
 
 	select {
 	case h := <-blockEpochChan:
-		setHeight(int32(h))
+		setHeight(h)
 	case err := <-blockErrorChan:
 		return err
 	case <-mainCtx.Done():
@@ -134,10 +134,10 @@ func (s *executor) run(mainCtx context.Context,
 			delete(blockEpochQueues, doneID)
 
 		case h := <-blockEpochChan:
-			setHeight(int32(h))
+			setHeight(h)
 			for _, queue := range blockEpochQueues {
 				select {
-				case queue.ChanIn() <- int32(h):
+				case queue.ChanIn() <- h:
 				case <-mainCtx.Done():
 					return mainCtx.Err()
 				}

lndclient/invoices_client.go

@@ -52,10 +52,10 @@ func (s *invoicesClient) WaitForFinished() {
 func (s *invoicesClient) SettleInvoice(ctx context.Context,
 	preimage lntypes.Preimage) error {
 
-	rpcCtx, cancel := context.WithTimeout(ctx, rpcTimeout)
+	timeoutCtx, cancel := context.WithTimeout(ctx, rpcTimeout)
 	defer cancel()
 
-	rpcCtx = s.invoiceMac.WithMacaroonAuth(ctx)
+	rpcCtx := s.invoiceMac.WithMacaroonAuth(timeoutCtx)
 	_, err := s.client.SettleInvoice(rpcCtx, &invoicesrpc.SettleInvoiceMsg{
 		Preimage: preimage[:],
 	})

lndclient/log.go

@@ -1,8 +1,9 @@
 package lndclient
 
 import (
-	"github.com/btcsuite/btclog"
 	"os"
+
+	"github.com/btcsuite/btclog"
 )
 
 // log is a logger that is initialized with no output filters.  This

lndclient/router_client.go

@@ -4,9 +4,10 @@ import (
 	"context"
 	"encoding/hex"
 	"fmt"
+	"time"
+
 	"github.com/lightningnetwork/lnd/lnrpc"
 	"github.com/lightningnetwork/lnd/routing/route"
-	"time"
 
 	"github.com/lightningnetwork/lnd/channeldb"
 	"google.golang.org/grpc/codes"

log.go

@@ -12,9 +12,8 @@ import (
 // means the package will not perform any logging by default until the caller
 // requests it.
 var (
-	backendLog     = btclog.NewBackend(logWriter{})
-	logger         = backendLog.Logger("CLIENT")
-	servicesLogger = backendLog.Logger("SERVICES")
+	backendLog = btclog.NewBackend(logWriter{})
+	logger     = backendLog.Logger("CLIENT")
 )
 
 // logWriter implements an io.Writer that outputs to both standard output and

loopdb/loopin.go

@@ -127,7 +127,10 @@ func deserializeLoopInContract(value []byte) (*LoopInContract, error) {
 		return nil, err
 	}
 
-	binary.Read(r, byteOrder, &contract.AmountRequested)
+	err = binary.Read(r, byteOrder, &contract.AmountRequested)
+	if err != nil {
+		return nil, err
+	}
 
 	n, err := r.Read(contract.SenderKey[:])
 	if err != nil {

loopdb/loopout.go

@@ -84,7 +84,10 @@ func deserializeLoopOutContract(value []byte, chainParams *chaincfg.Params) (
 		return nil, err
 	}
 
-	binary.Read(r, byteOrder, &contract.AmountRequested)
+	err = binary.Read(r, byteOrder, &contract.AmountRequested)
+	if err != nil {
+		return nil, err
+	}
 
 	contract.PrepayInvoice, err = wire.ReadVarString(r, 0)
 	if err != nil {

loopin.go

@@ -493,7 +493,7 @@ func (s *loopInSwap) waitForSwapComplete(ctx context.Context,
 		case err := <-swapInvoiceErr:
 			return err
 
-		// An update to the swap invoice occured. Check the new state
+		// An update to the swap invoice occurred. Check the new state
 		// and update the swap state accordingly.
 		case update := <-swapInvoiceChan:
 			s.log.Infof("Received swap invoice update: %v",

loopout_test.go

@@ -176,7 +176,7 @@ func TestCustomSweepConfTarget(t *testing.T) {
 	// Notify the confirmation notification for the HTLC.
 	ctx.AssertRegisterConf()
 
-	blockEpochChan <- int32(ctx.Lnd.Height + 1)
+	blockEpochChan <- ctx.Lnd.Height + 1
 
 	htlcTx := wire.NewMsgTx(2)
 	htlcTx.AddTxOut(&wire.TxOut{
@@ -238,7 +238,7 @@ func TestCustomSweepConfTarget(t *testing.T) {
 
 	// The sweep should have a fee that corresponds to the custom
 	// confirmation target.
-	sweepTx := assertSweepTx(testRequest.SweepConfTarget)
+	_ = assertSweepTx(testRequest.SweepConfTarget)
 
 	// We'll then notify the height at which we begin using the default
 	// confirmation target.
@@ -249,7 +249,7 @@ func TestCustomSweepConfTarget(t *testing.T) {
 
 	// We should expect to see another sweep using the higher fee since the
 	// spend hasn't been confirmed yet.
-	sweepTx = assertSweepTx(DefaultSweepConfTarget)
+	sweepTx := assertSweepTx(DefaultSweepConfTarget)
 
 	// Notify the spend so that the swap reaches its final state.
 	ctx.NotifySpend(sweepTx, 0)

server_mock_test.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 
-	"testing"
 	"time"
 
 	"github.com/btcsuite/btcd/chaincfg"
@@ -20,20 +19,14 @@ var (
 
 	testLoopOutOnChainCltvDelta = int32(30)
 	testChargeOnChainCltvDelta  = int32(100)
-	testCltvDelta               = 50
 	testSwapFee                 = btcutil.Amount(210)
-	testInvoiceExpiry           = 180 * time.Second
 	testFixedPrepayAmount       = btcutil.Amount(100)
 	testMinSwapAmount           = btcutil.Amount(10000)
 	testMaxSwapAmount           = btcutil.Amount(1000000)
-	testTxConfTarget            = 2
-	testRepublishDelay          = 10 * time.Second
 )
 
 // serverMock is used in client unit tests to simulate swap server behaviour.
 type serverMock struct {
-	t *testing.T
-
 	expectedSwapAmt  btcutil.Amount
 	swapInvoiceAmt   btcutil.Amount
 	prepayInvoiceAmt btcutil.Amount

store_mock_test.go

@@ -25,11 +25,6 @@ type storeMock struct {
 	t *testing.T
 }
 
-type finishData struct {
-	preimage lntypes.Hash
-	result   loopdb.SwapState
-}
-
 // NewStoreMock instantiates a new mock store.
 func newStoreMock(t *testing.T) *storeMock {
 	return &storeMock{

sweep/sweeper.go

@@ -112,7 +112,7 @@ func (s *Sweeper) GetSweepFee(ctx context.Context,
 	case *btcutil.AddressPubKeyHash:
 		weightEstimate.AddP2PKHOutput()
 	default:
-		return 0, fmt.Errorf("unknown adress type %T", destAddr)
+		return 0, fmt.Errorf("unknown address type %T", destAddr)
 	}
 
 	addInputEstimate(&weightEstimate)

test/log.go

@@ -1,8 +1,9 @@
 package test
 
 import (
-	"github.com/btcsuite/btclog"
 	"os"
+
+	"github.com/btcsuite/btclog"
 )
 
 // log is a logger that is initialized with no output filters.  This

test/testutils.go

@@ -91,5 +91,8 @@ func GetInvoice(hash lntypes.Hash, amt btcutil.Amount, memo string) (
 
 // DumpGoroutines dumps all currently running goroutines.
 func DumpGoroutines() {
-	pprof.Lookup("goroutine").WriteTo(os.Stdout, 1)
+	err := pprof.Lookup("goroutine").WriteTo(os.Stdout, 1)
+	if err != nil {
+		panic(err)
+	}
 }

test/timeout.go

@@ -15,7 +15,10 @@ func Guard(t *testing.T) func() {
 	go func() {
 		select {
 		case <-time.After(5 * time.Second):
-			pprof.Lookup("goroutine").WriteTo(os.Stdout, 1)
+			err := pprof.Lookup("goroutine").WriteTo(os.Stdout, 1)
+			if err != nil {
+				panic(err)
+			}
 
 			panic("test timeout")
 		case <-done:

testcontext_test.go

@@ -21,14 +21,6 @@ var (
 		1, 1, 1, 1, 2, 2, 2, 2,
 		3, 3, 3, 3, 4, 4, 4, 4,
 	})
-	testPrepayPreimage = lntypes.Preimage([32]byte{
-		1, 1, 1, 1, 2, 2, 2, 2,
-		3, 3, 3, 3, 4, 4, 4, 4,
-		1, 1, 1, 1, 2, 2, 2, 2,
-		3, 3, 3, 3, 4, 4, 4, 5,
-	})
-
-	testStartingHeight = uint32(600)
 )
 
 // testContext contains functionality to support client unit tests.