fix(rpc-json): disable key-wallet default features#587
Open
fix(rpc-json): disable key-wallet default features#587
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds architectural analysis and refactor proposal documentation for the key-wallet crate, covering account-type system redesign, derivation issues, reorg safety handling, PSBT migration planning, and a BDK architecture comparison. Additionally, the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
rpc-json/Cargo.toml (1)
28-28: Technical change looks good; consider whether "fix" is the appropriate PR prefix.The dependency configuration change is correct and safe:
bip32::DerivationPathis a core type that doesn't require thestdormanagerfeatures- Explicitly enabling
serdepreserves the required serialization support- This successfully removes unnecessary dependencies (tokio, rayon) from WASM builds
However, per coding guidelines, the PR title prefix should accurately describe the nature of the change. The "fix" prefix typically indicates a bug correction, whereas this change appears to be a dependency optimization to reduce build size. Unless this is addressing a specific build failure or bug, consider whether "chore" or "perf" would be more appropriate prefixes.
As per coding guidelines, check whether the PR title prefix allowed in the pr-title.yml workflow accurately describes the changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rpc-json/Cargo.toml` at line 28, The dependency change in Cargo.toml (the key-wallet = { path = "../key-wallet", default-features = false, features=["serde"] } line) is fine, but the PR title prefix "fix" is misleading; update the PR title to use a more accurate prefix such as "chore" or "perf" to reflect a dependency/build-size optimization, and if your repository's pr-title.yml workflow validation would reject this prefix, update that workflow's allowed prefixes to include the chosen prefix so the CI check passes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@rpc-json/Cargo.toml`:
- Line 28: The dependency change in Cargo.toml (the key-wallet = { path =
"../key-wallet", default-features = false, features=["serde"] } line) is fine,
but the PR title prefix "fix" is misleading; update the PR title to use a more
accurate prefix such as "chore" or "perf" to reflect a dependency/build-size
optimization, and if your repository's pr-title.yml workflow validation would
reject this prefix, update that workflow's allowed prefixes to include the
chosen prefix so the CI check passes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 58ffda6b-cf5a-466e-b1e4-6c4b465f4d7e
📒 Files selected for processing (1)
rpc-json/Cargo.toml
shumkov
force-pushed
the
fix/rpc-key-wallet-defaults
branch
from
7852251 to
68a7566
Compare
Remove `manager` from key-wallet's default features so crates that only need key types (rpc-json, dpp in WASM builds) don't pull in tokio/rayon. Crates that need the manager module (dash-spv, key-wallet-ffi) already explicitly enable it via features=["manager"] or features=["parallel-filters"]. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
shumkov
force-pushed
the
fix/rpc-key-wallet-defaults
branch
from
68a7566 to
761d62b
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
key-wallet/PSBT_MIGRATION.md (1)
7-19: Add explicit language to fenced code blocks.Line 7 and Line 29 fenced blocks are unlabeled. Add
text(or an appropriate language) to satisfy markdownlint MD040 and improve rendering consistency.Proposed markdown fix
-``` +```text key-wallet/src/psbt/ ├── mod.rs — PartiallySignedTransaction, signing logic (1894 lines) ... └── macros.rs — Internal macros (179 lines)@@
-+text
key-wallet::psbt
├── dashcore::Transaction, TxOut, Script, PublicKey, PrivateKey
...
only dependency on key-walletAlso applies to: 29-37
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@key-wallet/PSBT_MIGRATION.md` around lines 7 - 19, Two unlabeled fenced code blocks in PSBT_MIGRATION.md (the directory listing block and the key-wallet::psbt example block) violate markdownlint MD040 and should be updated to specify a language; edit the two code fences shown around the directory tree and the key-wallet::psbt snippet to use a language tag such as ```text (replace the opening triple backticks on those blocks with ```text) so rendering and linting pass, referencing the blocks containing the directory tree (key-wallet/src/psbt/) and the key-wallet::psbt example.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@key-wallet/BDK_ARCHITECTURE_COMPARISON.md`:
- Around line 63-69: The unlabeled fenced code block starting at the ASCII tree
in the markdown (near the description of ChangeSet / the TxGraph) should be
labeled to satisfy MD040; change the fence from triple backticks with no
language to a text-labeled fence (```text) so the ASCII diagram is treated as
plain text. Update the block that shows "TxGraph ├── txid → Transaction ..."
(the unlabeled block around the ChangeSet section) to use ```text and ensure the
closing fence remains triple backticks.
In `@key-wallet/Cargo.toml`:
- Line 12: The PR title scope is incorrect for the change in
key-wallet/Cargo.toml; update the PR title to use the correct scope (e.g.,
fix(key-wallet) or chore(key-wallet)) or split the changes into separate PRs if
other crates (like rpc-json) are also modified; ensure the chosen scope matches
the crate name "key-wallet" and that the title conforms to the pr-title.yml
allowed prefixes so the CI workflow recognizes it.
In `@key-wallet/PSBT_MIGRATION.md`:
- Around line 1-161: The PR mixes an architecture/design doc (PSBT_MIGRATION.md)
with an unrelated rpc-json dependency fix; split the change by removing or
relocating PSBT_MIGRATION.md into its own focused PR (or alternatively update
the current PR title/description to reflect that it adds the PSBT_MIGRATION.md
design doc) so the PR scope matches its contents; reference the
PSBT_MIGRATION.md file in key-wallet and ensure the rpc-json fix remains in a
separate PR with an appropriate pr-title.yml prefix.
In `@key-wallet/REORG_SAFETY.md`:
- Around line 63-69: The fenced code block showing the TxGraph example lacks a
language tag and triggers markdownlint MD040; update the opening fence for that
block (the one containing "TxGraph" and the four bullet lines including
"anchors: Map<(txid, BlockId) → A>" and "last_evicted: Map<txid → timestamp>")
to use ```text so the block becomes a text-code fence and satisfies the linter.
---
Nitpick comments:
In `@key-wallet/PSBT_MIGRATION.md`:
- Around line 7-19: Two unlabeled fenced code blocks in PSBT_MIGRATION.md (the
directory listing block and the key-wallet::psbt example block) violate
markdownlint MD040 and should be updated to specify a language; edit the two
code fences shown around the directory tree and the key-wallet::psbt snippet to
use a language tag such as ```text (replace the opening triple backticks on
those blocks with ```text) so rendering and linting pass, referencing the blocks
containing the directory tree (key-wallet/src/psbt/) and the key-wallet::psbt
example.
🪄 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: 51525338-83f5-4fbd-a5e5-94a7ffdc8e6a
📒 Files selected for processing (6)
key-wallet/ACCOUNT_TYPE_REFACTOR.mdkey-wallet/BDK_ARCHITECTURE_COMPARISON.mdkey-wallet/Cargo.tomlkey-wallet/KEY_DERIVATION_ISSUES.mdkey-wallet/PSBT_MIGRATION.mdkey-wallet/REORG_SAFETY.md
✅ Files skipped from review due to trivial changes (2)
- key-wallet/KEY_DERIVATION_ISSUES.md
- key-wallet/ACCOUNT_TYPE_REFACTOR.md
Comment on lines
+63
to
+69
| The indexer decides what's relevant (e.g. "does this output pay to one of my script pubkeys"). BDK ships `KeychainTxOutIndex` as a standard indexer, but any implementation works. | ||
|
|
||
| ### 4. `ChangeSet` Pattern — Atomic Persistence | ||
|
|
||
| Every state mutation produces a `ChangeSet`: | ||
|
|
||
| ```rust |
Contributor
There was a problem hiding this comment.
Label fenced block language (MD040).
Line 63 block is unlabeled; use text for the ASCII tree block.
Proposed markdown fix
-```
+```text
TxGraph
├── txid → Transaction (full or partial "floating output")
├── spend relationships (which tx spends which outpoint)
├── anchors (block confirmations per tx)
└── timestamps (first-seen, last-seen, evicted)</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.0)</summary>
[warning] 63-63: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @key-wallet/BDK_ARCHITECTURE_COMPARISON.md around lines 63 - 69, The
unlabeled fenced code block starting at the ASCII tree in the markdown (near the
description of ChangeSet / the TxGraph) should be labeled to satisfy MD040;
change the fence from triple backticks with no language to a text-labeled fence
(text) so the ASCII diagram is treated as plain text. Update the block that shows "TxGraph ├── txid → Transaction ..." (the unlabeled block around the ChangeSet section) to use text and ensure the closing fence remains triple
backticks.
</details>
<!-- fingerprinting:phantom:triton:hawk:77032a02-512f-4716-b981-cb4242ed2d26 -->
<!-- This is an auto-generated comment by CodeRabbit -->
|
|
||
| [features] | ||
| default = ["secp256k1/std", "bip39/std", "getrandom", "rand", "manager"] | ||
| default = ["secp256k1/std", "bip39/std", "getrandom", "rand"] |
Contributor
There was a problem hiding this comment.
PR title scope appears inaccurate for this change.
The change is in key-wallet/Cargo.toml, but the title scope is fix(rpc-json). Please align scope/title with the actual crate changed (or split into focused PRs if both crates are intentionally changed).
As per coding guidelines, "Check whether the PR title prefix allowed in the pr-title.yml workflow accurately describes the changes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@key-wallet/Cargo.toml` at line 12, The PR title scope is incorrect for the
change in key-wallet/Cargo.toml; update the PR title to use the correct scope
(e.g., fix(key-wallet) or chore(key-wallet)) or split the changes into separate
PRs if other crates (like rpc-json) are also modified; ensure the chosen scope
matches the crate name "key-wallet" and that the title conforms to the
pr-title.yml allowed prefixes so the CI workflow recognizes it.
Comment on lines
+1
to
+161
| # PSBT Migration: key-wallet → dash | ||
|
|
||
| ## Current situation | ||
|
|
||
| `key-wallet/src/psbt/` contains a full BIP174 PSBT implementation (~4100 lines across 9 files). It is exported from `key-wallet` as `pub mod psbt` and consumed only by `key-wallet/tests/psbt.rs`. Nothing inside `key-wallet`'s own wallet/account/transaction logic imports it. | ||
|
|
||
| ``` | ||
| key-wallet/src/psbt/ | ||
| ├── mod.rs — PartiallySignedTransaction, signing logic (1894 lines) | ||
| ├── map/ | ||
| │ ├── input.rs — PSBT Input map (616 lines) | ||
| │ ├── output.rs — PSBT Output map (187 lines) | ||
| │ ├── global.rs — PSBT Global map (251 lines) | ||
| │ └── mod.rs — Map trait (34 lines) | ||
| ├── serialize.rs — BIP174 binary serialization (486 lines) | ||
| ├── raw.rs — Raw key/value encoding (228 lines) | ||
| ├── error.rs — Error types (240 lines) | ||
| └── macros.rs — Internal macros (179 lines) | ||
| ``` | ||
|
|
||
| ### Why it doesn't belong in key-wallet | ||
|
|
||
| PSBT is a **transaction serialization and signing coordination format** defined in BIP174. It operates on `Transaction`, `TxIn`, `TxOut`, `Script`, and ECDSA signatures — all types that live in the `dash/` crate. It has no dependency on HD wallet accounts, address pools, gap limits, mnemonics, or any other concept from `key-wallet`. | ||
|
|
||
| The only reason it ended up in `key-wallet` is that `key-wallet`'s `bip32` module provides `ExtendedPrivKey`, `ExtendedPubKey`, and `KeySource`, which PSBT uses for BIP32 derivation metadata in the global map. Because `dash/` has no `bip32` module at all, there was nowhere else to put it. | ||
|
|
||
| ### The actual dependency graph | ||
|
|
||
| ``` | ||
| key-wallet::psbt | ||
| ├── dashcore::Transaction, TxOut, Script, PublicKey, PrivateKey | ||
| ├── dashcore::sighash::{EcdsaSighashType, SighashCache} | ||
| ├── dashcore::crypto::ecdsa | ||
| └── key-wallet::bip32::{ExtendedPrivKey, ExtendedPubKey, KeySource} | ||
| ↑ | ||
| only dependency on key-wallet | ||
| ``` | ||
|
|
||
| `bip32` is the only thing tying PSBT to `key-wallet`. If `bip32` were in `dash/`, PSBT could move there without any further changes. | ||
|
|
||
| --- | ||
|
|
||
| ## Why the correct home is `dash/` | ||
|
|
||
| `dash/` is the Dash protocol library. It already contains: | ||
| - `blockdata/transaction/` — `Transaction`, `TxIn`, `TxOut` | ||
| - `crypto/` — ECDSA keys and signatures | ||
| - `sighash/` — sighash computation | ||
| - `bip143.rs`, `bip152.rs`, `bip158.rs` — other Bitcoin protocol extensions | ||
|
|
||
| PSBT belongs in this layer. It is a protocol-level transaction format, not a wallet-level concept. The correct import for consumers should be `dashcore::psbt::Psbt`, not `key_wallet::psbt::Psbt`. | ||
|
|
||
| --- | ||
|
|
||
| ## Migration plan | ||
|
|
||
| ### Step 1 — Move `bip32` into `dash/` | ||
|
|
||
| This is the prerequisite. `key-wallet`'s `bip32` module wraps and extends `ExtendedPrivKey`/`ExtendedPubKey` with Dash-specific derivation (256-bit `Normal256` child numbers for DashPay). It needs to move to `dash/src/bip32/` and be re-exported from `dashcore`. | ||
|
|
||
| `key-wallet` then imports `bip32` from `dashcore` instead of defining it locally. This is independently valuable — `bip32` types are already used in `dashcore`'s signer interface and have no reason to be wallet-only. | ||
|
|
||
| ### Step 2 — Move `psbt/` into `dash/src/psbt/` | ||
|
|
||
| Once `bip32` is in `dash/`, all PSBT imports become: | ||
|
|
||
| ```rust | ||
| // Before | ||
| use crate::bip32::{ExtendedPrivKey, ExtendedPubKey, KeySource}; | ||
| use dashcore::blockdata::transaction::Transaction; | ||
|
|
||
| // After (from inside dash/) | ||
| use crate::bip32::{ExtendedPrivKey, ExtendedPubKey, KeySource}; | ||
| use crate::blockdata::transaction::Transaction; | ||
| ``` | ||
|
|
||
| The internal `crate::psbt::*` references stay unchanged. Only the crate boundary moves. | ||
|
|
||
| ### Step 3 — Re-export from `key-wallet` for backwards compatibility | ||
|
|
||
| Add a re-export in `key-wallet/src/lib.rs` so existing consumers do not break: | ||
|
|
||
| ```rust | ||
| // key-wallet/src/lib.rs | ||
| pub use dashcore::psbt; | ||
| ``` | ||
|
|
||
| This keeps `key_wallet::psbt::Psbt` working while the canonical path becomes `dashcore::psbt::Psbt`. The re-export can be deprecated and removed in a later breaking release. | ||
|
|
||
| ### Step 4 — Wire PSBT into the transaction builder | ||
|
|
||
| The transaction builder at `key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs` currently builds transactions without PSBT. Once PSBT is in `dash/` and easily importable, the builder should: | ||
|
|
||
| 1. Construct a `Psbt` from the unsigned transaction and UTXO set | ||
| 2. Add BIP32 derivation metadata for each input (the account xpub and child derivation path) | ||
| 3. Return `Psbt` from `build_transaction()` instead of raw `Transaction` | ||
| 4. Sign via `Psbt::sign()` when a private key is available | ||
| 5. Finalize and extract the signed `Transaction` for broadcast | ||
|
|
||
| This is the original motivation for including PSBT in `key-wallet` in the first place — it was added in anticipation of this signing flow but never connected. | ||
|
|
||
| --- | ||
|
|
||
| ## File mapping | ||
|
|
||
| | Current path | Target path | | ||
| |---|---| | ||
| | `key-wallet/src/psbt/mod.rs` | `dash/src/psbt/mod.rs` | | ||
| | `key-wallet/src/psbt/map/input.rs` | `dash/src/psbt/map/input.rs` | | ||
| | `key-wallet/src/psbt/map/output.rs` | `dash/src/psbt/map/output.rs` | | ||
| | `key-wallet/src/psbt/map/global.rs` | `dash/src/psbt/map/global.rs` | | ||
| | `key-wallet/src/psbt/map/mod.rs` | `dash/src/psbt/map/mod.rs` | | ||
| | `key-wallet/src/psbt/serialize.rs` | `dash/src/psbt/serialize.rs` | | ||
| | `key-wallet/src/psbt/raw.rs` | `dash/src/psbt/raw.rs` | | ||
| | `key-wallet/src/psbt/error.rs` | `dash/src/psbt/error.rs` | | ||
| | `key-wallet/src/psbt/macros.rs` | `dash/src/psbt/macros.rs` | | ||
| | `key-wallet/tests/psbt.rs` | `dash/tests/psbt.rs` | | ||
|
|
||
| --- | ||
|
|
||
| ## Import changes required in PSBT source files | ||
|
|
||
| All `crate::psbt::*` internal references stay unchanged. The only imports that need updating are the cross-crate ones: | ||
|
|
||
| ```rust | ||
| // Current (inside key-wallet) | ||
| use crate::bip32::KeySource; | ||
| use crate::bip32::{self, ExtendedPrivKey, ExtendedPubKey}; | ||
| use dashcore::blockdata::script::ScriptBuf; | ||
| use dashcore::blockdata::transaction::txout::TxOut; | ||
| use dashcore::blockdata::transaction::Transaction; | ||
| use dashcore::crypto::ecdsa; | ||
| use dashcore::crypto::key::{PrivateKey, PublicKey}; | ||
| use dashcore::sighash::{self, EcdsaSighashType, SighashCache}; | ||
| use dashcore::Amount; | ||
|
|
||
| // After move (inside dash/) | ||
| use crate::bip32::KeySource; // bip32 now in dash/ | ||
| use crate::bip32::{self, ExtendedPrivKey, ExtendedPubKey}; | ||
| use crate::blockdata::script::ScriptBuf; // already in dash/ | ||
| use crate::blockdata::transaction::txout::TxOut; // already in dash/ | ||
| use crate::blockdata::transaction::Transaction; // already in dash/ | ||
| use crate::crypto::ecdsa; // already in dash/ | ||
| use crate::crypto::key::{PrivateKey, PublicKey}; // already in dash/ | ||
| use crate::sighash::{self, EcdsaSighashType, SighashCache}; // already in dash/ | ||
| use crate::Amount; // already in dash/ | ||
| ``` | ||
|
|
||
| Every `dashcore::` prefix becomes `crate::` — a mechanical find-and-replace. | ||
|
|
||
| --- | ||
|
|
||
| ## Summary | ||
|
|
||
| | Concern | Status | | ||
| |---|---| | ||
| | PSBT is used by key-wallet wallet logic | No — zero imports from `wallet/` | | ||
| | PSBT is used by external crates (ffi, spv) | No — only `key-wallet/tests/psbt.rs` | | ||
| | PSBT has non-protocol dependencies on key-wallet | Only `bip32`, which itself should move to `dash/` | | ||
| | Migration is a breaking API change | Only if `key_wallet::psbt` re-export is removed; can be done gradually | | ||
| | Transaction builder should use PSBT after migration | Yes — that was the original intent | |
Contributor
There was a problem hiding this comment.
PR scope/title does not match the actual change set.
This file introduces architecture/migration design docs, while the PR is titled as a fix(rpc-json) dependency-feature tweak. That mixes unrelated concerns and makes review/release intent ambiguous. Please split docs into a separate PR (or retitle/re-scope this PR to match the actual content).
As per coding guidelines, "Check whether the PR title prefix allowed in the pr-title.yml workflow accurately describes the changes... If a PR mixes unrelated concerns... suggest splitting it into separate focused PRs."
🧰 Tools
🪛 LanguageTool
[style] ~39-~39: Consider an alternative to strengthen your wording.
Context: ...sh/, PSBT could move there without any further changes. --- ## Why the correct home is dash...
(CHANGES_ADJUSTMENTS)
[style] ~100-~100: ‘in anticipation of’ might be wordy. Consider a shorter alternative.
Context: ...llet` in the first place — it was added in anticipation of this signing flow but never connected. ...
(EN_WORDINESS_PREMIUM_IN_ANTICIPATION_OF)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@key-wallet/PSBT_MIGRATION.md` around lines 1 - 161, The PR mixes an
architecture/design doc (PSBT_MIGRATION.md) with an unrelated rpc-json
dependency fix; split the change by removing or relocating PSBT_MIGRATION.md
into its own focused PR (or alternatively update the current PR
title/description to reflect that it adds the PSBT_MIGRATION.md design doc) so
the PR scope matches its contents; reference the PSBT_MIGRATION.md file in
key-wallet and ensure the rpc-json fix remains in a separate PR with an
appropriate pr-title.yml prefix.
Comment on lines
+63
to
+69
| ``` | ||
| TxGraph | ||
| ├── txid → Transaction | ||
| ├── anchors: Map<(txid, BlockId) → A> — all known confirmations | ||
| ├── last_seen: Map<txid → timestamp> — mempool first-seen | ||
| └── last_evicted: Map<txid → timestamp> — eviction timestamp | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add language to fenced code block.
Line 63 fenced block should specify a language (use text) to satisfy markdownlint MD040.
Proposed markdown fix
-```
+```text
TxGraph
├── txid → Transaction
├── anchors: Map<(txid, BlockId) → A> — all known confirmations
├── last_seen: Map<txid → timestamp> — mempool first-seen
└── last_evicted: Map<txid → timestamp> — eviction timestamp</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 63-63: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@key-wallet/REORG_SAFETY.md` around lines 63 - 69, The fenced code block
showing the TxGraph example lacks a language tag and triggers markdownlint
MD040; update the opening fence for that block (the one containing "TxGraph" and
the four bullet lines including "anchors: Map<(txid, BlockId) → A>" and
"last_evicted: Map<txid → timestamp>") to use ```text so the block becomes a
text-code fence and satisfies the linter.
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
1 participant
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.
Summary
Add
default-features = falseto key-wallet dep in rpc-json.rpc-json only needs key-wallet for type definitions and serde serialization — it doesn't use the wallet manager, SPV, or parallel filter functionality.
Without this fix, any crate depending on dashcore-rpc transitively enables key-wallet's default features (
manager,std) which pulls intokioandrayon. This inflates WASM builds unnecessarily (e.g. platform's wasm-sdk).Changes
rpc-json/Cargo.toml: adddefault-features = falseon key-wallet depTesting
cargo check --workspacepassesSummary by CodeRabbit
Chores
managerfeature from default build configuration. Applications requiring this feature must now explicitly enable it.Documentation