ci: Enable staticcheck lints and fix violations (wormhole-foundation#4424)

* ci: Enable staticcheck lints and fix violations

- Ensure `error` types are the final return value
- Ensure error messages begin with a lowercase letter (official Go style
  recommendation)
- Enable style rule such that Duration variable names don't end with
  time units
- Remove exception for a file that has since been deleted

* fix unit tests errors and apply formatting

* improve variable name

Author: johnsaigle

.golangci.yml

@@ -168,7 +168,7 @@ linters:
         # All of these lints should eventually be added.
         # They occurred during the migration to v2 and were disabled to make the upgrade easier.
         # https://golangci-lint.run/usage/linters/#staticcheck
-        ["all", "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022", "-SA9003", "-QF1003", "-QF1006", "-QF1008", "-QF1011", "-S1009", "-ST1005", "-ST1008", "-ST1011", "-ST1017", "-ST1018", "-ST1019", "-ST1023"]
+        ["all", "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022", "-SA9003", "-QF1003", "-QF1006", "-QF1008", "-QF1011", "-S1009", "-ST1017", "-ST1018", "-ST1019", "-ST1023"]
     unparam:
           # Inspect exported functions.
           #
@@ -187,10 +187,6 @@ linters:
       - legacy
       - std-error-handling
     rules:
-      - linters:
-          - unused
-        path: pkg/supervisor/supervisor_testhelpers.go
-        text: ^func.*supervisor.*(waitSettle|waitSettleError).*$
         # This file contains hard-coded Sui core contract addresses that are marked as hardcoded credentials.
       - path: pkg/txverifier/sui_test.go
         text: 'G101: Potential hardcoded credentials'

node/hack/findmissing/findmissing.go

@@ -19,27 +19,28 @@ var (
 	onlyChain      = flag.String("only", "", "Only check this chain")
 )
 
-func getAdminClient(ctx context.Context, addr string) (*grpc.ClientConn, error, nodev1.NodePrivilegedServiceClient) {
+func getAdminClient(ctx context.Context, addr string) (*grpc.ClientConn, nodev1.NodePrivilegedServiceClient, error) {
 	conn, err := grpc.DialContext(ctx, fmt.Sprintf("unix:///%s", addr), grpc.WithTransportCredentials(insecure.NewCredentials()))
 
 	if err != nil {
 		log.Fatalf("failed to connect to %s: %v", addr, err)
 	}
 
 	c := nodev1.NewNodePrivilegedServiceClient(conn)
-	return conn, err, c
+	return conn, c, err
 }
 
 func main() {
 	flag.Parse()
 
 	ctx := context.Background()
 
-	conn, err, admin := getAdminClient(ctx, *adminRPC)
-	defer conn.Close()
+	conn, admin, err := getAdminClient(ctx, *adminRPC)
 	if err != nil {
+		conn.Close()
 		log.Fatalf("failed to get admin client: %v", err)
 	}
+	defer conn.Close()
 
 	var only vaa.ChainID
 	if *onlyChain != "" {

node/hack/repair_eth/repair_eth.go

@@ -90,15 +90,15 @@ func usesBlockscout(chainId vaa.ChainID) bool {
 	return chainId == vaa.ChainIDOasis || chainId == vaa.ChainIDAurora || chainId == vaa.ChainIDKarura || chainId == vaa.ChainIDAcala
 }
 
-func getAdminClient(ctx context.Context, addr string) (*grpc.ClientConn, error, nodev1.NodePrivilegedServiceClient) {
+func getAdminClient(ctx context.Context, addr string) (*grpc.ClientConn, nodev1.NodePrivilegedServiceClient, error) {
 	conn, err := grpc.DialContext(ctx, fmt.Sprintf("unix:///%s", addr), grpc.WithTransportCredentials(insecure.NewCredentials()))
 
 	if err != nil {
 		log.Fatalf("failed to connect to %s: %v", addr, err)
 	}
 
 	c := nodev1.NewNodePrivilegedServiceClient(conn)
-	return conn, err, c
+	return conn, c, err
 }
 
 type logEntry struct {
@@ -284,11 +284,12 @@ func main() {
 
 	missingMessages := make(map[eth_common.Address]map[uint64]bool)
 
-	conn, err, admin := getAdminClient(ctx, *adminRPC)
-	defer conn.Close()
+	conn, admin, err := getAdminClient(ctx, *adminRPC)
 	if err != nil {
+		conn.Close()
 		log.Fatalf("failed to get admin client: %v", err)
 	}
+	defer conn.Close()
 
 	// A polygon VAA that was not reobserved before the blocks aged out of guardian rpc nodes
 	ignoreAddress, _ := vaa.StringToAddress("0000000000000000000000005a58505a96d1dbf8df91cb21b54419fc36e93fde")

node/hack/repair_solana/repair.go

@@ -33,15 +33,15 @@ const (
 	postMessageInstructionID = 0x01
 )
 
-func getAdminClient(ctx context.Context, addr string) (*grpc.ClientConn, error, nodev1.NodePrivilegedServiceClient) {
+func getAdminClient(ctx context.Context, addr string) (*grpc.ClientConn, nodev1.NodePrivilegedServiceClient, error) {
 	conn, err := grpc.DialContext(ctx, fmt.Sprintf("unix:///%s", addr), grpc.WithTransportCredentials(insecure.NewCredentials()))
 
 	if err != nil {
 		log.Fatalf("failed to connect to %s: %v", addr, err)
 	}
 
 	c := nodev1.NewNodePrivilegedServiceClient(conn)
-	return conn, err, c
+	return conn, c, err
 }
 
 func main() {
@@ -50,11 +50,12 @@ func main() {
 	ctx := context.Background()
 	sr := rpc.New(*solanaRPC)
 
-	conn, err, admin := getAdminClient(ctx, *adminRPC)
-	defer conn.Close()
+	conn, admin, err := getAdminClient(ctx, *adminRPC)
 	if err != nil {
+		conn.Close()
 		log.Fatalf("failed to get admin client: %v", err)
 	}
+	defer conn.Close()
 
 	for _, emitter := range sdk.KnownEmitters {
 		if emitter.ChainID != vaa.ChainIDSolana {

node/hack/repair_terra/repair.go

@@ -60,15 +60,15 @@ var (
 	sleepTime = flag.Int("sleepTime", 1, "Time to sleep between http requests")
 )
 
-func getAdminClient(ctx context.Context, addr string) (*grpc.ClientConn, error, nodev1.NodePrivilegedServiceClient) {
+func getAdminClient(ctx context.Context, addr string) (*grpc.ClientConn, nodev1.NodePrivilegedServiceClient, error) {
 	conn, err := grpc.DialContext(ctx, fmt.Sprintf("unix:///%s", addr), grpc.WithTransportCredentials(insecure.NewCredentials()))
 
 	if err != nil {
 		log.Fatalf("failed to connect to %s: %v", addr, err)
 	}
 
 	c := nodev1.NewNodePrivilegedServiceClient(conn)
-	return conn, err, c
+	return conn, c, err
 }
 
 func getSequencesForTxhash(txhash string, fcd string, contractAddressLogKey string, coreContract string, emitter Emitter, chainID vaa.ChainID) ([]uint64, error) {
@@ -286,11 +286,12 @@ func main() {
 
 	missingMessages := make(map[uint64]bool)
 
-	conn, err, admin := getAdminClient(ctx, *adminRPC)
-	defer conn.Close()
+	conn, admin, err := getAdminClient(ctx, *adminRPC)
 	if err != nil {
+		conn.Close()
 		log.Fatalf("failed to get admin client: %v", err)
 	}
+	defer conn.Close()
 
 	log.Printf("Requesting missing messages for %s", emitter.Emitter)
 

node/pkg/accountant/submit_obs.go

@@ -21,8 +21,10 @@ import (
 	"go.uber.org/zap"
 )
 
-const batchSize = 10
-const delayInMS = 100 * time.Millisecond
+const (
+	batchSize    = 10
+	batchTimeout = 100 * time.Millisecond
+)
 
 // baseWorker is the entry point for the base accountant worker.
 func (acct *Accountant) baseWorker(ctx context.Context) error {
@@ -63,7 +65,7 @@ func (acct *Accountant) worker(ctx context.Context, isNTT bool) error {
 // handleBatch reads a batch of events from the channel, either until a timeout occurs or the batch is full,
 // and submits them to the smart contract.
 func (acct *Accountant) handleBatch(ctx context.Context, subChan chan *common.MessagePublication, wormchainConn AccountantWormchainConn, contract string, prefix []byte, tag string) error {
-	ctx, cancel := context.WithTimeout(ctx, delayInMS)
+	ctx, cancel := context.WithTimeout(ctx, batchTimeout)
 	defer cancel()
 
 	msgs, err := common.ReadFromChannelWithTimeout[*common.MessagePublication](ctx, subChan, batchSize)

node/pkg/guardiansigner/amazonkms.go

@@ -84,7 +84,7 @@ func NewAmazonKmsSigner(ctx context.Context, keyPath string) (*AmazonKms, error)
 	region := getRegionFromArn(keyPath)
 
 	if region == "" {
-		return nil, errors.New("Invalid KMS ARN")
+		return nil, errors.New("invalid KMS ARN")
 	}
 
 	amazonKmsSigner := AmazonKms{
@@ -97,7 +97,7 @@ func NewAmazonKmsSigner(ctx context.Context, keyPath string) (*AmazonKms, error)
 	// an error. This is why the region is first extracted from the keyPath.
 	cfg, err := config.LoadDefaultConfig(timeoutCtx, config.WithDefaultRegion(amazonKmsSigner.region))
 	if err != nil {
-		return nil, errors.New("Failed to load KMS default config")
+		return nil, errors.New("failed to load KMS default config")
 	}
 
 	amazonKmsSigner.client = kms.NewFromConfig(cfg)
@@ -116,12 +116,12 @@ func NewAmazonKmsSigner(ctx context.Context, keyPath string) (*AmazonKms, error)
 	_, err = asn1.Unmarshal(pubKeyOutput.PublicKey, &asn1Pubkey)
 
 	if err != nil {
-		return nil, fmt.Errorf("Failed to unmarshal KMS public key: %w", err)
+		return nil, fmt.Errorf("failed to unmarshal KMS public key: %w", err)
 	}
 
 	// The public key is expected to be at least `MINIMUM_KMS_PUBKEY_LENGTH` bytes long.
 	if len(asn1Pubkey.PublicKey.Bytes) < MINIMUM_KMS_PUBKEY_LENGTH {
-		return nil, errors.New("Invalid KMS public key length")
+		return nil, errors.New("invalid KMS public key length")
 	}
 
 	// It is possible to use `ethcrypto.UnmarshalPubkey(asn1Pubkey.PublicKey.Bytes)`` to get the public key,
@@ -160,7 +160,7 @@ func (a *AmazonKms) Sign(ctx context.Context, hash []byte) (signature []byte, er
 	r, s, err := derSignatureToRS(res.Signature)
 
 	if err != nil {
-		return nil, fmt.Errorf("Failed to decode signature: %w", err)
+		return nil, fmt.Errorf("failed to decode signature: %w", err)
 	}
 
 	// if s is greater than secp256k1HalfN, we need to subtract secp256k1N from it
@@ -196,7 +196,7 @@ func (a *AmazonKms) Sign(ctx context.Context, hash []byte) (signature []byte, er
 
 	// Reaching this return implies that it wasn't possible to generate a valid signature. This shouldn't
 	// happen, unless there is something seriously wrong with the KMS service.
-	return nil, fmt.Errorf("Failed to generate valid signature")
+	return nil, fmt.Errorf("failed to generate valid signature")
 }
 
 func (a *AmazonKms) PublicKey(ctx context.Context) ecdsa.PublicKey {

node/pkg/node/node.go

@@ -53,6 +53,23 @@ const (
 	observationRequestPerChainBufferSize = 100
 )
 
+type ComponentAlreadyConfiguredError struct {
+	componentName string
+}
+
+func (e ComponentAlreadyConfiguredError) Error() string {
+	return fmt.Sprintf("component %s is already configured and cannot be configured a second time", e.componentName)
+}
+
+type ComponentDependencyError struct {
+	componentName  string
+	dependencyName string
+}
+
+func (e ComponentDependencyError) Error() string {
+	return fmt.Sprintf("component %s requires %s to be configured first, check the order of your options", e.componentName, e.dependencyName)
+}
+
 type PrometheusCtxKey struct{}
 
 type G struct {
@@ -156,20 +173,20 @@ func (g *G) applyOptions(ctx context.Context, logger *zap.Logger, options []*Gua
 	for _, option := range options {
 		// check that this component has not been configured yet
 		if _, ok := configuredComponents[option.name]; ok {
-			return fmt.Errorf("Component %s is already configured and cannot be configured a second time.", option.name)
+			return ComponentAlreadyConfiguredError{componentName: option.name}
 		}
 
 		// check that all dependencies have been met
 		for _, dep := range option.dependencies {
 			if _, ok := configuredComponents[dep]; !ok {
-				return fmt.Errorf("Component %s requires %s to be configured first. Check the order of your options.", option.name, dep)
+				return ComponentDependencyError{componentName: option.name, dependencyName: dep}
 			}
 		}
 
 		// run the config
 		err := option.f(ctx, logger, g)
 		if err != nil {
-			return fmt.Errorf("Error applying option for component %s: %w", option.name, err)
+			return fmt.Errorf("error applying option for component %s: %w", option.name, err)
 		}
 
 		// mark the component as configured

node/pkg/node/node_test.go

@@ -927,15 +927,15 @@ func TestGuardianConfigs(t *testing.T) {
 					nil,   // nttWormchainConn
 				),
 			},
-			err: "Check the order of your options.",
+			err: ComponentDependencyError{componentName: "accountant", dependencyName: "db"}.Error(),
 		},
 		{
 			name: "double-configuration",
 			opts: []*GuardianOption{
 				GuardianOptionDatabase(nil),
 				GuardianOptionDatabase(nil),
 			},
-			err: "Component db is already configured and cannot be configured a second time",
+			err: ComponentAlreadyConfiguredError{componentName: "db"}.Error(),
 		},
 	}
 	runGuardianConfigTests(t, tc)
@@ -989,6 +989,7 @@ func runGuardianConfigTests(t *testing.T, testCases []testCaseGuardianConfig) {
 				if tc.err == "" {
 					assert.Equal(t, tc.err, r)
 				}
+				// Check that the string logged by the fatal hook contains the error message.
 				assert.Contains(t, r, tc.err)
 				rootCtxCancel()
 			case <-rootCtx.Done():

node/pkg/txverifier/evm.go

@@ -173,7 +173,7 @@ func (tv *TransferVerifier[ethClient, Connector]) UpdateReceiptDetails(
 	invalidErr := receipt.Validate()
 	if invalidErr != nil {
 		return errors.Join(
-			errors.New("ProcessReceipt was called with an invalid Transfer Receipt:"),
+			errors.New("ProcessReceipt was called with an invalid Transfer Receipt"),
 			invalidErr,
 		)
 	}
@@ -485,7 +485,7 @@ func (tv *TransferVerifier[evmClient, connector]) ProcessReceipt(
 	invalidErr := receipt.Validate()
 	if invalidErr != nil {
 		return nil, errors.Join(
-			errors.New("ProcessReceipt was called with an invalid Transfer Receipt:"),
+			errors.New("ProcessReceipt was called with an invalid Transfer Receipt"),
 			invalidErr,
 		)
 	}
@@ -634,7 +634,7 @@ func parseLogMessagePublishedPayload(
 	// Note: vaa.DecodeTransferPayloadHdr performs validation on data, e.g. length checks.
 	hdr, err := vaa.DecodeTransferPayloadHdr(data)
 	if err != nil {
-		return nil, errors.Join(errors.New("could not parse LogMessagePublished payload:"), err)
+		return nil, errors.Join(errors.New("could not parse LogMessagePublished payload"), err)
 	}
 	return &TransferDetails{
 		PayloadType:      VAAPayloadType(hdr.Type),
@@ -688,7 +688,7 @@ func (tv *TransferVerifier[ethClient, connector]) fetchLogMessageDetails(details
 				zap.String("originAddressRaw", details.OriginAddress.String()),
 				zap.String("tokenChain", details.TokenChain.String()),
 			)
-			return newDetails, errors.New("unwrap call for foreign asset returned the zero address. Either token has not been registered or there is a bug in the program.")
+			return newDetails, errors.New("unwrap call for foreign asset returned the zero address, either token has not been registered or there is a bug in the program")
 		} else {
 			originAddress = unwrappedAddress
 		}