refactor(swift-sdk)!: remove dead public API in SwiftDashSDK (6 audit entries) - #4640
Closed
llbartekll wants to merge 5 commits into
Closed
refactor(swift-sdk)!: remove dead public API in SwiftDashSDK (6 audit entries)#4640llbartekll wants to merge 5 commits into
llbartekll wants to merge 5 commits into
Conversation
…nager classes and their README `PlatformWallet.fromSeed` / `.fromMnemonic` and `getIdentityManager(for:)` have hits only in three SwiftTests files. Neither SwiftExampleApp nor the SDK itself ever constructs these classes — the real entry point is `PlatformWalletManager` → `ManagedPlatformWallet` → sub-wallets. `PlatformWallet/README.md` (600 lines) documented that dead path in its entirety and had drifted from the code: `getPrimaryIdentityId`/`setPrimaryIdentity` (removed — IdentityManager.swift says so itself), `identity.sendContactRequest(recipientId:senderKeyIndex:)` (no such signature), `Identifier` as a struct with `.bytes`/`.hexString` (it is `typealias Identifier = Data`), and a `PlatformWalletError` enum that does not match `PlatformWalletResult.swift`. The module's only README taught the dead API. The removed tests are `XCTAssertNotNil` on non-optionals and handle-inequality checks. `Package.swift` loses the `exclude` entry for the deleted README. BREAKING CHANGE: `PlatformWallet` and `IdentityManager` were public in the SwiftDashSDK product. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…egacy keychain helpers, control-rule enum, trivial mutators - `WalletStorage.enableBiometricProtection` / `retrieveSeedWithBiometric` plus `biometricKeychainAccount` and the `LocalAuthentication` import: the file header itself admitted "not yet wired to a caller". The biometric path stored a raw seed under a different accessibility policy from everything else — public API with no consumer that still had to be kept correct. The `biometricSetupFailed`/`biometricAuthenticationFailed` error cases go with it. - `KeychainManager.deleteAllPrivateKeys(for:)`, `deleteSpecialKey`, `hasSpecialKey`: no call sites (the only hit in the app is a comment). - `storePrivateKeyNonisolated` folded back into `storePrivateKey`, which is now `nonisolated` directly. Its doc claimed `PlatformWalletPersistenceHandler` calls it — the only caller was its own `@MainActor` wrapper. - `ControlRuleType` and `PersistentToken.tokensWithControlRulePredicate`: a 36-line switch of identical predicates with no callers. - `PersistentIdentity.updateRevision` / `updateDPNSName` / `removePublicKey` and `PersistentDataContract.updateVersion` / `addDocument` / `removeDocument`: no call sites. The identically-named frozen copies in `DashSchemaFrozenModels.swift` are left untouched. Out of scope, contrary to the audit entry: - `PersistentDocumentType.persistentProperties` has four real call sites in `DocumentTypeDetailsView` — not dead code. - The `SwiftExampleApp/Services/KeychainManager.swift` shim has 24 call sites, not two; removing it is a global rename. BREAKING CHANGE: the removed symbols were public in the SwiftDashSDK product. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…recated wrapper and the tokenTransfer stub Two audit entries in one commit, because both edit `FFI/StateTransitionExtensions.swift`. Dead helpers and a one-conformer protocol: - `encodeContractField` and `withOptionalCStrings` in StateTransitionExtensions were `fileprivate` and documented as "Used by dataContractCreate" — which lives in SDK.swift and has its own `withOptionalCStrings`. The duplicate invited the two copies to diverge. - `protocol Signer` has one conformer, and its only requirement `canSign(identityPublicKey:)` has zero callers — the live one is the two-argument `canSign(publicKey:keyType:)` on `KeychainSigner`. The file called itself "legacy" and told new code not to use it. - `Addresses.getBalance` / `getNonce` / `exists` / `getTotalBalance`: no call sites in SwiftExampleApp or SwiftTests. Deprecated and stub surface: - `ManagedCoreWallet.broadcastTransaction(_:)` — `@available(*, deprecated)` with no Swift caller; `broadcastTransactionWithOutcome(_:)` is the live one. - `FinalizedCoreTransaction.takeForAbandon()` — an alias for `takeForBroadcast()` with one caller, inlined at the call site. - `SDK.tokenTransfer(tokenId:fromIdentityId:toIdentityId:amount:)` — public API that always throws `notImplemented`. A stub in a public SDK is a trap for its users. BREAKING CHANGE: the removed symbols were public in the SwiftDashSDK product. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- `Helpers/TestKeyGenerator.swift` — an `internal struct` with no reference anywhere in Sources, byte-identical to the SwiftExampleApp copy (also unreferenced, removed in the app-side commit). - `ConcurrencyCompat.swift` — `SendableOpaquePointer` has no uses, and the `#if compiler(<6.2)` block with the retroactive `OpaquePointer: Sendable` is compiled out on the installed Swift 6.3.3 toolchain. The file contributed nothing to the build. - The `ErrorCode` / `SDKConfig` typealiases in SwiftDashSDK.swift — no uses. `@_exported import DashSDKFFI` stays, since clients deliberately reach for the raw C types. Out of scope, contrary to the audit entry — these items HAVE consumers, so they are not dead code: - `Config/TestnetNodes.swift`: `TestnetNodesLoader` and `loadFromYAML` are called from `LoadIdentityView`. Removing them cuts a working app feature (even if it runs on synthetic data) rather than cleaning up. - `DataManager.saveTokenBalance` / `fetchTokenBalances`: used by `TokenBalanceUInt64PersistenceTests` — test-only, not dead. BREAKING CHANGE: the removed typealiases were public in the SwiftDashSDK product. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…Handler - `addDelta` — the balance-delta consumer it served was removed (a comment a few dozen lines above says so); only the definition hit remained. - `LoadAllocation.cStringPointerArrays` — never appended to, only released in `release()`. - `entriesCount` across six allocation classes (`LoadAllocation` plus five `Shielded*LoadAllocation`) — assigned in six places, read in none. The release path's real counter is `entriesInitialized`; its doc now states the contract without referring to the removed field. - `IdentityKeyEntrySnapshot.publicKeyHash` — filled via `dataFromTuple20` and never read. The `dataFromTuple20` helper goes with it. - The `// MARK: - Watch-only Restore: Wallet Metadata` header over an empty section. Vestigial fields implied contracts that no longer exist — a reader would assume `publicKeyHash` is persisted. `IdentityEntrySnapshot.status` / `.label` from the same audit entry are KEPT: removing them requires rewriting initialisers in DashPayPersistenceTests and is more entangled than the rest of the entry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
llbartekll
requested review from
QuantumExplorer,
ZocoLini and
shumkov
as code owners
September 9, 2026 12:03
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
5 tasks
Collaborator
|
🕓 Queued for automated review — 17th in line, estimated start in ~6 h (commit 3b10eef)
|
llbartekll
changed the title
llbartekll
force-pushed
the
refactor/wallet-dead-code-swift-app
branch
from
September 9, 2026 12:14
3da3136 to
29421fe
Compare
llbartekll
force-pushed
the
refactor/wallet-dead-code-swift-sdk
branch
from
September 9, 2026 12:14
4537473 to
3b10eef
Compare
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
Third part of the cleanup following the 2026-09-08 audit (
dead-codecategory). Scope: swift-sdk, Effort S, Risk low — 6 entries.What was done?
PlatformWallet/IdentityManagerclasses,PlatformWallet/README.md(600 lines) and three test files. The real entry point isPlatformWalletManager→ManagedPlatformWallet→ sub-wallets; the README taught the dead path and had drifted from the code (getPrimaryIdentityIdremoved,Identifierdescribed as a struct when it istypealias Identifier = Data)WalletStorage("not yet wired to a caller", per its own header),deleteAllPrivateKeys(for:),deleteSpecialKey,hasSpecialKey,ControlRuleType+tokensWithControlRulePredicate, and the trivial mutators onPersistentIdentity/PersistentDataContract;storePrivateKeyNonisolatedfolded back intostorePrivateKey, which is nownonisolateddirectlyencodeContractField/withOptionalCStrings(a duplicate ofSDK.swift's), the one-conformerSignerprotocol whose sole requirement has no callers, fourAddressesconvenience methods, the deprecatedbroadcastTransaction(_:), thetakeForAbandon()alias, and thetokenTransferstub that always throwsnotImplementedHelpers/TestKeyGenerator.swift(twin of the app copy removed in #4639),ConcurrencyCompat.swift(its#if compiler(<6.2)block is compiled out on Swift 6.3.3 — the file contributed nothing), and theErrorCode/SDKConfigtypealiasesaddDelta,cStringPointerArrays,entriesCountacross six allocation classes (assigned 6×, read 0×),IdentityKeyEntrySnapshot.publicKeyHash+dataFromTuple20, and an empty MARK headerTotal: 23 files, 1,439 deletions, 19 insertions.
074 and 077 share a commit because both edit
FFI/StateTransitionExtensions.swift.Where this departs from the audit
Four audit claims were rejected — these items have consumers, so they are not dead code:
PersistentDocumentType.persistentProperties— the audit itself says "1 caller"; grep finds 4 call sites inDocumentTypeDetailsView.Config/TestnetNodes.swift—TestnetNodesLoaderandloadFromYAMLare called fromLoadIdentityView. Removing them cuts a working app feature (even if it runs on synthetic data), which is not cleanup.DataManager.saveTokenBalance/fetchTokenBalances— used byTokenBalanceUInt64PersistenceTests. Test-only, not dead.SwiftExampleApp/Services/KeychainManager.swiftshim — the audit says "two call sites"; there are 24. It is an alias in active use, and removing it is a global rename.Worth flagging at review:
updateRevision/updateDPNSName/removePublicKeyalso exist inDashSchemaFrozenModels.swiftas frozen schema copies. Those are untouched — only thePersistence/Models/counterparts were removed.tokensWithControlRulePredicatehas a live counterpart inkotlin-sdk/.../dao/TokenDao.kt(6 hits). Removing the Swift side widens the Swift/Kotlin divergence — leaving that judgement to reviewers.How Has This Been Tested?
xcodebuild -scheme SwiftDashSDK -sdk iphonesimulator✅xcodebuild -project SwiftExampleApp.xcodeproj -scheme SwiftExampleApp -sdk iphonesimulator✅(Builds ran on
iPhone 17— theiPhone 16destination inCLAUDE.mddoes not exist on the test machine.)Breaking Changes
Yes. The removed symbols were public in the
SwiftDashSDKSwiftPM product. No in-repo consumers were found, but an out-of-repo app linking this module will fail to compile. Per the task owner's decision these are removed outright, with no@available(*, deprecated)grace period.Code comments point at two possible external consumers — Yappr (
swift-sdk-036) and dashwallet-ios (TestnetFaucet). Worth confirming before merge.Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code