all: more linters

.golangci.yml

@@ -19,32 +19,53 @@ linters:
  - govet
  - ineffassign
  - misspell
- # - staticcheck
  - unconvert
- # - unused
  - varcheck
+ - typecheck
+ - unused
+ - staticcheck
+ - bidichk
+ - durationcheck
+ - exportloopref
+ - gosec
+
+ #- structcheck # lots of false positives
+ #- errcheck #lot of false positives
+ # - contextcheck
+ # - errchkjson # lots of false positives
+ # - errorlint # this check crashes
+ # - exhaustive # silly check
+ # - makezero # false positives
+ # - nilerr # several intentional
 
 linters-settings:
  gofmt:
  simplify: true
  goconst:
  min-len: 3 # minimum length of string constant
  min-occurrences: 6 # minimum number of occurrences
+ gosec:
+ excludes:
+ - G404 # Use of weak random number generator - lots of FP
+ - G107 # Potential http request -- those are intentional
+ - G306 # G306: Expect WriteFile permissions to be 0600 or less
 
 issues:
  exclude-rules:
- - path: crypto/blake2b/
- linters:
- - deadcode
- - path: crypto/bn256/cloudflare
- linters:
- - deadcode
- - path: p2p/discv5/
- linters:
- - deadcode
- - path: core/vm/instructions_test.go
- linters:
- - goconst
- - path: cmd/faucet/
+ - path: crypto/bn256/cloudflare/optate.go
  linters:
  - deadcode
+ - staticcheck
+ - path: internal/build/pgp.go
+ text: 'SA1019: package golang.org/x/crypto/openpgp is deprecated'
+ - path: core/vm/contracts.go
+ text: 'SA1019: package golang.org/x/crypto/ripemd160 is deprecated'
+ - path: accounts/usbwallet/trezor.go
+ text: 'SA1019: package github.com/golang/protobuf/proto is deprecated'
+ - path: accounts/usbwallet/trezor/
+ text: 'SA1019: package github.com/golang/protobuf/proto is deprecated'
+ exclude:
+ - 'SA1019: event.TypeMux is deprecated: use Feed'
+ - 'SA1019: strings.Title is deprecated'
+ - 'SA1029: should not use built-in type string as key for value'
+ - 'G306: Expect WriteFile permissions to be 0600 or less'

accounts/abi/abi_test.go

@@ -1038,9 +1038,7 @@ func TestABI_EventById(t *testing.T) {
  }
  if event == nil {
  t.Errorf("We should find a event for topic %s, test #%d", topicID.Hex(), testnum)
- }
-
- if event.ID != topicID {
+ } else if event.ID != topicID {
  t.Errorf("Event id %s does not match topic %s, test #%d", event.ID.Hex(), topicID.Hex(), testnum)
  }
 

accounts/abi/bind/backends/simulated_test.go

@@ -655,8 +655,7 @@ func TestHeaderByNumber(t *testing.T) {
  }
  if latestBlockHeader == nil {
  t.Errorf("received a nil block header")
- }
- if latestBlockHeader.Number.Uint64() != uint64(0) {
+ } else if latestBlockHeader.Number.Uint64() != uint64(0) {
  t.Errorf("expected block header number 0, instead got %v", latestBlockHeader.Number.Uint64())
  }
 

accounts/external/backend.go

@@ -152,10 +152,6 @@ func (api *ExternalSigner) SelfDerive(bases []accounts.DerivationPath, chain eth
  log.Error("operation SelfDerive not supported on external signers")
 }
 
-func (api *ExternalSigner) signHash(account accounts.Account, hash []byte) ([]byte, error) {
- return []byte{}, fmt.Errorf("operation not supported on external signers")
-}
-
 // SignData signs keccak256(data). The mimetype parameter describes the type of data being signed
 func (api *ExternalSigner) SignData(account accounts.Account, mimeType string, data []byte) ([]byte, error) {
  var res hexutil.Bytes

accounts/keystore/account_cache_test.go

@@ -383,7 +383,7 @@ func TestUpdatedKeyfileContents(t *testing.T) {
  time.Sleep(1000 * time.Millisecond)
 
  // Now replace file contents with crap
- if err := os.WriteFile(file, []byte("foo"), 0644); err != nil {
+ if err := os.WriteFile(file, []byte("foo"), 0600); err != nil {
  t.Fatal(err)
  return
  }

accounts/keystore/passphrase_test.go

@@ -52,7 +52,7 @@ func TestKeyEncryptDecrypt(t *testing.T) {
  t.Errorf("test %d: key address mismatch: have %x, want %x", i, key.Address, address)
  }
  // Recrypt with a new password and start over
- password += "new data appended"
+ password += "new data appended" // nolint: gosec
  if keyjson, err = EncryptKey(key, password, veryLightScryptN, veryLightScryptP); err != nil {
  t.Errorf("test %d: failed to recrypt key %v", i, err)
  }

cmd/devp2p/internal/ethtest/snap.go

@@ -104,6 +104,7 @@ func (s *Suite) TestSnapGetAccountRange(t *utesting.T) {
  // Max bytes: 0. Expect to deliver one account.
  {0, root, zero, ffHash, 1, firstKey, firstKey},
  } {
+ tc := tc
  if err := s.snapGetAccountRange(t, &tc); err != nil {
  t.Errorf("test %d \n root: %x\n range: %#x - %#x\n bytes: %d\nfailed: %v", i, tc.root, tc.origin, tc.limit, tc.nBytes, err)
  }
@@ -194,6 +195,7 @@ func (s *Suite) TestSnapGetStorageRanges(t *utesting.T) {
  expSlots: 2,
  },
  } {
+ tc := tc
  if err := s.snapGetStorageRanges(t, &tc); err != nil {
  t.Errorf("test %d \n root: %x\n range: %#x - %#x\n bytes: %d\n #accounts: %d\nfailed: %v",
  i, tc.root, tc.origin, tc.limit, tc.nBytes, len(tc.accounts), err)
@@ -291,6 +293,7 @@ func (s *Suite) TestSnapGetByteCodes(t *utesting.T) {
  expHashes: 4,
  },
  } {
+ tc := tc
  if err := s.snapGetByteCodes(t, &tc); err != nil {
  t.Errorf("test %d \n bytes: %d\n #hashes: %d\nfailed: %v", i, tc.nBytes, len(tc.hashes), err)
  }
@@ -436,6 +439,7 @@ func (s *Suite) TestSnapTrieNodes(t *utesting.T) {
  },
  },
  } {
+ tc := tc
  if err := s.snapGetTrieNodes(t, &tc); err != nil {
  t.Errorf("test %d \n #hashes %x\n root: %#x\n bytes: %d\nfailed: %v", i, len(tc.expHashes), tc.root, tc.nBytes, err)
  }

cmd/devp2p/internal/v5test/framework.go

@@ -60,11 +60,9 @@ type conn struct {
  remoteAddr *net.UDPAddr
  listeners []net.PacketConn
 
- log logger
- codec *v5wire.Codec
- lastRequest v5wire.Packet
- lastChallenge *v5wire.Whoareyou
- idCounter uint32
+ log logger
+ codec *v5wire.Codec
+ idCounter uint32
 }
 
 type logger interface {

cmd/geth/accountcmd.go

@@ -239,14 +239,15 @@ func ambiguousAddrRecovery(ks *keystore.KeyStore, err *keystore.AmbiguousAddrErr
  }
  fmt.Println("Testing your password against all of them...")
  var match *accounts.Account
- for _, a := range err.Matches {
- if err := ks.Unlock(a, auth); err == nil {
- match = &a
+ for i, a := range err.Matches {
+ if e := ks.Unlock(a, auth); e == nil {
+ match = &err.Matches[i]
  break
  }
  }
  if match == nil {
  utils.Fatalf("None of the listed files could be unlocked.")
+ return accounts.Account{}
  }
  fmt.Printf("Your password unlocked %s\n", match.URL)
  fmt.Println("In order to avoid this warning, you need to remove the following duplicate key files:")

cmd/geth/les_test.go

@@ -81,41 +81,6 @@ func (g *gethrpc) getNodeInfo() *p2p.NodeInfo {
  return g.nodeInfo
 }
 
-func (g *gethrpc) waitSynced() {
- // Check if it's synced now
- var result interface{}
- g.callRPC(&result, "eth_syncing")
- syncing, ok := result.(bool)
- if ok && !syncing {
- g.geth.Logf("%v already synced", g.name)
- return
- }
-
- // Actually wait, subscribe to the event
- ch := make(chan interface{})
- sub, err := g.rpc.Subscribe(context.Background(), "eth", ch, "syncing")
- if err != nil {
- g.geth.Fatalf("%v syncing: %v", g.name, err)
- }
- defer sub.Unsubscribe()
- timeout := time.After(4 * time.Second)
- select {
- case ev := <-ch:
- g.geth.Log("'syncing' event", ev)
- syncing, ok := ev.(bool)
- if ok && !syncing {
- break
- }
- g.geth.Log("Other 'syncing' event", ev)
- case err := <-sub.Err():
- g.geth.Fatalf("%v notification: %v", g.name, err)
- break
- case <-timeout:
- g.geth.Fatalf("%v timeout syncing", g.name)
- break
- }
-}
-
 // ipcEndpoint resolves an IPC endpoint based on a configured value, taking into
 // account the set data folders as well as the designated platform we're currently
 // running on.

cmd/puppeth/ssh.go

@@ -30,7 +30,7 @@ import (
  "github.com/ethereum/go-ethereum/log"
  "golang.org/x/crypto/ssh"
  "golang.org/x/crypto/ssh/agent"
- "golang.org/x/crypto/ssh/terminal"
+ "golang.org/x/term"
 )
 
 // sshClient is a small wrapper around Go's SSH client with a few utility methods
@@ -101,7 +101,7 @@ func dial(server string, pubkey []byte) (*sshClient, error) {
  key, err := ssh.ParsePrivateKey(buf)
  if err != nil {
  fmt.Printf("What's the decryption password for %s? (won't be echoed)\n>", path)
- blob, err := terminal.ReadPassword(int(os.Stdin.Fd()))
+ blob, err := term.ReadPassword(int(os.Stdin.Fd()))
  fmt.Println()
  if err != nil {
  log.Warn("Couldn't read password", "err", err)
@@ -118,7 +118,7 @@ func dial(server string, pubkey []byte) (*sshClient, error) {
  }
  auths = append(auths, ssh.PasswordCallback(func() (string, error) {
  fmt.Printf("What's the login password for %s at %s? (won't be echoed)\n> ", username, server)
- blob, err := terminal.ReadPassword(int(os.Stdin.Fd()))
+ blob, err := term.ReadPassword(int(os.Stdin.Fd()))
 
  fmt.Println()
  return string(blob), err

cmd/puppeth/wizard.go

@@ -34,7 +34,7 @@ import (
  "github.com/ethereum/go-ethereum/core"
  "github.com/ethereum/go-ethereum/log"
  "github.com/peterh/liner"
- "golang.org/x/crypto/ssh/terminal"
+ "golang.org/x/term"
 )
 
 // config contains all the configurations needed by puppeth that should be saved
@@ -228,7 +228,7 @@ func (w *wizard) readDefaultFloat(def float64) float64 {
 // line and returns it. The input will not be echoed.
 func (w *wizard) readPassword() string {
  fmt.Printf("> ")
- text, err := terminal.ReadPassword(int(os.Stdin.Fd()))
+ text, err := term.ReadPassword(int(os.Stdin.Fd()))
  if err != nil {
  log.Crit("Failed to read password", "err", err)
  }

cmd/utils/diskusage.go

@@ -33,6 +33,7 @@ func getFreeDiskSpace(path string) (uint64, error) {
 
  // Available blocks * size per block = available space in bytes
  var bavail = stat.Bavail
+ // nolint:staticcheck
  if stat.Bavail < 0 {
  // FreeBSD can have a negative number of blocks available
  // because of the grace limit.

core/blockchain_sethead_test.go

@@ -51,6 +51,7 @@ type rewindTest struct {
  expHeadBlock uint64 // Block number of the expected head full block
 }
 
+//nolint:unused
 func (tt *rewindTest) dump(crash bool) string {
  buffer := new(strings.Builder)
 

core/blockchain_snapshot_test.go

@@ -150,6 +150,7 @@ func (basic *snapshotTestBasic) verify(t *testing.T, chain *BlockChain, blocks [
  }
 }
 
+//nolint:unused
 func (basic *snapshotTestBasic) dump() string {
  buffer := new(strings.Builder)
 
@@ -341,54 +342,6 @@ func (snaptest *setHeadSnapshotTest) test(t *testing.T) {
  snaptest.verify(t, newchain, blocks)
 }
 
-// restartCrashSnapshotTest is the test type used to test this scenario:
-// - have a complete snapshot
-// - restart chain
-// - insert more blocks with enabling the snapshot
-// - commit the snapshot
-// - crash
-// - restart again
-type restartCrashSnapshotTest struct {
- snapshotTestBasic
- newBlocks int
-}
-
-func (snaptest *restartCrashSnapshotTest) test(t *testing.T) {
- // It's hard to follow the test case, visualize the input
- // log.Root().SetHandler(log.LvlFilterHandler(log.LvlTrace, log.StreamHandler(os.Stderr, log.TerminalFormat(true))))
- // fmt.Println(tt.dump())
- chain, blocks := snaptest.prepare(t)
-
- // Firstly, stop the chain properly, with all snapshot journal
- // and state committed.
- chain.Stop()
-
- newchain, err := NewBlockChain(snaptest.db, nil, params.AllEthashProtocolChanges, snaptest.engine, vm.Config{}, nil, nil)
- if err != nil {
- t.Fatalf("Failed to recreate chain: %v", err)
- }
- newBlocks, _ := GenerateChain(params.TestChainConfig, blocks[len(blocks)-1], snaptest.engine, snaptest.gendb, snaptest.newBlocks, func(i int, b *BlockGen) {})
- newchain.InsertChain(newBlocks)
-
- // Commit the entire snapshot into the disk if requested. Note only
- // (a) snapshot root and (b) snapshot generator will be committed,
- // the diff journal is not.
- newchain.Snapshots().Cap(newBlocks[len(newBlocks)-1].Root(), 0)
-
- // Simulate the blockchain crash
- // Don't call chain.Stop here, so that no snapshot
- // journal and latest state will be committed
-
- // Restart the chain after the crash
- newchain, err = NewBlockChain(snaptest.db, nil, params.AllEthashProtocolChanges, snaptest.engine, vm.Config{}, nil, nil)
- if err != nil {
- t.Fatalf("Failed to recreate chain: %v", err)
- }
- defer newchain.Stop()
-
- snaptest.verify(t, newchain, blocks)
-}
-
 // wipeCrashSnapshotTest is the test type used to test this scenario:
 // - have a complete snapshot
 // - restart, insert more blocks without enabling the snapshot
@@ -431,7 +384,7 @@ func (snaptest *wipeCrashSnapshotTest) test(t *testing.T) {
  SnapshotLimit: 256,
  SnapshotWait: false, // Don't wait rebuild
  }
- newchain, err = NewBlockChain(snaptest.db, config, params.AllEthashProtocolChanges, snaptest.engine, vm.Config{}, nil, nil)
+ _, err = NewBlockChain(snaptest.db, config, params.AllEthashProtocolChanges, snaptest.engine, vm.Config{}, nil, nil)
  if err != nil {
  t.Fatalf("Failed to recreate chain: %v", err)
  }

core/blockchain_test.go

@@ -2576,6 +2576,7 @@ func TestTransactionIndices(t *testing.T) {
  t.Fatalf("failed to create temp freezer db: %v", err)
  }
  gspec.MustCommit(ancientDb)
+ l := l
  chain, err = NewBlockChain(ancientDb, nil, params.TestChainConfig, ethash.NewFaker(), vm.Config{}, nil, &l)
  if err != nil {
  t.Fatalf("failed to create tester chain: %v", err)
@@ -2601,6 +2602,7 @@ func TestTransactionIndices(t *testing.T) {
  limit = []uint64{0, 64 /* drop stale */, 32 /* shorten history */, 64 /* extend history */, 0 /* restore all */}
  tails := []uint64{0, 67 /* 130 - 64 + 1 */, 100 /* 131 - 32 + 1 */, 69 /* 132 - 64 + 1 */, 0}
  for i, l := range limit {
+ l := l
  chain, err = NewBlockChain(ancientDb, nil, params.TestChainConfig, ethash.NewFaker(), vm.Config{}, nil, &l)
  if err != nil {
  t.Fatalf("failed to create tester chain: %v", err)

core/rawdb/freezer_test.go

@@ -277,7 +277,7 @@ func TestFreezerReadonlyValidate(t *testing.T) {
 
  // Re-openening as readonly should fail when validating
  // table lengths.
- f, err = NewFreezer(dir, "", true, 2049, tables)
+ _, err = NewFreezer(dir, "", true, 2049, tables)
  if err == nil {
  t.Fatal("readonly freezer should fail with differing table lengths")
  }

core/rawdb/freezer_utils_test.go

@@ -43,7 +43,7 @@ func TestCopyFrom(t *testing.T) {
  {"foo", "bar", 8, true},
  }
  for _, c := range cases {
- os.WriteFile(c.src, content, 0644)
+ os.WriteFile(c.src, content, 0600)
 
  if err := copyFrom(c.src, c.dest, c.offset, func(f *os.File) error {
  if !c.writePrefix {