-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
continuations: Remove the need for dummy segments #245
Conversation
…segments, re-export dummy_proof in proof_gen and cleanup
plonky2 = { git = "https://github.com/0xPolygonZero/plonky2.git", rev = "dc77c77f2b06500e16ad4d7f1c2b057903602eed" } | ||
plonky2_maybe_rayon = "0.2.0" | ||
plonky2_util = "0.2.0" | ||
starky = "0.4.0" | ||
plonky2_util = { git = "https://github.com/0xPolygonZero/plonky2.git", rev = "dc77c77f2b06500e16ad4d7f1c2b057903602eed" } | ||
starky = { git = "https://github.com/0xPolygonZero/plonky2.git", rev = "dc77c77f2b06500e16ad4d7f1c2b057903602eed" } |
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.
We'll be doing a release this week normally, so can probably hold off until then and revert to using version tags
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.
In the end we didn't end up using the API changes on plonky2 right? With the new approach for dummy segment removal. Do we still need a version bump there? (It's not included in #276 as per discussion in the sync meeting).
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.
Sorry, I had missed this comment before: we still need methods such as conditional_assert_eq which are not on Plonky2 v.0.2.2.
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.
Yeah but we won't be bumping plonky2 yet (because of the STARK batching thing), so we can keep the main
reference / commit rev for the time being.
@@ -137,7 +136,7 @@ pub(crate) fn ctl_looking_memory<F: Field>(i: usize) -> Vec<Column<F>> { | |||
/// Returns the number of `KeccakSponge` tables looking into the `LogicStark`. | |||
pub(crate) const fn num_logic_ctls() -> usize { | |||
const U8S_PER_CTL: usize = 32; | |||
ceil_div_usize(KECCAK_RATE_BYTES, U8S_PER_CTL) | |||
KECCAK_RATE_BYTES.div_ceil(U8S_PER_CTL) |
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.
There'll be additional changes coming from latest plonky2 release, but we can probably do an upgrade on develop
which you can merge after FYI.
This PR removes the need for dummy segments. It supersedes #218.