Skip to content

Aggregator Committee - Drop previous runners and align tests#592

Merged
GalRogozinski merged 21 commits intoaggregator-committeefrom
agg-comm-drop-previous-duty-types
Dec 24, 2025
Merged

Aggregator Committee - Drop previous runners and align tests#592
GalRogozinski merged 21 commits intoaggregator-committeefrom
agg-comm-drop-previous-duty-types

Conversation

@MatheusFranco99
Copy link
Contributor

Overview

This PR drops the AggregatorRunner and SyncCommitteeContributorRunner and align the associated tests to the AggregatorCommitteeRunner.

@MatheusFranco99 MatheusFranco99 self-assigned this Dec 17, 2025
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 17, 2025

Skipped: This PR changes more files than the configured file change limit: (1198 files found, 150 file limit)

Copy link
Contributor

@GalRogozinski GalRogozinski left a comment

Choose a reason for hiding this comment

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

Looks nice

Comment on lines +126 to +127
// It's only used for the Proposer role.
// TODO: rename it to associate with Proposer role
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I added this one. But I think this should belong to a dedicated PR, what do you think?

case BNRoleSyncCommitteeContribution:
return RoleSyncCommitteeContribution // Still use old runner for now
case BNRoleAggregator, BNRoleSyncCommitteeContribution:
return RoleAggregatorCommittee // Still use old runner for now
Copy link
Contributor

Choose a reason for hiding this comment

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

the comment is correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. Removed it

