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

In-circuit blockhash calculation #98

Closed
wants to merge 49 commits into from
Closed

In-circuit blockhash calculation #98

wants to merge 49 commits into from

Conversation

ggkitsas
Copy link

@ggkitsas ggkitsas commented May 25, 2023

MOVED TO #129

### Description

In-circuit blockhash calculation. Design documentation can be found here: https://www.notion.so/taikoxyz/In-circuit-blockhash-calculation-docs-069984cbc14446a9a1ea88387178e9ce

### Issue Link

[https://github.com//issues/79]

### Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] 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

Copy link

@Brechtpd Brechtpd left a comment

Choose a reason for hiding this comment

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

Went quickly over some of the code, very superficial feedback for now.

Would be great if you could share whatever design documents you have for this circuit as well because that'll make it easier to understand and check against the code.

zkevm-circuits/src/pi_circuit2.rs Outdated Show resolved Hide resolved
zkevm-circuits/src/pi_circuit2.rs Outdated Show resolved Hide resolved
zkevm-circuits/src/pi_circuit2.rs Outdated Show resolved Hide resolved
@ggkitsas ggkitsas requested a review from Brechtpd June 27, 2023 20:54
Copy link

@Brechtpd Brechtpd left a comment

Choose a reason for hiding this comment

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

I think good to go to merge in the master in this branch (or apply your circuits on top of the master branch in another branch)!

That'll also make the reviewing a bit easier because now some of the code is shared with the old public input circuit. If all the anchor specific stuff is in its own sub-circuit it'll be easier to just focus on that. Of course also okay if you first want to do the 256 block hashes and then do the update to main.

zkevm-circuits/src/pi_circuit2.rs Outdated Show resolved Hide resolved
@ggkitsas
Copy link
Author

I think good to go to merge in the master in this branch (or apply your circuits on top of the master branch in another branch)!

That'll also make the reviewing a bit easier because now some of the code is shared with the old public input circuit. If all the anchor specific stuff is in its own sub-circuit it'll be easier to just focus on that. Of course also okay if you first want to do the 256 block hashes and then do the update to main.

I will merge in one go when finished with the previous hashes work. I'll add this work in this PR

@ggkitsas ggkitsas force-pushed the blockhash branch 5 times, most recently from 9a6939c to 059d926 Compare July 14, 2023 18:57
@ggkitsas ggkitsas requested a review from Brechtpd July 14, 2023 19:02
@ggkitsas
Copy link
Author

added solution for #107

@ggkitsas
Copy link
Author

MOVED TO #129

@ggkitsas ggkitsas closed this Jul 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants