chore(hashes): drop no-std supp for hashes#520
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThe changes systematically refactor feature flags across multiple crates by removing Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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.
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 `@hashes/Cargo.toml`:
- Around line 15-18: The workspace removed no-std support causing dashcore's
no-std builds to fail because hashes' hex.rs and impls.rs use std::io and
std::error::Error unconditionally; restore a no-std-compatible contract by
either re-introducing a "std" feature in hashes' Cargo.toml (e.g., add a feature
like std = ["alloc/std"] or similar) and gate std-only imports behind
cfg(feature = "std") in hex.rs and impls.rs, or keep the removed no-std surface
but then update dash/Cargo.toml/dash/src/lib.rs to drop the no-std feature; if
you choose to keep no-std support, replace uses of std::io and std::error::Error
with cfg-gated alternatives and use core2 equivalents where dash/src/lib.rs
expects core2 compatibility so no-std builds of dashcore can compile against
hashes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c04f5d5a-3de8-43d4-bfa6-1d80f0a0f5c1
📒 Files selected for processing (18)
dash/Cargo.tomlhashes/Cargo.tomlhashes/src/error.rshashes/src/hash160.rshashes/src/hash_x11.rshashes/src/hex.rshashes/src/hmac.rshashes/src/impls.rshashes/src/lib.rshashes/src/ripemd160.rshashes/src/sha1.rshashes/src/sha256.rshashes/src/sha256d.rshashes/src/sha256t.rshashes/src/sha512.rshashes/src/sha512_256.rskey-wallet-manager/Cargo.tomlkey-wallet/Cargo.toml
💤 Files with no reviewable changes (14)
- hashes/src/sha256.rs
- hashes/src/error.rs
- hashes/src/sha512_256.rs
- hashes/src/sha256d.rs
- hashes/src/hmac.rs
- hashes/src/ripemd160.rs
- hashes/src/sha512.rs
- hashes/src/sha256t.rs
- hashes/src/sha1.rs
- hashes/src/lib.rs
- hashes/src/hash160.rs
- hashes/src/hash_x11.rs
- hashes/src/hex.rs
- hashes/src/impls.rs
Comment on lines
15
to
18
| [features] | ||
| default = ["std"] | ||
| std = ["alloc", "internals/std"] | ||
| alloc = ["internals/alloc"] | ||
| default = [] | ||
| schemars = ["actual-schemars"] | ||
| serde-std = ["serde/std"] |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== hashes feature surface ==\n'
rg -n -C2 '^\[features\]|^default\s*=|^std\s*=|^alloc\s*=' hashes/Cargo.toml
printf '\n== dash no-std surface ==\n'
rg -n -C2 'cfg\(not\(feature = "std"\)\)|core2::io|core2::error|no-std\s*=' dash/Cargo.toml dash/src/lib.rs
printf '\n== std-only APIs still present in hashes ==\n'
rg -n -C2 'use std::io|impl std::error::Error|impl.*io::Write|impl.*io::Read' \
hashes/src/impls.rs hashes/src/hex.rsRepository: dashpay/rust-dashcore
Length of output: 4053
Coordinate the no-std removal with downstream feature contracts.
hashes now lacks std/alloc features but uses std::io and std::error::Error unconditionally in hex.rs and impls.rs. Meanwhile, dashcore still exposes a no-std feature (line 44 of dash/Cargo.toml) with no-std code paths in dash/src/lib.rs using core2. This creates a broken contract: no-std builds of dashcore will fail when compiling its hashes dependency.
Either remove the workspace no-std surface or keep hashes no-std compatible (e.g., gate std::io imports with cfg features and use core2 equivalents for no-std paths).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hashes/Cargo.toml` around lines 15 - 18, The workspace removed no-std support
causing dashcore's no-std builds to fail because hashes' hex.rs and impls.rs use
std::io and std::error::Error unconditionally; restore a no-std-compatible
contract by either re-introducing a "std" feature in hashes' Cargo.toml (e.g.,
add a feature like std = ["alloc/std"] or similar) and gate std-only imports
behind cfg(feature = "std") in hex.rs and impls.rs, or keep the removed no-std
surface but then update dash/Cargo.toml/dash/src/lib.rs to drop the no-std
feature; if you choose to keep no-std support, replace uses of std::io and
std::error::Error with cfg-gated alternatives and use core2 equivalents where
dash/src/lib.rs expects core2 compatibility so no-std builds of dashcore can
compile against hashes.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v0.42-dev #520 +/- ##
=============================================
- Coverage 66.82% 66.76% -0.06%
=============================================
Files 313 313
Lines 64756 64746 -10
=============================================
- Hits 43272 43230 -42
- Misses 21484 21516 +32
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
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 by CodeRabbit
Refactor
Tests