Skip to content

Comments

apollo_propeller: add reconstruction types and spawn task#12007

Closed
sirandreww-starkware wants to merge 1 commit into01-26-apollo_propeller_add_validated_unit_handling_and_shard_broadcastingfrom
01-26-apollo_propeller_add_reconstruction_types_and_spawn_task
Closed

apollo_propeller: add reconstruction types and spawn task#12007
sirandreww-starkware wants to merge 1 commit into01-26-apollo_propeller_add_validated_unit_handling_and_shard_broadcastingfrom
01-26-apollo_propeller_add_reconstruction_types_and_spawn_task

Conversation

@sirandreww-starkware
Copy link
Contributor

@sirandreww-starkware sirandreww-starkware commented Jan 26, 2026

Note

Low Risk
Mostly additive, currently unused reconstruction scaffolding and a minor dependency feature tweak; limited behavior change unless/when wired into the processing flow.

Overview
Adds scaffolding in MessageProcessor for erasure-coded message reconstruction: introduces ReconstructionResult/ReconstructionSuccess types and a spawn_reconstruction_task helper that runs reconstruct_message_from_shards on the Rayon pool and returns the result via a oneshot channel.

Updates dependencies to enable rand’s std/std_rng features and adjusts imports to include reconstruction-related types (ReconstructionError, MerkleProof).

Written by Cursor Bugbot for commit 21443c9. This will update automatically on new commits. Configure here.

@reviewable-StarkWare
Copy link

This change is Reviewable

@github-actions
Copy link

github-actions bot commented Jan 26, 2026

Copy link
Contributor Author

sirandreww-starkware commented Jan 26, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Copy link
Contributor

@guy-starkware guy-starkware left a comment

Choose a reason for hiding this comment

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

@guy-starkware reviewed 3 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @noamsp-starkware, @ShahakShama, and @sirandreww-starkware).


crates/apollo_propeller/src/message_processor.rs line 220 at r1 (raw file):

        rayon::spawn(move || {
            let result =
                rebuild_message(shards, message_root, my_shard_index, data_count, coding_count)

Is there a reason rebuild_message doesn't just emit ReconstructionResult?

Copy link
Contributor

@guy-starkware guy-starkware left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on noamsp-starkware, ShahakShama, and sirandreww-starkware).

@sirandreww-starkware sirandreww-starkware changed the base branch from 01-26-apollo_propeller_add_validated_unit_handling_and_shard_broadcasting to graphite-base/12007 February 22, 2026 07:36
@sirandreww-starkware sirandreww-starkware force-pushed the 01-26-apollo_propeller_add_reconstruction_types_and_spawn_task branch from fc5b531 to 901ad8e Compare February 22, 2026 07:51
@sirandreww-starkware sirandreww-starkware changed the base branch from graphite-base/12007 to 01-26-apollo_propeller_add_validated_unit_handling_and_shard_broadcasting February 22, 2026 07:51
Copy link
Contributor Author

@sirandreww-starkware sirandreww-starkware left a comment

Choose a reason for hiding this comment

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

@sirandreww-starkware made 2 comments and resolved 1 discussion.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on guy-starkware, noamsp-starkware, and ShahakShama).


crates/apollo_propeller/src/message_processor.rs line 220 at r1 (raw file):

Previously, guy-starkware wrote…

Is there a reason rebuild_message doesn't just emit ReconstructionResult?

AI answer:
reconstruct_message_from_shards lives in the sharding utility layer — it only knows about erasure coding, Merkle trees, and padding. ReconstructionSuccess (and ReconstructionResult) are processor-level concepts private to MessageProcessor, where the tuple fields get domain-specific names (my_shard, my_shard_proof) tied to the node's role. Pushing that type down into sharding.rs would couple the generic reconstruction logic to the processor's internal model. Keeping the tuple return keeps the sharding layer composable and the boundary clean.

My answer:
I don't want spinoff workers to emit events, the message processor both 1) needs to know the result of the build to update its state and 2) needs to figure out what to emit, or if to emit in the first place.

Copy link
Contributor Author

@sirandreww-starkware sirandreww-starkware left a comment

Choose a reason for hiding this comment

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

@sirandreww-starkware resolved 1 discussion.
Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on guy-starkware, noamsp-starkware, and ShahakShama).

Copy link
Contributor Author

@sirandreww-starkware sirandreww-starkware left a comment

Choose a reason for hiding this comment

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

@sirandreww-starkware partially reviewed 3 files.
Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on guy-starkware, noamsp-starkware, and ShahakShama).

@sirandreww-starkware sirandreww-starkware force-pushed the 01-26-apollo_propeller_add_validated_unit_handling_and_shard_broadcasting branch from b5519d6 to d9e2711 Compare February 22, 2026 09:15
@sirandreww-starkware sirandreww-starkware force-pushed the 01-26-apollo_propeller_add_reconstruction_types_and_spawn_task branch from 901ad8e to 21443c9 Compare February 22, 2026 09:15
Copy link
Contributor

@guy-starkware guy-starkware left a comment

Choose a reason for hiding this comment

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

@guy-starkware reviewed 3 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on noamsp-starkware and ShahakShama).


crates/apollo_propeller/src/message_processor.rs line 220 at r1 (raw file):

Previously, sirandreww-starkware (Andrew Luka) wrote…

AI answer:
reconstruct_message_from_shards lives in the sharding utility layer — it only knows about erasure coding, Merkle trees, and padding. ReconstructionSuccess (and ReconstructionResult) are processor-level concepts private to MessageProcessor, where the tuple fields get domain-specific names (my_shard, my_shard_proof) tied to the node's role. Pushing that type down into sharding.rs would couple the generic reconstruction logic to the processor's internal model. Keeping the tuple return keeps the sharding layer composable and the boundary clean.

My answer:
I don't want spinoff workers to emit events, the message processor both 1) needs to know the result of the build to update its state and 2) needs to figure out what to emit, or if to emit in the first place.

ok. I am not familiar enough with the logical boundaries, but if you think this is better I will go with it.

Copy link
Collaborator

@ShahakShama ShahakShama left a comment

Choose a reason for hiding this comment

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

@ShahakShama made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on noamsp-starkware and sirandreww-starkware).


crates/apollo_propeller/Cargo.toml line 19 at r3 (raw file):

libp2p.workspace = true
prost.workspace = true
rand = { workspace = true, features = ["std", "std_rng"] }

Why?

Copy link
Collaborator

@ShahakShama ShahakShama left a comment

Choose a reason for hiding this comment

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

@ShahakShama reviewed all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on noamsp-starkware and sirandreww-starkware).

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.

4 participants