fix: duplicate and old attestations aggregates #1303
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Now we can see an small amount of attestations per block (previously we had 4/6 even with just one committee) And we could also see that for a particular slot we have published the most recent attestation: |
Arkenan
approved these changes
Sep 26, 2024
Comment on lines
213
to
222
| defp process_attestations(attestations) do | ||
| attestations | ||
| |> Enum.group_by(& &1.data.index) | ||
| |> Enum.map(fn {_, attestations} -> | ||
| Enum.max_by( | ||
| attestations, | ||
| &(&1.aggregation_bits |> BitVector.count()) | ||
| ) | ||
| end) | ||
| end |
There was a problem hiding this comment.
nit: maybe "filter_latest_attestations" or something more descriptive? or deduplicate_attestations?
There was a problem hiding this comment.
Modified it to select_best_aggregates/1 to convey the actual functionality, selecting between aggregates the one with the most set bits
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Right now we are publishing old aggregates (slot -2) instead of the current (slot -1), also we publish several aggregates for the same subcommittee, this will be solved by this PR.
Description
This PR solves both issues explained in 1254, on one hand the old attestation (being slot - 2 the most recen attestation always) was due to picking the slot on
ignore/2from the store (slot - 1) instead of taking it from the current block (slot). For the second issue, duplications was due to a lacking of filtering in the block builder steps, as with sync committees, we need to group attestations by committee index and just keep the ones with the greatest pop count (the most amount of set bits)Resolves #1254