Explicit handling of public inputs #2
Conversation
| /// - the proof or the pubs are not serializable respectively as a `risc0_zkvm::InnerReceipt` and a `risc0_zkvm::Journal` | ||
| /// - the proof is not valid | ||
| pub fn verify(vk: Vk, proof: Proof, pubs: PublicInputs) -> Result<(), VerifyError> { | ||
| let receipt = deserialize_full_proof(proof, pubs)?; |
There was a problem hiding this comment.
I've prefered that you inlined deserialized_full_proof() code here because a deserialize function that take teo buffer is a strange and un expected interface.
Anyway I's just a mater of taste... it's not madatory
| receipt.verify(image_id.0).map_err(Into::into) | ||
| /// - the proof or the pubs are not serializable respectively as a `risc0_zkvm::InnerReceipt` and a `risc0_zkvm::Journal` | ||
| /// - the proof is not valid | ||
| pub fn verify(vk: Vk, proof: Proof, pubs: PublicInputs) -> Result<(), VerifyError> { |
There was a problem hiding this comment.
I think is better to take pubs as slice of bytes instead of use the serialized data.
tests/integration.rs
Outdated
| fn should_not_verify_invalid_pubs() { | ||
| let (vk, proof, mut pubs) = load_data(Path::new("./resources/valid_proof_1.json")); | ||
|
|
||
| pubs[0] = pubs.first().unwrap().wrapping_add(1); | ||
|
|
||
| assert!(matches!( | ||
| verify(vk.into(), &proof, &pubs), | ||
| Err(VerifyError::InvalidData { | ||
| cause: DeserializeError::InvalidPublicInputs | ||
| }) | ||
| )); | ||
| } |
There was a problem hiding this comment.
You're testing that your code is able to catch an inssu in the serialized inputs and not that the verification fails if your are using invalid pubs.
If you would to leave that your verify() method take serialized public input here you should deserialize them change the values and serialize them again... But (as you know) I prefer to change the interface.
There was a problem hiding this comment.
Ok, I wrote a wrong comment... Iwas bysed by the the test name: maybe a should_not_deserialize_invelid_pubs() is better.
There was a problem hiding this comment.
Ok renamed accordingly.
tests/integration.rs
Outdated
|
|
||
| assert!(verify(&proof_raw_data, image_id_data.into()).is_ok()); | ||
| assert!(verify(vk.into(), &proof, &pubs).is_ok()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn should_not_verify_invalid_proof() { |
There was a problem hiding this comment.
Also here: should_not_deserialize_invalid_proof() is a better name.
There was a problem hiding this comment.
Ok renamed accordingly.
src/proof.rs
Outdated
| /// The full proof (a serialized `risc0_zkvm::Receipt`) containing both proof and public inputs | ||
| pub type _FullProof<'a> = &'a [u8]; | ||
|
|
There was a problem hiding this comment.
Actually no, I'm removing it.
Now there is a neat separation: proof (inner receipt) and pubs (journal)
JackPiri
force-pushed
the
gp/public_inputs
branch
from
cb33bd6 to
19ee2d0
Compare
JackPiri
force-pushed
the
gp/public_inputs
branch
from
986af1b to
a07ead2
Compare
Added unit test
JackPiri
force-pushed
the
gp/public_inputs
branch
from
a07910a to
bbac31b
Compare
No description provided.