Skip to content

fix(chainlink): report error when response_slot < min_context_slot#1317

Closed
0xzrf wants to merge 1 commit into
magicblock-labs:masterfrom
0xzrf:min_slot_err_report
Closed

fix(chainlink): report error when response_slot < min_context_slot#1317
0xzrf wants to merge 1 commit into
magicblock-labs:masterfrom
0xzrf:min_slot_err_report

Conversation

@0xzrf

@0xzrf 0xzrf commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Report error inside RemoteAccountProvider::fetch() when response_slot < min_context_slot instead of panicking on assert!

Summary by CodeRabbit

  • Bug Fixes
    • Improved error handling for remote account provider to gracefully handle cases where response slots don't meet minimum requirements. Pending requests are now properly notified instead of causing system failures.

@coderabbitai

coderabbitai Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

In RemoteAccountProvider::fetch, the async RPC-fetch loop replaces an assertion with explicit runtime validation. When the RPC response slot is below the minimum context slot threshold, the code now formats an error message, notifies pending requests via notify_error, and returns early instead of panicking. Otherwise, execution proceeds to existing account count validation and result handling.

Suggested reviewers

  • GabrielePicco
  • thlorenz
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@0xzrf 0xzrf force-pushed the min_slot_err_report branch from 4ce0a58 to 0745dfe Compare June 13, 2026 11:01

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

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

Inline comments:
In `@magicblock-chainlink/src/remote_account_provider/mod.rs`:
- Around line 2130-2137: The post-fetch check that logs "Response slot
{response_slot} < {min_context_slot}..." (using response_slot, min_context_slot,
pubkeys_str and notify_error) is unreachable because the fetch loop already
retries on slot < min_context_slot via the retry! path; move this explicit
min-context-slot failure reporting into the retry-exhaustion branch that handles
the retry! failure for the slot < min_context_slot case so callers receive a
deterministic "Minimum context slot ... not reached" error; remove the redundant
post-fetch branch or make it a no-op, and ensure the error text matches the
existing downstream "Minimum context slot ... not reached" wording so callers
get the same contract.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 01b39403-dd10-441f-ac48-f522377531fb

📥 Commits

Reviewing files that changed from the base of the PR and between a76aa3b and 0745dfe.

📒 Files selected for processing (1)
  • magicblock-chainlink/src/remote_account_provider/mod.rs

Comment on lines +2130 to +2137
if response_slot < min_context_slot {
let err_msg = format!(
"Response slot {response_slot} < {min_context_slot} fetching accounts {}",
pubkeys_str(&pubkeys)
);
notify_error(&err_msg);
return;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Min-context-slot failure reason is effectively bypassed by current control flow

At Line 2130, this check runs after the fetch loop that already handles slot < min_context_slot (Line 2027) via retry!, so this branch is effectively unreachable in normal execution. On retry exhaustion, callers get a generic max-retries error instead of a deterministic min-context-slot-not-reached reason, which weakens the cross-layer error contract.

Please move the explicit min-context-slot failure reporting into the retry-exhaustion path for the slot < min_context_slot case (and keep the error text aligned with existing “Minimum context slot … not reached” handling used downstream).

🤖 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 `@magicblock-chainlink/src/remote_account_provider/mod.rs` around lines 2130 -
2137, The post-fetch check that logs "Response slot {response_slot} <
{min_context_slot}..." (using response_slot, min_context_slot, pubkeys_str and
notify_error) is unreachable because the fetch loop already retries on slot <
min_context_slot via the retry! path; move this explicit min-context-slot
failure reporting into the retry-exhaustion branch that handles the retry!
failure for the slot < min_context_slot case so callers receive a deterministic
"Minimum context slot ... not reached" error; remove the redundant post-fetch
branch or make it a no-op, and ensure the error text matches the existing
downstream "Minimum context slot ... not reached" wording so callers get the
same contract.

@0xzrf 0xzrf closed this Jun 13, 2026
@0xzrf 0xzrf deleted the min_slot_err_report branch June 13, 2026 11:19
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.

1 participant