feat(platform-wallet): derive owner/voting provider keys Rust-side - #4338
Merged
QuantumExplorer merged 3 commits intoAug 8, 2026
Merged
Conversation
The secp256k1 provider families were the only key material the iOS app
derived in Swift, and the only ones sourced from a throwaway key-wallet
rebuilt from the mnemonic. Everything else — payments via
`CoreTransactionBuilder`, identity/DPNS via the identity signer, and the
BLS operator / Ed25519 platform-node provider keys via
`derive_provider_key_at_index` — already derives from the running wallet
on the Rust side. Owner and voting were on the odd path only because
`ProviderKeyKind` never had variants for them.
Adds `ProviderKeyKind::{Owner, Voting}` (account tags 9 / 8) with a
secp256k1 branch: the public key and P2PKH address come off the account
xpub (non-hardened, no seed needed, as with BLS operator keys), and the
private key from `derive_from_seed_private_key_at`, which applies the
DIP-3 account path exactly once.
The branch runs the same seed-vs-xpub cross-check the operator family
does. That guard is precisely what catches this class of bug: the two
sides derive independently, and a mismatched (public, private) pair has no
local symptom — it signs for an address nobody expects. On the voting
family that surfaces only as Platform rejecting the vote as having no
voter identity, because the voter identity is derived from the signing
key's own hash160.
`ProviderDerivedKey` / the FFI struct gain `address` and
`private_key_wif`, both non-null only for the secp256k1 families. WIF is
encoded Rust-side so the network byte and compression flag have one home.
The WIF is zeroized on free like the hex scalar.
Tests pin owner and voting derivation against explicit
`m/9'/{5'|1'}/3'/{2'|1'}/index` paths at indexes 0/1/19 on both networks,
plus a guard that the doubled path is not what we derive.
Note: this is a `#[repr(C)]` change — consumers need a rebuilt xcframework.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ly gate `Account::derive_from_seed_private_key_at` gates on `is_watch_only`, so it refused exactly the external-signable wallets the seed argument exists to serve — the iOS app's shape, where the seed lives in the Keychain and the caller resolves it on demand. On device: "Watch-only wallet: private keys not available". The module docs already warned about this asymmetry and are why the BLS and Ed25519 families use key-wallet's gate-free #881 entry points. secp256k1 has no such entry point, so derive inline in the same shape as `BLSAccount::operator_private_key_at`: raw seed → master xpriv → the account type's own DIP-3 path → non-hardened child. The path comes from the account type, so it is still applied exactly once. The existing tests passed throughout because they build seed-bearing wallets and never reach the gate. Adds a watch-only test that pins both halves: the gated wrapper is expected to refuse such an account, and the gate-free derivation must still match m/9'/5'/3'/{2'|1'}/index. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
QuantumExplorer
requested review from
ZocoLini,
lklimek,
llbartekll and
shumkov
as code owners
August 8, 2026 08:12
Contributor
|
Warning Review limit reached
Next review available in: 28 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughProvider key derivation now supports secp256k1 owner and voting keys. Rust exposes optional addresses and WIF values through FFI. Swift maps these values into ChangesProvider key derivation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SwiftSDK
participant ProviderKeyFFI
participant RustWallet
participant AccountXpub
participant RawSeed
SwiftSDK->>ProviderKeyFFI: request provider key at index
ProviderKeyFFI->>RustWallet: dispatch owner or voting key kind
RustWallet->>AccountXpub: derive public key and address
RustWallet->>RawSeed: derive private key and WIF
RustWallet-->>ProviderKeyFFI: return key, address, and WIF
ProviderKeyFFI-->>SwiftSDK: expose address and WIF strings
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/rs-platform-wallet-ffi/src/provider_key_at_index.rs (1)
160-168: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the invalid-kind diagnostic.
The dispatch accepts voting and owner tags, but the error says that only operator and platform-node tags are valid. Include all four supported kinds in this message.
Proposed fix
- "unknown provider key kind {other} (expected {PROVIDER_KEY_KIND_OPERATOR} \ - operator or {PROVIDER_KEY_KIND_PLATFORM_NODE} platform node)" + "unknown provider key kind {other} (expected {PROVIDER_KEY_KIND_VOTING} \ + voting, {PROVIDER_KEY_KIND_OWNER} owner, \ + {PROVIDER_KEY_KIND_OPERATOR} operator, or \ + {PROVIDER_KEY_KIND_PLATFORM_NODE} platform node)"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rs-platform-wallet-ffi/src/provider_key_at_index.rs` around lines 160 - 168, Update the invalid-kind diagnostic in the provider key kind dispatch to list all supported tags: operator, platform node, voting, and owner. Preserve the existing invalid-parameter result and formatting while correcting the expected-values text.
🧹 Nitpick comments (1)
packages/rs-platform-wallet/src/wallet/provider_key_at_index.rs (1)
1019-1179: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd direct tests for
PlatformWallet::derive_provider_key_at_index.No test calls this method. Cover
ProviderKeyKind::OwnerandProviderKeyKind::Votingwithinclude_privateset to bothfalseandtrue. Assertpublic_key_bytes,address,private_key, andprivate_key_wif.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rs-platform-wallet/src/wallet/provider_key_at_index.rs` around lines 1019 - 1179, Add direct tests for PlatformWallet::derive_provider_key_at_index covering both ProviderKeyKind::Owner and Voting, with include_private set to false and true. Assert each result’s public_key_bytes and address, and verify private_key and private_key_wif match the expected presence and values for the requested flag. Use deterministic wallet/seed fixtures and fixed indices so the tests validate the public and private derivation outputs directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/rs-platform-wallet-ffi/src/provider_key_at_index.rs`:
- Around line 160-168: Update the invalid-kind diagnostic in the provider key
kind dispatch to list all supported tags: operator, platform node, voting, and
owner. Preserve the existing invalid-parameter result and formatting while
correcting the expected-values text.
---
Nitpick comments:
In `@packages/rs-platform-wallet/src/wallet/provider_key_at_index.rs`:
- Around line 1019-1179: Add direct tests for
PlatformWallet::derive_provider_key_at_index covering both
ProviderKeyKind::Owner and Voting, with include_private set to false and true.
Assert each result’s public_key_bytes and address, and verify private_key and
private_key_wif match the expected presence and values for the requested flag.
Use deterministic wallet/seed fixtures and fixed indices so the tests validate
the public and private derivation outputs directly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e1045041-fa5b-4824-98e1-bfb6f1285e5e
📒 Files selected for processing (3)
packages/rs-platform-wallet-ffi/src/provider_key_at_index.rspackages/rs-platform-wallet/src/wallet/provider_key_at_index.rspackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift
Collaborator
|
🕓 Ready for review — 1 ahead in queue (commit db24fe3) |
This was referenced Aug 8, 2026
…e the scalar Review follow-ups on the secp256k1 provider arm. **Path source (found by the new end-to-end tests).** The private side resolved the account path from `account_type.derivation_path(network)` using the WALLET's network, while the public side comes off `account.account_xpub`, built from the ACCOUNT's own path and network. Two independent sources for one path: they disagree the moment those networks differ, and the cross-check then rejected a correct seed. Now taken from `account.derivation_path()`, so the sides agree by construction and the cross-check verifies the seed rather than the path. This is exactly the gap review flagged: the path tests re-derived the private side and never called `derive_provider_key_at_index`, so nothing compared it against the account xpub. **Scalar wiping (P2).** `dashcore::PrivateKey` is `Copy` with no `Drop`, so unlike the `Zeroizing` outputs nothing scrubbed it. Both copies are now erased with `non_secure_erase` — the `PrivateKey` and the extended key it came from — after the outputs are produced and before returning. **Invalid-kind diagnostic (P3).** Now names all four accepted kinds instead of only operator and platform node. **Tests.** New `provider_ecdsa_key_tests` drives the real entry point: a public listing carries an address and no private material; a private reveal is internally consistent (WIF ↔ scalar ↔ public key ↔ P2PKH on the wallet's own network); indexes and the two families produce distinct keys; a foreign seed is refused by the cross-check; and an explicitly supplied seed matches the resident one, which is the external-signable shape the app runs. They also document that `derive_provider_key_at_index` is synchronous and takes the manager lock with `blocking_read`, so it must be called off the async executor — the tests use `block_in_place`, as a caller must. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
QuantumExplorer
deleted the
feat/provider-ecdsa-keys-via-platform-wallet
branch
August 8, 2026 08:43
QuantumExplorer
added a commit
that referenced
this pull request
Aug 8, 2026
Unused after #4338 moved the secp256k1 provider families onto `providerKeyAtIndex`: no caller remains in platform (including the example app) or in dashwallet-ios. Removed rather than fixed. It applied the account derivation path twice — it asked callers for the account root while the FFI applies the account's own path itself — so every owner/voting key came from `m/9'/5'/3'/1'/9'/5'/3'/1'/index`. Nothing failed locally, because the keys were well-formed and deterministic; it surfaced only as Platform rejecting masternode votes as having no voter identity. Correcting it would have left the last of the parallel derivation path #4338 exists to consolidate, and kept the variant WITHOUT the seed-vs-xpub cross-check, watch-only support, or address that `providerKeyAtIndex` provides. Its ambiguity was the bug — called on an account, taking a wallet, the relationship between the account's own path and the passed key implicit — so even corrected, the next reader has to re-derive why "m" is right. Deleting removes the question. Callers wanting key material use `providerKeyAtIndex` (provider families, DIP-3 path resolved Rust-side and cross-checked) or `Wallet.derivePrivateKey(path:)` (explicit full path, no implicit path to apply twice). `Account`'s doc now says this, and describes what the type actually is: a handle whose lifetime `Wallet.getAccount(type:)` uses to report that an account exists. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 8, 2026
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The secp256k1 provider families (masternode owner and voting keys) were the only key material the iOS wallet derived in Swift, and the only ones sourced from a throwaway key-wallet rebuilt from the mnemonic. Everything else already derives Rust-side from the running wallet:
CoreTransactionBuilderderive_provider_key_at_indexAccount.derivePrivateKeyWIF← the outlierOwner and voting were on the odd path only because
ProviderKeyKindnever had variants for them. That outlier turned out to be broken: the Swift wrapper pre-derived the account root and passed it as the "master", applying the DIP-3 path twice, so every owner/voting key came fromm/9'/5'/3'/1'/9'/5'/3'/1'/index. Nothing failed locally — the keys were well-formed — and it surfaced only as Platform rejecting masternode votes as having no voter identity.What
ProviderKeyKind::{Owner, Voting}(account tags 9 / 8) with a secp256k1 branch: public key and P2PKH address off the account xpub (non-hardened, no seed — as with BLS operator), private key derived from the raw seed.ProviderDerivedKey/ the FFI struct gainaddressandprivate_key_wif, non-null only for the ECDSA families. WIF is encoded Rust-side so the network byte and compression flag have one home; it is zeroized on free like the hex scalar.The watch-only gate
The first cut used
Account::derive_from_seed_private_key_at, which gates onis_watch_onlyand so refused exactly the external-signable wallets the seed argument exists to serve — on device: "Watch-only wallet: private keys not available". This module's own docs already warn about that asymmetry, and are why the BLS/Ed25519 families use key-wallet's gate-free entry points. secp256k1 has none, so the derivation is inline in the same shape asBLSAccount::operator_private_key_at: raw seed → master xpriv → the account type's DIP-3 path → non-hardened child.Testing
platform-walletprovider-key tests: 9/9.m/9'/{5'|1'}/3'/{2'|1'}/indexat indexes 0, 1, 19, on mainnet and testnetNotes
#[repr(C)]change — consumers need a rebuilt xcframework.derivePrivateKeyWIF), but fix(swift-sdk): stop applying the account derivation path twice #4334 is still worth landing: that API stays public and wrong-by-default for the next caller.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes