fix(settlement): keep transient tracker errors retryable (H-02 follow-up)

[nahimterrazas]

Summary

Follow-up to #410 for audit finding H-02 (transient settlement-callback failure permanently marks confirmed orders as failed).

#410 added the retryable SettlementError::StorageUnavailable / BackendUnavailable variants (classified RetryLater by settlement_failure_policy) and retyped the leaf tracker functions — SettlementMessageTracker::load / save now map StorageError::Backend → StorageUnavailable.

But several intermediate tracker methods still wrapped that load in .map_err(|e| SettlementError::ValidationFailed(...))?, overwriting the retryable variant before it reached the engine. ValidationFailed is classified FailOrder (terminal), so a momentary storage/Redis fault after on-chain confirmation still moved the order to Failed — after which recovery excludes it, stranding solver capital. The recovery replay funnels through the same site, so recovery could not rescue it either.

This PR removes those re-wraps so the typed error propagates via ?, keeping transient storage faults retryable. No new types, no policy change.

Sites fixed (all 5 in the same class)

Site	File
MessageTracker::track_submission	hyperlane.rs
MessageTracker::check_delivery	hyperlane.rs
BroadcasterMessageTracker::track_submission	broadcaster.rs
BroadcasterMessageTracker::store_proof	broadcaster.rs
BroadcasterMessageTracker::mark_verified	broadcaster.rs

track_submission runs on the PostFill confirm callback and the recovery replay (recover_post_fill_state), so it was a live strand. check_delivery is reached via can_claim, which currently swallows the error and returns false — so it was not yet a live strand, but it is the identical shape and is fixed here to close the class and prevent a future refactor from silently re-arming it.

The soft path at broadcaster.rs buffer_coverage_check (tracing::warn! on load error → returns None) is intentionally left untouched — it already degrades gracefully and never reaches the terminal policy.

The change

// before
let mut state = self.load_message(&order_id).await
    .map_err(|e| SettlementError::ValidationFailed(format!("... {e}")))?
    .unwrap_or(...);
// after
let mut state = self.load_message(&order_id).await?   // StorageUnavailable stays retryable
    .unwrap_or(...);

Genuinely permanent conditions stay terminal: deserialization of corrupt persisted state maps to StorageError::Serialization → ValidationFailed (unchanged), and config/decode/missing-chain errors keep their ValidationFailed. Only true backend/connection faults become retryable.

Tests

Added 5 regression tests (one per fixed site) that inject a transient fault at the real StorageInterface::get_bytes seam (MockStorageInterface returning StorageError::Backend) and assert the tracker method surfaces SettlementError::StorageUnavailable(_) rather than ValidationFailed. Each was confirmed RED (failing on ValidationFailed) before the fix and GREEN after.

Uses a test-only solver-storage = { features = ["testing"] } dev-dependency (mirrors the existing solver-service pattern) to reach MockStorageInterface.

Verification

• cargo test -p solver-settlement → 186 passed, 0 failed (181 baseline + 5 new)
• cargo fmt --all -- --check → clean
• cargo clippy -p solver-settlement --all-features --all-targets -- -D warnings --allow deprecated → clean

Net effect

A momentary RPC/storage/message-tracker fault in the cross-chain post-fill callback (Hyperlane or Broadcaster) now leaves the order non-terminal for recovery to re-drive, instead of stranding an on-chain-confirmed order in Failed.

Summary by CodeRabbit

• Bug Fixes

  • Improved handling of transient storage issues so they are now reported as retryable errors instead of being treated as validation failures.
  • Applied this behavior across submission tracking, proof storage, verification, and delivery checks for more reliable recovery during temporary backend outages.

• Tests

  • Added coverage to confirm transient storage failures keep their retryable classification in key settlement flows.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b2af09ea-4478-4c2c-83ed-e038495733d5

📥 Commits

Reviewing files that changed from the base of the PR and between 926e39a and de49600.

📒 Files selected for processing (3)

• crates/solver-settlement/Cargo.toml
• crates/solver-settlement/src/implementations/broadcaster.rs
• crates/solver-settlement/src/implementations/hyperlane.rs

---

📝 Walkthrough

Walkthrough

This PR changes error handling in solver-settlement's Broadcaster and Hyperlane message trackers so that storage load failures propagate their typed SettlementError (preserving StorageUnavailable classification) instead of being collapsed into ValidationFailed. New tests verify retryable behavior; a dev-dependency is added.

Changes

Error propagation fix

Layer / File(s)	Summary
Broadcaster tracker error propagation crates/solver-settlement/src/implementations/broadcaster.rs, crates/solver-settlement/Cargo.toml	track_submission, store_proof, and mark_verified now propagate typed errors from load() instead of mapping to ValidationFailed; new tracker_resilience_tests verify StorageUnavailable is preserved; solver-storage testing dependency added.
Hyperlane tracker error propagation crates/solver-settlement/src/implementations/hyperlane.rs	MessageTracker::track_submission and HyperlaneSettlement::check_delivery propagate typed errors from load_message() instead of mapping to ValidationFailed; new tests confirm StorageUnavailable is preserved for both paths.

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

Possibly related PRs

• openintentsframework/oif-solver#392: Both PRs adjust how SettlementError is propagated/classified so transient storage/service failures remain retryable StorageUnavailable-type errors.

Suggested reviewers: shahnami, tirumerla, pepebndc

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	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: keeping transient tracker errors retryable.
Description check	✅ Passed	The description covers the summary, related issue, testing, and regression tests, though it does not use the exact Testing Process heading.
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/h02-tracker-transient-rewrap

---

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!