Skip to content

chore(dashcore): drop no-std supp in dashcore#521

Merged
xdustinface merged 1 commit intov0.42-devfrom
chore/drop-nostd-dashcore
Mar 19, 2026
Merged

chore(dashcore): drop no-std supp in dashcore#521
xdustinface merged 1 commit intov0.42-devfrom
chore/drop-nostd-dashcore

Conversation

@ZocoLini
Copy link
Copy Markdown
Collaborator

@ZocoLini ZocoLini commented Mar 12, 2026

need hashes std to be dropped first

Summary by CodeRabbit

  • Refactor

    • Library unified to a single standard-library (std) build, removing no-std conditional paths.
    • Error trait implementations simplified and standardized across the codebase, making error behavior more consistent.
    • Public prelude and re-exports consolidated to std types; some public modules were simplified or removed.
  • Chores

    • Dependency and feature declarations updated across crates to align with the unified std build and optional features.

@ZocoLini ZocoLini marked this pull request as draft March 12, 2026 01:07
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 12, 2026

📝 Walkthrough

Walkthrough

Removed conditional no-std/std compilation and reworked feature wiring: Cargo manifests updated to stop forcing "std" in some consumers; many std-gated impl std::error::Error blocks were deleted or made unconditional; crate prelude and io exports unified to std-based re-exports.

Changes

Cohort / File(s) Summary
Cargo manifests
dash-spv/Cargo.toml, dash/Cargo.toml, key-wallet-ffi/Cargo.toml, key-wallet-manager/Cargo.toml, rpc-json/Cargo.toml
Removed or stopped forcing std in several dashcore consumers, adjusted default-features usage, added/rewired many optional dependencies and feature lists in dash/Cargo.toml.
Crate root & prelude
dash/src/lib.rs
Removed no-std compile_error and non-std io_extras; added unconditional pub use std::io and unified prelude exports to std-based types/collections.
Removed std Error impls
dash/src/address.rs, dash/src/base58.rs, dash/src/bip152.rs, dash/src/bip158.rs, dash/src/blockdata/block.rs, dash/src/blockdata/locktime/absolute.rs, dash/src/merkle_tree/block.rs, dash/src/string.rs, dash/src/taproot.rs, ...
Deleted many #[cfg(feature = "std")] impl std::error::Error blocks, removing std Error trait bridges for multiple error types.
Unconditional / moved Error impls & encodings
dash/src/blockdata/locktime/relative.rs, dash/src/blockdata/transaction/outpoint.rs, dash/src/crypto/ecdsa.rs, dash/src/crypto/sighash.rs, dash/src/crypto/taproot.rs, dash/src/sign_message.rs, dash/src/error.rs, dash/src/consensus/encode.rs, dash/src/pow.rs, ...
Removed #[cfg(feature = "std")] guards so several impl std::error::Error implementations, some impl_vec! encodings, and tests/imports are now unconditional.
Network surface & modules
dash/src/network/mod.rs
Removed multiple std-gated public modules and re-exports (address, many message_* modules) and removed std Error trait impl for the network Error.
Vec / alloc re-exports
dash/src/ephemerealdata/chain_lock.rs, dash/src/ephemerealdata/instant_lock.rs
Removed or tightened Vec import/re-exports; some re-exports are now test-only rather than gated by std.
Key & error handling adjustments
dash/src/crypto/key.rs, internals/src/error.rs
Made PrivateKey derive Debug unconditionally (removed manual impl), changed PublicKey::read_from to propagate underlying error, and simplified write_err macro to a single unconditional body.
Misc gating removals & tests
dash/src/amount.rs, dash/src/consensus/serde.rs, dash/src/blockdata/script/mod.rs, dash/src/consensus/encode.rs, dash/src/bip158.rs, ...
Removed various #[cfg(feature = "std")] guards from tests, imports, small methods and encoding impls; minor import tidy-ups.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 Hops of change across the glade,
Guards unpinned and pathways made.
Std steps in where branches stood,
Errors trimmed and exports good.
A nibble, a twitch — the build is glad.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'chore(dashcore): drop no-std supp in dashcore' directly and clearly describes the main change—removing no-std (no standard library) support from the dashcore crate, which aligns with the extensive modifications across dependency declarations, error trait implementations, and module exports throughout the changeset.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/drop-nostd-dashcore
📝 Coding Plan
  • Generate coding plan for human review comments

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.

