Split aux tables based on size#504
Conversation
|
If I understand this correctly, the saving comes from replacing:
I wonder if we can apply this change for all cases, and we can move back to a single table. |
Redesign how public statements are handled. Previously public statements were a hashed list of consecutive statements. We had an area of the circuit to open all the public statements of input pods, and an area to copy statements to be public in the current pod. Now the public statements live in a tree of parametrizable depth. We remove the area of input pod public statements and the area of current pod public statements. Instead we introduce an operation to open a public statement from an input pod (which consumes a regular statement slot), which is needed when we want to use an input statement. And each statement slot can become a public statement of the current pod, with a limit on the total by a parameter field. Both the opening input pod statement and the making a statement public use tables, so the number of times such operations can be made is lower than the total number of statements. Moreover, a pod can start from an empty tree of statements and only output public statements generated in that pod, or it can extend the tree of statements from the first input pod, which allows us to carry statements forward in a recursive chain of pods at no cost. Aside from this change - I've removed the Copy operation because it's no longer needed. We used it previously to make a private or input statement public by copying it over the public area. But now any statement can be public. - Applied a bugfix I saw in #504 in `CustomPredicateVerifyQueryTarget::size` - Removed passing `sts_hash` in the serialize/deserialize operations, because that value is better derived from the list of statements for a simpler verification flow. - Renamed `self_statement` to `raw_statement` because we no longer have the concept of self in a pod (we only have it in a custom predicate module). The `raw_statement` is used to build the statement tree, and for introduction pods it has an empty placeholder instead of their verifier data hash that needs to be replaced by a normalization step. - Added a method to the Pod trait to get the public statement tree - The MainPodBuilder will automatically open an input pod statement if an operation depends on it and is not found in the previous statement. This makes the API behave like before. - Change how we distinguish MainPod from IntroPod. An IntroPod was previously detected by inspecting the first statement and looking for an intro statement with blank verifier data hash. Now this would require an opening, so I've extended the public inputs to include a field that signals when the pod is main. If the pod is main, it's verifier data hash must be in the VDSet; so no intro can lie about it. If it claims to be intro, when we open a statement we validate that it's indeed intro and place its verifier data hash inside.
e49804b to
fe21338
Compare
fe21338 to
adfb6be
Compare
ed255
left a comment
There was a problem hiding this comment.
Overall looks good! This is a great optimization which removes 4433 gates!
I've made two comments about consistency and simplification after the new change, please take a look.
| op_type, // output | ||
| op_args: entry.op_args.clone(), // input | ||
| }; | ||
| let query_hash = query.hash(builder); |
There was a problem hiding this comment.
All previous cases use a new method called hash_*_query but here we're still using the type *QueryTarget with its hash method. Could we just add a hash_custom_predicate_verify_query method so that it all looks uniform?
| let (aux_tag_ok, resolved_merkle_claim) = | ||
| aux.as_type::<MerkleClaimTarget>(builder, OperationAuxTableTag::MerkleProof as u32); | ||
| let (aux_tag_ok, resolved_query_hash) = | ||
| aux.as_type::<HashOutTarget>(builder, OperationAuxTableTag::MerkleProof as u32); |
There was a problem hiding this comment.
The TableEntryTarget is made generic so that you can unflatten a resolved entry into different types. But in this PR all the resolved table entries are HashOutTarget.
For this reason I propose changing the implementation of MuxTableTarget and TableEntryTarget so that instead of working with T: Flattenable and flattened_entry: &[Target] it works with plain HashOutTarget. This will simplify the code.
Then there's no need to keep the type TableEntryTarget, the result of MuxTableTarget::get can already be a HashOutTarget
adfb6be to
91d757e
Compare
The aux table holds supporting data for certain operations, including Merkle container and state transition ops,
SignedBy,PublicKeyOf, and custom predicate operations. It has a fixed row size, based on the largest possible value. Most aux table entries are quite small -MerkleTreeStateTransitionClaimTargetis 18 field elements, and all others are smaller. However,CustomPredicateVerifyQueryTargetis 314 field elements. Because of this outlier, the table rows are much larger than is needed for typical cases, making lookups more expensive than they need to be.This change stores only a query hash in the aux table, for every operation kind:
HashOutTarget, 4 field elements) instead of a full entry, so the row size is the same for every operation and no longer driven by theCustomPredicateVerifyQueryTargetoutlier — narrower rows, fewer Poseidon constraints per lookup.Contains/NotContains, state transition,OpenInputStatement,PublicKeyOf,SignedBy, and CustomPredVerify.verify_custom_circuithashes (statement, op_type, op_args)), instead of unhashing a wide row and doing flattened equality.The hashed input is prefixed with a domain-separation tag (
OperationAuxQueryKind), distinct from the row tag (OperationAuxTableTag): the row tag labels the physical table segment, while the query kind separates semantic shapes that share a row tag, such asContains/NotContainsor transition Insert / Update / Delete.