-
Notifications
You must be signed in to change notification settings - Fork 44
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
WEB3-262: Add Selector, Receipt and Seal utilities #365
Conversation
contracts/src/groth16.rs
Outdated
let seal_bytes = seal.to_vec(); | ||
let (selector, stripped_seal) = seal_bytes.split_at(4); | ||
// Fake receipt seal is 32 bytes | ||
let receipt = if stripped_seal.len() == 32 { |
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.
A set inclusion seal with one path element will also be 32 bytes.
contracts/src/groth16.rs
Outdated
let receipt = if stripped_seal.len() == 32 { | ||
if selector != [0u8; 4] { | ||
return Err(anyhow::anyhow!( | ||
"Invalid selector {} for fake receipt", | ||
hex::encode(selector) | ||
)); | ||
}; | ||
Receipt::new( | ||
InnerReceipt::Fake(FakeReceipt::new(claim)), | ||
journal.as_ref().to_vec(), | ||
) | ||
} else { |
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.
I don't think this function should handle fake receipts. It's currently an odd compromise, where it handles Groth16 and Fake receipts, but not set inclusion receipts and it would not handle e.g. Plonk receipts if we add them later. We probably needs two types of decoding. One would be a method that decodes to a specific type (e.g. Groth16, Fake, or SetInclusion), and another that decodes any seal in a list of known types. For the later, we will need a table included in the library mapping selectors to verifier parameter (digests). At the very least, that table should include Groth16, SetInclusion and Fake verifier parameters for the current version. We could additionally include previous versions in the this table as well. We've discussed this elsewhere, and it seems likely to be useful.
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.
I like this a lot! Modulo my comments, I think we should merge this as is. I did mention there is some opportunity to consolidate with the upcoming work on composition, and I'll take care or that when I get back to composition next week. I think we should not wait to merge this.
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.
Nice!
Groth16 seal utility functions to easily convert an onchain seal into a risc0 Receipt --------- Co-authored-by: Victor Snyder-Graf <[email protected]>
Groth16 seal utility functions to easily convert an onchain seal into a risc0 Receipt