fix(wallet): derive masternode owner/voting keys through the Rust-side resolver - #937
Merged
Merged
Conversation
`Account.derivePrivateKeyWIF(wallet:masterPath:index:)` asked callers for the account root path while the FFI applied the account's own path on top, so every owner/voting key came off a doubly-applied branch. The keys were well-formed, so the screen looked correct while showing keys that were not the masternode's. The SDK withdrew that call (platform#4339) after moving the derivation Rust-side (platform#4338). Route both families through `providerKeyAtIndex`, as the operator and platform-node families already do: it owns the DIP-3 path and cross-checks the derived key against the account xpub. Deliberately not rebuilding the path app-side — composing it here is what allowed the double application. `accountRootPath` and `accountType` become write-only with the derivation gone, and the throwaway derivation wallet now backs only the fallback address pool, so its comment no longer claims otherwise. Values on the Masternode Keys screen change: the new ones are correct.
📝 WalkthroughWalkthroughMasternode owner and voting key derivation now uses managed-wallet provider keys. The model maps key types to provider kinds, caches resolved keys by index, retains fallback address-pool loading, and reads WIF and hexadecimal values directly from provider-derived keys. ChangesProvider-key masternode derivation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
romchornyi
changed the title
romchornyi
changed the title
This was referenced Aug 8, 2026
QuantumExplorer
added a commit
that referenced
this pull request
Aug 8, 2026
#936 and #937 landed independently and each brought its own memoised wrapper over `ManagedPlatformWallet.providerKeyAtIndex` — byte-identical bodies, one for the ECDSA owner/voting family and one for BLS/Ed25519. Two caches to keep in step and two places to change. Collapse both into a file-private `ProviderKeyResolver`, now the single call site for all four families. `MasternodeProviderKeyDeriver` keeps its distinct job — it carries the address pool the owner/voting rows join against — and delegates derivation. Operator/evonode-operator have no address rows, so their wrapper shell goes away entirely and the model holds a resolver directly; `tenderdashNodeKeyBase64` moves onto the resolver, still guarded on the Ed25519 kind. Also drops three pieces of state left dead by #937, when derivation moved Rust-side: * `wallet` — written in init, never read since. It held a `Wallet` owned by the `WalletManager` that `derivationWallet()` builds per call and that this class never retained, so it was a reference into a graph released at the end of init. Unused, so harmless, but not worth keeping. * `key` — written, never read. * the bound `network` — it selected the coin type back when this class composed the DIP-3 path app-side. Now a boolean test, with a comment for why the check stays. No behaviour change. Same kinds, same `includePrivate: true`, same cache lifetime (one per deriver instance). The host-wallet guard now runs before the `derivationWallet()` guard rather than after, which cannot change the outcome: `derivationWallet()` returns nil unless that same wallet is present, so the guard only fires where the other would have. Verified: clean `dashpay` build, no warnings left in the file. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
Issue being fixed or feature implemented
Two things, one change.
developno longer compiles againstv4.2-dev. dashpay/platform#4339 withdrewAccount.derivePrivateKeyWIF(wallet:masterPath:index:)from the Swift SDK, andDerivationPathKeysModelwas its only caller. Anyone who pulls a currentplatformcheckout hitsValue of type 'Account' has no member 'derivePrivateKeyWIF'.The withdrawn call was returning the wrong keys. From the upstream removal:
So the Masternode Keys screen has been rendering owner/voting keys off a doubly-applied branch. They looked right — correct format, stable across launches — they simply were not the masternode's keys.
What was done?
ManagedPlatformWallet.providerKeyAtIndex(kind:index:includePrivate:), the same resolver the operator (BLS) and platform-node (Ed25519) families already use viaProviderKeyDeriver.ProviderKeyKindgainedvotingECDSA/ownerECDSAin feat(platform-wallet): derive owner/voting provider keys Rust-side platform#4338, which is what makes this possible. The resolver owns the DIP-3 path and cross-checks the derived key against the account xpub.accountRootPath + indexhere would have been the smaller diff, but app-side path composition is exactly what allowed the double application; getting hardened-vs-soft wrong at the last level fails the same silent way.ProviderKeyDeriver— the screen renders a private-key row per index and the Ed25519 path pulls the seed through the mnemonic resolver on every call.accountRootPathandaccountTypebecame write-only once the derivation moved; the throwaway derivation wallet now backs only the fallback address pool, so its comment no longer claims it is "used ONLY for private-key derivation", and the type doc no longer implies this class builds the paths.How Has This Been Tested?
Clean
dashpaybuild againstplatformatv4.2-dev(bce107b61a, rust-dashcore pin944e53a5) with a freshly builtDashSDKFFI.xcframework— 0 errors, no warnings in the touched file.Not yet verified on device against a known-good source. The derived values on the Masternode Keys screen will change, and the point of the change is that the new ones are correct — but confirming that means comparing a rendered owner/voting key against the same key from another wallet or a ProRegTx. Worth doing before this ships to anyone who might copy those keys out.
Breaking Changes
No API changes. User-visible behaviour does change: owner/voting keys shown on the Masternode Keys screen differ from previous builds. Anyone who copied a key from an earlier build should re-check it — the earlier value came from the wrong derivation branch.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit