Merge pull request lightninglabs#817 from lightninglabs/lint_misc_fixes

multi: add linters for switch exhaustiveness, if-else chains, and err handling

.golangci.yml

@@ -23,8 +23,15 @@ linters-settings:
     excludes:
       - G402 # Look for bad TLS connection settings.
       - G306 # Poor file permissions used when writing to a new file.
+  exhaustive:
+    default-signifies-exhaustive: true
+  gocritic:
+      enabled-checks:
+        - singleCaseSwitch
+        - ifElseChain
+        - assignOp
+        - unlambda
   staticcheck:
-    go: "1.19"
     checks: ["-SA1019"]
 
 linters:
@@ -34,6 +41,11 @@ linters:
     - tagliatelle
     - whitespace
     - gosec
+    - unused
+    - gocritic
+    - unconvert
+    - nilerr
+    - exhaustive
 
 issues:
   exclude-rules:

asset/asset.go

@@ -778,11 +778,7 @@ func (g *GroupKey) IsEqual(otherGroupKey *GroupKey) bool {
 		return false
 	}
 
-	return slices.EqualFunc(
-		g.Witness, otherGroupKey.Witness, func(a, b []byte) bool {
-			return bytes.Equal(a, b)
-		},
-	)
+	return slices.EqualFunc(g.Witness, otherGroupKey.Witness, bytes.Equal)
 }
 
 // IsEqualGroup returns true if this group key describes the same asset group

asset/mock.go

@@ -270,6 +270,8 @@ func SignOutputRaw(priv *btcec.PrivateKey, tx *wire.MsgTx,
 			signDesc.Output.Value, signDesc.Output.PkScript,
 			leaf, signDesc.HashType, privKey,
 		)
