From a72c21a2a91ac5c631e1fdb08daa139080b2998b Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 12 Sep 2024 12:01:19 +0100 Subject: [PATCH 01/12] chore: `golangci-lint` CI workflow --- .github/workflows/golangci-lint.yml | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 .github/workflows/golangci-lint.yml diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml new file mode 100644 index 00000000000..123936378de --- /dev/null +++ b/.github/workflows/golangci-lint.yml @@ -0,0 +1,26 @@ +name: golangci-lint + +on: + push: + branches: [ libevm ] + pull_request: + branches: [ libevm ] + workflow_dispatch: + +permissions: + contents: read + pull-requests: read + +jobs: + golangci: + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: stable + - name: golangci-lint + uses: golangci/golangci-lint-action@v6 + with: + version: v1.60 From b3f9e06584440dc84f21c6326a22f88591ca0dde Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 12 Sep 2024 12:01:48 +0100 Subject: [PATCH 02/12] fix: make `golangci-lint` happy --- libevm/hookstest/stub.go | 2 +- libevm/pseudo/type_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libevm/hookstest/stub.go b/libevm/hookstest/stub.go index 2915d487453..3363576abf4 100644 --- a/libevm/hookstest/stub.go +++ b/libevm/hookstest/stub.go @@ -12,7 +12,7 @@ import ( ) // Register clears any registered [params.Extras] and then registers `extras` -// for the liftime of the current test, clearing them via tb's +// for the lifetime of the current test, clearing them via tb's // [testing.TB.Cleanup]. func Register[C params.ChainConfigHooks, R params.RulesHooks](tb testing.TB, extras params.Extras[C, R]) { params.TestOnlyClearRegisteredExtras() diff --git a/libevm/pseudo/type_test.go b/libevm/pseudo/type_test.go index 0b25c945ce2..a640f089c1a 100644 --- a/libevm/pseudo/type_test.go +++ b/libevm/pseudo/type_test.go @@ -67,6 +67,7 @@ func testType[T any](t *testing.T, name string, ctor func() *Pseudo[T], init T, }) } +//nolint:ineffassign // Although `typ` is overwritten it's to demonstrate different approaches. func ExamplePseudo_TypeAndValue() { typ, val := From("hello").TypeAndValue() @@ -98,5 +99,4 @@ func TestPointer(t *testing.T) { ptr.payload = 314159 assert.Equal(t, 314159, val.Get().payload, "after setting via pointer") }) - } From 09b10e1b85356618b1c396add006dcc560007a10 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 12 Sep 2024 12:02:40 +0100 Subject: [PATCH 03/12] chore: bump `actions/{checkout,setup-go}` versions --- .github/workflows/go.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 62517d0699a..ee315c0612a 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -11,9 +11,9 @@ jobs: go_test_short: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Set up Go - uses: actions/setup-go@v2 + uses: actions/setup-go@v5 with: go-version: 1.21.4 - name: Run tests From 303154854e80abb1ef21a0d5a0ac47a70c5119d8 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 12 Sep 2024 13:38:23 +0100 Subject: [PATCH 04/12] chore: overhaul `.golanci.yml` config --- .golangci.yml | 116 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 79 insertions(+), 37 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 0343c4b4ebf..c6cf1715e21 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -3,58 +3,100 @@ run: timeout: 20m tests: true - # default is true. Enables skipping of directories: - # vendor$, third_party$, testdata$, examples$, Godeps$, builtin$ - skip-dirs-use-default: true - skip-files: - - core/genesis_alloc.go + skip-dirs-use-default: false linters: - disable-all: true enable: + # Every available linter at the time of writing was considered (quickly) and + # inclusion was liberal. Linters are good at detecting code smells, but if + # we find that a particular one causes too many false positives then we can + # first configure it better or, as a last resort, remove it.. + - containedctx + - errcheck + - forcetypeassert + # TODO(arr4n) add and configure gci + - gocheckcompilerdirectives + - gofmt - goimports - - gosimple + - gomodguard + - gosec - govet - ineffassign + # TODO(arr4n): investigate ireturn - misspell + - nakedret + - nestif + - nilerr + - nolintlint + - reassign + - revive + - sloglint + - staticcheck + - tagliatelle + - testableexamples + - testifylint + - thelper + - tparallel - unconvert - - typecheck + - usestdlibvars - unused - - staticcheck - - bidichk - - durationcheck - - exportloopref - whitespace - # - structcheck # lots of false positives - # - errcheck #lot of false positives - # - contextcheck - # - errchkjson # lots of false positives - # - errorlint # this check crashes - # - exhaustive # silly check - # - makezero # false positives - # - nilerr # several intentional - linters-settings: gofmt: simplify: true + revive: + rules: + - name: unused-parameter + # Method parameters may be equired by interfaces and forcing them to be + # named _ is of questionable benefit. + disabled: true + gomodguard: + blocked: + modules: + - github.com/ava-labs/avalanchego: + - github.com/ava-labs/coreth: + - github.com/ava-labs/subnet-evm: issues: - exclude-rules: - - path: crypto/bn256/cloudflare/optate.go + exclude-rules: + - path-except: libevm linters: - - deadcode + # If any issue is flagged in a non-libevm file, add the linter here + # because the problem isn't under our control. + - containedctx + - forcetypeassert + - errcheck + - gofmt + - gosec + - nakedret + - nestif + - nilerr + - nolintlint + - revive - staticcheck - - path: internal/build/pgp.go - text: 'SA1019: "golang.org/x/crypto/openpgp" is deprecated: this package is unmaintained except for security fixes.' - - path: core/vm/contracts.go - text: 'SA1019: "golang.org/x/crypto/ripemd160" is deprecated: RIPEMD-160 is a legacy hash and should not be used for new applications.' - - path: accounts/usbwallet/trezor.go - text: 'SA1019: "github.com/golang/protobuf/proto" is deprecated: Use the "google.golang.org/protobuf/proto" package instead.' - - path: accounts/usbwallet/trezor/ - text: 'SA1019: "github.com/golang/protobuf/proto" is deprecated: Use the "google.golang.org/protobuf/proto" package instead.' - exclude: - - 'SA1019: event.TypeMux is deprecated: use Feed' - - 'SA1019: strings.Title is deprecated' - - 'SA1019: strings.Title has been deprecated since Go 1.18 and an alternative has been available since Go 1.0: The rule Title uses for word boundaries does not handle Unicode punctuation properly. Use golang.org/x/text/cases instead.' - - 'SA1029: should not use built-in type string as key for value' + - tagliatelle + - testableexamples + - testifylint + - thelper + - tparallel + - usestdlibvars + - varnamelen + - wastedassign + - whitespace + include: + # Many of the default exclusions are because, verbatim "Annoying issue", + # which defeats the point of a linter. + - EXC0002 + - EXC0004 + - EXC0005 + - EXC0006 + - EXC0007 + - EXC0008 + - EXC0009 + - EXC0010 + - EXC0011 + - EXC0012 + - EXC0013 + - EXC0014 + - EXC0015 From 5979f36f473b4a8ef1d3fc4093831bba6544f459 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 12 Sep 2024 14:15:53 +0100 Subject: [PATCH 05/12] fix: all linter issues --- libevm/ethtest/rand.go | 6 +++--- libevm/hookstest/stub.go | 9 +++++++++ libevm/pseudo/constructor_test.go | 1 + libevm/pseudo/type.go | 4 ++-- libevm/pseudo/type_test.go | 3 ++- params/config.libevm.go | 2 +- params/config.libevm_test.go | 6 +++--- params/example.libevm_test.go | 2 +- 8 files changed, 22 insertions(+), 11 deletions(-) diff --git a/libevm/ethtest/rand.go b/libevm/ethtest/rand.go index dfabcfedca1..b8c88fe5314 100644 --- a/libevm/ethtest/rand.go +++ b/libevm/ethtest/rand.go @@ -17,7 +17,7 @@ func NewPseudoRand(seed uint64) *PseudoRand { // Address returns a pseudorandom address. func (r *PseudoRand) Address() (a common.Address) { - r.Read(a[:]) + r.Read(a[:]) //nolint:gosec,errcheck // Guaranteed nil error return a } @@ -29,13 +29,13 @@ func (r *PseudoRand) AddressPtr() *common.Address { // Hash returns a pseudorandom hash. func (r *PseudoRand) Hash() (h common.Hash) { - r.Read(h[:]) + r.Read(h[:]) //nolint:gosec,errcheck // Guaranteed nil error return h } // Bytes returns `n` pseudorandom bytes. func (r *PseudoRand) Bytes(n uint) []byte { b := make([]byte, n) - r.Read(b) + r.Read(b) //nolint:gosec,errcheck // Guaranteed nil error return b } diff --git a/libevm/hookstest/stub.go b/libevm/hookstest/stub.go index 3363576abf4..719a16664f9 100644 --- a/libevm/hookstest/stub.go +++ b/libevm/hookstest/stub.go @@ -15,6 +15,7 @@ import ( // for the lifetime of the current test, clearing them via tb's // [testing.TB.Cleanup]. func Register[C params.ChainConfigHooks, R params.RulesHooks](tb testing.TB, extras params.Extras[C, R]) { + tb.Helper() params.TestOnlyClearRegisteredExtras() tb.Cleanup(params.TestOnlyClearRegisteredExtras) params.RegisterExtras(extras) @@ -32,6 +33,7 @@ type Stub struct { // Register is a convenience wrapper for registering s as both the // [params.ChainConfigHooks] and [params.RulesHooks] via [Register]. func (s *Stub) Register(tb testing.TB) { + tb.Helper() Register(tb, params.Extras[*Stub, *Stub]{ NewRules: func(_ *params.ChainConfig, _ *params.Rules, _ *Stub, blockNum *big.Int, isMerge bool, timestamp uint64) *Stub { return s @@ -39,6 +41,9 @@ func (s *Stub) Register(tb testing.TB) { }) } +// PrecompileOverride uses the s.PrecompileOverrides map, if non-empty, as the +// canonical source of all overrides. If the map is empty then no precompiles +// are overridden. func (s Stub) PrecompileOverride(a common.Address) (libevm.PrecompiledContract, bool) { if len(s.PrecompileOverrides) == 0 { return nil, false @@ -47,6 +52,8 @@ func (s Stub) PrecompileOverride(a common.Address) (libevm.PrecompiledContract, return p, ok } +// CanExecuteTransaction proxies arguments to the s.CanExecuteTransactionFn +// function if non-nil, otherwise it acts as a noop. func (s Stub) CanExecuteTransaction(from common.Address, to *common.Address, sr libevm.StateReader) error { if f := s.CanExecuteTransactionFn; f != nil { return f(from, to, sr) @@ -54,6 +61,8 @@ func (s Stub) CanExecuteTransaction(from common.Address, to *common.Address, sr return nil } +// CanCreateContract proxies arguments to the s.CanCreateContractFn function if +// non-nil, otherwise it acts as a noop. func (s Stub) CanCreateContract(cc *libevm.AddressContext, sr libevm.StateReader) error { if f := s.CanCreateContractFn; f != nil { return f(cc, sr) diff --git a/libevm/pseudo/constructor_test.go b/libevm/pseudo/constructor_test.go index a28f3dd42de..7b681c48d63 100644 --- a/libevm/pseudo/constructor_test.go +++ b/libevm/pseudo/constructor_test.go @@ -14,6 +14,7 @@ func TestConstructor(t *testing.T) { testConstructor[struct{ x string }](t) } +//nolint:thelper // This is the test itself so we want local line numbers reported. func testConstructor[T any](t *testing.T) { var zero T t.Run(fmt.Sprintf("%T", zero), func(t *testing.T) { diff --git a/libevm/pseudo/type.go b/libevm/pseudo/type.go index 8d1568638ed..822198f9737 100644 --- a/libevm/pseudo/type.go +++ b/libevm/pseudo/type.go @@ -106,10 +106,10 @@ func MustNewValue[T any](t *Type) *Value[T] { } // Get returns the value. -func (a *Value[T]) Get() T { return a.t.val.get().(T) } +func (v *Value[T]) Get() T { return v.t.val.get().(T) } //nolint:forcetypeassert // invariant // Set sets the value. -func (a *Value[T]) Set(v T) { a.t.val.mustSet(v) } +func (v *Value[T]) Set(val T) { v.t.val.mustSet(val) } // MarshalJSON implements the [json.Marshaler] interface. func (t *Type) MarshalJSON() ([]byte, error) { return t.val.MarshalJSON() } diff --git a/libevm/pseudo/type_test.go b/libevm/pseudo/type_test.go index a640f089c1a..8a47fe562b8 100644 --- a/libevm/pseudo/type_test.go +++ b/libevm/pseudo/type_test.go @@ -24,6 +24,7 @@ func TestType(t *testing.T) { testType(t, "nil pointer", Zero[*float64], (*float64)(nil), new(float64), 0) } +//nolint:thelper // This is the test itself so we want local line numbers reported. func testType[T any](t *testing.T, name string, ctor func() *Pseudo[T], init T, setTo T, invalid any) { t.Run(name, func(t *testing.T) { typ, val := ctor().TypeAndValue() @@ -67,7 +68,7 @@ func testType[T any](t *testing.T, name string, ctor func() *Pseudo[T], init T, }) } -//nolint:ineffassign // Although `typ` is overwritten it's to demonstrate different approaches. +//nolint:ineffassign,testableexamples // Although `typ` is overwritten it's to demonstrate different approaches func ExamplePseudo_TypeAndValue() { typ, val := From("hello").TypeAndValue() diff --git a/params/config.libevm.go b/params/config.libevm.go index 2f4730b8aa3..3ef29d98f2e 100644 --- a/params/config.libevm.go +++ b/params/config.libevm.go @@ -140,7 +140,7 @@ func notStructMessage[T any]() string { return fmt.Sprintf("%T is not a struct nor a pointer to a struct", x) } -// An ExtraPayloadGettter provides strongly typed access to the extra payloads +// An ExtraPayloadGetter provides strongly typed access to the extra payloads // carried by [ChainConfig] and [Rules] structs. The only valid way to construct // a getter is by a call to [RegisterExtras]. type ExtraPayloadGetter[C ChainConfigHooks, R RulesHooks] struct { diff --git a/params/config.libevm_test.go b/params/config.libevm_test.go index 129cc76ec5b..482b66545dd 100644 --- a/params/config.libevm_test.go +++ b/params/config.libevm_test.go @@ -102,18 +102,18 @@ func TestRegisterExtras(t *testing.T) { tt.register() defer TestOnlyClearRegisteredExtras() - in := &ChainConfig{ + input := &ChainConfig{ ChainID: big.NewInt(142857), extra: tt.ccExtra, } - buf, err := json.Marshal(in) + buf, err := json.Marshal(input) require.NoError(t, err) got := new(ChainConfig) require.NoError(t, json.Unmarshal(buf, got)) assert.Equal(t, tt.ccExtra.Interface(), got.extraPayload().Interface()) - assert.Equal(t, in, got) + assert.Equal(t, input, got) gotRules := got.Rules(nil, false, 0) assert.Equal(t, tt.wantRulesExtra, gotRules.extraPayload().Interface()) diff --git a/params/example.libevm_test.go b/params/example.libevm_test.go index e2a7340d561..f9ff2a4c5c6 100644 --- a/params/example.libevm_test.go +++ b/params/example.libevm_test.go @@ -107,7 +107,7 @@ func (r RulesExtra) PrecompileOverride(addr common.Address) (_ libevm.Precompile // implementation detail. func (r RulesExtra) CanCreateContract(*libevm.AddressContext, libevm.StateReader) error { if time.Unix(int64(r.timestamp), 0).UTC().Day() != int(time.Tuesday) { - return errors.New("uh oh!") + return errors.New("uh oh") } return nil } From aa33c6cfc7be56e3bf22a433f9298cb06d8c1971 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 12 Sep 2024 14:20:42 +0100 Subject: [PATCH 06/12] chore: exclude non-libevm linters + change deprecated option --- .github/workflows/golangci-lint.yml | 1 - .golangci.yml | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 123936378de..be2fc92a6c6 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -9,7 +9,6 @@ on: permissions: contents: read - pull-requests: read jobs: golangci: diff --git a/.golangci.yml b/.golangci.yml index c6cf1715e21..600ce21da79 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -3,7 +3,6 @@ run: timeout: 20m tests: true - skip-dirs-use-default: false linters: enable: @@ -59,6 +58,7 @@ linters-settings: - github.com/ava-labs/subnet-evm: issues: + exclude-dirs-use-default: false exclude-rules: - path-except: libevm linters: @@ -69,6 +69,8 @@ issues: - errcheck - gofmt - gosec + - gosimple + - govet - nakedret - nestif - nilerr From d148cf90c8cfa455b03ff7ec3540f92f91799c1b Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 12 Sep 2024 14:27:03 +0100 Subject: [PATCH 07/12] fix: add overflow check in example --- params/example.libevm_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/params/example.libevm_test.go b/params/example.libevm_test.go index f9ff2a4c5c6..cc9f43ab1ec 100644 --- a/params/example.libevm_test.go +++ b/params/example.libevm_test.go @@ -14,6 +14,7 @@ import ( "errors" "fmt" "log" + "math" "math/big" "time" @@ -106,7 +107,7 @@ func (r RulesExtra) PrecompileOverride(addr common.Address) (_ libevm.Precompile // to state allows it to be configured on-chain however this is an optional // implementation detail. func (r RulesExtra) CanCreateContract(*libevm.AddressContext, libevm.StateReader) error { - if time.Unix(int64(r.timestamp), 0).UTC().Day() != int(time.Tuesday) { + if r.timestamp > math.MaxInt64 || time.Unix(int64(r.timestamp), 0).UTC().Day() != int(time.Tuesday) { return errors.New("uh oh") } return nil From 1ba0f23cd4756c25cbadad604ada641755a8f0dd Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 12 Sep 2024 14:33:41 +0100 Subject: [PATCH 08/12] fix: try again; different local version? --- params/example.libevm_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/params/example.libevm_test.go b/params/example.libevm_test.go index cc9f43ab1ec..e9d6f6fae59 100644 --- a/params/example.libevm_test.go +++ b/params/example.libevm_test.go @@ -14,7 +14,6 @@ import ( "errors" "fmt" "log" - "math" "math/big" "time" @@ -107,7 +106,7 @@ func (r RulesExtra) PrecompileOverride(addr common.Address) (_ libevm.Precompile // to state allows it to be configured on-chain however this is an optional // implementation detail. func (r RulesExtra) CanCreateContract(*libevm.AddressContext, libevm.StateReader) error { - if r.timestamp > math.MaxInt64 || time.Unix(int64(r.timestamp), 0).UTC().Day() != int(time.Tuesday) { + if time.Unix(int64(r.timestamp), 0).UTC().Day() != int(time.Tuesday) { // #nosec G115 return errors.New("uh oh") } return nil From d47a4f8e3eb0dccc24af56cd2592d0cbfb9f31d7 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 12 Sep 2024 14:37:36 +0100 Subject: [PATCH 09/12] chore: this is trying my patience --- params/example.libevm_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/params/example.libevm_test.go b/params/example.libevm_test.go index e9d6f6fae59..0ac411bdafb 100644 --- a/params/example.libevm_test.go +++ b/params/example.libevm_test.go @@ -106,7 +106,7 @@ func (r RulesExtra) PrecompileOverride(addr common.Address) (_ libevm.Precompile // to state allows it to be configured on-chain however this is an optional // implementation detail. func (r RulesExtra) CanCreateContract(*libevm.AddressContext, libevm.StateReader) error { - if time.Unix(int64(r.timestamp), 0).UTC().Day() != int(time.Tuesday) { // #nosec G115 + if time.Unix(int64(r.timestamp), 0).UTC().Day() != int(time.Tuesday) { //nolint:gosec // triggering G115 overflow return errors.New("uh oh") } return nil From 112be7284ee181751b93d28c733762f7de9ed879 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 12 Sep 2024 15:00:31 +0100 Subject: [PATCH 10/12] chore: enable `gci` and fix ordering --- .golangci.yml | 30 ++++++++++++++++++---------- core/state_transition.libevm_test.go | 3 ++- core/vm/contracts.libevm.go | 3 ++- core/vm/contracts.libevm_test.go | 9 +++++---- libevm/ethtest/evm.go | 3 ++- libevm/ethtest/rand.go | 3 ++- libevm/libevm.go | 3 ++- params/config.libevm_test.go | 3 ++- params/example.libevm_test.go | 2 +- params/json.libevm_test.go | 3 ++- 10 files changed, 40 insertions(+), 22 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 600ce21da79..50be17eef8f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -9,11 +9,11 @@ linters: # Every available linter at the time of writing was considered (quickly) and # inclusion was liberal. Linters are good at detecting code smells, but if # we find that a particular one causes too many false positives then we can - # first configure it better or, as a last resort, remove it.. + # configure it better or, as a last resort, remove it. - containedctx - errcheck - forcetypeassert - # TODO(arr4n) add and configure gci + - gci - gocheckcompilerdirectives - gofmt - goimports @@ -42,20 +42,29 @@ linters: - whitespace linters-settings: - gofmt: - simplify: true - revive: - rules: - - name: unused-parameter - # Method parameters may be equired by interfaces and forcing them to be - # named _ is of questionable benefit. - disabled: true + gci: + custom-order: true + sections: + - standard + - default + - localmodule + # The rest of these break developer expections, in increasing order of + # divergence, so are at the end to increase the chance of being seen. + - alias + - dot + - blank gomodguard: blocked: modules: - github.com/ava-labs/avalanchego: - github.com/ava-labs/coreth: - github.com/ava-labs/subnet-evm: + revive: + rules: + - name: unused-parameter + # Method parameters may be equired by interfaces and forcing them to be + # named _ is of questionable benefit. + disabled: true issues: exclude-dirs-use-default: false @@ -67,6 +76,7 @@ issues: - containedctx - forcetypeassert - errcheck + - gci - gofmt - gosec - gosimple diff --git a/core/state_transition.libevm_test.go b/core/state_transition.libevm_test.go index fff4267bcf0..15235b40133 100644 --- a/core/state_transition.libevm_test.go +++ b/core/state_transition.libevm_test.go @@ -4,12 +4,13 @@ import ( "fmt" "testing" + "github.com/stretchr/testify/require" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/libevm" "github.com/ethereum/go-ethereum/libevm/ethtest" "github.com/ethereum/go-ethereum/libevm/hookstest" - "github.com/stretchr/testify/require" ) func TestCanExecuteTransaction(t *testing.T) { diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go index 23057d6f9c8..3aa5bed9d53 100644 --- a/core/vm/contracts.libevm.go +++ b/core/vm/contracts.libevm.go @@ -3,9 +3,10 @@ package vm import ( "fmt" + "github.com/holiman/uint256" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/params" - "github.com/holiman/uint256" ) // evmCallArgs mirrors the parameters of the [EVM] methods Call(), CallCode(), diff --git a/core/vm/contracts.libevm_test.go b/core/vm/contracts.libevm_test.go index 585cce03f21..3b2c98a887f 100644 --- a/core/vm/contracts.libevm_test.go +++ b/core/vm/contracts.libevm_test.go @@ -4,6 +4,11 @@ import ( "fmt" "testing" + "github.com/holiman/uint256" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/exp/rand" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/crypto" @@ -11,10 +16,6 @@ import ( "github.com/ethereum/go-ethereum/libevm/ethtest" "github.com/ethereum/go-ethereum/libevm/hookstest" "github.com/ethereum/go-ethereum/params" - "github.com/holiman/uint256" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "golang.org/x/exp/rand" ) type precompileStub struct { diff --git a/libevm/ethtest/evm.go b/libevm/ethtest/evm.go index 069a949ac64..171ff60e030 100644 --- a/libevm/ethtest/evm.go +++ b/libevm/ethtest/evm.go @@ -5,13 +5,14 @@ package ethtest import ( "testing" + "github.com/stretchr/testify/require" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/core/state" "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/params" - "github.com/stretchr/testify/require" ) // NewZeroEVM returns a new EVM backed by a [rawdb.NewMemoryDatabase]; all other diff --git a/libevm/ethtest/rand.go b/libevm/ethtest/rand.go index b8c88fe5314..cacce61f51e 100644 --- a/libevm/ethtest/rand.go +++ b/libevm/ethtest/rand.go @@ -1,8 +1,9 @@ package ethtest import ( - "github.com/ethereum/go-ethereum/common" "golang.org/x/exp/rand" + + "github.com/ethereum/go-ethereum/common" ) // PseudoRand extends [rand.Rand] (*not* crypto/rand). diff --git a/libevm/libevm.go b/libevm/libevm.go index 8b7d85bd49e..bd11f971fef 100644 --- a/libevm/libevm.go +++ b/libevm/libevm.go @@ -1,8 +1,9 @@ package libevm import ( - "github.com/ethereum/go-ethereum/common" "github.com/holiman/uint256" + + "github.com/ethereum/go-ethereum/common" ) // PrecompiledContract is an exact copy of vm.PrecompiledContract, mirrored here diff --git a/params/config.libevm_test.go b/params/config.libevm_test.go index 482b66545dd..9a5f5a374fc 100644 --- a/params/config.libevm_test.go +++ b/params/config.libevm_test.go @@ -5,9 +5,10 @@ import ( "math/big" "testing" - "github.com/ethereum/go-ethereum/libevm/pseudo" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/ethereum/go-ethereum/libevm/pseudo" ) type rawJSON struct { diff --git a/params/example.libevm_test.go b/params/example.libevm_test.go index 0ac411bdafb..6a32a985ab4 100644 --- a/params/example.libevm_test.go +++ b/params/example.libevm_test.go @@ -106,7 +106,7 @@ func (r RulesExtra) PrecompileOverride(addr common.Address) (_ libevm.Precompile // to state allows it to be configured on-chain however this is an optional // implementation detail. func (r RulesExtra) CanCreateContract(*libevm.AddressContext, libevm.StateReader) error { - if time.Unix(int64(r.timestamp), 0).UTC().Day() != int(time.Tuesday) { //nolint:gosec // triggering G115 overflow + if time.Unix(int64(r.timestamp), 0).UTC().Day() != int(time.Tuesday) { //nolint:gosec // G115 timestamp won't overflow int64 for millions of years so this is someone else's problem return errors.New("uh oh") } return nil diff --git a/params/json.libevm_test.go b/params/json.libevm_test.go index 780b8f220e6..aa9f7928fdf 100644 --- a/params/json.libevm_test.go +++ b/params/json.libevm_test.go @@ -6,8 +6,9 @@ import ( "math/big" "testing" - "github.com/ethereum/go-ethereum/libevm/pseudo" "github.com/stretchr/testify/require" + + "github.com/ethereum/go-ethereum/libevm/pseudo" ) type nestedChainConfigExtra struct { From 09f8e69f398b4f4b1fc4409679af56d7edf6dbaa Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 12 Sep 2024 20:19:53 +0100 Subject: [PATCH 11/12] chore: mark `ethclient/gethclient` test as flaky --- .github/workflows/go.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 2946da9a931..f7f3ded7d30 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -18,6 +18,6 @@ jobs: go-version: 1.21.4 - name: Run tests run: | # Upstream flakes are race conditions exacerbated by concurrent tests - FLAKY_REGEX='go-ethereum/(eth|accounts/keystore|eth/downloader|miner)$'; + FLAKY_REGEX='go-ethereum/(eth|accounts/keystore|eth/downloader|miner|ethclient/gethclient)$'; go list ./... | grep -P "${FLAKY_REGEX}" | xargs -n 1 go test -short; go test -short $(go list ./... | grep -Pv "${FLAKY_REGEX}"); From 7a29ad66a4fa1e44e492ef9b9ae7fc705b1a3ebb Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 12 Sep 2024 20:25:10 +0100 Subject: [PATCH 12/12] chore: mark `eth/catalyst` test as flaky --- .github/workflows/go.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index f7f3ded7d30..851cb51eb9b 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -18,6 +18,6 @@ jobs: go-version: 1.21.4 - name: Run tests run: | # Upstream flakes are race conditions exacerbated by concurrent tests - FLAKY_REGEX='go-ethereum/(eth|accounts/keystore|eth/downloader|miner|ethclient/gethclient)$'; + FLAKY_REGEX='go-ethereum/(eth|accounts/keystore|eth/downloader|miner|ethclient/gethclient|eth/catalyst)$'; go list ./... | grep -P "${FLAKY_REGEX}" | xargs -n 1 go test -short; go test -short $(go list ./... | grep -Pv "${FLAKY_REGEX}");