Skip to content

chore(deps): bump platform to 54048b9352#849

Merged
lklimek merged 5 commits into
v1.0-devfrom
chore/bump-platform-990c33dcd2
Apr 24, 2026
Merged

chore(deps): bump platform to 54048b9352#849
lklimek merged 5 commits into
v1.0-devfrom
chore/bump-platform-990c33dcd2

Conversation

@lklimek
Copy link
Copy Markdown
Contributor

@lklimek lklimek commented Apr 24, 2026

Summary

Bumps the dashpay/platform dependencies (dash-sdk,
rs-sdk-trusted-context-provider) from rev 2cc4cdcf8 to the current tip
of branch chore/bump-rust-dashcore-309fac8 (sha 54048b9352…).

Branch tip now includes a merge of v3.1-dev into
chore/bump-rust-dashcore-309fac8, so this pulls in additional platform
changes beyond the earlier snapshot.

Follow-on to #834 (the prior bump to 2cc4cdcf8). Pulls in the further
rust-dashcore changes that have accumulated on that branch since.

API adaptations

The new rust-dashcore revision (18b4f2a4) reshapes the QRInfo handoff on
MasternodeListEngine. All four mechanical adaptations land in
src/ui/tools/masternode_list_diff_screen.rs:

  1. Removed QRInfoBlockData import. The bundle type no longer exists —
    callers now pre-populate engine.block_container directly via
    feed_block_height(height, hash). build_qr_info_block_data now
    returns Vec<(CoreBlockHeight, BlockHash)> and the callsites drain it
    into the container before invoking feed_qr_info.
  2. Renamed qr_info_work_block_hashes -> qr_info_referenced_block_hashes.
    The two old enumeration helpers were folded into a single function that
    returns every hash the engine needs heights for.
  3. Dropped the 4th &block_data argument from both
    MasternodeListEngine::feed_qr_info(...) callsites — the new signature
    is (qr_info, verify_tip_non_rotated, verify_rotated).
  4. Imported NetworkExt trait. Network::known_genesis_block_hash()
    moved behind dash_sdk::dpp::dashcore::network::constants::NetworkExt
    and now requires the trait to be in scope.

The "build first, commit second" pattern in build_qr_info_block_data is
preserved so a height-resolution failure does not pollute the engine's
container.

Verification

  • cargo check --all-features — clean
  • cargo +nightly fmt --all — clean
  • CI will run clippy + full test suite on this PR

Not in scope

  • The 18 pre-existing unreachable_patterns warnings (Network is
    #[non_exhaustive] upstream but exhaustively matched in several screens).
    Unrelated to this bump.

🤖 Co-authored by Claudius the Magnificent AI Agent

Summary by CodeRabbit

Release Notes

  • Dependencies

    • Updated pinned dependencies to latest upstream commits.
  • Refactor

    • Improved network handling by removing fallback code paths for unsupported variants, ensuring explicit support for Mainnet, Testnet, Devnet, and Regtest.
    • Refined internal data processing logic for better efficiency.
  • Bug Fixes

    • Enhanced robustness of network configuration and connection handling.

lklimek and others added 2 commits April 24, 2026 11:49
…9fac8 tip, merges v3.1-dev)

Updates dash-sdk and rs-sdk-trusted-context-provider to the current tip of
dashpay/platform's chore/bump-rust-dashcore-309fac8 branch, which now
includes a merge of v3.1-dev. Bumps from the prior 2cc4cdcf8 pin and pulls
in additional rust-dashcore (18b4f2a4) changes plus everything that has
landed on v3.1-dev since.

The new rust-dashcore revision introduces small breaking API changes in
MasternodeListEngine (QRInfoBlockData removed, qr_info_work_block_hashes
renamed, feed_qr_info signature reduced); downstream adaptations land in
the follow-up commit so a bisect can cleanly distinguish "bump broke API"
from "adaptation". The v3.1-dev merge does not introduce additional API
churn affecting this consumer.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…Info API

Follow-on to 8440fb2 (platform bump to 54048b9352). The new rust-dashcore
revision (18b4f2a4) reshapes the QRInfo handoff on MasternodeListEngine:

- The `QRInfoBlockData` type is gone. Callers now pre-populate
  `engine.block_container` directly via `feed_block_height(height, hash)`
  instead of building a separate bundle struct and passing it in.
- `MasternodeListEngine::feed_qr_info` lost its 4th `&block_data` parameter
  to match — it now takes only `(qr_info, verify_tip_non_rotated, verify_rotated)`.
- `qr_info_work_block_hashes` was folded into `qr_info_referenced_block_hashes`,
  which now enumerates every hash the engine needs heights for.
- `Network::known_genesis_block_hash()` moved behind the `NetworkExt` trait;
  added the `use dash_sdk::dpp::dashcore::network::constants::NetworkExt;` import.

`build_qr_info_block_data` now returns `Vec<(CoreBlockHeight, BlockHash)>` —
the caller drains it into `engine.block_container.feed_block_height` before
invoking `feed_qr_info`. The "build first, commit second" pattern is preserved
so a height-resolution failure does not pollute the engine's container.

Verified with `cargo check --all-features` (clean) and `cargo +nightly fmt`.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 24, 2026

Warning

Rate limit exceeded

@lklimek has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 17 minutes and 6 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 17 minutes and 6 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0cf8fd98-429e-4a8e-98d2-8542b3d5c678

📥 Commits

Reviewing files that changed from the base of the PR and between 9f865c6 and 0a2fe90.

📒 Files selected for processing (1)
  • src/ui/tools/masternode_list_diff_screen.rs
📝 Walkthrough

Walkthrough

This PR systematically removes catch-all match arms for the Network enum across the codebase, making pattern matching exhaustive for known variants (Mainnet/Testnet/Devnet/Regtest). Unknown network variants will no longer silently fall back to default behavior. Additionally, two git-based dependencies are pinned to a specific commit.

Changes

Cohort / File(s) Summary
Dependency Pinning
Cargo.toml
Pinned dash-sdk and rs-sdk-trusted-context-provider git dependencies to commit 54048b9352c5c8361f4cbfaa6a188dd3244e00c4.
Network Match Exhaustiveness (RPC & Configuration)
src/config.rs, src/app.rs
Removed wildcard match arms from NetworkConfig::default_rpc_port, Config::config_for_network, Config::update_config_for_network, and ZMQ listener endpoint selection. Unknown network variants will no longer return default ports or listeners.
Network Match Exhaustiveness (Backend Tasks & Context)
src/backend_task/core/mod.rs, src/backend_task/platform_info.rs, src/context/connection_status.rs, src/context/mod.rs, src/context/wallet_lifecycle.rs
Removed fallback branches from GetBestChainLocks, epoch_estimated_time, RPC online status derivation, default_platform_version, and wallet_network_key() matching. Removes panic and silent defaults for unhandled variants.
Network Match Exhaustiveness (Core P2P & SPV)
src/components/core_p2p_handler.rs, src/spv/manager.rs
Removed error early-return and fallback match arms for TCP port selection and SPV data directory derivation; unknown variants will no longer error out immediately or silently default.
UI Network Display Handling
src/ui/network_chooser_screen.rs, src/mcp/server.rs, src/ui/theme.rs
Removed generic fallback labels ("Unknown", "this network", and default accent color) from network display name mapping and theme accent color function. Only explicit variants are now handled.
Masternode List Diff Processing
src/ui/tools/masternode_list_diff_screen.rs
Refactored QRInfo preprocessing to resolve referenced block hashes into a flat (CoreBlockHeight, BlockHash) list, pre-populating the block container before calling feed_qr_info, decoupling from the previous direct bundle consumption.
Test Framework Update
tests/backend-e2e/framework/mnlist_helpers.rs
Added NetworkExt import from dash_sdk::dpp::dashcore::network::constants to enable extension trait access.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 Wildcards trimmed, exhaustive code now flows,
No more silent defaults for networks unknown,
Four paths crystal clear, no mystery shows,
Each network a home, no guessing tossed.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: bumping platform dependencies to a specific commit hash.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/bump-platform-990c33dcd2

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.

@thepastaclaw
Copy link
Copy Markdown
Collaborator

thepastaclaw commented Apr 24, 2026

