Skip to content

Commit abde130

Browse files
llbartekllclaude
andcommitted
fix(swift-sdk): only claim "newer build" where the evidence has a direction
A hash disagreement is symmetric — it says the store's shape of an entity is not the live model's, not which came first. V1/V2/V3 are unfrozen, so adding one attribute to any live model makes every EXISTING store disagree on that entity: an older store, reported as a newer one, with "reset the wallet" as the offered remedy. An unregistered version identifier is ambiguous the same way (pre-V1, or a version since dropped from the plan). Both become `.unplaceable`, which rethrows SwiftData's own error. The one asymmetric fact left — the store carries an entity this schema does not have — keeps `newerThanRegistered`, and with it the only honest "update the app". Last round's cancellation fix overshot: latching above the handle guard also latched on the guard's deliberately uncached no-op return, so a manager built and shut down before `configure()` could never produce a support export again. The latch is now as conditional as the state it accompanies — a live handle, or a pass actually in flight, counted for both halves. The flag was also polled at only three points, so `shutdown()`'s drain could wait for the whole-wallet fingerprint, the per-account snapshots and `logTxoAnomalies` — the stages that dominate the queue hold. Each is gated now, and the audit's check no longer hides inside `if let allTransactions`, where it was skipped exactly when the audit had already been declined. The #4438 audit knew only BIP44 addresses, so a mixed send's own CoinJoin change was booked as "unattributed" — documented as peers' outputs — and a CoinJoin-side output missing from PersistentTxo was invisible: a third route to the false all-clear. The pool now covers CoinJoin accounts, the account check compares against the account the pool named, and the missing counts are reported per side. Also: every early return out of the memory half now emits its `diff_incomplete=true` summary, since an absent summary reads as a truncated log; and the export entry point documents that its only caller is the host app, by design. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent d86a763 commit abde130

4 files changed

Lines changed: 243 additions & 69 deletions

File tree

