refactor(committor): remove legacy Commit and Finalize tasks

[snawaz]

Summary

Remove the unused separate Commit and Finalize tasks and their recovery code now that all commit paths use CommitFinalize*.

Closes #1765.
Refs #1739.

Breaking Changes

• None
• Yes — migration path described below

The Rust task API removes CommitTask, FinalizeTask, and their enum variants. Use CommitFinalizeTask; CommitDelivery moves to tasks::commit_delivery.

Test Plan

36 targeted unit tests and the buffer lifecycle integration test passed. Clippy, formatting, and integration compilation passed.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: magicblock-labs/magicblock-validator/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: f70f8837-853a-4450-8680-91c5234cb9d9

📥 Commits

Reviewing files that changed from the base of the PR and between bf05e9c and ca34fdd.

📒 Files selected for processing (25)

• magicblock-committor-service/src/committor_processor.rs
• magicblock-committor-service/src/intent_executor/error.rs
• magicblock-committor-service/src/intent_executor/mod.rs
• magicblock-committor-service/src/intent_executor/single_stage_executor.rs
• magicblock-committor-service/src/intent_executor/two_stage_executor.rs
• magicblock-committor-service/src/intent_executor/utils.rs
• magicblock-committor-service/src/persist/error.rs
• magicblock-committor-service/src/persist/types/commit_status.rs
• magicblock-committor-service/src/persist/types/commit_strategy.rs
• magicblock-committor-service/src/tasks/commit_delivery.rs
• magicblock-committor-service/src/tasks/commit_finalize_task.rs
• magicblock-committor-service/src/tasks/commit_stage_task.rs
• magicblock-committor-service/src/tasks/commit_task.rs
• magicblock-committor-service/src/tasks/intent_size_validator.rs
• magicblock-committor-service/src/tasks/mod.rs
• magicblock-committor-service/src/tasks/task_builder.rs
• magicblock-committor-service/src/tasks/task_strategist.rs
• magicblock-committor-service/src/tasks/utils.rs
• magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs
• magicblock-committor-service/src/transaction_preparator/mod.rs
• test-integration/test-committor-service/tests/common.rs
• test-integration/test-committor-service/tests/test_delivery_preparator.rs
• test-integration/test-committor-service/tests/test_intent_executor.rs
• test-integration/test-committor-service/tests/test_transaction_preparator.rs
• test-integration/test-committor-service/tests/utils/transactions.rs

💤 Files with no reviewable changes (7)

• test-integration/test-committor-service/tests/utils/transactions.rs
• magicblock-committor-service/src/persist/types/commit_strategy.rs
• magicblock-committor-service/src/persist/error.rs
• magicblock-committor-service/src/persist/types/commit_status.rs
• magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs
• magicblock-committor-service/src/tasks/commit_task.rs
• magicblock-committor-service/src/transaction_preparator/mod.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The committor service replaces separate Commit and Finalize task variants with combined CommitFinalize tasks. Task construction, delivery planning, execution, error handling, buffer preparation, and integration tests now use the combined task model.

Changes

Combined commit-finalize task flow

Layer / File(s)	Summary
Define and build combined tasks magicblock-committor-service/src/tasks/commit_delivery.rs, magicblock-committor-service/src/tasks/commit_finalize_task.rs, magicblock-committor-service/src/tasks/commit_stage_task.rs, magicblock-committor-service/src/tasks/commit_task.rs, magicblock-committor-service/src/tasks/mod.rs, magicblock-committor-service/src/tasks/task_builder.rs, magicblock-committor-service/src/tasks/intent_size_validator.rs, magicblock-committor-service/src/tasks/utils.rs	The task API removes separate commit and finalize task types. CommitDelivery moves to its own module, and TaskBuilderImpl builds combined tasks through shared construction logic. Size validation and account-budget calculations are updated for the combined task model.
Plan delivery and transaction stages magicblock-committor-service/src/tasks/task_strategist.rs	Task strategy construction and tests use combined tasks. They cover inline and buffered state or diff delivery, lookup-table cases, and splitting stages when combined compute usage exceeds the limit.
Update execution and recovery paths magicblock-committor-service/src/intent_executor/*, magicblock-committor-service/src/transaction_preparator/*, magicblock-committor-service/src/committor_processor.rs, magicblock-committor-service/src/persist/error.rs, magicblock-committor-service/src/persist/types/*	Execution and recovery paths no longer perform separate-finalization recovery for unfinalized-account errors. Commit-ID recovery and buffer preparation now select combined tasks. The changes also remove related error variants, status helpers, strategy helpers, and the commit-signature query method.
Migrate integration fixtures and coverage test-integration/test-committor-service/tests/common.rs, test-integration/test-committor-service/tests/test_delivery_preparator.rs, test-integration/test-committor-service/tests/test_intent_executor.rs, test-integration/test-committor-service/tests/test_transaction_preparator.rs, test-integration/test-committor-service/tests/utils/transactions.rs	Integration fixtures and tests use combined tasks in place of separate commit and finalize tasks. Delivery preparation, cleanup, transaction preparation, and nonce checks are updated; obsolete fixture helpers are removed.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant TaskBuilderImpl
  participant TaskStrategist
  participant DeliveryPreparator
  participant SingleStageExecutor
  TaskBuilderImpl->>TaskStrategist: provide combined commit-finalize tasks
  TaskStrategist->>DeliveryPreparator: provide planned task delivery
  DeliveryPreparator->>SingleStageExecutor: provide prepared tasks

Loading

Suggested reviewers: taco-paco

Merge Risk: ⚪ Minimal · up to ca34f

No confirmed issue remains to resolve before merging; proceed with normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to ca34f

Existing authority and ordering controls appear preserved. The main residual risk is compatibility during upgrade and rollback, which is not fully established. No introduced security vulnerability was established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The sensitive outcomes remain base-layer updates to the selected accounts' data and lamports, followed where requested by undelegation. The inspected construction and instruction paths preserve account targets and executor-supplied validator authority; they do not establish an expansion to additional accounts, credentials, or environments.

Trust Boundaries and Controls

• observed — Combined instructions bind the account pubkey, commit ID, and undelegation flag to the supplied validator; buffer addresses derive from validator, account, and commit ID. Failed-intent replay separately checks delegation-session continuity and persisted nonce eligibility, rejecting verification errors. Pending bundles follow a distinct existing rescheduling path.

Resilience and Maintainability Implications

• inferred — Local retry and cleanup ownership remain consistent with the current task universe. Cross-worker atomicity between recovery validation and scheduling is not established, but that recovery code is unchanged, so the uncertainty is not evidence of a regression introduced here.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 59.77% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 87 functions across 18 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: removing the legacy separate Commit and Finalize tasks.
Description check	✅ Passed	The description explains the task removal, the replacement API, the breaking change, and the reported test results. It is directly related to the changeset.
Linked Issues check	✅ Passed	[#1765] The change removes CommitTask, FinalizeTask, their enum variants, and the separate-finalization recovery paths. Builders, preparation, cleanup, and fixtures now use CommitFinalizeTask wh…
Out of Scope Changes check	✅ Passed	The API and helper removals concern legacy task handling, its recovery flow, or abstractions made redundant by combined tasks. The replacement CommitDelivery module and fixture and test updates supp…

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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