smaller PR

.golangci.yml

@@ -2,6 +2,7 @@ version: "2"
 
 run:
   tests: true
+  timeout: 5m
 
 linters:
   # Enable specific linters
@@ -10,6 +11,38 @@ linters:
     - misspell
     - testifylint
     - thelper
+    - copyloopvar # Detect copy loops
+    - errcheck # Detect unchecked errors
+    - govet # Reports suspicious constructs
+    - ineffassign # Detect unused assignments
+    - staticcheck # Go static analysis
+    - unused # Detect unused constants, variables, functions and types
+
+    # Additional recommended linters
+    - gosec # Security checker
+    - misspell # Find commonly misspelled words
+    - bodyclose # Check HTTP response bodies are closed
+    - goconst # Find repeated strings that could be constants
+    - gocognit # Check cognitive complexity
+    - whitespace # Check trailing whitespace
+    - thelper # Detect test helpers not using t.Helper()
+    - tparallel # Detect incorrect usage of t.Parallel()
+
+  # enable lager because changes are huge
+    # - revive
+    #    - gocritic # A more opinionated linter
+
+
+  settings:
+    gocritic:
+      enable-all: true
+      disabled-checks:
+        - dupSubExpr
+        - dupImport
+        - paramTypeCombine
+
+
+
   exclusions:
     generated: lax
     presets:

cmd/demo/main.go

@@ -4,53 +4,91 @@ import (
 	"fmt"
 	"math"
 	"os"
+	"path/filepath"
+	"strings"
 
 	wasmvm "github.com/CosmWasm/wasmvm/v3"
 )
 
+// 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")
 }

ibc_test.go

@@ -70,7 +70,7 @@ 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"`
 }

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.")
 	}
 
@@ -277,14 +277,13 @@ func cNext(ref C.IteratorReference, gasMeter *C.gas_meter_t, usedGas *cu64, key
 	// 		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 +335,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.")
 	}
 
@@ -356,7 +355,7 @@ func nextPart(ref C.IteratorReference, gasMeter *C.gas_meter_t, usedGas *cu64, o
 	// check iter.Error() ????
 	iter.Next()
 	gasAfter := gm.GasConsumed()
-	*usedGas = (cu64)(gasAfter - gasBefore)
+	*usedGas = (cu64(gasAfter - gasBefore))
 
 	*output = newUnmanagedVector(out)
 	return C.GoError_None
@@ -384,7 +383,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.")
 	}
 
@@ -398,7 +397,7 @@ func cHumanizeAddress(ptr *C.api_t, src C.U8SliceView, dest *C.UnmanagedVector,
 		*errOut = newUnmanagedVector([]byte(err.Error()))
 		return C.GoError_User
 	}
-	if len(h) == 0 {
+	if h == "" {
 		panic(fmt.Sprintf("`api.HumanizeAddress()` returned an empty string for %q", s))
 	}
 	*dest = newUnmanagedVector([]byte(h))
@@ -412,7 +411,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 +438,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 +478,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/v3/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)
+		}
 	}
 }
 

internal/api/lib.go

@@ -44,7 +44,7 @@ type Cache struct {
 type Querier = types.Querier
 
 func InitCache(config types.VMConfig) (Cache, error) {
-	// libwasmvm would create this directory too but we need it earlier for the lockfile
+	// libwasmvm would create this directory too but we need it earlier for the lockfile.
 	err := os.MkdirAll(config.Cache.BaseDir, 0o755)
 	if err != nil {
 		return Cache{}, fmt.Errorf("could not create base directory")
@@ -83,7 +83,10 @@ func InitCache(config types.VMConfig) (Cache, error) {
 func ReleaseCache(cache Cache) {
 	C.release_cache(cache.ptr)
 
-	cache.lockfile.Close() // Also releases the file lock
+	if err := cache.lockfile.Close(); err != nil {
+		// Just log the error since this is cleanup code
+		fmt.Printf("failed to close lockfile: %v\n", err)
+	} // Also releases the file lock.
 }
 
 func StoreCode(cache Cache, wasm []byte, persist bool) ([]byte, error) {