feat(dash-spv): devnet LLMQ routing overrides (chainlocks/isd24/platform)#786
feat(dash-spv): devnet LLMQ routing overrides (chainlocks/isd24/platform)#786thepastaclaw wants to merge 2 commits into
Conversation
…atform Mirror Dash Core's `-llmqchainlocks`, `-llmqinstantsenddip0024`, and `-llmqplatform` flags so an SPV client can talk to small devnets that reroute these roles onto `LLMQ_DEVNET` (or any other devnet quorum type). Extends PR dashpay#784's OnceLock override pattern (Option B from the issue) with three new process-global slots. - Add `set_devnet_chain_locks_type`, `set_devnet_isd_type`, `set_devnet_platform_type`, their `*_override()` getters, and the Dash-Core-compatible name parser `devnet_llmq_type_from_name`. - Apply the overrides in `NetworkLLMQExt::{chain_locks_type, isd_llmq_type, platform_type}` for Devnet only; mainnet/testnet/ regtest routing is untouched. - `enabled_llmq_types` stays as the three default devnet types — setters reject non-devnet LLMQ types, so overrides are already a subset. - Surface the three knobs through `ClientConfig` builders, validate them as devnet-only, and apply them in `apply_global_overrides`. - New CLI flags `--llmq-chainlocks`, `--llmq-instantsend-dip0024`, `--llmq-platform`, rejected on non-devnet networks. - Tests cover the parser, the setter lifecycle (set/idempotent/ conflict) via NetworkLLMQExt, non-devnet-type rejection, and cross-network `ClientConfig` rejection. Closes dashpay#785 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
|
Warning Review limit reached
More reviews will be available in 27 minutes and 35 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds devnet-only LLMQ routing overrides to allow small devnets to reroute ChainLocks, InstantSend DIP24, and Platform quorums onto a custom LLMQ type. The feature flows from process-global override storage through network routing, client configuration, and CLI argument parsing, enabling parity with Dash Core's ChangesDevnet LLMQ Routing Overrides
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
dash/src/sml/llmq_type/mod.rs (1)
518-537: ⚡ Quick winPre-existing:
From<u8>is missing the arm for 107 (LlmqtypeDevnetPlatform).The PR notes already flag this as out-of-scope follow-up. Currently
107_u8maps toLlmqtypeUnknown. This won't block devnet override functionality since overrides are set by name, but consensus decoding of quorum commitments using type 107 will misparse.Suggested fix
105 => LLMQType::LlmqtypeDevnetDIP0024, 106 => LLMQType::LlmqtypeTestnetPlatform, + 107 => LLMQType::LlmqtypeDevnetPlatform, _ => LLMQType::LlmqtypeUnknown,🤖 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 `@dash/src/sml/llmq_type/mod.rs` around lines 518 - 537, The From<u8> implementation for LLMQType is missing the match arm for value 107, so bytes with value 107 currently fall through to LlmqtypeUnknown; update the match in impl From<u8> for LLMQType to include a branch mapping 107 => LLMQType::LlmqtypeDevnetPlatform (matching the enum variant name LlmqtypeDevnetPlatform) so that decoding of quorum types correctly recognizes devnet platform type 107.
🤖 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.
Nitpick comments:
In `@dash/src/sml/llmq_type/mod.rs`:
- Around line 518-537: The From<u8> implementation for LLMQType is missing the
match arm for value 107, so bytes with value 107 currently fall through to
LlmqtypeUnknown; update the match in impl From<u8> for LLMQType to include a
branch mapping 107 => LLMQType::LlmqtypeDevnetPlatform (matching the enum
variant name LlmqtypeDevnetPlatform) so that decoding of quorum types correctly
recognizes devnet platform type 107.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 67b60436-4fbf-4f6d-a74c-5b1429748a25
📒 Files selected for processing (4)
dash-spv/src/client/config.rsdash-spv/src/main.rsdash/src/sml/llmq_type/mod.rsdash/src/sml/llmq_type/network.rs
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #786 +/- ##
==========================================
+ Coverage 72.21% 72.31% +0.09%
==========================================
Files 321 321
Lines 70467 70681 +214
==========================================
+ Hits 50890 51115 +225
+ Misses 19577 19566 -11
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Clean, scoped PR that mirrors the existing set_llmq_devnet_params OnceLock pattern for three additional devnet routing slots. The fix commit closes a real decoder gap for LlmqtypeDevnetPlatform (107). One suggestion: ClientConfig::validate() accepts impossible quorum types for the new override fields and only fails later in apply_global_overrides() at client startup, after validate() has already created the storage directory.
🟡 1 suggestion(s)
1 additional finding(s) omitted (not in diff).
🤖 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 `dash-spv/src/client/config.rs`:
- [SUGGESTION] dash-spv/src/client/config.rs:236-275: `validate()` doesn't reject invalid LLMQ types for the new override fields
On devnet, `validate()` only checks that the network is `Devnet`; it never checks whether `llmq_chainlocks_type` / `llmq_instantsend_dip0024_type` / `llmq_platform_type` actually hold a devnet-routable quorum. A programmatic caller can therefore build `ClientConfig::new(Network::Devnet).with_llmq_chainlocks_type(LLMQType::Llmqtype50_60)`, get `Ok(())` from `validate()` (after `validate()` has already invoked `create_dir_all` on the storage path), and only hit the real domain check later when `apply_global_overrides()` runs during client startup via `set_devnet_*_type()`. The CLI path is safe because it parses through `devnet_llmq_type_from_name()`, but the public Rust builder API exposes the wider surface and the validation contract becomes misleading. Mirror the domain check from `set_devnet_routing_override` inside `validate()` so misconfiguration surfaces before any side effects.
github-actions
Bot
added
the
merge-conflict
|
This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them. |
1 participant
Summary
Closes #785. Adds support for Dash Core's devnet LLMQ routing overrides —
-llmqchainlocks,-llmqinstantsenddip0024, and-llmqplatform— so adash-spvclient can talk to small devnets that reroute these roles ontoLLMQ_DEVNET(or another devnet quorum type).Implementation follows PR #784's pragmatic Option B: three new process-global
OnceLocks with idempotent setters, mirroring the establishedset_llmq_devnet_paramscontract.dash/src/sml/llmq_type/mod.rs— new staticsDEVNET_CHAIN_LOCKS_OVERRIDE/DEVNET_ISD_OVERRIDE/DEVNET_PLATFORM_OVERRIDE; helpersset_devnet_chain_locks_type/set_devnet_isd_type/set_devnet_platform_type; gettersdevnet_*_override(); Dash-Core-name parserdevnet_llmq_type_from_nameacceptingllmq_devnet,llmq_devnet_dip0024,llmq_devnet_platform(plus this crate'sllmq_dev_platformspelling).dash/src/sml/llmq_type/network.rs—NetworkLLMQExt::{chain_locks_type, isd_llmq_type, platform_type}honor the overrides onDevnetonly. Mainnet/testnet/regtest arms are untouched.enabled_llmq_typesstays as the three default devnet types — setters already reject anything outside that set, so overrides are a strict subset.dash-spv/src/client/config.rs—ClientConfiggainsllmq_chainlocks_type/llmq_instantsend_dip0024_type/llmq_platform_type, matching builders,validate()devnet-only rejection, and wiring throughapply_global_overrides().dash-spv/src/main.rs— new CLI flags--llmq-chainlocks,--llmq-instantsend-dip0024,--llmq-platform, each rejected on non-devnet networks.Acceptance criteria
ClientConfigcarries the overrides through to startup (apply_global_overrides)chain_locks_type/isd_llmq_type/platform_typereturn the overridden values on Devnet when setenabled_llmq_typescovers the overridden types (already does — override domain is a subset of the default devnet list)dash-spv#784 testtest_llmq_devnet_override_lifecyclestays greenValidation
cargo test -p dashcore --lib sml::llmq_type— 15 passed (4 new: parser, non-devnet-type rejection, three role lifecycle tests; existing devnet-params lifecycle still green).cargo test -p dash-spv --lib client::config— 12 passed (4 new: builder coverage + three cross-network rejection tests).cargo test -p dashcore --lib— 546 passed, 0 failed, 15 ignored.cargo test -p dash-spv --lib— 441 passed, 0 failed, 2 ignored.cargo fmt -p dashcore -p dash-spv --checkclean.cargo clippy -p dashcore -p dash-spv --all-targets -- -D warningsclean.cargo run -p dash-spv --bin dash-spv -- --helpshows the three new flags under their Dash-Core-named forms.Test plan
-llmqinstantsenddip0024=llmq_devnetto confirm InstantSend locks verifyNotes
dash-spv#784: once set, the override is sticky for the lifetime of the process. Conflict re-sets error; identical re-sets are idempotent.impl From<u8> for LLMQTypeindash/src/sml/llmq_type/mod.rsis missing the107 => LlmqtypeDevnetPlatformarm — a pre-existing decoder gap unrelated to this PR, but now relevant if Platform routing is exercised on the wire.🤖 Generated with Claude Code
Summary by CodeRabbit