Skip to content

Commit

Permalink
Upgrade to go 1.24 and fix lint errors (#681)
Browse files Browse the repository at this point in the history
* Remove deprecated linters

* Use new range in benchmark loops

* Add timestamp overflow check for validator registrations

* Define gasLimit as a uint64

* Replace "golang.org/x/exp/slices" with stdlib "slices"

* Use require.JSONEq

* Add a bunch of //nolint:gosec comments for timestamp conversions

* Disable deprecated tenv linter

* Define new errTimestampOverflow var

* Bump golang to v1.24

* Run go mod tidy

* Replace context.Background() with t.Context() in tests
  • Loading branch information
jtraglia authored Feb 19, 2025
1 parent 13160be commit 3a0417e
Show file tree
Hide file tree
Showing 15 changed files with 82 additions and 83 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
- name: Set up Go
uses: actions/setup-go@v3
with:
go-version: ^1.22
go-version: ^1.24
id: go

- name: Check out code into the Go module directory
Expand All @@ -40,7 +40,7 @@ jobs:
- name: Set up Go
uses: actions/setup-go@v3
with:
go-version: ^1.22
go-version: ^1.24
id: go

- name: Check out code into the Go module directory
Expand Down
7 changes: 1 addition & 6 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ linters:
- gocritic
- godot
- godox
- gomnd
- lll
- musttag
- mnd
Expand All @@ -29,6 +28,7 @@ linters:
- wsl
- interfacebloat
- exhaustruct
- tenv

#
# Disabled because of generics:
Expand All @@ -38,11 +38,6 @@ linters:
- sqlclosecheck
- wastedassign

#
# Disabled because deprecated:
#
- execinquery

linters-settings:
#
# The G108 rule throws a false positive. We're not actually vulnerable. If
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# syntax=docker/dockerfile:1
FROM golang:1.22 as builder
FROM golang:1.24 as builder
ARG VERSION
WORKDIR /build

Expand Down
12 changes: 6 additions & 6 deletions common/ssz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func TestSSZBuilderSubmission(t *testing.T) {
buffer := new(bytes.Buffer)
err = json.Compact(buffer, jsonBytes)
require.NoError(t, err)
require.Equal(t, buffer.Bytes(), marshalledJSONBytes)
require.JSONEq(t, buffer.String(), string(marshalledJSONBytes))
})
}
}
Expand Down Expand Up @@ -146,7 +146,7 @@ func TestSSZGetHeaderResponse(t *testing.T) {
buffer := new(bytes.Buffer)
err = json.Compact(buffer, jsonBytes)
require.NoError(t, err)
require.Equal(t, buffer.Bytes(), marshalledJSONBytes)
require.JSONEq(t, buffer.String(), string(marshalledJSONBytes))
})
}
}
Expand All @@ -160,14 +160,14 @@ func BenchmarkDecoding(b *testing.B) {

payload := new(builderSpec.VersionedSignedBuilderBid)
b.Run("capella json", func(b *testing.B) {
for i := 0; i < b.N; i++ {
for range b.N {
err = json.Unmarshal(jsonBytes, &payload)
require.NoError(b, err)
}
})
payload.Capella = new(builderApiCapella.SignedBuilderBid)
b.Run("capella ssz", func(b *testing.B) {
for i := 0; i < b.N; i++ {
for range b.N {
err = payload.Capella.UnmarshalSSZ(sszBytes)
require.NoError(b, err)
}
Expand All @@ -180,14 +180,14 @@ func BenchmarkDecoding(b *testing.B) {
require.NoError(b, err)
payload = new(builderSpec.VersionedSignedBuilderBid)
b.Run("deneb json", func(b *testing.B) {
for i := 0; i < b.N; i++ {
for range b.N {
err = json.Unmarshal(jsonBytes, &payload)
require.NoError(b, err)
}
})
payload.Deneb = new(builderApiDeneb.SignedBuilderBid)
b.Run("deneb ssz", func(b *testing.B) {
for i := 0; i < b.N; i++ {
for range b.N {
err = payload.Deneb.UnmarshalSSZ(sszBytes)
require.NoError(b, err)
}
Expand Down
6 changes: 3 additions & 3 deletions common/types_spec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func TestSubmitBuilderBlockJSON(t *testing.T) {
require.NoError(t, err)
expectedJSONBytes := buffer.Bytes()

require.Equal(t, expectedJSONBytes, marshalledJSONBytes)
require.JSONEq(t, string(expectedJSONBytes), string(marshalledJSONBytes))
}

func TestSignedBeaconBlockJSON(t *testing.T) {
Expand Down Expand Up @@ -56,7 +56,7 @@ func TestSignedBeaconBlockJSON(t *testing.T) {
marshalledJSONBytes, err := json.Marshal(blockRequest)
require.NoError(t, err)

require.Equal(t, expectedJSONBytes, marshalledJSONBytes)
require.JSONEq(t, string(expectedJSONBytes), string(marshalledJSONBytes))
})
}
}
Expand Down Expand Up @@ -91,7 +91,7 @@ func TestSignedBlindedBlockJSON(t *testing.T) {
marshalledJSONBytes, err := json.Marshal(blockRequest)
require.NoError(t, err)

require.Equal(t, expectedJSONBytes, marshalledJSONBytes)
require.JSONEq(t, string(expectedJSONBytes), string(marshalledJSONBytes))
})
}
}
Expand Down
3 changes: 1 addition & 2 deletions common/utils_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package common

import (
"context"
"fmt"
"net/http"
"os"
Expand All @@ -22,7 +21,7 @@ import (
func TestMakePostRequest(t *testing.T) {
// Test errors
var x chan bool
resp, err := makeRequest(context.Background(), *http.DefaultClient, http.MethodGet, "", x)
resp, err := makeRequest(t.Context(), *http.DefaultClient, http.MethodGet, "", x)
require.Error(t, err)
require.Nil(t, resp)

Expand Down
10 changes: 9 additions & 1 deletion database/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@ package database

import (
"database/sql"
"errors"
"math"
"strconv"
"time"

builderApiV1 "github.com/attestantio/go-builder-client/api/v1"
"github.com/flashbots/go-boost-utils/utils"
)

var errTimestampOverflow = errors.New("timestamp overflow")

func NewNullInt64(i int64) sql.NullInt64 {
return sql.NullInt64{
Int64: i,
Expand Down Expand Up @@ -78,6 +82,10 @@ func (reg ValidatorRegistrationEntry) ToSignedValidatorRegistration() (*builderA
return nil, err
}

if reg.Timestamp > uint64(math.MaxInt64) {
return nil, errTimestampOverflow
}

return &builderApiV1.SignedValidatorRegistration{
Message: &builderApiV1.ValidatorRegistration{
Pubkey: pubkey,
Expand All @@ -93,7 +101,7 @@ func SignedValidatorRegistrationToEntry(valReg builderApiV1.SignedValidatorRegis
return ValidatorRegistrationEntry{
Pubkey: valReg.Message.Pubkey.String(),
FeeRecipient: valReg.Message.FeeRecipient.String(),
Timestamp: uint64(valReg.Message.Timestamp.Unix()),
Timestamp: uint64(valReg.Message.Timestamp.Unix()), //nolint:gosec
GasLimit: valReg.Message.GasLimit,
Signature: valReg.Signature.String(),
}
Expand Down
2 changes: 1 addition & 1 deletion datastore/datastore.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func (ds *Datastore) SaveValidatorRegistration(entry builderApiV1.SignedValidato

// then save in redis
pk := common.NewPubkeyHex(entry.Message.Pubkey.String())
err = ds.redis.SetValidatorRegistrationTimestampIfNewer(pk, uint64(entry.Message.Timestamp.Unix()))
err = ds.redis.SetValidatorRegistrationTimestampIfNewer(pk, uint64(entry.Message.Timestamp.Unix())) //nolint:gosec
if err != nil {
return errors.Wrap(err, "failed saving validator registration to redis")
}
Expand Down
48 changes: 24 additions & 24 deletions datastore/redis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ func TestRedisValidatorRegistration(t *testing.T) {
key := common.ValidPayloadRegisterValidator.Message.Pubkey
value := common.ValidPayloadRegisterValidator
pkHex := common.NewPubkeyHex(key.String())
err := cache.SetValidatorRegistrationTimestamp(pkHex, uint64(value.Message.Timestamp.Unix()))
err := cache.SetValidatorRegistrationTimestamp(pkHex, uint64(value.Message.Timestamp.Unix())) //nolint:gosec
require.NoError(t, err)
result, err := cache.GetValidatorRegistrationTimestamp(common.NewPubkeyHex(key.String()))
require.NoError(t, err)
require.Equal(t, result, uint64(value.Message.Timestamp.Unix()))
require.Equal(t, result, uint64(value.Message.Timestamp.Unix())) //nolint:gosec
})

t.Run("Returns nil if validator registration is not in cache", func(t *testing.T) {
Expand All @@ -60,7 +60,7 @@ func TestRedisValidatorRegistration(t *testing.T) {
value := common.ValidPayloadRegisterValidator

pkHex := common.NewPubkeyHex(key.String())
timestamp := uint64(value.Message.Timestamp.Unix())
timestamp := uint64(value.Message.Timestamp.Unix()) //nolint:gosec

err := cache.SetValidatorRegistrationTimestampIfNewer(pkHex, timestamp)
require.NoError(t, err)
Expand Down Expand Up @@ -156,7 +156,7 @@ func TestBuilderBids(t *testing.T) {
require.NoError(t, err)
require.Equal(t, big.NewInt(expectedValue), value.ToBig())

topBidValue, err := cache.GetTopBidValue(context.Background(), cache.client.Pipeline(), slot, parentHash, proposerPubkey)
topBidValue, err := cache.GetTopBidValue(t.Context(), cache.client.Pipeline(), slot, parentHash, proposerPubkey)
require.NoError(t, err)
require.Equal(t, big.NewInt(expectedValue), topBidValue)

Expand All @@ -168,18 +168,18 @@ func TestBuilderBids(t *testing.T) {
}

ensureBidFloor := func(expectedValue int64) {
floorValue, err := cache.GetFloorBidValue(context.Background(), cache.client.Pipeline(), slot, parentHash, proposerPubkey)
floorValue, err := cache.GetFloorBidValue(t.Context(), cache.client.Pipeline(), slot, parentHash, proposerPubkey)
require.NoError(t, err)
require.Equal(t, big.NewInt(expectedValue), floorValue)
}

// deleting a bid that doesn't exist should not error
err := cache.DelBuilderBid(context.Background(), cache.client.Pipeline(), slot, parentHash, proposerPubkey, bApubkey)
err := cache.DelBuilderBid(t.Context(), cache.client.Pipeline(), slot, parentHash, proposerPubkey, bApubkey)
require.NoError(t, err)

// submit ba1=10
payload, getPayloadResp, getHeaderResp := common.CreateTestBlockSubmission(t, bApubkey, uint256.NewInt(10), &opts)
resp, err := cache.SaveBidAndUpdateTopBid(context.Background(), cache.NewPipeline(), trace, payload, getPayloadResp, getHeaderResp, time.Now(), false, nil)
resp, err := cache.SaveBidAndUpdateTopBid(t.Context(), cache.NewPipeline(), trace, payload, getPayloadResp, getHeaderResp, time.Now(), false, nil)
require.NoError(t, err)
require.True(t, resp.WasBidSaved, resp)
require.True(t, resp.WasTopBidUpdated)
Expand All @@ -189,7 +189,7 @@ func TestBuilderBids(t *testing.T) {
ensureBidFloor(10)

// deleting ba1
err = cache.DelBuilderBid(context.Background(), cache.client.Pipeline(), slot, parentHash, proposerPubkey, bApubkey)
err = cache.DelBuilderBid(t.Context(), cache.client.Pipeline(), slot, parentHash, proposerPubkey, bApubkey)
require.NoError(t, err)

// best bid and floor should still exist, because it was the floor bid
Expand All @@ -198,7 +198,7 @@ func TestBuilderBids(t *testing.T) {

// submit ba2=5 (should not update, because floor is 10)
payload, getPayloadResp, getHeaderResp = common.CreateTestBlockSubmission(t, bApubkey, uint256.NewInt(5), &opts)
resp, err = cache.SaveBidAndUpdateTopBid(context.Background(), cache.NewPipeline(), trace, payload, getPayloadResp, getHeaderResp, time.Now(), false, nil)
resp, err = cache.SaveBidAndUpdateTopBid(t.Context(), cache.NewPipeline(), trace, payload, getPayloadResp, getHeaderResp, time.Now(), false, nil)
require.NoError(t, err)
require.False(t, resp.WasBidSaved, resp)
require.False(t, resp.WasTopBidUpdated)
Expand All @@ -209,7 +209,7 @@ func TestBuilderBids(t *testing.T) {

// submit ba3c=5 (should not update, because floor is 10)
payload, getPayloadResp, getHeaderResp = common.CreateTestBlockSubmission(t, bApubkey, uint256.NewInt(5), &opts)
resp, err = cache.SaveBidAndUpdateTopBid(context.Background(), cache.NewPipeline(), trace, payload, getPayloadResp, getHeaderResp, time.Now(), true, nil)
resp, err = cache.SaveBidAndUpdateTopBid(t.Context(), cache.NewPipeline(), trace, payload, getPayloadResp, getHeaderResp, time.Now(), true, nil)
require.NoError(t, err)
require.True(t, resp.WasBidSaved)
require.False(t, resp.WasTopBidUpdated)
Expand All @@ -221,7 +221,7 @@ func TestBuilderBids(t *testing.T) {

// submit bb1=20
payload, getPayloadResp, getHeaderResp = common.CreateTestBlockSubmission(t, bBpubkey, uint256.NewInt(20), &opts)
resp, err = cache.SaveBidAndUpdateTopBid(context.Background(), cache.NewPipeline(), trace, payload, getPayloadResp, getHeaderResp, time.Now(), false, nil)
resp, err = cache.SaveBidAndUpdateTopBid(t.Context(), cache.NewPipeline(), trace, payload, getPayloadResp, getHeaderResp, time.Now(), false, nil)
require.NoError(t, err)
require.True(t, resp.WasBidSaved)
require.True(t, resp.WasTopBidUpdated)
Expand All @@ -232,7 +232,7 @@ func TestBuilderBids(t *testing.T) {

// submit bb2c=22
payload, getPayloadResp, getHeaderResp = common.CreateTestBlockSubmission(t, bBpubkey, uint256.NewInt(22), &opts)
resp, err = cache.SaveBidAndUpdateTopBid(context.Background(), cache.NewPipeline(), trace, payload, getPayloadResp, getHeaderResp, time.Now(), true, nil)
resp, err = cache.SaveBidAndUpdateTopBid(t.Context(), cache.NewPipeline(), trace, payload, getPayloadResp, getHeaderResp, time.Now(), true, nil)
require.NoError(t, err)
require.True(t, resp.WasBidSaved)
require.True(t, resp.WasTopBidUpdated)
Expand All @@ -243,7 +243,7 @@ func TestBuilderBids(t *testing.T) {

// submit bb3c=12 (should update top bid, using floor at 20)
payload, getPayloadResp, getHeaderResp = common.CreateTestBlockSubmission(t, bBpubkey, uint256.NewInt(12), &opts)
resp, err = cache.SaveBidAndUpdateTopBid(context.Background(), cache.NewPipeline(), trace, payload, getPayloadResp, getHeaderResp, time.Now(), true, nil)
resp, err = cache.SaveBidAndUpdateTopBid(t.Context(), cache.NewPipeline(), trace, payload, getPayloadResp, getHeaderResp, time.Now(), true, nil)
require.NoError(t, err)
require.True(t, resp.WasBidSaved)
require.True(t, resp.WasTopBidUpdated)
Expand Down Expand Up @@ -290,7 +290,7 @@ func TestCheckAndSetLastSlotAndHashDelivered(t *testing.T) {
newHash := "0x0000000000000000000000000000000000000000000000000000000000000000"

// should return redis.Nil if wasn't set
slot, err := cache.GetLastSlotDelivered(context.Background(), cache.NewPipeline())
slot, err := cache.GetLastSlotDelivered(t.Context(), cache.NewPipeline())
require.ErrorIs(t, err, redis.Nil)
require.Equal(t, uint64(0), slot)

Expand All @@ -299,7 +299,7 @@ func TestCheckAndSetLastSlotAndHashDelivered(t *testing.T) {
require.NoError(t, err)

// should get slot
slot, err = cache.GetLastSlotDelivered(context.Background(), cache.NewPipeline())
slot, err = cache.GetLastSlotDelivered(t.Context(), cache.NewPipeline())
require.NoError(t, err)
require.Equal(t, newSlot, slot)

Expand Down Expand Up @@ -426,8 +426,8 @@ func TestGetBuilderLatestValue(t *testing.T) {
},
}

_, err = cache.client.TxPipelined(context.Background(), func(pipeliner redis.Pipeliner) error {
return cache.SaveBuilderBid(context.Background(), pipeliner, slot, parentHash, proposerPubkey, builderPubkey, time.Now().UTC(), getHeaderResp)
_, err = cache.client.TxPipelined(t.Context(), func(pipeliner redis.Pipeliner) error {
return cache.SaveBuilderBid(t.Context(), pipeliner, slot, parentHash, proposerPubkey, builderPubkey, time.Now().UTC(), getHeaderResp)
})
require.NoError(t, err)

Expand All @@ -439,7 +439,7 @@ func TestGetBuilderLatestValue(t *testing.T) {

func TestPipelineNilCheck(t *testing.T) {
cache := setupTestRedis(t)
f, err := cache.GetFloorBidValue(context.Background(), cache.NewPipeline(), 0, "1", "2")
f, err := cache.GetFloorBidValue(t.Context(), cache.NewPipeline(), 0, "1", "2")
require.NoError(t, err)
require.Equal(t, big.NewInt(0), f)
}
Expand All @@ -450,24 +450,24 @@ func TestPipelineNilCheck(t *testing.T) {
// key1 := "test1"
// key2 := "test123"
// val := "foo"
// err := cache.client.Set(context.Background(), key1, val, 0).Err()
// err := cache.client.Set(t.Context(), key1, val, 0).Err()
// require.NoError(t, err)

// _, err = cache.client.TxPipelined(context.Background(), func(pipeliner redis.Pipeliner) error {
// c := tx.Get(context.Background(), key1)
// _, err := tx.Exec(context.Background())
// _, err = cache.client.TxPipelined(t.Context(), func(pipeliner redis.Pipeliner) error {
// c := tx.Get(t.Context(), key1)
// _, err := tx.Exec(t.Context())
// require.NoError(t, err)
// str, err := c.Result()
// require.NoError(t, err)
// require.Equal(t, val, str)

// err = tx.Set(context.Background(), key2, val, 0).Err()
// err = tx.Set(t.Context(), key2, val, 0).Err()
// require.NoError(t, err)
// return nil
// })
// require.NoError(t, err)

// str, err := cache.client.Get(context.Background(), key2).Result()
// str, err := cache.client.Get(t.Context(), key2).Result()
// require.NoError(t, err)
// require.Equal(t, val, str)
// }
3 changes: 1 addition & 2 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/flashbots/mev-boost-relay

go 1.22.0
go 1.24.0

require (
github.com/NYTimes/gziphandler v1.1.1
Expand Down Expand Up @@ -31,7 +31,6 @@ require (
go.opentelemetry.io/otel/sdk v1.25.0
go.opentelemetry.io/otel/sdk/metric v1.25.0
go.uber.org/atomic v1.11.0
golang.org/x/exp v0.0.0-20231110203233-9a3e6036ecaa
golang.org/x/text v0.21.0
)

Expand Down
Loading

0 comments on commit 3a0417e

Please sign in to comment.