-
-
Notifications
You must be signed in to change notification settings - Fork 312
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
Do single call to produce attestation #3917
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!
Codecov Report
@@ Coverage Diff @@
## master #3917 +/- ##
==========================================
+ Coverage 36.52% 36.53% +0.01%
==========================================
Files 324 324
Lines 8980 8982 +2
Branches 1410 1410
==========================================
+ Hits 3280 3282 +2
Misses 5556 5556
Partials 144 144 |
Performance Report✔️ no performance regression detected Full benchmark results
|
@dapplion The DVT compatibility issue mentioned in #5103 is due to the changes in this PR. This also introduced a deviation compared to other CL clients as those send a request per committee index. I am wondering if we really need this optimization as there is an upper bound of 64 requests due to MAX_COMMITTEES_PER_SLOT but of course it will put more load on the BN either way. I really want to keep the optimization while also fixing the compatibility issues we have. Considering all of that, instead of adding branches to the current attenstation service I think we might just be best of creating a 2nd "distributed" attestation service. Edit: opened a PR to propose and discuss a possible solution #5258 |
Motivation
Beacon node's endpoint produceAttestationData return data is not dependant on committeeIndex. We can do a single call for all committees and save resources
Description
Produce a single attestation for all committees, and clone mutate before signing
Closes #3098