@@ -48,12 +48,11 @@ const (

BNRoleValidatorRegistration
BNRoleVoluntaryExit
BNRoleAggregatorCommittee // Internal SSV role combining Aggregator + SyncCommitteeContribution
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't get what I am seeing on Github
This is deleted but later used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BNRoleAggregatorCommittee was added in the original PR but I removed it here because it's not a beacon role, only a runner role 😅

@@ -180,7 +180,7 @@ func AggregatorCommitteeValueCheckF(
return func(data []byte) error {
cd := &types.AggregatorCommitteeConsensusData{}
if err := cd.Decode(data); err != nil {
return errors.Wrap(err, "failed decoding aggregator committee consensus data")
return types.WrapError(types.AggCommConsensusDataDecodeErrorCode, errors.Wrap(err, "failed decoding aggregator committee consensus data"))
Copy link
Contributor

Choose a reason for hiding this comment

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

in this file we still need 2 value check for attestation aggregator and SC contribution?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought i have removed it here, but I actually only did in a later PR. Still, removing it here as well. Thanks!

@@ -1,39 +0,0 @@
package validatorconsensusdata
Copy link
Contributor

Choose a reason for hiding this comment

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

no need for new tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Several tests are added but in a new package

@@ -1,23 +0,0 @@
package validatorconsensusdata
Copy link
Contributor

Choose a reason for hiding this comment

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

no need for new tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same

@GalRogozinski GalRogozinski merged commit f573cf0 into aggregator-committee Dec 24, 2025
2 checks passed
@GalRogozinski GalRogozinski deleted the agg-comm-drop-previous-duty-types branch December 24, 2025 09:09
GalRogozinski pushed a commit that referenced this pull request Jan 29, 2026
* draft

* fix aggregator committee - sync committee contribution only test

* bug fix

* bug fix

* add 20-validator test for sync committee aggregator

* add test for aggregator and sync committee contribution duties

* lint

* merge with main

* missing tests

* support fulu in GetAggregateAndProofs

* fix fulu aggregate and proof

* fix missing fulu cases

* fix leftovers

* fix passing slot in contributionProofMsg

* Revert "fix passing slot in contributionProofMsg"

This reverts commit 226659d.

* Aggregator Committee - Drop previous runners and align tests (#592)

* remove aggregatorCommittee from BeaconRole

* remove aggregator and scc roles

* remove agg and scc from ValidatorConsensusData

* generate ssz encoding

* align testingutils to remove reference to agg and scc alone

* add agg committee consensus data tests (and remove agg and scc from validator consensus data tests)

* generate types JSON tests

* drop agg and scc runners; fix agg committee runner issue

* align testingutils for agg committee tests

* value check tests

* preconsensus tests

* post consensus tests

* duties tests

* runner construction tests

* consensus tests

* happy flow test

* dutyexe tests

* add test docs; fix msg processing test

* add all tests

* generate JSON tests

* apply suggestions

* Aggregator Committee Mixed Duties Tests (#593)

* remove aggregatorCommittee from BeaconRole

* remove aggregator and scc roles

* remove agg and scc from ValidatorConsensusData

* generate ssz encoding

* align testingutils to remove reference to agg and scc alone

* add agg committee consensus data tests (and remove agg and scc from validator consensus data tests)

* generate types JSON tests

* drop agg and scc runners; fix agg committee runner issue

* align testingutils for agg committee tests

* value check tests

* preconsensus tests

* post consensus tests

* duties tests

* runner construction tests

* consensus tests

* happy flow test

* dutyexe tests

* add test docs; fix msg processing test

* add all tests

* generate JSON tests

* drop weird json tests in unintended directory

* add mixed agg+scc pre-consensus tests

* generate JSON tests

* add error code

* increase number of Contributors

* add  post-consensus mixed agg committee tests

* generate JSON tests

* fixed remaining mixed tests

* generate JSON tests

* fix lint (remove unused functions)

* avoid in-place sorting

* update ssz hash tags

* apply suggestions

* solve TODOs

* revert deleted Alan runner roles

* revert deleted ValidatorConsensusData methods

* generate tests

* Revert "generate tests"

This reverts commit 441a53a.

* Revert "revert deleted ValidatorConsensusData methods"

This reverts commit cb04d43.

* Revert "revert deleted Alan runner roles"

This reverts commit 69a6a65.

* Agg comm improvements (#594)

* remove aggregatorCommittee from BeaconRole

* remove aggregator and scc roles

* remove agg and scc from ValidatorConsensusData

* generate ssz encoding

* align testingutils to remove reference to agg and scc alone

* add agg committee consensus data tests (and remove agg and scc from validator consensus data tests)

* generate types JSON tests

* drop agg and scc runners; fix agg committee runner issue

* align testingutils for agg committee tests

* value check tests

* preconsensus tests

* post consensus tests

* duties tests

* runner construction tests

* consensus tests

* happy flow test

* dutyexe tests

* add test docs; fix msg processing test

* add all tests

* generate JSON tests

* drop weird json tests in unintended directory

* add mixed agg+scc pre-consensus tests

* generate JSON tests

* add error code

* increase number of Contributors

* add  post-consensus mixed agg committee tests

* generate JSON tests

* add agg committee duty validation; add psgi msg sorting;

* add sorting and duty validation tests

* generate JSON tests

* fixed remaining mixed tests

* generate JSON tests

* fix lint (remove unused functions)

* change AggregatorCommitteeConsensusData to reduce duplicated data overhead

* align tests

* generate JSON tests

* fix maximum ssz sizes

* avoid in-place sorting

* add test docs

* remove unused test docs

* fix maximum-size tests

* generate JSON tests

* fix test: duty with diff slots

* add max size test for aggCommCD

* add size tests for phase0 and electra attestations; fix ssz max size for attestation in AggCommCD

* fix lint issues

* fix test dir (no multiple duty)

* fix versions data

* maximum duty possible test

* fix lint

* apply suggestions (remove sorting feature; remove unused errors; use subnet for contribution)

* clarify validator sync committee index usage

* change subnetID computation to avoid errors

* generate JSON tests

* tests for: invalid quorum; invalid quorum then valid quorum;

* generate JSON tests

* generate JSON tests with new error numbers

* remove deprecated partial signature types

* generate JSON tests

* generate SSZ files

* value check att decoding check

* generate JSON tests

* apply suggestions

* Rename ValidatorConsensusData to ProposerConsensusData (#596)

* remove aggregatorCommittee from BeaconRole

* remove aggregator and scc roles

* remove agg and scc from ValidatorConsensusData

* generate ssz encoding

* align testingutils to remove reference to agg and scc alone

* add agg committee consensus data tests (and remove agg and scc from validator consensus data tests)

* generate types JSON tests

* drop agg and scc runners; fix agg committee runner issue

* align testingutils for agg committee tests

* value check tests

* preconsensus tests

* post consensus tests

* duties tests

* runner construction tests

* consensus tests

* happy flow test

* dutyexe tests

* add test docs; fix msg processing test

* add all tests

* generate JSON tests

* drop weird json tests in unintended directory

* add mixed agg+scc pre-consensus tests

* generate JSON tests

* add error code

* increase number of Contributors

* add  post-consensus mixed agg committee tests

* generate JSON tests

* add agg committee duty validation; add psgi msg sorting;

* add sorting and duty validation tests

* generate JSON tests

* fixed remaining mixed tests

* generate JSON tests

* fix lint (remove unused functions)

* change AggregatorCommitteeConsensusData to reduce duplicated data overhead

* align tests

* generate JSON tests

* fix maximum ssz sizes

* avoid in-place sorting

* add test docs

* remove unused test docs

* fix maximum-size tests

* generate JSON tests

* fix test: duty with diff slots

* add max size test for aggCommCD

* add size tests for phase0 and electra attestations; fix ssz max size for attestation in AggCommCD

* fix lint issues

* fix test dir (no multiple duty)

* fix versions data

* maximum duty possible test

* fix lint

* apply suggestions (remove sorting feature; remove unused errors; use subnet for contribution)

* clarify validator sync committee index usage

* change subnetID computation to avoid errors

* generate JSON tests

* tests for: invalid quorum; invalid quorum then valid quorum;

* generate JSON tests

* generate JSON tests with new error numbers

* rename ValidatorConsensusData -> ProposerConsensusData

* align tests

* generate JSON tests

* fix renaming on merge

* make runner role explicit

* generate JSON tests

* Aggregator Committee - Fix committee runners management (#597)

* remove aggregatorCommittee from BeaconRole

* remove aggregator and scc roles

* remove agg and scc from ValidatorConsensusData

* generate ssz encoding

* align testingutils to remove reference to agg and scc alone

* add agg committee consensus data tests (and remove agg and scc from validator consensus data tests)

* generate types JSON tests

* drop agg and scc runners; fix agg committee runner issue

* align testingutils for agg committee tests

* value check tests

* preconsensus tests

* post consensus tests

* duties tests

* runner construction tests

* consensus tests

* happy flow test

* dutyexe tests

* add test docs; fix msg processing test

* add all tests

* generate JSON tests

* drop weird json tests in unintended directory

* add mixed agg+scc pre-consensus tests

* generate JSON tests

* add error code

* increase number of Contributors

* add  post-consensus mixed agg committee tests

* generate JSON tests

* add agg committee duty validation; add psgi msg sorting;

* add sorting and duty validation tests

* generate JSON tests

* fixed remaining mixed tests

* generate JSON tests

* fix lint (remove unused functions)

* change AggregatorCommitteeConsensusData to reduce duplicated data overhead

* align tests

* generate JSON tests

* fix maximum ssz sizes

* avoid in-place sorting

* add test docs

* remove unused test docs

* fix maximum-size tests

* generate JSON tests

* fix test: duty with diff slots

* add max size test for aggCommCD

* add size tests for phase0 and electra attestations; fix ssz max size for attestation in AggCommCD

* fix lint issues

* fix test dir (no multiple duty)

* fix versions data

* maximum duty possible test

* fix lint

* apply suggestions (remove sorting feature; remove unused errors; use subnet for contribution)

* clarify validator sync committee index usage

* change subnetID computation to avoid errors

* generate JSON tests

* tests for: invalid quorum; invalid quorum then valid quorum;

* generate JSON tests

* generate JSON tests with new error numbers

* remove deprecated partial signature types

* generate JSON tests

* generate SSZ files

* fix committee to have agg and comm runners

* align testing utils. Fix Committee constructor to a common one, and align tests execution

* add test for comm + agg comm duties in the same slot

* generate JSON tests

* remove unused function

* add tests for error cases in committee

* add test for mixed duties for multiple slots

* remove unused parameter

* set fork-persistent values for psig types

* generate JSON tests

* set max ssz sizes exportable

---------

Co-authored-by: Alan <alan@ssvlabs.io>
Co-authored-by: Nikita Kryuchkov <nkryuchkov10@gmail.com>
Co-authored-by: MatheusFranco99 <48058141+MatheusFranco99@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants