Skip to content

refactor: clean dash core and hashes for warnings and clippy #109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
QuantumExplorer merged 16 commits into from
Aug 21, 2025
Merged

refactor: clean dash core and hashes for warnings and clippy #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
f03f52a
clean up
QuantumExplorer Aug 21, 2025
cd85a28
fixed warnings
QuantumExplorer Aug 21, 2025
bb5a2c3
more fixes
QuantumExplorer Aug 21, 2025
c4ad177
fixed clippy warnings
QuantumExplorer Aug 21, 2025
e223e26
last warnings
QuantumExplorer Aug 21, 2025
8fe78aa
added dashcore and dashcore hashes to strict checking
QuantumExplorer Aug 21, 2025
1a53abd
fixes
QuantumExplorer Aug 21, 2025
538e825
fixes
QuantumExplorer Aug 21, 2025
97be886
fixes
QuantumExplorer Aug 21, 2025
c32c354
fixes
QuantumExplorer Aug 21, 2025
4f15119
more cleanup
QuantumExplorer Aug 21, 2025
01514c9
cleanup
QuantumExplorer Aug 21, 2025
599b9ed
more work
QuantumExplorer Aug 21, 2025
76fdbd2
more work
QuantumExplorer Aug 21, 2025
38b01e7
more work
QuantumExplorer Aug 21, 2025
f56d9e1
work
QuantumExplorer Aug 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/strict-checks.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
"strict_check_crates": [
"key-wallet",
"key-wallet-manager",
"key-wallet-ffi"
"key-wallet-ffi",
"dash",
"hashes"
],
Comment on lines 2 to 8

@coderabbitai coderabbitai Bot Aug 21, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Validate crate names match Cargo package names.

The workflow currently refers to “dashcore_hashes” and “dashcore”, but this file lists “dash” and “hashes”. If package names don’t match exactly, the CI gating will silently misbehave.

Run:

If the actual packages are “dashcore”/“dashcore-hashes” (or similar), update either this JSON or the workflow accordingly.

🏁 Script executed:

#!/bin/bash
set -euo pipefail
# List package names from cargo metadata
cargo metadata --no-deps --format-version=1 | jq -r '.packages[].name' | sort -u | tee /tmp/all_pkgs.txt
echo "strict_check_crates from JSON:"
jq -r '.strict_check_crates[]' .github/strict-checks.json | tee /tmp/strict_pkgs.txt
echo "Missing in workspace (JSON -> Cargo):"
comm -23 <(sort /tmp/strict_pkgs.txt) <(sort /tmp/all_pkgs.txt) || true
echo "Present in workspace but not in JSON (Cargo -> JSON):"
comm -13 <(sort /tmp/strict_pkgs.txt) <(sort /tmp/all_pkgs.txt) || true

Length of output: 330

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Extract package names from all Cargo.toml files
find . -name Cargo.toml -maxdepth 3 | while read -r manifest; do
  # Only consider the [package] section's name field
  awk '/^\[package\]/{p=1} p && /^name[[:space:]]*=/{gsub(/"/,"",$3); print $3; exit}' "$manifest"
done | sort -u > all_pkgs.txt

# Read the strict_check_crates list
jq -r '.strict_check_crates[]' .github/strict-checks.json | sort -u > strict_pkgs.txt

echo "Missing in workspace (JSON → Cargo):"
comm -23 strict_pkgs.txt all_pkgs.txt || true

echo "Present in workspace but not in JSON (Cargo → JSON):"
comm -13 strict_pkgs.txt all_pkgs.txt || true

Length of output: 3127

Update strict_check_crates to match actual Cargo package names

The entries in .github/strict-checks.json for “dash” and “hashes” don’t correspond to any Cargo packages in the workspace. According to your Cargo.toml manifests, the actual crate names are “dashcore” and “dashcore_hashes”.

Please update the JSON accordingly:

--- .github/strict-checks.json
+++ .github/strict-checks.json
@@ strict_check_crates
-    "dash",
-    "hashes"
+    "dashcore",
+    "dashcore_hashes"

