ci: Add govet shadow detection

.golangci.yml

@@ -118,9 +118,7 @@ linters:
         # Regular expressions must match complete canonical struct package/name/structname.
         # If this list is empty, all structs are tested.
         # Default: []
-        - ./+governor.tokenConfigEntry$
-        # TODO: MessagePublications _should_ be exhaustive but this will require many changes.
-        - ./_common.MessagePublication$
+        - .+/governor\.tokenConfigEntry$
       exclude:
         - .+/cobra\.Command$
         - .+/http\.Client$
@@ -143,6 +141,11 @@ linters:
         - sloppyLen
         - underef
         - unslice
+    # TODO: Add this later.
+    # gosec:
+    #   config:
+    #     global:
+    #       audit: true
     govet:
       # The following list includes all the linters that are disabled by default.
       # Uncomment any of these to enable them:
@@ -158,7 +161,7 @@ linters:
         - lostcancel           # Check that context cancel functions are called
         - nilness              # Check for redundant or impossible nil comparisons
         - reflectvaluecompare  # Check for comparing reflect.Value values with == or reflect.DeepEqual
-        # - shadow               # Check for possible unintended shadowing of variables
+        - shadow               # Check for possible unintended shadowing of variables
         - sigchanyzer          # Check for unbuffered channels with signal.Notify
         - slog                 # Check for invalid structured logging calls
         - sortslice            # Check the argument type of sort.Slice

node/cmd/ccq/p2p.go

