fix: stop the test suite wiping the simulator's wallet

.github/workflows/integration-tests.yml

@@ -62,7 +62,15 @@ jobs:
           echo "⏱️  Starting integration tests at $(date)"
 
           run_tests() {
-            set -o pipefail && xcodebuild test \
+            # RNMigrationCleanupTests deletes the real ~/Documents/mmkv and ~/Documents/ldk, so it
+            # skips itself unless this is set. This lane runs on a simulator it erases between
+            # attempts; the unit lane does not, and must not run it.
+            #
+            # The TEST_RUNNER_ prefix is required: xcodebuild does not pass the invoking shell's
+            # environment to the simulator-hosted runner, only variables named TEST_RUNNER_<VAR>,
+            # which it forwards with the prefix stripped. Without it the guard never sees the
+            # variable, every test in the suite skips, and the lane still reports green.
+            set -o pipefail && TEST_RUNNER_BITKIT_DESTRUCTIVE_TESTS=1 xcodebuild test \
               -scheme Bitkit \
               -destination 'platform=iOS Simulator,name=iPhone 16,OS=latest' \
               -enableCodeCoverage NO \
@@ -72,6 +80,7 @@ jobs:
               -only-testing:BitkitTests/PaymentFlowTests \
               -only-testing:BitkitTests/BlocktankRefundAddressLiveIntegrationTests \
               -only-testing:BitkitTests/AddressTypeIntegrationTests \
+              -only-testing:BitkitTests/RNMigrationCleanupTests \
               | xcbeautify --report junit
           }
 

.github/workflows/unit-tests.yml

@@ -70,12 +70,12 @@ jobs:
             -destination 'platform=iOS Simulator,name=iPhone 16,OS=latest' \
             -enableCodeCoverage NO \
             -parallel-testing-enabled NO \
-            -skip-testing:BitkitTests/TxBumpingTests \
             -skip-testing:BitkitTests/UtxoSelectionTests \
             -skip-testing:BitkitTests/BlocktankTests \
             -skip-testing:BitkitTests/PaymentFlowTests \
             -skip-testing:BitkitTests/BlocktankRefundAddressLiveIntegrationTests \
             -skip-testing:BitkitTests/AddressTypeIntegrationTests \
+            -skip-testing:BitkitTests/RNMigrationCleanupTests \
             | xcbeautify --report junit
           echo "✅ Unit tests completed at $(date)"
 

Bitkit/Utilities/InstallationMarker.swift