@github-actions github-actions Bot added the merge-conflict The PR conflicts with the target branch. label Mar 18, 2026
@github-actions
Copy link
Copy Markdown

This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them.

@ZocoLini ZocoLini force-pushed the chore/drop-nostd-dashcore branch from 99b001c to 0900af5 Compare March 18, 2026 16:54
@github-actions github-actions Bot removed the merge-conflict The PR conflicts with the target branch. label Mar 18, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.37%. Comparing base (074acad) to head (b408a3e).
⚠️ Report is 1 commits behind head on v0.42-dev.

Additional details and impacted files
@@              Coverage Diff              @@
##           v0.42-dev     #521      +/-   ##
=============================================
+ Coverage      66.06%   66.37%   +0.30%     
=============================================
  Files            311      311              
  Lines          64976    64976              
=============================================
+ Hits           42928    43127     +199     
+ Misses         22048    21849     -199     
Flag Coverage Δ
core 75.13% <ø> (ø)
ffi 37.53% <ø> (+1.90%) ⬆️
rpc 28.28% <ø> (ø)
spv 81.19% <ø> (+0.04%) ⬆️
wallet 65.93% <ø> (ø)
Files with missing lines Coverage Δ
dash/src/address.rs 52.95% <ø> (ø)
dash/src/amount.rs 87.85% <ø> (ø)
dash/src/base58.rs 80.54% <ø> (ø)
dash/src/bip152.rs 80.25% <ø> (ø)
dash/src/bip158.rs 94.80% <ø> (ø)
dash/src/blockdata/block.rs 68.80% <ø> (ø)
dash/src/blockdata/locktime/absolute.rs 59.58% <ø> (ø)
dash/src/blockdata/locktime/relative.rs 61.98% <ø> (ø)
dash/src/blockdata/script/mod.rs 48.20% <ø> (ø)
dash/src/blockdata/transaction/outpoint.rs 77.50% <ø> (ø)
... and 15 more

... and 18 files with indirect coverage changes

@github-actions
Copy link
Copy Markdown

This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them.

@github-actions github-actions Bot added the merge-conflict The PR conflicts with the target branch. label Mar 18, 2026
@ZocoLini ZocoLini force-pushed the chore/drop-nostd-dashcore branch from 0900af5 to 5581f99 Compare March 18, 2026 19:50
@github-actions github-actions Bot removed the merge-conflict The PR conflicts with the target branch. label Mar 18, 2026
@ZocoLini ZocoLini force-pushed the chore/drop-nostd-dashcore branch from 5581f99 to 80bbdbb Compare March 18, 2026 20:11
@ZocoLini ZocoLini marked this pull request as ready for review March 18, 2026 20:12
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: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
dash/src/lib.rs (2)

31-44: ⚠️ Potential issue | 🟡 Minor

Documentation still references removed no-std feature.

Lines 42-43 document the no-std feature flag, but this PR is removing no-std support. Additionally, line 33 describes std as "default" when it's now effectively required. Please update the documentation to reflect these changes.

📝 Suggested documentation update
 //! ## Available feature flags
 //!
-//! * `std` - the usual dependency on `std` (default).
+//! * `std` - dependency on `std` (required).
 //! * `secp-recovery` - enables calculating public key from a signature and message.
 //! * `signer` - enables singing and validation ECDSA helpers.
 //! * `base64` - (dependency), enables encoding of PSBTs and message signatures.
 //! * `unstable` - enables unstable features for testing.
 //! * `rand` - (dependency), makes it more convenient to generate random values.
 //! * `bincode` - (dependency), implements bincode serialization and deserialization.
 //! * `serde` - (dependency), implements `serde`-based serialization and deserialization.
 //! * `secp-lowmemory` - optimizations for low-memory devices.
