feat: merge vid dispersal into block building

[mrain]

Closes #<ISSUE_NUMBER>

This PR:

• To avoid duplicated computation, vid dispersal is computed during block building.
• We cannot concurrently fan out the vid shares while doing dispersal, because we have to wait for the payload commitment, which is only available after the dispersal is done.
• Wrap the potentially heavy BlockPayload in Arc to avoid expensive cloning when passing the events.

This PR does not:

Key places to review:

[gemini-code-assist]

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

[claude]

Claude finished @mrain's task in 2m 45s —— View job

---

Code Review: PR #4652 — Merge VID dispersal into block building

• Gather context and read existing review comments
• Read and analyze all changed files
• Post detailed review feedback

---

Summary

Clean architectural simplification. By moving VID dispersal into the block builder, you eliminate:

• The separate VidDisperser task and its own JoinSet / dedup map / GC
• The RequestVidDisperse / VidDisperseCreated round-trip through the consensus state machine
• One redundant encoding of the payload (it was encoded once for the commitment and again for dispersal)

The new flow computes dispersal in the same spawn_blocking that builds the block, extracts the commitment for the proposal, and then kicks off fanout on a separate blocking task. The aggregate_commit helper in the VID crate is a nice addition with a correctness test. Wrapping BlockPayload in Arc to avoid cloning the (potentially large) payload through events is a good call.

Findings

1. Fire-and-forget spawn_blocking for fanout — silent panic loss
block.rs:220 — The spawn_blocking call for fan_out drops its JoinHandle, which means:

• If the closure panics, it is silently swallowed — no log, no error propagation.
• The current code does log fan_out errors, but a panic bypasses that if let Err.

This is a known Tokio footgun: dropping a JoinHandle from spawn_blocking detaches the task, and panics on detached tasks are silently lost. The old VidDisperser used a JoinSet which surfaced panics via join_next().

Consider at minimum logging on JoinError::is_panic():

let handle = spawn_blocking(move || { ... });
tokio::spawn(async move {
    if let Err(e) = handle.await {
        if e.is_panic() {
            error!(%view, "vid share fanout panicked");
        }
    }
});

Or adding the handle to the existing JoinSet so panics surface through next().

Fix this →

2. blocks_reconstructed now set before shares are actually delivered
consensus.rs:749 — Previously, blocks_reconstructed was set in the VidDisperseCreated handler, which fired after dispersal completed (shares were sent). Now it's set immediately when the block is built, before fan_out runs. This means consensus considers the block "reconstructed" before the leader's own loopback share has been delivered.

If any code path checks blocks_reconstructed to decide whether the leader can vote on its own proposal, there's a window where the leader considers the block reconstructed but hasn't received its share yet. I haven't traced every consumer of blocks_reconstructed to verify, but the name strongly implies "payload data is locally available" — and with fire-and-forget fanout, the share arrives asynchronously.

Worth verifying that no downstream logic depends on blocks_reconstructed implying "the leader has its VID share." If the semantics are "the leader built this block and doesn't need reconstruction," the current approach is correct, but the timing change is subtle.

3. Minor: metadata field added alongside Arc<BlockPayload> — asymmetric wrapping
block.rs:58-59 — BlockBuilderOutput wraps payload in Arc but keeps metadata as a bare value. The PR description says the Arc is "to avoid expensive cloning when passing the events." If metadata can also be large (e.g. namespace tables with many entries), it might benefit from the same treatment. Not a bug, just worth considering for consistency if metadata ever grows.

Positive observations

• Correctness test for commitment equivalence (vid/src/avidm_gf2/namespaced.rs:521-547): The aggregate_commit_matches_commit test directly verifies the key invariant — that per-namespace ns_disperse_one + aggregate_commit yields the same commitment as the monolithic commit. This is exactly the right test for this refactor.

• Clean removal of VidDisperser: All references are consistently removed across coordinator, consensus, tests, and error types. No stale variants or dead code left behind.

• disperse_params now takes Arc<[u8]> instead of &T::BlockPayload: This avoids a redundant encode() call — the caller already has the bytes, so passing them in directly is the right API.

• Test updates are thorough: All test harnesses, assertions, and integration tests are updated to reflect the removal of the dispersal round-trip. Tests that previously waited for is_vid_disperse correctly no longer do so.

---