chore(swift-sdk): remove Account.derivePrivateKeyWIF by QuantumExplorer · Pull Request #4339 · dashpay/platform

QuantumExplorer · 2026-08-08T08:20:35Z

Replaces #4334, which fixed this function. Deleting it is better now that nothing calls it.

Why it's unused

#4338 moves the secp256k1 provider families (masternode owner / voting keys) onto providerKeyAtIndex. After it, Account.derivePrivateKeyWIF has zero callers — in platform (including the example app) and in dashwallet-ios.

Why delete rather than fix

It applied the account derivation path twice: it asked callers for the account root path, while account_derive_private_key_as_wif_at → derive_xpriv_from_master_xpriv resolves Account::derivation_path() itself. So every owner/voting key came from

m/9'/5'/3'/1'/9'/5'/3'/1'/index instead of m/9'/5'/3'/1'/index

Nothing failed locally — the keys were well-formed and deterministic, and round-tripped through WIF parsing and signing without complaint. It surfaced only as Platform rejecting masternode votes as having no voter identity, because the voter identity is derived from the signing key's own hash160.

Fixing it (#4334) would have kept 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. The ambiguity was the bug: called on an account, taking a wallet, with the relationship between the account's own path and the passed key implicit. Even corrected, the next reader has to re-derive why "m" is right. Deleting removes the question instead of documenting the answer.

Replacements

need	use
masternode provider keys	`ManagedPlatformWallet.providerKeyAtIndex(kind:index:includePrivate:)` — DIP-3 path resolved Rust-side, seed cross-checked against the account xpub, works for watch-only
an explicit path	`Wallet.derivePrivateKey(path:)` — full path in, key out; no implicit path to apply twice

Account's doc now points at both, and describes what the type actually is: a handle whose lifetime Wallet.getAccount(type:) uses to report that an account exists (and create it as a side effect). That is its one remaining caller.

Coverage

No regression. #4334's Swift tests pinned the DIP-3 paths, but #4338's Rust tests pin the same paths at the layer that derives them — plus the watch-only case the Swift tests never reached, which is what the app actually runs.

Testing

dashwallet-ios builds clean against this branch (the only real consumer)
no derivePrivateKeyWIF definition or call site remains in either repo

Depends on #4338 landing first, since that removes the last caller.

🤖 Generated with Claude Code

Summary by CodeRabbit

Breaking Changes
- Removed the account-level private key derivation API.
- Private key derivation is no longer performed through account objects.

coderabbitai · 2026-08-08T08:21:03Z

Warning

Review limit reached

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

Next review available in:25 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 Plus

Run ID: edbd2997-f81a-40b6-ae8d-9131a67a3f2f

📥 Commits

Reviewing files that changed from the base of the PR and between 0556874 and ae0c34f.

📒 Files selected for processing (1)

packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/Account.swift

📝 Walkthrough

Walkthrough

The Swift Account documentation now describes FFI handle ownership and account behavior. The account-level derivePrivateKeyWIF(wallet:masterPath:index:) method and its derivation logic were removed.

Changes

Account API cleanup

Layer / File(s)	Summary
Account contract and API removal `packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/Account.swift`	The documentation now describes FFI handle ownership and account existence. The account-level private-key derivation API was removed in favor of provider-key or explicit-path wallet APIs.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

dashpay/platform#4334: This PR also modifies Account.derivePrivateKeyWIF in Account.swift.

Suggested reviewers:shumkov, llbartekll, zocolini

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 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 describes the removal of the unused Account.derivePrivateKeyWIF API.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch chore/remove-account-derive-private-key-wif

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

thepastaclaw · 2026-08-08T08:21:06Z

🕓 Ready for review — 2 ahead in queue (commit 0556874)
Queue position: 3/3
ETA: start ~08:54 UTC · complete ~09:09 UTC (median 14m across 30 recent reviews; 2 slots)
Queued 18m ago · Last checked: 2026-08-08 08:40 UTC

coderabbitai

Actionable comments posted: 1

🤖 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.
Inline comments:
In `@packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/Account.swift`:
- Around line 6-8: Update the documentation comment for Account to describe that
it deliberately exposes no account-specific public operations, while retaining
the explanation of getAccount(type:) and handle cleanup. Do not state that
Account has no members, since it stores private state such as the FFI handle and
wallet.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 143ef858-41f0-4534-ae6b-a6232143d9be

📥 Commits

Reviewing files that changed from the base of the PR and between 8f98180 and 0556874.

📒 Files selected for processing (1)

packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/Account.swift

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>

QuantumExplorer requested review from ZocoLini, llbartekll and shumkov as code owners August 8, 2026 08:20

github-actionsBot added this to the v4.2.0 milestone Aug 8, 2026

coderabbitaiBot reviewed Aug 8, 2026
View reviewed changes

Comment threadpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/Account.swift Outdated

QuantumExplorer force-pushed the chore/remove-account-derive-private-key-wif branch from 0556874 to ae0c34fCompare August 8, 2026 08:45

QuantumExplorer merged commit 3bf802f into v4.2-devAug 8, 2026
16 checks passed

QuantumExplorer deleted the chore/remove-account-derive-private-key-wif branch August 8, 2026 08:47

romchornyi mentioned this pull request Aug 8, 2026
fix(wallet): derive masternode owner/voting keys through the Rust-side resolver dashpay/dashwallet-ios#937
Merged
5 tasks

QuantumExplorer mentioned this pull request Aug 8, 2026
feat(voting): derive keys through the platform wallet; rework node selection dashpay/dashwallet-ios#936
Merged

Navigation Menu

Uh oh!

chore(swift-sdk): remove Account.derivePrivateKeyWIF - #4339

chore(swift-sdk): remove Account.derivePrivateKeyWIF#4339
QuantumExplorer merged 1 commit into
v4.2-devfrom
chore/remove-account-derive-private-key-wif

QuantumExplorer commented Aug 8, 2026•
edited by coderabbitai Bot
Loading

Uh oh!

coderabbitaiBot commented Aug 8, 2026•
edited
Loading

Review limit reached

Walkthrough

Changes

Uh oh!

thepastaclaw commented Aug 8, 2026•
edited
Loading

Uh oh!

coderabbitaiBot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

QuantumExplorer commented Aug 8, 2026• edited by coderabbitai BotLoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why it's unused

Why delete rather than fix

Replacements

Coverage

Testing

Summary by CodeRabbit

Uh oh!

coderabbitaiBot commented Aug 8, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Uh oh!

thepastaclaw commented Aug 8, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitaiBot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

QuantumExplorer commented Aug 8, 2026•
edited by coderabbitai Bot
Loading

coderabbitaiBot commented Aug 8, 2026•
edited
Loading

thepastaclaw commented Aug 8, 2026•
edited
Loading