-
Notifications
You must be signed in to change notification settings - Fork 39
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
wip: add keccak #66
wip: add keccak #66
Conversation
// SHOULD it be u64instructions? | ||
L::Instruction: U32Instructions { | ||
// alloate 5*5 u64 register for storing 25 8byte lanes | ||
// state is 25 * 64 bit |
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.
This looks really good so far! I'm really impressed.
Here are some notes based on this reference: https://keccak.team/keccak_specs_summary.html#:~:text=Pseudo%2Dcode%20description%20of%20the%20permutations.
- The high level strategy, from my understanding, is to compute one round of Keccak in a single row in the STARK.
- You need to have a way to keep track of "i", the round index, to know which round constant to use.
- Thus, you should allocate 24 columns and constrain them such that they cycle from:
[1, 0, 0, ..., 0]
[0, 1, 0, ..., 0]
[0, 0, 1, ..., 0]
until it wraps around...
Let's call this set of 24 columns ROUND_CONSTANT_SELECTOR.
-
Now, to get the right round constant you should essentially compute a dot product between ROUND_CONSTANT_SELECTOR and ROUND_CONSTANTS which you can do by in a single constraint by allocating another column called SELECTED_ROUND_CONSTANT.
-
From that point on, your steps look mostly correct!
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.
Another option is to use a Bus like we do in SHA256, but since there are only 24 round constants John's suggestion might be similar in performance.
For now, I would not worry about constraining the round constants, write them directly to the trace and we can add these constraints in the end.
c_i = self.bitwise_xor(&c_i, &state.get(y * 5 + x), operations); | ||
} | ||
// Does it suffice to constrain for every row of c_i, it need to satisfy its relationship with state at the same row? | ||
self.assert_equal_transition(&c_arr.get(x), &c_i); |
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.
note that you're actually being a bit wasteful by allocating columns for both c_arr and the final c_i
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.
how to directly enforce a series of transition constraint on c_arr without the helper column?
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 overall!
operations: &mut ByteLookupOperations, | ||
)where | ||
// SHOULD it be u64instructions? | ||
L::Instruction: U32Instructions { |
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.
U32Instruction is actually OK since our U64 is implemented with u32 limbs. It might be a little confusing though, we might change the name.
// SHOULD it be u64instructions? | ||
L::Instruction: U32Instructions { | ||
// alloate 5*5 u64 register for storing 25 8byte lanes | ||
// state is 25 * 64 bit |
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.
Another option is to use a Bus like we do in SHA256, but since there are only 24 round constants John's suggestion might be similar in performance.
For now, I would not worry about constraining the round constants, write them directly to the trace and we can add these constraints in the end.
19b8eec
to
745d316
Compare
b893555
to
ff70694
Compare
let state_after_iota = self.alloc_array::<U64Register>(25); | ||
// iota | ||
let tmp = self.bitwise_xor(&state_after_chi.get(0), &round_constant, operations); | ||
self.set_to_expression(&state_after_iota.get(0), tmp.expr()); |
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.
Every time you assign a temp you are assigning a new column that is basically waisted. Instead you can use
self.set_bitwise_xor
to specify the result register.
No description provided.