chore(deps): fix quinn-proto Dependabot alert, document blocked serde_with - #938
Merged
Conversation
Lockfile-only bump of quinn-proto 0.11.14 -> 0.11.15, clearing the remote memory-exhaustion advisory in unbounded out-of-order stream reassembly. The crate is an inert optional entry (reqwest's http3 chain) that no feature set of this app links -- `cargo check --all-features --all-targets` never compiles it -- so the bump is dependency hygiene, not a fix for reachable behavior. The second open advisory, GHSA-7gcf-g7xr-8hxj (serde_with below 3.21.0), is deliberately left unfixed: serde_with 2.x is a hard requirement of dashcore-rpc-json, which reaches this repo through pinned revisions of dashpay/platform and dashpay/rust-dashcore. Both still declare serde_with = "2.1.0" at their current development heads, so no lockfile bump can resolve it -- it needs an upstream change in dashpay/rust-dashcore first. A TODO in Cargo.toml and a CHANGELOG note record the blocker and the re-check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collaborator
|
🔍 Review in progress — actively reviewing now (commit 3f47cbf) |
Contributor
📝 WalkthroughWalkthroughCurrent epoch handling now triggers protocol-version confirmation through DPNS data, updates cached epoch values after successful retrieval, and distinguishes confirmed from unconfirmed versions when retrieval is blocked. Changelog and Cargo metadata document dependency advisory status and shielded-operation protocol detection. ChangesPlatform and Security Updates
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant PlatformInfoTask
participant DPNSDataContract
participant ExtendedEpochInfo
participant ContextCache
PlatformInfoTask->>DPNSDataContract: fetch contract to trigger protocol ratcheting
DPNSDataContract-->>ContextCache: update confirmed protocol version
PlatformInfoTask->>ExtendedEpochInfo: fetch current epoch
ExtendedEpochInfo-->>ContextCache: update epoch and fee multiplier
PlatformInfoTask-->>PlatformInfoTask: format confirmed or unconfirmed unavailable response
Suggested labels: Suggested reviewers: 🚥 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 |
![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
🤖 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 `@CHANGELOG.md`:
- Around line 19-22: Update the changelog dependency provenance statement to
identify dashcore-rpc-json as the direct owner of the serde_with = "2.1.0"
declaration, and describe dashpay/platform and dashpay/rust-dashcore only as
repositories supplying pinned revisions. Replace “current development heads”
with the exact relevant revision(s), including the referenced rust-dashcore
commit, so the record is reproducible.
🪄 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: 8c50f522-4561-4a05-8d16-98bc4cb3e03f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
CHANGELOG.mdCargo.toml
Comment on lines
+19
to
+22
| resolved in this repository. `serde_with` 2.x is required by | ||
| `dashcore-rpc-json`, which arrives through pinned revisions of | ||
| `dashpay/platform` and `dashpay/rust-dashcore`; both still declare | ||
| `serde_with = "2.1.0"` at their current development heads as of 2026-07-27. |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the dependency provenance precise and reproducible.
The supplied upstream manifest shows serde_with = "2.1.0" declared directly by dashcore-rpc-json; it does not establish that both dashpay/platform and dashpay/rust-dashcore directly declare the dependency. Distinguish the direct pin owner from the repository that pins that revision, and prefer recording exact revisions over “current development heads” for an auditable changelog. (github.com)
🤖 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 `@CHANGELOG.md` around lines 19 - 22, Update the changelog dependency
provenance statement to identify dashcore-rpc-json as the direct owner of the
serde_with = "2.1.0" declaration, and describe dashpay/platform and
dashpay/rust-dashcore only as repositories supplying pinned revisions. Replace
“current development heads” with the exact relevant revision(s), including the
referenced rust-dashcore commit, so the record is reproducible.
Source: MCP tools
`PlatformInfoTaskRequestType::CurrentEpochInfo` stored `sdk.protocol_version_number()` unconditionally — including when the DPNS fetch that drives the SDK's ratchet failed. That number is seeded locally (`with_initial_version`, v12 in production; the per-network `min_protocol_version` in tests), so a total network failure cached the app's own build-time constant as if the connected network had reported it. This is a live bug on v1.0-dev, not a test artifact: - `AppContext::set_platform_protocol_version` documents `0` as "not fetched yet", and `feature_gate.rs` relies on an unknown version reading as unmet. With the seed cached, `Capability::ShieldedProtocol` (activation v12) opens on zero network evidence. - `mcp::resolve`'s `BestEffortIfUnpopulated` retry polls that same `0` sentinel, so the first failed refresh silenced every later retry for the rest of the session. Cache the ratcheted value only when the DPNS fetch actually succeeded (a proved, verified response), and restore the epoch-derived assignment on the `fetch_current` success path — the most direct network-sourced value, which also lets detection self-heal once dashpay/platform#4231 lands. When nothing is confirmed, the epoch text names the version as unconfirmed instead of printing `0`. The three `mcp::resolve` protocol-version tests pass unmodified; they were correct regression detectors all along. Also moves `mod tests` to the end of the file — it preceded `extract_withdrawal_record`, `build_withdrawals_result` and `impl AppContext`, tripping `clippy::items_after_test_module` under CI's `-D warnings`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lklimek
enabled auto-merge (squash)
July 27, 2026 19:50
Contributor
There was a problem hiding this comment.
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 `@src/backend_task/platform_info.rs`:
- Around line 253-268: Update format_unavailable_current_epoch_info to remove
the issue reference and fee-cache implementation details from both
protocol-version branches. Replace them with a brief, calm, jargon-free message
directing users to try again later, while leaving technical diagnostics to
BannerHandle::with_details.
🪄 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: 88b3d4f7-57df-4085-877c-81b0058b0c14
📒 Files selected for processing (2)
CHANGELOG.mdsrc/backend_task/platform_info.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- CHANGELOG.md
Comment on lines
+253
to
+268
| /// `protocol_version` is `None` while the connected network has not confirmed one. | ||
| fn format_unavailable_current_epoch_info(protocol_version: Option<u32>) -> String { | ||
| 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.)" | ||
| ), | ||
| None => "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(), |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove internal implementation details from the fallback message.
This user-facing result exposes dashpay/platform#4231 and fee-cache internals. Replace them with a brief actionable message (for example, “Current epoch details are temporarily unavailable. Please try again later.”); keep diagnostics in BannerHandle::with_details.
As per coding guidelines, “User-facing error messages must be calm, brief, jargon-free, actionable” and must not expose technical 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 253 - 268, Update
format_unavailable_current_epoch_info to remove the issue reference and
fee-cache implementation details from both protocol-version branches. Replace
them with a brief, calm, jargon-free message directing users to try again later,
while leaving technical diagnostics to BannerHandle::with_details.
Source: Coding guidelines
lklimek
disabled auto-merge
July 27, 2026 19:56
lklimek
enabled auto-merge (squash)
July 27, 2026 19:56
orchardpaytl
pushed a commit
to orchardpaytl/orchardpay
that referenced
this pull request
Jul 27, 2026
…_with (dashpay#938) * chore(deps): bump quinn-proto to fix GHSA-4w2j-m93h-cj5j Lockfile-only bump of quinn-proto 0.11.14 -> 0.11.15, clearing the remote memory-exhaustion advisory in unbounded out-of-order stream reassembly. The crate is an inert optional entry (reqwest's http3 chain) that no feature set of this app links -- `cargo check --all-features --all-targets` never compiles it -- so the bump is dependency hygiene, not a fix for reachable behavior. The second open advisory, GHSA-7gcf-g7xr-8hxj (serde_with below 3.21.0), is deliberately left unfixed: serde_with 2.x is a hard requirement of dashcore-rpc-json, which reaches this repo through pinned revisions of dashpay/platform and dashpay/rust-dashcore. Both still declare serde_with = "2.1.0" at their current development heads, so no lockfile bump can resolve it -- it needs an upstream change in dashpay/rust-dashcore first. A TODO in Cargo.toml and a CHANGELOG note record the blocker and the re-check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(platform): only cache a protocol version the network confirmed `PlatformInfoTaskRequestType::CurrentEpochInfo` stored `sdk.protocol_version_number()` unconditionally — including when the DPNS fetch that drives the SDK's ratchet failed. That number is seeded locally (`with_initial_version`, v12 in production; the per-network `min_protocol_version` in tests), so a total network failure cached the app's own build-time constant as if the connected network had reported it. This is a live bug on v1.0-dev, not a test artifact: - `AppContext::set_platform_protocol_version` documents `0` as "not fetched yet", and `feature_gate.rs` relies on an unknown version reading as unmet. With the seed cached, `Capability::ShieldedProtocol` (activation v12) opens on zero network evidence. - `mcp::resolve`'s `BestEffortIfUnpopulated` retry polls that same `0` sentinel, so the first failed refresh silenced every later retry for the rest of the session. Cache the ratcheted value only when the DPNS fetch actually succeeded (a proved, verified response), and restore the epoch-derived assignment on the `fetch_current` success path — the most direct network-sourced value, which also lets detection self-heal once dashpay/platform#4231 lands. When nothing is confirmed, the epoch text names the version as unconfirmed instead of printing `0`. The three `mcp::resolve` protocol-version tests pass unmodified; they were correct regression detectors all along. Also moves `mod tests` to the end of the file — it preceded `extract_withdrawal_record`, `build_withdrawals_result` and `impl AppContext`, tripping `clippy::items_after_test_module` under CI's `-D warnings`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit 86419a2)
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
Two open Dependabot alerts. One is fixed here with a one-line lockfile bump; the other is
provably not fixable from this repository, so it is documented instead of worked around.
What changed
quinn-proto0.11.14 → 0.11.15 (HIGH, GHSA-4w2j-m93h-cj5j) — remote memory exhaustion fromunbounded out-of-order stream reassembly.
cargo update -p quinn-proto --precise 0.11.15touched exactly two lines of
Cargo.lock(version + checksum); 192 other resolved dependenciesare unchanged.
Worth knowing for reviewers: the crate is an inert optional entry reached through
reqwest'shttp3 feature chain, and it is not in this app's active dependency graph —
cargo tree -i quinn-protoreports nothing with or without--all-features, and the verification run belownever compiles it. Dependabot flags on lockfile presence regardless of reachability, so the bump
is dependency hygiene rather than a fix for behavior a user could ever hit.
serde_with(MEDIUM, GHSA-7gcf-g7xr-8hxj) — documented as blocked, not fixed. A panic whenserializing empty
KeyValueMapentries; the advisory wants ≥ 3.21.0, the lock holds 2.3.3. The2.x constraint is not ours:
dashcore-rpc-jsondeclaresserde_with = "2.1.0"in its ownmanifest, and it reaches us through two levels of pinned git revisions this repo does not
control —
dashpay/platform@288a6cae, which itself pinsdashpay/rust-dashcore@18c68d4c.cargo update -p serde_with --precise 3.21.0therefore fails outright ("candidateversions found which didn't match: 3.21.0 ... required by package
dashcore-rpc-json").There is also no newer upstream revision to move to: both
dashpay/rust-dashcore's currentdevhead and the newer rev (
70d4bf8e) thatdashpay/platformHEAD now pins still declareserde_with = "2.1.0"inrpc-json/Cargo.tomlas of 2026-07-27.Why no workaround
A
[patch]override or a fork would force a 2 → 3 major-version jump on a crate this repo doesnot consume directly, with the API-break risk landing on upstream code we do not own. That is a
maintainer decision and belongs in a PR against
dashpay/rust-dashcore, not in a mechanicaldependency bump. This PR records the blocker in two places instead:
TODOinCargo.tomlabove the pinneddashpay/platformdependency block, namingthe advisory, where the real constraint lives, and when to re-check;
### Securitysection inCHANGELOG.mdcarrying the full story for both advisories.Also in this PR: two CI failures inherited from
v1.0-dev, one of them a live bugThis branch was cut after #936 ("work around dash-sdk epoch-proof regression via version
ratchet") merged to
v1.0-dev, and CI came back red on both Clippy and the test suite —neither caused by the dependency bump above (confirmed:
v1.0-dev's own CI shows the same twofailures before this PR existed; nothing in this PR's original diff touches either file).
clippy::items_after_test_module: fix(platform): work around dash-sdk epoch-proof regression via version ratchet #936 left a#[cfg(test)] mod testsblockahead of real functions in
src/backend_task/platform_info.rs. Moved it to the end of thefile — mechanical, no logic change.
mcp::resolvetests were failingbecause fix(platform): work around dash-sdk epoch-proof regression via version ratchet #936 made
CurrentEpochInfocachesdk.protocol_version_number()unconditionally,including when the DPNS "ratchet trigger" fetch that's supposed to justify trusting that value
had just failed. On failure, that call returns the SDK's local hardcoded seed (11 on the
Mainnet mock, 12 in production) — not anything the network confirmed — and fix(platform): work around dash-sdk epoch-proof regression via version ratchet #936 wrote it into
the cache as if it had been. Concretely, that means
Capability::ShieldedProtocol(activationv12) could open off the app's own seed with zero network evidence, and the
0sentinel thatthe best-effort protocol-version refresh relies on to keep retrying was destroyed on the very
first failed attempt, silencing retries for the rest of the session. This is a live bug on
v1.0-devtoday (fix(platform): work around dash-sdk epoch-proof regression via version ratchet #936 is already merged), found and fixed while chasing a CI failure, notdiscovered by intentional review. Fix: only cache the ratcheted value when the trigger fetch
actually succeeded; restore the direct, network-sourced assignment on the epoch-fetch success
path; and surface "not confirmed yet" in the fee-multiplier-unavailable message instead of a
bogus
0. All three tests pass unmodified — confirmed with a RED-first reproduction(temporarily reinstating fix(platform): work around dash-sdk epoch-proof regression via version ratchet #936's exact unconditional-cache line reproduces the failure on
demand) before restoring the fix.
ProtocolRefresh::Requiredstill returnsOkwhen bothfetches fail — deliberately left alone, since the capability gate fails closed downstream
either way; this is a logging-loudness gap, not a safety one.
Testing
cargo build --all-features,cargo fmt --all -- --check, andcargo clippy --all-features --all-targets -- -D warningsall clean. Scopedcargo nextest run --all-features -E 'test(backend_task::platform_info) + test(mcp::resolve)':7 passed, 0 failed — includes the three previously-failing
mcp::resolvetests (unmodified)plus two
platform_infotests updated for theOption<u32>signature change and one newregression test (
a_failed_ratchet_trigger_leaves_the_protocol_version_unconfirmed) proven viaRED-first reproduction.
cargo check --all-features --all-targetson the original dependencybump alone was exit 0, zero warnings, as before.
Summary by CodeRabbit