Linter PR 3: enable additional linters

.circleci/config.yml

@@ -168,7 +168,7 @@ jobs:
 
   format-go:
     docker:
-      - image: cimg/go:1.21.4
+      - image: cimg/go:1.23.4
     steps:
       - run:
           name: Install gofumpt
@@ -186,7 +186,7 @@ jobs:
   # Build types and cosmwam package without cgo
   wasmvm_no_cgo:
     docker:
-      - image: cimg/go:1.21.4
+      - image: cimg/go:1.23.4
     steps:
       - checkout
       - run:
@@ -205,7 +205,7 @@ jobs:
   # Build types and cosmwasm with libwasmvm linking disabled
   nolink_libwasmvm:
     docker:
-      - image: cimg/go:1.21.4
+      - image: cimg/go:1.23.4
     steps:
       - checkout
       - run:
@@ -223,7 +223,7 @@ jobs:
 
   tidy-go:
     docker:
-      - image: cimg/go:1.21.4
+      - image: cimg/go:1.23.4
     steps:
       - checkout
       - run:
@@ -241,7 +241,7 @@ jobs:
 
   format-scripts:
     docker:
-      - image: cimg/go:1.21.4
+      - image: cimg/go:1.23.4
     steps:
       - run:
           name: Install shfmt
@@ -298,7 +298,7 @@ jobs:
   # Test the Go project and run benchmarks
   wasmvm_test:
     docker:
-      - image: cimg/go:1.21.4
+      - image: cimg/go:1.23.4
     environment:
       GORACE: "halt_on_error=1"
       BUILD_VERSION: $(echo ${CIRCLE_SHA1} | cut -c 1-10)

.github/workflows/lint-go.yml

@@ -20,6 +20,11 @@ jobs:
       - uses: actions/checkout@v4
       - uses: actions/setup-go@v5
         with:
+<<<<<<< HEAD
+
+
+=======
+>>>>>>> faddat/errcheck
           go-version: "1.23.4"
           cache: false
       - name: golangci-lint

.golangci.yml

@@ -1,13 +1,33 @@
 run:
   tests: true
+  timeout: 5m
 
 linters:
   # Enable specific linter
   # https://golangci-lint.run/usage/linters/#enabled-by-default
   enable:
-    - gofumpt
-    - gci
-    - testifylint
+    - copyloopvar # Detect copy loops
+    - errcheck # Detect unchecked errors
+    - gosimple # Simplify code
+    - govet # Reports suspicious constructs
+    - ineffassign # Detect unused assignments
+    - staticcheck # Go static analysis
+    - typecheck # Go type checker
+    - unused # Detect unused constants, variables, functions and types
+
+    # Additional recommended linters
+    - gocritic # A more opinionated linter
+    - gosec # Security checker
+    - misspell # Find commonly misspelled words
+    - revive # a metalinter with more checks
+    - bodyclose # Check HTTP response bodies are closed
+    - goconst # Find repeated strings that could be constants
+    #    - gocyclo # Check function complexity
+    - godot # Check comment endings
+    - gocognit # Check cognitive complexity
+    - whitespace # Check trailing whitespace
+    - thelper # Detect test helpers not using t.Helper()
+    - tparallel # Detect incorrect usage of t.Parallel()
 
 linters-settings:
   gci:
@@ -35,7 +55,30 @@ linters-settings:
     # Drops lexical ordering for custom sections.
     # Default: false
     no-lex-order: true
+  gocritic:
+    # Enable all gocritic checks.
+    disabled-checks:
+      - dupSubExpr
+  gocyclo:
+    min-complexity: 15
+  gocognit:
+    min-complexity: 20
+  dupl:
+    threshold: 100
+  goconst:
+    min-len: 3
+    min-occurrences: 3
+  revive:
+    rules:
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#var-naming
+      - name: var-naming
+        severity: warning
+        disabled: true
 
 issues:
+  exclude-use-default: false
   max-issues-per-linter: 0
   max-same-issues: 0
+  exclude-dirs:
+    - vendor/
+    - third_party/

cmd/demo/main.go

@@ -4,53 +4,91 @@ import (
 	"fmt"
 	"math"
 	"os"
+	"path/filepath"
+	"strings"
 
 	wasmvm "github.com/CosmWasm/wasmvm/v2"
 )
 
+// PrintDebug enables debug printing when true.
 const (
-	PRINT_DEBUG  = true
-	MEMORY_LIMIT = 32  // MiB
-	CACHE_SIZE   = 100 // MiB
+	PrintDebug = true
+	// MemoryLimit defines the memory limit in MiB.
+	MemoryLimit = 32
+	// CacheSize defines the cache size in MiB.
+	CacheSize = 100
 )
 
