From bc6c75c79da998a53e23eaf54e4da61eeeededd6 Mon Sep 17 00:00:00 2001 From: chris erway Date: Thu, 7 Jul 2022 18:37:20 -0400 Subject: [PATCH 01/16] fix golangci-lint errors --- .github/workflows/reviewdog.yml | 1 + cmd/catchpointdump/net.go | 13 +++++++++++-- data/transactions/logic/assembler.go | 4 ++-- data/transactions/logic/eval.go | 6 +++--- ledger/accountdb.go | 2 +- ledger/acctonline.go | 2 -- ledger/tracker.go | 5 ++++- network/wsNetwork.go | 10 +++++----- network/wsNetwork_test.go | 4 ++-- .../expect/catchpointCatchupWebProxy/webproxy.go | 2 +- 10 files changed, 30 insertions(+), 19 deletions(-) diff --git a/.github/workflows/reviewdog.yml b/.github/workflows/reviewdog.yml index 6cc82a6e55..3aed9d8512 100644 --- a/.github/workflows/reviewdog.yml +++ b/.github/workflows/reviewdog.yml @@ -24,6 +24,7 @@ jobs: tool_name: "Lint Errors" level: "error" fail_on_error: true + filter_mode: "nofilter" # Non-Blocking Warnings Section reviewdog-warnings: runs-on: ubuntu-latest diff --git a/cmd/catchpointdump/net.go b/cmd/catchpointdump/net.go index 9e8dc45611..072e87458c 100644 --- a/cmd/catchpointdump/net.go +++ b/cmd/catchpointdump/net.go @@ -303,7 +303,11 @@ func deleteLedgerFiles(deleteTracker bool) error { func loadAndDump(addr string, tarFile string, genesisInitState ledgercore.InitState) error { // delete current ledger files. - deleteLedgerFiles(true) + err := deleteLedgerFiles(true) + if err != nil { + reportErrorf("Unable to delete ledger files : %v", err) + return err + } cfg := config.GetDefaultLocal() l, err := ledger.OpenLedger(logging.Base(), "./ledger", false, genesisInitState, cfg) if err != nil { @@ -311,7 +315,12 @@ func loadAndDump(addr string, tarFile string, genesisInitState ledgercore.InitSt return err } - defer deleteLedgerFiles(!loadOnly) + defer func() { + err := deleteLedgerFiles(!loadOnly) + if err != nil { + reportErrorf("Unable to delete ledger files : %v", err) + } + }() defer l.Close() catchupAccessor := ledger.MakeCatchpointCatchupAccessor(l, logging.Base()) diff --git a/data/transactions/logic/assembler.go b/data/transactions/logic/assembler.go index 6a05596b2d..3c2147f725 100644 --- a/data/transactions/logic/assembler.go +++ b/data/transactions/logic/assembler.go @@ -1305,7 +1305,7 @@ func (ops *OpStream) trace(format string, args ...interface{}) { func (ops *OpStream) typeError(err error) { if ops.typeTracking { - ops.error(err) + _ = ops.error(err) } } @@ -1413,7 +1413,7 @@ func (ops *OpStream) assemble(text string) error { // bail out on the assembly as a whole. spec, ok = OpsByName[AssemblerMaxVersion][opstring] if ok { - ops.errorf("%s opcode was introduced in TEAL v%d", opstring, spec.Version) + _ = ops.errorf("%s opcode was introduced in TEAL v%d", opstring, spec.Version) } else { spec, ok = keywords[opstring] } diff --git a/data/transactions/logic/eval.go b/data/transactions/logic/eval.go index d2e30b47c3..d34feaa931 100644 --- a/data/transactions/logic/eval.go +++ b/data/transactions/logic/eval.go @@ -1310,17 +1310,17 @@ func opLt(cx *EvalContext) error { // opSwap, opLt, and opNot always succeed (return nil). So error checking elided in Gt,Le,Ge func opGt(cx *EvalContext) error { - opSwap(cx) + _ = opSwap(cx) return opLt(cx) } func opLe(cx *EvalContext) error { - opGt(cx) + _ = opGt(cx) return opNot(cx) } func opGe(cx *EvalContext) error { - opLt(cx) + _ = opLt(cx) return opNot(cx) } diff --git a/ledger/accountdb.go b/ledger/accountdb.go index 276a1707f9..42b3b65be4 100644 --- a/ledger/accountdb.go +++ b/ledger/accountdb.go @@ -301,7 +301,7 @@ type compactAccountDeltas struct { } // onlineAccountDelta track all changes of account state within a range, -// used in conjunction wih compactOnlineAccountDeltas to group and represent per-account changes. +// used in conjunction with compactOnlineAccountDeltas to group and represent per-account changes. // oldAcct represents the "old" state of the account in the DB, and compared against newAcct[0] // to determine if the acct became online or went offline. type onlineAccountDelta struct { diff --git a/ledger/acctonline.go b/ledger/acctonline.go index fff9a22662..1de4138f61 100644 --- a/ledger/acctonline.go +++ b/ledger/acctonline.go @@ -620,7 +620,6 @@ func (ao *onlineAccounts) lookupOnlineAccountData(rnd basics.Round, addr basics. } // the round number cannot be found in deltas, it is in history inHistory = true - err = nil } paramsOffset, err = ao.roundParamsOffset(rnd) if err != nil { @@ -744,7 +743,6 @@ func (ao *onlineAccounts) onlineTop(rnd basics.Round, voteRnd basics.Round, n ui } // the round number cannot be found in deltas, it is in history inMemory = false - err = nil } modifiedAccounts := make(map[basics.Address]*ledgercore.OnlineAccount) diff --git a/ledger/tracker.go b/ledger/tracker.go index ded5ce4816..7479771f2d 100644 --- a/ledger/tracker.go +++ b/ledger/tracker.go @@ -434,7 +434,10 @@ func (tr *trackerRegistry) commitSyncer(deferredCommits chan *deferredCommitCont if !ok { return } - tr.commitRound(commit) + err := tr.commitRound(commit) + if err != nil { + tr.log.Errorf("commitSyncer: commitRound error %s", err.Error()) + } case <-tr.ctx.Done(): // drain the pending commits queue: drained := false diff --git a/network/wsNetwork.go b/network/wsNetwork.go index eefd3b0325..054025d15c 100644 --- a/network/wsNetwork.go +++ b/network/wsNetwork.go @@ -201,7 +201,7 @@ type GossipNode interface { // this node to send corresponding MsgOfInterest notifications to any // newly connecting peers. This should be called before the network // is started. - RegisterMessageInterest(protocol.Tag) error + RegisterMessageInterest(protocol.Tag) // SubstituteGenesisID substitutes the "{genesisID}" with their network-specific genesisID. SubstituteGenesisID(rawURL string) string @@ -2308,7 +2308,7 @@ func SetUserAgentHeader(header http.Header) { // this node to send corresponding MsgOfInterest notifications to any // newly connecting peers. This should be called before the network // is started. -func (wn *WebsocketNetwork) RegisterMessageInterest(t protocol.Tag) error { +func (wn *WebsocketNetwork) RegisterMessageInterest(t protocol.Tag) { wn.messagesOfInterestMu.Lock() defer wn.messagesOfInterestMu.Unlock() @@ -2321,11 +2321,11 @@ func (wn *WebsocketNetwork) RegisterMessageInterest(t protocol.Tag) error { wn.messagesOfInterest[t] = true wn.updateMessagesOfInterestEnc() - return nil + return } // DeregisterMessageInterest will tell peers to no longer send us traffic with a protocol Tag -func (wn *WebsocketNetwork) DeregisterMessageInterest(t protocol.Tag) error { +func (wn *WebsocketNetwork) DeregisterMessageInterest(t protocol.Tag) { wn.messagesOfInterestMu.Lock() defer wn.messagesOfInterestMu.Unlock() @@ -2338,7 +2338,7 @@ func (wn *WebsocketNetwork) DeregisterMessageInterest(t protocol.Tag) error { delete(wn.messagesOfInterest, t) wn.updateMessagesOfInterestEnc() - return nil + return } func (wn *WebsocketNetwork) updateMessagesOfInterestEnc() { diff --git a/network/wsNetwork_test.go b/network/wsNetwork_test.go index 424586f019..048650840d 100644 --- a/network/wsNetwork_test.go +++ b/network/wsNetwork_test.go @@ -1839,8 +1839,8 @@ func TestWebsocketNetworkMessageOfInterest(t *testing.T) { waitReady(t, netB, readyTimeout.C) // have netB asking netA to send it only AgreementVoteTag and ProposalPayloadTag - require.NoError(t, netB.RegisterMessageInterest(ft1)) - require.NoError(t, netB.RegisterMessageInterest(ft2)) + netB.RegisterMessageInterest(ft1) + netB.RegisterMessageInterest(ft2) // send another message which we can track, so that we'll know that the first message was delivered. netB.Broadcast(context.Background(), protocol.VoteBundleTag, []byte{0, 1, 2, 3, 4}, true, nil) messageFilterArriveWg.Wait() diff --git a/test/e2e-go/cli/goal/expect/catchpointCatchupWebProxy/webproxy.go b/test/e2e-go/cli/goal/expect/catchpointCatchupWebProxy/webproxy.go index 335d5bafb2..61a7f840a8 100644 --- a/test/e2e-go/cli/goal/expect/catchpointCatchupWebProxy/webproxy.go +++ b/test/e2e-go/cli/goal/expect/catchpointCatchupWebProxy/webproxy.go @@ -55,8 +55,8 @@ func main() { mu.Unlock() // prevent requests for block #2 to go through. if strings.HasSuffix(request.URL.String(), "/block/2") { - response.Write([]byte("webProxy prevents block 2 from serving")) response.WriteHeader(http.StatusBadRequest) + _, _ = response.Write([]byte("webProxy prevents block 2 from serving")) return } if *webProxyLogFile != "" { From 389c6d69afa4411624bee1c120b245a2d75818db Mon Sep 17 00:00:00 2001 From: chris erway Date: Fri, 5 Aug 2022 21:01:29 -0400 Subject: [PATCH 02/16] fix new lint errors --- network/wsNetwork.go | 2 -- protocol/codec_tester.go | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/network/wsNetwork.go b/network/wsNetwork.go index 054025d15c..c36a0db5b6 100644 --- a/network/wsNetwork.go +++ b/network/wsNetwork.go @@ -2321,7 +2321,6 @@ func (wn *WebsocketNetwork) RegisterMessageInterest(t protocol.Tag) { wn.messagesOfInterest[t] = true wn.updateMessagesOfInterestEnc() - return } // DeregisterMessageInterest will tell peers to no longer send us traffic with a protocol Tag @@ -2338,7 +2337,6 @@ func (wn *WebsocketNetwork) DeregisterMessageInterest(t protocol.Tag) { delete(wn.messagesOfInterest, t) wn.updateMessagesOfInterestEnc() - return } func (wn *WebsocketNetwork) updateMessagesOfInterestEnc() { diff --git a/protocol/codec_tester.go b/protocol/codec_tester.go index 694c2c492c..9594a4e430 100644 --- a/protocol/codec_tester.go +++ b/protocol/codec_tester.go @@ -433,7 +433,7 @@ func RunEncodingTest(t *testing.T, template msgpMarshalUnmarshal) { // some objects might appen to the original error additional info. // we ensure that invalidObject error is not failing the test. - if errors.As(err, &ErrInvalidObject) { + if errors.Is(err, ErrInvalidObject) { continue } require.NoError(t, err) From 675dcf29e42ec44e983eb496e205ee1b14419f4d Mon Sep 17 00:00:00 2001 From: chris erway Date: Fri, 5 Aug 2022 21:29:08 -0400 Subject: [PATCH 03/16] update for latest master --- data/transactions/logic/assembler.go | 14 +++++++------- data/transactions/logic/eval.go | 6 +++--- .../expect/catchpointCatchupWebProxy/webproxy.go | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/data/transactions/logic/assembler.go b/data/transactions/logic/assembler.go index 4bb9ff772c..a0d9625a92 100644 --- a/data/transactions/logic/assembler.go +++ b/data/transactions/logic/assembler.go @@ -1230,7 +1230,7 @@ func pseudoImmediatesError(ops *OpStream, name string, specs map[int]OpSpec) { immediateCounts[i] = numImms i++ } - ops.error(name + " expects " + joinIntsOnOr("immediate argument", immediateCounts...)) + ops.error(name + " expects " + joinIntsOnOr("immediate argument", immediateCounts...)) //nolint:errcheck } // getSpec finds the OpSpec we need during assembly based on its name, our current version, and the immediates passed in @@ -1255,7 +1255,7 @@ func getSpec(ops *OpStream, name string, args []string) (OpSpec, string, bool) { } pseudo.Name = name if pseudo.Version > ops.Version { - ops.errorf("%s opcode with %s was introduced in v%d", pseudo.Name, joinIntsOnOr("immediate", len(args)), pseudo.Version) + ops.errorf("%s opcode with %s was introduced in v%d", pseudo.Name, joinIntsOnOr("immediate", len(args)), pseudo.Version) //nolint:errcheck } if len(args) == 0 { return pseudo, pseudo.Name + " without immediates", true @@ -1266,9 +1266,9 @@ func getSpec(ops *OpStream, name string, args []string) (OpSpec, string, bool) { if !ok { spec, ok = OpsByName[AssemblerMaxVersion][name] if ok { - ops.errorf("%s opcode was introduced in v%d", name, spec.Version) + ops.errorf("%s opcode was introduced in v%d", name, spec.Version) //nolint:errcheck } else { - ops.errorf("unknown opcode: %s", name) + ops.errorf("unknown opcode: %s", name) //nolint:errcheck } } return spec, spec.Name, ok @@ -1482,7 +1482,7 @@ func (ops *OpStream) trace(format string, args ...interface{}) { func (ops *OpStream) typeError(err error) { if ops.typeTracking { - _ = ops.error(err) + ops.error(err) //nolint:errcheck } } @@ -1711,12 +1711,12 @@ func (ops *OpStream) resolveLabels() { // all branch instructions (currently) are opcode byte and 2 offset bytes, and the destination is relative to the next pc as if the branch was a no-op naturalPc := lr.position + 3 if ops.Version < backBranchEnabledVersion && dest < naturalPc { - ops.errorf("label %#v is a back reference, back jump support was introduced in v4", lr.label) + ops.errorf("label %#v is a back reference, back jump support was introduced in v4", lr.label) //nolint:errcheck continue } jump := dest - naturalPc if jump > 0x7fff { - ops.errorf("label %#v is too far away", lr.label) + ops.errorf("label %#v is too far away", lr.label) //nolint:errcheck continue } raw[lr.position+1] = uint8(jump >> 8) diff --git a/data/transactions/logic/eval.go b/data/transactions/logic/eval.go index 9695eb1b39..e2c0287cf3 100644 --- a/data/transactions/logic/eval.go +++ b/data/transactions/logic/eval.go @@ -1316,17 +1316,17 @@ func opLt(cx *EvalContext) error { // opSwap, opLt, and opNot always succeed (return nil). So error checking elided in Gt,Le,Ge func opGt(cx *EvalContext) error { - _ = opSwap(cx) + opSwap(cx) //nolint:errcheck return opLt(cx) } func opLe(cx *EvalContext) error { - _ = opGt(cx) + opGt(cx) //nolint:errcheck return opNot(cx) } func opGe(cx *EvalContext) error { - _ = opLt(cx) + opLt(cx) //nolint:errcheck return opNot(cx) } diff --git a/test/e2e-go/cli/goal/expect/catchpointCatchupWebProxy/webproxy.go b/test/e2e-go/cli/goal/expect/catchpointCatchupWebProxy/webproxy.go index 61a7f840a8..3b524cc154 100644 --- a/test/e2e-go/cli/goal/expect/catchpointCatchupWebProxy/webproxy.go +++ b/test/e2e-go/cli/goal/expect/catchpointCatchupWebProxy/webproxy.go @@ -56,7 +56,7 @@ func main() { // prevent requests for block #2 to go through. if strings.HasSuffix(request.URL.String(), "/block/2") { response.WriteHeader(http.StatusBadRequest) - _, _ = response.Write([]byte("webProxy prevents block 2 from serving")) + response.Write([]byte("webProxy prevents block 2 from serving")) //nolint:errcheck return } if *webProxyLogFile != "" { From 18c51feaa337800d25a388348e39aa7bec9aaa63 Mon Sep 17 00:00:00 2001 From: chris erway Date: Fri, 5 Aug 2022 21:33:46 -0400 Subject: [PATCH 04/16] switch to github-pr-check, github-pr-review not working --- .github/workflows/reviewdog.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/reviewdog.yml b/.github/workflows/reviewdog.yml index 3aed9d8512..f9ec865773 100644 --- a/.github/workflows/reviewdog.yml +++ b/.github/workflows/reviewdog.yml @@ -20,7 +20,7 @@ jobs: with: golangci_lint_version: "v1.41.1" golangci_lint_flags: "-c .golangci.yml --allow-parallel-runners" - reporter: "github-pr-review" + reporter: "github-pr-check" tool_name: "Lint Errors" level: "error" fail_on_error: true From 4fefd94a992d7a536ec1f564a8b2cadc0a23b880 Mon Sep 17 00:00:00 2001 From: chris erway Date: Tue, 16 Aug 2022 11:50:39 -0400 Subject: [PATCH 05/16] fix latest linter errors from master --- .golangci.yml | 4 +++- crypto/merklesignature/const.go | 3 ++- crypto/stateproof/coinGenerator.go | 7 ++++--- data/transactions/logic/assembler.go | 10 +++++----- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 9cf49999f3..98d844cf6d 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -7,7 +7,7 @@ linters: enable: - errcheck - gofmt - - golint + - revive - govet - ineffassign - misspell @@ -41,6 +41,8 @@ issues: - Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked # "EXC0005 staticcheck: Developers tend to write in C-style with an explicit 'break' in a 'switch', so it's ok to ignore" - ineffective break statement. Did you mean to break out of the outer loop + # revive: irrelevant error about naming + - "var-naming: don't use leading k in Go names" exclude-rules: # Add all linters here -- Comment this block out for testing linters diff --git a/crypto/merklesignature/const.go b/crypto/merklesignature/const.go index c98321b514..767f14aaef 100644 --- a/crypto/merklesignature/const.go +++ b/crypto/merklesignature/const.go @@ -18,6 +18,7 @@ package merklesignature import ( "fmt" + "github.com/algorand/go-algorand/crypto" ) @@ -40,7 +41,7 @@ const ( var NoKeysCommitment = Commitment{} func init() { - // no keys generated, inner tree of merkle siganture scheme is empty. + // no keys generated, inner tree of merkle signature scheme is empty. o, err := New(KeyLifetimeDefault+1, KeyLifetimeDefault+2, KeyLifetimeDefault) if err != nil { panic(fmt.Errorf("initializing empty merkle signature scheme failed, err: %w", err)) diff --git a/crypto/stateproof/coinGenerator.go b/crypto/stateproof/coinGenerator.go index 320232fbaa..2abf26f3a1 100644 --- a/crypto/stateproof/coinGenerator.go +++ b/crypto/stateproof/coinGenerator.go @@ -18,9 +18,10 @@ package stateproof import ( "encoding/binary" - "golang.org/x/crypto/sha3" "math/big" + "golang.org/x/crypto/sha3" + "github.com/algorand/go-algorand/crypto" "github.com/algorand/go-algorand/protocol" ) @@ -75,7 +76,7 @@ func makeCoinGenerator(choice *coinChoiceSeed) coinGenerator { choice.version = VersionForCoinGenerator rep := crypto.HashRep(choice) shk := sha3.NewShake256() - shk.Write(rep) + shk.Write(rep) //nolint:errcheck threshold := prepareRejectionSamplingThreshold(choice.signedWeight) return coinGenerator{shkContext: shk, signedWeight: choice.signedWeight, threshold: threshold} @@ -111,7 +112,7 @@ func (cg *coinGenerator) getNextCoin() uint64 { var randNumFromXof uint64 for { var shakeDigest [8]byte - cg.shkContext.Read(shakeDigest[:]) + cg.shkContext.Read(shakeDigest[:]) //nolint:errcheck randNumFromXof = binary.LittleEndian.Uint64(shakeDigest[:]) z := &big.Int{} diff --git a/data/transactions/logic/assembler.go b/data/transactions/logic/assembler.go index 26e9ea6c6f..d537f3a434 100644 --- a/data/transactions/logic/assembler.go +++ b/data/transactions/logic/assembler.go @@ -1571,10 +1571,10 @@ func (ops *OpStream) assemble(text string) error { directive := first[1:] switch directive { case "pragma": - ops.pragma(tokens) + ops.pragma(tokens) //nolint:errcheck ops.trace("%3d: #pragma line\n", ops.sourceLine) default: - ops.errorf("Unknown directive: %s", directive) + ops.errorf("Unknown directive: %s", directive) //nolint:errcheck } continue } @@ -1618,8 +1618,8 @@ func (ops *OpStream) assemble(text string) error { } } ops.trackStack(args, returns, append([]string{expandedName}, current[1:]...)) - spec.asm(ops, &spec, current[1:]) - if spec.deadens() { // An unconditional branch deadens the following code + spec.asm(ops, &spec, current[1:]) //nolint:errcheck + if spec.deadens() { // An unconditional branch deadens the following code ops.known.deaden() } if spec.Name == "callsub" { @@ -1636,7 +1636,7 @@ func (ops *OpStream) assemble(text string) error { if errors.Is(err, bufio.ErrTooLong) { err = errors.New("line too long") } - ops.error(err) + ops.error(err) //nolint:errcheck } // backward compatibility: do not allow jumps behind last instruction in v1 From 504bd3d393e3628425ceedad4de58b2bde51b698 Mon Sep 17 00:00:00 2001 From: chris erway Date: Tue, 16 Aug 2022 12:43:54 -0400 Subject: [PATCH 06/16] codec_tester: make randomizeValue generate valid HashType values --- protocol/codec_tester.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/protocol/codec_tester.go b/protocol/codec_tester.go index 9594a4e430..5c174169ff 100644 --- a/protocol/codec_tester.go +++ b/protocol/codec_tester.go @@ -17,7 +17,6 @@ package protocol import ( - "errors" "fmt" "io/ioutil" "math/rand" @@ -230,7 +229,12 @@ func randomizeValue(v reflect.Value, datapath string, tag string, remainingChang switch v.Kind() { case reflect.Uint, reflect.Uintptr, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: - v.SetUint(rand.Uint64()) + if strings.HasSuffix(datapath, "HashFactory/HashType") { + // generate random value that will avoid protocol.ErrInvalidObject from HashType.Validate() + v.SetUint(rand.Uint64() % 3) + } else { + v.SetUint(rand.Uint64()) + } *remainingChanges-- case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: v.SetInt(int64(rand.Uint64())) @@ -431,11 +435,6 @@ func RunEncodingTest(t *testing.T, template msgpMarshalUnmarshal) { continue } - // some objects might appen to the original error additional info. - // we ensure that invalidObject error is not failing the test. - if errors.Is(err, ErrInvalidObject) { - continue - } require.NoError(t, err) } } From 1c3d9c720e13ffc98e1c5b27303564511cf3276f Mon Sep 17 00:00:00 2001 From: chris erway Date: Tue, 16 Aug 2022 12:54:28 -0400 Subject: [PATCH 07/16] demote deleteLedgerFiles to nolint:errcheck --- cmd/catchpointdump/net.go | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/cmd/catchpointdump/net.go b/cmd/catchpointdump/net.go index 072e87458c..c305236209 100644 --- a/cmd/catchpointdump/net.go +++ b/cmd/catchpointdump/net.go @@ -303,11 +303,7 @@ func deleteLedgerFiles(deleteTracker bool) error { func loadAndDump(addr string, tarFile string, genesisInitState ledgercore.InitState) error { // delete current ledger files. - err := deleteLedgerFiles(true) - if err != nil { - reportErrorf("Unable to delete ledger files : %v", err) - return err - } + deleteLedgerFiles(true) //nolint:errcheck cfg := config.GetDefaultLocal() l, err := ledger.OpenLedger(logging.Base(), "./ledger", false, genesisInitState, cfg) if err != nil { @@ -315,12 +311,7 @@ func loadAndDump(addr string, tarFile string, genesisInitState ledgercore.InitSt return err } - defer func() { - err := deleteLedgerFiles(!loadOnly) - if err != nil { - reportErrorf("Unable to delete ledger files : %v", err) - } - }() + defer deleteLedgerFiles(!loadOnly) //nolint:errcheck defer l.Close() catchupAccessor := ledger.MakeCatchpointCatchupAccessor(l, logging.Base()) From 8e9c6fb4f9e489b335a578ae189999f22f1dd199 Mon Sep 17 00:00:00 2001 From: chris erway Date: Tue, 16 Aug 2022 13:57:18 -0400 Subject: [PATCH 08/16] codec_tester: find more HashType fields (in merklearray.Tree) --- protocol/codec_tester.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocol/codec_tester.go b/protocol/codec_tester.go index 5c174169ff..b5db2178b8 100644 --- a/protocol/codec_tester.go +++ b/protocol/codec_tester.go @@ -229,7 +229,7 @@ func randomizeValue(v reflect.Value, datapath string, tag string, remainingChang switch v.Kind() { case reflect.Uint, reflect.Uintptr, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: - if strings.HasSuffix(datapath, "HashFactory/HashType") { + if strings.HasSuffix(datapath, "/HashType") { // generate random value that will avoid protocol.ErrInvalidObject from HashType.Validate() v.SetUint(rand.Uint64() % 3) } else { From 8c56e366feed940eeb60ff0987748656f42e2fd3 Mon Sep 17 00:00:00 2001 From: chris erway Date: Tue, 16 Aug 2022 23:06:37 -0400 Subject: [PATCH 09/16] fix codec_tester when used with pointer fields --- protocol/codec_test.go | 21 +++++++++++++++++++++ protocol/codec_tester.go | 9 ++++++--- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/protocol/codec_test.go b/protocol/codec_test.go index 79814dadc6..ae26f1e20d 100644 --- a/protocol/codec_test.go +++ b/protocol/codec_test.go @@ -199,3 +199,24 @@ func TestEncodeJSON(t *testing.T) { require.True(t, reflect.DeepEqual(v, nsv)) require.True(t, reflect.DeepEqual(v, sv)) } + +func TestRandomizeObjectWithPtrField(t *testing.T) { + partitiontest.PartitionTest(t) + + type testObjA struct { + U64 uint64 + } + type testObjB struct { + U16 uint16 + ObjA *testObjA + } + + obj, err := RandomizeObject(&testObjB{}) + require.NoError(t, err) + objB, ok := obj.(*testObjB) + require.True(t, ok) + t.Logf("%+v %+v", objB, objB.ObjA) + require.NotZero(t, objB.U16) + require.NotZero(t, objB.ObjA) + require.NotZero(t, objB.ObjA.U64) +} diff --git a/protocol/codec_tester.go b/protocol/codec_tester.go index b5db2178b8..06a43821e0 100644 --- a/protocol/codec_tester.go +++ b/protocol/codec_tester.go @@ -247,6 +247,12 @@ func randomizeValue(v reflect.Value, datapath string, tag string, remainingChang } v.SetString(string(buf)) *remainingChanges-- + case reflect.Ptr: + v.Set(reflect.New(v.Type().Elem())) + err := randomizeValue(reflect.Indirect(v), datapath, tag, remainingChanges, seenTypes) + if err != nil { + return err + } case reflect.Struct: st := v.Type() if !seenTypes[st] { @@ -431,9 +437,6 @@ func RunEncodingTest(t *testing.T, template msgpMarshalUnmarshal) { t.Skip() return } - if err == nil { - continue - } require.NoError(t, err) } From 5cf4dc370263e3b8ecbc8dc0e598ea8e594e38ee Mon Sep 17 00:00:00 2001 From: chris erway Date: Tue, 16 Aug 2022 23:51:08 -0400 Subject: [PATCH 10/16] explain ignoring shakehash read/write --- crypto/stateproof/coinGenerator.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crypto/stateproof/coinGenerator.go b/crypto/stateproof/coinGenerator.go index 2abf26f3a1..fa88c57706 100644 --- a/crypto/stateproof/coinGenerator.go +++ b/crypto/stateproof/coinGenerator.go @@ -76,7 +76,7 @@ func makeCoinGenerator(choice *coinChoiceSeed) coinGenerator { choice.version = VersionForCoinGenerator rep := crypto.HashRep(choice) shk := sha3.NewShake256() - shk.Write(rep) //nolint:errcheck + shk.Write(rep) //nolint:errcheck // ShakeHash.Write may panic, but does not return error threshold := prepareRejectionSamplingThreshold(choice.signedWeight) return coinGenerator{shkContext: shk, signedWeight: choice.signedWeight, threshold: threshold} @@ -112,7 +112,7 @@ func (cg *coinGenerator) getNextCoin() uint64 { var randNumFromXof uint64 for { var shakeDigest [8]byte - cg.shkContext.Read(shakeDigest[:]) //nolint:errcheck + cg.shkContext.Read(shakeDigest[:]) //nolint:errcheck // ShakeHash.Read never returns error randNumFromXof = binary.LittleEndian.Uint64(shakeDigest[:]) z := &big.Int{} From 76442f5cd5a36b8bcb3e2f45144e5129b095042a Mon Sep 17 00:00:00 2001 From: chris erway Date: Wed, 17 Aug 2022 09:05:40 -0400 Subject: [PATCH 11/16] codec_tester: be more precise in identifying crypto.HashType --- protocol/codec_tester.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/protocol/codec_tester.go b/protocol/codec_tester.go index 06a43821e0..ff7a088f76 100644 --- a/protocol/codec_tester.go +++ b/protocol/codec_tester.go @@ -229,9 +229,9 @@ func randomizeValue(v reflect.Value, datapath string, tag string, remainingChang switch v.Kind() { case reflect.Uint, reflect.Uintptr, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: - if strings.HasSuffix(datapath, "/HashType") { - // generate random value that will avoid protocol.ErrInvalidObject from HashType.Validate() - v.SetUint(rand.Uint64() % 3) + if typ := v.Type(); strings.HasSuffix(typ.PkgPath(), "go-algorand/crypto") && typ.Name() == "HashType" { + // generate value that will avoid protocol.ErrInvalidObject from HashType.Validate() + v.SetUint(rand.Uint64() % 3) // 3 is crypto.MaxHashType } else { v.SetUint(rand.Uint64()) } From 638a31a9b799100c8597662d97e417c347069db5 Mon Sep 17 00:00:00 2001 From: chris erway Date: Wed, 17 Aug 2022 09:34:42 -0400 Subject: [PATCH 12/16] improve codec_tester performance a little bit --- ledger/accountdb.go | 2 +- protocol/codec_tester.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/ledger/accountdb.go b/ledger/accountdb.go index 342331ed08..4c96b15cf6 100644 --- a/ledger/accountdb.go +++ b/ledger/accountdb.go @@ -1593,7 +1593,7 @@ func (bo *baseOnlineAccountData) SetCoreAccountData(ad *ledgercore.AccountData) type resourceFlags uint8 const ( - resourceFlagsHolding resourceFlags = 0 //nolint:deadcode,varcheck + resourceFlagsHolding resourceFlags = 0 resourceFlagsNotHolding resourceFlags = 1 resourceFlagsOwnership resourceFlags = 2 resourceFlagsEmptyAsset resourceFlags = 4 diff --git a/protocol/codec_tester.go b/protocol/codec_tester.go index ff7a088f76..d6d2c375e7 100644 --- a/protocol/codec_tester.go +++ b/protocol/codec_tester.go @@ -229,7 +229,8 @@ func randomizeValue(v reflect.Value, datapath string, tag string, remainingChang switch v.Kind() { case reflect.Uint, reflect.Uintptr, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: - if typ := v.Type(); strings.HasSuffix(typ.PkgPath(), "go-algorand/crypto") && typ.Name() == "HashType" { + if strings.HasSuffix(datapath, "/HashType") && + strings.HasSuffix(v.Type().PkgPath(), "go-algorand/crypto") && v.Type().Name() == "HashType" { // generate value that will avoid protocol.ErrInvalidObject from HashType.Validate() v.SetUint(rand.Uint64() % 3) // 3 is crypto.MaxHashType } else { From c6b1486bad858e1ba7b17f458e71ffb72fc9a5de Mon Sep 17 00:00:00 2001 From: chris erway Date: Wed, 17 Aug 2022 11:10:01 -0400 Subject: [PATCH 13/16] clean for nolintlint, golint, staticcheck, typecheck --- .golangci-warnings.yml | 6 ++--- .golangci.yml | 21 ++++++++++++++- cmd/algokey/keyreg.go | 8 ++++-- cmd/catchpointdump/net.go | 10 +++++-- cmd/goal/account.go | 16 +++-------- cmd/goal/application.go | 12 ++++++--- cmd/loadgenerator/main.go | 8 +++--- data/ledger.go | 2 +- data/transactions/logic/assembler.go | 27 ++++++++++--------- data/transactions/logic/eval.go | 6 ++--- .../catchpointCatchupWebProxy/webproxy.go | 2 +- test/framework/fixtures/expectFixture.go | 3 +-- util/sleep_linux_32.go | 2 +- util/sleep_linux_64.go | 2 +- 14 files changed, 75 insertions(+), 50 deletions(-) diff --git a/.golangci-warnings.yml b/.golangci-warnings.yml index c0d9e1e387..769ea9ddbd 100644 --- a/.golangci-warnings.yml +++ b/.golangci-warnings.yml @@ -5,14 +5,12 @@ run: linters: disable-all: true enable: - - staticcheck + - deadcode + - partitiontest - structcheck - typecheck - varcheck - - deadcode - - gosimple - unused - - partitiontest linters-settings: diff --git a/.golangci.yml b/.golangci.yml index 98d844cf6d..98ef1416ba 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -3,18 +3,37 @@ run: tests: false linters: + # default: deadcode, errcheck, gosimple, govet, ineffassign, staticcheck, typecheck, unused, varcheck disable-all: true enable: - errcheck - gofmt - - revive + - gosimple - govet - ineffassign - misspell + - nolintlint + - revive + - staticcheck + - typecheck severity: default-severity: error +linters-settings: + nolintlint: + # require naming a specific linter X using //nolint:X + require-specific: true + # require comments like "//nolint:errcheck // Explanation of why we are ignoring linter here..." + require-explanation: true + errcheck: + exclude-functions: + # data/transactions/logic/assembler.go uses ops.error, warn, to append log messages: OK to ignore for this case + - (*github.com/algorand/go-algorand/data/transactions/logic.OpStream).errorf + - (*github.com/algorand/go-algorand/data/transactions/logic.OpStream).error + - (*github.com/algorand/go-algorand/data/transactions/logic.OpStream).warnf + - (*github.com/algorand/go-algorand/data/transactions/logic.OpStream).warn + issues: # use these new lint checks on code since #2574 new-from-rev: eb019291beed556ec6ac1ceb4a15114ce4df0c57 diff --git a/cmd/algokey/keyreg.go b/cmd/algokey/keyreg.go index 24ddafdd1e..7b3bf5ef82 100644 --- a/cmd/algokey/keyreg.go +++ b/cmd/algokey/keyreg.go @@ -75,10 +75,14 @@ func init() { keyregCmd.Flags().Uint64Var(¶ms.fee, "fee", minFee, "transaction fee") keyregCmd.Flags().Uint64Var(¶ms.firstValid, "firstvalid", 0, "first round where the transaction may be committed to the ledger") - keyregCmd.MarkFlagRequired("firstvalid") // nolint:errcheck + if err := keyregCmd.MarkFlagRequired("firstvalid"); err != nil { + panic(err) + } keyregCmd.Flags().Uint64Var(¶ms.lastValid, "lastvalid", 0, fmt.Sprintf("last round where the generated transaction may be committed to the ledger, defaults to firstvalid + %d", txnLife)) keyregCmd.Flags().StringVar(¶ms.network, "network", "mainnet", "the network where the provided keys will be registered, one of mainnet/testnet/betanet") - keyregCmd.MarkFlagRequired("network") // nolint:errcheck + if err := keyregCmd.MarkFlagRequired("network"); err != nil { + panic(err) + } keyregCmd.Flags().BoolVar(¶ms.offline, "offline", false, "set to bring an account offline") keyregCmd.Flags().StringVarP(¶ms.txFile, "outputFile", "o", "", fmt.Sprintf("write signed transaction to this file, or '%s' to write to stdout", stdoutFilenameValue)) keyregCmd.Flags().StringVar(¶ms.partkeyFile, "keyfile", "", "participation keys to register, file is opened to fetch metadata for the transaction; only specify when bringing an account online to vote in Algorand consensus") diff --git a/cmd/catchpointdump/net.go b/cmd/catchpointdump/net.go index c305236209..9073ece66e 100644 --- a/cmd/catchpointdump/net.go +++ b/cmd/catchpointdump/net.go @@ -303,7 +303,9 @@ func deleteLedgerFiles(deleteTracker bool) error { func loadAndDump(addr string, tarFile string, genesisInitState ledgercore.InitState) error { // delete current ledger files. - deleteLedgerFiles(true) //nolint:errcheck + if err := deleteLedgerFiles(true); err != nil { + reportWarnf("Error deleting ledger files: %v", err) + } cfg := config.GetDefaultLocal() l, err := ledger.OpenLedger(logging.Base(), "./ledger", false, genesisInitState, cfg) if err != nil { @@ -311,7 +313,11 @@ func loadAndDump(addr string, tarFile string, genesisInitState ledgercore.InitSt return err } - defer deleteLedgerFiles(!loadOnly) //nolint:errcheck + defer func() { + if err := deleteLedgerFiles(!loadOnly); err != nil { + reportWarnf("Error deleting ledger files: %v", err) + } + }() defer l.Close() catchupAccessor := ledger.MakeCatchpointCatchupAccessor(l, logging.Base()) diff --git a/cmd/goal/account.go b/cmd/goal/account.go index 6cd3b9a87e..cf40e011cb 100644 --- a/cmd/goal/account.go +++ b/cmd/goal/account.go @@ -540,9 +540,7 @@ func printAccountInfo(client libgoal.Client, address string, onlyShowAssetIds bo var createdAssets []generatedV2.Asset if account.CreatedAssets != nil { createdAssets = make([]generatedV2.Asset, len(*account.CreatedAssets)) - for i, asset := range *account.CreatedAssets { - createdAssets[i] = asset - } + copy(createdAssets, *account.CreatedAssets) sort.Slice(createdAssets, func(i, j int) bool { return createdAssets[i].Index < createdAssets[j].Index }) @@ -551,9 +549,7 @@ func printAccountInfo(client libgoal.Client, address string, onlyShowAssetIds bo var heldAssets []generatedV2.AssetHolding if account.Assets != nil { heldAssets = make([]generatedV2.AssetHolding, len(*account.Assets)) - for i, assetHolding := range *account.Assets { - heldAssets[i] = assetHolding - } + copy(heldAssets, *account.Assets) sort.Slice(heldAssets, func(i, j int) bool { return heldAssets[i].AssetId < heldAssets[j].AssetId }) @@ -562,9 +558,7 @@ func printAccountInfo(client libgoal.Client, address string, onlyShowAssetIds bo var createdApps []generatedV2.Application if account.CreatedApps != nil { createdApps = make([]generatedV2.Application, len(*account.CreatedApps)) - for i, app := range *account.CreatedApps { - createdApps[i] = app - } + copy(createdApps, *account.CreatedApps) sort.Slice(createdApps, func(i, j int) bool { return createdApps[i].Id < createdApps[j].Id }) @@ -573,9 +567,7 @@ func printAccountInfo(client libgoal.Client, address string, onlyShowAssetIds bo var optedInApps []generatedV2.ApplicationLocalState if account.AppsLocalState != nil { optedInApps = make([]generatedV2.ApplicationLocalState, len(*account.AppsLocalState)) - for i, appLocalState := range *account.AppsLocalState { - optedInApps[i] = appLocalState - } + copy(optedInApps, *account.AppsLocalState) sort.Slice(optedInApps, func(i, j int) bool { return optedInApps[i].Id < optedInApps[j].Id }) diff --git a/cmd/goal/application.go b/cmd/goal/application.go index 77596f38a3..41470707d7 100644 --- a/cmd/goal/application.go +++ b/cmd/goal/application.go @@ -189,9 +189,15 @@ func init() { infoAppCmd.MarkFlagRequired("app-id") - methodAppCmd.MarkFlagRequired("method") // nolint:errcheck // follow previous required flag format - methodAppCmd.MarkFlagRequired("from") // nolint:errcheck - methodAppCmd.Flags().MarkHidden("app-arg") // nolint:errcheck + panicIfErr(methodAppCmd.MarkFlagRequired("method")) + panicIfErr(methodAppCmd.MarkFlagRequired("from")) + panicIfErr(methodAppCmd.Flags().MarkHidden("app-arg")) +} + +func panicIfErr(err error) { + if err != nil { + panic(err) + } } type appCallArg struct { diff --git a/cmd/loadgenerator/main.go b/cmd/loadgenerator/main.go index 25026f7f90..1d28323d88 100644 --- a/cmd/loadgenerator/main.go +++ b/cmd/loadgenerator/main.go @@ -66,9 +66,9 @@ func loadMnemonic(mnemonic string) crypto.Seed { // Like shared/pingpong/accounts.go func findRootKeys(algodDir string) []*crypto.SignatureSecrets { keylist := make([]*crypto.SignatureSecrets, 0, 5) - err := filepath.Walk(algodDir, func(path string, info fs.FileInfo, err error) error { + err := filepath.Walk(algodDir, func(path string, info fs.FileInfo, _ error) error { var handle db.Accessor - handle, err = db.MakeErasableAccessor(path) + handle, err := db.MakeErasableAccessor(path) if err != nil { return nil // don't care, move on } @@ -241,7 +241,7 @@ func generateTransactions(restClient client.RestClient, cfg config, privateKeys sendSize = transactionBlockSize } // create sendSize transaction to send. - txns := make([]transactions.SignedTxn, sendSize, sendSize) + txns := make([]transactions.SignedTxn, sendSize) for i := range txns { tx := transactions.Transaction{ Header: transactions.Header{ @@ -289,7 +289,7 @@ func generateTransactions(restClient client.RestClient, cfg config, privateKeys for i := 0; i < nroutines; i++ { totalSent += sent[i] } - dt := time.Now().Sub(start) + dt := time.Since(start) fmt.Fprintf(os.Stdout, "sent %d/%d in %s (%.1f/s)\n", totalSent, sendSize, dt.String(), float64(totalSent)/dt.Seconds()) if cfg.TxnsToSend != 0 { // We attempted what we were asked. We're done. diff --git a/data/ledger.go b/data/ledger.go index 8fc03cb6eb..101da721af 100644 --- a/data/ledger.go +++ b/data/ledger.go @@ -184,7 +184,7 @@ func (l *Ledger) Circulation(r basics.Round) (basics.MicroAlgos, error) { } } - totals, err := l.OnlineTotals(r) //nolint:typecheck + totals, err := l.OnlineTotals(r) if err != nil { return basics.MicroAlgos{}, err } diff --git a/data/transactions/logic/assembler.go b/data/transactions/logic/assembler.go index 15b94f8e62..aa531c9bf0 100644 --- a/data/transactions/logic/assembler.go +++ b/data/transactions/logic/assembler.go @@ -723,7 +723,7 @@ func asmMethod(ops *OpStream, spec *OpSpec, args []string) error { if err != nil { // Warn if an invalid signature is used. Don't return an error, since the ABI is not // governed by the core protocol, so there may be changes to it that we don't know about - ops.warnf("Invalid ARC-4 ABI method signature for method op: %s", err.Error()) // nolint:errcheck + ops.warnf("Invalid ARC-4 ABI method signature for method op: %s", err.Error()) } hash := sha512.Sum512_256(methodSig) ops.ByteLiteral(hash[0:4]) @@ -1230,7 +1230,7 @@ func pseudoImmediatesError(ops *OpStream, name string, specs map[int]OpSpec) { immediateCounts[i] = numImms i++ } - ops.error(name + " expects " + joinIntsOnOr("immediate argument", immediateCounts...)) //nolint:errcheck + ops.error(name + " expects " + joinIntsOnOr("immediate argument", immediateCounts...)) } // getSpec finds the OpSpec we need during assembly based on its name, our current version, and the immediates passed in @@ -1255,7 +1255,7 @@ func getSpec(ops *OpStream, name string, args []string) (OpSpec, string, bool) { } pseudo.Name = name if pseudo.Version > ops.Version { - ops.errorf("%s opcode with %s was introduced in v%d", pseudo.Name, joinIntsOnOr("immediate", len(args)), pseudo.Version) //nolint:errcheck + ops.errorf("%s opcode with %s was introduced in v%d", pseudo.Name, joinIntsOnOr("immediate", len(args)), pseudo.Version) } if len(args) == 0 { return pseudo, pseudo.Name + " without immediates", true @@ -1266,9 +1266,9 @@ func getSpec(ops *OpStream, name string, args []string) (OpSpec, string, bool) { if !ok { spec, ok = OpsByName[AssemblerMaxVersion][name] if ok { - ops.errorf("%s opcode was introduced in v%d", name, spec.Version) //nolint:errcheck + ops.errorf("%s opcode was introduced in v%d", name, spec.Version) } else { - ops.errorf("unknown opcode: %s", name) //nolint:errcheck + ops.errorf("unknown opcode: %s", name) } } return spec, spec.Name, ok @@ -1496,7 +1496,7 @@ func (ops *OpStream) trace(format string, args ...interface{}) { func (ops *OpStream) typeError(err error) { if ops.typeTracking { - ops.error(err) //nolint:errcheck + ops.error(err) } } @@ -1571,10 +1571,10 @@ func (ops *OpStream) assemble(text string) error { directive := first[1:] switch directive { case "pragma": - ops.pragma(tokens) //nolint:errcheck + ops.pragma(tokens) //nolint:errcheck // report bad pragma line error, but continue assembling ops.trace("%3d: #pragma line\n", ops.sourceLine) default: - ops.errorf("Unknown directive: %s", directive) //nolint:errcheck + ops.errorf("Unknown directive: %s", directive) } continue } @@ -1618,8 +1618,9 @@ func (ops *OpStream) assemble(text string) error { } } ops.trackStack(args, returns, append([]string{expandedName}, current[1:]...)) - spec.asm(ops, &spec, current[1:]) //nolint:errcheck - if spec.deadens() { // An unconditional branch deadens the following code + spec.asm(ops, &spec, current[1:]) //nolint:errcheck // ignore error and continue, to collect more errors + + if spec.deadens() { // An unconditional branch deadens the following code ops.known.deaden() } if spec.Name == "callsub" { @@ -1636,7 +1637,7 @@ func (ops *OpStream) assemble(text string) error { if errors.Is(err, bufio.ErrTooLong) { err = errors.New("line too long") } - ops.error(err) //nolint:errcheck + ops.error(err) } // backward compatibility: do not allow jumps behind last instruction in v1 @@ -1741,12 +1742,12 @@ func (ops *OpStream) resolveLabels() { // all branch instructions (currently) are opcode byte and 2 offset bytes, and the destination is relative to the next pc as if the branch was a no-op naturalPc := lr.position + 3 if ops.Version < backBranchEnabledVersion && dest < naturalPc { - ops.errorf("label %#v is a back reference, back jump support was introduced in v4", lr.label) //nolint:errcheck + ops.errorf("label %#v is a back reference, back jump support was introduced in v4", lr.label) continue } jump := dest - naturalPc if jump > 0x7fff { - ops.errorf("label %#v is too far away", lr.label) //nolint:errcheck + ops.errorf("label %#v is too far away", lr.label) continue } raw[lr.position+1] = uint8(jump >> 8) diff --git a/data/transactions/logic/eval.go b/data/transactions/logic/eval.go index 1603d53d44..994d018735 100644 --- a/data/transactions/logic/eval.go +++ b/data/transactions/logic/eval.go @@ -1341,17 +1341,17 @@ func opLt(cx *EvalContext) error { // opSwap, opLt, and opNot always succeed (return nil). So error checking elided in Gt,Le,Ge func opGt(cx *EvalContext) error { - opSwap(cx) //nolint:errcheck + opSwap(cx) //nolint:errcheck // opSwap always succeeds return opLt(cx) } func opLe(cx *EvalContext) error { - opGt(cx) //nolint:errcheck + opGt(cx) //nolint:errcheck // opGt always succeeds return opNot(cx) } func opGe(cx *EvalContext) error { - opLt(cx) //nolint:errcheck + opLt(cx) //nolint:errcheck // opLt always succeeds return opNot(cx) } diff --git a/test/e2e-go/cli/goal/expect/catchpointCatchupWebProxy/webproxy.go b/test/e2e-go/cli/goal/expect/catchpointCatchupWebProxy/webproxy.go index 62bd46b7a0..3ba4764383 100644 --- a/test/e2e-go/cli/goal/expect/catchpointCatchupWebProxy/webproxy.go +++ b/test/e2e-go/cli/goal/expect/catchpointCatchupWebProxy/webproxy.go @@ -58,7 +58,7 @@ func main() { // prevent requests for block #2 to go through. if strings.HasSuffix(request.URL.String(), "/block/2") { response.WriteHeader(http.StatusBadRequest) - response.Write([]byte("webProxy prevents block 2 from serving")) //nolint:errcheck + response.Write([]byte("webProxy prevents block 2 from serving")) //nolint:errcheck // don't care return } if *webProxyLogFile != "" { diff --git a/test/framework/fixtures/expectFixture.go b/test/framework/fixtures/expectFixture.go index 3d7293d404..35b7489689 100644 --- a/test/framework/fixtures/expectFixture.go +++ b/test/framework/fixtures/expectFixture.go @@ -18,7 +18,6 @@ package fixtures import ( "bytes" - "fmt" "os" "os/exec" "path" @@ -148,7 +147,7 @@ func (ef *ExpectFixture) Run() { if match, _ := regexp.MatchString(ef.testFilter, testName); match { ef.t.Run(testName, func(t *testing.T) { if reason, ok := disabledTest[testName]; ok { - t.Skip(fmt.Sprintf("Skipping %s test: %s", testName, reason)) + t.Skipf("Skipping %s test: %s", testName, reason) } partitiontest.PartitionTest(t) // Check if this expect test should by run, may SKIP diff --git a/util/sleep_linux_32.go b/util/sleep_linux_32.go index 1d155fac03..50a0e696c2 100644 --- a/util/sleep_linux_32.go +++ b/util/sleep_linux_32.go @@ -31,5 +31,5 @@ func NanoSleep(d time.Duration) { Nsec: int32(d.Nanoseconds() % time.Second.Nanoseconds()), Sec: int32(d.Nanoseconds() / time.Second.Nanoseconds()), } - syscall.Nanosleep(timeSpec, nil) // nolint:errcheck + syscall.Nanosleep(timeSpec, nil) // nolint:errcheck // ignoring error } diff --git a/util/sleep_linux_64.go b/util/sleep_linux_64.go index 2897ceaa17..b2f7a69dbe 100644 --- a/util/sleep_linux_64.go +++ b/util/sleep_linux_64.go @@ -30,5 +30,5 @@ func NanoSleep(d time.Duration) { Nsec: d.Nanoseconds() % time.Second.Nanoseconds(), Sec: d.Nanoseconds() / time.Second.Nanoseconds(), } - syscall.Nanosleep(timeSpec, nil) // nolint:errcheck + syscall.Nanosleep(timeSpec, nil) // nolint:errcheck // ignoring error } From f3c3d3e3bd0d926982572b95f2a868c8e14a446b Mon Sep 17 00:00:00 2001 From: chris erway Date: Wed, 17 Aug 2022 17:00:54 -0400 Subject: [PATCH 14/16] fix panic in goal app flags --- cmd/goal/application.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/goal/application.go b/cmd/goal/application.go index 41470707d7..eaa7de91eb 100644 --- a/cmd/goal/application.go +++ b/cmd/goal/application.go @@ -191,7 +191,7 @@ func init() { panicIfErr(methodAppCmd.MarkFlagRequired("method")) panicIfErr(methodAppCmd.MarkFlagRequired("from")) - panicIfErr(methodAppCmd.Flags().MarkHidden("app-arg")) + panicIfErr(appCmd.PersistentFlags().MarkHidden("app-arg")) } func panicIfErr(err error) { From 20952b738ea45e4b595c4ec32f8bb36ef15ebf42 Mon Sep 17 00:00:00 2001 From: chris erway Date: Thu, 18 Aug 2022 09:06:36 -0400 Subject: [PATCH 15/16] TestRandomizeObjectWithPtrField: run a few and fail if all zero --- protocol/codec_test.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/protocol/codec_test.go b/protocol/codec_test.go index ae26f1e20d..56582aa82f 100644 --- a/protocol/codec_test.go +++ b/protocol/codec_test.go @@ -211,12 +211,17 @@ func TestRandomizeObjectWithPtrField(t *testing.T) { ObjA *testObjA } - obj, err := RandomizeObject(&testObjB{}) - require.NoError(t, err) - objB, ok := obj.(*testObjB) - require.True(t, ok) - t.Logf("%+v %+v", objB, objB.ObjA) - require.NotZero(t, objB.U16) - require.NotZero(t, objB.ObjA) - require.NotZero(t, objB.ObjA.U64) + // run a few and fail if all ints are zero + sawNonZeroInt := false + for i := 0; i < 10; i++ { + obj, err := RandomizeObject(&testObjB{}) + require.NoError(t, err) + objB, ok := obj.(*testObjB) + require.True(t, ok) + require.NotNil(t, objB.ObjA) + if objB.U16 != 0 || objB.ObjA.U64 != 0 { + sawNonZeroInt = true + } + } + require.True(t, sawNonZeroInt, "RandomizeObject made all zeroes for ints") } From b563b39dd6bb1a2fe361a0c0e3d50c4b51f9ddf3 Mon Sep 17 00:00:00 2001 From: chris erway Date: Thu, 18 Aug 2022 11:15:43 -0400 Subject: [PATCH 16/16] codec_tester: split u16 and u64 tests --- protocol/codec_test.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/protocol/codec_test.go b/protocol/codec_test.go index 56582aa82f..1b1d4e6a60 100644 --- a/protocol/codec_test.go +++ b/protocol/codec_test.go @@ -212,16 +212,21 @@ func TestRandomizeObjectWithPtrField(t *testing.T) { } // run a few and fail if all ints are zero - sawNonZeroInt := false + sawNonZeroU16 := false + sawNonZeroU64 := false for i := 0; i < 10; i++ { obj, err := RandomizeObject(&testObjB{}) require.NoError(t, err) objB, ok := obj.(*testObjB) require.True(t, ok) require.NotNil(t, objB.ObjA) - if objB.U16 != 0 || objB.ObjA.U64 != 0 { - sawNonZeroInt = true + if objB.U16 != 0 { + sawNonZeroU16 = true + } + if objB.ObjA.U64 != 0 { + sawNonZeroU64 = true } } - require.True(t, sawNonZeroInt, "RandomizeObject made all zeroes for ints") + require.True(t, sawNonZeroU16, "RandomizeObject made all zeroes for testObjB.U16") + require.True(t, sawNonZeroU64, "RandomizeObject made all zeroes for testObjA.U64") }