-//! * `no-std` - enables additional features required for this crate to be usable
-//!   without std. Does **not** disable `std`. Depends on `core2`.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@dash/src/lib.rs` around lines 31 - 44, Update the crate-level documentation
in dash/src/lib.rs to remove the stale `no-std` feature entry and adjust the
`std` feature description: delete the bullet mentioning `no-std` (lines
referencing the `no-std` feature) and change the `std` bullet (the line that
currently says "`std` - the usual dependency on `std` (default).") to reflect
that `std` is required (e.g., "`std` - required; this crate depends on the
standard library"). Leave the rest of the feature list unchanged.

64-65: ⚠️ Potential issue | 🟡 Minor

Migrate from alloc to std equivalents after removing no-std support.

The extern crate alloc declaration and #[macro_use] are actively used in four files:

  • dash/src/hash_types.rs:78use crate::alloc::string::ToString
  • dash/src/ephemerealdata/instant_lock.rs:6use alloc::vec::Vec
  • dash/src/consensus/serde.rs:19use crate::alloc::string::ToString
  • dash/src/blockdata/script/mod.rs:51use alloc::rc::Rc

If no-std support is being removed and std is now required, replace these with their std:: equivalents (or use the standard prelude) and remove the extern crate alloc declaration from lib.rs.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@dash/src/lib.rs` around lines 64 - 65, Remove the top-level #[macro_use]
extern crate alloc declaration in lib.rs and replace all uses of alloc types
with their std equivalents: change alloc::vec::Vec to std::vec::Vec (or plain
Vec), crate::alloc::string::ToString to std::string::ToString (or use the
prelude's ToString), and alloc::rc::Rc to std::rc::Rc; update the four affected
modules (hash_types.rs, ephemerealdata/instant_lock.rs, consensus/serde.rs,
blockdata/script/mod.rs) to import the std types or rely on the standard prelude
and delete the extern crate alloc line from lib.rs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@dash/Cargo.toml`:
- Line 22: The examples 'handshake', 'ecdsa-psbt', and 'taproot-psbt' reference
a non-existent "std" feature in Cargo.toml; remove the stale requirement by
deleting the line `required-features = ["std"]` from the handshake example and
by removing `"std"` from the required-features arrays for ecdsa-psbt and
taproot-psbt so their required-features only list valid features defined in the
[features] section.

In `@internals/src/error.rs`:
- Around line 18-19: The docs for the write_err! macro are out of date: they
claim the source text is appended in no-std mode but the implementation (see the
let _ = &$source; line and the write!($writer, $string $(, $args)*) call) now
always formats only $string and $args. Update the macro documentation/comment to
state that write_err! no longer appends source text in any mode, clarify that
$source is only referenced to silence clippy and not emitted, and remove or
rephrase any wording implying conditional no-std behavior for source appending.

---

Outside diff comments:
In `@dash/src/lib.rs`:
- Around line 31-44: Update the crate-level documentation in dash/src/lib.rs to
remove the stale `no-std` feature entry and adjust the `std` feature
description: delete the bullet mentioning `no-std` (lines referencing the
`no-std` feature) and change the `std` bullet (the line that currently says
"`std` - the usual dependency on `std` (default).") to reflect that `std` is
required (e.g., "`std` - required; this crate depends on the standard library").
Leave the rest of the feature list unchanged.
- Around line 64-65: Remove the top-level #[macro_use] extern crate alloc
declaration in lib.rs and replace all uses of alloc types with their std
equivalents: change alloc::vec::Vec to std::vec::Vec (or plain Vec),
crate::alloc::string::ToString to std::string::ToString (or use the prelude's
ToString), and alloc::rc::Rc to std::rc::Rc; update the four affected modules
(hash_types.rs, ephemerealdata/instant_lock.rs, consensus/serde.rs,
blockdata/script/mod.rs) to import the std types or rely on the standard prelude
and delete the extern crate alloc line from lib.rs.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9a81fad0-3470-48c9-a4db-934741eb1137

📥 Commits

Reviewing files that changed from the base of the PR and between 074acad and 80bbdbb.

📒 Files selected for processing (31)
  • dash-spv/Cargo.toml
  • dash/Cargo.toml
  • dash/src/address.rs
  • dash/src/amount.rs
  • dash/src/base58.rs
  • dash/src/bip152.rs
  • dash/src/bip158.rs
  • dash/src/blockdata/block.rs
  • dash/src/blockdata/locktime/absolute.rs
  • dash/src/blockdata/locktime/relative.rs
  • dash/src/blockdata/script/mod.rs
  • dash/src/blockdata/transaction/outpoint.rs
  • dash/src/consensus/encode.rs
  • dash/src/crypto/ecdsa.rs
  • dash/src/crypto/key.rs
  • dash/src/crypto/sighash.rs
  • dash/src/crypto/taproot.rs
  • dash/src/ephemerealdata/chain_lock.rs
  • dash/src/ephemerealdata/instant_lock.rs
  • dash/src/error.rs
  • dash/src/lib.rs
  • dash/src/merkle_tree/block.rs
  • dash/src/network/mod.rs
  • dash/src/pow.rs
  • dash/src/sign_message.rs
  • dash/src/string.rs
  • dash/src/taproot.rs
  • internals/src/error.rs
  • key-wallet-ffi/Cargo.toml
  • key-wallet-manager/Cargo.toml
  • rpc-json/Cargo.toml
💤 Files with no reviewable changes (22)
  • dash/src/crypto/taproot.rs
  • dash/src/ephemerealdata/chain_lock.rs
  • dash/src/blockdata/block.rs
  • dash/src/network/mod.rs
  • dash/src/consensus/encode.rs
  • dash/src/bip158.rs
  • dash/src/sign_message.rs
  • dash/src/blockdata/locktime/absolute.rs
  • dash/src/crypto/sighash.rs
  • dash/src/blockdata/locktime/relative.rs
  • dash/src/blockdata/script/mod.rs
  • dash/src/string.rs
  • dash/src/amount.rs
  • dash/src/address.rs
  • dash/src/base58.rs
  • dash/src/bip152.rs
  • dash/src/taproot.rs
  • dash/src/pow.rs
  • dash/src/crypto/ecdsa.rs
  • dash/src/merkle_tree/block.rs
  • dash/src/error.rs
  • dash/src/blockdata/transaction/outpoint.rs

Comment thread dash/Cargo.toml Outdated
Comment thread internals/src/error.rs
coderabbitai[bot]
coderabbitai Bot previously approved these changes Mar 18, 2026
@github-actions github-actions Bot added the ready-for-review CodeRabbit has approved this PR label Mar 18, 2026
@ZocoLini ZocoLini force-pushed the chore/drop-nostd-dashcore branch from 80bbdbb to 57e23a3 Compare March 18, 2026 20:53
@github-actions github-actions Bot removed the ready-for-review CodeRabbit has approved this PR label Mar 18, 2026
@ZocoLini ZocoLini force-pushed the chore/drop-nostd-dashcore branch from 57e23a3 to b191e23 Compare March 18, 2026 20:59
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

🧹 Nitpick comments (1)
dash/src/ephemerealdata/instant_lock.rs (1)

5-12: Consider simplifying Vec imports if std is now unconditional.

Since the PR drops no-std support, std is always available. Both alloc::vec::Vec and std::vec::Vec are the same type, so the conditional import logic adds unnecessary complexity. You could simplify to:

use std::vec::Vec;

Or rely on the prelude and remove these imports entirely, since Vec is already in scope by default.

If this is an intentional intermediate state (per the PR description about hashes std dependency), consider adding a brief comment explaining why alloc::vec::Vec is still used.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@dash/src/ephemerealdata/instant_lock.rs` around lines 5 - 12, The conditional
imports of Vec in instant_lock.rs (the #[cfg(not(test)) use alloc::vec::Vec;]
and #[cfg(test)] pub use std::vec::Vec;) are unnecessary now that std is always
available; remove both conditional imports and replace with a single use
std::vec::Vec; (or remove them entirely and rely on the prelude) to simplify the
module imports—update any comment near the top of instant_lock.rs if you intend
to keep alloc for historical reasons and reference the Vec import removal in
that comment.
🤖 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-manager/Cargo.toml`:
- Line 21: The dashcore dependency in Cargo.toml currently pulls in its default
features (secp-recovery, bincode) and should be made explicit to match
key-wallet's pattern; update the dashcore entry to include default-features =
false so dashcore does not enable its defaults transitively (ensure the
dependency line referencing dashcore in Cargo.toml is changed to include
default-features = false while keeping the path = "../dash" unchanged).

---

Nitpick comments:
In `@dash/src/ephemerealdata/instant_lock.rs`:
- Around line 5-12: The conditional imports of Vec in instant_lock.rs (the
#[cfg(not(test)) use alloc::vec::Vec;] and #[cfg(test)] pub use std::vec::Vec;)
are unnecessary now that std is always available; remove both conditional
imports and replace with a single use std::vec::Vec; (or remove them entirely
and rely on the prelude) to simplify the module imports—update any comment near
the top of instant_lock.rs if you intend to keep alloc for historical reasons
and reference the Vec import removal in that comment.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2cb4bf0f-b3e3-4e6d-9768-ab06ab84c749

📥 Commits

Reviewing files that changed from the base of the PR and between 80bbdbb and 57e23a3.

📒 Files selected for processing (31)
  • dash-spv/Cargo.toml
  • dash/Cargo.toml
  • dash/src/address.rs
  • dash/src/amount.rs
  • dash/src/base58.rs
  • dash/src/bip152.rs
  • dash/src/bip158.rs
  • dash/src/blockdata/block.rs
  • dash/src/blockdata/locktime/absolute.rs
  • dash/src/blockdata/locktime/relative.rs
  • dash/src/blockdata/script/mod.rs
  • dash/src/blockdata/transaction/outpoint.rs
  • dash/src/consensus/encode.rs
  • dash/src/crypto/ecdsa.rs
  • dash/src/crypto/key.rs
  • dash/src/crypto/sighash.rs
  • dash/src/crypto/taproot.rs
  • dash/src/ephemerealdata/chain_lock.rs
  • dash/src/ephemerealdata/instant_lock.rs
  • dash/src/error.rs
  • dash/src/lib.rs
  • dash/src/merkle_tree/block.rs
  • dash/src/network/mod.rs
  • dash/src/pow.rs
  • dash/src/sign_message.rs
  • dash/src/string.rs
  • dash/src/taproot.rs
  • internals/src/error.rs
  • key-wallet-ffi/Cargo.toml
  • key-wallet-manager/Cargo.toml
  • rpc-json/Cargo.toml
💤 Files with no reviewable changes (22)
  • dash/src/address.rs
  • dash/src/base58.rs
  • dash/src/crypto/sighash.rs
  • dash/src/blockdata/locktime/absolute.rs
  • dash/src/ephemerealdata/chain_lock.rs
  • dash/src/blockdata/block.rs
  • dash/src/blockdata/script/mod.rs
  • dash/src/pow.rs
  • dash/src/bip152.rs
  • dash/src/string.rs
  • dash/src/crypto/taproot.rs
  • dash/src/consensus/encode.rs
  • dash/src/sign_message.rs
  • dash/src/blockdata/transaction/outpoint.rs
  • dash/src/bip158.rs
  • dash/src/error.rs
  • dash/src/taproot.rs
  • dash/src/merkle_tree/block.rs
  • dash/src/blockdata/locktime/relative.rs
  • dash/src/crypto/ecdsa.rs
  • dash/src/network/mod.rs
  • dash/src/amount.rs
🚧 Files skipped from review as they are similar to previous changes (4)
  • key-wallet-ffi/Cargo.toml
  • rpc-json/Cargo.toml
  • dash/src/crypto/key.rs
  • dash/Cargo.toml

Comment thread key-wallet-manager/Cargo.toml
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: 3

🧹 Nitpick comments (1)
rpc-json/Cargo.toml (1)

29-29: Avoid implicit default-feature coupling on dashcore.

This currently enables dashcore defaults implicitly (including bincode). Keep default-features = false and list only required features explicitly to prevent future feature drift.

🔧 Suggested manifest adjustment
-dashcore = { path = "../dash", features=["secp-recovery", "rand-std", "signer", "serde"] }
+dashcore = { path = "../dash", default-features = false, features=["secp-recovery", "rand-std", "signer", "serde"] }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@rpc-json/Cargo.toml` at line 29, The dashcore dependency currently pulls in
default features implicitly; update the Cargo.toml dashcore entry to set
default-features = false and explicitly enumerate only the required features
(e.g., "secp-recovery", "rand-std", "signer", "serde") so the dependency line
for dashcore disables defaults and lists the exact features to avoid unintended
feature drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@dash/Cargo.toml`:
- Line 22: The crate-level docs in dash/src/lib.rs still mention legacy
`std`/`no-std` feature flags which no longer match the manifest (see Cargo.toml
features like the default set with "secp-recovery" and "bincode"); update the
documentation block in dash/src/lib.rs to remove or replace any references to
`std` and `no-std` and instead describe the current feature model (e.g., listing
actual feature names such as "secp-recovery" and "bincode" or referring readers
to Cargo.toml), ensuring the crate doc comment (the top-level module doc ///! or
/*! */ section) accurately reflects the current feature flags and integration
guidance.
- Line 22: The change that alters default features in Cargo.toml (the line
setting default = ["secp-recovery", "bincode"]) drops no-std support and should
be treated as a breaking change; update the PR title and commit message to
include a BREAKING: marker (or add an explicit breaking note in the PR/release
notes) and ensure the changelog/release entry documents the removed no-std
support so downstream users are aware; also verify pr-title.yml workflow rules
align with this new classification.

In `@dash/src/lib.rs`:
- Line 124: The change unconditionally exposing std via `pub use std::io` breaks
no-std support but the crate docs and advertised feature flags (the doc comments
in lib.rs that describe std/no-std) and the PR title were not updated; update
the crate-level documentation in lib.rs to remove or clearly mark that no-std is
no longer supported, adjust the feature flag docs to reflect that `std` is now
required (or document the new support matrix), and change the PR title to
indicate a breaking change (e.g., prefix with "BREAKING:") so workflows and
downstream consumers are aware; ensure references to `pub use std::io` remain
and that any cargo feature declarations or README statements match this new
requirement.

---

Nitpick comments:
In `@rpc-json/Cargo.toml`:
- Line 29: The dashcore dependency currently pulls in default features
implicitly; update the Cargo.toml dashcore entry to set default-features = false
and explicitly enumerate only the required features (e.g., "secp-recovery",
"rand-std", "signer", "serde") so the dependency line for dashcore disables
defaults and lists the exact features to avoid unintended feature drift.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c8fcdef0-930f-47c2-80b0-b7d23427a117

📥 Commits

Reviewing files that changed from the base of the PR and between 57e23a3 and b191e23.

📒 Files selected for processing (32)
  • dash-spv/Cargo.toml
  • dash/Cargo.toml
  • dash/src/address.rs
  • dash/src/amount.rs
  • dash/src/base58.rs
  • dash/src/bip152.rs
  • dash/src/bip158.rs
  • dash/src/blockdata/block.rs
  • dash/src/blockdata/locktime/absolute.rs
  • dash/src/blockdata/locktime/relative.rs
  • dash/src/blockdata/script/mod.rs
  • dash/src/blockdata/transaction/outpoint.rs
  • dash/src/consensus/encode.rs
  • dash/src/consensus/serde.rs
  • dash/src/crypto/ecdsa.rs
  • dash/src/crypto/key.rs
  • dash/src/crypto/sighash.rs
  • dash/src/crypto/taproot.rs
  • dash/src/ephemerealdata/chain_lock.rs
  • dash/src/ephemerealdata/instant_lock.rs
  • dash/src/error.rs
  • dash/src/lib.rs
  • dash/src/merkle_tree/block.rs
  • dash/src/network/mod.rs
  • dash/src/pow.rs
  • dash/src/sign_message.rs
  • dash/src/string.rs
  • dash/src/taproot.rs
  • internals/src/error.rs
  • key-wallet-ffi/Cargo.toml
  • key-wallet-manager/Cargo.toml
  • rpc-json/Cargo.toml
💤 Files with no reviewable changes (22)
  • dash/src/bip152.rs
  • dash/src/address.rs
  • dash/src/merkle_tree/block.rs
  • dash/src/sign_message.rs
  • dash/src/crypto/taproot.rs
  • dash/src/consensus/serde.rs
  • dash/src/crypto/sighash.rs
  • dash/src/blockdata/locktime/relative.rs
  • dash/src/amount.rs
  • dash/src/error.rs
  • dash/src/taproot.rs
  • dash/src/blockdata/block.rs
  • dash/src/ephemerealdata/chain_lock.rs
  • dash/src/consensus/encode.rs
  • dash/src/base58.rs
  • dash/src/blockdata/locktime/absolute.rs
  • dash/src/string.rs
  • dash/src/crypto/ecdsa.rs
  • dash/src/pow.rs
  • dash/src/bip158.rs
  • dash/src/network/mod.rs
  • dash/src/blockdata/transaction/outpoint.rs
🚧 Files skipped from review as they are similar to previous changes (4)
  • dash/src/ephemerealdata/instant_lock.rs
  • dash-spv/Cargo.toml
  • internals/src/error.rs
  • key-wallet-manager/Cargo.toml

Comment thread dash/Cargo.toml
Comment thread dash/src/lib.rs
coderabbitai[bot]
coderabbitai Bot previously approved these changes Mar 18, 2026
@github-actions github-actions Bot added the ready-for-review CodeRabbit has approved this PR label Mar 18, 2026
@ZocoLini ZocoLini force-pushed the chore/drop-nostd-dashcore branch from b191e23 to b408a3e Compare March 18, 2026 21:36
@github-actions github-actions Bot removed the ready-for-review CodeRabbit has approved this PR label Mar 18, 2026
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.

♻️ Duplicate comments (1)
internals/src/error.rs (1)

14-15: ⚠️ Potential issue | 🟠 Major

Source error details are now silently discarded from error messages.

The write_err! macro now binds $source only to silence clippy but never includes it in the output. Multiple callers rely on this macro to format errors with their sources:

  • dash/src/parse.rs:62ParseIntError loses underlying core::num::ParseIntError details
  • dash/src/string.rs:57FromHexError::ParseHex loses the parsing error
  • dash/src/error.rs:26Error::Encode loses the encoding error details

Users will see messages like "failed to parse '...' as a... integer" without the underlying cause. Consider whether this loss of diagnostic context is acceptable, or if the source should be appended via std::error::Error::source() now that std is always available.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internals/src/error.rs` around lines 14 - 15, The write_err! macro is
currently binding $source only to silence clippy and not including it in the
output, which drops underlying error context (affecting callers like
ParseIntError in dash::parse, FromHexError::ParseHex, and Error::Encode); update
the write_err! macro so it appends the source error to the formatted message
instead of discarding it—e.g., format the primary message via write!($writer,
$string $(, $args)*) and then, if $source is present, write the source using
Display (or use std::error::Error::source() if appropriate) so the resulting
output includes the original error details (ensure $source is formatted safely
and no clippy warning remains).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@internals/src/error.rs`:
- Around line 14-15: The write_err! macro is currently binding $source only to
silence clippy and not including it in the output, which drops underlying error
context (affecting callers like ParseIntError in dash::parse,
FromHexError::ParseHex, and Error::Encode); update the write_err! macro so it
appends the source error to the formatted message instead of discarding it—e.g.,
format the primary message via write!($writer, $string $(, $args)*) and then, if
$source is present, write the source using Display (or use
std::error::Error::source() if appropriate) so the resulting output includes the
original error details (ensure $source is formatted safely and no clippy warning
remains).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4e4152d8-1de1-4165-b996-d1bdfb268618

📥 Commits

Reviewing files that changed from the base of the PR and between b191e23 and b408a3e.

📒 Files selected for processing (32)
  • dash-spv/Cargo.toml
  • dash/Cargo.toml
  • dash/src/address.rs
  • dash/src/amount.rs
  • dash/src/base58.rs
  • dash/src/bip152.rs
  • dash/src/bip158.rs
  • dash/src/blockdata/block.rs
  • dash/src/blockdata/locktime/absolute.rs
  • dash/src/blockdata/locktime/relative.rs
  • dash/src/blockdata/script/mod.rs
  • dash/src/blockdata/transaction/outpoint.rs
  • dash/src/consensus/encode.rs
  • dash/src/consensus/serde.rs
  • dash/src/crypto/ecdsa.rs
  • dash/src/crypto/key.rs
  • dash/src/crypto/sighash.rs
  • dash/src/crypto/taproot.rs
  • dash/src/ephemerealdata/chain_lock.rs
  • dash/src/ephemerealdata/instant_lock.rs
  • dash/src/error.rs
  • dash/src/lib.rs
  • dash/src/merkle_tree/block.rs
  • dash/src/network/mod.rs
  • dash/src/pow.rs
  • dash/src/sign_message.rs
  • dash/src/string.rs
  • dash/src/taproot.rs
  • internals/src/error.rs
  • key-wallet-ffi/Cargo.toml
  • key-wallet-manager/Cargo.toml
  • rpc-json/Cargo.toml
💤 Files with no reviewable changes (22)
  • dash/src/ephemerealdata/chain_lock.rs
  • dash/src/bip152.rs
  • dash/src/address.rs
  • dash/src/base58.rs
  • dash/src/blockdata/locktime/relative.rs
  • dash/src/crypto/sighash.rs
  • dash/src/error.rs
  • dash/src/bip158.rs
  • dash/src/string.rs
  • dash/src/consensus/serde.rs
  • dash/src/blockdata/transaction/outpoint.rs
  • dash/src/amount.rs
  • dash/src/taproot.rs
  • dash/src/blockdata/block.rs
  • dash/src/sign_message.rs
  • dash/src/crypto/ecdsa.rs
  • dash/src/merkle_tree/block.rs
  • dash/src/consensus/encode.rs
  • dash/src/blockdata/locktime/absolute.rs
  • dash/src/network/mod.rs
  • dash/src/crypto/taproot.rs
  • dash/src/pow.rs
🚧 Files skipped from review as they are similar to previous changes (5)
  • dash/src/blockdata/script/mod.rs
  • dash-spv/Cargo.toml
  • key-wallet-ffi/Cargo.toml
  • dash/src/lib.rs
  • dash/src/ephemerealdata/instant_lock.rs

@xdustinface xdustinface merged commit b144e22 into v0.42-dev Mar 19, 2026
43 of 44 checks passed
@xdustinface xdustinface deleted the chore/drop-nostd-dashcore branch March 19, 2026 02:30
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.

2 participants