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

Light client PoC #1644

Merged
merged 66 commits into from
Oct 25, 2023
Merged

Light client PoC #1644

merged 66 commits into from
Oct 25, 2023

Conversation

adria0
Copy link
Member

@adria0 adria0 commented Oct 5, 2023

Description

This PR merges the current milestone's light client PoC into main. Take into consideration:

  • The circuit is almost fished, although there are some small things that can be to be improved
  • There are some changes that maybe needs to be done in the go wrapper (like sending proofs results in the wrapper call, instead of go code to retrieve himself) that will requiere changes in the light client. This is the main cause to currently disable tests since it requieres to access to a RPC node. In a future changes will be mocked.
  • There's still pending some tests passing from a local geth node and mainnet blocks, when finished, the idea is to reuse some of the code for the MPT tests, to prove that a set of mainnet block state transitions can be successfully verified.

Issue Link

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

  • A circuit that verifies the state updates requiered to go from one state root to another
  • The witness generator for this circuit, that also, calls internally the mpt-witness-generator wrapper.
  • Tests to verify mainnet blocks
  • Test to verify a local geth node with eth_accessListByNumber RPC enabled
  • A draft of a server to create proofs and serve them. Note that this can be finished when local geth node tests will be passing.
  • Upstreamed MPT golang witness generator changes from miha branch

Copy link
Contributor

@rrtoledo rrtoledo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, mostly minor comments

light-client-poc/src/circuit/state_update.rs Outdated Show resolved Hide resolved
light-client-poc/src/circuit/state_update.rs Outdated Show resolved Hide resolved
light-client-poc/src/circuit/state_update.rs Outdated Show resolved Hide resolved
light-client-poc/src/circuit/state_update.rs Show resolved Hide resolved
light-client-poc/src/circuit/state_update.rs Outdated Show resolved Hide resolved
mpt-witness-generator/README.md Outdated Show resolved Hide resolved
mpt-witness-generator/README.md Outdated Show resolved Hide resolved
zkevm-circuits/src/mpt_circuit.rs Outdated Show resolved Hide resolved
zkevm-circuits/src/mpt_circuit/witness_row.rs Outdated Show resolved Hide resolved
light-client-poc/src/circuit/state_update.rs Outdated Show resolved Hide resolved
light-client-poc/src/circuit/state_update.rs Outdated Show resolved Hide resolved
.query_advice(pi_mpt.new_root.lo(), Rotation::cur())
- meta.query_advice(pi_mpt.old_root.lo(), Rotation::next()))
+ (meta.query_advice(pi_mpt.new_root.hi(), Rotation::cur())
- meta.query_advice(pi_mpt.old_root.hi(), Rotation::next()));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't be new_root.lo() - old_root.lo() != 0 and new_root.hi() - old_root.hi() != 0 and the whole expression still be 0 (having a + b = MOD)?

Copy link
Member Author

@adria0 adria0 Oct 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, it's little ugly to compare to hashes this way.
I put this in this way since I think that there's a really small chance to find two MPT transitions (lets say (lo0, hi0) and (lo1, hi1)) from the same root such the lo0+Δ=lo1 and hi0-Δ=h1.

It need to be changed or properly re-checked it if in any moment it is used in a non-PoC, for sure.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probability is small, but I think it would be still better to avoid having this possibility (same for new_root_propagation).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 58ba874

light-client-poc/src/circuit/state_update.rs Show resolved Hide resolved
light-client-poc/src/circuit/state_update.rs Outdated Show resolved Hide resolved
@adria0
Copy link
Member Author

adria0 commented Oct 13, 2023

Looks good, mostly minor comments

Thanks for the review!

Copy link
Collaborator

@miha-stopar miha-stopar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@adria0 adria0 enabled auto-merge October 25, 2023 15:59
@adria0 adria0 added this pull request to the merge queue Oct 25, 2023
Merged via the queue into main with commit c1ec848 Oct 25, 2023
13 checks passed
@adria0 adria0 deleted the zklight branch October 25, 2023 16:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI Issues related to the Continuous Integration mechanisms of the repository. crate-zkevm-circuits Issues related to the zkevm-circuits workspace member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add keccak circuit to zk light client PoC zk light client Circuit Proof of Concept
5 participants