Review Gate

Commit: 0a2fe902

  • Debounce: 4m ago (need 30m)

  • CI checks: checks still running (2 pending)

  • CodeRabbit review: comment found

  • Off-peak hours: peak window (5am-11am PT) — currently 07:15 AM PT Friday

  • Run review now (check to override)

…tches

Upstream Network enum stabilized to exactly four variants
(Mainnet, Testnet, Devnet, Regtest) at the 54048b9352 bump, making
the defensive `_ => …` fallbacks at 17 match sites unreachable. With
`-D warnings` enabled in CI these became hard errors.

Deleted the `_` arm (and any tombstone comments) at each site. Where the
catch-all did semantically distinct work (returning an Error, panicking,
logging a tracing::warn!, emitting a debug-formatted fallback string),
that behavior is now dead code — the four explicit arms already cover the
entire enum domain. The unreachable `P2PError::UnsupportedNetwork` variant
is left in place as it's part of the pub API surface.

Also:
- `src/app_dir.rs::core_cookie_path`: the fallback removal simplified the
  closure to always return `Ok(...)`, so `and_then` is now idiomatically
  `map` (clippy::bind_instead_of_map).
- `tests/backend-e2e/framework/mnlist_helpers.rs`: added the missing
  `NetworkExt` trait import (same fix as in
  src/ui/tools/masternode_list_diff_screen.rs from the prior commit).

Part of the chore/bump-rust-dashcore-309fac8 platform bump (PR #849).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lklimek lklimek requested a review from Copilot April 24, 2026 12:18
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the pinned dashpay/platform dependencies to revision 54048b9352 and applies the required downstream API adjustments (notably around NetworkExt and MasternodeListEngine QRInfo feeding).

Changes:

  • Bump dash-sdk / rs-sdk-trusted-context-provider git revisions to 54048b9352… (and refresh Cargo.lock).
  • Adapt masternode list diff tool to the updated MasternodeListEngine QRInfo API (block height pre-feeding + updated feed_qr_info signature).
  • Remove now-unneeded fallback match arms for Network across several components (suggesting Network is now exhaustive upstream).

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
Cargo.toml Pins platform dependencies to 54048b9352….
Cargo.lock Updates the resolved dependency graph for the new platform tip (incl. new crates).
src/ui/tools/masternode_list_diff_screen.rs Updates QRInfo block/height preparation and feed_qr_info callsites; imports NetworkExt.
tests/backend-e2e/framework/mnlist_helpers.rs Imports NetworkExt for known_genesis_block_hash() usage.
src/ui/theme.rs Removes a default Network match arm in network_accent.
src/ui/network_chooser_screen.rs Removes “unknown network” fallback labels in Network match arms.
src/spv/manager.rs Removes fallback ports/dir naming for non-standard networks.
src/mcp/server.rs Removes "unknown" fallback in network display formatting.
src/context/wallet_lifecycle.rs Removes fallback mapping of unknown network to mainnet wallet key.
src/context/mod.rs Makes default_platform_version() exhaustive over supported networks.
src/context/connection_status.rs Makes chainlock “online” computation exhaustive by network.
src/config.rs Makes network-to-config selection and port defaults exhaustive by network.
src/components/core_p2p_handler.rs Makes default P2P port selection exhaustive by network.
src/backend_task/platform_info.rs Makes epoch timing selection exhaustive by network.
src/backend_task/core/mod.rs Makes active-network chainlock selection exhaustive by network.
src/app_dir.rs Simplifies cookie/config selection by relying on exhaustive Network matching.
src/app.rs Makes ZMQ endpoint default selection exhaustive by network.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/ui/tools/masternode_list_diff_screen.rs Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

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 the current code and only fix it if needed.

Inline comments:
In `@src/ui/tools/masternode_list_diff_screen.rs`:
- Around line 410-438: build_qr_info_block_data currently fails the entire
QRInfo when the cycle-boundary lookup (get_block_hash_and_cache(height +
WORK_DIFF_DEPTH)) hits tip-proximal missing blocks and also uses
block_container.feed_block_height directly instead of the engine API; update
build_qr_info_block_data to treat a "height out of range" / RPC not-found error
from get_block_hash_and_cache as non-fatal (skip adding the cycle_boundary pair)
while still returning Err for other errors, and ensure callers use
MasternodeListEngine::feed_block_height (or call engine.feed_block_height)
rather than block_container.feed_block_height so the engine-level hooks/caching
remain consistent; reference functions: build_qr_info_block_data,
MasternodeListEngine::qr_info_referenced_block_hashes, get_height_and_cache,
get_block_hash_and_cache, and feed_block_height on the engine.
🪄 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

Run ID: 03b0c41e-f500-4b75-b78e-6ae54e0ed638

📥 Commits

Reviewing files that changed from the base of the PR and between dbfc4d4 and 9f865c6.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (16)
  • Cargo.toml
  • src/app.rs
  • src/app_dir.rs
  • src/backend_task/core/mod.rs
  • src/backend_task/platform_info.rs
  • src/components/core_p2p_handler.rs
  • src/config.rs
  • src/context/connection_status.rs
  • src/context/mod.rs
  • src/context/wallet_lifecycle.rs
  • src/mcp/server.rs
  • src/spv/manager.rs
  • src/ui/network_chooser_screen.rs
  • src/ui/theme.rs
  • src/ui/tools/masternode_list_diff_screen.rs
  • tests/backend-e2e/framework/mnlist_helpers.rs
💤 Files with no reviewable changes (12)
  • src/context/connection_status.rs
  • src/backend_task/platform_info.rs
  • src/mcp/server.rs
  • src/app.rs
  • src/components/core_p2p_handler.rs
  • src/context/wallet_lifecycle.rs
  • src/backend_task/core/mod.rs
  • src/context/mod.rs
  • src/ui/theme.rs
  • src/config.rs
  • src/ui/network_chooser_screen.rs
  • src/spv/manager.rs

Comment thread src/ui/tools/masternode_list_diff_screen.rs
…height feed API, tolerate missing cycle-boundary blocks

Addresses three reviewer concerns on `src/ui/tools/masternode_list_diff_screen.rs`
raised against PR #849:

1. **Build-first, commit-second contract restored.** `build_qr_info_block_data`
   is documented as a pure build step, but previously called
   `get_height_and_cache`, which mutated the engine via `feed_block_height` on
   every RPC miss. A mid-loop failure would leave the engine partially
   populated. Introduce a private `resolve_height` helper that only reads from
   the engine's `block_container`, then the local `block_height_cache`, then
   falls back to Core RPC (populating `block_height_cache` but never the
   engine). Refactor `get_height_and_cache` to delegate to `resolve_height`
   and then feed the engine explicitly. All 12 existing callers keep their
   previous lookup+feed semantics. `build_qr_info_block_data` now calls
   `resolve_height`, making the build phase side-effect-free on the engine.

2. **Unified feed-block-height API.** Every call site in this file that fed
   block heights into the engine via the pass-through form
   `engine.feed_block_height(...)` has been converted to the explicit
   container form `engine.block_container.feed_block_height(...)`. Touched
   sites: `get_height_and_cache` (line 417), `feed_mn_list_diff_heights`
   (lines 706, 720), and `feed_quorum_entry_height` (line 735). Sites that
   already used the explicit form (1440, 4305) were left alone. No other
   feed_* APIs in this file had the same inconsistency.

3. **Graceful degradation on missing cycle-boundary blocks.** In
   `build_qr_info_block_data`, when `get_block_hash_and_cache` fails to
   resolve a cycle-boundary height (`height + WORK_DIFF_DEPTH`) we no longer
   abort the whole ingestion. We log a `tracing::warn!` with the referenced
   hash, referenced height, missing boundary height, and underlying error,
   then `continue` — the engine may still succeed in `feed_qr_info`; if it
   doesn't, the error surfaces from `feed_qr_info` where it belongs.
@lklimek lklimek enabled auto-merge (squash) April 24, 2026 14:18
@lklimek lklimek merged commit 22e2103 into v1.0-dev Apr 24, 2026
5 checks passed
@lklimek lklimek deleted the chore/bump-platform-990c33dcd2 branch April 24, 2026 14:20
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.

3 participants