-var SUPPORTED_CAPABILITIES = []string{"staking"}
+// SupportedCapabilities defines the list of supported staking capabilities.
+var SupportedCapabilities = []string{"staking"}
 
-// This is just a demo to ensure we can compile a static go binary
+// exitCode tracks the code that the program will exit with.
+var exitCode = 0
+
+// main is the entry point for the demo application that tests wasmvm functionality.
 func main() {
+	defer func() {
+		os.Exit(exitCode)
+	}()
+
+	if len(os.Args) < 2 {
+		fmt.Println("Usage: demo <file|version>")
+		exitCode = 1
+		return
+	}
+
 	file := os.Args[1]
 
 	if file == "version" {
 		libwasmvmVersion, err := wasmvm.LibwasmvmVersion()
 		if err != nil {
-			panic(err)
+			fmt.Printf("Error getting libwasmvm version: %v\n", err)
+			exitCode = 1
+			return
 		}
 		fmt.Printf("libwasmvm: %s\n", libwasmvmVersion)
 		return
 	}
 
 	fmt.Printf("Running %s...\n", file)
-	bz, err := os.ReadFile(file)
+
+	// Validate file path
+	cleanPath := filepath.Clean(file)
+	if filepath.IsAbs(cleanPath) || strings.Contains(cleanPath, "..") {
+		fmt.Println("Error: invalid file path")
+		exitCode = 1
+		return
+	}
+
+	bz, err := os.ReadFile(cleanPath)
 	if err != nil {
-		panic(err)
+		fmt.Printf("Error reading file: %v\n", err)
+		exitCode = 1
+		return
 	}
 	fmt.Println("Loaded!")
 
-	err = os.MkdirAll("tmp", 0o755)
+	err = os.MkdirAll("tmp", 0o750)
 	if err != nil {
-		panic(err)
+		fmt.Printf("Error creating tmp directory: %v\n", err)
+		exitCode = 1
+		return
 	}
-	vm, err := wasmvm.NewVM("tmp", SUPPORTED_CAPABILITIES, MEMORY_LIMIT, PRINT_DEBUG, CACHE_SIZE)
+	vm, err := wasmvm.NewVM("tmp", SupportedCapabilities, MemoryLimit, PrintDebug, CacheSize)
 	if err != nil {
-		panic(err)
+		fmt.Printf("Error creating VM: %v\n", err)
+		exitCode = 1
+		return
 	}
+	defer vm.Cleanup()
 
 	checksum, _, err := vm.StoreCode(bz, math.MaxUint64)
 	if err != nil {
-		panic(err)
+		fmt.Printf("Error storing code: %v\n", err)
+		exitCode = 1
+		return
 	}
 	fmt.Printf("Stored code with checksum: %X\n", checksum)
 
-	vm.Cleanup()
 	fmt.Println("finished")
 }

go.mod

@@ -1,6 +1,6 @@
 module github.com/CosmWasm/wasmvm/v2
 
