test: add test vector for block production with merging aggregates#897
Conversation
|
Thanks a lot, can you just rebase after #895 and follow the new doc writer conventions for the documentation of the test vectors just for uniformity? :) Also I haven't checked in details but would be nice to ensure that this test doesn't already exist with the new coverage added in the PR. |
unnawut
force-pushed
the
builder-recursive-fold-vector
branch
from
a8a9f9e to
2d82838
Compare
|
Accidentally pushed. I want to do a bit of touch up before marking for review again |
unnawut
force-pushed
the
builder-recursive-fold-vector
branch
4 times, most recently
from
62810fc to
3219764
Compare
…roofs The block builder's collapse-by-data branch in build_block merges multiple single-message aggregates for the same AttestationData via SingleMessageAggregate.aggregate(children=..., raw_xmss=[]). This is the only proposal-time path that constructs a recursive single-message aggregate, and no existing fork-choice vector reached it because the local aggregator absorbs same-data proofs before block build time. Gossiping two aggregated proofs with overlapping participant sets at slot 2 interval 3 (past the aggregate phase) bypasses the local aggregator, so both proofs migrate side by side into the known pool at slot 2 interval 4. The greedy picker takes both because the second still adds one uncovered validator on top of the first, exercising the merge branch on overlapping children rather than the unrealistic fully disjoint shape.
unnawut
force-pushed
the
builder-recursive-fold-vector
branch
from
3219764 to
269529e
Compare
@tcoratger I checked and yep there's I've moved my new test into the same file as yours though (test_signature_aggregation.py) so they stay close together. |
tcoratger
left a comment
tcoratger
left a comment
There was a problem hiding this comment.
Thanks for adding this! 🙏 I went through it carefully and the scenario is really nicely constructed. The TickStep to slot 1 / interval 3 is a neat touch: it pushes the gossips past the aggregate phase so the two overlapping proofs land in the known pool unmerged, which is exactly what forces the recursive in-block fold — so this genuinely covers a different path from the leaf-merge sibling. 👍
Just a few small things below, mostly around the test-vector docstring conventions, plus one question about crypto coverage. Nothing blocking — feel free to push back on any of them. 🚀
|
@tcoratger Applied your comments and re-reviewed. Please have another look! I'm still not too familiar with Given/When/Then format so feel free to guide me more |
…old vector Assert head identity (head_root_label) alongside head_slot so the Then "head is block_2" claim is verified by root, not just slot. Docstring: split the phase-glossing parenthetical and the two-fact head bullet into one-fact-per-line bullets, and drop the Given gossip bullet that duplicated the Timing section, per the consensus test-vector documentation standard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 participants
🗒️ Description
Add a test vector that asserts if a block producer has 2 aggregates of the same message, they get merged successfully for block inclusion.
Since proof bytes are not deterministic, this vector checks that all attesters are accounted for in the produced block.
I was also trying to add split and merge but seems like it needs a leanVM commit bump so I'll defer that to later.