Feat: add scroll patches #1

chokobole · 2022-11-23T05:47:28Z

I applied diffs to our geth which is on top of op-geth. The diffs are reflected up until scroll-tech/go-ethereum@ae2ab39. You can test it with the script named run_geth.sh.

The most important changes are followings.

SelfDestruct opcodes are disabled.
With zktrie options on, the code is hashed using not Keccak but Poseidon.

The things that are different from scroll are followings. They might be included in the future but not in this PR.

P2P are still enabled. See here.
Difficulty OP are not modified. See here
Optimize tx mempool. See here
Fix Miner bug (I haven't yet faced this situation) See here.
Improvement: I made unit test pass all but the tests that keeps failed originally. See here

Bignumber output from js runtime is represented in scientific notation. And custom test from scroll tries to compare in bigint. So if the output from js runtime doesn't match, it tries to compare results in bigint.

jyc228 · 2022-11-24T02:50:55Z

All of the commit titles are set to same title(feat ~~ add scroll) and cotexts are put on commit bodies.
please denote commit context with commit title.
also please check if all commits are categorized as 'feat'.
You have said that the reason of chaigning logger.~~ to vm.~~ was to prevent circular referenceing on scroll code.
To minimize the code modification, how about modifying scroll code rather than geth code?
It would be better to put default configuration on the function that creates database than modifying entire code to create database.

chokobole · 2022-11-24T06:07:59Z

@jyc228 I modified the title of commits and put the space between words to // [Scroll:BEGIN] and // [Scorll:END].

chokobole · 2022-11-30T01:53:00Z

Oh @jyc228 I forgot to say! I left issues about what you are concerned. I'll tackle down the issue after PoC. R again?

eth/tracers/api_blockresult.go

chokobole · 2022-12-02T00:40:47Z

@Pangssu By the way, when this commit is merged, the withdrawal should be changed to support poseidon hash!

core/evm.go

core/vm/evm.go

core/vm/jump_table.go

internal/debug/flags.go

trie/trie_test.go

eth/tracers/api_test.go

This is not meant to disable forever this opcode. Once zkevm spec can cover it, we need to re-enable it.

Added traces are followings. Followings are added to ExecutionResult. - From *AccountWrapper - To *AccountWrapper - AccountCreated *AccountWrapper - AccountsAfter []*AccountWrapper - CodeHash *common.Hash - ByteCode string Followings are added to StructLogRes. - ExtraData Because of cyclic dependency, tracer codes are moved to core/vm.

Added `run_geth.sh` helps to test geth with zktrie or not. ```shell $ ZK=1 sh run_geth.sh # with zk trie $ sh run_geth.sh # without zk trie ```

Pangssu

LGTM

This commit squashes the op-geth fork history into a more maintainable diff for rebasing upon upstream geth. reference-optimistic-geth changes (origins of op-geth in early Bedrock development stage): - Deposit TX Type - Enable deposit tx in EVM/tx pool - Change deposit nonce to not be the max nonce - Extend PayloadAttributesV1 with a Transactions field - Force deposits at the start of each L2 block - Fix height check - noTxPool flag, reproduce block in verifier mode without tx pool interference - Fix RPC json marshalling (ref op-geth PR 4) - Deposit txs block height check in block body validation (ref op-geth PR 5) - core: do not try to reinject deposit txs into tx-pool (ref-op-geth PR 6) - deposit source hash field instead of L2 block height and tx index combination - Include invalid deposits, rewind state, but always persist mint (#10) - Provide gas to Call/Create in deposit transactions (#12) - Add docker builds (ref-op-geth PR 16, 17) - Don't panic on deposit transaction signature values or chain ID (ref-op-geth PR 18) - core: Add version to DepositTx (ref-op-geth PR 19) - Enable Geth build/lint/test in CircleCI (ref-op-geth PR 23) - core: Include guaranteed gas in the gas pool (ref-op-geth PR 21) - core: handle base fee, l1 availability fee, tx fee (ref-op-geth PR 27) - fix: deposit tx hash - fix l1 fee cache, rpc, tracing and tx pool - core: remove deposit-tx sub-type (a.k.a. deposit version byte) - eth/catalyst: allow engine user to reorg own chain - miner: restore ability to reorg deep as block builder - params: print Optimism consensus type in banner - core/types: remove unused protected() method, see upstream PR 23376 - core: do not mutate original balance value in tx pool l1 cost adjustment - core: subtract deposit gas from pool, so other txs do not use the same gas. And fail tx processing if deposits reach gas limit - core/types: deposits do not tip, avoid basefee subtraction - Unmeter the L1 Attributes Transaction - miner: handle force tx errors as critical, clean up diff - ci: Switch branch - eth,miner: return STATUS_INVALID when failing to process forced transactions in request (ref-op-geth PR 40) - verifier: forward tx to sequencer based on flag - txpool: add flag to disable tx gossip (ref-op-geth PR 42) - Add op-geth version in addition to geth version (ref-op-geth PR 43) - ci: CircleCI improvements (ref-op-geth PR 44) - Rename to op-geth - Build latest tag on optimism branch op-geth changes: - Expose cache config in simulated backend (#2) - Add EIP-1559 parameters - eth/catalyst: update payload id computation (#1) - make eip1559 configurable (#4) - post-merge network should not log warnings about missing transition information (#5) - Make the simulator more configurable (#6) - fix OPB-6 - IsDepositTx check instead of artificial nonce value check (#7) - Simulated backend - enable proof of stake consensus type and fix performance issue (#8) - accounts: simulated backend consensus engine option and immediate tx indexing - consensus/beacon: recognize all blocks as reached TTD with 0 TTD in chain config - Add --rollup.historicalhttp CLI flag and fix backend iface - Flags and interfaces for historical RPC requests (#12) - Redirect historical RPC requests (#13) - Use the pre-existing ethereum.NotFound error (#18) - Add historical endpoint to TraceBlockByNumber and TraceBlockByHash (#19) - Add historical endpoint to TraceTransaction (#20) - Add historical endpoint to TraceCall (#21) - optimism: fee params from info txi, update l1 cost func GPO params read (#15) - add hardcoded addresses for fee payouts (#23) - dynamic gas limit via engine API (#22) Co-authored-by: Matthew Slipper <me@matthewslipper.com> Co-authored-by: Joshua Gutow <jgutow@oplabs.co> Co-authored-by: protolambda <proto@protolambda.com> Co-authored-by: Mark Tyneway <mark.tyneway@gmail.com> Co-authored-by: Maurelian <maurelian@protonmail.ch>

This commit squashes the kanvas-geth fork history into a more maintainable diff for rebasing upon upstream geth. See https://github.com/wemixkanvas/go-ethereum/commits/op-geth-base for more details about how op-geth changes geth code. kanvas-geth changes: - Merge pull request #17 from light-scale/fix/change-contract-address - fix: change predeployed contract address - Merge pull request #15 from light-scale/ci/lint - Merge pull request #16 from light-scale/ci/pipeline - ci: add ci pipeline and test pipeline - ci: add worklows to run lint on push - ci: change the style of the code so that linting succeeds - Merge pull request #14 from light-scale/ci/test-disable - disable unittest temporarily for CI introduction - Merge pull request #13 from light-scale/feat/enable-tx-pool-sync-by-force - feat: enable tx pool sync by force - Merge pull request #11 from light-scale/feat/remove-daisy-chain - feat: remove daisy chain - Merge pull request #10 from light-scale/feat/set-pending-block-gas-limit - Merge pull request #12 from light-scale/zktrie - chore: upgrade zktrie library - feat: enable setting pending block gas limit - chore: rename optimism to kanvas - ci: remove circle ci config - Merge pull request #8 from light-scale/feat/add-fee-recipients-to-proof - feat(core, eth): add Optimism{Base,L1}FeeRecipient to block trace - feat: add Mint to TransactionTrace - Merge pull request #5 from light-scale/feat/add-L1CostFunc-to-EVM-BlockContext - fix(cmd): list ipcAPIs on alphabetical order - fix(eth): add L1CostFunc to evm's block context - refac(light): remove unneeded member variable - perf(trie): disable preimage by default - chore: rename voost to kanvas - Merge pull request #3 from light-scale/feat/enable-use-keccak-for-codehash - feat: use poseidon as code hash only with zktrie - fix(cmd, core): load database with Zktrie when possible - fix: fix segfault when referencing to Database - fix(common, signer): make unit test passed We wrapped code with comments like below to separate code change for scroll-geth from op-geth. ```go // [Scroll: START] // changes for scroll-geth // [Scroll: END] ``` scroll-geth changes: - Merge pull request #1 from light-scale/feat/add-scroll-patches - chore: add run_geth.sh - feat: add GetBlockResultByNumberOrHash rpc - feat: add CacheConfig to Backend interface - feat(core): add some methods to StateDB interface - feat: add MPTWitness to CacheConfig - feat: use zktrie for state trie - feat: apply poseidon to codehash - feat(core): add ReturnValue to Receipt - feat: add detailed contents to traces for zkevm - feat(core/vm, eth/tracers): add CaptureStateAfter hook - feat(core/vm): disable SelfDestruct opcode - test(cmd, internal): add custom genesis test

chokobole requested review from Pangssu and lightscale-luke November 23, 2022 05:47

chokobole changed the base branch from optimism to dev November 23, 2022 05:51

chokobole requested a review from jyc228 November 23, 2022 06:14

test(cmd, internal): add custom genesis test

0c6e75c

Bignumber output from js runtime is represented in scientific notation. And custom test from scroll tries to compare in bigint. So if the output from js runtime doesn't match, it tries to compare results in bigint.

chokobole force-pushed the feat/add-scroll-patches branch 3 times, most recently from 1590f68 to fdf02a8 Compare November 24, 2022 05:56

chokobole mentioned this pull request Nov 24, 2022

Make scroll geth diff maintainable #2

Closed

chokobole force-pushed the feat/add-scroll-patches branch from fdf02a8 to 866ac11 Compare November 24, 2022 09:34

Pangssu reviewed Nov 30, 2022

View reviewed changes

eth/tracers/api_blockresult.go Show resolved Hide resolved

Pangssu self-requested a review November 30, 2022 07:58

Pangssu reviewed Dec 3, 2022

View reviewed changes

core/evm.go Show resolved Hide resolved

core/vm/evm.go Show resolved Hide resolved

core/vm/jump_table.go Outdated Show resolved Hide resolved

internal/debug/flags.go Show resolved Hide resolved

trie/trie_test.go Show resolved Hide resolved

eth/tracers/api_test.go Show resolved Hide resolved

Pangssu self-requested a review December 3, 2022 15:24

chokobole force-pushed the feat/add-scroll-patches branch from 866ac11 to 06d9c56 Compare December 4, 2022 13:05

chokobole added 11 commits December 4, 2022 22:06

feat(core/vm): disable SelfDestruct opcode

b7dd71e

This is not meant to disable forever this opcode. Once zkevm spec can cover it, we need to re-enable it.

feat(core/vm, eth/tracers): add CaptureStateAfter hook

abca1f3

feat(core): add ReturnValue to Receipt

106e034

feat: apply poseidon to codehash

2ec3d63

feat: use zktrie for state trie

4f89628

feat: add MPTWitness to CacheConfig

6cda8c9

feat(core): add some methods to StateDB interface

1fccd49

feat: add CacheConfig to Backend interface

6641a6b

feat: add GetBlockResultByNumberOrHash rpc

31a0e72

chore: add run_geth.sh

ffda45c

Added `run_geth.sh` helps to test geth with zktrie or not. ```shell $ ZK=1 sh run_geth.sh # with zk trie $ sh run_geth.sh # without zk trie ```

chokobole force-pushed the feat/add-scroll-patches branch from 06d9c56 to ffda45c Compare December 4, 2022 13:06

Pangssu approved these changes Dec 4, 2022

View reviewed changes

Pangssu self-requested a review December 4, 2022 13:40

chokobole merged commit 3b0d4c3 into dev Dec 4, 2022

chokobole deleted the feat/add-scroll-patches branch December 4, 2022 13:57

chokobole mentioned this pull request Dec 7, 2022

Feat: enable use keccak for codehash #3

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: add scroll patches #1

Feat: add scroll patches #1

chokobole commented Nov 23, 2022 •

edited

Loading

jyc228 commented Nov 24, 2022 •

edited by lightscale-luke

Loading

chokobole commented Nov 24, 2022

chokobole commented Nov 30, 2022

chokobole commented Dec 2, 2022

Pangssu left a comment

Feat: add scroll patches #1

Feat: add scroll patches #1

Conversation

chokobole commented Nov 23, 2022 • edited Loading

jyc228 commented Nov 24, 2022 • edited by lightscale-luke Loading

chokobole commented Nov 24, 2022

chokobole commented Nov 30, 2022

chokobole commented Dec 2, 2022

Pangssu left a comment

Choose a reason for hiding this comment

chokobole commented Nov 23, 2022 •

edited

Loading

jyc228 commented Nov 24, 2022 •

edited by lightscale-luke

Loading