Skip to content
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

verifyBlobs util #862

Merged
merged 3 commits into from
Nov 18, 2024
Merged

verifyBlobs util #862

merged 3 commits into from
Nov 18, 2024

Conversation

0x0aa0
Copy link
Contributor

@0x0aa0 0x0aa0 commented Nov 4, 2024

Why are these changes needed?

Adds a verifyBlobs function to RollupUtils that accepts input of multiple blob proofs for verification

Checks

  • I've made sure the lint is passing in this PR.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, in that case, please comment that they are not relevant.
  • I've checked the new test coverage and the coverage percentage didn't drop.
  • Testing Strategy
    • Unit tests
    • Integration tests
    • This PR is not tested :(

@0x0aa0 0x0aa0 marked this pull request as ready for review November 7, 2024 22:18
@0x0aa0 0x0aa0 requested review from mooselumph and gpsanant November 7, 2024 22:27
*/
function verifyBlobs(
IEigenDAServiceManager.BlobHeader[] calldata blobHeaders,
IEigenDAServiceManager eigenDAServiceManager,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make this the first param imo

Merkle.verifyInclusionKeccak(
blobVerificationProofs[i].inclusionProof,
blobVerificationProofs[i].batchMetadata.batchHeader.blobHeadersRoot,
keccak256(abi.encodePacked(EigenDAHasher.hashBlobHeader(blobHeaders[i]))),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

double hash?


// make sure that the adversaryThresholdPercentage is at least the given quorumAdversaryThresholdPercentage
uint8 _adversaryThresholdPercentage = uint8(quorumAdversaryThresholdPercentages[blobHeaders[i].quorumBlobParams[j].quorumNumber]);
if(_adversaryThresholdPercentage > 0){

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need this check?

);

// bitmap of quorum numbers in all quorumBlobParams
uint256 confirmedQuorumsBitmap;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't return this?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if not, why not just point to the indices in quorumBlobParams to verify?

@0x0aa0 0x0aa0 requested a review from gpsanant November 15, 2024 23:13
@@ -31,10 +31,10 @@ library EigenDARollupUtils {
* @param blobVerificationProof the relevant data needed to prove inclusion of the blob and that the trust assumptions were as expected
*/
function verifyBlob(
IEigenDAServiceManager.BlobHeader calldata blobHeader,
IEigenDAServiceManager.BlobHeader memory blobHeader,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why cant we keep this calldata?

@0x0aa0 0x0aa0 merged commit fbe6d3b into master Nov 18, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants