diff --git a/app/app.go b/app/app.go index 3d38d95291..12d1f208e7 100644 --- a/app/app.go +++ b/app/app.go @@ -714,13 +714,21 @@ func wireCoreWorkflow(ctx context.Context, life *lifecycle.Manager, conf Config, coreConsensus := consensusController.CurrentConsensus() // initially points to DefaultConsensus() // Priority protocol always uses QBFTv2. - err = wirePrioritise(ctx, conf, life, p2pNode, peerIDs, lock.Threshold, + isync, err := wirePrioritise(ctx, conf, life, p2pNode, peerIDs, lock.Threshold, sender.SendReceive, defaultConsensus, sched, p2pKey, deadlineFunc, consensusController, lock.ConsensusProtocol) if err != nil { return err } + // Gate the plural SyncContributions encoding on a threshold of peers being on a + // version that understands it (charon >= v1.11), re-evaluated each info-sync + // round. isync is nil for leader-cast (no priority protocol), leaving the + // fetcher on the backwards-compatible single encoding. + if isync != nil { + fetch.RegisterSyncContributionV2(isync.SyncContributionsSupported) + } + track, err := newTracker(ctx, life, deadlineFunc, peers, eth2Cl) if err != nil { return err @@ -765,11 +773,11 @@ func wirePrioritise(ctx context.Context, conf Config, life *lifecycle.Manager, p peers []peer.ID, threshold int, sendFunc p2p.SendReceiveFunc, coreCons core.Consensus, sched core.Scheduler, p2pKey *k1.PrivateKey, deadlineFunc func(duty core.Duty) (time.Time, bool), consensusController core.ConsensusController, clusterPreferredProtocol string, -) error { +) (*infosync.Component, error) { cons, ok := coreCons.(*qbft.Consensus) if !ok { // Priority protocol not supported for leader cast. - return nil + return nil, nil //nolint:nilnil // No infosync component and no error is valid here. } // exchangeTimeout of 6 seconds (half a slot) is a good thumb suck. @@ -779,7 +787,7 @@ func wirePrioritise(ctx context.Context, conf Config, life *lifecycle.Manager, p prio, err := priority.NewComponent(ctx, p2pNode, peers, threshold, sendFunc, p2p.RegisterHandler, cons, exchangeTimeout, p2pKey, deadlineFunc) if err != nil { - return err + return nil, err } // The initial protocols order as defined by implementation is altered by: @@ -837,7 +845,7 @@ func wirePrioritise(ctx context.Context, conf Config, life *lifecycle.Manager, p life.RegisterStart(lifecycle.AsyncAppCtx, lifecycle.StartPeerInfo, lifecycle.HookFuncCtx(prio.Start)) - return nil + return isync, nil } // newTracker creates and starts a new tracker instance. diff --git a/core/aggsigdb/memory.go b/core/aggsigdb/memory.go index 01e6562580..c3ae95f833 100644 --- a/core/aggsigdb/memory.go +++ b/core/aggsigdb/memory.go @@ -43,7 +43,12 @@ type MemDB struct { // Store implements core.AggSigDB, see its godoc. func (db *MemDB) Store(ctx context.Context, duty core.Duty, set core.SignedDataSet) error { for pubKey, data := range set { - if err := db.store(ctx, duty, pubKey, data); err != nil { + subcommIdx, err := core.SyncSubcommitteeIndex(duty.Type, data) + if err != nil { + return err + } + + if err := db.store(ctx, memDBKey{duty: duty, pubKey: pubKey, subcommIdx: subcommIdx}, data); err != nil { return err } } @@ -51,7 +56,7 @@ func (db *MemDB) Store(ctx context.Context, duty core.Duty, set core.SignedDataS return nil } -func (db *MemDB) store(ctx context.Context, duty core.Duty, pubKey core.PubKey, data core.SignedData) error { +func (db *MemDB) store(ctx context.Context, key memDBKey, data core.SignedData) error { clone, err := data.Clone() // Clone before storing. if err != nil { return err @@ -59,7 +64,7 @@ func (db *MemDB) store(ctx context.Context, duty core.Duty, pubKey core.PubKey, response := make(chan error, 1) cmd := writeCommand{ - memDBKey: memDBKey{duty, pubKey}, + memDBKey: key, data: clone, response: response, } @@ -83,14 +88,14 @@ func (db *MemDB) store(ctx context.Context, duty core.Duty, pubKey core.PubKey, } // Await implements core.AggSigDB, see its godoc. -func (db *MemDB) Await(ctx context.Context, duty core.Duty, pubKey core.PubKey) (core.SignedData, error) { +func (db *MemDB) Await(ctx context.Context, duty core.Duty, pubKey core.PubKey, subcommIdx core.SubcommitteeIndex) (core.SignedData, error) { cancel := make(chan struct{}) defer close(cancel) response := make(chan core.SignedData, 1) query := readQuery{ - memDBKey: memDBKey{duty, pubKey}, + memDBKey: memDBKey{duty: duty, pubKey: pubKey, subcommIdx: subcommIdx}, response: response, cancel: cancel, } @@ -146,7 +151,7 @@ func (db *MemDB) execCommand(command writeCommand) { _ = db.deadliner.Add(command.duty) // TODO(corver): Distinguish between no deadline supported vs already expired. - key := memDBKey{command.duty, command.pubKey} + key := command.memDBKey if existing, ok := db.data[key]; ok { equal, err := dataEqual(existing, command.data) @@ -178,7 +183,7 @@ func dataEqual(x core.SignedData, y core.SignedData) (bool, error) { // execQuery returns true if the query was successfully executed. // If the requested entry is found in the DB it will return it via query.response channel. func (db *MemDB) execQuery(query readQuery) bool { - data, ok := db.data[memDBKey{query.duty, query.pubKey}] + data, ok := db.data[query.memDBKey] if !ok { return false } @@ -228,6 +233,11 @@ func cancelled(cancel <-chan struct{}) bool { type memDBKey struct { duty core.Duty pubKey core.PubKey + // subcommIdx is the sync subcommittee index for sync-committee aggregator + // duties (DutyPrepareSyncContribution, DutySyncContribution), and 0 otherwise. + // A validator can occupy multiple sync subcommittees in the same slot, so it + // disambiguates their otherwise-colliding aggregated signatures. + subcommIdx core.SubcommitteeIndex } // writeCommand holds the data to write into the database. diff --git a/core/aggsigdb/memory_internal_test.go b/core/aggsigdb/memory_internal_test.go index 2c722217f1..6d71db92e4 100644 --- a/core/aggsigdb/memory_internal_test.go +++ b/core/aggsigdb/memory_internal_test.go @@ -33,7 +33,7 @@ func TestDutyExpiration(t *testing.T) { err := db.Store(ctx, duty, core.SignedDataSet{pubkey: sig}) require.NoError(t, err) - resp, err := db.Await(ctx, duty, pubkey) + resp, err := db.Await(ctx, duty, pubkey, 0) require.NoError(t, err) require.Equal(t, sig, resp) @@ -78,7 +78,7 @@ func TestCancelledQuery(t *testing.T) { errCh := make(chan error, 2) go func() { - _, err := db.Await(qctx, duty, pubkey) + _, err := db.Await(qctx, duty, pubkey, 0) errCh <- err wg.Done() @@ -87,7 +87,7 @@ func TestCancelledQuery(t *testing.T) { require.Equal(t, 1, <-queryCount) wg.Go(func() { - _, err := db.Await(qctx, duty, pubkey) + _, err := db.Await(qctx, duty, pubkey, 0) errCh <- err }) diff --git a/core/aggsigdb/memory_test.go b/core/aggsigdb/memory_test.go index 6c9d5dce1b..847c4e2c2d 100644 --- a/core/aggsigdb/memory_test.go +++ b/core/aggsigdb/memory_test.go @@ -45,7 +45,7 @@ func testMemDB(t *testing.T, newMemDB func(core.Deadliner) core.AggSigDB) { err := db.Store(context.Background(), testDuty, core.SignedDataSet{testPubKey: testSignedData}) require.NoError(t, err) - result, err := db.Await(context.Background(), testDuty, testPubKey) + result, err := db.Await(context.Background(), testDuty, testPubKey, 0) require.NoError(t, err) require.EqualValues(t, testSignedData, result) @@ -67,7 +67,7 @@ func testMemDB(t *testing.T, newMemDB func(core.Deadliner) core.AggSigDB) { }) go func() { - result, err := db.Await(context.Background(), testDuty, testPubKey) + result, err := db.Await(context.Background(), testDuty, testPubKey, 0) resChan <- struct { result core.SignedData err error @@ -97,7 +97,7 @@ func testMemDB(t *testing.T, newMemDB func(core.Deadliner) core.AggSigDB) { ctx2, cancel2 := context.WithCancel(context.Background()) go func() { - _, err := db.Await(ctx2, testDuty, testPubKey) + _, err := db.Await(ctx2, testDuty, testPubKey, 0) errChan <- err }() @@ -122,7 +122,7 @@ func testMemDB(t *testing.T, newMemDB func(core.Deadliner) core.AggSigDB) { ctx2, cancel2 := context.WithCancel(context.Background()) cancel2() - _, err := db.Await(ctx2, testDuty, testPubKey) + _, err := db.Await(ctx2, testDuty, testPubKey, 0) require.Error(t, err) require.Equal(t, "context canceled", err.Error()) }) @@ -143,7 +143,7 @@ func testMemDB(t *testing.T, newMemDB func(core.Deadliner) core.AggSigDB) { errChan := make(chan error) go func() { - _, err := db.Await(context.Background(), testDuty, testPubKey) + _, err := db.Await(context.Background(), testDuty, testPubKey, 0) errChan <- err }() @@ -195,7 +195,7 @@ func testMemDB(t *testing.T, newMemDB func(core.Deadliner) core.AggSigDB) { err = db.Store(context.Background(), testDuty, core.SignedDataSet{testPubKey: testSignedData}) require.NoError(t, err) - result, err := db.Await(context.Background(), testDuty, testPubKey) + result, err := db.Await(context.Background(), testDuty, testPubKey, 0) require.NoError(t, err) require.EqualValues(t, testSignedData, result) }) @@ -213,7 +213,7 @@ func testMemDB(t *testing.T, newMemDB func(core.Deadliner) core.AggSigDB) { err := db.Store(context.Background(), testDuty, core.SignedDataSet{testPubKey: testSignedData}) require.NoError(t, err) - result, err := db.Await(context.Background(), testDuty, testPubKey) + result, err := db.Await(context.Background(), testDuty, testPubKey, 0) require.NoError(t, err) require.EqualValues(t, testSignedData, result) @@ -224,7 +224,7 @@ func testMemDB(t *testing.T, newMemDB func(core.Deadliner) core.AggSigDB) { err = db.Store(context.Background(), testDuty, core.SignedDataSet{testPubKey: testSignedData}) require.Equal(t, err.Error(), aggsigdb.ErrStopped.Error()) - _, err = db.Await(context.Background(), testDuty, testPubKey) + _, err = db.Await(context.Background(), testDuty, testPubKey, 0) require.Equal(t, err.Error(), aggsigdb.ErrStopped.Error()) }) } diff --git a/core/aggsigdb/memory_v2.go b/core/aggsigdb/memory_v2.go index fad63c0749..adcdd2c2b5 100644 --- a/core/aggsigdb/memory_v2.go +++ b/core/aggsigdb/memory_v2.go @@ -33,15 +33,13 @@ func NewMemDBV2(deadliner core.Deadliner) *MemDBV2 { } } -func (m *MemDBV2) store(duty core.Duty, pubKey core.PubKey, data core.SignedData) error { +func (m *MemDBV2) store(key memDBKey, data core.SignedData) error { data, err := data.Clone() if err != nil { return err } - _ = m.deadliner.Add(duty) // TODO(corver): Distinguish between no deadline supported vs already expired. - - key := memDBKey{duty, pubKey} + _ = m.deadliner.Add(key.duty) // TODO(corver): Distinguish between no deadline supported vs already expired. if existing, ok := m.data[key]; ok { equal, err := dataEqual(existing, data) @@ -52,7 +50,7 @@ func (m *MemDBV2) store(duty core.Duty, pubKey core.PubKey, data core.SignedData } } else { m.data[key] = data - m.keysByDuty[duty] = append(m.keysByDuty[duty], key) + m.keysByDuty[key.duty] = append(m.keysByDuty[key.duty], key) } return nil @@ -71,7 +69,12 @@ func (m *MemDBV2) Store(ctx context.Context, duty core.Duty, set core.SignedData } for pubKey, data := range set { - if err := m.store(duty, pubKey, data); err != nil { + subcommIdx, err := core.SyncSubcommitteeIndex(duty.Type, data) + if err != nil { + return err + } + + if err := m.store(memDBKey{duty: duty, pubKey: pubKey, subcommIdx: subcommIdx}, data); err != nil { return err } } @@ -86,7 +89,7 @@ func (m *MemDBV2) Store(ctx context.Context, duty core.Duty, set core.SignedData return nil } -func (m *MemDBV2) Await(ctx context.Context, duty core.Duty, pubKey core.PubKey) (core.SignedData, error) { +func (m *MemDBV2) Await(ctx context.Context, duty core.Duty, pubKey core.PubKey, subcommIdx core.SubcommitteeIndex) (core.SignedData, error) { errMustLoop := errors.New("still needs loop") query := func() (core.SignedData, error) { @@ -99,7 +102,7 @@ func (m *MemDBV2) Await(ctx context.Context, duty core.Duty, pubKey core.PubKey) case <-m.closed: return nil, ErrStopped default: - data, ok := m.data[memDBKey{duty, pubKey}] + data, ok := m.data[memDBKey{duty: duty, pubKey: pubKey, subcommIdx: subcommIdx}] if !ok { return nil, errMustLoop } diff --git a/core/aggsigdb/memory_v2_internal_test.go b/core/aggsigdb/memory_v2_internal_test.go index 27b0c15348..a73683dcd8 100644 --- a/core/aggsigdb/memory_v2_internal_test.go +++ b/core/aggsigdb/memory_v2_internal_test.go @@ -29,7 +29,7 @@ func TestDutyExpirationV2(t *testing.T) { err := db.Store(ctx, duty, core.SignedDataSet{pubkey: sig}) require.NoError(t, err) - resp, err := db.Await(ctx, duty, pubkey) + resp, err := db.Await(ctx, duty, pubkey, 0) require.NoError(t, err) require.Equal(t, sig, resp) @@ -62,7 +62,7 @@ func TestCancelledQueryV2(t *testing.T) { errStore := make([]error, awaitAmt) for idx := range awaitAmt { go func() { - _, err := db.Await(qctx, duty, pubkey) + _, err := db.Await(qctx, duty, pubkey, 0) errStore[idx] = err wg.Done() diff --git a/core/dutydb/memory.go b/core/dutydb/memory.go index c831366718..4b899b0964 100644 --- a/core/dutydb/memory.go +++ b/core/dutydb/memory.go @@ -478,18 +478,38 @@ func (db *MemDB) storeAggAttestationUnsafe(unsignedData core.UnsignedData) error return nil } -// storeSyncContributionUnsafe stores the unsigned aggregated attestation. It is unsafe since it assumes the lock is held. +// storeSyncContributionUnsafe stores the unsigned sync committee contributions. +// A validator can aggregate for multiple sync subcommittees in the same slot, so +// the unsigned data is a collection of contributions (one per subcommittee), each +// stored independently by (slot, subcommittee index, beacon block root). It is +// unsafe since it assumes the lock is held. func (db *MemDB) storeSyncContributionUnsafe(unsignedData core.UnsignedData) error { cloned, err := unsignedData.Clone() // Clone before storing. if err != nil { return err } - contrib, ok := cloned.(core.SyncContribution) - if !ok { - return errors.New("invalid unsigned sync committee contribution") + // The unsigned data is either a plural SyncContributions (v1.11+, one per + // subcommittee) or, for backwards compatibility, a single SyncContribution. + switch contrib := cloned.(type) { + case core.SyncContributions: + for _, entry := range contrib { + if err := db.storeSyncContributionEntryUnsafe(entry); err != nil { + return err + } + } + + return nil + case core.SyncContribution: + return db.storeSyncContributionEntryUnsafe(contrib) + default: + return errors.New("invalid unsigned sync committee contributions") } +} +// storeSyncContributionEntryUnsafe stores a single sync committee contribution. +// It is unsafe since it assumes the lock is held. +func (db *MemDB) storeSyncContributionEntryUnsafe(contrib core.SyncContribution) error { contribRoot, err := contrib.HashTreeRoot() if err != nil { return errors.Wrap(err, "hash sync committee contribution") diff --git a/core/dutydb/memory_test.go b/core/dutydb/memory_test.go index 8c263f7664..d3c5a29977 100644 --- a/core/dutydb/memory_test.go +++ b/core/dutydb/memory_test.go @@ -320,7 +320,7 @@ func TestMemDBSyncContribution(t *testing.T) { for range queries { contrib := testutil.RandomSyncCommitteeContribution() set := core.UnsignedDataSet{ - testutil.RandomCorePubKey(t): core.NewSyncContribution(contrib), + testutil.RandomCorePubKey(t): core.SyncContributions{core.NewSyncContribution(contrib)}, } var ( @@ -373,13 +373,13 @@ func TestMemDBSyncContribution(t *testing.T) { contrib1.Slot = slot contrib1.SubcommitteeIndex = subcommIdx contrib1.BeaconBlockRoot = beaconBlockRoot - unsigned1 := core.NewSyncContribution(contrib1) + unsigned1 := core.SyncContributions{core.NewSyncContribution(contrib1)} contrib2 := testutil.RandomSyncCommitteeContribution() contrib2.Slot = slot contrib2.SubcommitteeIndex = subcommIdx contrib2.BeaconBlockRoot = beaconBlockRoot - unsigned2 := core.NewSyncContribution(contrib2) + unsigned2 := core.SyncContributions{core.NewSyncContribution(contrib2)} // Store them. err := db.Store(ctx, duty, core.UnsignedDataSet{ @@ -405,7 +405,7 @@ func TestMemDBSyncContribution(t *testing.T) { testutil.RandomCorePubKey(t): testutil.RandomDenebCoreVersionedAggregateAttestation(), }) require.Error(t, err) - require.ErrorContains(t, err, "invalid unsigned sync committee contribution") + require.ErrorContains(t, err, "invalid unsigned sync committee contributions") }) } diff --git a/core/fetcher/fetcher.go b/core/fetcher/fetcher.go index 5d30e1d959..4b20f8368e 100644 --- a/core/fetcher/fetcher.go +++ b/core/fetcher/fetcher.go @@ -6,6 +6,7 @@ import ( "context" "fmt" "math" + "slices" "strings" "sync" @@ -36,16 +37,17 @@ func New(eth2Cl eth2wrap.Client, feeRecipientFunc func(core.PubKey) string, buil // Fetcher fetches proposed duty data. type Fetcher struct { - eth2Cl eth2wrap.Client - feeRecipientFunc func(core.PubKey) string - subs []func(context.Context, core.Duty, core.UnsignedDataSet) error - aggSigDBFunc func(context.Context, core.Duty, core.PubKey) (core.SignedData, error) - awaitAttDataFunc func(ctx context.Context, slot, commIdx uint64) (*eth2p0.AttestationData, error) - builderEnabled bool - graffitiBuilder *GraffitiBuilder - electraSlot eth2p0.Slot - fetchOnlyCommIdx0 bool - attDataCache sync.Map // Cache for early-fetched attestation data (map[uint64]core.UnsignedDataSet) + eth2Cl eth2wrap.Client + feeRecipientFunc func(core.PubKey) string + subs []func(context.Context, core.Duty, core.UnsignedDataSet) error + aggSigDBFunc func(context.Context, core.Duty, core.PubKey, core.SubcommitteeIndex) (core.SignedData, error) + awaitAttDataFunc func(ctx context.Context, slot, commIdx uint64) (*eth2p0.AttestationData, error) + syncContributionV2Func func(slot uint64) bool + builderEnabled bool + graffitiBuilder *GraffitiBuilder + electraSlot eth2p0.Slot + fetchOnlyCommIdx0 bool + attDataCache sync.Map // Cache for early-fetched attestation data (map[uint64]core.UnsignedDataSet) } // Subscribe registers a callback for fetched duties. @@ -189,10 +191,19 @@ func (f *Fetcher) Fetch(ctx context.Context, duty core.Duty, defSet core.DutyDef // RegisterAggSigDB registers a function to get resolved aggregated signed data from AggSigDB. // Note: This is not thread safe and should only be called *before* Fetch. -func (f *Fetcher) RegisterAggSigDB(fn func(context.Context, core.Duty, core.PubKey) (core.SignedData, error)) { +func (f *Fetcher) RegisterAggSigDB(fn func(context.Context, core.Duty, core.PubKey, core.SubcommitteeIndex) (core.SignedData, error)) { f.aggSigDBFunc = fn } +// RegisterSyncContributionV2 registers a function reporting whether, for a given slot, the +// cluster supports the plural SyncContributions encoding, i.e. at least a threshold of peers +// advertised charon >= v1.11. When it returns false, or is unregistered, the fetcher falls +// back to a single SyncContribution per validator for backwards compatibility. +// Note: This is not thread safe and should only be called *before* Fetch. +func (f *Fetcher) RegisterSyncContributionV2(fn func(slot uint64) bool) { + f.syncContributionV2Func = fn +} + // RegisterAwaitAttData registers a function to get attestation data from DutyDB. // Note: This is not thread safe and should only be called *before* Fetch. func (f *Fetcher) RegisterAwaitAttData(fn func(ctx context.Context, slot uint64, commIdx uint64) (*eth2p0.AttestationData, error)) { @@ -288,7 +299,7 @@ func (f *Fetcher) fetchAggregatorData(ctx context.Context, slot uint64, defSet c } // Query AggSigDB for DutyPrepareAggregator to get beacon committee selections. - prepAggData, err := f.aggSigDBFunc(ctx, core.NewPrepareAggregatorDuty(slot), pubkey) + prepAggData, err := f.aggSigDBFunc(ctx, core.NewPrepareAggregatorDuty(slot), pubkey, 0) if err != nil { return core.UnsignedDataSet{}, err } @@ -365,7 +376,7 @@ func (f *Fetcher) fetchProposerData(ctx context.Context, slot uint64, defSet cor // Fetch previously aggregated randao reveal from AggSigDB dutyRandao := core.NewRandaoDuty(slot) - randaoData, err := f.aggSigDBFunc(ctx, dutyRandao, pubkey) + randaoData, err := f.aggSigDBFunc(ctx, dutyRandao, pubkey, 0) if err != nil { return nil, err } @@ -425,69 +436,176 @@ func (f *Fetcher) fetchContributionData(ctx context.Context, slot uint64, defSet resp := make(core.UnsignedDataSet) - for pubkey := range defSet { - // Query AggSigDB for DutyPrepareSyncContribution to get sync committee selection. - selectionData, err := f.aggSigDBFunc(ctx, core.NewPrepareSyncContributionDuty(slot), pubkey) + subcommSize, err := f.syncSubcommitteeSize(ctx) + if err != nil { + return core.UnsignedDataSet{}, err + } + + // The plural SyncContributions encoding (one entry per subcommittee) is only + // used once a threshold of peers are on a version that understands it + // (charon >= v1.11). Until then fall back to a single SyncContribution per + // validator, matching the pre-v1.11 wire format. Peers behind that threshold + // won't decode the plural form and simply skip contributing. + syncContribV2 := f.syncContributionV2Func != nil && f.syncContributionV2Func(slot) + + for pubkey, def := range defSet { + // A validator can occupy multiple sync subcommittees in the same slot, + // producing a distinct contribution for each it aggregates. + subcommIdxs, err := syncSubcommittees(def, subcommSize) if err != nil { return core.UnsignedDataSet{}, err } - selection, ok := selectionData.(core.SyncCommitteeSelection) - if !ok { - return core.UnsignedDataSet{}, errors.New("invalid sync committee selection") - } + var contribs core.SyncContributions - subcommIdx := selection.SubcommitteeIndex + for _, subcommIdx := range subcommIdxs { + contrib, ok, err := f.fetchSubcommContribution(ctx, slot, pubkey, subcommIdx) + if err != nil { + return core.UnsignedDataSet{}, err + } else if !ok { + continue // Validator is not an aggregator for this subcommittee. + } - // Check if the validator is an aggregator for the sync committee. - ok, err = eth2exp.IsSyncCommAggregator(ctx, f.eth2Cl, selection.SelectionProof) - if err != nil { - return core.UnsignedDataSet{}, err - } else if !ok { + contribs = append(contribs, contrib) + + if !syncContribV2 { + // Backwards-compatible mode: a single contribution per validator + // (the lowest aggregated subcommittee, subcommIdxs is sorted). + break + } + } + + if len(contribs) == 0 { pt.addNotSelected(pubkey.String()) continue } - // Query AggSigDB for DutySyncMessage to get beacon block root. - syncMsgData, err := f.aggSigDBFunc(ctx, core.NewSyncMessageDuty(slot), pubkey) - if err != nil { - return core.UnsignedDataSet{}, err - } + pt.addResolved(pubkey.String()) - msg, ok := syncMsgData.(core.SignedSyncMessage) - if !ok { - return core.UnsignedDataSet{}, errors.New("invalid sync committee message") + if syncContribV2 { + resp[pubkey] = contribs + } else { + resp[pubkey] = contribs[0] // Single SyncContribution (old wire format). } + } - blockRoot := msg.BeaconBlockRoot + return resp, nil +} - // Query BN for sync committee contribution. - opts := ð2api.SyncCommitteeContributionOpts{ - Slot: eth2p0.Slot(slot), - SubcommitteeIndex: subcommIdx, - BeaconBlockRoot: blockRoot, - } +// fetchSubcommContribution fetches the sync committee contribution for a single +// validator and subcommittee. It returns ok=false if the validator is not an +// aggregator for the subcommittee. +func (f *Fetcher) fetchSubcommContribution(ctx context.Context, slot uint64, pubkey core.PubKey, subcommIdx core.SubcommitteeIndex) (core.SyncContribution, bool, error) { + // Query AggSigDB for DutyPrepareSyncContribution to get the sync committee selection. + selectionData, err := f.aggSigDBFunc(ctx, core.NewPrepareSyncContributionDuty(slot), pubkey, subcommIdx) + if err != nil { + return core.SyncContribution{}, false, err + } - eth2Resp, err := f.eth2Cl.SyncCommitteeContribution(ctx, opts) - if err != nil { - return core.UnsignedDataSet{}, err - } + selection, ok := selectionData.(core.SyncCommitteeSelection) + if !ok { + return core.SyncContribution{}, false, errors.New("invalid sync committee selection") + } - contribution := eth2Resp.Data - if contribution == nil { - // Some beacon nodes return nil if the beacon block root is not found for the subcommittee, return retryable error. - // This could happen if the beacon node didn't subscribe to the correct subnet. - return core.UnsignedDataSet{}, errors.New("sync committee contribution not found by root (retryable)", z.U64("subcommidx", subcommIdx), z.Hex("root", blockRoot[:])) - } + // Check if the validator is an aggregator for the subcommittee. + ok, err = eth2exp.IsSyncCommAggregator(ctx, f.eth2Cl, selection.SelectionProof) + if err != nil { + return core.SyncContribution{}, false, err + } else if !ok { + return core.SyncContribution{}, false, nil + } - pt.addResolved(pubkey.String()) + // Query AggSigDB for DutySyncMessage to get the beacon block root. + syncMsgData, err := f.aggSigDBFunc(ctx, core.NewSyncMessageDuty(slot), pubkey, 0) + if err != nil { + return core.SyncContribution{}, false, err + } + + msg, ok := syncMsgData.(core.SignedSyncMessage) + if !ok { + return core.SyncContribution{}, false, errors.New("invalid sync committee message") + } + + blockRoot := msg.BeaconBlockRoot + + // Query BN for the sync committee contribution. + opts := ð2api.SyncCommitteeContributionOpts{ + Slot: eth2p0.Slot(slot), + SubcommitteeIndex: uint64(subcommIdx), + BeaconBlockRoot: blockRoot, + } - resp[pubkey] = core.SyncContribution{ - SyncCommitteeContribution: *contribution, + eth2Resp, err := f.eth2Cl.SyncCommitteeContribution(ctx, opts) + if err != nil { + return core.SyncContribution{}, false, err + } + + contribution := eth2Resp.Data + if contribution == nil { + // Some beacon nodes return nil if the beacon block root is not found for the subcommittee, return retryable error. + // This could happen if the beacon node didn't subscribe to the correct subnet. + return core.SyncContribution{}, false, errors.New("sync committee contribution not found by root (retryable)", z.U64("subcommidx", uint64(subcommIdx)), z.Hex("root", blockRoot[:])) + } + + return core.SyncContribution{SyncCommitteeContribution: *contribution}, true, nil +} + +// syncSubcommitteeSize returns the number of validators per sync subcommittee +// (SYNC_COMMITTEE_SIZE / SYNC_COMMITTEE_SUBNET_COUNT), used to map a sync +// committee position to its subcommittee index. +func (f *Fetcher) syncSubcommitteeSize(ctx context.Context) (uint64, error) { + eth2Resp, err := f.eth2Cl.Spec(ctx, ð2api.SpecOpts{}) + if err != nil { + return 0, err + } + + commSize, ok := eth2Resp.Data["SYNC_COMMITTEE_SIZE"].(uint64) + if !ok { + return 0, errors.New("invalid SYNC_COMMITTEE_SIZE") + } + + subnetCount, ok := eth2Resp.Data["SYNC_COMMITTEE_SUBNET_COUNT"].(uint64) + if !ok || subnetCount == 0 { + return 0, errors.New("invalid SYNC_COMMITTEE_SUBNET_COUNT") + } + + subcommSize := commSize / subnetCount + if subcommSize == 0 { + // Guard against a division by zero in syncSubcommittees; a valid spec has + // SYNC_COMMITTEE_SIZE (512) well above SYNC_COMMITTEE_SUBNET_COUNT (4). + return 0, errors.New("invalid sync subcommittee size", z.U64("comm_size", commSize), z.U64("subnet_count", subnetCount)) + } + + return subcommSize, nil +} + +// syncSubcommittees returns the sorted, unique sync subcommittee indices that the +// validator of the provided sync committee duty definition occupies. subcommSize +// is the number of validators per subcommittee. +func syncSubcommittees(def core.DutyDefinition, subcommSize uint64) ([]core.SubcommitteeIndex, error) { + syncDef, ok := def.(core.SyncCommitteeDefinition) + if !ok { + return nil, errors.New("invalid sync committee duty definition") + } + + seen := make(map[core.SubcommitteeIndex]bool) + + var subcommIdxs []core.SubcommitteeIndex + + for _, idx := range syncDef.ValidatorSyncCommitteeIndices { + subcommIdx := core.SubcommitteeIndex(uint64(idx) / subcommSize) + if seen[subcommIdx] { + continue } + + seen[subcommIdx] = true + + subcommIdxs = append(subcommIdxs, subcommIdx) } - return resp, nil + slices.Sort(subcommIdxs) + + return subcommIdxs, nil } // verifyFeeRecipient logs a warning when fee recipient is not correctly populated in the block. diff --git a/core/fetcher/fetcher_test.go b/core/fetcher/fetcher_test.go index 94a995463a..143d28caf5 100644 --- a/core/fetcher/fetcher_test.go +++ b/core/fetcher/fetcher_test.go @@ -162,7 +162,7 @@ func TestFetchAggregator(t *testing.T) { } fetch := mustCreateFetcher(t, bmock) - fetch.RegisterAggSigDB(func(ctx context.Context, duty core.Duty, key core.PubKey) (core.SignedData, error) { + fetch.RegisterAggSigDB(func(ctx context.Context, duty core.Duty, key core.PubKey, _ core.SubcommitteeIndex) (core.SignedData, error) { require.Equal(t, core.NewPrepareAggregatorDuty(slot), duty) return signedCommSubByPubKey[key], nil @@ -301,7 +301,7 @@ func TestFetchBlocks(t *testing.T) { duty := core.NewProposerDuty(slot) fetch := mustCreateFetcherWithAddressAndGraffiti(t, bmock, feeRecipientAddr, graffitiBuilder) - fetch.RegisterAggSigDB(func(ctx context.Context, duty core.Duty, key core.PubKey) (core.SignedData, error) { + fetch.RegisterAggSigDB(func(ctx context.Context, duty core.Duty, key core.PubKey, _ core.SubcommitteeIndex) (core.SignedData, error) { return randaoByPubKey[key], nil }) @@ -356,8 +356,12 @@ func TestFetchSyncContribution(t *testing.T) { slot = 1 vIdxA = 2 vIdxB = 3 - subCommIdxA = 4 - subCommIdxB = 5 + subCommIdxA = 0 + subCommIdxB = 1 + // subcommitteeSize is SYNC_COMMITTEE_SIZE / SYNC_COMMITTEE_SUBNET_COUNT + // (512 / 4) for the mainnet spec used by beaconmock. A sync committee + // position maps to subcommittee = position / subcommitteeSize. + subcommitteeSize = 128 ) var ( @@ -371,10 +375,18 @@ func TestFetchSyncContribution(t *testing.T) { vIdxB: testutil.RandomCorePubKey(t), } - // Construct duty definition set. + // Construct duty definition set with sync committee positions that map to each + // validator's subcommittee (position / subcommitteeSize). + dutyA := testutil.RandomSyncCommitteeDuty(t) + dutyA.ValidatorIndex = vIdxA + dutyA.ValidatorSyncCommitteeIndices = []eth2p0.CommitteeIndex{eth2p0.CommitteeIndex(subCommIdxA * subcommitteeSize)} + dutyB := testutil.RandomSyncCommitteeDuty(t) + dutyB.ValidatorIndex = vIdxB + dutyB.ValidatorSyncCommitteeIndices = []eth2p0.CommitteeIndex{eth2p0.CommitteeIndex(subCommIdxB * subcommitteeSize)} + defSet := map[core.PubKey]core.DutyDefinition{ - pubkeysByIdx[vIdxA]: core.NewSyncCommitteeDefinition(testutil.RandomSyncCommitteeDuty(t)), - pubkeysByIdx[vIdxB]: core.NewSyncCommitteeDefinition(testutil.RandomSyncCommitteeDuty(t)), + pubkeysByIdx[vIdxA]: core.NewSyncCommitteeDefinition(dutyA), + pubkeysByIdx[vIdxB]: core.NewSyncCommitteeDefinition(dutyB), } var ( @@ -455,7 +467,9 @@ func TestFetchSyncContribution(t *testing.T) { } fetch := mustCreateFetcher(t, bmock) - fetch.RegisterAggSigDB(func(ctx context.Context, duty core.Duty, key core.PubKey) (core.SignedData, error) { + // All peers on v1.11+ -> plural SyncContributions encoding. + fetch.RegisterSyncContributionV2(func(uint64) bool { return true }) + fetch.RegisterAggSigDB(func(ctx context.Context, duty core.Duty, key core.PubKey, _ core.SubcommitteeIndex) (core.SignedData, error) { if duty.Type == core.DutyPrepareSyncContribution { require.Equal(t, core.NewPrepareSyncContributionDuty(slot), duty) return commSelectionsByPubkey[key], nil @@ -474,8 +488,11 @@ func TestFetchSyncContribution(t *testing.T) { require.Len(t, resDataSet, 2) for pubkey, data := range resDataSet { - contribution, ok := data.(core.SyncContribution) + contributions, ok := data.(core.SyncContributions) require.True(t, ok) + require.Len(t, contributions, 1) // Each validator aggregates a single subcommittee here. + + contribution := contributions[0] require.Equal(t, eth2p0.Slot(slot), contribution.Slot) selection, ok := commSelectionsByPubkey[pubkey].(core.SyncCommitteeSelection) @@ -508,12 +525,124 @@ func TestFetchSyncContribution(t *testing.T) { require.NoError(t, err) }) + t.Run("backwards compatible single contribution", func(t *testing.T) { + bmock, err := beaconmock.New(t.Context()) + require.NoError(t, err) + + bmock.SyncCommitteeContributionFunc = func(_ context.Context, _ eth2p0.Slot, subcommitteeIndex uint64, beaconBlockRoot eth2p0.Root) (*altair.SyncCommitteeContribution, error) { + return &altair.SyncCommitteeContribution{ + Slot: slot, + BeaconBlockRoot: beaconBlockRoot, + SubcommitteeIndex: subcommitteeIndex, + AggregationBits: testutil.RandomBitVec128(), + Signature: testutil.RandomEth2Signature(), + }, nil + } + + fetch := mustCreateFetcher(t, bmock) + // No RegisterSyncContribV2 → gate off → single (pre-v1.11) encoding. + fetch.RegisterAggSigDB(func(_ context.Context, duty core.Duty, key core.PubKey, _ core.SubcommitteeIndex) (core.SignedData, error) { + if duty.Type == core.DutyPrepareSyncContribution { + return commSelectionsByPubkey[key], nil + } + + if duty.Type == core.DutySyncMessage { + return syncMsgsByPubkey[key], nil + } + + return nil, errors.New("unsupported duty") + }) + + fetch.Subscribe(func(_ context.Context, _ core.Duty, resDataSet core.UnsignedDataSet) error { + require.Len(t, resDataSet, 2) + + for _, data := range resDataSet { + // Single SyncContribution, not the plural SyncContributions. + _, ok := data.(core.SyncContribution) + require.True(t, ok) + } + + return nil + }) + + err = fetch.Fetch(ctx, duty, defSet) + require.NoError(t, err) + }) + + t.Run("aggregator in multiple subcommittees", func(t *testing.T) { + // The bug scenario: a single validator occupies two sync subcommittees in + // the same slot and must produce a distinct contribution for each. + pubkey := pubkeysByIdx[vIdxA] + + multiDuty := testutil.RandomSyncCommitteeDuty(t) + multiDuty.ValidatorIndex = vIdxA + multiDuty.ValidatorSyncCommitteeIndices = []eth2p0.CommitteeIndex{ + eth2p0.CommitteeIndex(subCommIdxA * subcommitteeSize), // subcommittee 0 + eth2p0.CommitteeIndex(subCommIdxB * subcommitteeSize), // subcommittee 1 + } + multiDefSet := map[core.PubKey]core.DutyDefinition{ + pubkey: core.NewSyncCommitteeDefinition(multiDuty), + } + + // An aggregator selection proof per subcommittee (both sigA and sigB are aggregators). + selectionsBySubcomm := map[core.SubcommitteeIndex]core.SignedData{ + subCommIdxA: core.NewSyncCommitteeSelection(ð2v1.SyncCommitteeSelection{ + ValidatorIndex: vIdxA, Slot: slot, SubcommitteeIndex: subCommIdxA, SelectionProof: blsSigFromHex(t, sigA), + }), + subCommIdxB: core.NewSyncCommitteeSelection(ð2v1.SyncCommitteeSelection{ + ValidatorIndex: vIdxA, Slot: slot, SubcommitteeIndex: subCommIdxB, SelectionProof: blsSigFromHex(t, sigB), + }), + } + + bmock, err := beaconmock.New(t.Context()) + require.NoError(t, err) + + bmock.SyncCommitteeContributionFunc = func(_ context.Context, _ eth2p0.Slot, subcommitteeIndex uint64, beaconBlockRoot eth2p0.Root) (*altair.SyncCommitteeContribution, error) { + return &altair.SyncCommitteeContribution{ + Slot: slot, + BeaconBlockRoot: beaconBlockRoot, + SubcommitteeIndex: subcommitteeIndex, + AggregationBits: testutil.RandomBitVec128(), + Signature: testutil.RandomEth2Signature(), + }, nil + } + + fetch := mustCreateFetcher(t, bmock) + fetch.RegisterSyncContributionV2(func(uint64) bool { return true }) // Plural encoding. + fetch.RegisterAggSigDB(func(_ context.Context, duty core.Duty, _ core.PubKey, subcommIdx core.SubcommitteeIndex) (core.SignedData, error) { + switch duty.Type { + case core.DutyPrepareSyncContribution: + return selectionsBySubcomm[subcommIdx], nil + case core.DutySyncMessage: + return syncMsgsByPubkey[pubkey], nil + default: + return nil, errors.New("unsupported duty") + } + }) + + fetch.Subscribe(func(_ context.Context, _ core.Duty, resDataSet core.UnsignedDataSet) error { + require.Len(t, resDataSet, 1) + + contribs, ok := resDataSet[pubkey].(core.SyncContributions) + require.True(t, ok) + require.Len(t, contribs, 2) // One contribution per subcommittee - the fix. + + require.ElementsMatch(t, []uint64{subCommIdxA, subCommIdxB}, + []uint64{contribs[0].SubcommitteeIndex, contribs[1].SubcommitteeIndex}) + + return nil + }) + + err = fetch.Fetch(ctx, duty, multiDefSet) + require.NoError(t, err) + }) + t.Run("not contribution aggregator", func(t *testing.T) { bmock, err := beaconmock.New(t.Context()) require.NoError(t, err) fetch := mustCreateFetcher(t, bmock) - fetch.RegisterAggSigDB(func(ctx context.Context, duty core.Duty, key core.PubKey) (core.SignedData, error) { + fetch.RegisterAggSigDB(func(ctx context.Context, duty core.Duty, key core.PubKey, _ core.SubcommitteeIndex) (core.SignedData, error) { if duty.Type == core.DutyPrepareSyncContribution { require.Equal(t, core.NewPrepareSyncContributionDuty(slot), duty) @@ -539,7 +668,7 @@ func TestFetchSyncContribution(t *testing.T) { require.NoError(t, err) fetch := mustCreateFetcher(t, bmock) - fetch.RegisterAggSigDB(func(ctx context.Context, duty core.Duty, key core.PubKey) (core.SignedData, error) { + fetch.RegisterAggSigDB(func(ctx context.Context, duty core.Duty, key core.PubKey, _ core.SubcommitteeIndex) (core.SignedData, error) { return nil, errors.New("error") }) diff --git a/core/infosync/infosync.go b/core/infosync/infosync.go index c8f6e51039..c483dbe8f2 100644 --- a/core/infosync/infosync.go +++ b/core/infosync/infosync.go @@ -27,6 +27,12 @@ const ( maxResults = 100 TopicProtocol = topicProtocol + + // minSyncContributionV2Version is the minimum charon version that supports the plural + // SyncContributions encoding for DutySyncContribution. The cluster uses that + // encoding once the quorum-agreed version topic includes at least this version, + // i.e. once at least a threshold of peers advertise it (see SyncContributionsSupported). + minSyncContributionV2Version = "v1.11" ) // New returns a new infosync component. @@ -72,6 +78,19 @@ func New(prioritiser *priority.Component, versions []version.SemVer, protocols [ c.addResult(res) } + // Gate the plural SyncContributions encoding on the cluster-agreed versions + // including one >= minSyncContributionV2Version. Since the version topic is quorum + // filtered, this means at least threshold peers are on that version, which + // is also the number needed to complete the duty; any smaller set of lagging + // peers simply won't participate in sync contributions (they can't decode the + // plural form) rather than blocking the duty. + enabled, err := anyVersionAtLeast(res.versions, minSyncContributionV2Version) + if err != nil { + return err + } + + c.addSyncContribResult(syncContribResult{slot: duty.Slot, enabled: enabled}) + return nil }) @@ -84,8 +103,9 @@ type Component struct { protocols []protocol.ID proposals []core.ProposalType - mu sync.Mutex - results []result + mu sync.Mutex + results []result + syncContribResults []syncContribResult } // Protocols returns the latest cluster wide supported protocols before the slot. @@ -126,6 +146,49 @@ func (c *Component) Proposals(slot uint64) []core.ProposalType { return resp } +// SyncContributionsSupported reports whether, per the latest info-sync round at +// or before the slot, the cluster-agreed (quorum-filtered) version topic includes +// a charon version >= minSyncContributionV2Version - i.e. whether at least a threshold of +// peers advertised it, and the cluster can use the plural SyncContributions +// encoding for DutySyncContribution. It defaults to false (the backwards-compatible +// single encoding) until a round has completed. Note this is a quorum signal, not +// all-peers: a sub-threshold set of lagging peers is not reflected here and will +// simply not participate in sync contributions rather than blocking them. +func (c *Component) SyncContributionsSupported(slot uint64) bool { + c.mu.Lock() + defer c.mu.Unlock() + + var resp bool + + for _, result := range c.syncContribResults { + if result.slot > slot { + break + } + + resp = result.enabled + } + + return resp +} + +// addSyncContribResult adds the sync-contribution gate result if it differs from the last. +func (c *Component) addSyncContribResult(result syncContribResult) { + c.mu.Lock() + defer c.mu.Unlock() + + last := len(c.syncContribResults) - 1 + if last >= 0 && c.syncContribResults[last].enabled == result.enabled { + // Same decision as previous, so don't add (keep the earliest slot it became true/false). + return + } + + c.syncContribResults = append(c.syncContribResults, result) + + if len(c.syncContribResults) >= maxResults { + c.syncContribResults = c.syncContribResults[1:] + } +} + // addResult adds the result to the results if it is different from the last result. func (c *Component) addResult(result result) { c.mu.Lock() @@ -190,6 +253,38 @@ func proposalsToStrings(proposals []core.ProposalType) []string { return resp } +// syncContribResult is a cluster-wide agreed-upon decision, for a given slot, on +// whether a threshold of peers support the plural SyncContributions encoding. +type syncContribResult struct { + slot uint64 + enabled bool +} + +// anyVersionAtLeast reports whether any of the cluster-agreed versions is at +// least minVer. The version topic is quorum filtered, so a version only appears +// here if at least threshold peers support it; the presence of one >= minVer +// therefore means at least threshold peers are on that version. Unparseable +// versions are ignored. +func anyVersionAtLeast(versions []string, minVer string) (bool, error) { + minSemVer, err := version.Parse(minVer) + if err != nil { + return false, err + } + + for _, v := range versions { + semVer, err := version.Parse(v) + if err != nil { + continue // Ignore unparseable versions from peers. + } + + if version.Compare(semVer, minSemVer) >= 0 { + return true, nil + } + } + + return false, nil +} + // result is a cluster-wide agreed-upon infosync result. type result struct { slot uint64 diff --git a/core/infosync/infosync_internal_test.go b/core/infosync/infosync_internal_test.go new file mode 100644 index 0000000000..720471d751 --- /dev/null +++ b/core/infosync/infosync_internal_test.go @@ -0,0 +1,79 @@ +// Copyright © 2022-2026 Obol Labs Inc. Licensed under the terms of a Business Source License 1.1 + +package infosync + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestAnyVersionAtLeast(t *testing.T) { + tests := []struct { + name string + versions []string + want bool + }{ + { + name: "contains v1.11", + versions: []string{"v1.11", "v1.10", "v1.9"}, + want: true, + }, + { + name: "only older versions", + versions: []string{"v1.10", "v1.9"}, + want: false, + }, + { + name: "future minor qualifies", + versions: []string{"v1.20.3-dev", "v1.10"}, + want: true, + }, + { + name: "future major qualifies", + versions: []string{"v2.0.0"}, + want: true, + }, + { + name: "pre-release qualifies", + versions: []string{"v1.11.0-rc1"}, + want: true, + }, + { + name: "unparseable ignored", + versions: []string{"garbage", "v1.10"}, + want: false, + }, + { + name: "empty stays off", + versions: nil, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := anyVersionAtLeast(tt.versions, minSyncContributionV2Version) + require.NoError(t, err) + require.Equal(t, tt.want, got) + }) + } +} + +func TestSyncContributionsSupported(t *testing.T) { + c := new(Component) + + // Defaults to false (backwards-compatible single encoding) before any round. + require.False(t, c.SyncContributionsSupported(10)) + + c.addSyncContribResult(syncContribResult{slot: 5, enabled: true}) + require.False(t, c.SyncContributionsSupported(4)) // Before first result. + require.True(t, c.SyncContributionsSupported(5)) + require.True(t, c.SyncContributionsSupported(100)) + + // A later round downgrades (e.g. an old peer joined). + c.addSyncContribResult(syncContribResult{slot: 8, enabled: false}) + require.True(t, c.SyncContributionsSupported(7)) + require.False(t, c.SyncContributionsSupported(8)) + require.False(t, c.SyncContributionsSupported(100)) +} diff --git a/core/interfaces.go b/core/interfaces.go index 2ce712072d..73fdd3af29 100644 --- a/core/interfaces.go +++ b/core/interfaces.go @@ -41,8 +41,12 @@ type Fetcher interface { Subscribe(func(context.Context, Duty, UnsignedDataSet) error) // RegisterAggSigDB registers a function to get resolved aggregated - // signed data from the AggSigDB (e.g., randao reveals). - RegisterAggSigDB(func(context.Context, Duty, PubKey) (SignedData, error)) + // signed data from the AggSigDB (e.g., randao reveals). The SubcommitteeIndex + // selects the sync subcommittee for sync-committee aggregator duties + // (DutyPrepareSyncContribution, DutySyncContribution) since a validator can + // have a distinct aggregated signature per subcommittee in a slot; it is 0 + // for all other duties. + RegisterAggSigDB(func(context.Context, Duty, PubKey, SubcommitteeIndex) (SignedData, error)) // RegisterAwaitAttData registers a function to get attestation data from DutyDB. RegisterAwaitAttData(func(ctx context.Context, slot uint64, commIdx uint64) (*eth2p0.AttestationData, error)) @@ -142,7 +146,10 @@ type ValidatorAPI interface { RegisterAwaitAggAttestation(fn func(ctx context.Context, slot uint64, attestationDataRoot eth2p0.Root, committeeIndex eth2p0.CommitteeIndex) (*eth2spec.VersionedAttestation, error)) // RegisterAwaitAggSigDB registers a function to query aggregated signed data from aggSigDB. - RegisterAwaitAggSigDB(func(context.Context, Duty, PubKey) (SignedData, error)) + // The SubcommitteeIndex selects the sync subcommittee for sync-committee aggregator duties + // (DutyPrepareSyncContribution, DutySyncContribution), where a validator can have a distinct + // aggregated signature per subcommittee in a slot; it is 0 for all other duties. + RegisterAwaitAggSigDB(func(context.Context, Duty, PubKey, SubcommitteeIndex) (SignedData, error)) // Subscribe registers a function to store partially signed data sets. Subscribe(func(context.Context, Duty, ParSignedDataSet) error) @@ -191,7 +198,11 @@ type AggSigDB interface { Store(context context.Context, duty Duty, data SignedDataSet) error // Await blocks and returns the aggregated signed duty data when available. - Await(context context.Context, duty Duty, pubKey PubKey) (SignedData, error) + // subcommIdx selects the sync subcommittee for sync-committee aggregator duties + // (DutyPrepareSyncContribution, DutySyncContribution), where a validator can have + // a distinct aggregated signature per subcommittee in a slot; it is 0 for all + // other duties (see IsSyncSubcommitteeDuty). + Await(context context.Context, duty Duty, pubKey PubKey, subcommIdx SubcommitteeIndex) (SignedData, error) // Run runs AggSigDB lifecycle until context is cancelled. Run(context context.Context) @@ -252,7 +263,7 @@ type wireFuncs struct { FetcherFetch func(context.Context, Duty, DutyDefinitionSet) error FetcherFetchOnly func(context.Context, Duty, DutyDefinitionSet, string, eth2p0.Root) error FetcherSubscribe func(func(context.Context, Duty, UnsignedDataSet) error) - FetcherRegisterAggSigDB func(func(context.Context, Duty, PubKey) (SignedData, error)) + FetcherRegisterAggSigDB func(func(context.Context, Duty, PubKey, SubcommitteeIndex) (SignedData, error)) FetcherRegisterAwaitAttData func(func(ctx context.Context, slot uint64, commIdx uint64) (*eth2p0.AttestationData, error)) ConsensusParticipate func(context.Context, Duty) error ConsensusPropose func(context.Context, Duty, UnsignedDataSet) error @@ -269,7 +280,7 @@ type wireFuncs struct { VAPIRegisterGetDutyDefinition func(func(context.Context, Duty) (DutyDefinitionSet, error)) VAPIRegisterPubKeyByAttestation func(func(ctx context.Context, slot, commIdx, valIdx uint64) (PubKey, error)) VAPIRegisterAwaitAggAttestation func(func(ctx context.Context, slot uint64, attestationRoot eth2p0.Root, committeeIndex eth2p0.CommitteeIndex) (*eth2spec.VersionedAttestation, error)) - VAPIRegisterAwaitAggSigDB func(func(context.Context, Duty, PubKey) (SignedData, error)) + VAPIRegisterAwaitAggSigDB func(func(context.Context, Duty, PubKey, SubcommitteeIndex) (SignedData, error)) VAPISubscribe func(func(context.Context, Duty, ParSignedDataSet) error) ParSigDBStoreInternal func(context.Context, Duty, ParSignedDataSet) error ParSigDBStoreExternal func(context.Context, Duty, ParSignedDataSet) error @@ -280,7 +291,7 @@ type wireFuncs struct { SigAggAggregate func(context.Context, Duty, map[PubKey][]ParSignedData) error SigAggSubscribe func(func(context.Context, Duty, SignedDataSet) error) AggSigDBStore func(context.Context, Duty, SignedDataSet) error - AggSigDBAwait func(context.Context, Duty, PubKey) (SignedData, error) + AggSigDBAwait func(context.Context, Duty, PubKey, SubcommitteeIndex) (SignedData, error) BroadcasterBroadcast func(context.Context, Duty, SignedDataSet) error } diff --git a/core/parsigdb/memory.go b/core/parsigdb/memory.go index 9590dd474d..45370f226d 100644 --- a/core/parsigdb/memory.go +++ b/core/parsigdb/memory.go @@ -134,7 +134,12 @@ func (db *MemDB) StoreExternal(ctx context.Context, duty core.Duty, signedSet co output := make(map[core.PubKey][]core.ParSignedData) for pubkey, sig := range signedSet { - sigs, ok, err := db.store(ctx, key{Duty: duty, PubKey: pubkey}, sig, exempt) + subcommIdx, err := core.SyncSubcommitteeIndex(duty.Type, sig.SignedData) + if err != nil { + return err + } + + sigs, ok, err := db.store(ctx, key{Duty: duty, PubKey: pubkey, SubcommIdx: subcommIdx}, sig, exempt) if err != nil { return err } else if !ok { @@ -324,6 +329,11 @@ func parSignedDataEqual(x, y core.ParSignedData) (bool, error) { type key struct { Duty core.Duty PubKey core.PubKey + // SubcommIdx is the sync subcommittee index for sync-committee aggregator + // duties (DutyPrepareSyncContribution, DutySyncContribution), and 0 otherwise. + // A validator can occupy multiple sync subcommittees in the same slot, so it + // disambiguates their otherwise-colliding partial signatures. + SubcommIdx core.SubcommitteeIndex } // exemptEntryKey indexes exempt-duty entries by share index, validator and duty type. diff --git a/core/proto_test.go b/core/proto_test.go index a8104ca341..c1a81745af 100644 --- a/core/proto_test.go +++ b/core/proto_test.go @@ -148,7 +148,7 @@ func TestUnsignedDataToProto(t *testing.T) { }, { Type: core.DutySyncContribution, - Data: core.NewSyncContribution(testutil.RandomSyncCommitteeContribution()), + Data: core.SyncContributions{core.NewSyncContribution(testutil.RandomSyncCommitteeContribution())}, }, } @@ -178,6 +178,44 @@ func TestUnsignedDataToProto(t *testing.T) { } } +// TestUnsignedDataSyncContributionCompat verifies the DutySyncContribution decoder +// is tolerant of both wire forms: the plural SyncContributions (v1.11+) and the +// single SyncContribution (pre-v1.11 / mixed-cluster backwards-compatible form). +func TestUnsignedDataSyncContributionCompat(t *testing.T) { + pubkey := testutil.RandomCorePubKey(t) + + tests := []struct { + name string + data core.UnsignedData + }{ + { + name: "single", + data: core.NewSyncContribution(testutil.RandomSyncCommitteeContribution()), + }, + { + name: "plural", + data: core.SyncContributions{ + core.NewSyncContribution(testutil.RandomSyncCommitteeContribution()), + core.NewSyncContribution(testutil.RandomSyncCommitteeContribution()), + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + set1 := core.UnsignedDataSet{pubkey: test.data} + + pb, err := core.UnsignedDataSetToProto(set1) + require.NoError(t, err) + + set2, err := core.UnsignedDataSetFromProto(core.DutySyncContribution, pb) + require.NoError(t, err) + + require.Equal(t, set1, set2) // Type and value preserved (no lossy normalisation). + }) + } +} + func TestParSignedData(t *testing.T) { for typ, signedData := range randomSignedData(t) { t.Run(typ.String(), func(t *testing.T) { diff --git a/core/signeddata.go b/core/signeddata.go index de872925b4..46a4907861 100644 --- a/core/signeddata.go +++ b/core/signeddata.go @@ -1307,6 +1307,25 @@ func NewPartialSignedSyncCommitteeSelection(selection *eth2v1.SyncCommitteeSelec } } +// SyncSubcommitteeIndex returns the sync subcommittee index that keys partial +// signatures for sync-committee aggregator duties (DutyPrepareSyncContribution +// and DutySyncContribution), read from the signed payload. It returns 0 for all +// other duty types, which are keyed by pubkey alone. +func SyncSubcommitteeIndex(typ DutyType, data SignedData) (SubcommitteeIndex, error) { + if !IsSyncSubcommitteeDuty(typ) { + return 0, nil + } + + switch d := data.(type) { + case SyncCommitteeSelection: + return SubcommitteeIndex(d.SubcommitteeIndex), nil + case SignedSyncContributionAndProof: + return SubcommitteeIndex(d.Message.Contribution.SubcommitteeIndex), nil + default: + return 0, errors.New("signed data is not a sync-committee aggregator duty") + } +} + // SyncCommitteeSelection wraps an eth2exp.SyncCommitteeSelection and implements SignedData. type SyncCommitteeSelection struct { eth2v1.SyncCommitteeSelection diff --git a/core/signeddata_test.go b/core/signeddata_test.go index d50c66dcdb..32149bf09b 100644 --- a/core/signeddata_test.go +++ b/core/signeddata_test.go @@ -1344,3 +1344,20 @@ func TestCloneSSZMarshaler(t *testing.T) { }) } } + +func TestSyncSubcommitteeIndex(t *testing.T) { + sel := testutil.RandomCoreSyncCommitteeSelection() + idx, err := core.SyncSubcommitteeIndex(core.DutyPrepareSyncContribution, sel) + require.NoError(t, err) + require.EqualValues(t, sel.SubcommitteeIndex, idx) + + contrib := testutil.RandomCoreSignedSyncContributionAndProof() + idx, err = core.SyncSubcommitteeIndex(core.DutySyncContribution, contrib) + require.NoError(t, err) + require.EqualValues(t, contrib.Message.Contribution.SubcommitteeIndex, idx) + + // Non-sync duty types are keyed by pubkey alone (index 0). + idx, err = core.SyncSubcommitteeIndex(core.DutyAttester, testutil.RandomCoreBeaconCommitteeSelection()) + require.NoError(t, err) + require.Zero(t, idx) +} diff --git a/core/ssz_test.go b/core/ssz_test.go index a316d59ea9..17a7764c92 100644 --- a/core/ssz_test.go +++ b/core/ssz_test.go @@ -121,7 +121,7 @@ func TestMarshalUnsignedProto(t *testing.T) { }, { dutyType: core.DutySyncContribution, - unsignedPtr: func() any { return new(core.SyncContribution) }, + unsignedPtr: func() any { return new(core.SyncContributions) }, }, } diff --git a/core/tracing.go b/core/tracing.go index 0a77a353b9..3519329a55 100644 --- a/core/tracing.go +++ b/core/tracing.go @@ -134,11 +134,11 @@ func WithTracing() WireOption { return withSpanStatus(span, clone.AggSigDBStore(ctx, duty, set)) } - w.AggSigDBAwait = func(parent context.Context, duty Duty, key PubKey) (SignedData, error) { + w.AggSigDBAwait = func(parent context.Context, duty Duty, key PubKey, subcommIdx SubcommitteeIndex) (SignedData, error) { ctx, span := tracer.Start(parent, "core/aggsigdb.Await") defer span.End() - sd, err := clone.AggSigDBAwait(ctx, duty, key) + sd, err := clone.AggSigDBAwait(ctx, duty, key, subcommIdx) return sd, withSpanStatus(span, err) } diff --git a/core/types.go b/core/types.go index 7538ca3636..cd4b6f3243 100644 --- a/core/types.go +++ b/core/types.go @@ -320,6 +320,21 @@ func (k PubKey) ToETH2() (eth2p0.BLSPubKey, error) { return resp, nil } +// SubcommitteeIndex identifies a sync committee subcommittee (0 to +// SYNC_COMMITTEE_SUBNET_COUNT-1). A validator can occupy multiple sync +// subcommittees in the same slot, each producing its own partial signature, so +// it forms part of the partial-signature key for sync-committee aggregator +// duties (see IsSyncSubcommitteeDuty). It is 0 for all other duty types. +type SubcommitteeIndex uint64 + +// IsSyncSubcommitteeDuty reports whether the duty type's partial signatures are +// keyed by sync subcommittee index (in addition to pubkey). A validator can +// occupy multiple sync subcommittees in the same slot, each producing its own +// partial signature, so the subcommittee index disambiguates them. +func IsSyncSubcommitteeDuty(typ DutyType) bool { + return typ == DutyPrepareSyncContribution || typ == DutySyncContribution +} + // DutyDefinition defines the duty including parameters required // to fetch the duty data, it is the result of resolving duties // at the start of an epoch. diff --git a/core/unsigneddata.go b/core/unsigneddata.go index 6ad42dc157..f8ab3e5cc2 100644 --- a/core/unsigneddata.go +++ b/core/unsigneddata.go @@ -31,6 +31,7 @@ var ( _ UnsignedData = VersionedAggregatedAttestation{} _ UnsignedData = VersionedProposal{} _ UnsignedData = SyncContribution{} + _ UnsignedData = SyncContributions{} // Some types also support SSZ marshalling and unmarshalling. _ ssz.Marshaler = AttestationData{} @@ -662,6 +663,51 @@ func (s *SyncContribution) UnmarshalSSZ(b []byte) error { return s.SyncCommitteeContribution.UnmarshalSSZ(b) } +// SyncContributions is a validator's set of sync committee contributions for a +// slot: one per subcommittee it aggregates. A validator can occupy multiple sync +// subcommittees in the same slot, each producing a distinct contribution +// (different members, aggregation bits and signature), so the unsigned data for +// DutySyncContribution is a collection while staying keyed by pubkey in the set. +// It is JSON-encoded (no SSZ) since it is a variable-length collection. +type SyncContributions []SyncContribution + +func (s SyncContributions) Clone() (UnsignedData, error) { + resp := make(SyncContributions, 0, len(s)) + + for _, contrib := range s { + cloned, err := contrib.Clone() + if err != nil { + return nil, err + } + + clonedContrib, ok := cloned.(SyncContribution) + if !ok { + return nil, errors.New("invalid cloned sync contribution") + } + + resp = append(resp, clonedContrib) + } + + return resp, nil +} + +func (s SyncContributions) MarshalJSON() ([]byte, error) { + b, err := json.Marshal([]SyncContribution(s)) + if err != nil { + return nil, errors.Wrap(err, "marshal sync contributions") + } + + return b, nil +} + +func (s *SyncContributions) UnmarshalJSON(input []byte) error { + if err := json.Unmarshal(input, (*[]SyncContribution)(s)); err != nil { + return errors.Wrap(err, "unmarshal sync contributions") + } + + return nil +} + // unmarshalUnsignedData returns an instantiated unsigned data based on the duty type. func unmarshalUnsignedData(typ DutyType, data []byte) (UnsignedData, error) { switch typ { @@ -692,12 +738,24 @@ func unmarshalUnsignedData(typ DutyType, data []byte) (UnsignedData, error) { return respVersioned, nil case DutySyncContribution: - var resp SyncContribution - if err := unmarshal(data, &resp); err != nil { + // A validator can aggregate for multiple sync subcommittees in a slot. + // v1.11+ nodes encode the set as plural SyncContributions (a JSON array); + // for backwards compatibility with older nodes (and while the cluster is + // mixed-version) the single-subcommittee case stays a single SyncContribution + // in the original wire format. Decode either: try the plural JSON array + // first (SyncContributions is JSON-only, so it fails cleanly on single + // SSZ/JSON bytes) and fall back to the single form. + var plural SyncContributions + if err := unmarshal(data, &plural); err == nil { + return plural, nil + } + + var single SyncContribution + if err := unmarshal(data, &single); err != nil { return nil, errors.Wrap(err, "unmarshal sync contribution") } - return resp, nil + return single, nil default: return nil, errors.New("unsupported unsigned data duty type") } diff --git a/core/validatorapi/validatorapi.go b/core/validatorapi/validatorapi.go index 9d06779e0d..2587fdd376 100644 --- a/core/validatorapi/validatorapi.go +++ b/core/validatorapi/validatorapi.go @@ -193,7 +193,7 @@ type Component struct { awaitProposalFunc func(ctx context.Context, slot uint64) (*eth2api.VersionedProposal, error) awaitSyncContributionFunc func(ctx context.Context, slot, subcommIdx uint64, beaconBlockRoot eth2p0.Root) (*altair.SyncCommitteeContribution, error) awaitAggAttFunc func(ctx context.Context, slot uint64, attestationRoot eth2p0.Root, committeeIndex eth2p0.CommitteeIndex) (*eth2spec.VersionedAttestation, error) - awaitAggSigDBFunc func(context.Context, core.Duty, core.PubKey) (core.SignedData, error) + awaitAggSigDBFunc func(context.Context, core.Duty, core.PubKey, core.SubcommitteeIndex) (core.SignedData, error) dutyDefFunc func(ctx context.Context, duty core.Duty) (core.DutyDefinitionSet, error) subs []func(context.Context, core.Duty, core.ParSignedDataSet) error } @@ -235,7 +235,7 @@ func (c *Component) RegisterAwaitAggAttestation(fn func(ctx context.Context, slo } // RegisterAwaitAggSigDB registers a function to query aggregated signed data from aggSigDB. -func (c *Component) RegisterAwaitAggSigDB(fn func(context.Context, core.Duty, core.PubKey) (core.SignedData, error)) { +func (c *Component) RegisterAwaitAggSigDB(fn func(context.Context, core.Duty, core.PubKey, core.SubcommitteeIndex) (core.SignedData, error)) { c.awaitAggSigDBFunc = fn } @@ -790,7 +790,7 @@ func (c Component) BeaconCommitteeSelections(ctx context.Context, opts *eth2api. duty := core.NewPrepareAggregatorDuty(uint64(slot)) for pk := range data { // Query aggregated subscription from aggsigdb for each duty and public key (this is blocking). - s, err := c.awaitAggSigDBFunc(ctx, duty, pk) + s, err := c.awaitAggSigDBFunc(ctx, duty, pk, 0) if err != nil { return nil, err } @@ -977,12 +977,16 @@ func (c Component) SubmitSyncCommitteeContributions(ctx context.Context, contrib return err } - psigsBySlot := make(map[eth2p0.Slot]core.ParSignedDataSet) + // A validator can aggregate for multiple sync subcommittees in the same slot, + // each with its own contribution. Group by (slot, subcommittee index) so a + // validator's contributions don't collide on the pubkey-keyed set. + psigsBySlotSubcomm := make(map[slotSubcomm]core.ParSignedDataSet) for _, contrib := range contributionAndProofs { var ( - slot = contrib.Message.Contribution.Slot - vIdx = contrib.Message.AggregatorIndex + slot = contrib.Message.Contribution.Slot + subcommIdx = core.SubcommitteeIndex(contrib.Message.Contribution.SubcommitteeIndex) + vIdx = contrib.Message.AggregatorIndex ) eth2Pubkey, ok := vals[vIdx] @@ -1013,16 +1017,16 @@ func (c Component) SubmitSyncCommitteeContributions(ctx context.Context, contrib return err } - _, ok = psigsBySlot[slot] - if !ok { - psigsBySlot[slot] = make(core.ParSignedDataSet) + key := slotSubcomm{Slot: slot, SubcommIdx: subcommIdx} + if _, ok := psigsBySlotSubcomm[key]; !ok { + psigsBySlotSubcomm[key] = make(core.ParSignedDataSet) } - psigsBySlot[slot][pk] = parSigData + psigsBySlotSubcomm[key][pk] = parSigData } - for slot, data := range psigsBySlot { - duty := core.NewSyncContributionDuty(uint64(slot)) + for key, data := range psigsBySlotSubcomm { + duty := core.NewSyncContributionDuty(uint64(key.Slot)) for _, sub := range c.subs { err = sub(ctx, duty, data) if err != nil { @@ -1041,7 +1045,10 @@ func (c Component) SyncCommitteeSelections(ctx context.Context, opts *eth2api.Sy return nil, err } - psigsBySlot := make(map[eth2p0.Slot]core.ParSignedDataSet) + // A validator can occupy multiple sync subcommittees in the same slot, each + // with its own selection. Group by (slot, subcommittee index) so a validator's + // selections don't collide on the pubkey-keyed set. + psigsBySlotSubcomm := make(map[slotSubcomm]core.ParSignedDataSet) for _, selection := range opts.Selections { eth2Pubkey, ok := vals[selection.ValidatorIndex] @@ -1062,16 +1069,16 @@ func (c Component) SyncCommitteeSelections(ctx context.Context, opts *eth2api.Sy return nil, err } - _, ok = psigsBySlot[selection.Slot] - if !ok { - psigsBySlot[selection.Slot] = make(core.ParSignedDataSet) + key := slotSubcomm{Slot: selection.Slot, SubcommIdx: core.SubcommitteeIndex(selection.SubcommitteeIndex)} + if _, ok := psigsBySlotSubcomm[key]; !ok { + psigsBySlotSubcomm[key] = make(core.ParSignedDataSet) } - psigsBySlot[selection.Slot][pubkey] = parSigData + psigsBySlotSubcomm[key][pubkey] = parSigData } - for slot, data := range psigsBySlot { - duty := core.NewPrepareSyncContributionDuty(uint64(slot)) + for key, data := range psigsBySlotSubcomm { + duty := core.NewPrepareSyncContributionDuty(uint64(key.Slot)) for _, sub := range c.subs { err = sub(ctx, duty, data) if err != nil { @@ -1082,11 +1089,11 @@ func (c Component) SyncCommitteeSelections(ctx context.Context, opts *eth2api.Sy var resp []*eth2v1.SyncCommitteeSelection - for slot, data := range psigsBySlot { - duty := core.NewPrepareSyncContributionDuty(uint64(slot)) + for key, data := range psigsBySlotSubcomm { + duty := core.NewPrepareSyncContributionDuty(uint64(key.Slot)) for pk := range data { - // Query aggregated sync committee selection from aggsigdb for each duty and public key (this is blocking). - s, err := c.awaitAggSigDBFunc(ctx, duty, pk) + // Query aggregated sync committee selection from aggsigdb for each duty, public key and subcommittee (this is blocking). + s, err := c.awaitAggSigDBFunc(ctx, duty, pk, key.SubcommIdx) if err != nil { return nil, err } @@ -1103,6 +1110,14 @@ func (c Component) SyncCommitteeSelections(ctx context.Context, opts *eth2api.Sy return wrapResponse(resp), nil } +// slotSubcomm groups sync-committee aggregator partial signatures by slot and +// sync subcommittee index, so a validator occupying multiple subcommittees in a +// slot does not collide on the pubkey-keyed ParSignedDataSet. +type slotSubcomm struct { + Slot eth2p0.Slot + SubcommIdx core.SubcommitteeIndex +} + // ProposerDuties obtains proposer duties for the given options. func (c Component) ProposerDuties(ctx context.Context, opts *eth2api.ProposerDutiesOpts) (*eth2api.Response[[]*eth2v1.ProposerDuty], error) { var span trace.Span diff --git a/core/validatorapi/validatorapi_test.go b/core/validatorapi/validatorapi_test.go index 392dd0a1ff..e347e6c455 100644 --- a/core/validatorapi/validatorapi_test.go +++ b/core/validatorapi/validatorapi_test.go @@ -1916,7 +1916,7 @@ func TestComponent_AggregateBeaconCommitteeSelections(t *testing.T) { }, } - vapi.RegisterAwaitAggSigDB(func(_ context.Context, duty core.Duty, pk core.PubKey) (core.SignedData, error) { + vapi.RegisterAwaitAggSigDB(func(_ context.Context, duty core.Duty, pk core.PubKey, _ core.SubcommitteeIndex) (core.SignedData, error) { require.Equal(t, core.NewPrepareAggregatorDuty(slot), duty) for _, val := range valSet { @@ -2055,6 +2055,7 @@ func TestComponent_SubmitSyncCommitteeContributions(t *testing.T) { pk, err := core.PubKeyFromBytes(pubkey[:]) require.NoError(t, err) + // Contributions are grouped by subcommittee, so the set is keyed by plain pubkey. data, ok := set[pk] require.True(t, ok) require.Equal(t, core.NewPartialSignedSyncContributionAndProof(contrib, 0), data) @@ -2385,7 +2386,7 @@ func TestComponent_AggregateSyncCommitteeSelectionsVerify(t *testing.T) { vapi, err := validatorapi.NewComponent(bmock, allPubSharesByKey, shareIdx, nil, false, 30000000) require.NoError(t, err) - vapi.RegisterAwaitAggSigDB(func(ctx context.Context, duty core.Duty, pubkey core.PubKey) (core.SignedData, error) { + vapi.RegisterAwaitAggSigDB(func(ctx context.Context, duty core.Duty, pubkey core.PubKey, subcommIdx core.SubcommitteeIndex) (core.SignedData, error) { require.Equal(t, core.NewPrepareSyncContributionDuty(slot), duty) for _, val := range valSet { @@ -2399,6 +2400,7 @@ func TestComponent_AggregateSyncCommitteeSelectionsVerify(t *testing.T) { for _, selection := range selections { if selection.ValidatorIndex == val.Index { require.Equal(t, eth2p0.Slot(slot), selection.Slot) + require.EqualValues(t, selection.SubcommitteeIndex, subcommIdx) return core.NewSyncCommitteeSelection(selection), nil } @@ -2408,15 +2410,14 @@ func TestComponent_AggregateSyncCommitteeSelectionsVerify(t *testing.T) { return nil, errors.New("unknown public key") }) + // A validator's selections are grouped by subcommittee, so subscribers are + // called once per subcommittee. Accumulate across calls and assert the union. + merged := core.ParSignedDataSet{} + vapi.Subscribe(func(ctx context.Context, duty core.Duty, set core.ParSignedDataSet) error { require.Equal(t, duty, core.NewPrepareSyncContributionDuty(slot)) - expect := core.ParSignedDataSet{ - pk1: core.NewPartialSignedSyncCommitteeSelection(selection1, shareIdx), - pk2: core.NewPartialSignedSyncCommitteeSelection(selection2, shareIdx), - } - - require.Equal(t, expect, set) + maps.Copy(merged, set) return nil }) @@ -2424,6 +2425,12 @@ func TestComponent_AggregateSyncCommitteeSelectionsVerify(t *testing.T) { eth2Resp, err := vapi.SyncCommitteeSelections(ctx, ð2api.SyncCommitteeSelectionsOpts{Selections: selections}) require.NoError(t, err) + expect := core.ParSignedDataSet{ + pk1: core.NewPartialSignedSyncCommitteeSelection(selection1, shareIdx), + pk2: core.NewPartialSignedSyncCommitteeSelection(selection2, shareIdx), + } + require.Equal(t, expect, merged) + got := eth2Resp.Data // Sort by VIdx before comparing. @@ -2434,6 +2441,95 @@ func TestComponent_AggregateSyncCommitteeSelectionsVerify(t *testing.T) { require.Equal(t, selections, got) } +// TestComponent_SyncCommitteeSelectionsMultiSubcommittee exercises the bug scenario: +// a single validator submitting selections for two different sync subcommittees in +// the same slot. Both must survive rather than colliding on (slot, pubkey). +func TestComponent_SyncCommitteeSelectionsMultiSubcommittee(t *testing.T) { + const ( + slot = 0 + shareIdx = 1 + vIdx = 1 + subcommA = 0 + subcommB = 1 + ) + + ctx := context.Background() + valSet := beaconmock.ValidatorSetA + + secret, err := tbls.GenerateSecretKey() + require.NoError(t, err) + + pubkey, err := tbls.SecretToPublicKey(secret) + require.NoError(t, err) + + pk, err := core.PubKeyFromBytes(pubkey[:]) + require.NoError(t, err) + + valSet[vIdx].Validator.PublicKey = eth2p0.BLSPubKey(pubkey) + + bmock, err := beaconmock.New(t.Context(), beaconmock.WithValidatorSet(valSet)) + require.NoError(t, err) + + // Two selections for the SAME validator in two different subcommittees. + newSelection := func(subcommIdx uint64) *eth2v1.SyncCommitteeSelection { + sel := testutil.RandomSyncCommitteeSelection() + sel.ValidatorIndex = valSet[vIdx].Index + sel.Slot = slot + sel.SubcommitteeIndex = subcommIdx + sel.SelectionProof = syncCommSelectionProof(t, bmock, secret, slot, subcommIdx) + + return sel + } + selections := []*eth2v1.SyncCommitteeSelection{newSelection(subcommA), newSelection(subcommB)} + + allPubSharesByKey := map[core.PubKey]map[int]tbls.PublicKey{pk: {shareIdx: pubkey}} + + vapi, err := validatorapi.NewComponent(bmock, allPubSharesByKey, shareIdx, nil, false, 30000000) + require.NoError(t, err) + + vapi.RegisterAwaitAggSigDB(func(_ context.Context, duty core.Duty, gotPk core.PubKey, subcommIdx core.SubcommitteeIndex) (core.SignedData, error) { + require.Equal(t, core.NewPrepareSyncContributionDuty(slot), duty) + require.Equal(t, pk, gotPk) + + for _, sel := range selections { + if sel.SubcommitteeIndex == uint64(subcommIdx) { + return core.NewSyncCommitteeSelection(sel), nil + } + } + + return nil, errors.New("selection not found") + }) + + // Selections are grouped by subcommittee, so subscribers are called once per + // subcommittee. Collect all stored entries and assert both survived. + stored := make(map[uint64]core.ParSignedData) + + vapi.Subscribe(func(_ context.Context, duty core.Duty, set core.ParSignedDataSet) error { + require.Equal(t, core.NewPrepareSyncContributionDuty(slot), duty) + require.Len(t, set, 1) // A single validator, keyed by plain pubkey per subcommittee group. + + data, ok := set[pk] + require.True(t, ok) + + sel, ok := data.SignedData.(core.SyncCommitteeSelection) + require.True(t, ok) + + stored[sel.SubcommitteeIndex] = data + + return nil + }) + + eth2Resp, err := vapi.SyncCommitteeSelections(ctx, ð2api.SyncCommitteeSelectionsOpts{Selections: selections}) + require.NoError(t, err) + + // Pre-fix these collided into a single entry; both must now be present. + require.Len(t, stored, 2) + require.Contains(t, stored, uint64(subcommA)) + require.Contains(t, stored, uint64(subcommB)) + + require.ElementsMatch(t, selections, eth2Resp.Data) +} + // syncCommSelectionProof returns the selection_proof corresponding to the provided altair.ContributionAndProof. // Refer get_sync_committee_selection_proof from https://github.com/ethereum/consensus-specs/blob/dev/specs/altair/validator.md#aggregation-selection. func syncCommSelectionProof(t *testing.T, eth2Cl eth2wrap.Client, secret tbls.PrivateKey, slot eth2p0.Slot, subcommIdx uint64) eth2p0.BLSSignature {