-
Notifications
You must be signed in to change notification settings - Fork 1
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(eigen-client-extra-features): Verifier #326
feat(eigen-client-extra-features): Verifier #326
Conversation
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.
LGTM. Be sure to remove all unused imports before merging!
pub path_to_points: String, | ||
} | ||
|
||
#[derive(Debug, Clone)] |
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.
can you add some docs to this struct?
}) | ||
} | ||
|
||
fn commit(&self, blob: Vec<u8>) -> Result<G1Affine, VerificationError> { |
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.
Add docs
Ok(()) | ||
} | ||
|
||
pub async fn verify_certificate(&self, cert: BlobInfo) -> Result<(), VerificationError> { |
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.
add docs
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.
Add docs
Done |
let blob_info = blob_info::BlobInfo::try_from(blob_info) | ||
.map_err(|e| anyhow::anyhow!("Failed to convert blob info: {}", e))?; | ||
self.verifier | ||
.verify_commitment(blob_info.blob_header.commitment.clone(), data) |
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.
In this code, I was looking at this file. Where does eigenda fit in the match (self.mode, self.pubdata_da)
result ?
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.
To clarify, is this computing the commitment locally to see if the DA has computed the same commitment ?
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.
We fit in (L1BatchCommitmentMode::Validium, PubdataSendingMode::Custom)
The verifier.verifyCommitment
function is the one that generates a commitment based on the blob and compares it to the one given by EigenDA.
The code you were looking at is the one that defines what would be sent to the L1, once we make the PR with full M0, that part will send the EigenDA commitment.
&format!("{}{}", cfg.path_to_points, "/g1.point"), | ||
"", | ||
&format!("{}{}", cfg.path_to_points, "/g2.point.powerOf2"), | ||
268435456, // 2 ^ 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.
small correction, its 2 ^ 28
let expected_commitment = G1Affine::new_unchecked( | ||
Fq::from(num_bigint::BigUint::from_bytes_be(&expected_commitment.x)), | ||
Fq::from(num_bigint::BigUint::from_bytes_be(&expected_commitment.y)), | ||
); |
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.
We should add a isInSubgroup and IsOnCurve checks. Good to catch it even though DA also does it on its end.
) -> Result<Vec<u8>, VerificationError> { | ||
let mut index = index; | ||
if proof.len() == 0 || proof.len() % 32 != 0 { | ||
return Err(VerificationError::WrongProof); |
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.
Would be good to differentiate the errors since the VerificationError::WrongProof
is used in many places.
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.
LGTM
9c06b40
into
eigen-client-extra-features
What ❔
This PR implements verifier for EigenDA
Since config was changed, recompile zkstack:
cargo install --path zkstack_cli/crates/zkstack --force --locked
Why ❔
Checklist
zkstack dev fmt
andzkstack dev lint
.