Skip to content

Sync aggregation v2: bitmap acks, partitioned block delivery, coordinated activation - #984

Draft
hackobi wants to merge 3 commits into
poc/sync-aggregationfrom
improve/sync-aggregation-v2
Draft

Sync aggregation v2: bitmap acks, partitioned block delivery, coordinated activation#984
hackobi wants to merge 3 commits into
poc/sync-aggregationfrom
improve/sync-aggregation-v2

Conversation

@hackobi

@hackobi hackobi commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Stacks on #983 and completes the three follow-ups its review named: aggregate byte size, the relay trust rule, and mixed-version activation. No consensus rule changes — voting, transaction validation, block construction and finality are untouched, and everything stays behind BLOCK_SYNC_AGGREGATION_ENABLED=false.

At 500 peers, the post-block burst goes from 251,984 calls (legacy) / 995 calls + 18.1 MB (v1 POC) to 2,492 calls + 0.54 MB.

Why v1 needed a v2

v1 problem v2 answer
34.4 KB JSON aggregate × ~500 sends = 18.1 MB burst from one node Bitmap over the block's committed peerlist: 183 B per aggregate at 500 peers
Secretary is the sole publisher of block + aggregate — and committee[0] is view-dependent (liveness-filtered draw, mutates on failover), so rounds can end with zero or duplicate publishers Every signing committee member publishes a deterministic slice; no single required publisher
No mixed-version story Activation height: the whole fleet flips behaviour at one agreed block

How it works

1. Bitmap wire format (v2)

{
  "version": 2,
  "blockNumber": 12345,
  "blockHash": "",
  "peerlistSize": 500,   // cross-checked against the locally derived index
  "ackBits": ""         // base64; bit i = index entry i acked the block
}

The bitmap indexes the committed peerlist (normalized, deduped, sorted) because it sits inside the block-hash preimage — every node holding the block derives the identical index, so no identities travel on the wire. It deliberately does not index validation_data.signatures: that map merges incrementally per node and is not hash-covered, so a bitmap over it would decode differently on different nodes.

Decoding fails closed: wrong byte length, non-canonical base64, peerlistSize mismatch, or any bit set beyond the index ⇒ 400. Receivers admit both wire versions indefinitely, so mixed v1/v2 fleets converge.

2. Partitioned block delivery

Peer p belongs to slice H(p, blockHash) mod S, owned by that position of the sorted signing committee. Properties:

  • Depends only on the peer identity, the signing committee and the block hash — never on peerlist ordering, which differs between nodes. Divergent views degrade to duplicate deliveries (deduped by handleNewBlock) or missed ones (repaired by fastSync, now ~1/S of peers instead of all).
  • The block-hash salt rotates slice ownership every block, so a crashed or withholding member cannot starve the same peers round after round.
  • Only members that signed the block are excluded from slices — a member that aborted mid-round holds neither the block nor a signature and stays an ordinary delivery target.
  • Total deliveries stay N − S; per-sender block bytes divide by S.

3. Partial aggregates + race buffering

Each member builds one partial bitmap from its own slice's responses, applies it locally, and broadcasts it. Receivers union partials through the same fail-closed admission — idempotent and order-independent, so there is no merge protocol, collection window, or new message type.

Because a member with a fast slice publishes while slower slices are still delivering, a partial routinely reaches a peer moments before its own block does. Receivers buffer aggregates addressed to lastBlockNumber + 1 (64 entries, 60 s TTL) and replay them through admission once the block lands, instead of dropping them.

4. Correctness fixes picked up along the way

  • Monotonicity guard: v1's applySyncAggregate could regress a peer's sync hint to an older block (the legacy path gets this guard from PeerManager.addPeer; the aggregate path bypassed it). Those hints feed sync-source selection, the forge-quorum pre-check and the network-ahead veto.
  • The syncNewBlock duplicate-block short-circuit returned a bare string without syncData, so already-synced peers could never be counted in an aggregate. It now returns the standard response shape.

Measured results

Same emulator, knobs and pass criteria as the #983 VPS run (five bursts per size, 20–100 ms jitter, 5% slow peers, 5% transient failures, bounded retries). Observed calls matched estimatePostBlockTraffic(n, s, true, 2) exactly at every size; every partial admitted everywhere; every non-signer delivered exactly once; every receiver's accepted union exactly peerlist − self.

Peers Legacy calls v2 calls Reduction Mean burst Partial aggregate Total wire/block
100 10,384 492 95.3% 573 ms 119 B 0.08 MB
250 63,484 1,242 98.0% 754 ms 143 B 0.22 MB
500 251,984 2,492 99.0% 1,294 ms 183 B 0.54 MB

Call model for a committee of S in N nodes:

