Skip to content

core/*: use comm idx as key in DutyDB for agg duty#4544

Open
KaloyanTanev wants to merge 6 commits into
mainfrom
kalo/debug-inconsistent-agg-parsigs
Open

core/*: use comm idx as key in DutyDB for agg duty#4544
KaloyanTanev wants to merge 6 commits into
mainfrom
kalo/debug-inconsistent-agg-parsigs

Conversation

@KaloyanTanev
Copy link
Copy Markdown
Collaborator

Fixing 2 bugs with partial sigs during aggregation duties.

  1. reportParSigs never warned on inconsistencies (core/tracker/tracker.go)
    The guard checked len(parsigsByRoot) <= 1 against the outer map (number of pubkeys) instead of the inner map (number of distinct roots per pubkey), so the warning was effectively dead code for single-validator cases. Fixed, and improved the log to show per-root msg_root hex, share indices, and a sample parsig JSON.

  2. Wrong aggregate returned to VCs in post-Electra (core/dutydb/memory.go, core/validatorapi/validatorapi.go, core/interfaces.go)
    In Electra, AttestationData.Index is always 0, so all committees in a slot share the same AttestationDataRoot. The aggKey in DutyDB only used {slot, root}, causing later committee aggregates to silently overwrite earlier ones. AggregateAttestation also ignored opts.CommitteeIndex entirely. Fixed by adding CommitteeIndex to aggKey (derived from CommitteeBits on post-Electra aggregates) and threading it through AwaitAggAttestation.

category: bug
ticket: #4006

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes aggregator-duty lookup and partial-signature diagnostics, especially for post-Electra attestations where multiple committees can share the same attestation data root.

Changes:

  • Adds committee index to aggregated attestation DutyDB lookup keys and threads it through validator API calls.
  • Adds Electra multi-committee DutyDB coverage.
  • Fixes partial-signature inconsistency reporting to check roots per pubkey.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
core/validatorapi/validatorapi.go Passes committee index into aggregate attestation lookup.
core/tracker/tracker.go Updates inconsistent partial-signature reporting logic and log payload.
core/interfaces.go Updates interfaces and wiring signatures for aggregate attestation lookup.
core/dutydb/memory.go Includes committee index in aggregated attestation storage/query keys.
core/dutydb/memory_test.go Adds Electra multi-committee aggregate lookup test and updates existing call.
core/dutydb/memory_internal_test.go Updates cancelled-query test for new signature.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread core/tracker/tracker.go Outdated
@KaloyanTanev KaloyanTanev changed the title core/*: use committee index as key in DutyDB for aggregations core/*: use comm idx as key in DutyDB for agg duty May 28, 2026
@sonarqubecloud
Copy link
Copy Markdown

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