[llbartekll]

Issue being fixed or feature implemented

Closes #5626.

A proved fetch on an SDK built without a context provider fails with drive_proof_verifier::Error::ContextProviderNotSet. That error is raised in Sdk::parse_proof_with_metadata_and_proof, after the node has answered, so it reaches the retry loop wrapped in an ExecutionError that names the node. CanRetry for Error treated every Error::Proof as retryable, and a retryable error bans its responder (update_address_ban_status) before the request moves to the next node. An SDK with a local setup fault therefore banned each healthy node it asked and failed only when it ran out of them.

What was done?

• impl CanRetry for Error: Error::Proof(ContextProviderNotSet) is no longer retryable. The request fails on the first answer, is not sent to another node, and bans none.
• The error keeps its Proof shape. Error::cannot_verify_answers already classifies it as the SDK's own setup, and bindings that match on Proof are unchanged. Re-mapping it to Error::Config in From<drive_proof_verifier::Error> was considered and rejected: it changes the public error shape for no gain.
• One sentence on parse_proof_with_metadata_and_proof's doc, next to the existing note on how StaleNodeError::ProtocolVersion is retried.

How Has This Been Tested?

• error::tests::should_not_retry_a_proved_fetch_without_a_context_provider: the variant is non-retryable, also under NoAvailableAddressesToRetry; sibling Proof failures (QuorumKeyUnavailable, ProtocolError) stay retryable, so the carve-out is narrow.
• sync::test::should_fail_without_banning_when_the_context_provider_is_not_set: drives retry over two nodes with retries: Some(3); the closure runs once, the error comes back as Proof(ContextProviderNotSet), and the node that answered is still live with no banned_until.
• cargo test -p dash-sdk --lib: 531 passed. Note: on current v5.1-dev the lib test target does not compile because fix(platform-wallet): retry an oversized InstantSend asset-lock transition with a ChainLock proof #5460 added a platform_version argument to broadcast_with_retries without updating address_nonce_tests.rs; fix(sdk): pass the platform version in the address nonce broadcast tests #5667 fixes that, and the run above applied its diff locally. This PR does not touch that file.
• cargo clippy -p dash-sdk --lib and cargo fmt --all -- --check clean.

Breaking Changes

None.

Checklist:

• I have performed a self-review of my own code
• My Rust, FFI, WASM, Swift or Kotlin changes follow the coding conventions
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
• I have made corresponding changes to the documentation if needed
• If I added or changed GroveDB structure, I described it in the area's structure.rs, regenerated grovedb-structure.json, and checked the structure viewer link posted on this pull request
• If I added or changed a system contract's document schemas, I checked the contract viewer link posted on this pull request

For repository code-owners and collaborators only

• I have assigned this pull request to a milestone

🤖 Generated with Claude Code

PR Hygiene · 332b511

• Bots — thepastaclaw not yet — /skip-bots proceeds without the ones not yet reported
• Self-review — posted; again after any push
• Build running
• Approvals
  • rust-sdk (packages/rs-sdk/src/error.rs, packages/rs-sdk/src/sdk.rs, packages/rs-sdk/src/sync.rs) — lklimek or shumkov

When every merge requirement is met, the PR Hygiene check passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.

Summary by CodeRabbit

• Bug Fixes
  • Proof verification errors caused by a missing context provider or a configuration issue that prevents quorum-key retrieval now return without retrying on another node. The responding node is not banned or skipped, and these errors remain distinct from other proof failures.
  • Other proof failures, stale-node errors, and timeouts remain eligible for retries.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: dashpay/platform
• Branch: fix/sdk-context-provider-not-set-no-ban
• File: .coderabbit.yaml
• Commit: 332b511
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: a96104c0-c1b9-484c-91f8-9eb937f0d7dd

Commits

Reviewing files that changed from the base of the PR and between e754587 and 332b511.

Files selected for processing: 2

• packages/rs-sdk/src/error.rs
• packages/rs-sdk/src/sdk.rs

Files skipped from review as they are similar to previous changes: 1

• packages/rs-sdk/src/sdk.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

The SDK now treats missing-context-provider and provider-configuration proof errors as non-retryable. Tests cover retry classification and verify that a missing-provider error does not trigger another attempt or ban the responding node.

Changes: Context provider proof errors

Layer / File(s)	Summary
Retry classification and policy tests packages/rs-sdk/src/error.rs	can_retry returns false for ContextProviderNotSet and QuorumKeyUnavailable caused by a provider configuration error. Tests cover these errors, the retry-exhaustion wrapper, and confirm that other proof errors remain retryable.
Retry behavior and documentation packages/rs-sdk/src/sync.rs, packages/rs-sdk/src/sdk.rs	A sync test verifies that retry returns the missing-provider error after one call without banning the responding node. SDK documentation describes the context-provider failure behavior.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: shumkov, thepastaclaw

Merge Risk: Minimal · up to 332b5

Requests without a context provider fail without banning healthy nodes. No merge-blocking risk was identified.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the fix and its scope: nodes are no longer banned when the SDK has no context provider. The changeset matches this. Error::can_retry now returns false for Proof(ContextProviderNotSet),…
Linked Issues check		Issue #5626 requires ContextProviderNotSet to be local, non-retryable, and unable to ban a node. CanRetry for Error now returns false for Error::Proof(ContextProviderNotSet). The sync test verif…
Out of Scope Changes check		The changes remain connected to issue #5626. The additional ContextProviderError::Config carve-out covers the same local context-provider setup fault. The tests cover retry classification and node-b…

Full details: Title check

Explanation

The title names the fix and its scope: nodes are no longer banned when the SDK has no context provider. The changeset matches this. Error::can_retry now returns false for Proof(ContextProviderNotSet), the retry path leaves the responding node unbanned, and tests cover both behaviors. The title is short, single-sentence, and specific, and it follows the conventional fix(sdk): form.

Full details: Linked Issues check

Explanation

Issue #5626 requires ContextProviderNotSet to be local, non-retryable, and unable to ban a node. CanRetry for Error now returns false for Error::Proof(ContextProviderNotSet). The sync test verifies one attempt, no failover, and no ban for the answering node. The error test also verifies that other proof errors remain retryable. The added documentation describes the behavior.

Full details: Out of Scope Changes check

Explanation

The changes remain connected to issue #5626. The additional ContextProviderError::Config carve-out covers the same local context-provider setup fault. The tests cover retry classification and node-ban behavior. The SDK documentation change explains the affected proof-parsing behavior. No unrelated change is shown in the reviewed PR evidence.

Full details: Docstring Coverage

Explanation

Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 3 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

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

• Autofix · 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.}

[github-actions]

Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[thepastaclaw]

🔍 Review in progress — actively reviewing now (commit 332b511) · triage: normal
██████████████████░░ 90% · taking longer than usual · running for 1 h 3 min
✅ triage → ✅ Phase 1 → ✅ verify 1 → ✅ gate → ⏳ Phase 2 (0/4 lanes) → ▫️ verify 2 → ▫️ publish
_{Estimated from recent reviews of this tier · updated 12:45 UTC · live progress}

[github-actions]

Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[thepastaclaw]

Final validation — Phase 1 + Phase 2

The change correctly prevents retries and responder bans for the exact ContextProviderNotSet failure, and the relevant CI checks are successful. However, the same SDK-local setup fault can surface as Proof(QuorumKeyUnavailable { error: ContextProviderError::Config(_) }), which the existing classification already identifies as unable to verify any answer; the new wildcard still retries and bans nodes for that case.

🟡 1 suggestion(s) | 💬 1 nitpick(s)

