-
Notifications
You must be signed in to change notification settings - Fork 26
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
Feat: Native Poseidon2 chip #1219
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
||
| Name | Operands | Description | | ||
|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-----------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | ||
| **VERIFY_BATCH** `[CHUNK, PID]` <br/><br/> Here `CHUNK` and `PID` are **constants** that determine different opcodes. `PID` is an internal identifier for particular Poseidon2 constants dependent on the field (see below). | `a,b,c,d,e,f,g` | Computes `mmcs::verify_batch`. In the native address space, `[a], [b], [d], [e], [f]` should be the array start pointers for the dimensions array, the opened values array (which contains more arrays), the proof (which contains arrays of length `CHUNK`) and the commitment (which is an array of length `CHUNK`). `[c]` should be the length of the opened values array (and so should be equal to the length of the dimensions array as well). `g` should be the reciprocal of the size (in field elements) of the values contained in the opened values array: if the opened values array contains field elements, `g` should be 1; if the opened values array contains extension field elements, `g` should be 1/4. | |
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 explanation is probably not going to be that helpful, if we could point to a rust function or a more detailed writeup, that would be better.
let aux_cols_factory = memory.aux_cols_factory(); | ||
|
||
let mut used_cells = 0; | ||
for record in self.record_set.verify_batch_records.iter() { |
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.
could this be parallelized?
This comment has been minimized.
This comment has been minimized.
I’m not sure where but we are logging “generate Poseidon2 trace” 60K times for an execution of a few microseconds. We should probably remove this. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
Merging to unblock other PRs and because I roughly get the idea.
The following should be done in follow-up:
- Docs (ideally markdown) explaining the constraints in more detail than just the PR description.
- For example, the top rows are all before the internal rows? The merkle proof compress rows are all adjacent? How exactly are PERM and COMP handled?
- It'd be nice if in the Air code there was more code-level separation (maybe functions) to clarify the big blocks that are only ever enabled for top-level (resp. internal).
- I had a comment about parallelizng trace gen as much as possible, although currently from benchmarks it doesn't seem too much of a concern.
It's from plonky3. fixed in openvm-org/stark-backend#27 |
Commit: 3c5c4a3 |
#1133, INT-2947
Adds a chip to complete all of
verify_batch
in a single instruction. This is meant to replace the currentNativePoseidon2Chip
. Associated with this, this PR adds a new OpCode (currently calledVERIFY_BATCH
) and two newAsmInstruction
andDslIr
variants, one each for the two cases where the opened values areFelt
s andExt
s.The instruction has 7 operands: dim, opened, opened_len, sibling, index, commit, opened_value_size_inv. dim, opened, sibling, index, commit are the locations of the pointers for the respective arrays; opened_len is the location of the length of the opened values array, and opened_value_size_inv is 1 when the opened values are
Felt
s and 1/4 when the opened values areExt
s.The chip itself works by having two types of rows: top level, and inside row. Top level rows are responsible for merging in the Merkle tree, while inside row rows are responsible for performing the rolling hash of the concatenated rows. The last top level row is responsible for the execution interaction. There are two types of top level rows: incorporate row, which are responsible for merging in the hash of the concatenated rows, and incorporate sibling, which are responsible for merging in the siblings. Incorporate row rows send an interaction to the last inside row row in each segment of inside row rows.
Copilot:
This pull request focuses on removing the Poseidon2 opcode and related code, and introducing a new
VERIFY_BATCH
opcode. Additionally, it includes some refactoring and the addition of new methods. Here are the most important changes:Opcode Changes:
opcode_offset
forPublishOpcode
from0x120
to0x020
incrates/toolchain/instructions/src/lib.rs
. RemovedPoseidon2Opcode
and related code.VERIFY_BATCH
opcode tailored for optimized verification indocs/specs/ISA.md
.Code Removal:
Poseidon2Opcode
incrates/vm/tests/integration_test.rs
and related test functions. [1] [2] [3] [4]Refactoring:
NativePoseidon2Cols
struct and added new structsTopLevelSpecificCols
,InsideRowSpecificCols
, andSimplePoseidonSpecificCols
inextensions/native/circuit/src/poseidon2/columns.rs
.New Methods:
write_usize
inVmChipTestBuilder
incrates/vm/src/arch/testing/mod.rs
.Dependency and Import Changes:
Poseidon2Opcode
and added imports forVerifyBatchOpcode
inextensions/native/circuit/src/extension.rs
. [1] [2] [3] [4]