Skip to content

Conversation

@xdustinface
Copy link
Collaborator

@xdustinface xdustinface commented Dec 27, 2025

Uses fully qualified syntax <T as $crate::Hash>::method() instead of relying on the Hash trait being in scope. This ensures hash_newtype! macros compile correctly when used with --no-default-features in CI overhaul PR #253.

Summary by CodeRabbit

  • Refactor
    • Internal code improvements to hash implementations without any changes to user-facing functionality or behavior.

✏️ Tip: You can customize this high-level summary in your review settings.

Uses fully qualified syntax `<T as $crate::Hash>::method()` instead of relying on the `Hash` trait being in scope. This ensures `hash_newtype!` macros compile correctly when used with `--no-default-features` in CI overhaul PR #253.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 27, 2025

📝 Walkthrough

Walkthrough

The hashes/src/util.rs file has been refactored to resolve method calls through explicit trait paths. Methods in the hash_newtype and hash_newtype_no_ord macros now use qualified syntax like <$hash as $crate::Hash>::method_name() instead of direct calls on inner types. The observable behavior is unchanged; this is internal path resolution improvement.

Changes

Cohort / File(s) Change Summary
Trait-qualified method resolution
hashes/src/util.rs
Refactored reverse, to_byte_array, as_byte_array, all_zeros, from_str, and From<[u8; LEN]> implementations to resolve method calls through explicit Hash trait paths using qualified syntax (e.g., <$hash as $crate::Hash>::to_byte_array()) instead of direct inner type calls within macros

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A macr­o's dance through trait-defined ways,
Paths now explicit, crystal and clear,
No new logic, just tidier days,
Where Hash trait guides each method's career! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main change: qualifying Hash trait calls in macros to support --no-default-features compilation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/hash-trait-macro

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3884314 and 04fab01.

📒 Files selected for processing (1)
  • hashes/src/util.rs
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{rs,toml}

📄 CodeRabbit inference engine (CLAUDE.md)

Never hardcode network parameters, addresses, or keys

Files:

  • hashes/src/util.rs
**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Use proper error types (thiserror) and propagate errors appropriately
Use tokio runtime for async operations
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality
Use secure random number generation for keys
Never log or expose private keys
Code must target Rust 1.89 minimum supported version (MSRV)
Format code using cargo fmt
Pass clippy linting without warnings

**/*.rs: MSRV (Minimum Supported Rust Version) is 1.89; ensure compatibility with this version for all builds
Unit tests should live alongside code with #[cfg(test)] annotation; integration tests use the tests/ directory
Use snake_case for function and variable names
Use UpperCamelCase for types and traits
Use SCREAMING_SNAKE_CASE for constants
Format code with rustfmt before commits; ensure cargo fmt --all is run
Run cargo clippy --workspace --all-targets -- -D warnings for linting; avoid warnings in CI
Prefer async/await via tokio for asynchronous operations

Files:

  • hashes/src/util.rs
🧠 Learnings (3)
📓 Common learnings
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 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.
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Use `#[no_mangle] extern "C"` attribute when implementing new FFI functions in Rust
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Applies to src/**/*.rs : Avoid `unwrap()` and `expect()` in library code; use proper error types (e.g., `thiserror`)
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Use `BTreeMap` for ordered data (accounts, transactions) and `HashMap` for lookups (address mappings); apply memory management strategies for old transaction data
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Applies to **/*.rs : Use `UpperCamelCase` for types and traits
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/**/*.rs : Use Arc<dyn TraitName> for trait objects to enable runtime polymorphism for NetworkManager and StorageManager
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Use `BTreeMap` for ordered data (accounts, transactions) and `HashMap` for lookups (address mappings); apply memory management strategies for old transaction data

Applied to files:

  • hashes/src/util.rs
📚 Learning: 2025-02-25T06:13:52.858Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 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/util.rs
🧬 Code graph analysis (1)
hashes/src/util.rs (1)
hashes/src/lib.rs (4)
  • to_byte_array (221-221)
  • from_byte_array (227-227)
  • as_byte_array (224-224)
  • all_zeros (234-234)
⏰ 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). (16)
  • GitHub Check: fuzz (hashes_sha512)
  • GitHub Check: fuzz (dash_outpoint_string)
  • GitHub Check: fuzz (hashes_sha1)
  • GitHub Check: fuzz (hashes_json)
  • GitHub Check: fuzz (hashes_sha256)
  • GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
  • GitHub Check: fuzz (dash_deserialize_block)
  • GitHub Check: fuzz (dash_deserialize_witness)
  • GitHub Check: fuzz (dash_deserialize_script)
  • GitHub Check: fuzz (dash_deserialize_address)
  • GitHub Check: Key Wallet Components Tests
  • GitHub Check: Pre-commit (ubuntu-latest)
  • GitHub Check: RPC Tests (stable, true)
  • GitHub Check: SPV Components Tests
  • GitHub Check: Core Components Tests
  • GitHub Check: Pre-commit (macos-latest)
🔇 Additional comments (2)
hashes/src/util.rs (2)

412-412: LGTM! Consistent with hash_newtype macro changes.

The hash_newtype_no_ord macro changes mirror those in hash_newtype, maintaining consistency across both macros. All trait method calls correctly use fully qualified syntax.

Also applies to: 417-417, 422-422, 432-437, 457-458


232-234: Fully qualified trait syntax correctly applied for --no-default-features compatibility.

The changes in both hash_newtype and hash_newtype_no_ord macros consistently use fully qualified syntax (<$hash as $crate::Hash>::method()) for all trait method calls. This ensures the macros work correctly even when the Hash trait is not in scope, which is essential for builds with --no-default-features.


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.

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