Review provenance

Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: glm-5.3-flash (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: low by gpt-6.1-sol (effort low) — This is a small, well-contained Rust SDK retry-policy change with focused tests that narrowly exempt one local configuration error while preserving existing retry behavior for sibling proof failures.
• Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer, glm-5.3-flash — rust-quality (completed, effort high); agent phase1-reviewer
• Phase 1 model: glm-5.3-flash — zai quota: 5h 97% left, weekly 90% left; passed over gemini-3.8-flash-high (antigravity quota lookup failed)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort medium); agent phase2-reviewer

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-sdk/src/error.rs`:
- [SUGGESTION] packages/rs-sdk/src/error.rs:530-534: QuorumKeyUnavailable with a Config error still retries and bans nodes for a local setup failure
  The new carve-out handles ContextProviderNotSet, but the same local-setup failure can occur when a configured context provider cannot supply quorum keys and returns ContextProviderError::Config. The SDK's own cannot_verify_answers classification already treats Proof(QuorumKeyUnavailable { error: Config(_) }) as a setup that cannot verify any answer. In this path verify_fetching_quorum_key preserves the error as Error::Proof, so it reaches the Error::Proof(_) => true arm; the retry loop then bans the responding node and tries another one even though no node's answer can be verified. Add this sibling shape to the non-retryable arm and cover it with a regression test.
- [NITPICK] packages/rs-sdk/src/error.rs:699: The exhaustion-wrapper assertion does not test propagation of the inner retry classification
  Error::NoAvailableAddressesToRetry is matched by the fallback arm in can_retry and is therefore non-retryable for every inner error. The assertion is correct, but it does not verify that ContextProviderNotSet remains non-retryable through wrapper recursion, because can_retry does not recurse into this wrapper. Clarify that the assertion pins the wrapper's independent non-retryable behavior so future changes do not mistake it for coverage of inner-error propagation.

[github-actions]

Bots are done — your move: post /self-reviewed.
Full checklist in the description.

[github-actions]

Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

The PR correctly prevents missing-provider and quorum-key configuration failures from retrying or banning responding nodes, and the prior review findings are fixed. One sibling local setup failure remains: a provider Config error returned while loading token configuration is wrapped as Proof(ContextProviderError(Config(_))) and still follows the blanket retry-and-ban path. CI checks for the reviewed head are still pending, with no reported code failures.

🟡 1 suggestion(s)

Review provenance

Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: glm-5.3-flash (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: low by gpt-6.1-sol (effort low) — This is a small, well-contained Rust SDK retry-policy change with focused tests, and its correctness is straightforward to verify without affecting consensus, funds, cryptography, or storage.
• Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer, glm-5.3-flash — rust-quality (completed, effort high); agent phase1-reviewer
• Phase 1 model: glm-5.3-flash — zai quota: 5h 94% left, weekly 90% left; passed over gemini-3.8-flash-high (antigravity quota lookup failed)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort medium); agent phase2-reviewer

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-sdk/src/error.rs`:
- [SUGGESTION] packages/rs-sdk/src/error.rs:534-538: Provider Config errors from token-configuration lookup still retry and ban nodes
  The new carve-out handles ContextProviderNotSet and QuorumKeyUnavailable carrying ContextProviderError::Config, but a Config error returned by another context-provider callback has a different verifier shape. In token_perpetual_distribution_last_claim.rs, get_token_configuration(...) uses ?, which produces drive_proof_verifier::Error::ContextProviderError(ContextProviderError::Config(_)); the SDK conversion preserves that as Error::Proof(...), so it reaches Error::Proof(_) => true here. A locally misconfigured provider can therefore ban each healthy responder and retry requests that no other node can make verifiable. Add this sibling shape to the non-retryable classification and add the corresponding shape to cannot_verify_answers so protocol-version refusal classification remains consistent.

Out-of-scope follow-up suggestions (1)

These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.

• Platform activation-height Config errors lose their typed local-setup attribution — The total-credits proof path converts get_platform_activation_height Config errors into a string-valued MissingContextRequirement drive error before the SDK sees them, so the SDK cannot apply the same typed non-retryable classification. This is a concrete related issue, but preserving the provider error requires a change in rs-drive-proof-verifier rather than this SDK-only PR.
  • Follow-up: Track a follow-up in rs-drive-proof-verifier to preserve ContextProviderError through the MissingContextRequirement mapping, then extend the SDK classification once the typed shape is available.

[github-actions]

Bots are done — your move: post /self-reviewed.
Full checklist in the description.

[github-actions]

Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[llbartekll]

/self-reviewed