test(key-wallet): pin scalar reduction paths, refusal of zero scalars and public keys off the curve

[kwvg]

Motivation

Conflicting agentic advice when working on rust-dashcore#1036 resulted in flagging permitting scalar reduction as a regression (against strictly refusing >R), then flagging later on that refusal conflicts with secp256k1's larger field and the HMAC output frequently >R.

This pull request pins behaviour for the sake of posterity as both are opposite approaches and tests still passed, making them seem plausible despite breaking effects.

Additional Information

• Dependency for feat: abstract BLS backend types behind newtypes, isolate to dashcore-crypto crate, switch to dash-pkc from blsful fork #1036

PR Hygiene · 6f3d5d4

• Bots — coderabbitai ✓
• Self-review — posted; again after any push
• Within your 5 open PRs
• Build green
• Approvals
  • key-wallet (key-wallet/src/derivation_bls_bip32.rs) — approved by ZocoLini

When every box is checked the PR Hygiene check passes and this can merge.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: dashpay/rust-dashcore/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 5b1a7b48-5937-4d38-9b01-6da1b75f4b08

📥 Commits

Reviewing files that changed from the base of the PR and between 82db28b and 6f3d5d4.

📒 Files selected for processing (1)

• key-wallet/src/derivation_bls_bip32.rs

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

---

📝 Walkthrough

Walkthrough

The change centralizes repeated BLS derivation HMAC logic and adds policy tests. The tests cover scalar reduction, derivation tweaks, endian conversion, zero-scalar rejection, and off-curve public-key rejection.

Changes

BLS derivation

Layer / File(s)	Summary
Shared derivation HMAC flow key-wallet/src/derivation_bls_bip32.rs	Adds MASTER_HMAC_KEY and derivation_hmac. Master, private-child, and public-child derivation use the shared helper.
Scalar policy validation key-wallet/src/derivation_bls_bip32.rs	Moves shared seed fixtures into the parent test module. Adds tests for scalar reduction, derivation tweaks, endian conversion, zero scalars, and off-curve public keys.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to 6f3d5

The current changes add test coverage and leave no actionable merge-blocking risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the primary change: tests for scalar reduction, zero-scalar rejection, and off-curve public-key rejection.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.15%. Comparing base (3d24459) to head (6f3d5d4).
⚠️ Report is 7 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #1038      +/-   ##
==========================================
+ Coverage   77.12%   77.15%   +0.02%     
==========================================
  Files         329      329              
  Lines       83862    83900      +38     
==========================================
+ Hits        64680    64729      +49     
+ Misses      19182    19171      -11     

Flag	Coverage Δ
core	78.24% <ø> (ø)
ffi	49.82% <ø> (-0.50%)	⬇️
rpc	20.00% <ø> (ø)
spv	92.08% <ø> (+0.02%)	⬆️
wallet	80.04% <100.00%> (+0.25%)	⬆️

Files with missing lines	Coverage Δ
key-wallet/src/derivation_bls_bip32.rs	94.27% <100.00%> (+0.23%)	⬆️

... and 30 files with indirect coverage changes

[github-actions]

Ready for review — needs QuantumExplorer or ZocoLini or xdustinface.
Full checklist in the description.

[kwvg]

/self-reviewed

[ZocoLini]

1. Hoist the fixture. Move SEED64 and master_from_seed64() up into mod tests, drop the pub(super). Both submodules already do use super::*, so no call site changes.
2. Delete abandon_master() — it is a byte-for-byte copy of master_from_seed64(), and avoiding a second pub(super) is what it cost. With 1+2 the PR comes out 6 lines shorter and with no visibility annotations: I tried it, 31 tests still pass.
3. Delete public_derivation_tracks_private_derivation — duplicates test_derivation_matches_through_private_and_public (line 824), and is unrelated to scalar reduction.
4. Delete scalar_above_the_order_is_accepted — subsumed by scalar_above_the_order_is_reduced_modulo_r, which unwraps the same CtOption.
5. Delete the seed_hmac() helper, keep SEED64_HMAC. The helper is a copy of production's HMAC, and the test already compares against the constant.

Plus, optionally: give child_derivation_reduces_its_tweak an assert!(tweak >= r) on one fixed index, so it pins what its name claims instead of only is_ok().

[kwvg]

/self-reviewed

[kwvg]

/self-reviewed

[github-actions]

Policy satisfied — this can merge.
Full checklist in the description.