Skip to content

Add aggregator gossip path unit and integration tests#367

Merged
tcoratger merged 5 commits intoleanEthereum:mainfrom
bomanaps:test/aggregator-gossip-paths
Feb 6, 2026
Merged

Add aggregator gossip path unit and integration tests#367
tcoratger merged 5 commits intoleanEthereum:mainfrom
bomanaps:test/aggregator-gossip-paths

Conversation

@bomanaps
Copy link
Contributor

@bomanaps bomanaps commented Feb 6, 2026

🗒️ Description

This PR adds comprehensive test coverage for the committee aggregation functionality introduced in PR #282.

Unit Tests (8) -

  • on_gossip_attestation with is_aggregator=True: subnet filtering, cross-subnet rejection, non-aggregator behavior
  • on_gossip_aggregated_attestation: proof verification, storage, invalid proof rejection, proof accumulation

Integration Tests (9)

  • aggregate_committee_signatures: proof creation from gossip signatures, cryptographic validity, grouping by attestation data
  • tick_interval aggregation trigger: interval 2 behavior for aggregators vs non-aggregators
  • End-to-end flow: gossip reception → interval tick → aggregation → proof storage

🔗 Related Issues or PRs

✅ Checklist

  • Ran tox checks to avoid unnecessary CI fails:
    uvx tox
  • Considered adding appropriate tests for the changes.
  • Considered updating the online docs in the ./docs/ directory.

@bomanaps
Copy link
Contributor Author

bomanaps commented Feb 6, 2026

cc @tcoratger @kamilsa please can you help me review this, thank you

Copy link
Collaborator

@tcoratger tcoratger left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this, just provided couple of comments.

In general, I think that th tests manually create validators, genesis state, and genesis block. The codebase already provides, for example in tests/lean_spec/conftest.py: genesis_state, genesis_block, base_store fixtures. Should be nice to reuse these.

The helper functions like _create_store_with_validators and _create_store_with_gossip_signatures duplicate existing infrastructure (I think).

@bomanaps
Copy link
Contributor Author

bomanaps commented Feb 6, 2026

Thanks a lot for this, just provided couple of comments.

In general, I think that th tests manually create validators, genesis state, and genesis block. The codebase already provides, for example in tests/lean_spec/conftest.py: genesis_state, genesis_block, base_store fixtures. Should be nice to reuse these.

The helper functions like _create_store_with_validators and _create_store_with_gossip_signatures duplicate existing infrastructure (I think).

You're right this pattern is duplicated across test_attestation_target.py, test_service.py, and our new tests.

The existing builders.py helpers use null keys (make_validators) or mock signatures (make_mock_signature), which don't support cryptographic verification. Our tests need real XMSS keys because on_gossip_attestation calls signature.verify().
I think we can extract to shared infrastructure in a follow-up PR:

  • Add make_validators_with_key_manager(key_manager, count) to builders.py
  • Add make_store_with_key_manager(key_manager, num_validators, validator_id) to builders.py
  • Refactor all three test files to use these shared helpers

Should I create that follow-up PR, or would you prefer the refactor included in this one?

@tcoratger
Copy link
Collaborator

Thanks a lot for this, just provided couple of comments.
In general, I think that th tests manually create validators, genesis state, and genesis block. The codebase already provides, for example in tests/lean_spec/conftest.py: genesis_state, genesis_block, base_store fixtures. Should be nice to reuse these.
The helper functions like _create_store_with_validators and _create_store_with_gossip_signatures duplicate existing infrastructure (I think).

You're right this pattern is duplicated across test_attestation_target.py, test_service.py, and our new tests.

The existing builders.py helpers use null keys (make_validators) or mock signatures (make_mock_signature), which don't support cryptographic verification. Our tests need real XMSS keys because on_gossip_attestation calls signature.verify(). I think we can extract to shared infrastructure in a follow-up PR:

  • Add make_validators_with_key_manager(key_manager, count) to builders.py
  • Add make_store_with_key_manager(key_manager, num_validators, validator_id) to builders.py
  • Refactor all three test files to use these shared helpers

Should I create that follow-up PR, or would you prefer the refactor included in this one?

Yes let's do this in followup PRs, no need to this here

@tcoratger tcoratger added the tests Scope: Changes to the spec tests label Feb 6, 2026
@tcoratger tcoratger added this to the pq-devnet-3 milestone Feb 6, 2026
@tcoratger tcoratger merged commit caa317b into leanEthereum:main Feb 6, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests Scope: Changes to the spec tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants