fix: core component tests #122
Conversation
WalkthroughThe PR adjusts Cargo metadata (Rust edition) and reorders/imports across multiple files, plus a minor macro formatting change and a schemars path update. No functional logic, control flow, or public API changes are introduced. Most edits are cosmetic in imports and tests. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
hashes/src/siphash24.rs (1)
48-50: Avoid multiple evaluation of $state in the macro.Using $state:expr expands the expression 4x (v0..v3). Restrict to an identifier to prevent side effects and keep intent explicit.
-macro_rules! compress { - ($state:expr) => {{ - compress!($state.v0, $state.v1, $state.v2, $state.v3) - }}; +macro_rules! compress { + ($state:ident) => { + compress!($state.v0, $state.v1, $state.v2, $state.v3) + };hashes/src/hmac.rs (1)
52-54: Typo in doc comment (“underyling” → “underlying”).Minor polish for docs.
Apply:
-/// Pair of underyling hash engines, used for the inner and outer hash of HMAC. +/// Pair of underlying hash engines, used for the inner and outer hash of HMAC.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (17)
hashes/Cargo.toml(1 hunks)hashes/benches/hashes.rs(1 hunks)hashes/src/hash160.rs(3 hunks)hashes/src/hash_x11.rs(1 hunks)hashes/src/hmac.rs(2 hunks)hashes/src/impls.rs(2 hunks)hashes/src/lib.rs(1 hunks)hashes/src/ripemd160.rs(2 hunks)hashes/src/serde_macros.rs(1 hunks)hashes/src/sha1.rs(2 hunks)hashes/src/sha256.rs(3 hunks)hashes/src/sha256d.rs(3 hunks)hashes/src/sha256t.rs(1 hunks)hashes/src/sha512.rs(2 hunks)hashes/src/sha512_256.rs(2 hunks)hashes/src/siphash24.rs(1 hunks)hashes/src/util.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
Files:
hashes/src/lib.rshashes/src/sha256.rshashes/src/impls.rshashes/src/hash160.rshashes/src/ripemd160.rshashes/src/sha256d.rshashes/src/sha512.rshashes/src/sha256t.rshashes/src/serde_macros.rshashes/src/hash_x11.rshashes/src/hmac.rshashes/src/siphash24.rshashes/src/util.rshashes/src/sha1.rshashes/src/sha512_256.rshashes/benches/hashes.rs
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.082Z
Learning: Applies to key-wallet/tests/**/*.rs : Keep test vectors in sync with Dash Core releases
📚 Learning: 2025-08-29T04:01:18.082Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.082Z
Learning: Applies to key-wallet/tests/bip32_tests.rs : Organize unit tests so key-derivation correctness lives in tests/bip32_tests.rs
Applied to files:
hashes/src/lib.rshashes/src/impls.rs
📚 Learning: 2025-02-25T06:13:52.858Z
Learnt from: QuantumExplorer
PR: dashpay/rust-dashcore#51
File: dash/src/sml/masternode_list/scores_for_quorum.rs:50-73
Timestamp: 2025-02-25T06:13:52.858Z
Learning: ScoreHash is a cryptographic hash in the rust-dashcore library, and therefore does not need collision handling when used as a key in collections like BTreeMap due to the extremely low probability of collisions.
Applied to files:
hashes/src/sha256.rs
📚 Learning: 2025-08-29T04:01:18.082Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.082Z
Learning: Applies to key-wallet/tests/**/*.rs : Use deterministic testing with known vectors (fixed seeds) in Rust tests
Applied to files:
hashes/src/impls.rs
📚 Learning: 2025-08-29T04:01:18.082Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.082Z
Learning: Applies to key-wallet/tests/**/*.rs : Use property-based tests for complex invariants where suitable
Applied to files:
hashes/src/impls.rs
📚 Learning: 2025-08-29T04:01:18.082Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.082Z
Learning: Applies to key-wallet/tests/address_tests.rs : Place address generation and validation tests in tests/address_tests.rs
Applied to files:
hashes/src/impls.rs
📚 Learning: 2025-08-29T04:01:18.082Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.082Z
Learning: Applies to key-wallet/tests/**/*.rs : Include integration tests that cover full wallet workflows
Applied to files:
hashes/src/impls.rs
📚 Learning: 2025-08-29T04:01:18.082Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.082Z
Learning: Applies to key-wallet/**/Cargo.toml : Set MSRV via rust-version = "1.89" in Cargo.toml
Applied to files:
hashes/Cargo.toml
🧬 Code graph analysis (2)
hashes/src/hash_x11.rs (1)
dash/src/merkle_tree/block.rs (2)
hex(707-707)hex(741-741)
hashes/benches/hashes.rs (1)
hashes/src/impls.rs (1)
hmac(166-187)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: Clippy (Non-strict)
- GitHub Check: Strict Warnings and Clippy Checks
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Core Components Tests
- GitHub Check: SPV Components Tests
🔇 Additional comments (32)
hashes/Cargo.toml (1)
12-12: Lock MSRV to 1.89 and fix description typo[package] name = "dashcore_hashes" version = { workspace = true} authors = ["Samuel Westrich <sam@dash.org>"] license = "CC0-1.0" repository = "https://github.com/rust-dashcore/dash_hashes/" documentation = "https://docs.rs/dash_hashes/" -description = "Hash functions used by the rust-dash eccosystem" +description = "Hash functions used by the rust-dash ecosystem" categories = ["algorithms"] keywords = [ "crypto", "dash", "hash", "digest" ] readme = "README.md" edition = "2021" +rust-version = "1.89" exclude = ["tests", "contrib"]hashes/src/util.rs (1)
586-589: Approve schemars import updatesNo deprecated schemars import paths remain.
hashes/src/ripemd160.rs (1)
412-412: Import reorders in tests look good.Purely cosmetic; no behavioral impact.
Also applies to: 505-505, 508-508
hashes/src/serde_macros.rs (1)
27-27: Serde import reshuffle is correct.Bringing serde::de into scope as de while keeping Serializer/Deserializer is idiomatic and maintains behavior.
hashes/src/hash160.rs (3)
27-27: LGTM! Import reordering improves consistency.This alphabetical reordering of imports aligns with Rust conventions and maintains consistency across the codebase.
54-54: LGTM! Import reordering improves consistency.The alphabetical ordering of imports is consistent with the pattern applied across other files in this PR.
108-110: LGTM! Import reordering improves consistency.The test imports are now properly ordered alphabetically, maintaining consistency with the broader refactoring effort.
hashes/src/lib.rs (1)
239-239: LGTM! Import reordering improves consistency.The alphabetical ordering of test imports aligns with the broader import organization effort across the codebase.
hashes/src/hash_x11.rs (1)
34-34: LGTM! Import reordering improves consistency.The alphabetical reordering of crate imports maintains consistency with the broader import organization effort across the hashes crate.
hashes/src/impls.rs (2)
26-26: LGTM! Import reordering improves consistency.Moving
HashEngineto the end of the import list follows alphabetical ordering conventions and aligns with the broader refactoring effort.
97-97: LGTM! Import reordering improves consistency.The alphabetical ordering of test imports maintains consistency with the import organization pattern applied throughout this PR.
hashes/src/sha256t.rs (1)
23-23: LGTM! Import reordering improves consistency.The alphabetical reordering of imports (
sha256beforeError) aligns with Rust conventions and the broader import organization effort across the hashes crate.hashes/src/sha512_256.rs (2)
32-32: Import reordering is fine.No functional impact; consistent with the PR’s cosmetic cleanup.
93-93: Test import reordering is fine.Purely cosmetic; no behavior change.
hashes/src/sha512.rs (3)
276-276: Test import reordering is fine.No behavioral changes.
354-354: serde_test import order tweak is fine.No effect on test behavior.
356-356: Crate import reordering is fine.Matches the PR’s consistent style change.
hashes/src/sha1.rs (3)
149-149: Test import reordering is fine.No functional change.
217-217: serde_test import order tweak is fine.No effect on tests.
219-219: Crate import reordering is fine.Cosmetic only; OK.
hashes/src/sha256.rs (3)
23-23: Top-level import reordering is fine.No logic changes; retains trait import for HashEngine methods.
520-520: Test import reordering is fine.Purely cosmetic.
698-698: serde_test import order tweak is fine.No behavior change.
hashes/benches/hashes.rs (2)
1-1: criterion import reordering is fine.No impact on benchmarks.
3-3: Manually verify Clippy on MSRV after trimming unused imports
After applying in hashes/benches/hashes.rs:-use dashcore_hashes::{hmac, siphash24, Hash, HashEngine}; +use dashcore_hashes::{hmac, siphash24};Manually run:
rustup toolchain install 1.89.0 --quiet rustup override set 1.89.0 cargo fmt --all --check cargo clippy --workspace --all-targets -- -D warningsand confirm no unused-import warnings remain.
hashes/src/hmac.rs (3)
375-375: Reordered serde_test imports look good.No functional impact; maintains assert_tokens/Configure/Token availability.
377-377: Crate import reorder in serde test is fine.Keeps sha512 in scope where required; no behavior change.
249-249: Import reorder in tests is cosmetic; local verification required
Reordering imports is purely aesthetic and keepssha256in scope. The sandbox couldn’t runcargo fmt, Clippy, or tests due to environment restrictions—please confirm on your machine thatcargo fmt --all -- --check RUSTFLAGS="-Dwarnings" cargo +1.89.0 clippy --all-targets --all-features -D warnings cargo +1.89.0 test -p hashes --features alloc,serdeall pass without warnings or failures.
hashes/src/sha256d.rs (4)
22-22: Top-level import reorder is OK.Cosmetic only; keeps module and type imports available.
49-49: Test import reorder is OK.No semantic change; matches style used elsewhere in the PR.
100-100: serde_test import reorder is OK.Purely stylistic; asserts remain unchanged.
102-102: Crate import reorder in serde test is OK.No behavioral impact.
QuantumExplorer
changed the title
QuantumExplorer
changed the title
Summary by CodeRabbit