Compute and provide proof at client #24
Conversation
|
|
||
| # General | ||
| sha2 = { version = "0.10.8", default-features = false } | ||
| c-kzg = { version = "2.0.0", default-features = false } | ||
| anyhow = { version = "1.0.95", default-features = false } | ||
| thiserror = { version = "2.0.9", default-features = false } | ||
| rust-kzg-bn254 = { version = "0.2.1", default-features = false } | ||
| rust-kzg-bn254 = { git = "https://github.com/Layr-Labs/rust-kzg-bn254", rev = "4ad14ea4ce9473e13ed6437140fcbbff3a8ccce1", default-features = false } |
There was a problem hiding this comment.
We should talk with anup about publishing new versions of the rust-kzg-bn254 lib so we don't need to rely on git dependencies like this (they tend to cause more dependency problems in my experience)
There was a problem hiding this comment.
need to have all imporant issues resolved
bin/host/src/eigenda_fetcher/mod.rs
Outdated
| @@ -192,20 +192,20 @@ where | |||
| )?; | |||
| } | |||
|
|
|||
| // TODO proof is at the random point, but we need to figure out where to generate | |||
| // | |||
| // proof is at the random point | |||
There was a problem hiding this comment.
Do we need this comment? What does it mean? Is it talking about the length proof? Which random point is it evaluated at? Need more context here.
bin/host/src/eigenda_fetcher/mod.rs
Outdated
| // Write the KZG Proof as the last element, needed for ZK | ||
| //blob_key[88..].copy_from_slice((blob_length).to_be_bytes().as_ref()); | ||
| //let blob_key_hash = keccak256(blob_key.as_ref()); | ||
| blob_key[88..].copy_from_slice((blob_length).to_be_bytes().as_ref()); |
There was a problem hiding this comment.
Is this a KZG proof? Isn't blob_length just... a blob length? Should we be using the length_proof instead?
bin/client/src/witness.rs
Outdated
| eigenda_blobs: Vec::new(), | ||
| commitments: Vec::new(), | ||
| proofs: Vec::new(), |
There was a problem hiding this comment.
Do we have any idea ahead of time as to how many blobs we will be reading? Would be useful to preallocate some capacity here.
There was a problem hiding this comment.
no, a derivation pipeline could be arbitrarily long, and it is stack dependent
bin/client/src/witness.rs
Outdated
| // TODO we should have to specify the details to get a kzg to perform a verification | ||
| let kzg = match KZG::setup( | ||
| "resources/g1.32mb.point", | ||
| "", |
There was a problem hiding this comment.
what does this comment mean?
There was a problem hiding this comment.
we should not need so many g1 and g2 points for kzg verification
| let mut kzg = match KZG::setup( | ||
| "resources/g1.32mb.point", | ||
| "", | ||
| "resources/g2.point.powerOf2", |
There was a problem hiding this comment.
does this work? I thought the client doesn't have access to the fs? What if its running inside a zkvm..?
There was a problem hiding this comment.
this code only runs natively to fetch all witnesses you would need. For verification, the kzg library should just provide a helper function
| ) { | ||
| Ok(k) => k, | ||
| Err(_) => return Err(OracleProviderError::Preimage( | ||
| PreimageOracleError::Other("does not contain header".into(), | ||
| ))), | ||
| }; |
There was a problem hiding this comment.
Changing these everywhere to use something like
.map_err(|_| OracleProviderError::Preimage(
PreimageOracleError::Other("does not contain header".into())
))?
will be cleaner
| ))), | ||
| }; | ||
|
|
||
| let output = self.compute_and_save_witness(&b)?; |
There was a problem hiding this comment.
don't use generic names like "output". better to use self-documenting names, makes the code much more easily readable.
|
|
||
| // nitro code https://github.com/Layr-Labs/nitro/blob/14f09745b74321f91d1f702c3e7bb5eb7d0e49ce/arbitrator/prover/src/kzgbn254.rs#L30 | ||
| impl<T: CommsClient + Sync + Send> CachedOracleEigenDAProvider<T> { | ||
| fn compute_and_save_witness(&mut self, blob: &[u8]) -> Result<Vec<u8>, OracleProviderError> { |
There was a problem hiding this comment.
you are not saving the witness in this function. change name
| let commitment = Bytes::copy_from_slice( | ||
| &[cert_blob_info.blob_header.commitment.x, cert_blob_info.blob_header.commitment.y] | ||
| .concat()); | ||
|
|
||
| let kzg_proof = Bytes::copy_from_slice(&output[64..128]); |
There was a problem hiding this comment.
can't you use &output[..64] for the commitment given you've already verified it above?
|
Thanks for review @samlaf, but just intend to be a draft PR and walk you through it, didn't expect so much comments, but thx |
Implement a witness generation at custom client