-
-
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
fix: reduce getSingleTrueBit() calls #7209
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## unstable #7209 +/- ##
============================================
- Coverage 49.21% 48.91% -0.30%
============================================
Files 598 601 +3
Lines 39803 40183 +380
Branches 2102 2060 -42
============================================
+ Hits 19588 19655 +67
- Misses 20175 20490 +315
+ Partials 40 38 -2 |
Performance Report✔️ no performance regression detected Full benchmark results
|
const insertOutcome = chain.attestationPool.add(committeeIndex, attestation, attDataRootHex); | ||
const insertOutcome = chain.attestationPool.add( | ||
committeeIndex, | ||
participationIndex, |
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.
what about calling this validatorCommitteeIndex
?
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.
imo bitIndex
or current name is good
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.
will change to committeeValidatorIndex
as in devnet 5 https://github.com/ChainSafe/lodestar/pull/7265/files#diff-c702dde1ffa90a6d2720c7b4930bc5c08cc1f302cfd09ed9e74d80755b92da8fR645
@nflaig says this may conflict with devnet-5 changes |
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.
@twoeths all / most of the changes in this PR are already implemented on devnet-5 branch and merging this would cause a lot of conflicts
Do we just wanna close this and get it shipped when merging devnet-5 to unstable?
sounds good to me |
Motivation
getSingleTrueBit()
is called during attestation validation and in opPool while it is not simple:Description
participationIndex
as a result of attestation validation, so we don't have to extract it inside opPoolfound when investigating #7206