legacy:  S·(N−S) + N·(N−S) + N·S
v1:      (N−S) + (N−1)
v2:      (N−S) + S·(N−1)

v2 spends ~2.5× v1's requests to buy: aggregate bytes down two orders of magnitude, per-sender block load ÷ S, and no single required publisher. A v1-mode regression run confirms unchanged v1 numbers, and all safety cases (v1's three plus four v2-specific rejections) fail closed.

Trust rule (review gate 1 of #983)

A sender that signed block B is trusted to relay liveness-only acknowledgement observations about B.

Sound because admission is fail-closed on every axis that could touch consensus: an aggregate is only accepted for a block the receiver itself already verified and stored; it can only reference identities committed in that block's hash-covered peerlist (or its signers) that are already known locally; it never marks a peer online, never regresses a hint, never adds a peer. The residual power of a malicious signer is a stale hint — bounded to liveness (a retried sync-source pick, or a forge round that starts and then fails to gather real signatures). The upgrade path to per-ack detached signatures is designed in the doc's Appendix A, including the hard requirement for domain separation (block signatures sign the bare block hash — an un-separated ack signature would be a block signature).

Rollout (review gate 2 of #983)

Env Default Meaning
BLOCK_SYNC_AGGREGATION_ENABLED false Master switch (unchanged from #983)
BLOCK_SYNC_AGGREGATION_VERSION 2 1 = secretary/JSON POC, 2 = this PR; invalid values warn and fall back
BLOCK_SYNC_AGGREGATION_ACTIVATION_HEIGHT 0 First height the send path activates; below it behaviour is byte-identical legacy

Invariant: any change to ENABLED or VERSION on a live fleet ships with a fresh future activation height, identical on every node, so the fleet flips in the same round. Receivers admit both versions regardless of height; laggards degrade to anti-entropy, never diverge. Rollback = unset the flag; there is no persistent state — aggregates only touch in-memory sync hints.

Reviewing this PR

Three commits, reviewable independently:

  1. feat(sync-aggregation) — all production changes: syncAggregation.ts (pure functions: codec, index, admission, partition, model), broadcastManager.ts (modes, partitioned publish, buffer), thin hooks in PoRBFT.ts / manageGCRRoutines.ts, config plumbing.
  2. test(sync-aggregation) — 23 new bun tests (codec canonicality, admission rejections, partition determinism/coverage/rotation, 20/50/500-node convergence) and the emulator's --aggregate-version=2 mode.
  3. docs(sync-aggregation)docs/poc/block-sync-aggregation.md carries the full design, the trust-rule analysis, the activation protocol, and eight recorded findings for maintainers.

Verification: 38/38 tests green; no new type or lint errors versus the base branch; emulator green in both modes.

Still required before enabling anywhere public

  • A real multi-host validator soak — the emulator proves transport-path scaling, not hundreds of databases, consensus loops and WAN behaviour.
  • Maintainer sign-off on the trust rule (or a decision to fund the Appendix A signature path).
  • Findings 6–8 in the doc: activation height as an in-band governance parameter, verifying the sender's block signature cryptographically instead of trusting the locally merged signature map, and a rate limit on updateSyncAggregate.
  • The biggest recorded finding is not in this PR's scope but is the natural next target: hello gossip messages every known peer roughly every 2 s — an O(N²) background load that now dwarfs the post-block burst.

…ctivation height

Encode acknowledgements as a bitmap over the hash-committed peerlist
(34.4 KB -> ~183 B at 500 peers), partition block delivery across signing
committee members with block-hash-rotated slices, publish per-member
partial aggregates, buffer next-block aggregates that race delivery, guard
sync-hint monotonicity, and gate the send path behind a coordinated
activation height. Receivers admit both wire versions; legacy behaviour is
byte-identical below the activation height or with the flag off.
23 new bun tests (bitmap codec canonicality, canonical index, v2 admission,
partition determinism/coverage/rotation, v2 traffic model, activation
gating, 20/50/500-node partial-aggregate convergence). Emulator gains
--aggregate-version=2 with partitioned delivery, per-member partials, v2
safety cases and byte accounting; VPS wrapper forwards AGGREGATE_VERSION.
Document the bitmap format and why the index excludes the non-hash-covered
signature map, partitioned dissemination and race buffering, the formal
liveness-only relay trust rule with its fail-closed analysis, the
activation-height rollout invariant, measured 100/250/500-peer emulator
results (18.1 MB -> 0.54 MB per block at 500 peers), eight recorded
maintainer findings, and the detached-signature appendix incl. the
domain-separation pitfall.
@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b6930666-6b7e-4ba1-8e71-41f062a0424c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@hackobi

hackobi commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

  Context: this stacks on #983 (base branch poc/sync-aggregation), so review only this PR's delta. It must not change any consensus rule — voting, validation, block construction and finality are untouched, and everything is gated behind BLOCK_SYNC_AGGREGATION_ENABLED (default false).

  Please focus on:
  1. src/libs/communications/syncAggregation.ts — bitmap encode/decode canonicality (LSB-first, trailing bits, base64 re-encode check), v2 admission fail-closed paths, and that the partition (partitionIndexFor / blockDeliveryPartition) is deterministic across nodes and covers every non-committee peer exactly once.
  2. src/libs/communications/broadcastManager.ts — the mode gating (legacy/v1/v2 per block height): verify no flag/version/height combination leaves zero block publishers; the pendingSyncAggregates buffer (bounds, TTL, drain in handleNewBlock) for leaks or replay issues; the monotonicity guard in applySyncAggregate.
  3. Legacy invariance: with the flag off or below the activation height, behavior must be byte-identical to the base branch.

  Known/accepted (don't flag): pre-existing tsc errors on the base branch, the liveness-only trust rule (documented in docs/poc/block-sync-aggregation.md), and the eight recorded maintainer findings in that doc.

@greptile-apps

greptile-apps Bot commented Aug 18, 2026

Copy link
Copy Markdown

Greptile Summary

The PR adds version-2 block-sync aggregation with canonical bitmap acknowledgements, deterministic partitioned delivery, coordinated activation, and race buffering.

  • Adds strict bitmap encoding, decoding, admission, and delivery-partition helpers.
  • Routes post-consensus publication through legacy, v1, or v2 behavior selected per block height.
  • Buffers next-block partial aggregates and adds monotonic sync-hint updates.
  • Extends configuration, tests, emulator support, and design documentation.

Confidence Score: 4/5

The buffered-aggregate lifecycle should be fixed before merging so blocks obtained through fastSync do not silently discard pending acknowledgement information.

Buffer replay is coupled exclusively to handleNewBlock even though supported synchronization paths can insert the awaited block directly, leaving valid pending aggregates to be pruned without application.

Files Needing Attention: src/libs/communications/broadcastManager.ts

Important Files Changed

Filename Overview
src/libs/communications/syncAggregation.ts Adds canonical bitmap codecs, fail-closed v2 admission, deterministic delivery partitioning, activation gating, and traffic modeling without an identified defect.
src/libs/communications/broadcastManager.ts Adds mode-aware publication, partitioned delivery, buffering, and monotonic updates, but buffered aggregates are not replayed when blocks arrive through fastSync.
src/libs/consensus/v2/PoRBFT.ts Delegates post-consensus publication to the mode-aware BroadcastManager without changing voting, validation, construction, or finality.
src/libs/network/manageGCRRoutines.ts Makes duplicate-block responses include the standard syncData response shape for aggregation.
src/config/loader.ts Loads and validates aggregation version and activation-height settings with safe defaults.
src/libs/communications/syncAggregation.test.ts Thoroughly covers bitmap canonicality, admission, partitioning, activation, and convergence, but does not cover buffer draining through alternate block-ingestion paths.

Sequence Diagram

sequenceDiagram
    participant P as Committee publisher
    participant R as Receiving node
    participant B as Pending aggregate buffer
    participant F as fastSync
    P->>R: Partial aggregate for height N+1
    R->>B: Buffer until block arrives
    F->>R: Download and insert block N+1
    Note over R,B: fastSync bypasses handleNewBlock drain
    R->>B: Later prune by height or TTL
    Note over R: Buffered acknowledgements are never applied
Loading

Fix all with Greploop Fix All in Claude Code

Prompt To Fix All With AI
### Issue 1
src/libs/communications/broadcastManager.ts:310-312
**Fast-sync bypasses aggregate replay**

When a next-height aggregate is buffered but the block subsequently arrives through `fastSync` or another direct insertion path, `drainPendingSyncAggregates` is never called, causing the valid acknowledgements to remain unused until they are pruned or expire.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "docs(sync-aggregation): v2 design, trust..." | Re-trigger Greptile

Comment on lines +310 to +312
// buffered; replay them now through the same admission path.
this.drainPendingSyncAggregates(block)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Fast-sync bypasses aggregate replay

When a next-height aggregate is buffered but the block subsequently arrives through fastSync or another direct insertion path, drainPendingSyncAggregates is never called, causing the valid acknowledgements to remain unused until they are pruned or expire.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/libs/communications/broadcastManager.ts
Line: 310-312

Comment:
**Fast-sync bypasses aggregate replay**

When a next-height aggregate is buffered but the block subsequently arrives through `fastSync` or another direct insertion path, `drainPendingSyncAggregates` is never called, causing the valid acknowledgements to remain unused until they are pruned or expire.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant