From 7b93cc3042e9a4a55594b3c3449590ba6b6d3ed5 Mon Sep 17 00:00:00 2001 From: chris erway Date: Thu, 2 Feb 2023 11:33:32 -0500 Subject: [PATCH 01/10] enable exportloopref linter --- .golangci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.golangci.yml b/.golangci.yml index 30b4d28128..ec9ff159e0 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -7,6 +7,7 @@ linters: disable-all: true enable: - errcheck + - exportloopref - gofmt - gosimple - govet @@ -107,6 +108,7 @@ issues: - path: _test\.go linters: - errcheck + - exportloopref # - gofmt - gosimple # - govet From 38223f6edc21dd52bfd531ffe6cead200bc11a39 Mon Sep 17 00:00:00 2001 From: chris erway Date: Thu, 2 Feb 2023 13:01:24 -0500 Subject: [PATCH 02/10] Add golangci-lint version check and warning to check_deps.sh --- scripts/check_deps.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/scripts/check_deps.sh b/scripts/check_deps.sh index a42405733d..ba87fffc0b 100755 --- a/scripts/check_deps.sh +++ b/scripts/check_deps.sh @@ -62,6 +62,14 @@ check_deps() { then missing_dep sqlite3 fi + + # Check version of golangci-lint + version_req=$(grep "github.com/golangci/golangci-lint/cmd/golangci-lint" scripts/buildtools/versions | awk '{print $2}') + version_cur=$($GO_BIN/golangci-lint version | grep -o 'v[0-9]\+\.[0-9]\+\.[0-9]\+') + if [ "$version_cur" != "$version_req" ] + then + echo "$YELLOW_FG[WARNING]$END_FG_COLOR \`golangci-lint\` version mismatch, expected $version_req, but got $version_cur. You may get different linter output than CI as a result." + fi } check_deps From 37e6c58b69b8a0de905c969a564f516b170cdef9 Mon Sep 17 00:00:00 2001 From: chris erway Date: Thu, 2 Feb 2023 13:09:27 -0500 Subject: [PATCH 03/10] fix loop variable pointer error --- .golangci.yml | 2 +- data/transactions/logic/assembler_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 82b6e7c78f..46463c18bf 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -108,7 +108,7 @@ issues: - path: _test\.go linters: - errcheck - - exportloopref + # - exportloopref # - gofmt - gosimple # - govet diff --git a/data/transactions/logic/assembler_test.go b/data/transactions/logic/assembler_test.go index cc5cd97a44..8cdf8174d0 100644 --- a/data/transactions/logic/assembler_test.go +++ b/data/transactions/logic/assembler_test.go @@ -658,9 +658,9 @@ func testProg(t testing.TB, source string, ver uint64, expected ...Expect) *OpSt } } else { var found *lineError - for _, err := range errors { - if err.Line == exp.l { - found = &err + for i := range errors { + if errors[i].Line == exp.l { + found = &errors[i] break } } From 9cdbeed83e62d49bce7dac98150a536fc695d894 Mon Sep 17 00:00:00 2001 From: chris erway Date: Thu, 2 Feb 2023 13:52:51 -0500 Subject: [PATCH 04/10] check go tools versions using go version -m --- scripts/check_deps.sh | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/scripts/check_deps.sh b/scripts/check_deps.sh index ba87fffc0b..b3a5c03caa 100755 --- a/scripts/check_deps.sh +++ b/scripts/check_deps.sh @@ -35,20 +35,36 @@ missing_dep() { } GO_DEPS=( - "$GO_BIN/stringer" - "$GO_BIN/msgp" - "$GO_BIN/golangci-lint" + "msgp" + "golangci-lint" + "oapi-codegen" + "swagger" ) +check_go_binary_version() { + binary_name=$1 + expected_version=$(grep "$binary_name" scripts/buildtools/versions | awk '{print $2}') + actual_version=$(go version -m "$GO_BIN/$binary_name" | awk 'NR==3 {print $3}') + + if [ "$expected_version" != "$actual_version" ]; then + echo "$YELLOW_FG[WARNING]$END_FG_COLOR $binary_name version mismatch, expected $expected_version, but got $actual_version" + else + echo "OK: $binary_name version $actual_version matches expected version $expected_version" + return 0 + fi +} + check_deps() { - for path in ${GO_DEPS[*]} + for dep in ${GO_DEPS[*]} do - if [ ! -f "$path" ] + if [ ! -f "$GO_BIN/$dep" ] then # Parameter expansion is faster than invoking another process. # https://www.linuxjournal.com/content/bash-parameter-expansion - missing_dep "${path##*/}" + missing_dep "${dep##*/}" fi + + check_go_binary_version "$dep" done # Don't print `shellcheck`s location. @@ -62,14 +78,6 @@ check_deps() { then missing_dep sqlite3 fi - - # Check version of golangci-lint - version_req=$(grep "github.com/golangci/golangci-lint/cmd/golangci-lint" scripts/buildtools/versions | awk '{print $2}') - version_cur=$($GO_BIN/golangci-lint version | grep -o 'v[0-9]\+\.[0-9]\+\.[0-9]\+') - if [ "$version_cur" != "$version_req" ] - then - echo "$YELLOW_FG[WARNING]$END_FG_COLOR \`golangci-lint\` version mismatch, expected $version_req, but got $version_cur. You may get different linter output than CI as a result." - fi } check_deps From 85c41497d22d8288d910251dd59800b4eec9562c Mon Sep 17 00:00:00 2001 From: chris erway Date: Thu, 2 Feb 2023 13:53:56 -0500 Subject: [PATCH 05/10] print warning but no OK if version mismatch --- scripts/check_deps.sh | 3 --- 1 file changed, 3 deletions(-) diff --git a/scripts/check_deps.sh b/scripts/check_deps.sh index b3a5c03caa..4d08da4a98 100755 --- a/scripts/check_deps.sh +++ b/scripts/check_deps.sh @@ -48,9 +48,6 @@ check_go_binary_version() { if [ "$expected_version" != "$actual_version" ]; then echo "$YELLOW_FG[WARNING]$END_FG_COLOR $binary_name version mismatch, expected $expected_version, but got $actual_version" - else - echo "OK: $binary_name version $actual_version matches expected version $expected_version" - return 0 fi } From 1ed00af10d81354950fdf1b8fa8f82b23516d3e6 Mon Sep 17 00:00:00 2001 From: chris erway Date: Thu, 2 Feb 2023 14:22:20 -0500 Subject: [PATCH 06/10] set MISSING if version mismatch --- scripts/check_deps.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/check_deps.sh b/scripts/check_deps.sh index 4d08da4a98..5eac23586d 100755 --- a/scripts/check_deps.sh +++ b/scripts/check_deps.sh @@ -48,6 +48,7 @@ check_go_binary_version() { if [ "$expected_version" != "$actual_version" ]; then echo "$YELLOW_FG[WARNING]$END_FG_COLOR $binary_name version mismatch, expected $expected_version, but got $actual_version" + MISSING=1 fi } From 2030c50bb4a7abec2fa57e1a65549fb7417caccb Mon Sep 17 00:00:00 2001 From: chris erway Date: Thu, 2 Feb 2023 15:00:37 -0500 Subject: [PATCH 07/10] use more strict gosec linter to find references to loop variables --- .golangci.yml | 4 ++++ cmd/goal/clerk.go | 4 ++-- cmd/opdoc/opdoc.go | 4 ++-- daemon/algod/api/server/v2/utils.go | 4 ++-- data/pools/transactionPool.go | 2 +- data/transactions/verify/txn.go | 2 +- ledger/store/accountsV2.go | 4 ++-- ledger/store/schema.go | 2 +- 8 files changed, 15 insertions(+), 11 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 46463c18bf..34234171bb 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -9,6 +9,7 @@ linters: - errcheck - exportloopref - gofmt + - gosec - gosimple - govet - ineffassign @@ -37,6 +38,8 @@ linters-settings: - (*github.com/algorand/go-algorand/data/transactions/logic.OpStream).warn # We do this 121 times and never check the error. - (*github.com/spf13/cobra.Command).MarkFlagRequired + gosec: # we are mostly only interested in G601 + excludes: [G101, G103, G104, G107, G202, G301, G302, G303, G304, G306, G307, G404] govet: settings: printf: @@ -112,6 +115,7 @@ issues: # - gofmt - gosimple # - govet + - gosec - ineffassign - misspell - nolintlint diff --git a/cmd/goal/clerk.go b/cmd/goal/clerk.go index ed226733d0..2147828947 100644 --- a/cmd/goal/clerk.go +++ b/cmd/goal/clerk.go @@ -221,8 +221,8 @@ func createSignedTransaction(client libgoal.Client, signTx bool, dataDir string, func writeSignedTxnsToFile(stxns []transactions.SignedTxn, filename string) error { var outData []byte - for _, stxn := range stxns { - outData = append(outData, protocol.Encode(&stxn)...) + for i := range stxns { + outData = append(outData, protocol.Encode(&stxns[i])...) } return writeFile(filename, outData, 0600) diff --git a/cmd/opdoc/opdoc.go b/cmd/opdoc/opdoc.go index e0b69fa111..a7f1894018 100644 --- a/cmd/opdoc/opdoc.go +++ b/cmd/opdoc/opdoc.go @@ -225,8 +225,8 @@ func opsToMarkdown(out io.Writer) (err error) { out.Write([]byte("# Opcodes\n\nOps have a 'cost' of 1 unless otherwise specified.\n\n")) opSpecs := logic.OpcodesByVersion(uint64(docVersion)) written := make(map[string]bool) - for _, spec := range opSpecs { - err = opToMarkdown(out, &spec, written) + for i := range opSpecs { + err = opToMarkdown(out, &opSpecs[i], written) if err != nil { return } diff --git a/daemon/algod/api/server/v2/utils.go b/daemon/algod/api/server/v2/utils.go index 32f18ea7d4..cf2648e8d4 100644 --- a/daemon/algod/api/server/v2/utils.go +++ b/daemon/algod/api/server/v2/utils.go @@ -310,8 +310,8 @@ func convertLogs(txn node.TxnWithStatus) *[][]byte { func convertInners(txn *node.TxnWithStatus) *[]PreEncodedTxInfo { inner := make([]PreEncodedTxInfo, len(txn.ApplyData.EvalDelta.InnerTxns)) - for i, itxn := range txn.ApplyData.EvalDelta.InnerTxns { - inner[i] = convertInnerTxn(&itxn) + for i := range txn.ApplyData.EvalDelta.InnerTxns { + inner[i] = convertInnerTxn(&txn.ApplyData.EvalDelta.InnerTxns[i]) } return &inner } diff --git a/data/pools/transactionPool.go b/data/pools/transactionPool.go index bed4e17e68..c99391ccc4 100644 --- a/data/pools/transactionPool.go +++ b/data/pools/transactionPool.go @@ -869,7 +869,7 @@ func (pool *TransactionPool) AssembleBlock(round basics.Round, deadline time.Tim } stats.TotalLength += uint64(encodedLen) if txib.Txn.Type == protocol.StateProofTx { - stats.StateProofStats = pool.getStateProofStats(&txib, encodedLen) + stats.StateProofStats = pool.getStateProofStats(&payset[i], encodedLen) } } stats.AverageFee = totalFees / uint64(stats.IncludedCount) diff --git a/data/transactions/verify/txn.go b/data/transactions/verify/txn.go index 2edd69aa05..564c9b4ba1 100644 --- a/data/transactions/verify/txn.go +++ b/data/transactions/verify/txn.go @@ -226,7 +226,7 @@ func txnGroupBatchPrep(stxs []transactions.SignedTxn, contextHdr *bookkeeping.Bl minFeeCount := uint64(0) feesPaid := uint64(0) for i, stxn := range stxs { - prepErr := txnBatchPrep(&stxn, i, groupCtx, verifier, evalTracer) + prepErr := txnBatchPrep(&stxs[i], i, groupCtx, verifier, evalTracer) if prepErr != nil { // re-wrap the error with more details prepErr.err = fmt.Errorf("transaction %+v invalid : %w", stxn, prepErr.err) diff --git a/ledger/store/accountsV2.go b/ledger/store/accountsV2.go index cb2c1d34eb..ecaaf5970f 100644 --- a/ledger/store/accountsV2.go +++ b/ledger/store/accountsV2.go @@ -710,8 +710,8 @@ func (w *accountsV2Writer) AccountsPutOnlineRoundParams(onlineRoundParamsData [] return err } - for i, onlineRoundParams := range onlineRoundParamsData { - _, err = insertStmt.Exec(startRound+basics.Round(i), protocol.Encode(&onlineRoundParams)) + for i := range onlineRoundParamsData { + _, err = insertStmt.Exec(startRound+basics.Round(i), protocol.Encode(&onlineRoundParamsData[i])) if err != nil { return err } diff --git a/ledger/store/schema.go b/ledger/store/schema.go index acbd102b15..339c660a11 100644 --- a/ledger/store/schema.go +++ b/ledger/store/schema.go @@ -205,7 +205,7 @@ func accountsInit(tx *sql.Tx, initAccounts map[basics.Address]basics.AccountData for addr, data := range initAccounts { _, err = tx.Exec("INSERT INTO accountbase (address, data) VALUES (?, ?)", - addr[:], protocol.Encode(&data)) + addr[:], protocol.Encode(&data)) //nolint:gosec // Encode does not hold on to reference if err != nil { return true, err } From 96507a98ac441a3600c941887373e4f174b7f9c8 Mon Sep 17 00:00:00 2001 From: chris erway Date: Thu, 2 Feb 2023 15:02:36 -0500 Subject: [PATCH 08/10] move gosec to warning linter --- .golangci-warnings.yml | 4 ++++ .golangci.yml | 4 ---- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.golangci-warnings.yml b/.golangci-warnings.yml index f8d2063475..440a07f7d2 100644 --- a/.golangci-warnings.yml +++ b/.golangci-warnings.yml @@ -6,12 +6,15 @@ linters: disable-all: true enable: - deadcode + - gosec - partitiontest - structcheck - varcheck - unused linters-settings: + gosec: # we are mostly only interested in G601 + excludes: [G101, G103, G104, G107, G202, G301, G302, G303, G304, G306, G307, G404] custom: partitiontest: path: cmd/partitiontest_linter/plugin.so @@ -51,6 +54,7 @@ issues: - path: _test\.go linters: - deadcode + - gosec - structcheck - varcheck - unused diff --git a/.golangci.yml b/.golangci.yml index 34234171bb..46463c18bf 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -9,7 +9,6 @@ linters: - errcheck - exportloopref - gofmt - - gosec - gosimple - govet - ineffassign @@ -38,8 +37,6 @@ linters-settings: - (*github.com/algorand/go-algorand/data/transactions/logic.OpStream).warn # We do this 121 times and never check the error. - (*github.com/spf13/cobra.Command).MarkFlagRequired - gosec: # we are mostly only interested in G601 - excludes: [G101, G103, G104, G107, G202, G301, G302, G303, G304, G306, G307, G404] govet: settings: printf: @@ -115,7 +112,6 @@ issues: # - gofmt - gosimple # - govet - - gosec - ineffassign - misspell - nolintlint From a46afcb0fd842ccf17f528e7fb59e443e93777c3 Mon Sep 17 00:00:00 2001 From: chris erway Date: Thu, 2 Feb 2023 15:12:12 -0500 Subject: [PATCH 09/10] don't set MISSING because maybe this check won't work --- scripts/check_deps.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/check_deps.sh b/scripts/check_deps.sh index 5eac23586d..4d08da4a98 100755 --- a/scripts/check_deps.sh +++ b/scripts/check_deps.sh @@ -48,7 +48,6 @@ check_go_binary_version() { if [ "$expected_version" != "$actual_version" ]; then echo "$YELLOW_FG[WARNING]$END_FG_COLOR $binary_name version mismatch, expected $expected_version, but got $actual_version" - MISSING=1 fi } From 42674a25fb28abe1ecb79112041e07088646c75c Mon Sep 17 00:00:00 2001 From: chris erway Date: Tue, 7 Feb 2023 17:08:51 -0500 Subject: [PATCH 10/10] skip version check on go1.17 on arm64 macs --- scripts/check_deps.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/check_deps.sh b/scripts/check_deps.sh index 4d08da4a98..95c7599f78 100755 --- a/scripts/check_deps.sh +++ b/scripts/check_deps.sh @@ -61,7 +61,11 @@ check_deps() { missing_dep "${dep##*/}" fi - check_go_binary_version "$dep" + # go 1.17 on arm64 macs has an issue checking binaries with "go version", skip version check + if [[ "$(uname)" != "Darwin" ]] || [[ "$(uname -m)" != "arm64" ]] || ! [[ "$(go version | awk '{print $3}')" < "go1.17" ]] + then + check_go_binary_version "$dep" + fi done # Don't print `shellcheck`s location.