Skip to content

test: add test vector for block production with merging aggregates #897

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
unnawut wants to merge 1 commit into
base: main
Choose a base branch
from
+91 −3
Open

test: add test vector for block production with merging aggregates #897

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 91 additions & 3 deletions tests/consensus/lstar/fork_choice/test_signature_aggregation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,22 @@
BlockSpec,
BlockStep,
ForkChoiceTestFiller,
GossipAggregatedAttestationStep,
StoreChecks,
TickStep,
)
from lean_spec.spec.forks import Slot, ValidatorIndex
from lean_spec.spec.forks import Interval, Slot, ValidatorIndex

pytestmark = pytest.mark.valid_until("Lstar")


@pytest.mark.real_crypto(smoke=True)
def test_multiple_specs_same_target_merge_into_one(
def test_multiple_attestations_same_target_merge_into_one(
fork_choice_test: ForkChoiceTestFiller,
) -> None:
"""
Two attestations sharing one target merge into a single aggregation.
Two attestations in the known pool sharing one target get merged
into a single aggregation.

Given
-----
Expand Down Expand Up @@ -83,6 +86,91 @@ def test_multiple_specs_same_target_merge_into_one(
)


def test_overlapping_proofs_same_target_recursively_merge_into_one(

@tcoratger tcoratger Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One thing worth double-checking: the leaf-merge sibling just above carries @pytest.mark.real_crypto(smoke=True), but this new test doesn't. In the default mocked lane the prover is stubbed out, so the real recursive fold/split only runs in the slower real-crypto lane — which means the fast lane here is really validating the bitfield bookkeeping (overlapping voters collapsing into one attestation) rather than the actual recursive proof.

Since the recursion is kind of the whole point of this vector, would it make sense to add @pytest.mark.real_crypto(smoke=True) to mirror the sibling, so the fast lane exercises the real fold too?

(If you intentionally left it off because real recursive fold isn't smoke-lane-stable yet — related to the leanVM bump you mentioned — that's totally fair, I just wanted to flag the asymmetry. 🙂)

fork_choice_test: ForkChoiceTestFiller,
) -> None:
"""
Two overlapping proofs (enforced by 2 separate gossips) for one target
fold into a single in-block attestation.
Comment on lines +93 to +94

@tcoratger tcoratger Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Tiny one: the test-vector convention only allows a parenthetical when it glosses a number, so "(enforced by 2 separate gossips)" is slightly off here (and uses "2" as a word). Since the Given section already says both proofs arrive by gossip, I think we can just drop the parenthetical and keep the summary as a clean one-liner. 👍


Given
-----
- 4 validators.
- the chain:
genesis -> block_1(1)
- one proof covers V0, V1, V2 targeting block_1.
- one proof covers V1, V2, V3 targeting block_1.
- the two proofs overlap on V1, V2.
- both proofs carry identical attestation data.
- the clock ticks past slot 1's aggregate phase before they gossip.
- both proofs arrive by gossip.
- both proofs wait unmerged in the known pool.

When
----
- block_2 is built on block_1, carrying no votes of its own.

Then
----
- the builder takes the first proof.
- the builder takes the second proof for its one uncovered voter.
- the builder folds both proofs into one attestation.
- block_2 holds 1 aggregated attestation covering V0, V1, V2, V3.
Comment on lines +115 to +118

@tcoratger tcoratger Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Small docstring-convention nit: these bullets describe the builder's internal selection steps, but the Then section is meant to mirror the assertions one-to-one — and nothing here actually observes selection order or "one uncovered voter" (the checks are just block_attestation_count=1, participants={0,1,2,3}, and head at slot 2).

Could we swap them for outcome-only bullets, like the sibling has at lines 43-44? Something like:

    - block_2 holds 1 aggregated attestation.
    - that aggregation covers V0, V1, V2, V3.
    - head is block_2 at slot 2.

That also naturally splits the count and the participant set onto their own lines, which matches the one-fact-per-line style. 🙂

- head is block_2 at slot 2.

Timing
------
- the proofs are gossipped at slot 1, interval 3 (past the aggregate phase).
Comment on lines +121 to +123

@tcoratger tcoratger Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Optional: this Timing line mostly restates the Given bullet about the clock ticking past the aggregate phase. No strong opinion — maybe keep the concrete interval here and drop the Given line (or vice versa), just so the fact lives in one place. Happy either way. 🙂

"""
fork_choice_test(
steps=[
BlockStep(
block=BlockSpec(slot=Slot(1), label="block_1"),
checks=StoreChecks(head_slot=Slot(1)),
),
TickStep(interval=int(Interval.from_slot(Slot(1))) + 3),
GossipAggregatedAttestationStep(
attestation=AggregatedAttestationSpec(
validator_indices=[
ValidatorIndex(0),
ValidatorIndex(1),
ValidatorIndex(2),
],
slot=Slot(1),
target_slot=Slot(1),
target_root_label="block_1",
),
),
GossipAggregatedAttestationStep(
attestation=AggregatedAttestationSpec(
validator_indices=[
ValidatorIndex(1),
ValidatorIndex(2),
ValidatorIndex(3),
],
slot=Slot(1),
target_slot=Slot(1),
target_root_label="block_1",
),
),
BlockStep(
block=BlockSpec(slot=Slot(2), label="block_2"),
checks=StoreChecks(
head_slot=Slot(2),
block_attestation_count=1,
block_attestations=[
AggregatedAttestationCheck(
participants={0, 1, 2, 3},
attestation_slot=Slot(1),
target_slot=Slot(1),
),
],
),
),
],
)


def test_different_targets_create_separate_aggregations(
fork_choice_test: ForkChoiceTestFiller,
) -> None:
Expand Down
Loading