Add revive linter (#838)

* Update golangci-lint to update gosec

* Adding Revive to linters

* bft: update justifier.AddBlock

* pr comments

* pr comments

---------

Co-authored-by: tony <[email protected]>

.golangci.yml

@@ -25,6 +25,7 @@ linters:
     - copyloopvar
     - whitespace
     - gosec
+    - revive
 
     # - structcheck # lots of false positives
     # - errcheck #lot of false positives
@@ -43,6 +44,16 @@ linters-settings:
       - G115
       - G406 # ignore ripe160 deprecation
       - G507 # ignore ripe160 deprecation
+  revive:
+    rules:
+      - name: var-naming
+        severity: warning
+        disabled: false
+        exclude: [""]
+        arguments:
+          - [] # AllowList
+          - [] # DenyList
+          - - upperCaseConst: true # Extra parameter (upperCaseConst|skipPackageNameChecks)
 
 issues:
   max-issues-per-linter: 1000

api/admin.go

@@ -34,7 +34,7 @@ func writeError(w http.ResponseWriter, errCode int, errMsg string) {
 }
 
 func getLogLevelHandler(logLevel *slog.LevelVar) http.HandlerFunc {
-	return func(w http.ResponseWriter, r *http.Request) {
+	return func(w http.ResponseWriter, _ *http.Request) {
 		response := logLevelResponse{
 			CurrentLevel: logLevel.Level().String(),
 		}

api/blocks/blocks_test.go

@@ -45,11 +45,11 @@ func TestBlock(t *testing.T) {
 
 	for name, tt := range map[string]func(*testing.T){
 		"testBadQueryParams":                    testBadQueryParams,
-		"testInvalidBlockId":                    testInvalidBlockId,
+		"testInvalidBlockID":                    testInvalidBlockID,
 		"testInvalidBlockNumber":                testInvalidBlockNumber,
-		"testGetBlockById":                      testGetBlockById,
+		"testGetBlockByID":                      testGetBlockByID,
 		"testGetBlockNotFound":                  testGetBlockNotFound,
-		"testGetExpandedBlockById":              testGetExpandedBlockById,
+		"testGetExpandedBlockByID":              testGetExpandedBlockByID,
 		"testGetBlockByHeight":                  testGetBlockByHeight,
 		"testGetBestBlock":                      testGetBestBlock,
 		"testGetFinalizedBlock":                 testGetFinalizedBlock,
@@ -111,7 +111,7 @@ func testGetJustifiedBlock(t *testing.T) {
 	assert.Equal(t, genesisBlock.Header().ID(), justified.ID)
 }
 
-func testGetBlockById(t *testing.T) {
+func testGetBlockByID(t *testing.T) {
 	res, statusCode := httpGet(t, ts.URL+"/blocks/"+blk.Header().ID().String())
 	rb := new(blocks.JSONCollapsedBlock)
 	if err := json.Unmarshal(res, rb); err != nil {
@@ -128,7 +128,7 @@ func testGetBlockNotFound(t *testing.T) {
 	assert.Equal(t, "null", strings.TrimSpace(string(res)))
 }
 
-func testGetExpandedBlockById(t *testing.T) {
+func testGetExpandedBlockByID(t *testing.T) {
 	res, statusCode := httpGet(t, ts.URL+"/blocks/"+blk.Header().ID().String()+"?expanded=true")
 	rb := new(blocks.JSONExpandedBlock)
 	if err := json.Unmarshal(res, rb); err != nil {
@@ -144,7 +144,7 @@ func testInvalidBlockNumber(t *testing.T) {
 	assert.Equal(t, http.StatusBadRequest, statusCode)
 }
 
-func testInvalidBlockId(t *testing.T) {
+func testInvalidBlockID(t *testing.T) {
 	_, statusCode := httpGet(t, ts.URL+"/blocks/"+invalidBytes32)
 	assert.Equal(t, http.StatusBadRequest, statusCode)
 }

api/debug/debug.go

@@ -56,8 +56,8 @@ func New(
 	allowCustomTracer bool,
 	bft bft.Committer,
 	allowedTracers []string,
-	soloMode bool) *Debug {
-
+	soloMode bool,
+) *Debug {
 	allowedMap := make(map[string]struct{})
 	for _, t := range allowedTracers {
 		allowedMap[t] = struct{}{}

api/debug/debug_test.go

@@ -54,9 +54,9 @@ func TestDebug(t *testing.T) {
 	for name, tt := range map[string]func(*testing.T){
 		"testTraceClauseWithInvalidTracerName":     testTraceClauseWithInvalidTracerName,
 		"testTraceClauseWithEmptyTracerTarget":     testTraceClauseWithEmptyTracerTarget,
-		"testTraceClauseWithBadBlockId":            testTraceClauseWithBadBlockId,
-		"testTraceClauseWithNonExistingBlockId":    testTraceClauseWithNonExistingBlockId,
-		"testTraceClauseWithBadTxId":               testTraceClauseWithBadTxId,
+		"testTraceClauseWithBadBlockID":            testTraceClauseWithBadBlockID,
+		"testTraceClauseWithNonExistingBlockID":    testTraceClauseWithNonExistingBlockID,
+		"testTraceClauseWithBadTxID":               testTraceClauseWithBadTxID,
 		"testTraceClauseWithNonExistingTx":         testTraceClauseWithNonExistingTx,
 		"testTraceClauseWithBadClauseIndex":        testTraceClauseWithBadClauseIndex,
 		"testTraceClauseWithTxIndexOutOfBound":     testTraceClauseWithTxIndexOutOfBound,
@@ -74,7 +74,7 @@ func TestDebug(t *testing.T) {
 		"testHandleTraceCall":                                testHandleTraceCall,
 		"testHandleTraceCallWithValidRevisions":              testHandleTraceCallWithValidRevisions,
 		"testHandleTraceCallWithRevisionAsNonExistingHeight": testHandleTraceCallWithRevisionAsNonExistingHeight,
-		"testHandleTraceCallWithRevisionAsNonExistingId":     testHandleTraceCallWithRevisionAsNonExistingId,
+		"testHandleTraceCallWithRevisionAsNonExistingID":     testHandleTraceCallWithRevisionAsNonExistingID,
 		"testHandleTraceCallWithMalfomredRevision":           testHandleTraceCallWithMalfomredRevision,
 		"testHandleTraceCallWithInsufficientGas":             testHandleTraceCallWithInsufficientGas,
 		"testHandleTraceCallWithBadBlockRef":                 testHandleTraceCallWithBadBlockRef,
@@ -167,7 +167,7 @@ func testTraceClauseWithEmptyTracerTarget(t *testing.T) {
 	assert.Equal(t, "target: unsupported", strings.TrimSpace(res))
 }
 
-func testTraceClauseWithBadBlockId(t *testing.T) {
+func testTraceClauseWithBadBlockID(t *testing.T) {
 	traceClauseOption := &TraceClauseOption{
 		Name:   "structLogger",
 		Target: "badBlockId/x/x",
@@ -176,13 +176,13 @@ func testTraceClauseWithBadBlockId(t *testing.T) {
 	assert.Equal(t, "target[0]: invalid length", strings.TrimSpace(res))
 }
 
-func testTraceClauseWithNonExistingBlockId(t *testing.T) {
+func testTraceClauseWithNonExistingBlockID(t *testing.T) {
 	_, _, _, err := debug.prepareClauseEnv(context.Background(), datagen.RandomHash(), 1, 1)
 
 	assert.Error(t, err)
 }
 
-func testTraceClauseWithBadTxId(t *testing.T) {
+func testTraceClauseWithBadTxID(t *testing.T) {
 	traceClauseOption := &TraceClauseOption{
 		Name:   "structLogger",
 		Target: fmt.Sprintf("%s/badTxId/x", blk.Header().ID()),
@@ -192,10 +192,10 @@ func testTraceClauseWithBadTxId(t *testing.T) {
 }
 
 func testTraceClauseWithNonExistingTx(t *testing.T) {
-	nonExistingTxId := "0x4500ade0d72115abfc77571aef752df45ba5e87ca81fbd67fbfc46d455b17f91"
+	nonExistingTxID := "0x4500ade0d72115abfc77571aef752df45ba5e87ca81fbd67fbfc46d455b17f91"
 	traceClauseOption := &TraceClauseOption{
 		Name:   "structLogger",
-		Target: fmt.Sprintf("%s/%s/x", blk.Header().ID(), nonExistingTxId),
+		Target: fmt.Sprintf("%s/%s/x", blk.Header().ID(), nonExistingTxID),
 	}
 	res := httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers", traceClauseOption, 403)
 	assert.Equal(t, "transaction not found", strings.TrimSpace(res))
@@ -380,7 +380,7 @@ func testHandleTraceCallWithRevisionAsNonExistingHeight(t *testing.T) {
 	assert.Equal(t, "revision: not found", strings.TrimSpace(res))
 }
 
-func testHandleTraceCallWithRevisionAsNonExistingId(t *testing.T) {
+func testHandleTraceCallWithRevisionAsNonExistingID(t *testing.T) {
 	nonExistingRevision := "0x4500ade0d72115abfc77571aef752df45ba5e87ca81fbd67fbfc46d455b17f91"
 
 	res := httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers/call?revision="+nonExistingRevision, &TraceCallOption{}, 400)

api/metrics.go

@@ -19,8 +19,8 @@ import (
 )
 
 var (
-	metricHttpReqCounter       = metrics.LazyLoadCounterVec("api_request_count", []string{"name", "code", "method"})
-	metricHttpReqDuration      = metrics.LazyLoadHistogramVec("api_duration_ms", []string{"name", "code", "method"}, metrics.BucketHTTPReqs)
+	metricHTTPReqCounter       = metrics.LazyLoadCounterVec("api_request_count", []string{"name", "code", "method"})
+	metricHTTPReqDuration      = metrics.LazyLoadHistogramVec("api_duration_ms", []string{"name", "code", "method"}, metrics.BucketHTTPReqs)
 	metricActiveWebsocketCount = metrics.LazyLoadGaugeVec("api_active_websocket_count", []string{"subject"})
 )
 
@@ -89,8 +89,8 @@ func metricsMiddleware(next http.Handler) http.Handler {
 		if subscription != "" {
 			metricActiveWebsocketCount().AddWithLabel(-1, map[string]string{"subject": subscription})
 		} else if enabled {
-			metricHttpReqCounter().AddWithLabel(1, map[string]string{"name": name, "code": strconv.Itoa(mrw.statusCode), "method": r.Method})
-			metricHttpReqDuration().ObserveWithLabels(time.Since(now).Milliseconds(), map[string]string{"name": name, "code": strconv.Itoa(mrw.statusCode), "method": r.Method})
+			metricHTTPReqCounter().AddWithLabel(1, map[string]string{"name": name, "code": strconv.Itoa(mrw.statusCode), "method": r.Method})
+			metricHTTPReqDuration().ObserveWithLabels(time.Since(now).Milliseconds(), map[string]string{"name": name, "code": strconv.Itoa(mrw.statusCode), "method": r.Method})
 		}
 	})
 }

api/node/node.go

@@ -26,7 +26,7 @@ func (n *Node) PeersStats() []*PeerStats {
 	return ConvertPeersStats(n.nw.PeersStats())
 }
 
-func (n *Node) handleNetwork(w http.ResponseWriter, req *http.Request) error {
+func (n *Node) handleNetwork(w http.ResponseWriter, _ *http.Request) error {
 	return utils.WriteJSON(w, n.PeersStats())
 }
 

api/request_logger_test.go

@@ -21,35 +21,35 @@ type mockLogger struct {
 	loggedData []interface{}
 }
 
-func (m *mockLogger) With(ctx ...interface{}) log.Logger {
+func (m *mockLogger) With(_ ...interface{}) log.Logger {
 	return m
 }
 
-func (m *mockLogger) Log(level slog.Level, msg string, ctx ...interface{}) {}
+func (m *mockLogger) Log(_ slog.Level, _ string, _ ...interface{}) {}
 
-func (m *mockLogger) Trace(msg string, ctx ...interface{}) {}
+func (m *mockLogger) Trace(_ string, _ ...interface{}) {}
 
-func (m *mockLogger) Write(level slog.Level, msg string, attrs ...any) {}
+func (m *mockLogger) Write(_ slog.Level, _ string, _ ...any) {}
 
-func (m *mockLogger) Enabled(ctx context.Context, level slog.Level) bool {
+func (m *mockLogger) Enabled(_ context.Context, _ slog.Level) bool {
 	return true
 }
 
 func (m *mockLogger) Handler() slog.Handler { return nil }
 
-func (m *mockLogger) New(ctx ...interface{}) log.Logger { return m }
+func (m *mockLogger) New(_ ...interface{}) log.Logger { return m }
 
-func (m *mockLogger) Debug(msg string, ctx ...interface{}) {}
+func (m *mockLogger) Debug(_ string, _ ...interface{}) {}
 
-func (m *mockLogger) Error(msg string, ctx ...interface{}) {}
+func (m *mockLogger) Error(_ string, _ ...interface{}) {}
 
-func (m *mockLogger) Crit(msg string, ctx ...interface{}) {}
+func (m *mockLogger) Crit(_ string, _ ...interface{}) {}
 
-func (m *mockLogger) Info(msg string, ctx ...interface{}) {
+func (m *mockLogger) Info(_ string, ctx ...interface{}) {
 	m.loggedData = append(m.loggedData, ctx...)
 }
 
-func (m *mockLogger) Warn(msg string, ctx ...interface{}) {
+func (m *mockLogger) Warn(_ string, ctx ...interface{}) {
 	m.loggedData = append(m.loggedData, ctx...)
 }
 
@@ -61,7 +61,7 @@ func TestRequestLoggerHandler(t *testing.T) {
 	mockLog := &mockLogger{}
 
 	// Define a test handler to wrap
-	testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	testHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		w.WriteHeader(http.StatusOK)
 		w.Write([]byte("OK"))
 	})

api/subscriptions/subscriptions.go

@@ -80,7 +80,7 @@ func New(repo *chain.Repository, allowedOrigins []string, backtraceLimit uint32,
 	return sub
 }
 
-func (s *Subscriptions) handleBlockReader(w http.ResponseWriter, req *http.Request) (*blockReader, error) {
+func (s *Subscriptions) handleBlockReader(_ http.ResponseWriter, req *http.Request) (*blockReader, error) {
 	position, err := s.parsePosition(req.URL.Query().Get("pos"))
 	if err != nil {
 		return nil, err
@@ -128,7 +128,7 @@ func (s *Subscriptions) handleEventReader(w http.ResponseWriter, req *http.Reque
 	return newEventReader(s.repo, position, eventFilter), nil
 }
 
-func (s *Subscriptions) handleTransferReader(w http.ResponseWriter, req *http.Request) (*transferReader, error) {
+func (s *Subscriptions) handleTransferReader(_ http.ResponseWriter, req *http.Request) (*transferReader, error) {
 	position, err := s.parsePosition(req.URL.Query().Get("pos"))
 	if err != nil {
 		return nil, err

api/subscriptions/types.go

@@ -43,9 +43,9 @@ func convertBlock(b *chain.ExtendedBlock) (*BlockMessage, error) {
 	}
 
 	txs := b.Transactions()
-	txIds := make([]thor.Bytes32, len(txs))
+	txIDs := make([]thor.Bytes32, len(txs))
 	for i, tx := range txs {
-		txIds[i] = tx.ID()
+		txIDs[i] = tx.ID()
 	}
 	return &BlockMessage{
 		Number:       header.Number(),
@@ -63,7 +63,7 @@ func convertBlock(b *chain.ExtendedBlock) (*BlockMessage, error) {
 		TxsRoot:      header.TxsRoot(),
 		TxsFeatures:  uint32(header.TxsFeatures()),
 		COM:          header.COM(),
-		Transactions: txIds,
+		Transactions: txIDs,
 		Obsolete:     b.Obsolete,
 	}, nil
 }

api/transactions/transactions_test.go

@@ -54,7 +54,7 @@ func TestTransaction(t *testing.T) {
 	// Get tx
 	for name, tt := range map[string]func(*testing.T){
 		"getTx":           getTx,
-		"getTxWithBadId":  getTxWithBadId,
+		"getTxWithBadID":  getTxWithBadID,
 		"txWithBadHeader": txWithBadHeader,
 		"getNonExistingRawTransactionWhenTxStillInMempool": getNonExistingRawTransactionWhenTxStillInMempool,
 		"getNonPendingRawTransactionWhenTxStillInMempool":  getNonPendingRawTransactionWhenTxStillInMempool,
@@ -70,7 +70,7 @@ func TestTransaction(t *testing.T) {
 	// Get tx receipt
 	for name, tt := range map[string]func(*testing.T){
 		"getTxReceipt":        getTxReceipt,
-		"getReceiptWithBadId": getReceiptWithBadId,
+		"getReceiptWithBadID": getReceiptWithBadID,
 		"handleGetTransactionReceiptByIDWithNonExistingHead": handleGetTransactionReceiptByIDWithNonExistingHead,
 	} {
 		t.Run(name, tt)
@@ -136,10 +136,10 @@ func sendTx(t *testing.T) {
 	assert.Equal(t, tx.ID().String(), txObj["id"], "should be the same transaction id")
 }
 
-func getTxWithBadId(t *testing.T) {
-	txBadId := "0x123"
+func getTxWithBadID(t *testing.T) {
+	txBadID := "0x123"
 
-	res := httpGetAndCheckResponseStatus(t, ts.URL+"/transactions/"+txBadId, 400)
+	res := httpGetAndCheckResponseStatus(t, ts.URL+"/transactions/"+txBadID, 400)
 
 	assert.Contains(t, string(res), "invalid length")
 }
@@ -156,21 +156,21 @@ func txWithBadHeader(t *testing.T) {
 	}
 }
 
-func getReceiptWithBadId(t *testing.T) {
-	txBadId := "0x123"
+func getReceiptWithBadID(t *testing.T) {
+	txBadID := "0x123"
 
-	httpGetAndCheckResponseStatus(t, ts.URL+"/transactions/"+txBadId+"/receipt", 400)
+	httpGetAndCheckResponseStatus(t, ts.URL+"/transactions/"+txBadID+"/receipt", 400)
 }
 
 func getNonExistingRawTransactionWhenTxStillInMempool(t *testing.T) {
-	nonExistingTxId := "0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"
+	nonExistingTxID := "0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"
 	queryParams := []string{
 		"?raw=true",
 		"?raw=true&pending=true",
 	}
 
 	for _, queryParam := range queryParams {
-		res := httpGetAndCheckResponseStatus(t, ts.URL+"/transactions/"+nonExistingTxId+queryParam, 200)
+		res := httpGetAndCheckResponseStatus(t, ts.URL+"/transactions/"+nonExistingTxID+queryParam, 200)
 
 		assert.Equal(t, "null\n", string(res))
 	}

api/utils/http_test.go

@@ -19,7 +19,7 @@ import (
 )
 
 func TestWrapHandlerFunc(t *testing.T) {
-	handlerFunc := func(w http.ResponseWriter, r *http.Request) error {
+	handlerFunc := func(_ http.ResponseWriter, r *http.Request) error {
 		return nil
 	}
 	wrapped := utils.WrapHandlerFunc(handlerFunc)
@@ -32,7 +32,7 @@ func TestWrapHandlerFunc(t *testing.T) {
 
 func TestWrapHandlerFuncWithGenericError(t *testing.T) {
 	genericErrorMsg := "This is a generic error request"
-	handlerFunc := func(w http.ResponseWriter, r *http.Request) error {
+	handlerFunc := func(_ http.ResponseWriter, r *http.Request) error {
 		return errors.New(genericErrorMsg)
 	}
 	wrapped := utils.WrapHandlerFunc(handlerFunc)
@@ -45,7 +45,7 @@ func TestWrapHandlerFuncWithGenericError(t *testing.T) {
 
 func TestWrapHandlerFuncWithBadRequestError(t *testing.T) {
 	badMsg := "This is a bad request"
-	handlerFunc := func(w http.ResponseWriter, r *http.Request) error {
+	handlerFunc := func(_ http.ResponseWriter, r *http.Request) error {
 		return utils.BadRequest(errors.New(badMsg))
 	}
 	wrapped := utils.WrapHandlerFunc(handlerFunc)
@@ -92,13 +92,13 @@ func callWrappedFunc(wrapped *http.HandlerFunc) *httptest.ResponseRecorder {
 }
 
 type mockReader struct {
-	Id   int
+	ID   int
 	Body string
 }
 
 func TestParseJSON(t *testing.T) {
 	var parsedRes mockReader
-	body := mockReader{Id: 1, Body: "test"}
+	body := mockReader{ID: 1, Body: "test"}
 	jsonBody, _ := json.Marshal(body)
 	req := httptest.NewRequest("GET", "http://example.com", bytes.NewReader(jsonBody))
 
@@ -118,10 +118,10 @@ func TestWriteJSON(t *testing.T) {
 	assert.Equal(t, http.StatusOK, rr.Code)
 	assert.Equal(t, utils.JSONContentType, rr.Header().Get("Content-Type"))
 
-	respObj := mockReader{Id: 1, Body: "test"}
+	respObj := mockReader{ID: 1, Body: "test"}
 	err = json.NewDecoder(rr.Body).Decode(&respObj)
 
 	assert.NoError(t, err)
-	assert.Equal(t, body.Id, respObj.Id)
+	assert.Equal(t, body.ID, respObj.ID)
 	assert.Equal(t, body.Body, respObj.Body)
 }