+	default:
+		// A witness V0 sign method should never appear here.
 	}
 	if err != nil {
 		return nil, err

cmd/tapcli/addrs.go

@@ -64,8 +64,7 @@ var newAddrCommand = cli.Command{
 }
 
 func newAddr(ctx *cli.Context) error {
-	switch {
-	case ctx.String(assetIDName) == "":
+	if ctx.String(assetIDName) == "" {
 		return cli.ShowSubcommandHelp(ctx)
 	}
 
@@ -163,7 +162,7 @@ func queryAddr(ctx *cli.Context) error {
 
 	addrs, err := client.QueryAddrs(ctxc, &taprpc.QueryAddrRequest{
 		CreatedAfter:  start,
-		CreatedBefore: int64(end),
+		CreatedBefore: end,
 		Limit:         int32(ctx.Int64(limitName)),
 		Offset:        int32(ctx.Int64(offsetName)),
 	})

cmd/tapcli/proofs.go

@@ -59,8 +59,7 @@ var verifyProofCommand = cli.Command{
 }
 
 func verifyProof(ctx *cli.Context) error {
-	switch {
-	case ctx.String(proofPathName) == "":
+	if ctx.String(proofPathName) == "" {
 		return cli.ShowSubcommandHelp(ctx)
 	}
 
@@ -126,8 +125,7 @@ func decodeProof(ctx *cli.Context) error {
 	client, cleanUp := getClient(ctx)
 	defer cleanUp()
 
-	switch {
-	case !ctx.IsSet(proofPathName):
+	if !ctx.IsSet(proofPathName) {
 		_ = cli.ShowCommandHelp(ctx, "decode")
 		return nil
 	}
@@ -176,8 +174,7 @@ var verifyOwnershipCommand = cli.Command{
 }
 
 func verifyOwnershipProof(ctx *cli.Context) error {
-	switch {
-	case ctx.String(proofPathName) == "":
+	if ctx.String(proofPathName) == "" {
 		return cli.ShowSubcommandHelp(ctx)
 	}
 

cmd/tapcli/universe.go

@@ -516,8 +516,7 @@ var universeProofInsertInsert = cli.Command{
 }
 
 func universeProofInsert(ctx *cli.Context) error {
-	switch {
-	case ctx.String(proofPathName) == "":
+	if ctx.String(proofPathName) == "" {
 		return cli.ShowSubcommandHelp(ctx)
 	}
 
@@ -617,8 +616,7 @@ var universeSyncCommand = cli.Command{
 }
 
 func universeSync(ctx *cli.Context) error {
-	switch {
-	case ctx.String(universeHostName) == "":
+	if ctx.String(universeHostName) == "" {
 		return cli.ShowSubcommandHelp(ctx)
 	}
 
@@ -721,8 +719,7 @@ var universeFederationAddCommand = cli.Command{
 }
 
 func universeFederationAdd(ctx *cli.Context) error {
-	switch {
-	case ctx.String(universeHostName) == "":
+	if ctx.String(universeHostName) == "" {
 		return cli.ShowSubcommandHelp(ctx)
 	}
 
@@ -771,9 +768,9 @@ var universeFederationDelCommand = cli.Command{
 }
 
 func universeFederationDel(ctx *cli.Context) error {
-	switch {
-	case ctx.String(universeHostName) == "" &&
-		ctx.Int(universeServerID) == 0:
+	if ctx.String(universeHostName) == "" &&
+		ctx.Int(universeServerID) == 0 {
+
 		return cli.ShowSubcommandHelp(ctx)
 	}
 

commitment/commitment_test.go

@@ -450,15 +450,16 @@ func TestMintAndDeriveTapCommitment(t *testing.T) {
 
 			var tapCommitment *TapCommitment
 
-			if includesAsset && includesAssetGroup {
+			switch {
+			case includesAsset && includesAssetGroup:
 				tapCommitment, err = proof.DeriveByAssetInclusion(
 					asset,
 				)
-			} else if includesAssetGroup {
+			case !includesAsset && includesAssetGroup:
 				tapCommitment, err = proof.DeriveByAssetExclusion(
 					asset.AssetCommitmentKey(),
 				)
-			} else {
+			case !includesAsset && !includesAssetGroup:
 				tapCommitment, err = proof.DeriveByAssetCommitmentExclusion(
 					asset.TapCommitmentKey(),
 				)

commitment/taproot.go

@@ -233,7 +233,7 @@ func (t *TapscriptPreimage) IsEmpty() bool {
 
 // Type returns the preimage type.
 func (t *TapscriptPreimage) Type() TapscriptPreimageType {
-	return TapscriptPreimageType(t.siblingType)
+	return t.siblingType
 }
 
 // TapHash returns the tap hash of this preimage according to its type.

itest/mint_batch_stress_test.go

@@ -121,7 +121,7 @@ func mintBatchStressTest(
 
 	// Update the asset name and metadata to match an index.
 	incrementMintAsset := func(asset *mintrpc.MintAsset, ind int) {
-		asset.Name = asset.Name + strconv.Itoa(ind)
+		asset.Name += strconv.Itoa(ind)
 		binary.PutUvarint(metadataPrefix, uint64(ind))
 		copy(asset.AssetMeta.Data[0:metaPrefixSize], metadataPrefix)
 	}
@@ -202,7 +202,7 @@ func mintBatchStressTest(
 	// outpoints matching the chain anchor of the group anchor.
 	mintOutpoint := collectibleAnchor.ChainAnchor.AnchorOutpoint
 
-	leafKeys, err := fetchAllLeafKeys(t, alice, &collectUniID)
+	leafKeys, err := fetchAllLeafKeys(alice, &collectUniID)
 	require.NoError(t, err)
 
 	require.Len(t, leafKeys, batchSize)

itest/psbt_test.go

@@ -602,7 +602,7 @@ func runPsbtInteractiveSplitSendTest(ctxt context.Context, t *harnessTest,
 		// Swap the sender and receiver nodes starting at the second
 		// iteration.
 		if i > 0 {
-			sendAmt = sendAmt / 2
+			sendAmt /= 2
 			changeAmt = sendAmt
 			sender, receiver = receiver, sender
 			senderSum, receiverSum = receiverSum, senderSum

itest/send_test.go

@@ -54,20 +54,9 @@ func testBasicSendUnidirectional(t *harnessTest) {
 
 		broadcastState := tapfreighter.SendStateBroadcast.String()
 		targetEventSelector := func(event *taprpc.SendAssetEvent) bool {
-			switch eventTyped := event.Event.(type) {
-			case *taprpc.SendAssetEvent_ExecuteSendStateEvent:
-				ev := eventTyped.ExecuteSendStateEvent
-
-				// Log send state execution.
-				timestamp := time.UnixMicro(ev.Timestamp)
-				t.Logf("Executing send state (%v): %v",
-					timestamp.Format(time.RFC3339Nano),
-					ev.SendState)
-
-				return ev.SendState == broadcastState
-			}
-
-			return false
+			return AssertSendEventExecuteSendState(
+				t, event, broadcastState,
+			)
 		}
 
 		timeout := 2 * defaultProofTransferReceiverAckTimeout
@@ -168,20 +157,9 @@ func testRestartReceiverCheckBalance(t *harnessTest) {
 
 		broadcastState := tapfreighter.SendStateBroadcast.String()
 		targetEventSelector := func(event *taprpc.SendAssetEvent) bool {
-			switch eventTyped := event.Event.(type) {
-			case *taprpc.SendAssetEvent_ExecuteSendStateEvent:
-				ev := eventTyped.ExecuteSendStateEvent
-
-				// Log send state execution.
-				timestamp := time.UnixMicro(ev.Timestamp)
-				t.Logf("Executing send state (%v): %v",
-					timestamp.Format(time.RFC3339Nano),
-					ev.SendState)
-
-				return ev.SendState == broadcastState
-			}
-
-			return false
+			return AssertSendEventExecuteSendState(
+				t, event, broadcastState,
+			)
 		}
 
 		timeout := 2 * defaultProofTransferReceiverAckTimeout
@@ -620,18 +598,9 @@ func testReattemptFailedSendHashmailCourier(t *harnessTest) {
 		// Define a target event selector to match the backoff wait
 		// event. This function selects for a specific event type.
 		targetEventSelector := func(event *taprpc.SendAssetEvent) bool {
-			switch eventTyped := event.Event.(type) {
-			case *taprpc.SendAssetEvent_ProofTransferBackoffWaitEvent:
-				ev := eventTyped.ProofTransferBackoffWaitEvent
-				if ev.TransferType != transferTypeSend {
-					return false
-				}
-
-				t.Logf("Found event ntfs: %v", ev)
-				return true
-			}
-
-			return false
+			return AssertSendEventProofTransferBackoffWaitTypeSend(
+				t, event,
+			)
 		}
 
 		// Expected number of events is one less than the number of
@@ -727,18 +696,9 @@ func testReattemptFailedSendUniCourier(t *harnessTest) {
 		// Define a target event selector to match the backoff wait
 		// event. This function selects for a specific event type.
 		targetEventSelector := func(event *taprpc.SendAssetEvent) bool {
-			switch eventTyped := event.Event.(type) {
-			case *taprpc.SendAssetEvent_ProofTransferBackoffWaitEvent:
-				ev := eventTyped.ProofTransferBackoffWaitEvent
-				if ev.TransferType != transferTypeSend {
-					return false
-				}
-
-				t.Logf("Found event ntfs: %v", ev)
-				return true
-			}
-
-			return false
+			return AssertSendEventProofTransferBackoffWaitTypeSend(
+				t, event,
+			)
 		}
 
 		// Expected number of events is one less than the number of
@@ -889,22 +849,21 @@ func testReattemptFailedReceiveUniCourier(t *harnessTest) {
 	// Define a target event selector to match the backoff wait event. This
 	// function selects for a specific event type.
 	targetEventSelector := func(event *taprpc.ReceiveAssetEvent) bool {
-		switch eventTyped := event.Event.(type) {
-		case *taprpc.ReceiveAssetEvent_ProofTransferBackoffWaitEvent:
-			ev := eventTyped.ProofTransferBackoffWaitEvent
-
-			// We are attempting to identify receive transfer types.
-			// Skip the event if it is not a receiving transfer
-			// type.
-			if ev.TransferType != taprpc.ProofTransferType_PROOF_TRANSFER_TYPE_RECEIVE {
-				return false
-			}
+		ev := event.GetProofTransferBackoffWaitEvent()
+		if ev == nil {
+			return false
+		}
 
-			t.Logf("Found event ntfs: %v", ev)
-			return true
+		// We are attempting to identify receive transfer types.
+		// Skip the event if it is not a receiving transfer
+		// type.
+		typeRecv := taprpc.ProofTransferType_PROOF_TRANSFER_TYPE_RECEIVE
+		if ev.TransferType != typeRecv {
+			return false
 		}
 
-		return false
+		t.Logf("Found event ntfs: %v", ev)
+		return true
 	}
 
 	// Expected minimum number of events to receive.
@@ -996,23 +955,12 @@ func testOfflineReceiverEventuallyReceives(t *harnessTest) {
 		// Define a target event selector to match the backoff wait
 		// event. This function selects for a specific event type.
 		targetEventSelector := func(event *taprpc.SendAssetEvent) bool {
-			switch eventTyped := event.Event.(type) {
-			case *taprpc.SendAssetEvent_ProofTransferBackoffWaitEvent:
-				ev := eventTyped.ProofTransferBackoffWaitEvent
-
-				// We're listening for events on the sender
-				// node. We therefore expect to receive
-				// deliver transfer type backoff wait events
-				// for sending transfers.
-				if ev.TransferType != transferTypeSend {
-					return false
-				}
-
-				t.Logf("Found event ntfs: %v", ev)
-				return true
-			}
-
-			return false
+			// We're listening for events on the sender node. We
+			// therefore expect to receive deliver transfer type
+			// backoff wait events for sending transfers.
+			return AssertSendEventProofTransferBackoffWaitTypeSend(
+				t, event,
+			)
 		}
 
 		// Lower bound number of proof delivery attempts.

itest/universe_pagination_test.go

@@ -88,7 +88,7 @@ func mintBatchAssetsTest(
 
 	// Update the asset name and metadata to match an index.
 	incrementMintAsset := func(asset *mintrpc.MintAsset, ind int) {
-		asset.Name = asset.Name + strconv.Itoa(ind)
+		asset.Name += strconv.Itoa(ind)
 		binary.PutUvarint(metadataPrefix, uint64(ind))
 		copy(asset.AssetMeta.Data[0:metaPrefixSize], metadataPrefix)
 	}
@@ -180,7 +180,7 @@ func mintBatchAssetsTest(
 		// all outpoints matching the chain anchor of the group anchor.
 		mintOutpoint := collectibleAnchor.ChainAnchor.AnchorOutpoint
 
-		leafKeys, err := fetchAllLeafKeys(t, alice, &collectUniID)
+		leafKeys, err := fetchAllLeafKeys(alice, &collectUniID)
 		require.NoError(t, err)
 
 		require.Len(t, leafKeys, len(mintBatch))
@@ -218,8 +218,8 @@ func mintBatchAssetsTest(
 }
 
 // fetchAllLeafKeys fetches all leaf keys for a given universe ID.
-func fetchAllLeafKeys(t *testing.T, alice TapdClient,
-	id *unirpc.ID) ([]*unirpc.AssetKey, error) {
+func fetchAllLeafKeys(alice TapdClient, id *unirpc.ID) ([]*unirpc.AssetKey,
+	error) {
 
 	keys := make([]*unirpc.AssetKey, 0)
 	offset := int32(0)