Fixing Geth Tests by willmeister · Pull Request #6 · ethereum-optimism/go-ethereum

willmeister · 2020-05-20T15:04:55Z

Skips tests that were failing at the time of the fork and fixes other failures.

… were failing

…test on a single linux distro / arch

karlfloersch

This looks great! I left a couple comments around the gas limit usage but overall super exciting! Big fan of the approach as well --happy we get to use GH actions

karlfloersch · 2020-05-21T14:02:14Z

    "constantinopleFixForkBlock": "0x9c40",
    "istanbulForkBlock": "0xc350",
-    "minGasLimit": "0x1388",
+    "minGasLimit": "0xee6b2800",


Do the tests fail with the lower gas limit?

For context, I'm hopeful that soon we can start decreasing the gas limit to be something that's much more reasonable, because that'll be required for ensuring our txs fit in an Ethereum block.

Yeah, they fail without it. Definitely down to decrease it, but probably in another PR.

karlfloersch · 2020-05-21T19:29:10Z

 func TestFreezerBasics(t *testing.T) {
+
+	// TODO: SKIPPING
+	t.Skip("This test fails on the last pre-fork commit. It may be OS X related")


karlfloersch · 2020-05-21T19:29:34Z

+echo "*** Running Tests ***"
+go run build/ci.go test -coverage $TEST_PACKAGES || { echo 'tests failed' ; exit 1; }
+
+echo "\n*** Tests Passed ***\n"


karlfloersch · 2020-05-21T19:31:13Z

-		ethNoFork, _  = NewProtocolManager(configNoFork, nil, downloader.FullSync, 1, new(event.TypeMux), new(testTxPool), engine, chainNoFork, dbNoFork, 1, nil, rollupBlockBuilderNoFork)
-		ethProFork, _ = NewProtocolManager(configProFork, nil, downloader.FullSync, 1, new(event.TypeMux), new(testTxPool), engine, chainProFork, dbProFork, 1, nil, rollupBlockBuilderProFork)
+		ethNoFork, _  = NewProtocolManager(configNoFork, nil, downloader.FullSync, 1, new(event.TypeMux), new(testTxPool), engine, chainNoFork, dbNoFork, 1, nil, rollupTransitionBatchBuilderNoFork)
+		ethProFork, _ = NewProtocolManager(configProFork, nil, downloader.FullSync, 1, new(event.TypeMux), new(testTxPool), engine, chainProFork, dbProFork, 1, nil, rollupTransitionBatchBuilderProFork)


karlfloersch · 2020-05-21T19:36:46Z

 	bt.skipLoad(`^GeneralStateTests/`)
 	// Skip random failures due to selfish mining test
 	bt.skipLoad(`.*bcForgedTest/bcForkUncle\.json`)
+	// Skip these tests because the OVM gas limit will be different


I think ideally the OVM gas limit will match Geth's gas limit because we will still need to make sure that the gas used in a single transaction is less than the block gas limit of Ethereum. The only thing that will be different is that we will also track a bit more information, in particular the number of unique SSTORES/SLOADS as well as the number of unique contracts touched.

Tbh we were just super insane about the gas limit before because we were being lazy about testing & gas limit is the easiest thing to mess up & not realize.

Fixing broken tests, skipping tests we intentionally break, and configuring CI within Github Actions

* Get basic getStorage/setStorage stubs working * Clean up tests * Add state_manager * Add StateManager set & getStorage * Add state mananger create function * Add get & increment nonce * Add getCodeContractBytecode * Add GetCodeContractHash * Add getCodeContractHash to the state manager * Add associateCodeContract to state manager * Pass the tests * go fmt * Add stateTransition to test with * Fix tests * Test deploying contract with transition state * Call executeTransaction on contract deployment * Added ExecutionManager deployment * Get contract deployments working * Cleanup logging * Get stubbed ExecutionManager working * Get a simple contract to deploy through the ExecutionManager * Refactor simpleAbiEncode * Revert unnecessary changes * Remove comments * Revert changes outside of this PR * Revert changes outside of this PR * Revert changes outside of this PR * Fix broken tests * Move OVM bytecode & ABI into constants * Add crazy printlines * Remove crazy comments * Add a bunch of debug printlns * Add helper fn for applying msgs to the EVM * Update ExecutionManager bytecode * Shim CREATE for EM to use correct addr * Add SimpleStorage test * Add the EM/SM to all new states * Force all txs to be routed through the EM * Remove unused files * Remove unused comments * Increment nonce after failed tx * Add debug statements * Use evm.Time for timestamp * Change EM deployment, fix broken tests, clean up * Add an OVM test & remove printlns * Fix lint errors & remove final printlns * Final cleanup--remove some comments * Limiting Geth to one transaction per block (#3) * Limiting Geth to one transaction per block * Adding TransitionBatchBuilder to build & submit rollup blocks * Adding L1MessageSender to Transaction (#4) * Adding L1MessageSender to Transaction * Adding logic to omit L1MessageSender in encoding / decoding when nil and never use it in hash computation Co-authored-by: ben-chain <ben@pseudonym.party> * Fixing Geth Tests (#6) Fixing broken tests, skipping tests we intentionally break, and configuring CI within Github Actions * Hex Trie -> Binary Trie (#7) *** Changing Hex Trie to Binary Trie *** Note: This changes and/or comments out a bunch of tests, so if things break down the line, this is likely the cause! * Ingest Block Batches (#8) Handling BlockBatches in Geth at `SendBlockBatches` endpoint (eth_sendBlockBatches) Other: * Adding PR template * Adding ability to set timestamp and making blocks use configured timestamp * Adding ability to encode original tx nonce in calldata * Adding L1MessageSender to Contract Creation Txs * Add L1MessageSender to Message * Increment nonce on CREATE failure * Fix bug where evm.Time=0 * Use state dump with hardcoded EM & SM addrs - ExecutionMgr address should always be 0x0000...dead0000 - StateMgr address should always be 0x0000...dead0001 * Move EM deployment into genesis block maker * Update EM contracts to latest version * Update EM to remove events * Fix the OVM tests * Skip an ungodly number of tests * Fix lint errors * Clean up logging * Cleanup more logs * Use local reference to state manager * Rename applyOvmToState(..) * Remove unneeded check * Clean up logging & add EM ABI panic * Add gas metering to SM & small refactor * Update core/vm/state_manager.go Co-authored-by: Kevin Ho <kevinjho1996@gmail.com> Co-authored-by: Mason Fischer <mason@kissr.co> Co-authored-by: Will Meister <william.k.meister@gmail.com> Co-authored-by: ben-chain <ben@pseudonym.party> Co-authored-by: Kevin Ho <kevinjho1996@gmail.com>

willmeister added 20 commits May 19, 2020 12:34

Fixing syntax error and commenting out gas check

a10fd8b

Merge branch 'master' into YAS-383/MakeGethTestsPass

26e2c89

Fixing config mismatch betwen aleth and parity

73153ca

skipping tests that failed at the time of fork and fixing others that…

3ea1e88

… were failing

push to trigger build

a75f24d

fixing linting issues

0c376d0

fixing lint issues

dc64529

fixing linting

9d33dee

fixing linting

6f17ce5

after running goimports

539c90b

after running goimports

ea327f0

updating travis to use latest go version

9e755e3

updating .travis.yml and go.mod from upstream

9c2a00d

updating various CI config files

0c58398

updating from upstream

b79ae8d

updating go path

dc668c6

ignoring ethereum tests that we purposefully break due to OVM changes

7895cb6

removing inadvertent test change, updating travis config to only run …

8392505

…test on a single linux distro / arch

Adding github action and dockerfile for CI

686c906

removing travis.yml in favor of Github Actions

7b94ddc

karlfloersch approved these changes May 21, 2020

View reviewed changes

willmeister merged commit 57f1ac2 into master May 21, 2020

willmeister deleted the YAS-383/MakeGethTestsPass branch May 21, 2020 19:56

karlfloersch pushed a commit that referenced this pull request Jul 16, 2020

Fixing Geth Tests (#6)

1f6f711

Fixing broken tests, skipping tests we intentionally break, and configuring CI within Github Actions

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing Geth Tests#6

Fixing Geth Tests#6
willmeister merged 20 commits intomasterfrom
YAS-383/MakeGethTestsPass

willmeister commented May 20, 2020

Uh oh!

karlfloersch left a comment

Uh oh!

karlfloersch May 21, 2020

Uh oh!

karlfloersch May 21, 2020

Uh oh!

willmeister May 21, 2020

Uh oh!

karlfloersch May 21, 2020

Uh oh!

karlfloersch May 21, 2020

Uh oh!

karlfloersch May 21, 2020

Uh oh!

karlfloersch May 21, 2020

Uh oh!

karlfloersch May 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

willmeister commented May 20, 2020

Uh oh!

karlfloersch left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants