Linter: to check tx.Rollback() by ruleguard

.golangci.yml

@@ -14,8 +14,15 @@ linters:
     - structcheck
     - unused
     - varcheck
+    - gocritic
 
 linters-settings:
+  gocritic:
+    enabled-checks:
+      - ruleguard
+    settings:
+      ruleguard:
+        rules: "rules.go"
   govet:
     disable:
       - deepequalerrors

cmd/integration/commands/refetence_db.go

@@ -255,9 +255,7 @@ func fToMdbx(ctx context.Context, to string) error {
 	if err1 != nil {
 		return err1
 	}
-	defer func() {
-		dstTx.Rollback()
-	}()
+	defer dstTx.Rollback()
 
 	commitEvery := time.NewTicker(5 * time.Second)
 	defer commitEvery.Stop()
@@ -341,14 +339,6 @@ MainLoop:
 	if err != nil {
 		return err
 	}
-	dstTx, err = dst.BeginRw(ctx)
-	if err != nil {
-		return err
-	}
-	err = dstTx.Commit()
-	if err != nil {
-		return err
-	}
 
 	return nil
 }
@@ -370,9 +360,7 @@ func kv2kv(ctx context.Context, src, dst ethdb.RwKV) error {
 	if err1 != nil {
 		return err1
 	}
-	defer func() {
-		dstTx.Rollback()
-	}()
+	defer dstTx.Rollback()
 
 	commitEvery := time.NewTicker(30 * time.Second)
 	defer commitEvery.Stop()
@@ -419,6 +407,7 @@ func kv2kv(ctx context.Context, src, dst ethdb.RwKV) error {
 				if err != nil {
 					return err
 				}
+				defer dstTx.Rollback()
 				c, err = dstTx.RwCursor(name)
 				if err != nil {
 					return err
@@ -442,14 +431,6 @@ func kv2kv(ctx context.Context, src, dst ethdb.RwKV) error {
 	if err != nil {
 		return err
 	}
-	dstTx, err = dst.BeginRw(ctx)
-	if err != nil {
-		return err
-	}
-	err = dstTx.Commit()
-	if err != nil {
-		return err
-	}
 	srcTx.Rollback()
 	log.Info("done")
 	return nil

cmd/snapshots/debug/debug_test.go

@@ -168,6 +168,7 @@ func TestMatreshkaStream(t *testing.T) {
 			if err != nil {
 				t.Fatal(err, currentBlock)
 			}
+			defer tx.Rollback()
 
 			dr := time.Since(ttt)
 			fmt.Println(currentBlock, "finished", "acc-", accDiffLen, "st-", stDiffLen, "codes - ", codesDiffLen, "all -", time.Since(tt), "chunk - ", dr, "blocks/s", 10000/dr.Seconds())

cmd/state/commands/check_change_sets.go

@@ -230,6 +230,7 @@ func CheckChangeSets(genesis *core.Genesis, blockNum uint64, chaindata string, h
 				if err != nil {
 					return err
 				}
+				defer rwtx.Rollback()
 				historyTx.Rollback()
 				historyTx, err = historyDb.BeginRo(context.Background())
 				if err != nil {

core/state/state_test.go

@@ -103,7 +103,7 @@ func (s *StateSuite) TestDump(c *checker.C) {
 
 func (s *StateSuite) SetUpTest(c *checker.C) {
 	s.kv = kv.NewMemKV()
-	tx, err := s.kv.BeginRw(context.Background())
+	tx, err := s.kv.BeginRw(context.Background()) //nolint
 	if err != nil {
 		panic(err)
 	}

ethdb/kv/kv_snapshot.go

@@ -214,7 +214,7 @@ func (s *SnapshotKV) BeginRo(ctx context.Context) (ethdb.Tx, error) {
 }
 
 func (s *SnapshotKV) BeginRw(ctx context.Context) (ethdb.RwTx, error) {
-	dbTx, err := s.db.BeginRw(ctx)
+	dbTx, err := s.db.BeginRw(ctx) //nolint
 	if err != nil {
 		return nil, err
 	}

ethdb/kv/kv_snapshot_property_test.go

@@ -45,6 +45,7 @@ func (m *getPutkvMachine) Init(t *rapid.T) {
 
 	txSn, err := m.snKV.BeginRw(context.Background())
 	require.NoError(t, err)
+	defer txSn.Rollback()
 
 	txModel, err := m.modelKV.BeginRw(context.Background())
 	require.NoError(t, err)
@@ -133,9 +134,9 @@ func (m *getPutkvMachine) Begin(t *rapid.T) {
 	if m.modelTX != nil && m.snTX != nil {
 		return
 	}
-	mtx, err := m.modelKV.BeginRw(context.Background())
+	mtx, err := m.modelKV.BeginRw(context.Background()) //nolint
 	require.NoError(t, err)
-	sntx, err := m.snKV.BeginRw(context.Background())
+	sntx, err := m.snKV.BeginRw(context.Background()) //nolint
 	require.NoError(t, err)
 	m.modelTX = mtx
 	m.snTX = sntx
@@ -189,6 +190,7 @@ func (m *getKVMachine) Init(t *rapid.T) {
 
 	txSn, err := m.snKV.BeginRw(context.Background())
 	require.NoError(t, err)
+	defer txSn.Rollback()
 
 	txModel, err := m.modelKV.BeginRw(context.Background())
 	require.NoError(t, err)
@@ -343,9 +345,9 @@ func (m *cursorKVMachine) Begin(t *rapid.T) {
 		return
 	}
 
-	mtx, err := m.modelKV.BeginRw(context.Background())
+	mtx, err := m.modelKV.BeginRw(context.Background()) //nolint
 	require.NoError(t, err)
-	sntx, err := m.snKV.BeginRw(context.Background())
+	sntx, err := m.snKV.BeginRw(context.Background()) //nolint
 	require.NoError(t, err)
 	m.modelTX = mtx
 	m.snTX = sntx

ethdb/kv/memory_database.go

@@ -54,7 +54,7 @@ func NewTestTx(t testing.TB) (ethdb.RwKV, ethdb.RwTx) {
 			tt.Cleanup(kv.Close)
 		}
 	}
-	tx, err := kv.BeginRw(context.Background())
+	tx, err := kv.BeginRw(context.Background()) //nolint
 	if err != nil {
 		t.Fatal(err)
 	}

go.mod

@@ -49,6 +49,7 @@ require (
 	github.com/petar/GoLLRB v0.0.0-20190514000832-33fb24c13b99
 	github.com/prometheus/client_golang v1.9.0
 	github.com/prometheus/tsdb v0.10.0
+	github.com/quasilyte/go-ruleguard/dsl v0.3.6
 	github.com/rs/cors v1.8.0
 	github.com/shirou/gopsutil/v3 v3.21.6
 	github.com/spf13/cobra v1.1.3

go.sum

@@ -870,6 +870,8 @@ github.com/prometheus/tsdb v0.6.2-0.20190402121629-4f204dcbc150/go.mod h1:qhTCs0
 github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU=
 github.com/prometheus/tsdb v0.10.0 h1:If5rVCMTp6W2SiRAQFlbpJNgVlgMEd+U2GZckwK38ic=
 github.com/prometheus/tsdb v0.10.0/go.mod h1:oi49uRhEe9dPUTlS3JRZOwJuVi6tmh10QSgwXEyGCt4=
+github.com/quasilyte/go-ruleguard/dsl v0.3.6 h1:W2wnISJifyda0x/RXq15Qjrsu9iOhX2gy4+Ku+owylw=
+github.com/quasilyte/go-ruleguard/dsl v0.3.6/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU=
 github.com/rcrowley/go-metrics v0.0.0-20181016184325-3113b8401b8a/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4=
 github.com/rjeczalik/notify v0.9.1/go.mod h1:rKwnCoCGeuQnwBtTSPL9Dad03Vh2n40ePRrjvIXnJho=
 github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg=
@@ -988,8 +990,6 @@ github.com/tklauser/numcpus v0.2.2 h1:oyhllyrScuYI6g+h/zUvNXNp1wy7x8qQy3t/piefld
 github.com/tklauser/numcpus v0.2.2/go.mod h1:x3qojaO3uyYt0i56EW/VUYs7uBvdl2fkfZFu0T9wgjM=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
-github.com/torquem-ch/mdbx-go v0.15.0 h1:HUPA06n7tEAyLXv+rFuWvk01FKEm2MavLD9VVEQqMCs=
-github.com/torquem-ch/mdbx-go v0.15.0/go.mod h1:T2fsoJDVppxfAPTLd1svUgH1kpPmeXdPESmroSHcL1E=
 github.com/torquem-ch/mdbx-go v0.16.0 h1:x4dTXDvy1w9MxjlAX6J3/bDqIqu4XQDqTR/u2jMnTiA=
 github.com/torquem-ch/mdbx-go v0.16.0/go.mod h1:T2fsoJDVppxfAPTLd1svUgH1kpPmeXdPESmroSHcL1E=
 github.com/ttacon/chalk v0.0.0-20160626202418-22c06c80ed31/go.mod h1:onvgF043R+lC5RZ8IT9rBXDaEDnpnw/Cl+HFiw+v/7Q=

rules.go

@@ -0,0 +1,68 @@
+// +build gorules
+
+package gorules
+
+// https://github.com/golang/go/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module
+import (
+	"github.com/quasilyte/go-ruleguard/dsl"
+	//quasilyterules "github.com/quasilyte/ruleguard-rules-test"
+)
+
+func init() {
+	//dsl.ImportRules("qrules", quasilyterules.Bundle)
+}
+
+func txDeferRollback(m dsl.Matcher) {
+	// Common pattern for long-living transactions:
+	//	tx, err := db.Begin()
+	//	if err != nil {
+	//		return err
+	//	}
+	//	defer tx.Rollback()
+	//
+	//	... code which uses database in transaction
+	//
+	//	err := tx.Commit()
+	//	if err != nil {
+	//		return err
+	//	}
+
+	m.Match(
+		`$tx, $err := $db.BeginRw($ctx); $chk; $rollback`,
+		`$tx, $err = $db.BeginRw($ctx); $chk; $rollback`,
+		`$tx, $err := $db.Begin($ctx); $chk; $rollback`,
+		`$tx, $err = $db.Begin($ctx); $chk; $rollback`,
+	).
+		Where(!m["rollback"].Text.Matches(`defer .*\.Rollback()`)).
+		//At(m["unlock"]).
+		Report(`Add "defer $tx.Rollback()" right after transaction creation error check. 
+			If you are in the loop - consider use "$db.View" or "$db.Update" or extract whole transaction to function.
+			Without rollback in defer - app can deadlock on error or panic.
+			Rules are in ./rules.go file.
+			`)
+
+}
+
+func mismatchingUnlock(m dsl.Matcher) {
+	// By default, an entire match position is used as a location.
+	// This can be changed by the At() method that binds the location
+	// to the provided named submatch.
+	//
+	// In the rules below text editor would get mismatching method
+	// name locations:
+	//
+	//   defer mu.RUnlock()
+	//            ^^^^^^^
+
+	m.Match(`$mu.Lock(); defer $mu.$unlock()`).
+		Where(m["unlock"].Text == "RUnlock").
+		At(m["unlock"]).
+		Report(`maybe $mu.Unlock() was intended?
+			Rules are in ./rules.go file.`)
+
+	m.Match(`$mu.RLock(); defer $mu.$unlock()`).
+		Where(m["unlock"].Text == "Unlock").
+		At(m["unlock"]).
+		Report(`maybe $mu.RUnlock() was intended?
+			Rules are in ./rules.go file.`)
+}

cmd/hack/binary-deps/main.go → tools.go

@@ -1,5 +1,9 @@
-// +build trick_go_mod_tidy
+// +build tools
 
+package tools
+
+// https://github.com/golang/go/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module
+//
 // This module is just a hack for 'go mod tidy' command
 //
 // Problem is - 'go mod tidy' removes from go.mod file lines if you don't use them in source code
@@ -12,8 +16,6 @@
 
 // build tag 'trick_go_mod_tidy' - is used to hide warnings of IDEA (because we can't import `main` packages in go)
 
-package main
-
 import (
 	_ "github.com/fjl/gencodec"
 	_ "github.com/kevinburke/go-bindata"

turbo/snapshotsync/headers_snapshot.go

@@ -4,14 +4,15 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"os"
+	"time"
+
 	"github.com/ledgerwatch/erigon/common"
 	"github.com/ledgerwatch/erigon/common/dbutils"
 	"github.com/ledgerwatch/erigon/core/rawdb"
 	"github.com/ledgerwatch/erigon/ethdb"
 	"github.com/ledgerwatch/erigon/ethdb/kv"
 	"github.com/ledgerwatch/erigon/log"
-	"os"
-	"time"
 )
 
 func CreateHeadersSnapshot(ctx context.Context, readTX ethdb.Tx, toBlock uint64, snapshotPath string) error {