From 32f28a9360d26a661d55915915f12fd3c70f012b Mon Sep 17 00:00:00 2001
From: Martin Holst Swende <martin@swende.se>
Date: Tue, 4 Sep 2018 10:49:18 +0200
Subject: [PATCH] core/vm, tests: update tests, enable constantinople
 statetests, fix SAR opcode (#17538)

This commit does a few things at once:

- Updates the tests to contain the latest data from ethereum/tests repo.
- Enables Constantinople state tests. This is needed to be able to
  fuzz-test the evm with constantinople rules.
- Fixes the error in opSAR that we've known about for some time. I was
  kind of saving it to see if we hit upon it with the random test
  generator, but it's difficult to both enable the tests and have the
  bug there -- we don't want to forget about it, so maybe it's better
  to just fix it.
---
 core/vm/instructions.go  |  2 +-
 tests/block_test.go      |  6 +++---
 tests/init_test.go       | 10 ++++++++++
 tests/state_test.go      |  3 ---
 tests/state_test_util.go | 13 ++++++++++++-
 tests/testdata           |  2 +-
 6 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/core/vm/instructions.go b/core/vm/instructions.go
index b7742e655..ca9e775ac 100644
--- a/core/vm/instructions.go
+++ b/core/vm/instructions.go
@@ -355,7 +355,7 @@ func opSAR(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memory *
 	defer interpreter.intPool.put(shift) // First operand back into the pool
 
 	if shift.Cmp(common.Big256) >= 0 {
-		if value.Sign() > 0 {
+		if value.Sign() >= 0 {
 			value.SetUint64(0)
 		} else {
 			value.SetInt64(-1)
diff --git a/tests/block_test.go b/tests/block_test.go
index 669d3ca08..c91119929 100644
--- a/tests/block_test.go
+++ b/tests/block_test.go
@@ -30,11 +30,11 @@ func TestBlockchain(t *testing.T) {
 	bt.skipLoad(`^bcForgedTest/bcForkUncle\.json`)
 	bt.skipLoad(`^bcMultiChainTest/(ChainAtoChainB_blockorder|CallContractFromNotBestBlock)`)
 	bt.skipLoad(`^bcTotalDifficultyTest/(lotsOfLeafs|lotsOfBranches|sideChainWithMoreTransactions)`)
-	// Constantinople is not implemented yet.
-	bt.skipLoad(`(?i)(constantinople)`)
+	// This test is broken
+	bt.fails(`blockhashNonConstArg_Constantinople`, "Broken test")
 
 	// Still failing tests
-	bt.skipLoad(`^bcWalletTest.*_Byzantium$`)
+	//	bt.skipLoad(`^bcWalletTest.*_Byzantium$`)
 
 	bt.walk(t, blockTestDir, func(t *testing.T, name string, test *BlockTest) {
 		if err := bt.checkFailure(t, name, test.Run()); err != nil {
diff --git a/tests/init_test.go b/tests/init_test.go
index 26e919d24..90a74448a 100644
--- a/tests/init_test.go
+++ b/tests/init_test.go
@@ -91,6 +91,7 @@ type testMatcher struct {
 	failpat      []testFailure
 	skiploadpat  []*regexp.Regexp
 	skipshortpat []*regexp.Regexp
+	whitelistpat *regexp.Regexp
 }
 
 type testConfig struct {
@@ -121,6 +122,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})
@@ -208,6 +213,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]<testType>.
diff --git a/tests/state_test.go b/tests/state_test.go
index adec4feb2..b61a1ca28 100644
--- a/tests/state_test.go
+++ b/tests/state_test.go
@@ -44,9 +44,6 @@ func TestState(t *testing.T) {
 			key := fmt.Sprintf("%s/%d", subtest.Fork, subtest.Index)
 			name := name + "/" + key
 			t.Run(key, func(t *testing.T) {
-				if subtest.Fork == "Constantinople" {
-					t.Skip("constantinople not supported yet")
-				}
 				withTrace(t, test.gasLimit(subtest), func(vmconfig vm.Config) error {
 					_, err := test.Run(subtest, vmconfig)
 					return st.checkFailure(t, name, err)
diff --git a/tests/state_test_util.go b/tests/state_test_util.go
index 84581fae1..5d2251e52 100644
--- a/tests/state_test_util.go
+++ b/tests/state_test_util.go
@@ -146,7 +146,18 @@ func (t *StateTest) Run(subtest StateSubtest, vmconfig vm.Config) (*state.StateD
 	if logs := rlpHash(statedb.Logs()); logs != common.Hash(post.Logs) {
 		return statedb, fmt.Errorf("post state logs hash mismatch: got %x, want %x", logs, post.Logs)
 	}
-	root, _ := statedb.Commit(config.IsEIP158(block.Number()))
+	// Commit block
+	statedb.Commit(config.IsEIP158(block.Number()))
+	// Add 0-value mining reward. This only makes a difference in the cases
+	// where
+	// - the coinbase suicided, or
+	// - there are only 'bad' transactions, which aren't executed. In those cases,
+	//   the coinbase gets no txfee, so isn't created, and thus needs to be touched
+	statedb.AddBalance(block.Coinbase(), new(big.Int))
+	// And _now_ get the state root
+	root := statedb.IntermediateRoot(config.IsEIP158(block.Number()))
+	// N.B: We need to do this in a two-step process, because the first Commit takes care
+	// of suicides, and we need to touch the coinbase _after_ it has potentially suicided.
 	if root != common.Hash(post.Root) {
 		return statedb, fmt.Errorf("post state root mismatch: got %x, want %x", root, post.Root)
 	}
diff --git a/tests/testdata b/tests/testdata
index 2bb0c3da3..ad2184adc 160000
--- a/tests/testdata
+++ b/tests/testdata
@@ -1 +1 @@
-Subproject commit 2bb0c3da3bbb15c528bcef2a7e5ac4bd73f81f87
+Subproject commit ad2184adca367c0b68c65b44519dba16e1d0b9e2
-- 
GitLab