From 6a87db6ac47f62337c6fbab4869eed7195a9621f Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Sat, 5 Jun 2021 08:21:08 +0200 Subject: [PATCH 1/8] core: check if sender is EOA --- core/error.go | 3 +++ core/state_transition.go | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/core/error.go b/core/error.go index bcbce09ba431..33cf874d15e4 100644 --- a/core/error.go +++ b/core/error.go @@ -87,4 +87,7 @@ var ( // ErrFeeCapTooLow is returned if the transaction fee cap is less than the // the base fee of the block. ErrFeeCapTooLow = errors.New("max fee per gas less than block base fee") + + // ErrSenderNoEOA is returned if the sender of a transaction is a contract. + ErrSenderNoEOA = errors.New("sender not an eoa") ) diff --git a/core/state_transition.go b/core/state_transition.go index b5b0ae990a41..74b3e8238de9 100644 --- a/core/state_transition.go +++ b/core/state_transition.go @@ -220,6 +220,11 @@ func (st *StateTransition) preCheck() error { st.msg.From().Hex(), msgNonce, stNonce) } } + // Make sure the sender is an EOA + if codesize := st.state.GetCodeSize(st.msg.From()); codesize != 0 { + return fmt.Errorf("%w: address %v, codesize: %d", ErrSenderNoEOA, + st.msg.From().Hex(), codesize) + } // Make sure that transaction gasFeeCap is greater than the baseFee (post london) if st.evm.ChainConfig().IsLondon(st.evm.Context.BlockNumber) { // Skip the checks if gas fields are zero and baseFee was explicitly disabled (eth_call) From c75795e6edb04615d95169228e019bfb957224a0 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Mon, 7 Jun 2021 12:19:36 +0200 Subject: [PATCH 2/8] core: add ErrSenderNoEOA test --- core/state_processor_test.go | 42 +++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/core/state_processor_test.go b/core/state_processor_test.go index ce6d2bcdde8b..5e029953fb6f 100644 --- a/core/state_processor_test.go +++ b/core/state_processor_test.go @@ -195,7 +195,7 @@ func TestStateProcessorErrors(t *testing.T) { } } - // One final error is ErrTxTypeNotSupported. For this, we need an older chain + // ErrTxTypeNotSupported, For this, we need an older chain { var ( db = rawdb.NewMemoryDatabase() @@ -244,6 +244,46 @@ func TestStateProcessorErrors(t *testing.T) { } } } + + // ErrSenderNoEOA, for this we need the sender to have contract code + { + var ( + db = rawdb.NewMemoryDatabase() + gspec = &Genesis{ + Config: config, + Alloc: GenesisAlloc{ + common.HexToAddress("0x71562b71999873DB5b286dF957af199Ec94617F7"): GenesisAccount{ + Balance: big.NewInt(1000000000000000000), // 1 ether + Nonce: 0, + Code: common.FromHex("0xB0B0FACE"), + }, + }, + } + genesis = gspec.MustCommit(db) + blockchain, _ = NewBlockChain(db, nil, gspec.Config, ethash.NewFaker(), vm.Config{}, nil, nil) + ) + defer blockchain.Stop() + for i, tt := range []struct { + txs []*types.Transaction + want string + }{ + { // ErrSenderNoEOA + txs: []*types.Transaction{ + mkDynamicTx(0, common.Address{}, params.TxGas-1000, big.NewInt(0), big.NewInt(0)), + }, + want: "could not apply tx 0 [0x88626ac0d53cb65308f2416103c62bb1f18b805573d4f96a3640bbbfff13c14f]: sender not an eoa: address 0x71562b71999873DB5b286dF957af199Ec94617F7, codesize: 4", + }, + } { + block := GenerateBadBlock(genesis, ethash.NewFaker(), tt.txs, gspec.Config) + _, err := blockchain.InsertChain(types.Blocks{block}) + if err == nil { + t.Fatal("block imported without errors") + } + if have, want := err.Error(), tt.want; have != want { + t.Errorf("test %d:\nhave \"%v\"\nwant \"%v\"\n", i, have, want) + } + } + } } // GenerateBadBlock constructs a "block" which contains the transactions. The transactions are not expected to be From f3e7238b22715c977c4953638b0e4e5f6db6140f Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Mon, 28 Jun 2021 13:33:38 +0200 Subject: [PATCH 3/8] core: check for empty blockhash instead of blocksize --- core/state_processor_test.go | 2 +- core/state_transition.go | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/core/state_processor_test.go b/core/state_processor_test.go index 5e029953fb6f..13a9eb810df6 100644 --- a/core/state_processor_test.go +++ b/core/state_processor_test.go @@ -271,7 +271,7 @@ func TestStateProcessorErrors(t *testing.T) { txs: []*types.Transaction{ mkDynamicTx(0, common.Address{}, params.TxGas-1000, big.NewInt(0), big.NewInt(0)), }, - want: "could not apply tx 0 [0x88626ac0d53cb65308f2416103c62bb1f18b805573d4f96a3640bbbfff13c14f]: sender not an eoa: address 0x71562b71999873DB5b286dF957af199Ec94617F7, codesize: 4", + want: "could not apply tx 0 [0x88626ac0d53cb65308f2416103c62bb1f18b805573d4f96a3640bbbfff13c14f]: sender not an eoa: address 0x71562b71999873DB5b286dF957af199Ec94617F7, codehash: 0x9280914443471259d4570a8661015ae4a5b80186dbc619658fb494bebc3da3d1", }, } { block := GenerateBadBlock(genesis, ethash.NewFaker(), tt.txs, gspec.Config) diff --git a/core/state_transition.go b/core/state_transition.go index 74b3e8238de9..d16264861411 100644 --- a/core/state_transition.go +++ b/core/state_transition.go @@ -25,9 +25,12 @@ import ( cmath "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/vm" + "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/params" ) +var emptyCodeHash = crypto.Keccak256Hash(nil) + /* The State Transitioning Model @@ -221,9 +224,9 @@ func (st *StateTransition) preCheck() error { } } // Make sure the sender is an EOA - if codesize := st.state.GetCodeSize(st.msg.From()); codesize != 0 { - return fmt.Errorf("%w: address %v, codesize: %d", ErrSenderNoEOA, - st.msg.From().Hex(), codesize) + if codeHash := st.state.GetCodeHash(st.msg.From()); codeHash != emptyCodeHash { + return fmt.Errorf("%w: address %v, codehash: %s", ErrSenderNoEOA, + st.msg.From().Hex(), codeHash) } // Make sure that transaction gasFeeCap is greater than the baseFee (post london) if st.evm.ChainConfig().IsLondon(st.evm.Context.BlockNumber) { From 1d071ab55779fbf3022f6efb32b44d1a0fd1fce6 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Thu, 1 Jul 2021 15:37:32 +0200 Subject: [PATCH 4/8] eth/tracers: clarify test output --- eth/tracers/api_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/eth/tracers/api_test.go b/eth/tracers/api_test.go index 4a4e5bbc33d2..9afd59d596bc 100644 --- a/eth/tracers/api_test.go +++ b/eth/tracers/api_test.go @@ -417,24 +417,24 @@ func TestOverriddenTraceCall(t *testing.T) { }, }, } - for _, testspec := range testSuite { + for i, testspec := range testSuite { result, err := api.TraceCall(context.Background(), testspec.call, rpc.BlockNumberOrHash{BlockNumber: &testspec.blockNumber}, testspec.config) if testspec.expectErr != nil { if err == nil { - t.Errorf("Expect error %v, get nothing", testspec.expectErr) + t.Errorf("test %d: want error %v, have nothing", i, testspec.expectErr) continue } if !errors.Is(err, testspec.expectErr) { - t.Errorf("Error mismatch, want %v, get %v", testspec.expectErr, err) + t.Errorf("test %d: error mismatch, want %v, have %v", i, testspec.expectErr, err) } } else { if err != nil { - t.Errorf("Expect no error, get %v", err) + t.Errorf("test %d: want no error, have %v", i, err) continue } ret := new(callTrace) if err := json.Unmarshal(result.(json.RawMessage), ret); err != nil { - t.Fatalf("failed to unmarshal trace result: %v", err) + t.Fatalf("test %d: failed to unmarshal trace result: %v", i, err) } if !jsonEqual(ret, testspec.expect) { // uncomment this for easier debugging From 4884220db5e82d107b8f38915abe15b99c9d695d Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Sat, 3 Jul 2021 18:44:13 +0200 Subject: [PATCH 5/8] core: allow empty-codehash (non-existing) sender to count as not-eoa --- core/state_transition.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/state_transition.go b/core/state_transition.go index d16264861411..68f3c74a3121 100644 --- a/core/state_transition.go +++ b/core/state_transition.go @@ -224,7 +224,7 @@ func (st *StateTransition) preCheck() error { } } // Make sure the sender is an EOA - if codeHash := st.state.GetCodeHash(st.msg.From()); codeHash != emptyCodeHash { + if codeHash := st.state.GetCodeHash(st.msg.From()); codeHash != emptyCodeHash && codeHash != (common.Hash{}) { return fmt.Errorf("%w: address %v, codehash: %s", ErrSenderNoEOA, st.msg.From().Hex(), codeHash) } From d98ff819280bdbe7c07b1f521a4aa453077ce754 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Fri, 30 Jul 2021 13:19:53 +0200 Subject: [PATCH 6/8] tests: revert "tests: remove whitelist feature (#23297)" This reverts commit 85afdeef37e17bffd9d098e86312c569995705e0. --- tests/init_test.go | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/tests/init_test.go b/tests/init_test.go index f1a4ca2cf087..1638f863e1c1 100644 --- a/tests/init_test.go +++ b/tests/init_test.go @@ -89,10 +89,11 @@ func findLine(data []byte, offset int64) (line int) { // testMatcher controls skipping and chain config assignment to tests. type testMatcher struct { - configpat []testConfig - failpat []testFailure - skiploadpat []*regexp.Regexp - slowpat []*regexp.Regexp + configpat []testConfig + failpat []testFailure + skiploadpat []*regexp.Regexp + slowpat []*regexp.Regexp + whitelistpat *regexp.Regexp } type testConfig struct { @@ -123,6 +124,10 @@ func (tm *testMatcher) fails(pattern string, reason string) { tm.failpat = append(tm.failpat, testFailure{regexp.MustCompile(pattern), reason}) } +func (tm *testMatcher) whitelist(pattern string) { + tm.whitelistpat = regexp.MustCompile(pattern) +} + // config defines chain config for tests matching the pattern. func (tm *testMatcher) config(pattern string, cfg params.ChainConfig) { tm.configpat = append(tm.configpat, testConfig{regexp.MustCompile(pattern), cfg}) @@ -212,6 +217,11 @@ func (tm *testMatcher) runTestFile(t *testing.T, path, name string, runTest inte if r, _ := tm.findSkip(name); r != "" { t.Skip(r) } + if tm.whitelistpat != nil { + if !tm.whitelistpat.MatchString(name) { + t.Skip("Skipped by whitelist") + } + } t.Parallel() // Load the file as map[string]. @@ -265,3 +275,14 @@ func runTestFunc(runTest interface{}, t *testing.T, name string, m reflect.Value m.MapIndex(reflect.ValueOf(key)), }) } + +func TestMatcherWhitelist(t *testing.T) { + t.Parallel() + tm := new(testMatcher) + tm.whitelist("invalid*") + tm.walk(t, rlpTestDir, func(t *testing.T, name string, test *RLPTest) { + if name[:len("invalidRLPTest.json")] != "invalidRLPTest.json" { + t.Fatalf("invalid test found: %s != invalidRLPTest.json", name) + } + }) +} From 1bb80cb74e5ae9290a98ff49b9dfb73afe6ce906 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Fri, 30 Jul 2021 13:42:59 +0200 Subject: [PATCH 7/8] tests: rename whitelist to runonly --- tests/init_test.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/init_test.go b/tests/init_test.go index 1638f863e1c1..361b7cc06248 100644 --- a/tests/init_test.go +++ b/tests/init_test.go @@ -89,11 +89,11 @@ func findLine(data []byte, offset int64) (line int) { // testMatcher controls skipping and chain config assignment to tests. type testMatcher struct { - configpat []testConfig - failpat []testFailure - skiploadpat []*regexp.Regexp - slowpat []*regexp.Regexp - whitelistpat *regexp.Regexp + configpat []testConfig + failpat []testFailure + skiploadpat []*regexp.Regexp + slowpat []*regexp.Regexp + runonlylistpat *regexp.Regexp } type testConfig struct { @@ -124,8 +124,8 @@ func (tm *testMatcher) fails(pattern string, reason string) { tm.failpat = append(tm.failpat, testFailure{regexp.MustCompile(pattern), reason}) } -func (tm *testMatcher) whitelist(pattern string) { - tm.whitelistpat = regexp.MustCompile(pattern) +func (tm *testMatcher) runonly(pattern string) { + tm.runonlylistpat = regexp.MustCompile(pattern) } // config defines chain config for tests matching the pattern. @@ -217,9 +217,9 @@ func (tm *testMatcher) runTestFile(t *testing.T, path, name string, runTest inte if r, _ := tm.findSkip(name); r != "" { t.Skip(r) } - if tm.whitelistpat != nil { - if !tm.whitelistpat.MatchString(name) { - t.Skip("Skipped by whitelist") + if tm.runonlylistpat != nil { + if !tm.runonlylistpat.MatchString(name) { + t.Skip("Skipped by runonly") } } t.Parallel() @@ -276,10 +276,10 @@ func runTestFunc(runTest interface{}, t *testing.T, name string, m reflect.Value }) } -func TestMatcherWhitelist(t *testing.T) { +func TestMatcherRunonlylist(t *testing.T) { t.Parallel() tm := new(testMatcher) - tm.whitelist("invalid*") + tm.runonly("invalid*") tm.walk(t, rlpTestDir, func(t *testing.T, name string, test *RLPTest) { if name[:len("invalidRLPTest.json")] != "invalidRLPTest.json" { t.Fatalf("invalid test found: %s != invalidRLPTest.json", name) From e9d8359f2a497f37ded898556cee6d9f6580d16d Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Fri, 30 Jul 2021 13:38:53 +0200 Subject: [PATCH 8/8] tests: skip broken test --- tests/init_test.go | 8 ++++---- tests/state_test.go | 7 +++++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/init_test.go b/tests/init_test.go index 361b7cc06248..969cf7139915 100644 --- a/tests/init_test.go +++ b/tests/init_test.go @@ -34,10 +34,10 @@ import ( ) var ( - baseDir = filepath.Join(".", "testdata") - blockTestDir = filepath.Join(baseDir, "BlockchainTests") - stateTestDir = filepath.Join(baseDir, "GeneralStateTests") - legacyStateTestDir = filepath.Join(baseDir, "LegacyTests", "Constantinople", "GeneralStateTests") + baseDir = filepath.Join(".", "testdata") + blockTestDir = filepath.Join(baseDir, "BlockchainTests") + stateTestDir = filepath.Join(baseDir, "GeneralStateTests") + //legacyStateTestDir = filepath.Join(baseDir, "LegacyTests", "Constantinople", "GeneralStateTests") transactionTestDir = filepath.Join(baseDir, "TransactionTests") vmTestDir = filepath.Join(baseDir, "VMTests") rlpTestDir = filepath.Join(baseDir, "RLPTests") diff --git a/tests/state_test.go b/tests/state_test.go index c2ca0e8d6948..242e9712a312 100644 --- a/tests/state_test.go +++ b/tests/state_test.go @@ -45,7 +45,8 @@ func TestState(t *testing.T) { // Uses 1GB RAM per tested fork st.skipLoad(`^stStaticCall/static_Call1MB`) - + // Un-skip this when https://github.com/ethereum/tests/issues/908 is closed + st.skipLoad(`^stQuadraticComplexityTest/QuadraticComplexitySolidity_CallDataCopy`) // Broken tests: // Expected failures: //st.fails(`^stRevertTest/RevertPrecompiledTouch(_storage)?\.json/Byzantium/0`, "bug in test") @@ -58,7 +59,9 @@ func TestState(t *testing.T) { // For Istanbul, older tests were moved into LegacyTests for _, dir := range []string{ stateTestDir, - legacyStateTestDir, + // legacy state tests are disabled, due to them not being + // regenerated for the no-sender-eoa change. + //legacyStateTestDir, } { st.walk(t, dir, func(t *testing.T, name string, test *StateTest) { for _, subtest := range test.Subtests() {