Conversation
| return | ||
| sorted_proofs = sorted( | ||
| proofs, | ||
| key=lambda proof: len(proof.participants.to_validator_indices()), |
There was a problem hiding this comment.
calc and sort them here with participants - covered directly, this is what really will give greedy coverage which imo would be the best coverage algo on average
| return self.model_copy(update={"safe_target": safe_target}) | ||
|
|
||
| def aggregate_committee_signatures(self) -> tuple["Store", list[SignedAggregatedAttestation]]: | ||
| def aggregate_committee_signatures_and_payloads(self) -> tuple["Store", list[SignedAggregatedAttestation]]: |
There was a problem hiding this comment.
| def aggregate_committee_signatures_and_payloads(self) -> tuple["Store", list[SignedAggregatedAttestation]]: | |
| def aggregate_signatures_and_payloads(self) -> tuple["Store", list[SignedAggregatedAttestation]]: |
|
|
||
| def aggregate( | ||
| self, | ||
| attestations: Collection[Attestation], |
There was a problem hiding this comment.
it needs to take attestation/attestation data as the params
There was a problem hiding this comment.
We have attestation_data from new_payloads and signatures_map
| # Use only keys from new_payloads and gossip_signatures | ||
| # know_payloads can be used to extend the proof with new_payloads and gossip_signatures | ||
| # but known_payloads are not recursively aggregated into their own proofs | ||
| attestation_keys = set(new_payloads.keys()) | set(gossip_signatures.keys()) |
There was a problem hiding this comment.
new payloads are use for aggregator and it can extend with known payloads if the validator indices in attestations for a particular data is not covered.
and known payloads are used for block proposer which will already have new payloads merged in and shouldn't require a lookup in new payloads (which should be an error condition)
There was a problem hiding this comment.
new payloads are use for aggregator and it can extend with known payloads if the validator indices in attestations for a particular data is not covered.
Yeah, this function does exactly that, it is for aggregation of payloads and signatures for the aggregator.
and known payloads are used for block proposer which will already have new payloads merged in and shouldn't require a lookup in new payloads (which should be an error condition)
I may not fully understand this, but known payloads refer to payloads that have already been accounted for and included in blocks. While performing aggregation, I use these known payloads if a particular attestation_data exists either in signatures_map or in new_payload, and the same payload is also present in known_payload and can contribute additional participant bits.
|
Adding these changes to #449 |
🗒️ Description
Implements recursive aggregation. Spec for devnet-4 (draft):
🔗 Related Issues or PRs
✅ Checklist
toxchecks to avoid unnecessary CI fails:uvx tox