fix(swift-sdk): address PR #3765 review feedback

- WalletKeyHealthSheet: branch HASH160 keys against pubkey-hash hex
instead of comparing the 33-byte derived pubkey to the 20-byte
stored hash; report unsupported key types as orphan with a clear
reason. Rederive returns per-key failures so the sheet can show
WHICH key is stuck. deleteOrphan also wipes the identity's
Keychain entries.
- KeychainManager: deleteIdentityPrivateKey now takes walletId
(symmetric with store) and also sweeps the legacy account.
Added deleteAllIdentityPrivateKeys(forIdentityIdBase58:).
- AddIdentityKeyView: system contract entries always win over
user-saved rows that share an ID, so DashPay's bounded document-
type metadata isn't masked by a parallel saved-contract entry.
- CreateIdentityView: shouldRegisterDashPayKeys computed gate
mirrors the UI section visibility; submit() and the funding-min
calc both route through it. KeyValidation.validatePrivateKeyForPublicKey
added to makeDashpayKeyPair before persisting.
- PlatformWalletManager.deleteWallet: keychain cleanup moved BEFORE
deleteWalletData so a partial-failure retry can still find the
identityIds to purge.
- StorageRecordDetailViews: AccountStorageDetailView now uses the
walletLabel(record.wallet) helper.
- SDK.swift: platform_version=11 pin has an explicit TODO + trigger
describing when to bump it.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: QuantumExplorer

packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift

@@ -544,6 +544,22 @@ public class PlatformWalletManager: ObservableObject {
 
 letidentityIds=try persistenceHandler.identityIdsForWallet(walletId: walletId)
 
+ // Wipe Keychain BEFORE the SwiftData identity deletion runs.
+ // Order matters for retry-safety: if `deleteWalletData`
+ // commits identity rows and then throws partway, a retry
+ // would see `identityIdsForWallet == []` and the
+ // `deleteAllKeychainItems(forIdentityId:)` sweep below
+ // could no longer find the keys to purge. Doing the
+ // keychain side first leaves at worst stale SwiftData
+ // rows on a retry — repeating the wipe is harmless, and
+ // every keychain call here is idempotent (no-op on "not
+ // found"). Mnemonic / metadata stay in `WalletStorage`
+ // for now so a retry can still derive any missed key.
+foridentityIdin identityIds {
+tryKeychainManager.shared.deleteAllKeychainItems(forIdentityId: identityId)
+}
+tryKeychainManager.shared.deleteAllIdentityPrivateKeys(forWalletId: walletId)
+
 try walletId.withUnsafeBytes{ raw in
 guardlet base = raw.baseAddress?.assumingMemoryBound(to:FFIByteTuple32.self)else{
 throwPlatformWalletError.nullPointer(
@@ -557,11 +573,6 @@ public class PlatformWalletManager: ObservableObject {
 
 try persistenceHandler.deleteWalletData(walletId: walletId)
 
-foridentityIdin identityIds {
-tryKeychainManager.shared.deleteAllKeychainItems(forIdentityId: identityId)
-}
-tryKeychainManager.shared.deleteAllIdentityPrivateKeys(forWalletId: walletId)
-
 letstorage=WalletStorage()
  // Delete metadata first so the mnemonic remains available for retry.
 try storage.deleteMetadata(for: walletId)

packages/swift-sdk/Sources/SwiftDashSDK/SDK.swift

@@ -255,6 +255,14 @@ public final class SDK: @unchecked Sendable {
 }else{
 switch network {
 case.mainnet,.testnet:
+ // TODO(platform-version-bump): bump mainnet/testnet to 12
+ // (or whatever PV is current) once drive-abci 3.1+ has
+ // rolled out on those networks and the new
+ // `getDocuments` V1 wire format is on by default. The
+ // trigger is: a HardFork shipping the V1 wire format
+ // becomes active on mainnet/testnet. Until then, pinning
+ // 11 keeps the SDK speaking the V0 protocol the active
+ // tenderdash quorums understand.
  resolvedPlatformVersion =11
 case.devnet,.regtest:
  resolvedPlatformVersion =12

packages/swift-sdk/Sources/SwiftDashSDK/Security/KeychainManager.swift

@@ -789,21 +789,96 @@ extension KeychainManager {
 returnfalse
 }
 
- /// Delete the identity private-key row for `derivationPath`.
- /// Idempotent; returns true on success or "not found".
+ /// Delete the identity private-key row for the
+ /// `(walletId, derivationPath)` pair — symmetric with
+ /// `storeIdentityPrivateKey` (which writes under the
+ /// `identity_privkey.<walletId>.<path>` account scheme).
+ ///
+ /// Idempotent; returns true on success or "not found". A
+ /// best-effort sweep of the legacy
+ /// (`identity_privkey.<path>` — no walletId) account is
+ /// included so callers don't end up with a half-migrated state
+ /// where the new-format row is gone but a legacy row at the
+ /// same path lingers.
 @discardableResult
-publicnonisolatedfunc deleteIdentityPrivateKey(derivationPath:String)->Bool{
-letaccount="identity_privkey.\(derivationPath)"
+publicnonisolatedfunc deleteIdentityPrivateKey(
+ walletId:Data,
+ derivationPath:String
+)->Bool{
+letwalletIdHex= walletId.toHexString()
+letnewAccount="identity_privkey.\(walletIdHex).\(derivationPath)"
+letlegacyAccount="identity_privkey.\(derivationPath)"
+varok=true
+foraccountin[newAccount, legacyAccount]{
+varquery:[String:Any]=[
+ kSecClass asString: kSecClassGenericPassword,
+ kSecAttrService asString: serviceName,
+ kSecAttrAccount asString: account,
+]
+iflet accessGroup = accessGroup {
+query[kSecAttrAccessGroup asString]= accessGroup
+}
+letstatus=SecItemDelete(query asCFDictionary)
+if status != errSecSuccess && status != errSecItemNotFound {
+ ok =false
+}
+}
+return ok
+}
+
+ /// Delete every `identity_privkey.*` keychain row whose
+ /// `IdentityPrivateKeyMetadata.identityId` matches
+ /// `identityIdBase58` — the base58 identity id Swift uses
+ /// throughout the persistence layer. Used by the key-health
+ /// sheet's "Delete orphan identity" action so cascading the
+ /// SwiftData row doesn't leave its keys' private bytes behind
+ /// in Keychain.
+ ///
+ /// Scans the metadata blob (`kSecAttrGeneric`) on every
+ /// matching keychain item — handles both new-format
+ /// (`identity_privkey.<walletId>.<path>`) and legacy-format
+ /// (`identity_privkey.<path>`) accounts uniformly. Idempotent;
+ /// no-op when nothing matches.
+publicnonisolatedfunc deleteAllIdentityPrivateKeys(forIdentityIdBase58 identityIdBase58:String)throws{
 varquery:[String:Any]=[
  kSecClass asString: kSecClassGenericPassword,
  kSecAttrService asString: serviceName,
- kSecAttrAccount asString: account,
+ kSecMatchLimit asString: kSecMatchLimitAll,
+ kSecReturnAttributes asString:true,
 ]
 iflet accessGroup = accessGroup {
 query[kSecAttrAccessGroup asString]= accessGroup
 }
-letstatus=SecItemDelete(query asCFDictionary)
-return status == errSecSuccess || status == errSecItemNotFound
+
+varresult:AnyObject?
+letstatus=SecItemCopyMatching(query asCFDictionary,&result)
+if status == errSecItemNotFound {
+return
+}
+guard status == errSecSuccess,let items = result as?[[String:Any]]else{
+throwKeychainError.retrieveFailed(status)
+}
+
+letdecoder=JSONDecoder()
+foritemin items {
+guardlet account =item[kSecAttrAccount asString]as?String,
+ account.hasPrefix("identity_privkey.")
+else{
+continue
+}
+guardlet metadataData =item[kSecAttrGeneric asString]as?Data,
+let metadata =try? decoder.decode(
+IdentityPrivateKeyMetadata.self,
+ from: metadataData
+)
+else{
+continue
+}
+guard metadata.identityId == identityIdBase58 else{
+continue
+}
+trydeleteGenericPassword(account: account)
+}
 }
 
  /// Delete every `identity_privkey.<derivationPath>` keychain row whose

packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletKeyHealthSheet.swift

@@ -189,9 +189,35 @@ enum WalletKeyHealthChecker {
  keyId: kid,
  network: network
 )
- derivedHex = preview.publicKeyHex
 
-if preview.publicKeyData == row.publicKeyData {
+ // `row.publicKeyData` stores whatever shape was
+ // registered on Platform — 33-byte compressed
+ // pubkey for `.ecdsaSecp256k1`, 20-byte HASH160
+ // for `.ecdsaHash160`. The Rust-derived preview
+ // is always the raw 33-byte pubkey; hash it
+ // ourselves before comparing for HASH160 rows.
+ // (Other variants — BLS, BIP13 script-hash,
+ // EdDSA — aren't produced by this preview path
+ // today; treat them as `notSupported` so the
+ // diagnostic surfaces them instead of silently
+ // misclassifying them as orphans.)
+letderivedComparableHex:String?
+switch row.keyTypeEnum ??.ecdsaSecp256k1 {
+case.ecdsaSecp256k1:
+ derivedComparableHex = preview.publicKeyHex
+ derivedHex = preview.publicKeyHex
+case.ecdsaHash160:
+lethashHex=SwiftDashSDK.KeychainManager
+.computePublicKeyHashHex(preview.publicKeyData)
+ derivedComparableHex = hashHex.isEmpty ?nil: hashHex
+ derivedHex = hashHex
+case.bls12_381,.bip13ScriptHash,.eddsa25519Hash160:
+ derivedComparableHex =nil
+ derivedHex = preview.publicKeyHex
+}
+
+iflet derivedComparableHex,
+ derivedComparableHex.caseInsensitiveCompare(storedHex)==.orderedSame {
  // pubkey matches the wallet's mnemonic →
  // look up the keychain bytes at the expected
  // walletId-namespaced account.
@@ -224,9 +250,20 @@ enum WalletKeyHealthChecker {
  reason:"No new-format Keychain entry (legacy-only entries don't count)"
 )
 }
+}elseif derivedComparableHex ==nil{
+ // Key type isn't one the diagnostic knows
+ // how to compare against a Rust-derived
+ // pubkey today (BLS / BIP13 / EdDSA). Report
+ // it as orphan with a clear reason so the
+ // user knows we can't verify it, rather than
+ // silently passing.
+letlabel= row.keyTypeEnum?.name ??"type \(row.keyType)"
+ status =.orphan(
+ reason:"Key type \(label) isn't supported by the diagnostic — can't verify against derived pubkey"
+)
 }else{
  status =.orphan(
- reason:"Stored pubkey \(storedHex.prefix(12))… doesn't match wallet's derivation \(derivedHex.prefix(12))…"
+ reason:"Stored \(storedHex.prefix(12))… doesn't match wallet's derivation \(derivedHex.prefix(12))…"
 )
 }
 }catch{
@@ -262,27 +299,53 @@ enum WalletKeyHealthChecker {
 return reports
 }
 
+ /// Per-key result of a rederive pass. `success` is the count of
+ /// keys whose Keychain entry was rewritten; `failures` lists each
+ /// key the pass tried and couldn't fix, with a reason — useful so
+ /// the sheet can show the user *which* key is stuck (not just
+ /// "Re-derived 0 keys").
+structRederiveOutcome{
+letsuccess:Int
+ /// `(keyId, reason)` for each `.needsRederive` key the loop
+ /// touched but did not fix.
+letfailures:[(UInt32,String)]
+}
+
  /// Re-derive every key in `report` whose status is
  /// `.needsRederive`, write fresh Keychain entries (at the new
  /// walletId-namespaced account), and update each
  /// `PersistentPublicKey.privateKeyKeychainIdentifier` to point
- /// at the new account. Returns the number of keys fixed.
+ /// at the new account.
+ ///
+ /// Returns a `RederiveOutcome` with both the count fixed and a
+ /// per-key list of failures. Individual key failures are
+ /// collected rather than thrown so one bad key doesn't block
+ /// repair of the rest of the identity's keys; throws only when
+ /// the whole batch is unrecoverable (e.g. the SwiftData save at
+ /// the end fails).
 @MainActor
 staticfunc rederive(
  report:WalletIdentityKeyHealthReport,
  wallet:ManagedPlatformWallet,
  walletId:Data,
  network:Network,
  modelContext:ModelContext
-)throws->Int{
+)throws->RederiveOutcome{
 varfixed=0
+varfailures:[(UInt32,String)]=[]
 forkeyin report.keys {
 guard case .needsRederive = key.status else{continue}
-letpreview=try wallet.deriveIdentityAuthKeyAtSlot(
- identityIndex: report.identityIndex,
- keyId: key.keyId,
- network: network
-)
+letpreview:ManagedPlatformWallet.IdentityRegistrationKeyPreview
+do{
+ preview =try wallet.deriveIdentityAuthKeyAtSlot(
+ identityIndex: report.identityIndex,
+ keyId: key.keyId,
+ network: network
+)
+}catch{
+ failures.append((key.keyId,"derivation failed: \(error.localizedDescription)"))
+continue
+}
 letpubkeyHashHex=SwiftDashSDK.KeychainManager.computePublicKeyHashHex(preview.publicKeyData)
 letmetadata=IdentityPrivateKeyMetadata(
  identityId: report.identityIdBase58,
@@ -302,6 +365,7 @@ enum WalletKeyHealthChecker {
  derivationPath: preview.derivationPath,
  metadata: metadata
 )else{
+ failures.append((key.keyId,"Keychain write at \(preview.derivationPath) returned nil"))
 continue
 }
  key.row.privateKeyKeychainIdentifier = pkid
@@ -321,22 +385,36 @@ enum WalletKeyHealthChecker {
 if fixed >0{
 try modelContext.save()
 }
-return fixed
+returnRederiveOutcome(success:fixed, failures: failures)
 }
 
- /// Cascade-delete an orphan identity from SwiftData. Safe to
- /// call now that the relationship inverses
- /// (`PersistentPublicKey.identity`, `PersistentDPNSName.identity`,
- /// `PersistentDashpayProfile.identity`, `PersistentDashpayContactRequest.owner`)
- /// are all Optional — see the doc comments on those models for
- /// the SwiftData cascade-on-non-optional crash this avoids.
+ /// Cascade-delete an orphan identity from SwiftData AND wipe its
+ /// associated Keychain entries. Order matters: clear the
+ /// Keychain side first (purely additive to the SwiftData state),
+ /// then drop the row. If Keychain wipe fails we still try the
+ /// SwiftData delete so the user can finish the operation; the
+ /// keychain error is surfaced to the caller.
 @MainActor
 staticfunc deleteOrphan(
  identity:PersistentIdentity,
  modelContext:ModelContext
 )throws{
+ // Snapshot the base58 id BEFORE the delete — once the row
+ // is removed its computed accessor is invalid.
+letidentityIdBase58= identity.identityIdBase58
+varkeychainError:Error?
+do{
+tryKeychainManager.shared.deleteAllIdentityPrivateKeys(
+ forIdentityIdBase58: identityIdBase58
+)
+}catch{
+ keychainError = error
+}
  modelContext.delete(identity)
 try modelContext.save()
+iflet keychainError {
+throw keychainError
+}
 }
 }
 
@@ -550,15 +628,23 @@ struct WalletKeyHealthSheet: View {
 privatefunc rederive(_ report:WalletIdentityKeyHealthReport){
 Task{@MainActorin
 do{
-letfixed=tryWalletKeyHealthChecker.rederive(
+letoutcome=tryWalletKeyHealthChecker.rederive(
  report: report,
  wallet: wallet,
  walletId: walletId,
  network: network,
  modelContext: modelContext
 )
- actionMessage ="Re-derived \(fixed) key\(fixed ==1?"":"s") for identity \(report.identityIdBase58.prefix(12))…"
- errorMessage =nil
+letfixed= outcome.success
+varmsg="Re-derived \(fixed) key\(fixed ==1?"":"s") for identity \(report.identityIdBase58.prefix(12))…"
+if !outcome.failures.isEmpty {
+letdetail= outcome.failures
+.map{"kid \($0.0): \($0.1)"}
+.joined(separator:"; ")
+ msg +=" — \(outcome.failures.count) failed: \(detail)"
+}
+ actionMessage = msg
+ errorMessage = outcome.failures.isEmpty ?nil: msg
  // Re-run the check so the report reflects the new
  // state (formerly-orange rows should turn green).
 awaitrunCheck()