Skip to content
This repository has been archived by the owner on Jul 5, 2024. It is now read-only.

Inserted extension node #1548

Closed
wants to merge 304 commits into from
Closed

Inserted extension node #1548

wants to merge 304 commits into from

Conversation

miha-stopar
Copy link
Collaborator

Description

Support for modified extension nodes.

This happens when an extension node is replaced by another (shorter or longer) extension node. For example, we have an extension node at n1 n2 n3 n4 n5 n6 with branch with two leaves at positions n and m. If we add a leaf at n1 n2 n3 n4 n7 where n5 != n7, a new extension node is inserted at n1 n2 n3 n4 with a new branch with an extension node at position n5 (with only one nibble n6) and a leaf at position n7. In this case, the S proof contains the extension node at n1 n2 n3 n4 n5 n6 and no underlying branch and leaf (the modification happens at n1 n2 n3 n4 n7 and only an extension node is find there), thus we need to add a placeholder branch and a placeholder leaf.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

miha-stopar and others added 18 commits October 27, 2022 16:23
Some refactoring which I believe decreases code duplication and
increases code readability.

Some TODOs:
- [x] The code required to get the previous rlc/mult data or the
inclusion in the parent check is quite complicated because of the
different node types. Instead it may be better to use a fixed location
to store this data in so a node can simply use this data directly
instead of having to figure out on its own where to find the data. This
is cleaner because this way each node can decide on its own how these
should be handled.
- [x] Currently RLP decoding is done using a couple of selectors that
are inputs from the prover. Then there are some checks if these are
correct, though these are not complete. I think it's easier to think
about this if we would just use a lookup to directly verify if these
selectors are set correctly so we don't have to worry about edge cases
are cases that are hard to constrain using custom gates.
- [x] There are currently many cases in the main state maching because
each row type is it's own state. However there is not really any reuse
between custom gates between these rows, except for branches. It'll
likely be quite a bit simple to just have a single state for account,
storage and extension and just use multiple rows in those states
- [x] May be a good idea to split up branches and extension nodes. (semi
done)
- [ ] There's a couple of circuit tools that were added in this PR to
make writing the MPT circuit more manageable. A lot of these tools can
still be greatly improved.
- [ ] Number of lookups has been reduced a lot, but there are still many
optimization possibilities (have not looked into reducing the expression
degree for example).
- [x] The circuit uses a fixed layout which uses around 100 columns (of
which a lot need to be byte constrained so needs a lot of lookups as
well). This makes the circuit quite a bit more dense than probably
required, a more flexible way to manager the required data so the
width/height can be choses would be very useful I think.
### Description

This PR simply merges the current `main` into the MPT branch `mpt2`.
This is the first step to get some further MPT changes merged into the
MPT branch without making the PRs too big.

```
This PR contains:
- Merge `main` into `mpt2` with the necessary changes to the MPT circuit
```

### Issue Link

[_link issue here_]

### Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update

### Contents

- [_item_]

### Rationale

[_design decisions and extended information_]

### How Has This Been Tested?

Intermediate step, so limited testing

---------

