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

[v2] Dispatcher Implementation #871

Merged
merged 1 commit into from
Nov 13, 2024
Merged

Conversation

ian-shim
Copy link
Contributor

@ian-shim ian-shim commented Nov 7, 2024

Why are these changes needed?

Dispatcher implementation which takes encoded blobs, forms a batch, distributes the data across DA nodes, then gathers, validates, and aggregates the attestation.

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 :(

@ian-shim
Copy link
Contributor Author

ian-shim commented Nov 7, 2024

To be rebased on top of #870

@ian-shim ian-shim force-pushed the v2-dispatcher branch 2 times, most recently from c78de00 to 469b084 Compare November 7, 2024 18:20
@ian-shim ian-shim marked this pull request as ready for review November 7, 2024 18:21
@ian-shim ian-shim requested a review from dmanc November 7, 2024 18:21
Y: [2]*big.Int{
b.BlobCommitments.LengthCommitment.Y.A0.BigInt(new(big.Int)),
b.BlobCommitments.LengthCommitment.Y.A1.BigInt(new(big.Int)),
},
Copy link
Contributor

@anupsv anupsv Nov 7, 2024

Choose a reason for hiding this comment

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

Should this be in the form of A1, A0 for G2 points ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Each extension in G2Point is represented as uint256[2] onchain

if err != nil {
return fmt.Errorf("failed to receive and validate signatures: %w", err)
}
numPassed, passedQuorums := numBlobsAttestedByQuorum(quorumAttestation.QuorumResults, batchData.Batch.BlobCertificates)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is deduplication of certs already handled upstream ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no deduplication. New batch should be constructed by blobs that haven't been processed yet. GetBlobMetadataByStatus takes lastUpdatedAt which gets updated as we pull new blobs and acts as a cursor. It only takes blobs that are updated after this value.

numBlobsAttestedByQuorum is added to 1) early terminate if no blobs received sufficient amount of signatures, or 2) remove unsuccessful quorums (to help reduce gas costs for bridging). However, this optimization seems pretty minor, and I think I'll remove this check to simplify.

@ian-shim ian-shim mentioned this pull request Nov 8, 2024
6 tasks
@ian-shim ian-shim force-pushed the v2-dispatcher branch 2 times, most recently from 4fe4eaf to 0042d91 Compare November 11, 2024 17:59
core/v2/types.go Outdated Show resolved Hide resolved
@@ -302,7 +302,7 @@ func (b *Batcher) updateConfirmationInfo(
blobsToRetry = append(blobsToRetry, batchData.blobs[blobIndex])
continue
}
proof = serializeProof(merkleProof)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does the serializeProof implementation need to be deleted? I don't see a delta.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's deleted in the same file L566

}, nil
}

func (m *nodeClientManager) GetClient(operatorID core.OperatorID, socket string) (clients.NodeClientV2, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happens if there is a socket update? Since this is only using the operatorID as the key, if there is a new socket it won't get reflected. Wouldn't it make more sense to use the socket as the key?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching this! That's a good idea

@ian-shim ian-shim merged commit 999fb15 into Layr-Labs:master Nov 13, 2024
6 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.

3 participants