From 5f75491a42b724c04024ac153695c44503db68d5 Mon Sep 17 00:00:00 2001 From: sajee_techi Date: Mon, 22 Dec 2025 11:48:22 +0530 Subject: [PATCH 1/3] fix(db): resolve account migration issues and clean up dirty V4 IDs Implements a robust version 5 migration for the accounts table to handle different upgrade paths and fix data corruption caused by a previous migration. Key changes: - Robust Column Addition: Replaces the direct 'ALTER TABLE' with `AddAccountFactoryAddressIfNotExists`. This uses a PRAGMA check to prevent crashes on builds where the column might already exist (e.g., AppKevin path). - ID Cleanup Logic: Introduces `CleanDirtyV4Accounts` to identify and fix accounts using the "bad" ID format (`address@factory@alias`). It reverts these IDs back to the standard format (`address@alias`). - Conflict Handling: During cleanup, ensures that if a correct ID already exists, the dirty duplicate is removed without overwriting existing valid data. - Migration Orchestration: Updates the version 5 migration map to execute these steps in the correct order: verify column -> populate data -> clean IDs. --- lib/services/db/backup/accounts.dart | 98 +++++++++++++++++++++++++++- 1 file changed, 95 insertions(+), 3 deletions(-) diff --git a/lib/services/db/backup/accounts.dart b/lib/services/db/backup/accounts.dart index 0562feaf..835d8762 100644 --- a/lib/services/db/backup/accounts.dart +++ b/lib/services/db/backup/accounts.dart @@ -124,10 +124,12 @@ class AccountsTable extends DBTable { // bad migration,https://github.com/citizenwallet/app/blob/d4f72940e11f1812c34dfb47c0bffe7488a1c32e/lib/services/db/backup/accounts.dart#L123 ], 5: [ - // Kevin start from 4 - // Others start from 3 - 'ALTER TABLE $name ADD COLUMN accountFactoryAddress TEXT DEFAULT ""', + // This migration handles both paths: + // - AppKevin (v4 -> v5): column already exists, just populate + // - AppOthers (v3 -> v5): column doesn't exist, add it then populate + 'AddAccountFactoryAddressIfNotExists', 'PopulateAccountFactoryAddressMigration', + 'CleanDirtyV4Accounts', ] }; @@ -138,9 +140,17 @@ class AccountsTable extends DBTable { for (final query in queries) { try { switch (query) { + case 'AddAccountFactoryAddressIfNotExists': + await _addAccountFactoryAddressIfNotExists(db, name); + continue; + case 'PopulateAccountFactoryAddressMigration': await _populateAccountFactoryAddressMigration(db, name); continue; + + case 'CleanDirtyV4Accounts': + await _cleanDirtyV4Accounts(db, name); + continue; } await db.execute(query); @@ -153,6 +163,24 @@ class AccountsTable extends DBTable { } } + Future _addAccountFactoryAddressIfNotExists( + Database db, + String name, + ) async { + final columnName = 'accountFactoryAddress'; + + // Check if column exists + final tableInfo = await db.rawQuery('PRAGMA table_info($name)'); + final hasColumn = tableInfo.any((col) => col['name'] == columnName); + + if (hasColumn) { + return; + } + + await db + .execute('ALTER TABLE $name ADD COLUMN $columnName TEXT DEFAULT ""'); + } + Future _populateAccountFactoryAddressMigration( Database db, String name) async { // Work directly with raw DB data, not DBAccount objects @@ -174,6 +202,70 @@ class AccountsTable extends DBTable { } } + Future _cleanDirtyV4Accounts(Database db, String name) async { + // Get all accounts from the database + List> accounts = await db.query(name); + + for (final Map account in accounts) { + final String currentId = account['id'] as String; + final String alias = account['alias'] as String; + final String addressStr = account['address'] as String; + final String accountFactoryAddressStr = + account['accountFactoryAddress'] as String; + + // Construct what the ID should be in the old format + final String oldFormatId = getAccountID(EthereumAddress.fromHex(addressStr), alias); + + // Construct what the ID would be in the new (bad) format + final String newFormatId = '$addressStr@$accountFactoryAddressStr@$alias'; + + // Check if current ID matches the new (bad) format + if (currentId == newFormatId) { + debugPrint('Cleaning dirty account: $currentId -> $oldFormatId'); + + // Check if an account with the old format ID already exists + final existingOldFormat = await db.query( + name, + where: 'id = ?', + whereArgs: [oldFormatId], + ); + + if (existingOldFormat.isEmpty) { + // No conflict: Insert new row with old ID format + final Map cleanAccount = Map.from(account); + cleanAccount['id'] = oldFormatId; + + await db.insert( + name, + cleanAccount, + conflictAlgorithm: ConflictAlgorithm.replace, + ); + + debugPrint('Inserted clean account with old format ID: $oldFormatId'); + } else { + // Conflict exists: Keep the existing old format, just log + debugPrint( + 'Old format ID already exists, keeping existing: $oldFormatId'); + } + + // Delete the row with new (bad) format ID + await db.delete( + name, + where: 'id = ?', + whereArgs: [currentId], + ); + + debugPrint('Deleted dirty account with new format ID: $currentId'); + } else if (currentId == oldFormatId) { + // Already in correct old format, do nothing + debugPrint('Account already in correct format: $currentId'); + } else { + // Unexpected format, log warning but don't touch it + debugPrint('Warning: Unexpected ID format: $currentId'); + } + } + } + // get account by id Future get(EthereumAddress address, String alias) async { final List> maps = await db.query( From e579e6945fe2e97b80ce285956071caf70d67b3c Mon Sep 17 00:00:00 2001 From: sajee_techi Date: Mon, 22 Dec 2025 13:26:11 +0530 Subject: [PATCH 2/3] fix(db): enforce account ID normalization for unexpected formats Updates the `_cleanDirtyV4Accounts` migration logic to handle and repair accounts that do not match either the standard format or the known "bad" V4 format. Key changes: - Active Correction: Replaces the warning log for "unexpected formats" with an active migration to the standard `oldFormatId`. - Integrity Protection: Before migrating an unexpected ID, the code checks for existing standard format IDs to prevent primary key conflicts. - Data Preservation: If no conflict exists, it creates a corrected record with the standard ID while preserving all associated account data. - Cleanup: Automatically deletes the record with the malformed/unexpected ID after the migration or conflict check is complete. --- lib/services/db/backup/accounts.dart | 40 ++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/lib/services/db/backup/accounts.dart b/lib/services/db/backup/accounts.dart index 835d8762..20248722 100644 --- a/lib/services/db/backup/accounts.dart +++ b/lib/services/db/backup/accounts.dart @@ -260,8 +260,44 @@ class AccountsTable extends DBTable { // Already in correct old format, do nothing debugPrint('Account already in correct format: $currentId'); } else { - // Unexpected format, log warning but don't touch it - debugPrint('Warning: Unexpected ID format: $currentId'); + // Unexpected format - force to old format + debugPrint( + 'Warning: Unexpected ID format, forcing to old format: $currentId -> $oldFormatId'); + + // Check if an account with the old format ID already exists + final existingOldFormat = await db.query( + name, + where: 'id = ?', + whereArgs: [oldFormatId], + ); + + if (existingOldFormat.isEmpty) { + // No conflict: Insert new row with old ID format, preserving all other columns + final Map cleanAccount = Map.from(account); + cleanAccount['id'] = oldFormatId; + + await db.insert( + name, + cleanAccount, + conflictAlgorithm: ConflictAlgorithm.replace, + ); + + debugPrint( + 'Inserted account with corrected old format ID: $oldFormatId'); + } else { + // Conflict exists: Keep the existing old format + debugPrint( + 'Old format ID already exists, keeping existing: $oldFormatId'); + } + + // Delete the row with unexpected format ID + await db.delete( + name, + where: 'id = ?', + whereArgs: [currentId], + ); + + debugPrint('Deleted account with unexpected format ID: $currentId'); } } } From 64bcadca49c9aed3a2ebfff5990686bd0b06bc9d Mon Sep 17 00:00:00 2001 From: sajee_techi Date: Mon, 22 Dec 2025 15:45:45 +0530 Subject: [PATCH 3/3] feat: add migration #7 to reconcile AppKevin and AppOthers credential formats Implements a unified migration strategy (v7) for both iOS and Android platforms to handle two divergent migration histories: - AppKevin users (v6->v7): Had bad migrations in v5-v6 that created credentials with incorrect accountFactoryAddress values in the key format. Migration #7 detects and cleans these dirty keys by reconstructing them with the correct accountFactoryAddress from getAccountFactoryAddressByAlias(). - AppOthers users (v4->v7): Skip empty migrations v5-v6 and migrate directly from old format (address@alias) to new format (address@accountFactoryAddress@alias) using the correct accountFactoryAddress. Changes: - Refactored key detection to categorize credentials into oldFormatKeys (2 parts) and dirtyNewFormatKeys (3 parts) for targeted handling - Added AppKevin path: validates existing 3-part keys and recreates them with correct accountFactoryAddress if needed (case-insensitive comparison) - Enhanced AppOthers path: migrates 2-part keys to 3-part format with proper accountFactoryAddress lookup - Enabled deletion of old/dirty keys (removed TODO comments) - Added comprehensive debug logging for migration tracking - Implemented idempotent logic: keys already in correct format are skipped Both user groups converge to the same final state at version 7 with credentials in format: address@accountFactoryAddress@alias where accountFactoryAddress is correctly determined by community alias. Platforms: iOS (apple.dart), Android (android.dart) --- lib/services/accounts/native/android.dart | 114 +++++++++++++++++----- lib/services/accounts/native/apple.dart | 113 ++++++++++++++++----- 2 files changed, 180 insertions(+), 47 deletions(-) diff --git a/lib/services/accounts/native/android.dart b/lib/services/accounts/native/android.dart index 56ae764d..40d3db09 100644 --- a/lib/services/accounts/native/android.dart +++ b/lib/services/accounts/native/android.dart @@ -101,45 +101,58 @@ class AndroidAccountsService extends AccountsServiceInterface { // bad migration, https://github.com/citizenwallet/app/blob/d4f72940e11f1812c34dfb47c0bffe7488a1c32e/lib/services/accounts/native/android.dart#L251 }, 7: () async { - // distinguish migration starting from 4 (Others) - //distinguish migration starting from 6 (Kevin, Jonas) + // This migration handles two paths: + // - AppKevin (v6 -> v7): Clean dirty keys with wrong accountFactoryAddress + // - AppOthers (v4 -> v7): Migrate from old format to new format // Read all credentials from secure storage final allValues = await _credentials.readAll(); - // Filter keys that match the old format: address@alias - // These are keys that don't start with backupPrefix and contain exactly one '@' - final oldFormatKeys = allValues.keys.where((key) { + // Separate keys into different formats + final oldFormatKeys = []; // address@alias + final dirtyNewFormatKeys = + []; // address@accountFactoryAddress@alias (dirty) + + for (final key in allValues.keys) { if (key.startsWith(backupPrefix) || key == versionPrefix || key == pinCodeKey || key == pinCodeCheckKey) { - return false; + continue; } - // Check if it matches address@alias format (one @ symbol) + final parts = key.split('@'); - if (parts.length != 2) { - return false; - } - // Validate that the first part is a valid Ethereum address - try { - EthereumAddress.fromHex(parts[0]); - return true; - } catch (_) { - return false; + // Check for old format: address@alias (2 parts) + if (parts.length == 2) { + try { + EthereumAddress.fromHex(parts[0]); + oldFormatKeys.add(key); + } catch (_) { + // Not a valid address, skip + } + } + // Check for new format: address@accountFactoryAddress@alias (3 parts) + else if (parts.length == 3) { + try { + EthereumAddress.fromHex(parts[0]); + EthereumAddress.fromHex(parts[1]); + dirtyNewFormatKeys.add(key); + } catch (_) { + // Not valid addresses, skip + } } - }).toList(); + } final toDelete = []; + // Handle AppOthers path: Migrate old format keys to new format for (final oldKey in oldFormatKeys) { final privateKeyValue = allValues[oldKey]; if (privateKeyValue == null) { continue; } - // Parse the old key format: address@alias final parts = oldKey.split('@'); if (parts.length != 2) { continue; @@ -149,7 +162,7 @@ class AndroidAccountsService extends AccountsServiceInterface { final alias = parts[1]; try { - // Get the account factory address for this alias + // Get the correct account factory address for this alias final accountFactoryAddress = getAccountFactoryAddressByAlias(alias); @@ -164,19 +177,74 @@ class AndroidAccountsService extends AccountsServiceInterface { // Write the credential with the new key format await _credentials.write(backup.key, backup.value); + debugPrint('Migrated old format key: $oldKey -> ${backup.key}'); + // Mark old key for deletion - // TODO: delete the old key - // toDelete.add(oldKey); + toDelete.add(oldKey); } catch (e) { - // If we can't determine the account factory address, skip this key debugPrint('Error migrating key $oldKey: $e'); continue; } } - // Delete all old format keys + // Handle AppKevin path: Clean dirty new format keys + for (final dirtyKey in dirtyNewFormatKeys) { + final privateKeyValue = allValues[dirtyKey]; + if (privateKeyValue == null) { + continue; + } + + final parts = dirtyKey.split('@'); + if (parts.length != 3) { + continue; + } + + final address = parts[0]; + final dirtyAccountFactoryAddress = parts[1]; + final alias = parts[2]; + + try { + // Get the CORRECT account factory address for this alias + final correctAccountFactoryAddress = + getAccountFactoryAddressByAlias(alias); + + // Check if the dirty key has the wrong accountFactoryAddress + if (dirtyAccountFactoryAddress.toLowerCase() != + correctAccountFactoryAddress.toLowerCase()) { + debugPrint( + 'Found dirty key with wrong accountFactoryAddress: $dirtyKey'); + debugPrint(' Dirty: $dirtyAccountFactoryAddress'); + debugPrint(' Correct: $correctAccountFactoryAddress'); + + // Create a BackupWalletV5 with the CORRECT account factory address + final cleanBackup = BackupWalletV5( + address: address, + alias: alias, + accountFactoryAddress: correctAccountFactoryAddress, + privateKey: privateKeyValue, + ); + + // Write the credential with the correct key format + await _credentials.write(cleanBackup.key, cleanBackup.value); + + debugPrint('Cleaned dirty key: $dirtyKey -> ${cleanBackup.key}'); + + // Mark dirty key for deletion + toDelete.add(dirtyKey); + } else { + // Key already has correct accountFactoryAddress, no action needed + debugPrint('Key already correct: $dirtyKey'); + } + } catch (e) { + debugPrint('Error cleaning dirty key $dirtyKey: $e'); + continue; + } + } + + // Delete all old and dirty keys for (final key in toDelete) { await _credentials.delete(key); + debugPrint('Deleted old/dirty key: $key'); } }, }; diff --git a/lib/services/accounts/native/apple.dart b/lib/services/accounts/native/apple.dart index 6a5f48b5..ef140db0 100644 --- a/lib/services/accounts/native/apple.dart +++ b/lib/services/accounts/native/apple.dart @@ -214,44 +214,55 @@ class AppleAccountsService extends AccountsServiceInterface { // bad migration, https://github.com/citizenwallet/app/blob/d4f72940e11f1812c34dfb47c0bffe7488a1c32e/lib/services/accounts/native/apple.dart#L264 }, 7: () async { - - // distinguish migration starting from 4 (Others) - //distinguish migration starting from 6 (Kevin, Jonas) - + // This migration handles two paths: + // - AppKevin (v6 -> v7): Clean dirty keys with wrong accountFactoryAddress + // - AppOthers (v4 -> v7): Migrate from old format to new format // Read all credentials from Keychain final allValues = await _credentials.readAll(); - // Filter keys that match the old format: address@alias - // These are keys that don't start with backupPrefix and contain exactly one '@' - final oldFormatKeys = allValues.keys.where((key) { + // Separate keys into different formats + final oldFormatKeys = []; // address@alias + final dirtyNewFormatKeys = + []; // address@accountFactoryAddress@alias (dirty) + + for (final key in allValues.keys) { if (key.startsWith(backupPrefix) || key == versionPrefix) { - return false; + continue; } - // Check if it matches address@alias format (one @ symbol) + final parts = key.split('@'); - if (parts.length != 2) { - return false; - } - // Validate that the first part is a valid Ethereum address - try { - EthereumAddress.fromHex(parts[0]); - return true; - } catch (_) { - return false; + // Check for old format: address@alias (2 parts) + if (parts.length == 2) { + try { + EthereumAddress.fromHex(parts[0]); + oldFormatKeys.add(key); + } catch (_) { + // Not a valid address, skip + } + } + // Check for new format: address@accountFactoryAddress@alias (3 parts) + else if (parts.length == 3) { + try { + EthereumAddress.fromHex(parts[0]); + EthereumAddress.fromHex(parts[1]); + dirtyNewFormatKeys.add(key); + } catch (_) { + // Not valid addresses, skip + } } - }).toList(); + } final toDelete = []; + // Handle AppOthers path: Migrate old format keys to new format for (final oldKey in oldFormatKeys) { final privateKeyValue = allValues[oldKey]; if (privateKeyValue == null) { continue; } - // Parse the old key format: address@alias final parts = oldKey.split('@'); if (parts.length != 2) { continue; @@ -261,7 +272,7 @@ class AppleAccountsService extends AccountsServiceInterface { final alias = parts[1]; try { - // Get the account factory address for this alias + // Get the correct account factory address for this alias final accountFactoryAddress = getAccountFactoryAddressByAlias(alias); @@ -276,16 +287,70 @@ class AppleAccountsService extends AccountsServiceInterface { // Write the credential with the new key format await _credentials.write(backup.key, backup.value); + debugPrint('Migrated old format key: $oldKey -> ${backup.key}'); + // Mark old key for deletion - // TODO: delete the old key - // toDelete.add(oldKey); + toDelete.add(oldKey); } catch (e) { - // If we can't determine the account factory address, skip this key debugPrint('Error migrating key $oldKey: $e'); continue; } } + // Handle AppKevin path: Clean dirty new format keys + for (final dirtyKey in dirtyNewFormatKeys) { + final privateKeyValue = allValues[dirtyKey]; + if (privateKeyValue == null) { + continue; + } + + final parts = dirtyKey.split('@'); + if (parts.length != 3) { + continue; + } + + final address = parts[0]; + final dirtyAccountFactoryAddress = parts[1]; + final alias = parts[2]; + + try { + // Get the CORRECT account factory address for this alias + final correctAccountFactoryAddress = + getAccountFactoryAddressByAlias(alias); + + // Check if the dirty key has the wrong accountFactoryAddress + if (dirtyAccountFactoryAddress.toLowerCase() != + correctAccountFactoryAddress.toLowerCase()) { + debugPrint( + 'Found dirty key with wrong accountFactoryAddress: $dirtyKey'); + debugPrint(' Dirty: $dirtyAccountFactoryAddress'); + debugPrint(' Correct: $correctAccountFactoryAddress'); + + // Create a BackupWalletV5 with the CORRECT account factory address + final cleanBackup = BackupWalletV5( + address: address, + alias: alias, + accountFactoryAddress: correctAccountFactoryAddress, + privateKey: privateKeyValue, + ); + + // Write the credential with the correct key format + await _credentials.write(cleanBackup.key, cleanBackup.value); + + debugPrint('Cleaned dirty key: $dirtyKey -> ${cleanBackup.key}'); + + // Mark dirty key for deletion + toDelete.add(dirtyKey); + } else { + // Key already has correct accountFactoryAddress, no action needed + debugPrint('Key already correct: $dirtyKey'); + } + } catch (e) { + debugPrint('Error cleaning dirty key $dirtyKey: $e'); + continue; + } + } + // Delete all old format keys for (final key in toDelete) { final saved = await _credentials.containsKey(key);