-
Notifications
You must be signed in to change notification settings - Fork 0
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
Carlo/8 bits #10
base: main
Are you sure you want to change the base?
Carlo/8 bits #10
Conversation
CarloModicaPortfolio
commented
Jan 7, 2025
- 8 bits libs
- refactor to PrimeField32
- added extraction for multiset chek
@@ -14,7 +16,7 @@ use crate::{NUM_ROUNDS, RATE_LIMBS, U64_LIMBS}; | |||
/// convention of `x, y, z` order, but it has the benefit that input lists map to AIR columns in a | |||
/// nicer way. | |||
#[repr(C)] | |||
pub(crate) struct KeccakCols<T> { | |||
pub struct KeccakCols<T> { | |||
/// The `i`th value is set to 1 if we are in the `i`th round, otherwise 0. | |||
pub step_flags: [T; NUM_ROUNDS], | |||
|
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 could unify the postimage
columns with the a_prime_prime
columns, since these are never both used in the same row. They effectively serve the same purpose, and by unifying these, we can get rid of the .when(not_final_row)
conditions. Doing the latter may reduce the degree of the STARK from 4 to 3.
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.
Similarly, @CarloModicaPortfolio says we can unify step_flags[NUM_ROUNDS-1]
with export
.
@@ -168,5 +179,78 @@ impl<AB: AirBuilder> Air<AB> for KeccakAir { | |||
} | |||
} | |||
} |
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.
All of the below can go away when we unify the columns as discussed elsewhere in this review.