refactor(prover): extract a prover-worker crate from asm-runner#143
Open
prajwolrg wants to merge 8 commits into
Open
refactor(prover): extract a prover-worker crate from asm-runner#143prajwolrg wants to merge 8 commits into
prajwolrg wants to merge 8 commits into
Conversation
|
Commit: 79ea607
|
prajwolrg
force-pushed
the
STR-3699-prover-worker
branch
2 times, most recently
from
60daeca to
9a71376
Compare
prajwolrg
force-pushed
the
STR-3699-prover-worker
branch
from
9a71376 to
3b4d6c0
Compare
prajwolrg
force-pushed
the
STR-3698-moho-worker
branch
2 times, most recently
from
f75ef6c to
1beadbd
Compare
Begin grouping the prover stack under a single crates/extensions/prover tree. Move the proof types crate there and rename it strata-asm-prover-types so the naming matches the forthcoming prover-worker and prover-storage crates. Pure relocation and rename; no logic changes.
Move the proof database crate alongside the relocated types crate and rename it strata-asm-prover-storage, framing it as the prover's storage layer (the analogue of asm-storage for the ASM worker). Pure relocation and rename; the storage traits and sled implementations are unchanged.
Proof scheduling and reconciliation lived inside the asm-runner binary, hard-wired to the concrete SledProofDb, so the orchestrator could not be tested against fakes or reused outside the binary. Introduce strata-asm-prover-worker, mirroring the ASM worker's shape: it defines a ProverContext umbrella trait and drives the orchestrator generically over it, while the binary supplies the concrete impl (AsmProverContext) wiring the sled stores and the Bitcoin client. The proving backend is split into native and sp1 modules behind the existing feature gate.
A periodically-ticking consumer (the prover orchestrator) needs to drain every buffered commitment at the top of each tick without parking on the channel, and to learn that the worker has shut down once the backlog is empty. recv() can only await; try_recv() covers both via Empty/Disconnected.
Proof requests were enqueued from the block watcher before the ASM worker had committed the block. Subscribe the orchestrator to the worker's post-commit stream instead: it expands each committed L1BlockCommitment into its ASM step proof and Moho recursive proof internally, so proofs are requested only for durably-stored blocks. The subscription becomes the orchestrator's sole input, dropping the ProverWorkerHandle/request_proof channel; the block watcher returns to only feeding blocks to the worker.
Replace the hand-rolled `ProofOrchestrator` loop with the same Service/State pattern the ASM worker uses: a logic-only `ProverService` over a data-only `ProverServiceState`, launched via `launch_async` and returning a `ProverWorkerHandle`. The ASM commit subscription becomes the service input through `StreamInput` + `TickingInput`. This removes the dedicated current-thread-runtime + LocalSet workaround in bootstrap. That workaround existed because the proving path was `!Send`; with the host trait now `Send`, the only remaining `!Send` sources were our own `L1BlockProvider` futures (now bounded `+ Send`) and the `ZkVmRemoteProgram::start_proving` wrapper (now inlined to call the host directly). A TODO tracks reverting both once zkaleido is updated.
…item The prover needs to chain off Moho-state commits rather than ASM commits, which means the Moho worker must fan out a post-commit notification the same way the ASM worker does. Rather than duplicate the subscriber registry, make `AsmSubscribers` a generic `Subscribers<T>` so both workers share one type; `Subscription<T>` was already generic. The prover can then subscribe to either worker's stream interchangeably, since both emit `L1BlockCommitment`. No behavior change: the ASM worker keeps emitting exactly as before.
The prover orchestrator and the Moho worker each subscribed to the ASM worker's commit stream independently, so a block's proof input could be assembled before the Moho worker had persisted that block's MohoState — the read of the parent's MohoState made this unlikely in practice but the ordering was no longer guaranteed once MohoState moved out of the ASM worker's inline write. Serialize the chain ASM -> Moho -> prover instead. The Moho worker now owns a subscriber registry and emits each block on its own stream after the MohoState is durably committed, and exposes subscribe_blocks() on its handle, mirroring the ASM worker. The prover subscribes to that stream, so every block it sees already has its MohoState available. Startup catch-up (sync_to_tip) deliberately stays silent, matching the no-replay semantics of the ASM commit stream.
prajwolrg
force-pushed
the
STR-3699-prover-worker
branch
from
a3b9179 to
3edaaee
Compare
prajwolrg
changed the base branch from
STR-3698-moho-worker
to
STR-3698-asm-worker-subscription
Collaborator
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Proof scheduling and reconciliation lived inside the
asm-runnerbinary atbin/asm-runner/src/prover/, hard-wired to the concreteSledProofDb, so the orchestrator could not be tested against fakes or reused outside the binary. This PR extracts that logic into a dedicatedstrata-asm-prover-workercrate (grouped under a newcrates/extensions/prover/tree), runs it on thestrata-serviceframework, and drives it from a worker commit subscription instead of an ad-hoc loop.The new worker mirrors the ASM worker's shape: it defines a
ProverContextumbrella trait (proof storage + remote-job tracking + chain/state reads) and drives the orchestrator generically over it, while the binary supplies the concrete impl (AsmProverContext) that wires the sled stores and the Bitcoin client together — the same split asstrata-asm-worker+asm-storage+ the binary'sworker_context.rs.Each commit compiles on its own. Grouped by theme:
Crate extraction (mechanical + the original substance)
relocate proof-types crate under extensions/prover— move + renamestrata-asm-proof-types→strata-asm-prover-types.relocate proof-db crate as extensions/prover/storage— move + renamestrata-asm-proof-db→strata-asm-prover-storage.extract proof orchestration into a prover-worker crate— introduceProverContext, make the orchestrator generic, split the proving backend intonative/sp1modules, and rewire the binary.Subscription-driven orchestration on the service framework
add Subscription::try_recv for non-blocking drain— lets a periodically-ticking consumer drain the backlog without parking on the channel.drive proof orchestration from the ASM commit subscription— replace the ad-hoc polling loop with the worker's per-block commit stream overlaid with a periodic tick.run prover worker on the strata-service framework— the proving path is nowSend, so the service runs on the standard async framework.Subscribe to the Moho stream, not the ASM stream
generalize the commit-subscriber registry over its item— makeAsmSubscribersa genericSubscribers<T>so both the ASM and Moho workers can expose the identical commit stream. No behavior change.drive proof orchestration from the Moho commit stream— the Moho worker now re-emits each block after persisting itsMohoState, and the prover subscribes to that stream instead of the ASM one.The
strata-asm-proof-impl(guest statements) crate intentionally stays incrates/proof/.Type of Change
Notes to Reviewers
Stacked on #129 (the Moho worker). This PR's base is
STR-3698-moho-worker, so review #129 first — the last two commits here depend on the Moho worker it introduces. The diff will shrink to just this PR's commits once #129 merges and the base retargets tomain.The relationship this PR establishes: ASM → Moho → prover. The Moho worker (from #129) folds a new
MohoStatefor every block the ASM worker commits. Before this PR, the prover orchestrator and the Moho worker each subscribed to the ASM worker's commit stream independently, so a block's proof input could be assembled before the Moho worker had persisted that block'sMohoState. Reading the parent'sMohoStatemade a stall unlikely in practice, but the ordering was no longer guaranteed onceMohoStatemoved out of the ASM worker's inline write (that was flagged as aFIXME(STR-3699/STR-3698)). This PR closes that gap: the Moho worker re-emits each block on its own subscription only after theMohoStateis durably committed, and the prover chains off that stream — so every block the prover sees already has itsMohoStateavailable. The genericSubscribers<T>is what lets both workers hand out the same stream type interchangeably.This is therefore not a pure no-op extraction: the final two commits are a deliberate behavioral change (serializing the proof pipeline to remove the race). The earlier extraction/relocation commits remain behavior-preserving. Startup catch-up (
sync_to_tip) deliberately stays silent, matching the no-replay semantics of the ASM commit stream — the prover, like before, relies on its periodic tick to reconcile in-flight proofs rather than on replay.Verified with
cargo check/clippy/testacross the affected crates under both default andsp1features; the migrated queue/scheduling unit tests, the new Moho emit/no-emit tests, and the runner/rpc tests pass.Now that the orchestrator is decoupled from sled, a fake
ProverContextcan exercise the full reconcile/schedule loop end-to-end — left as a follow-up rather than expanding this PR's scope.Checklist
Related Issues
STR-3699, STR-3698