This repository has been archived by the owner on Jul 5, 2024. It is now read-only.
WIP Remove nibbles #237
Merged
WIP Remove nibbles #237
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* add pc gadget * add pc constraint * fix * tab -> space * use new way to build op circuit * remove redundent assign * fix Co-authored-by: mask-pp <huan_zi266@sina.com> Co-authored-by: gaswhat <hs@scroll.tech>
* Introduce a global Context for bus-mapping
- Add Context type that can be accessed an mutated by
`gen_associated_ops` of each execution step which serves two purposes
- Maintain an execution state so that we can fill information about
the current Context in which the step was executed (for example, we
will add a Call context to handle reverts)
- Keep track of all the data required for the circuits (like the list
of Operations related to the Bus Mapping)
- Remove the global counter from each {StorageOp, MemoryOp, StackOp},
and create a generic wrapper type `Operation` with the global counter
and an op.
- In OperationContainer, separate each Operation Type into a different
vector, but keeping a single method for inserting Operations.
- Add a helper method in Context to push an Operation while assigning
and incrementing the GlobalCounter automatically. This simplifies
implementation of new bus-mapping opcodes.
- Introduce ProgramCounter and GlobalCounter `inc_pre` methods to
replace the `advance_pc` and `advance_gc` macros.
- Fix increment of the GlobalCounter per ExecutionStep (an ExecutionStep
will not increment the GlobalCounter, only [data] Operations will).
Resolve #128
Resolve #126
* Change `Context` by `TraceContext`
* Remove `NoOp` arm from `Target` enum
* Fix `OperationContainer` broken intra-doc links
Co-authored-by: CPerezz <c.perezbaro@gmail.com>
* Fix the test error for geth-utils on MacOS * fix the flag Co-authored-by: gaswhat <hs@scroll.tech>
Since we share a lot of code in Draft PRs with no purpose at all of running tests for them, it's a waste of time and resources to run the entire CI each time we push something to a Draft PR or we simply open one. Therefore, thanks to @han0110 who found that was possible to avoid this in the workflow config, it has been added. Resolves: #145
* split gas info into gas and gas cost * remove gas info reference
* Add EQ to comparator gadget
* implement jumpdest code * fix fmt * change format * mark _step Co-authored-by: Dream Wu <wwuwwei@126.com>
* udpate toolchain * fix ci lint stable: remove override and use toolchain
* fix operation container and write test * implement ADD opcode in bus-mapping * extend stack methods
* Refactor all opcodes to use constraint builder + other utils * Small misc improvements * minor import and doc improvements * Merge fixes
* Implement MLOAD/MSTORE opcodes * Feedback
- Introduce the CircuitInputBuilder, which allows building the circuit
inputs by taking types returned by geth and parsing them to generate
the appropiate outputs. The CircuitInputBuilder works by firs taking
a block and then transactions and their associated execution traces.
- The CircuitInputBuilder works by steps:
1. Take a block
2. Take each tx in the block
3. Take each exec trace in the tx
- The CircuitInpubBuilder in general is designed with 3 concepts that
apply to 3 levels (block, tx, exec step):
a. Input data: types obtained from geth (or in the future other
modules like the partial merkle tree).
b. context: data that must be available from one step to the other.
c. Output data: data generated from Input data + context, that forms
the circuit inputs.
- Previously there was a single type for ExecutionStep used both for
deserializing and for circuit input. Now the types are split into
eth_types (which contains all types that geth / web3 can return), and
internal structs in the CircuitInputBuilder which only contain the
inputs required by the circuit.
- Introduce the eth_types modules (mentioned before). Some of the types
are reexported from rust-web3, others not available in external crates
are defined.
- Introduce helper macros useful for tests to declare Address, Word and
Word->Word maps from hex strings.
- Introduce mock data generators for tests.
- Rename `EvmWord` to `Word`.
- Rename `ExecutionTrace` to `ExecTrace`.
- Rename `ExecutionStep` to `ExecStep`.
* add rho related checks * feedback: add reasons to panic * doc coef converter * revamp biguint_to_f * doc rho checks * randomly modify the test and catch a bug * add more docs to block counts * complete the block count doc
- Implement all the opcodes already implemented in the circuit, and some
more.
- Add convencience `map` methods to StackAddress and MemoryAddress.
- New implemented opcodes in bus-mapping:
- All unary opcodes: ISZERO, NOT.
- All binary opcodes: ADD, SUB, MUL, DIV, SDIV, MOD, SMOD,
SIGNEXTEND, LT, GT, SLT, SGT, EQ, AND, OR, XOR, BYTE, SHL, SHR,
SAR.
- All ternary opcodes: ADDMOD, MULMOD.
- MSTORE
- PC
- JUMPDEST
- PUSH2..32
- DUP1..16
- SWAP1..16
… bus-mapping. (#171) * Add ProviderError * Add rpc module * Add ethrs-providers to deps and url & tokio to dev-deps * Add `GethQueries` and getBlockByHash fn * Change `GethQueries` by `GethClient` * Add `eth_getBlockByNumber` support * Update `serde` version to `1.0.130`. * Impl `Serialize` for `GethExecStep` & `GethExecStepInternal` Since the `json_rpc` lib that we use requires the response types to be: `Serialize + Deserialize`, it has been needed to impl `Serialize` for the types that were lacking it. - GethExecStepInternal now derives `Serialize`. - Implemented `From<GethExecStep> for GethExecStepInternal` which is used in the next point. - Implement `Serialize` for `GethExecStep` serializing it as a `GethExecStepInternal` using the previous `From` impl mentioned. * Add `ResultGethExecTrace` & `ResultGethExecStep` Geth return value for `debug_traceBlockByHash` and `debug_traceBlockByNumber` isn't the expected. While the expected format was to get from these calls `Vec<GethExecTrace>`, you are given a similar vector but that contains a `result` Json field that makes no sense previous to each trace. Therefore it has been needed to simulate this as a new type and derive `Serialize` and `Deserialize` for it so that the response from this call can be parsed from the JSON and turned into usable data structures. * Add support for debug_traceBlockByHash/Number Following @ed255 suggestions the following extra JSON-RPC methods have been provided support: - debug_traceBlockByHash - debug_traceBlockByNumber Resolves: #23 * Fix clippy complaints * Fix review suggestions from @ed255 - Renames ResultGethExecTraces and ResultGethExecTrace. - Fix `From<GethExecStep` impl for `GethExecStepInternal`. Co-authored-by: Eduard S. <eduardsanou@posteo.net>
* use nightly * cargo fmt * clippy happy * rename to remove stable * lint more * handle unused fields in keccak * fix doc * specify exact nightly version
Implement error detection for EVM execution steps. This PR adds all the
possible errors that can happen during EVM execution, with some of the
error detection incomplete due to missing access to a state trie. List
of errors as declared by geth:
- Errors not reported
- [x] ErrInvalidJump
- [x] ErrReturnDataOutOfBounds
- [x] ErrExecutionReverted
- Errors ignored
- [x] ErrCodeStoreOutOfGas
- [x] ErrMaxCodeSizeExceeded
- [x] ErrInvalidCode
- [ ] ErrContractAddressCollision
- [ ] ErrInsufficientBalance
- [x] ErrDepth
- Errors reported
- [x] ErrWriteProtection
- [x] ErrOutOfGas
- [x] ErrGasUintOverflow
- [x] ErrStackUnderflow
- [x] ErrStackOverflow
- [x] ErrInvalidOpCode
Each error has a test that uses the external_tracer and produces the
error.
The two incomplete error detections are for
`ErrContractAddressCollision` and `ErrInsufficientBalance`. Detecting
these errors requires keeping track of the state, which we don't do yet.
Also, bump go-ethereum version in geth-utils.
* bitwise circuit * fix rebase error * rebase and fix lint and tests * fix the xor test Co-authored-by: gaswhat <hs@scroll.tech> Co-authored-by: Haichen Shen <shenhaichen@gmail.com>
* first draft jump circuit * fix operations lookup * load bytecode from executionstep * change comments * remove tag&push_rindex * remove assgin_byte_table method as unused now * add jumpi circuit & lookup selector * remove unused comment * make impl_op_gadget return vec of contraint and revert lookup selector feature * remove bytecode in bussmapping * use vec! to simplify Vec:: * remove reset_expression * update with latest main branch * fix fmt check * fix clippy * fmt resolve * fix ci & rebase long test time mitigation * use select expression * update to use dest cells intead of linear combination Co-authored-by: Dream Wu <wwuwwei@126.com>
Extend the types returned by the CircuitInputBuilder to match the requirements from the circuits. Rename CallContext to Call because the struct will be required for the circuits. Instead of keeping call metadata in a call stack, keep all call metadata in a vector, and use a call stack with indices to this vector. Extend Call, Transaction, ExecStep and Block with fields required by the circuits. Calculate addresses in bus-mapping. Integrate StateDB into CircuitInputBuilder and detect InsufficientBalance and ContractAddressCollision execution errors. Remove web3 dependency in favour of ethers-core for eth_types. Resolve #190 Resolve #187 Resolve #180 Resolve #172
Remove unecessary Serialize implementations for types used in rpc results Resolve #179
* enable rho check selectors * fix chunk rotate, 13->9 lookup, and block count check * docs and nitpicks
* replace with kate * fix clippy * update dev graph * igonore evm_word test * fix evm word * fix unexpected changes * update reference * test compilation success * fix from_u64 and fmt * fix test
…ally, nonce balance c row has been removed
miha-stopar
force-pushed
the
remove-nibbles
branch
from
ed9be08
to
85166bb
Compare
github-actions
bot
added
crate-bus-mapping
z2trillion
pushed a commit
that referenced
this pull request
Jan 11, 2023
* add precompile * update circuits * downgrade * fix clippy * constraint is_precompile * add todo * assert no fail * compress tests * calc gas_cost * clippy * fmt * add zero check * fix zero check * add negative test * fix return data * fix clippy * fix ci test * disable profile by default * fix merge Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
At first I thought having key nibbles as witnesses will make constraints on storage key and account address easier (because each byte in a key is split into two nibbles and nibbles determine how to navigate through the trie), but it seems now it brings too much overhead, like checking proper conversion between key nibbles and key bytes.
In this PR, key nibbles as witnesses are being removed and Key and Account address RLC are computed based on bytes instead of nibbles. This will also make easier to write the constraints for S and C proofs of different lengths (when value is added or deleted). In this case, key in S and C leaves is different and it needs to be checked that the difference is as it should be - for example, one nibble less when a branch or extension node replaces the leaf (but it turns out we don't need nibble witnesses to handle that).