Skip to content

feat(hashes)!: switch proof of work backend to dash-pow, bump Base SDK crates to dashpay/base-sdk@addedfa - #1214

Merged
xdustinface merged 5 commits into
dashpay:devfrom
kwvg:switch_pow
Oct 23, 2026
Merged

xdustinface merged 5 commits into
dashpay:devfrom
kwvg:switch_pow

Conversation

@kwvg

@kwvg kwvg commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Additional Information

  • As base-sdk crates are expected to share the same revision, they've been bumped to the latest develop as of this writing (addedfa1). This has the effect of requiring updates to existing callers due to base-sdk#61 and base-sdk#62.

  • The majority of performance improvements associated with dash-pow come from support for hardware accelerated AES and SIMD extensions for x86_64 and ArmV8. For more information, see base-sdk#80. portable is deliberately all-or-nothing, having dropped aes_hw as a separate feature in base-sdk#66. Every additional feature is another configuration to fuzz and validate on any change or optimisation, and the benefits thereof are too narrow.

    • wide only supports acceleration for x86_64, Aarch64 and WASM (source)

    • On AArch64, AES requires NEON, the hardware path asks for nothing beyond AES. A separate AES switch cover the exact same chips.

    • x86_64 chips are a bit different, Intel artificially restricted AVX/AVX2 to the Core and Xeon lineups till 2020 (source) so it is plausible for a chip to have AES-NI but lack AVX/AVX2. Unfortunately, dropping down from x86_64-v3 (source) would hamper performance on all chips Haswell and older, so they get portable alongside pre-Haswell chips.

Benchmarks

On an Apple M4 Pro, rustc 1.99.1 for hashing an 80-byte input using criterion.

Backend Time vs rs-x11-hash
rs-x11-hash 0.1.9 (sphlib C, -O2) 5.30 µs baseline
dashpay/base-sdk@addedfa, portable 5.29 µs -0.5%
dashpay/base-sdk@addedfa, targeting system requirements 2.94 µs -44%

On an Intel Xeon E3-1275 v6, rustc 1.99.1 for hashing an 80-byte input using criterion.

Backend Time vs rs-x11-hash
rs-x11-hash 0.1.9 (sphlib C, -O2) 13.48 µs baseline
dashpay/base-sdk@addedfa, portable 11.82 µs -12%
dashpay/base-sdk@addedfa, targeting system requirements 5.17 µs -62%

PR Hygiene · da224ee

  • Bots — coderabbitai ✓
  • Self-review — posted; again after any push
  • Build green
  • Approvals
    • files with no dedicated owner (CHANGELOG.md, Cargo.toml, crypto/src/bls.rs and 4 more) — QuantumExplorer or ZocoLini or xdustinface
    • xdustinface requested changes — waiting for them to re-review or dismiss

When every merge requirement is met, the PR Hygiene check passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.

@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

📝 Walkthrough

The workspace dependency revision changed. BLS and ECDSA calls now use digest-specific methods. The X11 hash engine now uses dash-pow for incremental hashing, and the hashes crate adds X11 benchmarks.

Changes: Cryptography and X11 hashing

Layer / File(s) Summary
SDK dependency and crypto API updates
Cargo.toml, crypto/src/bls.rs, crypto/src/ecdsa.rs, CHANGELOG.md
The workspace updates the base-sdk revision and adds dash-pow. BLS verification and secure aggregate verification use digest-specific methods. ECDSA signing and verification delegate to digest-specific methods. EcdsaSignature adds is_low_s, and recoverable-signature parsing uses PkcRecSigBytes::from_bytes. The changelog records these API changes.
Incremental X11 hashing integration
Cargo.toml, hashes/Cargo.toml, hashes/src/hash_x11.rs, hashes/benches/x11.rs, CHANGELOG.md
The X11 feature now uses dash-pow instead of rs-x11-hash, with target-specific portable features. HashEngine updates the X11 state incrementally, tracks input length, and finalizes cloned or owned state to produce hashes. Tests cover multiple input lengths. The crate adds benchmarks for six input sizes and an 80-byte header. The changelog records the X11 changes.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Refactor

Suggested reviewers: quantumexplorer

Merge Risk: ⚪ Minimal · up to da224

No merge-blocking behavior is established; the remaining comment is only an editorial wording preference.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 4 files. (2 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely identifies the two main changes: switching the proof-of-work backend to dash-pow and updating the Base SDK crates to the specified revision.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 4 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Oct 22, 2026
@kwvg kwvg changed the title refactor(hashes): switch proof of work backend to dash-pow, bump Base SDK crates to dashpay/base-sdk@addedfa feat(hashes)!: switch proof of work backend to dash-pow, bump Base SDK crates to dashpay/base-sdk@addedfa Oct 23, 2026
@kwvg
kwvg marked this pull request as ready for review October 23, 2026 06:42
@github-actions

Copy link
Copy Markdown
Contributor

Waiting for bot review — coderabbitai not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

@github-actions github-actions Bot added the waiting-bots Waiting for the review bots to report on this head label Oct 23, 2026
coderabbitai[bot]
coderabbitai Bot previously approved these changes Oct 23, 2026
@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Bots are done — your move: address xdustinface requested changes, then post /self-reviewed.
Full checklist in the description.

@github-actions github-actions Bot added waiting-self-review Waiting for the author to post /self-reviewed and removed waiting-bots Waiting for the review bots to report on this head labels Oct 23, 2026
@kwvg

kwvg commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

/self-reviewed

@github-actions github-actions Bot removed the waiting-self-review Waiting for the author to post /self-reviewed label Oct 23, 2026

@xdustinface xdustinface left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@kwvg the Ubuntu / spv, Ubuntu / ffi and Windows / spv failures come from this PR, not from flakiness. Unoptimized dash-pow takes about 1.1 ms per X11 hash on x86_64, against about 50 µs for rs-x11-hash. Header decompression then blocks the network reader for seconds at a time, and the dashd tests time out. The hashes are correct. Adding this to the root Cargo.toml fixes it:

[profile.dev.package.dash-pow]
opt-level = 3
Measurement dev (rs-x11-hash) PR PR + override
X11 per header, debug, x86_64 (Rosetta) 44 to 70 µs 1113 to 1122 µs 17 to 20 µs
X11 per header, debug + llvm-cov, aarch64 57 µs 443 µs 17 µs
X11 per header, release, aarch64 8.4 to 10.3 µs 8.6 to 10.7 µs same as PR
dash-spv dashd suite (35 tests) all pass, 133 s 3 fail, 209 s all pass, 94 s
dash-spv-ffi dashd suite (7 tests) all pass, 51 s all pass, 86 s all pass, 52 s

The suites ran on aarch64 with portable forced to match CI's x86_64 path. All variants produce identical hashes. The override adds about 3 to 5 s to compile dash-pow once, and rust-cache keeps that build between runs.

Follow-ups, not blocking:

  • Cargo ignores profile overrides from dependencies, so downstream crates need the same override in their own workspace. Worth a changelog note.
  • dash-pow selects hardware AES at compile time only, so default x86_64 builds never use AES-NI. Runtime detection in base-sdk would fix that.

@github-actions github-actions Bot added the waiting-self-review Waiting for the author to post /self-reviewed label Oct 23, 2026
kwvg and others added 2 commits October 23, 2026 17:07
Co-authored-by: xdustinface <xdustinfacex@gmail.com>
@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Waiting for bot review — coderabbitai not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

@github-actions github-actions Bot added waiting-bots Waiting for the review bots to report on this head and removed waiting-self-review Waiting for the author to post /self-reviewed labels Oct 23, 2026

@coderabbitai coderabbitai Bot left a comment

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.

🔇 Additional comments: 3
Cargo.toml: 1

49-51: LGTM!

CHANGELOG.md: 1

18-18: LGTM!

Also applies to: 75-83

CHANGELOG.md-86-86: 1

86-86: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Fix the verb agreement in the X11 entry.

Use “rs-x11-hash always reads 80 bytes.”

Source: Linters/SAST tools


ℹ️ Review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between 61f59c9 and da224ee.

📒 Files selected for processing: 2
  • CHANGELOG.md
  • Cargo.toml

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

@github-actions

Copy link
Copy Markdown
Contributor

Bots are done — your move: address xdustinface requested changes, then post /self-reviewed.
Full checklist in the description.

@github-actions github-actions Bot added waiting-self-review Waiting for the author to post /self-reviewed and removed waiting-bots Waiting for the review bots to report on this head labels Oct 23, 2026
@codecov

codecov Bot commented Oct 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.68%. Comparing base (6baf1d3) to head (da224ee).

Files with missing lines Patch % Lines
hashes/src/hash_x11.rs 92.85% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1214      +/-   ##
==========================================
- Coverage   78.96%   78.68%   -0.28%     
==========================================
  Files         304      304              
  Lines       80988    80998      +10     
==========================================
- Hits        63951    63733     -218     
- Misses      17037    17265     +228     
Flag Coverage Δ
core 77.75% <92.85%> (+0.01%) ⬆️
ffi 49.80% <ø> (-1.96%) ⬇️
rpc 52.05% <ø> (ø)
spv 92.33% <ø> (-0.10%) ⬇️
wallet 82.54% <ø> (ø)
Files with missing lines Coverage Δ
hashes/src/hash_x11.rs 74.13% <92.85%> (+5.38%) ⬆️

... and 19 files with indirect coverage changes

@kwvg

kwvg commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

/self-reviewed

@github-actions

Copy link
Copy Markdown
Contributor

Ready for review — files with no dedicated owner: QuantumExplorer or ZocoLini or xdustinface · re-review or resolve: xdustinface.
Full checklist in the description.

@github-actions github-actions Bot added ready-for-human Bots have reported, the author has self-reviewed, and the build is green: this needs a human. and removed waiting-self-review Waiting for the author to post /self-reviewed labels Oct 23, 2026
@xdustinface
xdustinface merged commit 962a462 into dashpay:dev Oct 23, 2026
68 of 70 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Bots have reported, the author has self-reviewed, and the build is green: this needs a human.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants