fix(platform): stop epoch-fetch retry storm from exhausting the DAPI rate limit - #950
Merged
Merged
Conversation
…rate limit ExtendedEpochInfo::fetch_current sends a descending-epoch-without-start query that dashpay/platform's current proof verifier (pin a18bd158) unconditionally rejects, so it fails identically on every DAPI node. The CurrentEpochInfo task fires automatically on every SPV Syncing->Synced transition, so each failure cycled the SDK's whole address pool and burned the shared per-client rate limit budget -- confirmed via live log analysis as the direct cause of DapiAllAddressesExhausted failures in unrelated flows, including identity top-up. Comment out the broken fetch behind a TODO(platform#4231) citing the revert condition, and hardcode fee_multiplier_permille to 1000 (1.0x) -- not a placeholder, but the value the pinned platform crate's own fee schedules (rs-platform-version v1.rs/v2.rs) actually write on-chain for every protocol version this pin knows. The DPNS-fetch protocol-version ratchet from #936 is untouched. Platform Info screen text now states the multiplier is fixed rather than claiming it's "unavailable". Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughPlatform Info no longer performs live epoch-detail requests. It refreshes protocol data through DPNS, republishes the default fixed fee multiplier, and reports confirmed or unconfirmed protocol versions. MCP tests now mock DPNS responses. ChangesPlatform Info refresh
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant PlatformInfoTask
participant DPNS
participant PlatformFeeEstimator
participant TaskContext
PlatformInfoTask->>DPNS: Retrieve protocol version
PlatformInfoTask->>PlatformFeeEstimator: Obtain default fee multiplier
PlatformInfoTask->>TaskContext: Store fee multiplier
PlatformInfoTask->>PlatformInfoTask: Format unavailable epoch details
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
lklimek
marked this pull request as ready for review
July 31, 2026 12:53
Collaborator
|
✅ Final review complete — no blockers (commit 943822d) |
lklimek
enabled auto-merge (squash)
July 31, 2026 12:54
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/backend_task/platform_info.rs (1)
258-283: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate trailing message text between the two match arms.
The paragraph starting "Epoch details cannot be read while dashpay/platform#4231 is unresolved..." (Line 270-272 and Line 278-280) is identical in both arms. Only the "Protocol Version" line differs between
SomeandNone. Extract the protocol-version line into a local variable and build oneformat!call to avoid keeping two copies of the same sentence in sync.♻️ Proposed refactor to remove the duplicated tail
fn format_hardcoded_current_epoch_info( protocol_version: Option<u32>, fee_multiplier_permille: u64, ) -> String { let fee_multiplier = fee_multiplier_permille as f64 / 1000.0; - match protocol_version { - Some(protocol_version) => format!( - "Current Epoch Information:\n\ - • Protocol Version: {protocol_version}\n\ - • Fee Multiplier: {fee_multiplier}x (a fixed value, not read from the network)\n\n\ - Epoch details cannot be read while dashpay/platform#4231 is unresolved. The fee \ - multiplier shown is the one every network charges today, and it will be read live \ - again once that fix is released." - ), - None => format!( - "Current Epoch Information:\n\ - • Protocol Version: the connected network has not confirmed one yet.\n\ - • Fee Multiplier: {fee_multiplier}x (a fixed value, not read from the network)\n\n\ - Epoch details cannot be read while dashpay/platform#4231 is unresolved. The fee \ - multiplier shown is the one every network charges today, and it will be read live \ - again once that fix is released." - ), - } + let protocol_version_line = match protocol_version { + Some(protocol_version) => protocol_version.to_string(), + None => "the connected network has not confirmed one yet.".to_string(), + }; + format!( + "Current Epoch Information:\n\ + • Protocol Version: {protocol_version_line}\n\ + • Fee Multiplier: {fee_multiplier}x (a fixed value, not read from the network)\n\n\ + Epoch details cannot be read while dashpay/platform#4231 is unresolved. The fee \ + multiplier shown is the one every network charges today, and it will be read live \ + again once that fix is released." + ) }🤖 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 `@src/backend_task/platform_info.rs` around lines 258 - 283, Refactor format_hardcoded_current_epoch_info so the match only constructs the differing protocol-version line in a local variable, then use one shared format! call for the fee multiplier and trailing epoch-details message. Preserve the existing output for both Some and None protocol versions.
🤖 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 `@src/backend_task/platform_info.rs`:
- Around line 258-283: Update both user-facing message arms in
format_hardcoded_current_epoch_info to remove the raw dashpay/platform#4231
tracker reference and replace it with calm, plain-language wording such as a
known upstream network issue. Keep the limitation and expected future
live-reading behavior clear, while retaining any technical reference only in
developer-facing logging or details if already supported.
---
Nitpick comments:
In `@src/backend_task/platform_info.rs`:
- Around line 258-283: Refactor format_hardcoded_current_epoch_info so the match
only constructs the differing protocol-version line in a local variable, then
use one shared format! call for the fee multiplier and trailing epoch-details
message. Preserve the existing output for both Some and None protocol versions.
🪄 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: CHILL
Plan: Pro Plus
Run ID: b48f84b2-6597-47d3-8cb1-07fa91343ec4
📒 Files selected for processing (3)
CHANGELOG.mdsrc/backend_task/platform_info.rssrc/mcp/resolve.rs
Comment on lines
+258
to
283
| /// The fee multiplier is a fixed value, not a network reading — see the | ||
| /// `TODO(platform#4231)` in the `CurrentEpochInfo` arm. | ||
| fn format_hardcoded_current_epoch_info( | ||
| protocol_version: Option<u32>, | ||
| fee_multiplier_permille: u64, | ||
| ) -> String { | ||
| let fee_multiplier = fee_multiplier_permille as f64 / 1000.0; | ||
| match protocol_version { | ||
| Some(protocol_version) => format!( | ||
| "Current Epoch Information:\n\ | ||
| • Protocol Version: {protocol_version}\n\ | ||
| • Epoch details and fee multiplier are temporarily unavailable while \ | ||
| dashpay/platform#4231 is unresolved.\n\n\ | ||
| (The fee multiplier cache was not updated.)" | ||
| • Fee Multiplier: {fee_multiplier}x (a fixed value, not read from the network)\n\n\ | ||
| Epoch details cannot be read while dashpay/platform#4231 is unresolved. The fee \ | ||
| multiplier shown is the one every network charges today, and it will be read live \ | ||
| again once that fix is released." | ||
| ), | ||
| None => "Current Epoch Information:\n\ | ||
| None => format!( | ||
| "Current Epoch Information:\n\ | ||
| • Protocol Version: the connected network has not confirmed one yet.\n\ | ||
| • Epoch details and fee multiplier are temporarily unavailable while \ | ||
| dashpay/platform#4231 is unresolved.\n\n\ | ||
| (The fee multiplier cache was not updated.)" | ||
| .to_string(), | ||
| • Fee Multiplier: {fee_multiplier}x (a fixed value, not read from the network)\n\n\ | ||
| Epoch details cannot be read while dashpay/platform#4231 is unresolved. The fee \ | ||
| multiplier shown is the one every network charges today, and it will be read live \ | ||
| again once that fix is released." | ||
| ), | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Raw issue-tracker reference exposed in user-facing text.
Both arms embed dashpay/platform#4231 directly in text returned as PlatformInfoTaskResult::TextResult, which is what users see. A GitHub issue reference is an internal implementation detail; a typical user cannot act on it and it is not calm, plain-language wording. Consider rephrasing to describe the limitation in plain terms (for example, "a known upstream network issue") and keep the tracker reference in logs or developer-facing details only.
Based on path instructions for src/**/*.rs: "User-facing error messages must be calm, brief, jargon-free, actionable, and describe what happened plus what the user can do. Do not expose raw errors, stack traces, SDK details, or error codes; attach technical details with BannerHandle::with_details."
🤖 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 `@src/backend_task/platform_info.rs` around lines 258 - 283, Update both
user-facing message arms in format_hardcoded_current_epoch_info to remove the
raw dashpay/platform#4231 tracker reference and replace it with calm,
plain-language wording such as a known upstream network issue. Keep the
limitation and expected future live-reading behavior clear, while retaining any
technical reference only in developer-facing logging or details if already
supported.
Source: Path instructions
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR: Adding funds to an identity (and other network actions) could fail with "All Dash network servers are temporarily unreachable" because an unrelated background check was silently burning through the app's shared connection budget.
User story
As a Dash Evo Tool user, I want adding funds to my identity to succeed reliably, so that a temporary unrelated background hiccup doesn't block me from using the app.
Scenario
Base flow
A user opens DET, SPV sync connects, and they try to add funds to an identity (or do any other Platform action).
Actual behavior
Every time the app's connection state settled from "syncing" back to "synced" — which can happen repeatedly in a short window — it silently made a network request that was guaranteed to fail on every server. Each failed attempt used up a slice of a budget shared across all of the app's network requests. When that budget ran out, unrelated actions like adding funds to an identity failed with a generic "All Dash network servers are temporarily unreachable" error, even though the network itself was healthy.
Expected behavior
The app no longer makes that guaranteed-to-fail request. The fee-rate figure it was trying (and failing) to refresh now shows as a fixed, accurate value instead, and the Platform Info screen says so plainly instead of implying something is broken.
Detailed discussion
What was done
Root-caused via live-log triage:
ExtendedEpochInfo::fetch_currentsends a descending-epoch-without-explicit-start query that the platform proof verifier at the current pin (a18bd158) unconditionally rejects (dashpay/platform#4231, open/unmerged). It fails identically on every DAPI node.CurrentEpochInforuns automatically on every SPVSyncing→Syncedtransition (src/app/reconcilers.rs), so each transition cycled the SDK's entire address pool and drained the shared per-client DAPI rate-limit budget — confirmed as the direct cause ofDapiAllAddressesExhaustedin unrelated flows such as identity top-up.src/backend_task/platform_info.rs: the brokenfetch_current()call is commented out behind aTODO(platform#4231)explaining the failure mode and citing the exact revert condition (PR merges,Cargo.toml/Cargo.lockrev advances pasta18bd158).fee_multiplier_permilleis now explicitly hardcoded to1000(1.0x) — not a placeholder default, but the value the pinned platform crate's own fee schedules (rs-platform-versionv1.rs:13/v2.rs:14,uses_version_fee_multiplier_permille: Some(1000)) actually write on-chain for every protocol version this pin knows. The DPNS-fetch protocol-version ratchet added in fix(platform): work around dash-sdk epoch-proof regression via version ratchet #936 is untouched — it works and is unrelated to this bug.format_unavailable_current_epoch_info→format_hardcoded_current_epoch_info, wording now states the multiplier is fixed rather than "unavailable".src/mcp/resolve.rs: three tests sourced their mocked protocol version from the now-disabledfetch_currentbranch and would have gone red; repointed at the working DPNS-fetch mock. Assertions still prove the ratchet mechanism itself works (a failed-ratchet case still asserts the version stays unconfirmed), not just "doesn't panic".CHANGELOG.md: user-facing entry, follows the PR fix(platform): work around dash-sdk epoch-proof regression via version ratchet #936 entry it builds on.No
FetchUnprovedvariant exists forExtendedEpochInfo(checkedrs-sdk/src/platform/fetch_unproved.rs), so there's no way to dodge the broken proof verifier client-side — this is a stopgap until dashpay/platform#4231 merges and the pin advances.Testing
cargo test --lib --all-features -- platform_info::tests resolve::tests— 9 passed, 0 failed (touched tests only, per repo convention).cargo clippy --all-features --all-targets -- -D warnings— clean.cargo fmt --all -- --check— clean.Breaking changes
None — internal error-handling and hardcoded-value change only; no persisted data, public API, or consensus-affecting change.
Checklist
cargo fmt --allcargo clippy --all-features --all-targets -- -D warningsdocs/user-stories.md— not applicable; restores existing behavior, no new storyCHANGELOG.mdupdatedPrior work
Follows up on #936 (
fix(platform): work around dash-sdk epoch-proof regression via version ratchet), which fixed protocol-version detection via the DPNS ratchet but left the brokenfetch_current()call itself running, which this PR now disables.Attribution
🤖 Co-authored by Claudius the Magnificent AI Agent
Summary by CodeRabbit
Bug Fixes
Documentation