fix(core): keep on-chain-confirmed orders retryable on transient local faults

[nahimterrazas]

Summary

A newly found strand in the solver-core order-status-persistence path: an order whose fill/claim already succeeded on-chain could be moved to terminal Failed by a transient local storage fault, then excluded from recovery — leaking the on-chain outcome and stranding solver capital.

handle_confirmed runs the !receipt.success gate first, so everything after it executes only when the on-chain transaction succeeded. From there it still:

• retrieves the order (transaction.rs:227 → TransactionError::Storage), and
• performs CAS-backed status/hash writes for Fill/PostFill/PreClaim/Claim (transaction.rs:372,419,471,522 → TransactionError::State).

The TransactionConfirmed catch-all (engine/mod.rs) mapped every non-SettlementCallback error to OrderStatus::Failed(tx_type, …). Failed is terminal (solver-types/order.rs), and recovery only loads non-terminal orders (recovery/mod.rs) and defensively skips Failed — so a momentary Redis latency spike, a lost CAS race after ORDER_CAS_MAX_RETRIES, or a data-repair miss on an on-chain-confirmed order stranded it permanently.

Fill and Claim confirmations have no settlement callback at all, so their only failure path was this catch-all.

Fix

• Extract the inline catch-all into SolverEngine::handle_confirmation_error.
• Add confirmation_failure_policy: since every error reaching it is a local fault after on-chain success, classify Storage/State/Service as retryable — leave the order in its current non-terminal status so startup recovery re-drives the stage from the attempt ledger + on-chain state, instead of writing terminal Failed.
• Mirrors the existing settlement_failure_policy / handle_settlement_stage_error design.
• Genuine on-chain reverts are unaffected — they terminate through the unchanged !receipt.success gate and handle_failed.

No behaviour change for success paths; no new error types.

Tests (RED → GREEN)

Three regression tests, each confirmed RED (order landed in Failed(Fill)/Failed(Claim); policy returned FailOrder) before the fix and GREEN after:

• confirmation_policy_retries_local_processing_errors
• confirmed_state_error_leaves_filled_order_non_terminal
• confirmed_storage_error_leaves_claimed_order_non_terminal

Verification

• cargo test -p solver-core → full suite passes (3 new)
• cargo fmt --all -- --check → clean
• cargo clippy -p solver-core --all-features --all-targets -- -D warnings --allow deprecated → clean

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of transaction errors that happen after an on-chain confirmation, so orders are no longer marked as permanently failed for recoverable local processing issues.
  • Updated error handling to keep affected orders in a non-terminal state when recovery is possible, allowing them to be retried after restart.
  • Expanded test coverage for post-confirmation error classification and order status behavior.

[coderabbitai]

📝 Walkthrough

Walkthrough

The solver engine now classifies errors occurring after on-chain transaction confirmation, preventing local processing faults from incorrectly marking orders as terminally failed. A new confirmation_failure_policy helper maps error variants to retry or fail decisions, and handle_confirmation_error applies this policy within the event loop, with new tests validating non-terminal status preservation.

Changes

Confirmation Error Handling

Layer / File(s)	Summary
Failure policy classification and routing crates/solver-core/src/engine/mod.rs	Adds confirmation_failure_policy to classify TransactionError variants into RetryLater/FailOrder, adds handle_confirmation_error to apply this policy and transition order status accordingly, and updates the TransactionConfirmed branch in run to delegate to the new handler.
Policy and handler test coverage crates/solver-core/src/engine/mod.rs	Adds unit tests confirming RetryLater classification for local processing errors and confirming order status remains non-terminal after State/Storage transaction errors.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• openintentsframework/oif-solver#390: Related propagation of post-confirmation settlement callback failures as TransactionError::SettlementCallback affecting confirmed-tx error handling.
• openintentsframework/oif-solver#392: Also modifies TransactionConfirmed/post-confirmation error routing to classify transient failures as retryable rather than terminal.

Suggested reviewers: shahnami, tirumerla, pepebndc

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly states the core fix: confirmed orders stay retryable after transient local faults.
Description check	✅ Passed	The description covers the bug, fix, tests, and verification, though it doesn't follow the template headings exactly.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/confirmed-order-transient-strand

---

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.}

[coderabbitai]

🧹 Nitpick comments (1)

crates/solver-core/src/engine/mod.rs (1)

1010-1040: 🧹 Nitpick | 🔵 Trivial

No in-process re-drive for RetryLater confirmations — recovery depends on restart.

For the RetryLater path the order is correctly left non-terminal (good, avoids stranding capital), but nothing re-emits TransactionConfirmed or otherwise re-drives the stage in-process; per the doc comment, re-driving happens via startup recovery. If a solver stays up for a long time after a transient Storage/State/Service fault, the order can sit in its non-terminal status until the next restart or an unrelated reconcile.

Consider surfacing this via a metric/alert on confirmation-processing failures, or scheduling an in-process reconcile (similar to spawn_delayed_deferred_retry) so recovery isn't restart-gated. This is a design/ops observation, not a blocker.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/solver-core/src/engine/mod.rs` around lines 1010 - 1040, The
RetryLater branch in handle_confirmation_error leaves the order non-terminal but
does not re-drive confirmation handling in-process, so recovery depends on
restart. Update this path to either schedule an in-process reconcile/retry
(similar to spawn_delayed_deferred_retry) or emit a clear metric/alert from
handle_confirmation_error so long-lived transient failures are observable and
can recover without waiting for startup recovery.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@crates/solver-core/src/engine/mod.rs`:
- Around line 1010-1040: The RetryLater branch in handle_confirmation_error
leaves the order non-terminal but does not re-drive confirmation handling
in-process, so recovery depends on restart. Update this path to either schedule
an in-process reconcile/retry (similar to spawn_delayed_deferred_retry) or emit
a clear metric/alert from handle_confirmation_error so long-lived transient
failures are observable and can recover without waiting for startup recovery.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ff88bd99-001b-4068-a9ef-ab02ab30b181

📥 Commits

Reviewing files that changed from the base of the PR and between b64493a and 1c48ebe.

📒 Files selected for processing (1)

• crates/solver-core/src/engine/mod.rs

[codecov]

Codecov Report

❌ Patch coverage is 86.04651% with 12 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
crates/solver-core/src/engine/mod.rs	86.0%	12 Missing ⚠️

📢 Thoughts on this report? Let us know!