[Keccak] Padding Validation #445
Conversation
github-actions
bot
added
the
crate-keccak
ChihChengLiang
force-pushed
the
padding-validation
branch
2 times, most recently
from
298925c
to
ec8f05b
Compare
ChihChengLiang
force-pushed
the
padding-validation
branch
3 times, most recently
from
4695ebc
to
a3e477b
Compare
ChihChengLiang
changed the title
There was a problem hiding this comment.
Some concerns regarding the usage of Selector for the various cases we have in the padding.
The rest looks good. But will do a second review round once the Selector ussage issue is resolved.
Nice job with that BTW @ChihChengLiang 🎉
| let q_enable = meta.selector(); | ||
| meta.create_gate("build word", |meta| { | ||
| let q_enable = meta.query_selector(q_enable); | ||
| let byte = meta.query_advice(byte, Rotation::cur()); |
There was a problem hiding this comment.
Can't we reduce 1 column here by using Rotation::next?
There was a problem hiding this comment.
So that in word we have:
- Prev: word_prev
- Cur: word_cur
- Next: byte
There was a problem hiding this comment.
We are using the byte column from PaddingConfig. We don't create a new column for byte.
The gate looks like this:
| byte (from PaddingConfig) | word |
|---|---|
| byte1 | byte1 |
| byte2 | 256 * byte1 + byte2 |
| ... | ... |
| byte8 | 256^7 *byte1 + 256^6 *byte2 + ... + byte8 |
In your proposed way, what would the gate look like?
There was a problem hiding this comment.
I meant that in the Rotation::next of word column we can add the byte one.
But anyway, I don;t think this matters much now. Let's just merge and focus on finishing the entire thing. And then we can move on into optimizations or details.
ChihChengLiang
force-pushed
the
padding-validation
branch
2 times, most recently
from
4990abe
to
bbe0543
Compare
There was a problem hiding this comment.
LGTM!
Let's file an issue for the Byte RLC which is commented out as missing, and merge this! :)
Nice job @ChihChengLiang 🎉
ChihChengLiang
force-pushed
the
padding-validation
branch
from
bbe0543
to
4cc368c
Compare
Problem
Adding the input padding validation to keccak
Spec privacy-scaling-explorations/zkevm-specs#161