@@ -67,9 +67,9 @@ func runP2P(
 
 	if len(protectedPeers) != 0 {
 		for _, peerId := range protectedPeers {
-			decodedPeerId, err := peer.Decode(peerId)
-			if err != nil {
-				logger.Error("error decoding protected ccq ID", zap.String("peerId", peerId), zap.Error(err))
+			decodedPeerId, peerErr := peer.Decode(peerId)
+			if peerErr != nil {
+				logger.Error("error decoding protected ccq ID", zap.String("peerId", peerId), zap.Error(peerErr))
 				continue
 			}
 			logger.Info("protecting ccq peer", zap.String("peerId", peerId))
@@ -151,7 +151,7 @@ func runP2P(
 					for _, p := range bootstrappers {
 						if _, exists := peerMap[p.ID.String()]; !exists {
 							logger.Info("attempting to reconnect to peer", zap.String("peer", p.ID.String()))
-							if err := h.Connect(ctx, p); err != nil {
+							if err = h.Connect(ctx, p); err != nil {
 								logger.Error("failed to reconnect to peer", zap.String("peer", p.ID.String()), zap.Error(err))
 							} else {
 								logger.Info("Reconnected to peer", zap.String("peer", p.ID.String()))
@@ -210,7 +210,7 @@ func runP2P(
 				logger.Info("query response received from gossip", zap.String("peerId", peerId), zap.Any("requestId", requestSignature))
 				if loggingMap.ShouldLogResponse(requestSignature) {
 					var queryRequest query.QueryRequest
-					if err := queryRequest.Unmarshal(queryResponse.Request.QueryRequest); err == nil {
+					if unmarshalErr := queryRequest.Unmarshal(queryResponse.Request.QueryRequest); unmarshalErr == nil {
 						logger.Info("logging response", zap.String("peerId", peerId), zap.Any("requestId", requestSignature), zap.Any("request", queryRequest), zap.Any("response", queryResponse))
 					} else {
 						logger.Error("logging response (failed to unmarshal request)", zap.String("peerId", peerId), zap.Any("requestId", requestSignature), zap.Any("response", queryResponse))

node/cmd/ccq/query_server.go

@@ -99,9 +99,9 @@ func runQueryServer(cmd *cobra.Command, args []string) {
 	}
 
 	if *verifyPermissions {
-		_, err := parseConfigFile(*permFile, env)
-		if err != nil {
-			fmt.Println(err)
+		_, parseErr := parseConfigFile(*permFile, env)
+		if parseErr != nil {
+			fmt.Println(parseErr)
 			os.Exit(1)
 		}
 		os.Exit(0)
@@ -151,9 +151,9 @@ func runQueryServer(cmd *cobra.Command, args []string) {
 			"version":   version.Version(),
 		}
 
-		tm, err := telemetry.NewLokiCloudLogger(context.Background(), logger, *telemetryLokiURL, "ccq_server", true, labels)
-		if err != nil {
-			logger.Fatal("Failed to initialize telemetry", zap.Error(err))
+		tm, lokiErr := telemetry.NewLokiCloudLogger(context.Background(), logger, *telemetryLokiURL, "ccq_server", true, labels)
+		if lokiErr != nil {
+			logger.Fatal("Failed to initialize telemetry", zap.Error(lokiErr))
 		}
 
 		defer tm.Close()
@@ -215,9 +215,8 @@ func runQueryServer(cmd *cobra.Command, args []string) {
 	go func() {
 		s := NewHTTPServer(*listenAddr, p2pSub.topic_req, permissions, signerKey, pendingResponses, logger, env, loggingMap)
 		logger.Sugar().Infof("Server listening on %s", *listenAddr)
-		err := s.ListenAndServe()
-		if err != nil && err != http.ErrServerClosed {
-			logger.Fatal("Server closed unexpectedly", zap.Error(err))
+		if serveErr := s.ListenAndServe(); serveErr != nil && serveErr != http.ErrServerClosed {
+			logger.Fatal("Server closed unexpectedly", zap.Error(serveErr))
 		}
 	}()
 
@@ -227,9 +226,8 @@ func runQueryServer(cmd *cobra.Command, args []string) {
 		statServer = NewStatusServer(*statusAddr, logger, env)
 		go func() {
 			logger.Sugar().Infof("Status server listening on %s", *statusAddr)
-			err := statServer.httpServer.ListenAndServe()
-			if err != nil && err != http.ErrServerClosed {
-				logger.Fatal("Status server closed unexpectedly", zap.Error(err))
+			if serveErr := statServer.httpServer.ListenAndServe(); serveErr != nil && serveErr != http.ErrServerClosed {
+				logger.Fatal("Status server closed unexpectedly", zap.Error(serveErr))
 			}
 		}()
 	}

node/cmd/guardiand/adminclient.go

@@ -720,10 +720,10 @@ func runChainGovernorResetReleaseTimer(cmd *cobra.Command, args []string) {
 	// defaults to 1 day if num_days isn't specified
 	numDays := uint32(1)
 	if len(args) > 1 {
-		numDaysArg, err := strconv.Atoi(args[1])
+		numDaysArg, parseErr := strconv.Atoi(args[1])
 
-		if numDaysArg > math.MaxUint32 || err != nil {
-			log.Fatalf("invalid num_days: %v", err)
+		if numDaysArg > math.MaxUint32 || parseErr != nil {
+			log.Fatalf("invalid num_days: %v", parseErr)
 		}
 
 		numDays = uint32(numDaysArg) // #nosec G115 -- This is validated above
@@ -850,12 +850,12 @@ func runSignExistingVaasFromCSV(cmd *cobra.Command, args []string) {
 	oldVAAReader := csv.NewReader(oldVAAFile)
 	numOldVAAs := 0
 	for {
-		row, err := oldVAAReader.Read()
-		if err != nil {
-			if err == io.EOF {
+		row, readErr := oldVAAReader.Read()
+		if readErr != nil {
+			if readErr == io.EOF {
 				break
 			}
-			log.Fatalf("failed to parse VAA CSV: %v", err)
+			log.Fatalf("failed to parse VAA CSV: %v", readErr)
 		}
 		if len(row) != 2 {
 			log.Fatalf("row [%d] does not have 2 elements", numOldVAAs)

node/cmd/guardiand/admintemplate.go

@@ -1413,28 +1413,28 @@ func runDelegatedManagerSetUpdateTemplate(cmd *cobra.Command, args []string) {
 		// Use raw manager set bytes if provided
 		managerSet = strings.TrimPrefix(*delegatedManagerSet, "0x")
 		// Validate it's valid hex
-		if _, err := hex.DecodeString(managerSet); err != nil {
-			log.Fatal("invalid manager-set (expected hex): ", err)
+		if _, hexErr := hex.DecodeString(managerSet); hexErr != nil {
+			log.Fatal("invalid manager-set (expected hex): ", hexErr)
 		}
 	} else if *delegatedManagerThreshold != "" && *delegatedManagerNumKeys != "" && *delegatedManagerPublicKeys != "" {
 		// Build secp256k1 multisig manager set from components
-		threshold, err := strconv.ParseUint(*delegatedManagerThreshold, 10, 8)
-		if err != nil {
-			log.Fatal("failed to parse threshold as uint8: ", err)
+		threshold, thresholdErr := strconv.ParseUint(*delegatedManagerThreshold, 10, 8)
+		if thresholdErr != nil {
+			log.Fatal("failed to parse threshold as uint8: ", thresholdErr)
 		}
-		numKeys, err := strconv.ParseUint(*delegatedManagerNumKeys, 10, 8)
-		if err != nil {
-			log.Fatal("failed to parse num-keys as uint8: ", err)
+		numKeys, numKeysErr := strconv.ParseUint(*delegatedManagerNumKeys, 10, 8)
+		if numKeysErr != nil {
+			log.Fatal("failed to parse num-keys as uint8: ", numKeysErr)
 		}
 		publicKeyStrs := strings.Split(*delegatedManagerPublicKeys, ",")
 
 		// Parse public keys into the format expected by vaa.Secp256k1MultisigManagerSet
 		publicKeys := make([][vaa.CompressedSecp256k1PublicKeyLength]byte, len(publicKeyStrs))
 		for i, pkStr := range publicKeyStrs {
 			pkHex := strings.TrimPrefix(strings.TrimSpace(pkStr), "0x")
-			pkBytes, err := hex.DecodeString(pkHex)
-			if err != nil {
-				log.Fatalf("public key %d is not valid hex: %v", i, err)
+			pkBytes, pkErr := hex.DecodeString(pkHex)
+			if pkErr != nil {
+				log.Fatalf("public key %d is not valid hex: %v", i, pkErr)
 			}
 			if len(pkBytes) != vaa.CompressedSecp256k1PublicKeyLength {
 				log.Fatalf("public key %d has invalid length: expected %d bytes, got %d", i, vaa.CompressedSecp256k1PublicKeyLength, len(pkBytes))
@@ -1447,9 +1447,9 @@ func runDelegatedManagerSetUpdateTemplate(cmd *cobra.Command, args []string) {
 			N:          uint8(numKeys),
 			PublicKeys: publicKeys,
 		}
-		managerSetBytes, err := managerSetStruct.Serialize()
-		if err != nil {
-			log.Fatal("failed to serialize manager set: ", err)
+		managerSetBytes, serializeErr := managerSetStruct.Serialize()
+		if serializeErr != nil {
+			log.Fatal("failed to serialize manager set: ", serializeErr)
 		}
 		managerSet = hex.EncodeToString(managerSetBytes)
 	} else {

node/cmd/guardiand/node.go

@@ -671,9 +671,9 @@ func runNode(cmd *cobra.Command, args []string) {
 	if env == common.UnsafeDevNet {
 		// Use the hostname as nodeName. For production, we don't want to do this to
 		// prevent accidentally leaking sensitive hostnames.
-		hostname, err := os.Hostname()
-		if err != nil {
-			panic(err)
+		hostname, hostErr := os.Hostname()
+		if hostErr != nil {
+			panic(hostErr)
 		}
 		*nodeName = hostname
 
@@ -691,9 +691,9 @@ func runNode(cmd *cobra.Command, args []string) {
 
 	// In devnet mode, we automatically set a number of flags that rely on deterministic keys.
 	if env == common.UnsafeDevNet {
-		g0key, err := peer.IDFromPrivateKey(devnet.DeterministicP2PPrivKeyByIndex(0))
-		if err != nil {
-			panic(err)
+		g0key, keyErr := peer.IDFromPrivateKey(devnet.DeterministicP2PPrivKeyByIndex(0))
+		if keyErr != nil {
+			panic(keyErr)
 		}
 
 		// Use the first guardian node as bootstrap
@@ -780,8 +780,8 @@ func runNode(cmd *cobra.Command, args []string) {
 	if env == common.UnsafeDevNet {
 		// Only if the signer is file-based should we generate the deterministic key and write it to disk
 		if st, _, _ := guardiansigner.ParseSignerUri(*guardianSignerUri); st == guardiansigner.FileSignerType {
-			err := devnet.GenerateAndStoreDevnetGuardianKey(*guardianKeyPath)
-			if err != nil {
+			genErr := devnet.GenerateAndStoreDevnetGuardianKey(*guardianKeyPath)
+			if genErr != nil {
 				logger.Fatal("failed to generate devnet guardian key", zap.Error(err))
 			}
 		}
@@ -803,22 +803,22 @@ func runNode(cmd *cobra.Command, args []string) {
 	// Load p2p private key
 	var p2pKey libp2p_crypto.PrivKey
 	if env == common.UnsafeDevNet {
-		idx, err := devnet.GetDevnetIndex()
-		if err != nil {
+		idx, idxErr := devnet.GetDevnetIndex()
+		if idxErr != nil {
 			logger.Fatal("Failed to parse hostname - are we running in devnet?")
 		}
 		p2pKey = devnet.DeterministicP2PPrivKeyByIndex(int64(idx))
 
 		if idx != 0 {
-			firstGuardianName, err := devnet.GetFirstGuardianNameFromBootstrapPeers(*p2pBootstrap)
-			if err != nil {
-				logger.Fatal("failed to get first guardian name from bootstrap peers", zap.String("bootstrapPeers", *p2pBootstrap), zap.Error(err))
+			firstGuardianName, lookupErr := devnet.GetFirstGuardianNameFromBootstrapPeers(*p2pBootstrap)
+			if lookupErr != nil {
+				logger.Fatal("failed to get first guardian name from bootstrap peers", zap.String("bootstrapPeers", *p2pBootstrap), zap.Error(lookupErr))
 			}
 			// try to connect to guardian-0
 			for {
 				//nolint:noctx // TODO: this should be refactored to use context.
-				_, err := net.LookupIP(firstGuardianName)
-				if err == nil {
+				_, resolveErr := net.LookupIP(firstGuardianName)
+				if resolveErr == nil {
 					break
 				}
 				logger.Info(fmt.Sprintf("Error resolving %s. Trying again...", firstGuardianName))

node/cmd/spy/spy.go

@@ -300,14 +300,14 @@ func runSpy(cmd *cobra.Command, args []string) {
 		if *p2pNetworkID != "" || *p2pBootstrap != "" {
 			logger.Fatal(`If "--env" is specified, "--network" and "--bootstrap" may not be specified`)
 		}
-		env, err := common.ParseEnvironment(*envStr)
-		if err != nil || (env != common.MainNet && env != common.TestNet) {
+		env, envErr := common.ParseEnvironment(*envStr)
+		if envErr != nil || (env != common.MainNet && env != common.TestNet) {
 			logger.Fatal(`Invalid value for "--env", should be "mainnet" or "testnet"`)
 		}
 		*p2pNetworkID = p2p.GetNetworkId(env)
-		*p2pBootstrap, err = p2p.GetBootstrapPeers(env)
-		if err != nil {
-			logger.Fatal("failed to determine p2p bootstrap peers", zap.String("env", string(env)), zap.Error(err))
+		*p2pBootstrap, envErr = p2p.GetBootstrapPeers(env)
+		if envErr != nil {
+			logger.Fatal("failed to determine p2p bootstrap peers", zap.String("env", string(env)), zap.Error(envErr))
 		}
 	} else {
 		// If they don't specify --env, then --network and --bootstrap are required.
@@ -363,8 +363,8 @@ func runSpy(cmd *cobra.Command, args []string) {
 			logger.Fatal(`If "--ethRPC" is specified, "--ethContract" must also be specified`)
 		}
 		s.vaaVerifier = NewVaaVerifier(logger, *ethRPC, *ethContract)
-		if err := s.vaaVerifier.GetInitialGuardianSet(); err != nil {
-			logger.Fatal(`Failed to read initial guardian set for VAA verification`, zap.Error(err))
+		if verifierErr := s.vaaVerifier.GetInitialGuardianSet(); verifierErr != nil {
+			logger.Fatal(`Failed to read initial guardian set for VAA verification`, zap.Error(verifierErr))
 		}
 	}
 
@@ -377,8 +377,8 @@ func runSpy(cmd *cobra.Command, args []string) {
 			case v := <-signedInC:
 				logger.Info("Received signed VAA",
 					zap.Any("vaa", v.Vaa))
-				if err := s.PublishSignedVAA(v.Vaa); err != nil {
-					logger.Error("failed to publish signed VAA", zap.Error(err), zap.Any("vaa", v.Vaa))
+				if pubErr := s.PublishSignedVAA(v.Vaa); pubErr != nil {
+					logger.Error("failed to publish signed VAA", zap.Error(pubErr), zap.Any("vaa", v.Vaa))
 				}
 			}
 		}

node/cmd/spy/spy_test.go

@@ -171,19 +171,19 @@ func TestSpyHandleGossipVAA(t *testing.T) {
 	go func() {
 		defer close(doneCh)
 		// receive is a blocking call, it will keep receiving/looping until the pipe breaks.
-		signedVAA, err := stream.Recv()
-		if errors.Is(err, io.EOF) {
+		signedVAA, recvErr := stream.Recv()
+		if errors.Is(recvErr, io.EOF) {
 			t.Log("the SignedVAA stream has closed, err == io.EOF. going to break.")
 			t.Fail()
 			return
 		}
-		if err != nil {
+		if recvErr != nil {
 			t.Log("SubscribeSignedVAA returned an error.")
 			t.Fail()
 			return
 		}
-		parsedRes, err := vaa.Unmarshal(signedVAA.VaaBytes)
-		if err != nil {
+		parsedRes, unmarshalErr := vaa.Unmarshal(signedVAA.VaaBytes)
+		if unmarshalErr != nil {
 			t.Log("failed unmarshaling VAA from response")
 			t.Fail()
 			return
@@ -236,19 +236,19 @@ func TestSpyHandleEmitterFilter(t *testing.T) {
 	go func() {
 		defer close(doneCh)
 		// receive is a blocking call, it will keep receiving/looping until the pipe breaks.
-		signedVAA, err := emitterFilterStream.Recv()
-		if errors.Is(err, io.EOF) {
+		signedVAA, recvErr := emitterFilterStream.Recv()
+		if errors.Is(recvErr, io.EOF) {
 			t.Log("the SignedVAA stream has closed, err == io.EOF. going to break.")
 			t.Fail()
 			return
 		}
-		if err != nil {
+		if recvErr != nil {
 			t.Log("SubscribeSignedVAA returned an error.")
 			t.Fail()
 			return
 		}
-		_, err = vaa.Unmarshal(signedVAA.VaaBytes)
-		if err != nil {
+		_, unmarshalErr := vaa.Unmarshal(signedVAA.VaaBytes)
+		if unmarshalErr != nil {
 			t.Log("failed unmarshaling VAA from response")
 			t.Fail()
 			return