@@ -16,7 +16,12 @@ enum InstallationMarker {
 
     /// App sandbox Documents directory (NOT app group) - gets deleted on uninstall
     private static var sandboxDocumentsUrl: URL {
-        FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0]
+        let documents = FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0]
+        // Tests delete the marker, and a missing marker makes the next real launch treat the keychain
+        // as orphaned and wipe it (`AppScene.handleOrphanedKeychain`). Give them their own directory so
+        // that only ever happens to a marker a test created. `Env.appStorageUrl` namespaces the app
+        // group the same way; this file is deliberately outside it, so it needs its own redirect.
+        return Env.isUnitTest ? documents.appendingPathComponent("unit-tests") : documents
     }
 
     static var markerPath: URL {
@@ -32,6 +37,7 @@ enum InstallationMarker {
     /// Should be called after handling any orphaned keychain detection
     static func create() throws {
         let data = UUID().uuidString.data(using: .utf8)!
+        try FileManager.default.createDirectory(at: sandboxDocumentsUrl, withIntermediateDirectories: true)
         try data.write(to: markerPath)
         Logger.info("Installation marker created", context: "InstallationMarker")
     }

Bitkit/Utilities/Keychain.swift

@@ -32,14 +32,27 @@ enum KeychainEntryType {
 }
 
 class Keychain {
+    private static let unitTestAccountPrefix = "unit-tests."
+
+    /// Under test, entries live under a prefixed account name so a suite cannot read, overwrite or
+    /// delete the wallet belonging to whoever is running it. `BitkitTests` is hosted in the app and
+    /// `Env.network` resolves to regtest for both, so tests and a Debug build otherwise share one
+    /// keychain access group. This mirrors what `Env.appStorageUrl` already does for file storage,
+    /// and only ever narrows what a process can see — the access group stays pinned either way.
+    /// Internal rather than private so tests that build their own `SecItem` queries can address the
+    /// same entry `save`/`load` use.
+    class func account(for key: KeychainEntryType) -> String {
+        Env.isUnitTest ? unitTestAccountPrefix + key.storageKey : key.storageKey
+    }
+
     class func save(key: KeychainEntryType, data: Data) throws {
         Logger.debug("Saving \(key.storageKey)", context: "Keychain")
 
         let query = [
             kSecClass as String: kSecClassGenericPassword as String,
             kSecAttrAccessible as String: kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly as String,
             kSecAttrSynchronizable as String: false,
-            kSecAttrAccount as String: key.storageKey,
+            kSecAttrAccount as String: account(for: key),
             kSecValueData as String: data,
             kSecAttrAccessGroup as String: Env.keychainGroup,
         ] as [String: Any]
@@ -87,7 +100,7 @@ class Keychain {
 
         let query = [
             kSecClass as String: kSecClassGenericPassword as String,
-            kSecAttrAccount as String: key.storageKey,
+            kSecAttrAccount as String: account(for: key),
             kSecAttrAccessGroup as String: Env.keychainGroup,
         ] as [String: Any]
 
@@ -130,7 +143,7 @@ class Keychain {
     class func delete(key: KeychainEntryType) throws {
         let query = [
             kSecClass as String: kSecClassGenericPassword as String,
-            kSecAttrAccount as String: key.storageKey,
+            kSecAttrAccount as String: account(for: key),
             kSecAttrAccessGroup as String: Env.keychainGroup,
         ] as [String: Any]
 
@@ -158,7 +171,7 @@ class Keychain {
         if existingData != nil {
             let searchQuery: [String: Any] = [
                 kSecClass as String: kSecClassGenericPassword,
-                kSecAttrAccount as String: key.storageKey,
+                kSecAttrAccount as String: account(for: key),
                 kSecAttrAccessGroup as String: Env.keychainGroup,
             ]
             let updateAttributes: [String: Any] = [
@@ -188,7 +201,7 @@ class Keychain {
     class func load(key: KeychainEntryType) throws -> Data? {
         let query = [
             kSecClass as String: kSecClassGenericPassword,
-            kSecAttrAccount as String: key.storageKey,
+            kSecAttrAccount as String: account(for: key),
             kSecReturnData as String: kCFBooleanTrue!,
             kSecMatchLimit as String: kSecMatchLimitOne,
             kSecAttrAccessGroup as String: Env.keychainGroup,
@@ -250,6 +263,12 @@ class Keychain {
     class func wipeEntireKeychain() throws {
         let keys = getAllKeyChainStorageKeys()
         for key in keys {
+            // A test run must only ever clear its own namespaced entries. Deleting an un-prefixed
+            // account here is what destroys the wallet on the simulator the suite runs against.
+            if Env.isUnitTest, !key.hasPrefix(unitTestAccountPrefix) {
+                continue
+            }
+
             let query = [
                 kSecClass as String: kSecClassGenericPassword as String,
                 kSecAttrAccount as String: key,

BitkitTests/AppStateIsolation.swift

@@ -39,6 +39,23 @@ extension XCTestCase {
         }
     }
 
+    /// Skips the test unless `BITKIT_DESTRUCTIVE_TESTS=1` is set. For the handful of suites that
+    /// deliberately operate on real, un-namespaceable state — the React-Native migration source under
+    /// `~/Documents`, for instance — and so can only run on a simulator that may be erased afterwards.
+    /// `integration-tests.yml` sets it; a plain `xcodebuild test` does not.
+    ///
+    /// To run one of these locally, set it as `TEST_RUNNER_BITKIT_DESTRUCTIVE_TESTS=1`. xcodebuild
+    /// forwards only variables named `TEST_RUNNER_<VAR>` into the simulator-hosted runner, stripping
+    /// the prefix; the unprefixed name never arrives, and the tests skip while the run reports green.
+    func skipUnlessDestructiveTestsEnabled(file: StaticString = #filePath, line: UInt = #line) throws {
+        try XCTSkipUnless(
+            ProcessInfo.processInfo.environment["BITKIT_DESTRUCTIVE_TESTS"] == "1",
+            "Destroys real on-disk state; set BITKIT_DESTRUCTIVE_TESTS=1 to run.",
+            file: file,
+            line: line
+        )
+    }
+
     /// Fails the test if it leaves any of `keys` in `UserDefaults.standard` changed. The regression
     /// guard for #733: a suite that should be writing to an isolated suite goes red here instead of
     /// silently corrupting the wallet on the simulator.

BitkitTests/ChannelMigrationPersistenceTests.swift

@@ -10,14 +10,13 @@ final class ChannelMigrationPersistenceTests: XCTestCase {
 
     override func setUp() {
         super.setUp()
+        // `pendingChannelMigration` is backed by UserDefaults.standard, which is the host app's own
+        // preferences. Nilling it unguarded discards a real pending migration — an RN channel manager
+        // and its monitors — so snapshot it first and put it back afterwards.
+        snapshotAppDefaults("rnPendingChannelMigration")
         migrations.pendingChannelMigration = nil
     }
 
-    override func tearDown() {
-        migrations.pendingChannelMigration = nil
-        super.tearDown()
-    }
-
     func testPendingMigrationIsRetainedWhenSetupFails() async {
         let migration = makeMigration(seed: 1)
         migrations.pendingChannelMigration = migration

BitkitTests/InstallationMarkerTests.swift

@@ -66,6 +66,30 @@ final class InstallationMarkerTests: XCTestCase {
         XCTAssertFalse(InstallationMarker.markerPath.path.contains("group.bitkit"))
     }
 
+    func testMarkerPathIsIsolatedUnderTest() {
+        XCTAssertTrue(InstallationMarker.markerPath.pathComponents.contains("unit-tests"))
+    }
+
+    /// `delete()` under test must not reach the real marker: a missing marker makes the next launch
+    /// treat the keychain as orphaned and wipe it. Plant a stand-in only when no real marker exists and
+    /// remove only that stand-in — on a simulator with a real install, deleting it is the bug itself.
+    func testDeleteLeavesTheRealMarkerAlone() throws {
+        let documents = FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0]
+        let realMarker = documents.appendingPathComponent(".bitkit_installed")
+        if !FileManager.default.fileExists(atPath: realMarker.path) {
+            try Data("probe".utf8).write(to: realMarker)
+            addTeardownBlock { try? FileManager.default.removeItem(at: realMarker) }
+        }
+
+        try InstallationMarker.create()
+        try InstallationMarker.delete()
+
+        XCTAssertTrue(
+            FileManager.default.fileExists(atPath: realMarker.path),
+            "InstallationMarker.delete() removed the real marker"
+        )
+    }
+
     func testCreateMarkerIsIdempotent() throws {
         // Create the marker
         try InstallationMarker.create()

BitkitTests/KeychainTests.swift

@@ -21,7 +21,7 @@ final class KeychainTests: XCTestCase {
         // Query the item with attributes returned
         let query: [String: Any] = [
             kSecClass as String: kSecClassGenericPassword,
-            kSecAttrAccount as String: KeychainEntryType.bip39Mnemonic(index: 0).storageKey,
+            kSecAttrAccount as String: Keychain.account(for: .bip39Mnemonic(index: 0)),
             kSecAttrAccessGroup as String: Env.keychainGroup,
             kSecReturnAttributes as String: true,
             kSecReturnData as String: false,
@@ -67,7 +67,7 @@ final class KeychainTests: XCTestCase {
 
         let query: [String: Any] = [
             kSecClass as String: kSecClassGenericPassword,
-            kSecAttrAccount as String: KeychainEntryType.securityPin.storageKey,
+            kSecAttrAccount as String: Keychain.account(for: .securityPin),
             kSecAttrAccessGroup as String: Env.keychainGroup,
             kSecReturnAttributes as String: true,
         ]
@@ -136,4 +136,28 @@ final class KeychainTests: XCTestCase {
             XCTAssertFalse((try? Keychain.exists(key: .bip39Passphrase(index: i))) ?? false)
         }
     }
+
+    /// The wipe must leave accounts outside the unit-test namespace alone, or running this suite deletes
+    /// the wallet on the simulator it runs against. The probe uses a unique account name rather than a
+    /// real key like `bip39_mnemonic_0`: on a simulator with a wallet that name is already taken, and
+    /// cleaning it up would delete the real seed.
+    func testWipeLeavesAccountsOutsideTheTestNamespace() throws {
+        let probe: [String: Any] = [
+            kSecClass as String: kSecClassGenericPassword,
+            kSecAttrAccount as String: "wipe-guard-probe-\(UUID().uuidString)",
+            kSecAttrAccessGroup as String: Env.keychainGroup,
+        ]
+        var item = probe
+        item[kSecValueData as String] = Data("probe".utf8)
+        XCTAssertEqual(SecItemAdd(item as CFDictionary, nil), errSecSuccess, "Failed to plant the probe account")
+        addTeardownBlock { SecItemDelete(probe as CFDictionary) }
+
+        try Keychain.wipeEntireKeychain()
+
+        XCTAssertEqual(
+            SecItemCopyMatching(probe as CFDictionary, nil),
+            errSecSuccess,
+            "wipeEntireKeychain deleted an account outside the unit-test namespace"
+        )
+    }
 }

BitkitTests/RNMigrationCleanupTests.swift

@@ -25,17 +25,29 @@ final class RNMigrationCleanupTests: XCTestCase {
         sandboxDocuments.appendingPathComponent("ldk")
     }
 
-    override func setUp() {
-        super.setUp()
+    /// `tearDown` runs even when `setUpWithError` skips, so it must not clean up a run that never
+    /// started — that is the destructive part.
+    private var didRunDestructiveSetUp = false
+
+    override func setUpWithError() throws {
+        try super.setUpWithError()
+        // `cleanupRNTestData` deletes the real ~/Documents/mmkv and ~/Documents/ldk, and
+        // `cleanupRNKeychain` deletes RN keychain items by service with no access-group filter. On a
+        // device mid-migration from React-Native Bitkit that is the migration source, so unlike the
+        // native keychain this cannot be namespaced away — it has to be opted into.
+        try skipUnlessDestructiveTestsEnabled()
+        didRunDestructiveSetUp = true
         // Clean up any existing test data
         cleanupRNTestData()
         try? Keychain.wipeEntireKeychain()
     }
 
     override func tearDown() {
-        // Clean up after tests
-        cleanupRNTestData()
-        try? Keychain.wipeEntireKeychain()
+        if didRunDestructiveSetUp {
+            // Clean up after tests
+            cleanupRNTestData()
+            try? Keychain.wipeEntireKeychain()
+        }
         super.tearDown()
     }