-go 1.21
+go 1.23
 
 require (
 	github.com/google/btree v1.0.0

ibc_test.go

@@ -70,12 +70,13 @@ type AccountInfo struct {
 	ChannelID string `json:"channel_id"`
 }
 
-// We just check if an error is returned or not
+// We just check if an error is returned or not.
 type AcknowledgeDispatch struct {
 	Err string `json:"error"`
 }
 
 func toBytes(t *testing.T, v interface{}) []byte {
+	t.Helper()
 	bz, err := json.Marshal(v)
 	require.NoError(t, err)
 	return bz

internal/api/callbacks.go

@@ -158,7 +158,7 @@ func cGet(ptr *C.db_t, gasMeter *C.gas_meter_t, usedGas *cu64, key C.U8SliceView
 		return C.GoError_BadArgument
 	}
 	// errOut is unused and we don't check `is_none` because of https://github.com/CosmWasm/wasmvm/issues/536
-	if !(*val).is_none {
+	if !val.is_none {
 		panic("Got a non-none UnmanagedVector we're about to override. This is a bug because someone has to drop the old one.")
 	}
 
@@ -231,7 +231,7 @@ func cScan(ptr *C.db_t, gasMeter *C.gas_meter_t, usedGas *cu64, start C.U8SliceV
 		// we received an invalid pointer
 		return C.GoError_BadArgument
 	}
-	if !(*errOut).is_none {
+	if !errOut.is_none {
 		panic("Got a non-none UnmanagedVector we're about to override. This is a bug because someone has to drop the old one.")
 	}
 
@@ -272,19 +272,13 @@ func cScan(ptr *C.db_t, gasMeter *C.gas_meter_t, usedGas *cu64, start C.U8SliceV
 
 //export cNext
 func cNext(ref C.IteratorReference, gasMeter *C.gas_meter_t, usedGas *cu64, key *C.UnmanagedVector, val *C.UnmanagedVector, errOut *C.UnmanagedVector) (ret C.GoError) {
-	// typical usage of iterator
-	// 	for ; itr.Valid(); itr.Next() {
-	// 		k, v := itr.Key(); itr.Value()
-	// 		...
-	// 	}
-
 	defer recoverPanic(&ret)
 	if ref.call_id == 0 || gasMeter == nil || usedGas == nil || key == nil || val == nil || errOut == nil {
 		// we received an invalid pointer
 		return C.GoError_BadArgument
 	}
 	// errOut is unused and we don't check `is_none` because of https://github.com/CosmWasm/wasmvm/issues/536
-	if !(*key).is_none || !(*val).is_none {
+	if !key.is_none || !val.is_none {
 		panic("Got a non-none UnmanagedVector we're about to override. This is a bug because someone has to drop the old one.")
 	}
 
@@ -336,7 +330,7 @@ func nextPart(ref C.IteratorReference, gasMeter *C.gas_meter_t, usedGas *cu64, o
 		return C.GoError_BadArgument
 	}
 	// errOut is unused and we don't check `is_none` because of https://github.com/CosmWasm/wasmvm/issues/536
-	if !(*output).is_none {
+	if !output.is_none {
 		panic("Got a non-none UnmanagedVector we're about to override. This is a bug because someone has to drop the old one.")
 	}
 
@@ -384,7 +378,7 @@ func cHumanizeAddress(ptr *C.api_t, src C.U8SliceView, dest *C.UnmanagedVector,
 	if dest == nil || errOut == nil {
 		return C.GoError_BadArgument
 	}
-	if !(*dest).is_none || !(*errOut).is_none {
+	if !dest.is_none || !errOut.is_none {
 		panic("Got a non-none UnmanagedVector we're about to override. This is a bug because someone has to drop the old one.")
 	}
 
@@ -412,7 +406,7 @@ func cCanonicalizeAddress(ptr *C.api_t, src C.U8SliceView, dest *C.UnmanagedVect
 	if dest == nil || errOut == nil {
 		return C.GoError_BadArgument
 	}
-	if !(*dest).is_none || !(*errOut).is_none {
+	if !dest.is_none || !errOut.is_none {
 		panic("Got a non-none UnmanagedVector we're about to override. This is a bug because someone has to drop the old one.")
 	}
 
@@ -439,7 +433,7 @@ func cValidateAddress(ptr *C.api_t, src C.U8SliceView, errOut *C.UnmanagedVector
 	if errOut == nil {
 		return C.GoError_BadArgument
 	}
-	if !(*errOut).is_none {
+	if !errOut.is_none {
 		panic("Got a non-none UnmanagedVector we're about to override. This is a bug because someone has to drop the old one.")
 	}
 
@@ -479,7 +473,7 @@ func cQueryExternal(ptr *C.querier_t, gasLimit cu64, usedGas *cu64, request C.U8
 		// we received an invalid pointer
 		return C.GoError_BadArgument
 	}
-	if !(*result).is_none || !(*errOut).is_none {
+	if !result.is_none || !errOut.is_none {
 		panic("Got a non-none UnmanagedVector we're about to override. This is a bug because someone has to drop the old one.")
 	}
 

internal/api/iterator.go

@@ -3,12 +3,13 @@ package api
 import (
 	"fmt"
 	"math"
+	"os"
 	"sync"
 
 	"github.com/CosmWasm/wasmvm/v2/types"
 )
 
-// frame stores all Iterators for one contract call
+// frame stores all Iterators for one contract call.
 type frame []types.Iterator
 
 // iteratorFrames contains one frame for each contract call, indexed by contract call ID.
@@ -17,7 +18,7 @@ var (
 	iteratorFramesMutex sync.Mutex
 )
 
-// this is a global counter for creating call IDs
+// this is a global counter for creating call IDs.
 var (
 	latestCallID      uint64
 	latestCallIDMutex sync.Mutex
@@ -44,13 +45,17 @@ func removeFrame(callID uint64) frame {
 	return remove
 }
 
-// endCall is called at the end of a contract call to remove one item the iteratorFrames
+// endCall is called at the end of a contract call to remove one item the iteratorFrames.
 func endCall(callID uint64) {
 	// we pull removeFrame in another function so we don't hold the mutex while cleaning up the removed frame
 	remove := removeFrame(callID)
 	// free all iterators in the frame when we release it
 	for _, iter := range remove {
-		iter.Close()
+		if err := iter.Close(); err != nil {
+			// In this cleanup code, we can't do much with the error
+			// Log it to stderr since that's better than silently ignoring it
+			fmt.Fprintf(os.Stderr, "failed to close iterator: %v\n", err)
+		}
 	}
 }