Signed-off-by: smtmfft <[email protected]>
Signed-off-by: smtmfft <[email protected]>
Co-authored-by: pinkiebell <[email protected]>
Co-authored-by: Haichen Shen <[email protected]>
Co-authored-by: z2trillion <[email protected]>
Co-authored-by: Eduard S <[email protected]>
Co-authored-by: adria0.eth <[email protected]>
Co-authored-by: Han <[email protected]>
Co-authored-by: smtmfft <[email protected]>
Co-authored-by: smtmfft <[email protected]>
Co-authored-by: Chih Cheng Liang <[email protected]>
Co-authored-by: DreamWuGit <[email protected]>
Co-authored-by: Rohit Narurkar <[email protected]>
Co-authored-by: Zhang Zhuo <[email protected]>
Co-authored-by: Ming <[email protected]>
Co-authored-by: Wu Sung Ming <[email protected]>
Co-authored-by: xiaodino <[email protected]>
Co-authored-by: Mason Liang <[email protected]>
Co-authored-by: Steven <[email protected]>
Co-authored-by: Carlos Pérez <[email protected]>
Co-authored-by: Paul <[email protected]>
Co-authored-by: AronisAt79 <[email protected]>
Co-authored-by: Mason Liang <[email protected]>
Co-authored-by: JohnWick2ETH <[email protected]>
Co-authored-by: kunxian xia <[email protected]>
Co-authored-by: Leo Lara <[email protected]>
Co-authored-by: zengzengzenghuy <[email protected]>
Co-authored-by: john xu <[email protected]>
Co-authored-by: Miha Stopar <[email protected]>
Co-authored-by: aguzmant103 <[email protected]>
Co-authored-by: Richord <[email protected]>
Co-authored-by: Kimi Wu <[email protected]>
Co-authored-by: Mickey <[email protected]>
Co-authored-by: Luozhu <[email protected]>
Co-authored-by: DoHoon Kim <[email protected]>
Co-authored-by: jeong0982 <[email protected]>
Co-authored-by: Andy Arditi <[email protected]>
Co-authored-by: David Nevado <[email protected]>
Co-authored-by: omahs <[email protected]>
Co-authored-by: Richord <[email protected]>
Co-authored-by: ntampakas <[email protected]>
Co-authored-by: ashWhiteHat <[email protected]>
Co-authored-by: naure <[email protected]>
Co-authored-by: Aurélien Nicolas <[email protected]>
Co-authored-by: testuser-at-490752553772 <[email protected]>
Co-authored-by: xiaodino <[email protected]>
Co-authored-by: Thomas Pan <[email protected]>
Co-authored-by: Akase Cho <[email protected]>
Co-authored-by: Vu <[email protected]>
Co-authored-by: Alex Beregszaszi <[email protected]>
Co-authored-by: Enrico Bottazzi <[email protected]>
Co-authored-by: adria0 <adria0@nowhere>
# Conflicts:
#	Cargo.lock
#	circuit-benchmarks/Cargo.toml
#	eth-types/src/geth_types.rs
#	gadgets/src/util.rs
#	keccak256/Cargo.toml
#	keccak256/src/arith_helpers.rs
#	keccak256/src/keccak_arith.rs
#	keccak256/src/lib.rs
#	keccak256/src/plain.rs
#	testool/src/compiler.rs
#	zkevm-circuits/Cargo.toml
#	zkevm-circuits/src/evm_circuit.rs
#	zkevm-circuits/src/pi_circuit.rs
#	zkevm-circuits/src/state_circuit.rs
#	zkevm-circuits/src/state_circuit/constraint_builder.rs
#	zkevm-circuits/src/table.rs
#	zkevm-circuits/src/table/keccak_table.rs
#	zkevm-circuits/src/table/mpt_table.rs
#	zkevm-circuits/src/tx_circuit/dev.rs
#	zkevm-circuits/src/tx_circuit/sign_verify.rs
#	zkevm-circuits/src/util.rs
#	zkevm-circuits/src/witness/mpt.rs
### Description

- Added some missing MPT constraints
- Integrated some EVM tools like degree reduction and lookups using
dedicated columns

### Issue Link

[_link issue here_]

### Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update

### Contents

- [_item_]

### Rationale

[_design decisions and extended information_]

### How Has This Been Tested?

[_explanation_]

---------

Co-authored-by: Cecilia Zhang <[email protected]>
Co-authored-by: CeciliaZ030 <[email protected]>
### Description

New descriptors are used which have `Vec<Node>` structure and thus the
conversion in `witness_row.rs` from `[][]byte` to `Vec<Node>` was
removed. Note that `keccak_data` is now part of the `Node` struct - each
node holds the byte streams that need to be hashed.

Additionally, an MPT benchmark was added (some structs needed to be made
public).

### Type of change

- [x] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [x] This change requires a documentation update

### Contents

- All tests updated.
- `prepare_witness` function removed from `witness_row.rs`.
- MPT test updated to not use `prepare_witness`.
- Benchmark added.
@github-actions github-actions bot added the crate-zkevm-circuits Issues related to the zkevm-circuits workspace member label Aug 1, 2023
@miha-stopar miha-stopar marked this pull request as draft August 1, 2023 14:58
Base automatically changed from mpt2 to main August 10, 2023 11:27
@github-actions github-actions bot added T-bench Type: benchmark improvements crate-circuit-benchmarks Issues related to the circuit-benchmarks workspace member crate-gadgets Issues related to the gadgets workspace member labels Aug 10, 2023
@miha-stopar
Copy link
Collaborator Author

Closed in favour of #1563. I am starting the modified extension node almost from scratch, so it was the easiest just to open a new branch (using the main which has now MPT merged).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
crate-circuit-benchmarks Issues related to the circuit-benchmarks workspace member crate-gadgets Issues related to the gadgets workspace member crate-zkevm-circuits Issues related to the zkevm-circuits workspace member T-bench Type: benchmark improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants