Skip to content

fix(key-wallet): derive BLS operator & Ed25519 platform-node keys per DashSync/dashbls#879

Merged
QuantumExplorer merged 2 commits into
devfrom
claude/issue-878-fix-2b9e2f
Jul 13, 2026
Merged

fix(key-wallet): derive BLS operator & Ed25519 platform-node keys per DashSync/dashbls#879
QuantumExplorer merged 2 commits into
devfrom
claude/issue-878-fix-2b9e2f

Conversation

@QuantumExplorer

@QuantumExplorer QuantumExplorer commented Jul 13, 2026

Copy link
Copy Markdown
Member

Fixes #878.

key-wallet derived wrong masternode operator BLS keys and Ed25519 platform node ids — they didn't match DashSync/dashwallet-ios (and therefore real on-chain ProRegTx registrations) for the same mnemonic. All three bugs from the issue are fixed together:

Bug 1 — spurious leading 0x00 in hardened BLS child derivation

ExtendedBLSPrivKey::derive_priv built the hardened HMAC input as 0x00 ‖ sk ‖ i ‖ {0,1} (the secp256k1 BIP32 convention). dashbls (ExtendedPrivateKey::PrivateChild) uses sk ‖ i ‖ {0,1} with no leading zero. Since every level of the provider paths is hardened, derivation diverged from the first child. (This fix is mode-independent — dashbls never serializes the pubkey for hardened children.)

Bug 2 — serialization mode of the HD chain

Non-hardened derivation feeds the parent public key into the HMAC, so the G1 serialization format is part of the derivation itself — dashbls parameterizes it as fLegacy. key-wallet only implemented the modern/IETF mode, while DashSync derives operator keys with fLegacy = true. The API now mirrors dashbls and supports both modes:

  • derive_priv / derive_pub / derive_path — modern (IETF) serialization, like dashbls PrivateChild(i) with the default fLegacy = false
  • derive_priv_legacy / derive_pub_legacy / derive_path_legacy — legacy Dash serialization (fLegacy = true)
  • *_with_mode(…, SerializationFormat) — explicit mode

The provider-key account layer (BLSAccount, BLS address pools) derives with the legacy variants, matching DashSync for masternode operator keys. Output serialization is likewise available in both formats: to_bytes() / public_key_bytes() (modern, unchanged — same bytes as v19+ ProRegTx payloads) and new to_bytes_legacy() / public_key_bytes_legacy() (DashSync's KeyKind_BLS format). Storage/serde formats are unchanged.

Bug 3 — hybrid secp256k1→BLS/Ed25519 seed in the default wallet flow

The default flow derived a secp256k1 BIP32 key at the provider path, reused its 32 secret bytes as the BLS/SLIP-0010 master seed, then applied the provider path again. Now:

  • BLSAccount::from_seed / EdDSAAccount::from_seed take the wallet seed (&[u8], e.g. the 64-byte BIP39 seed), feed it directly into the target-curve master (dashbls FromSeed / SLIP-0010) and apply the account path once; the stored account xpub is the account-level key, so key i is its ith child — exactly DashSync's structure.
  • Wallet::add_bls_account / add_eddsa_account with None use the wallet's own seed (new Wallet::wallet_seed_bytes()); wallets without a seed (extended-priv-key based, watch-only, external-signable) get a typed error, and default account creation skips these accounts for seedless wallets instead of fabricating keys that never correspond to anything on-chain.
  • BLSAccount::has_internal_and_external() corrected to false (operator keys live in a single account/i pool), which also unblocks the seed-based signing path derive_from_seed_private_key_at for operator accounts (it previously always errored).

Test vectors

Reference vectors were generated with dashbls itself (dashpay/bls-signatures @ 0842b17, the C++ library DashSync uses via FFI) using ExtendedPrivateKey::FromSeed + PrivateChild(i, fLegacy=true/false) through its Rust bindings, from the standard BIP39 test mnemonic (abandon … about, empty passphrase):

  • module-level vectors in derivation_bls_bip32.rs for both modes (master, m/9'/5'/3'/3' chain, legacy non-hardened children 0–2 in both output serializations, modern-mode children, chia-style short-seed vectors, cross-mode divergence check),
  • wallet-level end-to-end tests in tests/provider_key_derivation_tests.rs (mainnet + testnet operator keys via both the watch-side xpub path and the seed-based signing path; Ed25519 platform node keys pinned to SLIP-0010 vectors verified with an independent implementation),
  • an FFI-level test asserting bls_account_derive_private_key_from_seed / eddsa_account_derive_private_key_from_seed return the reference keys.

Breaking / migration notes

  • BLSAccount::from_seed, EdDSAAccount::from_seed, Wallet::add_bls_account, Wallet::add_eddsa_account signatures changed ([u8; 32]&[u8]), and the seed parameter now means the wallet seed, not a raw curve seed.
  • Keys previously persisted from these accounts change after this fix — intentional, per the issue: the old ones never corresponded to anything on-chain.
  • Default account creation no longer creates operator/platform accounts for wallets built from a bare extended private key (no seed exists to derive them correctly).

Testing

  • cargo test -p key-wallet --all-features — 564 passed
  • cargo test -p key-wallet-ffi — 260 passed
  • cargo test -p dash-spv with dashd integration tests — all passed
  • cargo test -p dash-spv-ffi --test dashd_sync — 7 passed
  • cargo fmt --check, cargo clippy --all-features --all-targets — clean

🤖 Generated with Claude Code

… DashSync/dashbls

Fixes three independent bugs that made masternode operator (BLS) and
platform node (Ed25519) keys diverge from DashSync/dashwallet-ios for
the same mnemonic (#878):

1. Hardened BLS child derivation prepended a spurious 0x00 to the HMAC
   input (secp256k1 BIP32 convention). dashbls uses sk(32) || i(4) || {0,1}
   with no leading zero, so every hardened level diverged.

2. Non-hardened BLS derivation fed the parent public key into the HMAC
   in modern/IETF serialization. dashbls serializes it with the legacy
   flag (fLegacy=true) throughout the HD chain. ExtendedBLSPrivKey/
   ExtendedBLSPubKey now use legacy bytes in the HMAC input and expose
   public_key_bytes_legacy()/to_bytes_legacy().

3. The default wallet flow derived a secp256k1 BIP32 key at the provider
   path, reused its secret bytes as the BLS/SLIP-0010 master seed, and
   applied the provider path again on top. BLSAccount::from_seed /
   EdDSAAccount::from_seed now take the wallet seed (e.g. the 64-byte
   BIP39 seed), feed it directly into the target-curve master and apply
   the account path once; add_bls_account/add_eddsa_account use the
   wallet's own seed. Seedless wallets (extended-priv-key based, watch-
   only, external-signable) skip auto-creation of these accounts and
   require an explicit seed.

Also fixes BLSAccount::has_internal_and_external() to false — operator
keys live in a single pool (account/i), so seed-based signing derivation
(derive_from_seed_private_key_at) now works and yields m/9'/coin'/3'/3'/i.

Derivation is pinned by test vectors generated with dashbls
(dashpay/bls-signatures @ 0842b17, ExtendedPrivateKey with fLegacy=true)
from the standard BIP39 test mnemonic, and SLIP-0010 vectors verified
with an independent implementation. Keys previously persisted from these
accounts change after this fix — the old ones never corresponded to
anything on-chain.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@QuantumExplorer, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 23 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: cae196da-df5d-494d-9595-0ca0c836db25

📥 Commits

Reviewing files that changed from the base of the PR and between 0ae8df4 and 4b69765.

📒 Files selected for processing (11)
  • key-wallet-ffi/src/account_derivation_tests.rs
  • key-wallet/examples/account_types.rs
  • key-wallet/src/account/account_collection_test.rs
  • key-wallet/src/account/bls_account.rs
  • key-wallet/src/account/eddsa_account.rs
  • key-wallet/src/derivation_bls_bip32.rs
  • key-wallet/src/managed_account/address_pool.rs
  • key-wallet/src/tests/mod.rs
  • key-wallet/src/tests/provider_key_derivation_tests.rs
  • key-wallet/src/wallet/accounts.rs
  • key-wallet/src/wallet/helper.rs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/issue-878-fix-2b9e2f

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.

…default, legacy explicit)

Mirror dashbls's fLegacy parameter instead of hard-coding legacy in the
HD chain: derive_priv/derive_pub/derive_path use the modern (IETF)
serialization like dashbls PrivateChild/PublicChild with the default
fLegacy=false, and *_legacy / *_with_mode variants select the legacy
Dash serialization. The provider-key account layer (BLSAccount, BLS
address pools) derives with the legacy variants, which is what
dashbls/DashSync use for masternode operator keys.

Hardened derivation never serializes the public key, so both modes agree
there; only non-hardened children differ. Modern-mode derivation is
pinned by dashbls-generated vectors alongside the legacy ones.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@QuantumExplorer

Copy link
Copy Markdown
Member Author

Reviewed

@QuantumExplorer
QuantumExplorer merged commit 56a4078 into dev Jul 13, 2026
33 of 34 checks passed
@QuantumExplorer
QuantumExplorer deleted the claude/issue-878-fix-2b9e2f branch July 13, 2026 22:07
QuantumExplorer added a commit that referenced this pull request Jul 14, 2026
…tform-node at index) (#881)

* feat(key-wallet): gate-free provider-key derivation entry points (#880)

Add wallet-state-agnostic entry points for deriving provider keys at an
index, with no is_watch_only gate in either direction:

- BLSAccount::operator_public_key_at(index): legacy-scheme non-hardened
  public derivation off the stored account xpub; works on resident AND
  watch-only accounts. Returns the extended key so both the legacy and
  modern/IETF serializations are obtainable. Rejects non-operator
  accounts so a mixed-up lookup fails loudly.
- BLSAccount::operator_private_key_at(seed, network, index): re-derives
  the full DIP-3 operator path from the raw BIP39 seed in the legacy BLS
  scheme, then the non-hardened child; needs no account state.
- EdDSAAccount::platform_node_key_at(seed, network, index): SLIP-0010
  hardened derivation of the platform node key from the raw seed.

The existing convenience wrappers are gated on is_watch_only in opposite
directions (public derivation requires watch-only, seed derivation
requires non-watch-only), so no single upstream API served both resident
and restored/external-signable wallets — forcing downstream consumers to
re-compose low-level primitives (see dashpay/platform#4120 and the stale
derivation bug behind #878/#879).

New tests pin the entry points to the existing dashbls / SLIP-0010
golden vectors and assert they behave identically on resident and
watch-only accounts.

Closes #880

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci: pin sanitizer jobs to nightly-2026-07-13 (rustc ICE on latest nightly)

The 2026-07-14 nightly (rustc daf2e5e18) ICEs while expanding the
libtest harness for dash-spv-ffi tests ("attribute is missing tokens:
rustc_test_entrypoint_marker" in rustc_ast/src/attr/mod.rs), breaking
the Address Sanitizer job for every PR. Pin ASAN/TSAN to the last
known-good nightly until a fixed nightly ships.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.

key-wallet: BLS operator & Ed25519 platform-node key derivation diverges from DashSync/dashbls (wrong masternode keys)

1 participant