-
Notifications
You must be signed in to change notification settings - Fork 858
Light client PoC #1644
Light client PoC #1644
Conversation
…lorations/zkevm-circuits into zklight
There was a problem hiding this 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
mpt-witness-generator/witness/gen_witness_from_infura_blockchain_test.go
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())); |
There was a problem hiding this comment.
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
)?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 58ba874
Thanks for the review! |
…circuits into zklight
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
Description
This PR merges the current milestone's light client PoC into main. Take into consideration:
Issue Link
Type of change
Contents
mpt-witness-generator
wrapper.eth_accessListByNumber
RPC enabled