‎packages/swift-sdk/Sources/SwiftDashSDK/Persistence/DashModelContainer.swift‎

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,13 @@ public enum DashModelContainer {
7373
/// lists (every v4.2.0-dev.1 store, until the remaining V1/V2 shapes
7474
/// are frozen). Inferred migration may open it.
7575
case driftedRegisteredVersion
76-
/// Written by a build this SDK does not know — a version identifier
77-
/// it never registered, or an entity its schema lacks. Inferred
78-
/// migration would open it and silently drop what the newer build
79-
/// wrote, so it must not run; the pre-fallback crash was the safe
80-
/// outcome here.
76+
/// Positively written by a NEWER build: the store carries an entity
77+
/// this schema does not have, which nothing older could have created.
78+
/// Inferred migration would open it and silently drop that entity's
79+
/// table, so it must not run; the pre-fallback crash was the safe
80+
/// outcome here. This is the only verdict the host may turn into
81+
/// "update the app", because it is the only one whose evidence has a
82+
/// direction — see `unplaceable` for why a hash disagreement does not.
8183
case newerThanRegistered(reason: String)
8284
/// The metadata reads but does not place the store against any
8385
/// registered version: no declared version identifier, or nothing to
@@ -170,12 +172,20 @@ public enum DashModelContainer {
170172
) -> StoreSchemaVerdict {
171173
if matchesRegisteredVersion { return .matchesRegisteredVersion }
172174

175+
// Nothing to place the store against — every schema failed to build a
176+
// model. Not a fact about the store at all.
177+
guard !registered.isEmpty else {
178+
return .unplaceable(reason: "no_registered_models")
179+
}
180+
173181
// SwiftData writes each `VersionedSchema.versionIdentifier` into the
174-
// store; one this plan never registered was written by a newer build,
175-
// and a store carrying none cannot be placed at all.
182+
// store. One this plan does not register places it nowhere, but says
183+
// nothing about which side is older: a pre-V1 store and a version
184+
// since dropped from `DashMigrationPlan.schemas` look exactly like a
185+
// future one from here.
176186
let registeredIdentifiers = Set(registered.map(\.identifier))
177187
if let unknown = storeVersionIdentifiers.first(where: { !registeredIdentifiers.contains($0) }) {
178-
return .newerThanRegistered(reason: "unregistered_version_identifier=\(unknown)")
188+
return .unplaceable(reason: "unregistered_version_identifier=\(unknown)")
179189
}
180190
// No identifier at all places the store nowhere. Older stores and
181191
// stores with truncated metadata land here too, so this is not a
@@ -193,7 +203,9 @@ public enum DashModelContainer {
193203
}
194204

195205
// An entity the current schema does not have can only have been
196-
// written by a newer build; inferred migration would drop its table.
206+
// written by a newer build — this is the one asymmetric fact
207+
// available here, so it is the one verdict allowed to say "newer".
208+
// Inferred migration would drop its table.
197209
let unknownEntities = Set(storeEntityHashes.keys).subtracting(currentEntities).sorted()
198210
if !unknownEntities.isEmpty {
199211
return .newerThanRegistered(
@@ -229,7 +241,16 @@ public enum DashModelContainer {
229241
guard !unexpectedDrift.isEmpty else {
230242
return .unplaceable(reason: "no_entity_disagreement")
231243
}
232-
return .newerThanRegistered(
244+
// Deliberately NOT `newerThanRegistered`. A hash disagreement is
245+
// symmetric: it says the store's shape of that entity is not the live
246+
// model's, not which of the two came first. V1/V2/V3 are still
247+
// unfrozen (only `PersistentAssetLock` is frozen), so adding one
248+
// attribute to any live model makes every EXISTING store disagree on
249+
// that entity — an older store, reported as a newer one, with a reset
250+
// offered as the remedy. Direction needs evidence this comparison does
251+
// not have; until the models are frozen, the honest verdict is that
252+
// the store cannot be placed.
253+
return .unplaceable(
233254
reason: "unexpected_entity_drift=\(unexpectedDrift.sorted().joined(separator: "|"))"
234255
)
235256
}

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

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -473,11 +473,34 @@ public class PlatformWalletManager: ObservableObject {
473473
/// must not participate in `ensureSyncNativeOpAllowed`.
474474
private var activeCoreDiagnosticsNativeOpCount = 0
475475
/// Set by `shutdown()` before it drains `activeCoreDiagnosticsNativeOpCount`.
476-
/// A diagnostic pass checks it before every FFI read, so the drain waits
477-
/// for at most the one read already in flight — never for the rest of an
476+
/// A diagnostic pass checks it before every stage, so the drain waits for
477+
/// at most the one stage already in flight — never for the rest of an
478478
/// export — and a support export can never outlive the process it is
479-
/// diagnosing. Never reset: a manager is shut down once.
479+
/// diagnosing. Never reset: a manager is shut down once, and a pass that
480+
/// has been told to stop must not be able to un-tell itself.
481+
///
482+
/// Which is why `shutdown()` raises it only when there is a real teardown
483+
/// or a pass to stop — see the call site. A never-configured manager takes
484+
/// an UNCACHED no-op return from `shutdown()` precisely so it can still be
485+
/// configured afterwards, and a one-way latch set on that path would
486+
/// silence diagnostics for the rest of a perfectly live manager's life.
480487
let coreDiagnosticsCancellation = CoreDiagnosticsCancellation()
488+
489+
/// Diagnostic passes in flight, including the SwiftData half that runs
490+
/// with no handle at all. `activeCoreDiagnosticsNativeOpCount` cannot
491+
/// stand in for this: it counts only admitted FFI work, so it is zero in
492+
/// exactly the unconfigured case whose database half still needs to be
493+
/// told to stop.
494+
private var activeCoreDiagnosticsPassCount = 0
495+
496+
func beginCoreDiagnosticsPass() {
497+
activeCoreDiagnosticsPassCount += 1
498+
}
499+
500+
func endCoreDiagnosticsPass() {
501+
guard activeCoreDiagnosticsPassCount > 0 else { return }
502+
activeCoreDiagnosticsPassCount -= 1
503+
}
481504
private var nativeOpDrainContinuations: [CheckedContinuation<Void, Never>] = []
482505

483506
/// Admission + bookkeeping shared by the async native entrypoints:
@@ -678,9 +701,18 @@ public class PlatformWalletManager: ObservableObject {
678701
// configured (`emitCoreWalletDiagnostics` runs its database half
679702
// with no handle), and the early return below would leave it with
680703
// no way to be told to stop — holding the queue, and every Rust
681-
// persister callback entering through it, across teardown. The
682-
// flag is one-way and costs nothing on the no-op path.
683-
coreDiagnosticsCancellation.cancel()
704+
// persister callback entering through it, across teardown.
705+
//
706+
// But only as conditionally as the shutdown state it accompanies:
707+
// the guard's no-op return is deliberately UNCACHED so a manager
708+
// built and shut down before `configure()` can still be configured
709+
// later, and this latch is one-way, so raising it there would leave
710+
// that live manager unable to produce a support export ever again.
711+
// A real teardown, or a pass actually in flight, is the whole set
712+
// of cases with something to cancel.
713+
if handle != NULL_HANDLE || activeCoreDiagnosticsPassCount > 0 {
714+
coreDiagnosticsCancellation.cancel()
715+
}
684716
guard handle != NULL_HANDLE else {
685717
// Never configured (or a test double without a handle):
686718
// nothing to tear down. Do not cache this no-op: a manager

0 commit comments

Comments
 (0)