Locations to verify:

  • File: .github/strict-checks.json
  • Lines: where “dash” and “hashes” are listed

After this change, re-run the CI to ensure the strict-check workflow picks up both crates correctly.

📝 Committable suggestion

‼️ 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.

Suggested change
"strict_check_crates": [
"key-wallet",
"key-wallet-manager",
"key-wallet-ffi"
"key-wallet-ffi",
"dash",
"hashes"
],
"strict_check_crates": [
"key-wallet",
"key-wallet-manager",
"key-wallet-ffi",
"dashcore",
"dashcore_hashes"
],
🤖 Prompt for AI Agents 
In .github/strict-checks.json around lines 2 to 8, the strict_check_crates array
contains incorrect crate names ("dash" and "hashes"); update those entries to
the actual Cargo package names used in the workspace by replacing "dash" with
"dashcore" and "hashes" with "dashcore_hashes", save the file, and re-run CI to
verify the strict-check workflow detects both crates.

Comment on lines +5 to 8

@coderabbitai coderabbitai Bot Aug 21, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Moving dash and hashes into strict checks is good, but CI doesn’t consume this file.

Right now, rust.yml hard-codes crate lists and never reads this JSON. These additions won’t enforce anything unless the workflow parses this file.

Wire rust.yml to read strict_check_crates from this JSON (see suggested changes in rust.yml comment). This avoids drift and ensures “dash” and “hashes” are actually denied on warnings.

"excluded_crates": [
"dash",
"dash-network",
"dash-network-ffi",
"hashes",
"internals",
"fuzz",
"rpc-client",
Expand Down
251 changes: 103 additions & 148 deletions .github/workflows/rust.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
on: [push, pull_request]
on:
push:
branches:
- master
- 'v**-dev'
pull_request:

name: Continuous integration

Expand All @@ -10,65 +15,58 @@ concurrency:
cancel-in-progress: true

jobs:
Tests:
name: Tests
spv_tests:
name: SPV Components Tests
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
include:
- rust: stable
env:
DO_COV: true
AS_DEPENDENCY: true
DO_NO_STD: false
- rust: beta
env:
AS_DEPENDENCY: true
DO_NO_STD: false
- rust: nightly
env:
# TODO: running with DO_BENCH: true causes the test to fail
# DO_BENCH: true
AS_DEPENDENCY: true
DO_NO_STD: false
DO_DOCS: true
- rust: 1.89.0
env:
AS_DEPENDENCY: true
steps:
- name: Checkout Crate
uses: actions/checkout@v4
- name: Setup Rust toolchain
uses: dtolnay/rust-toolchain@stable
with:
toolchain: ${{ matrix.rust }}
- name: Running test script
env: ${{ matrix.env }}
run: ./contrib/test.sh
- name: Cache cargo dependencies
uses: Swatinem/rust-cache@v2
- name: Test dash-spv crate
run: cargo test -p dash-spv --all-features
- name: Test dash-spv-ffi crate
run: cargo test -p dash-spv-ffi --all-features

workspace-tests:
name: Workspace Tests
key_wallet_tests:
name: Key Wallet Components Tests
runs-on: ubuntu-latest
steps:
- name: Checkout Crate
uses: actions/checkout@v4
- name: Setup Rust toolchain
uses: dtolnay/rust-toolchain@stable
- name: Cache cargo dependencies
uses: Swatinem/rust-cache@v2
- name: Test key-wallet crate
run: cargo test -p key-wallet --all-features
- name: Test key-wallet-manager crate
run: cargo test -p key-wallet-manager --all-features
- name: Test key-wallet-ffi crate
run: cargo test -p key-wallet-ffi --all-features

core_components_tests:
name: Core Components Tests
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
rust: [stable, beta, nightly, 1.89.0]
steps:
- name: Checkout Crate
uses: actions/checkout@v4
- name: Setup Rust toolchain
uses: dtolnay/rust-toolchain@stable
with:
toolchain: ${{ matrix.rust }}
- name: Cache cargo dependencies
uses: Swatinem/rust-cache@v2
- name: Run workspace tests
run: cargo test --workspace --all-features
- name: Run workspace tests (no default features)
run: cargo test --workspace --no-default-features
- name: Build workspace (release mode)
run: cargo build --workspace --release
- name: Test dashcore crate
run: cargo test -p dashcore --all-features
- name: Test dashcore_hashes crate
run: cargo test -p dashcore_hashes --all-features
- name: Test dash-network crate
run: cargo test -p dash-network --all-features
- name: Test internals crate
run: cargo test -p dashcore-private --all-features
- name: Run script-based tests
run: ./contrib/test.sh

clippy:
name: Clippy (Non-strict)
Expand All @@ -79,19 +77,18 @@ jobs:
- name: Setup Rust toolchain
uses: dtolnay/rust-toolchain@stable
with:
toolchain: stable
components: clippy
- name: Run clippy (excluding strict-checked crates)
run: |
# Auto-discover all workspace crates and exclude strict-checked ones
STRICT_CRATES=("key-wallet" "key-wallet-manager" "key-wallet-ffi")
STRICT_CRATES=("key-wallet" "key-wallet-manager" "key-wallet-ffi" "dashcore_hashes" "dashcore")
mapfile -t ALL_CRATES < <(cargo metadata --no-deps --format-version=1 | jq -r '.packages[].name' | sort -u)
for crate in "${ALL_CRATES[@]}"; do
if printf '%s\n' "${STRICT_CRATES[@]}" | grep -qx "$crate"; then
continue
fi
echo "Checking $crate (warnings allowed)..."
cargo clippy -p "$crate" --all-features --all-targets || true
echo "Checking $crate (warnings allowed, errors will fail)..."
cargo clippy -p "$crate" --all-features --all-targets -- -W warnings
done

strict-checks:
Expand All @@ -103,7 +100,6 @@ jobs:
- name: Setup Rust toolchain
uses: dtolnay/rust-toolchain@stable
with:
toolchain: stable
components: clippy
- name: Cache cargo dependencies
uses: Swatinem/rust-cache@v2
Expand Down Expand Up @@ -143,6 +139,30 @@ jobs:

- name: Clippy key-wallet-ffi (deny all warnings)
run: cargo clippy -p key-wallet-ffi --all-features --lib --bins --tests -- -D warnings

# Check dashcore with strict warnings
- name: Check dashcore (deny warnings)
env:
RUSTFLAGS: "-D warnings"
run: |
cargo check -p dashcore --all-features --lib --bins --tests
cargo build -p dashcore --all-features --lib --bins
cargo test -p dashcore --all-features --lib --bins

- name: Clippy dashcore (deny all warnings)
run: cargo clippy -p dashcore --all-features --lib --bins --tests -- -D warnings

# Check dashcore_hashes with strict warnings
- name: Check dashcore_hashes (deny warnings)
env:
RUSTFLAGS: "-D warnings"
run: |
cargo check -p dashcore_hashes --all-features --lib --bins --tests
cargo build -p dashcore_hashes --all-features --lib --bins
cargo test -p dashcore_hashes --all-features --lib --bins

- name: Clippy dashcore_hashes (deny all warnings)
run: cargo clippy -p dashcore_hashes --all-features --lib --bins --tests -- -D warnings

fmt:
name: Format
Expand All @@ -153,106 +173,41 @@ jobs:
- name: Setup Rust toolchain
uses: dtolnay/rust-toolchain@stable
with:
toolchain: stable
components: rustfmt
- name: Check formatting
run: cargo fmt --all -- --check

# TODO: need to support compiling rust-x11-hash under s390x
# Cross:
# name: Cross testing
# if: ${{ !github.event.act }}
# runs-on: ubuntu-latest
# steps:
# - name: Checkout Crate
# uses: actions/checkout@v4
# - name: Checkout Toolchain
# uses: dtolnay/rust-toolchain@stable
# with:
# profile: minimal
# toolchain: stable
# override: true
# - name: Install target
# run: rustup target add s390x-unknown-linux-gnu
# - name: install cross
# run: cargo install cross
# - name: run cross test
# run: cross test --target s390x-unknown-linux-gnu

# TODO: need to fix the ability to run the embedded tests
# Embedded:
# runs-on: ubuntu-latest
# env:
# RUSTFLAGS: "-C link-arg=-Tlink.x"
# CARGO_TARGET_THUMBV7M_NONE_EABI_RUNNER: "qemu-system-arm -cpu cortex-m3 -machine mps2-an385 -nographic -semihosting-config enable=on,target=native -kernel"
# steps:
# - name: Checkout
# uses: actions/checkout@v3
# - name: Set up QEMU
# run: sudo apt update && sudo apt install -y qemu-system-arm gcc-arm-none-eabi
# - name: Checkout Toolchain
# uses: dtolnay/rust-toolchain@nightly
# with:
# targets: thumbv7m-none-eabi
# - name: Install src
# run: rustup component add rust-src
# - name: Run dash/embedded
# run: cd dash/embedded && cargo run --target thumbv7m-none-eabi
# - name: Run hashes/embedded no alloc
# run: cd hashes/embedded && cargo run --target thumbv7m-none-eabi
# - name: Run hashes/embedded with alloc
# run: cd hashes/embedded && cargo run --target thumbv7m-none-eabi --features=alloc

rpc-tests:
name: RPC Tests
runs-on: ubuntu-latest
strategy:
matrix:
include:
- rust: stable
env:
RUSTFMTCHK: true
- rust: nightly
env:
RUSTFMTCHK: false
- rust: 1.89.0
env:
PIN_VERSIONS: true
steps:
- name: Checkout Crate
uses: actions/checkout@v4
- name: Setup Rust toolchain
uses: dtolnay/rust-toolchain@stable
with:
toolchain: ${{ matrix.rust }}
- name: Running test script
env: ${{ matrix.env }}
run: ./contrib/test.sh
rpc_tests:
name: RPC Tests
runs-on: ubuntu-latest
strategy:
matrix:
include:
- rust: stable
env:
PIN_VERSIONS: true
steps:
- name: Checkout Crate
uses: actions/checkout@v4
- name: Setup Rust toolchain
uses: dtolnay/rust-toolchain@stable
- name: Running test script
env: ${{ matrix.env }}
run: ./contrib/test.sh

integrations-tests:
name: Integration Tests
runs-on: ubuntu-latest
strategy:
matrix:
rust: [stable]
bitcoinversion:
[
"0.18.0",
"0.18.1",
"0.19.0.1",
"0.19.1",
"0.20.0",
"0.20.1",
"0.21.0",
]
steps:
- name: Checkout Crate
uses: actions/checkout@v4
- name: Setup Rust toolchain
uses: dtolnay/rust-toolchain@stable
with:
toolchain: ${{ matrix.rust }}
- name: Running test script
env:
BITCOINVERSION: ${{ matrix.bitcoinversion }}
run: ./contrib/test-rpc.sh
integrations_tests:
name: Integration Tests
runs-on: ubuntu-latest
strategy:
matrix:
rust: [stable]
dashversion: ["22.0.0", "22.1.3"]
steps:
- name: Checkout Crate
uses: actions/checkout@v4
- name: Setup Rust toolchain
uses: dtolnay/rust-toolchain@stable
- name: Running test script
env:
DASHVERSION: ${{ matrix.dashversion }}
run: ./contrib/test-rpc.sh
12 changes: 10 additions & 2 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@ resolver = "2"
[workspace.package]
version = "0.39.6"

[patch.crates-io.dashcore_hashes]
path = "hashes"
[patch.crates-io]
dashcore_hashes = { path = "hashes" }
# Use fixed version of elliptic-curve-tools with DefaultIsZeroes trait bound
elliptic-curve-tools = { git = "https://github.com/QuantumExplorer/elliptic-curve-tools", branch = "fix/DefaultIsZeroesToSumOfProducts" }

[profile.release]
# Default to unwinding for most crates

[profile.dev]
# Default to unwinding for most crates

Loading
Loading