From 206ce8262ad195ad3690913ef23d476a9167551c Mon Sep 17 00:00:00 2001 From: Lukas Korba Date: Fri, 15 Nov 2024 11:54:29 +0100 Subject: [PATCH] Remote error handling and byte alignment resolved - All AddressBookClient operations have been updated to return a remote store result - Undefined Behaviour resolved for byte load operation - Typos fixed --- .../AddressBookEncryption.swift | 19 +++- .../AddressBookInterface.swift | 8 +- .../AddressBookLiveKey.swift | 89 +++++++++++-------- .../AddressBook/AddressBookStore.swift | 32 ++++--- .../SendConfirmationStore.swift | 8 +- .../Features/SendFlow/SendFlowStore.swift | 8 +- .../TransactionListStore.swift | 8 +- 7 files changed, 113 insertions(+), 59 deletions(-) diff --git a/modules/Sources/Dependencies/AddressBookClient/AddressBookEncryption.swift b/modules/Sources/Dependencies/AddressBookClient/AddressBookEncryption.swift index 36b6c74f..e04c3ca5 100644 --- a/modules/Sources/Dependencies/AddressBookClient/AddressBookEncryption.swift +++ b/modules/Sources/Dependencies/AddressBookClient/AddressBookEncryption.swift @@ -218,8 +218,23 @@ extension AddressBookClient { return nil } - return bytes.withUnsafeBytes { - $0.load(as: Int.self).bigEndian + return bytes.withUnsafeBytes { ptr -> Int in + // Check if the pointer is properly aligned for Int + if ptr.baseAddress?.alignedUp(toMultipleOf: MemoryLayout.alignment) == ptr.baseAddress { + // Memory is already aligned + return ptr.load(as: Int.self).bigEndian + } else { + // Handle unaligned memory + var value: Int = 0 + + withUnsafeMutableBytes(of: &value) { valuePtr in + // Copy bytes manually to handle unaligned access + for i in 0...size) { + valuePtr[i] = ptr[i] + } + } + return value.bigEndian + } } } diff --git a/modules/Sources/Dependencies/AddressBookClient/AddressBookInterface.swift b/modules/Sources/Dependencies/AddressBookClient/AddressBookInterface.swift index d3fe5f06..e2d21f97 100644 --- a/modules/Sources/Dependencies/AddressBookClient/AddressBookInterface.swift +++ b/modules/Sources/Dependencies/AddressBookClient/AddressBookInterface.swift @@ -17,8 +17,8 @@ extension DependencyValues { @DependencyClient public struct AddressBookClient { - public let allLocalContacts: () throws -> AddressBookContacts - public let syncContacts: (AddressBookContacts?) async throws -> AddressBookContacts - public let storeContact: (Contact) throws -> (contacts: AddressBookContacts, storeResult: StoreResult) - public let deleteContact: (Contact) throws -> AddressBookContacts + public let allLocalContacts: () throws -> (contacts: AddressBookContacts, remoteStoreResult: RemoteStoreResult?) + public let syncContacts: (AddressBookContacts?) async throws -> (contacts: AddressBookContacts, remoteStoreResult: RemoteStoreResult) + public let storeContact: (Contact) throws -> (contacts: AddressBookContacts, remoteStoreResult: RemoteStoreResult) + public let deleteContact: (Contact) throws -> (contacts: AddressBookContacts, remoteStoreResult: RemoteStoreResult?) } diff --git a/modules/Sources/Dependencies/AddressBookClient/AddressBookLiveKey.swift b/modules/Sources/Dependencies/AddressBookClient/AddressBookLiveKey.swift index 82ab74b8..6345c252 100644 --- a/modules/Sources/Dependencies/AddressBookClient/AddressBookLiveKey.swift +++ b/modules/Sources/Dependencies/AddressBookClient/AddressBookLiveKey.swift @@ -22,9 +22,10 @@ extension AddressBookClient: DependencyKey { static let int64Size = MemoryLayout.size } - public enum StoreResult: Equatable { + public enum RemoteStoreResult: Equatable { + case failure + case notAttempted case success - case remoteFailed } public enum AddressBookClientError: Error { @@ -48,10 +49,10 @@ extension AddressBookClient: DependencyKey { allLocalContacts: { // return latest known contacts or load ones for the first time guard latestKnownContacts == nil else { - return latestKnownContacts ?? .empty + return (latestKnownContacts ?? .empty, nil) } - // contacts haven't been loaded from the locale storage yet, do it + // contacts haven't been loaded from the local storage yet, do it do { guard let documentsDirectory = FileManager.default.urls(for: .documentDirectory, in: .userDomainMask).first else { throw AddressBookClientError.documentsFolder @@ -63,7 +64,7 @@ extension AddressBookClient: DependencyKey { if let contactsData = try? Data(contentsOf: encryptedFileURL) { let contacts = try AddressBookClient.contactsFrom(encryptedData: contactsData) - // file exists and was successfuly decrypted; + // file exists and was successfully decrypted and parsed; // try to find the unencrypted file and delete it let unencryptedFileURL = documentsDirectory.appendingPathComponent(Constants.unencryptedFilename) if FileManager.default.fileExists(atPath: unencryptedFileURL.path) { @@ -71,20 +72,26 @@ extension AddressBookClient: DependencyKey { } latestKnownContacts = contacts - return contacts + return (contacts, nil) } else { // Fallback to the unencrypted file check and resolution let unencryptedFileURL = documentsDirectory.appendingPathComponent(Constants.unencryptedFilename) if let contactsData = try? Data(contentsOf: unencryptedFileURL) { - // Unencrypted file exists; ensure data are parsed, re-saved as encrypted, and file deteled - let contacts = try AddressBookClient.contactsFrom(contactsData) + // Unencrypted file exists; ensure data are parsed, re-saved as encrypted, and the original file deleted. + + var contacts = try AddressBookClient.contactsFrom(contactsData) // try to encrypt and store the data + var remoteStoreResult: RemoteStoreResult do { - try AddressBookClient.storeContacts(contacts, remoteStorage: remoteStorage, remoteStore: false) + remoteStoreResult = try AddressBookClient.storeContacts(contacts, remoteStorage: remoteStorage, remoteStore: false) + + let result = try syncContacts(contacts: contacts, remoteStorage: remoteStorage, storeAfterSync: true) + remoteStoreResult = result.remoteStoreResult + contacts = result.contacts } catch { - // the store of the new file failed, skip the file remove + // the store of the new file failed locally, skip the file remove latestKnownContacts = contacts throw error } @@ -92,9 +99,9 @@ extension AddressBookClient: DependencyKey { try? FileManager.default.removeItem(at: unencryptedFileURL) latestKnownContacts = contacts - return contacts + return (contacts, remoteStoreResult) } else { - return .empty + return (.empty, nil) } } } catch { @@ -104,16 +111,17 @@ extension AddressBookClient: DependencyKey { syncContacts: { let abContacts = $0 ?? latestKnownContacts ?? AddressBookContacts.empty - let syncedContacts = try syncContacts(contacts: abContacts, remoteStorage: remoteStorage) - - latestKnownContacts = syncedContacts + let result = try syncContacts(contacts: abContacts, remoteStorage: remoteStorage) - return syncedContacts + latestKnownContacts = result.contacts + + return result }, storeContact: { let abContacts = latestKnownContacts ?? AddressBookContacts.empty - var syncedContacts = try syncContacts(contacts: abContacts, remoteStorage: remoteStorage, storeAfterSync: false) + let result = try syncContacts(contacts: abContacts, remoteStorage: remoteStorage, storeAfterSync: false) + var syncedContacts = result.contacts // if already exists, remove it if syncedContacts.contacts.contains($0) { @@ -122,31 +130,32 @@ extension AddressBookClient: DependencyKey { syncedContacts.contacts.append($0) - let storeResult = try storeContacts(syncedContacts, remoteStorage: remoteStorage) + let remoteStoreResult = try storeContacts(syncedContacts, remoteStorage: remoteStorage) // update the latest known contacts latestKnownContacts = syncedContacts - return (syncedContacts, storeResult) + return (syncedContacts, remoteStoreResult) }, deleteContact: { let abContacts = latestKnownContacts ?? AddressBookContacts.empty - var syncedContacts = try syncContacts(contacts: abContacts, remoteStorage: remoteStorage, storeAfterSync: false) - + let result = try syncContacts(contacts: abContacts, remoteStorage: remoteStorage, storeAfterSync: false) + var syncedContacts = result.contacts + // if it doesn't exist, do nothing guard syncedContacts.contacts.contains($0) else { - return syncedContacts + return (syncedContacts, nil) } syncedContacts.contacts.remove($0) - try storeContacts(syncedContacts, remoteStorage: remoteStorage) + let remoteStoreResult = try storeContacts(syncedContacts, remoteStorage: remoteStorage) // update the latest known contacts latestKnownContacts = syncedContacts - return syncedContacts + return (syncedContacts, remoteStoreResult) } ) } @@ -155,7 +164,7 @@ extension AddressBookClient: DependencyKey { contacts: AddressBookContacts, remoteStorage: RemoteStorageClient, storeAfterSync: Bool = true - ) throws -> AddressBookContacts { + ) throws -> (contacts: AddressBookContacts, remoteStoreResult: RemoteStoreResult) { // Ensure remote contacts are prepared var remoteContacts: AddressBookContacts = .empty var storeData = true @@ -207,43 +216,49 @@ extension AddressBookClient: DependencyKey { } } + var remoteStoreResult = RemoteStoreResult.notAttempted if storeAfterSync { - try storeContacts(syncedContacts, remoteStorage: remoteStorage, remoteStore: storeData) + remoteStoreResult = try storeContacts(syncedContacts, remoteStorage: remoteStorage, remoteStore: storeData) } - return syncedContacts + return (syncedContacts, remoteStoreResult) } - @discardableResult private static func storeContacts( + private static func storeContacts( _ abContacts: AddressBookContacts, remoteStorage: RemoteStorageClient, remoteStore: Bool = true - ) throws -> StoreResult { + ) throws -> RemoteStoreResult { // encrypt data let encryptedContacts = try AddressBookClient.encryptContacts(abContacts) + let filenameForEncryptedFile = try AddressBookClient.filenameForEncryptedFile() + // store encrypted data to the local storage guard let documentsDirectory = FileManager.default.urls(for: .documentDirectory, in: .userDomainMask).first else { throw AddressBookClientError.documentsFolder } - - let filenameForEncryptedFile = try AddressBookClient.filenameForEncryptedFile() let fileURL = documentsDirectory.appendingPathComponent(filenameForEncryptedFile) try encryptedContacts.write(to: fileURL) - - var storeResult = StoreResult.success - + // store encrypted data to the remote storage + var isRemoteSuccess = remoteStore + if remoteStore { do { try remoteStorage.storeAddressBookContacts(encryptedContacts, filenameForEncryptedFile) } catch { - storeResult = .remoteFailed + isRemoteSuccess = false } } - - return storeResult + + switch (remoteStore, isRemoteSuccess) { + case (true, true): return .success + case (true, false): return .failure + case (false, true): return .notAttempted + case (false, false): return .notAttempted + } } private static func filenameForEncryptedFile() throws -> String { diff --git a/modules/Sources/Features/AddressBook/AddressBookStore.swift b/modules/Sources/Features/AddressBook/AddressBookStore.swift index 005367f5..59859f05 100644 --- a/modules/Sources/Features/AddressBook/AddressBookStore.swift +++ b/modules/Sources/Features/AddressBook/AddressBookStore.swift @@ -204,13 +204,17 @@ public struct AddressBook { } if let contact { do { - let contacts = try addressBook.deleteContact(contact) + let result = try addressBook.deleteContact(contact) + let abContacts = result.contacts + if let remoteStoreResult = result.remoteStoreResult, remoteStoreResult == .failure { + // TODO: [#1408] error handling https://github.com/Electric-Coin-Company/zashi-ios/issues/1408 + } return .concatenate( - .send(.fetchedABContacts(contacts, false)), + .send(.fetchedABContacts(abContacts, false)), .send(.updateDestination(nil)) ) } catch { - // TODO: [#1408] erro handling https://github.com/Electric-Coin-Company/zashi-ios/issues/1408 + // TODO: [#1408] error handling https://github.com/Electric-Coin-Company/zashi-ios/issues/1408 } } return .none @@ -240,15 +244,15 @@ public struct AddressBook { do { let result = try addressBook.storeContact(Contact(address: state.address, name: state.name)) let abContacts = result.contacts - if result.storeResult == .remoteFailed { - // TODO: [#1408] erro handling https://github.com/Electric-Coin-Company/zashi-ios/issues/1408 + if result.remoteStoreResult == .failure { + // TODO: [#1408] error handling https://github.com/Electric-Coin-Company/zashi-ios/issues/1408 } return .concatenate( .send(.fetchedABContacts(abContacts, false)), .send(.contactStoreSuccess) ) } catch { - // TODO: [#1408] erro handling https://github.com/Electric-Coin-Company/zashi-ios/issues/1408 + // TODO: [#1408] error handling https://github.com/Electric-Coin-Company/zashi-ios/issues/1408 return .send(.updateDestination(nil)) } @@ -261,10 +265,14 @@ public struct AddressBook { case .fetchABContactsRequested: do { - let abContacts = try addressBook.allLocalContacts() + let result = try addressBook.allLocalContacts() + let abContacts = result.contacts + if let remoteStoreResult = result.remoteStoreResult, remoteStoreResult == .failure { + // TODO: [#1408] error handling https://github.com/Electric-Coin-Company/zashi-ios/issues/1408 + } return .send(.fetchedABContacts(abContacts, true)) } catch { - // TODO: [#1408] erro handling https://github.com/Electric-Coin-Company/zashi-ios/issues/1408 + // TODO: [#1408] error handling https://github.com/Electric-Coin-Company/zashi-ios/issues/1408 return .none } @@ -273,10 +281,14 @@ public struct AddressBook { if requestToSync { return .run { send in do { - let syncedContacts = try await addressBook.syncContacts(abContacts) + let result = try await addressBook.syncContacts(abContacts) + let syncedContacts = result.contacts + if result.remoteStoreResult == .failure { + // TODO: [#1408] error handling https://github.com/Electric-Coin-Company/zashi-ios/issues/1408 + } await send(.fetchedABContacts(syncedContacts, false)) } catch { - // TODO: [#1408] erro handling https://github.com/Electric-Coin-Company/zashi-ios/issues/1408 + // TODO: [#1408] error handling https://github.com/Electric-Coin-Company/zashi-ios/issues/1408 } } } else { diff --git a/modules/Sources/Features/SendConfirmation/SendConfirmationStore.swift b/modules/Sources/Features/SendConfirmation/SendConfirmationStore.swift index d2006ff8..4532ad9d 100644 --- a/modules/Sources/Features/SendConfirmation/SendConfirmationStore.swift +++ b/modules/Sources/Features/SendConfirmation/SendConfirmationStore.swift @@ -179,10 +179,14 @@ public struct SendConfirmation { state.canSendMail = MFMailComposeViewController.canSendMail() state.alias = nil do { - let abContacts = try addressBook.allLocalContacts() + let result = try addressBook.allLocalContacts() + let abContacts = result.contacts + if let remoteStoreResult = result.remoteStoreResult, remoteStoreResult == .failure { + // TODO: [#1408] error handling https://github.com/Electric-Coin-Company/zashi-ios/issues/1408 + } return .send(.fetchedABContacts(abContacts)) } catch { - // TODO: [#1408] erro handling https://github.com/Electric-Coin-Company/zashi-ios/issues/1408 + // TODO: [#1408] error handling https://github.com/Electric-Coin-Company/zashi-ios/issues/1408 return .none } diff --git a/modules/Sources/Features/SendFlow/SendFlowStore.swift b/modules/Sources/Features/SendFlow/SendFlowStore.swift index 96a393b7..53479b60 100644 --- a/modules/Sources/Features/SendFlow/SendFlowStore.swift +++ b/modules/Sources/Features/SendFlow/SendFlowStore.swift @@ -247,13 +247,17 @@ public struct SendFlow { case .onAppear: state.memoState.charLimit = zcashSDKEnvironment.memoCharLimit do { - let abContacts = try addressBook.allLocalContacts() + let result = try addressBook.allLocalContacts() + let abContacts = result.contacts + if let remoteStoreResult = result.remoteStoreResult, remoteStoreResult == .failure { + // TODO: [#1408] error handling https://github.com/Electric-Coin-Company/zashi-ios/issues/1408 + } return .merge( .send(.exchangeRateSetupChanged), .send(.fetchedABContacts(abContacts)) ) } catch { - // TODO: [#1408] erro handling https://github.com/Electric-Coin-Company/zashi-ios/issues/1408 + // TODO: [#1408] error handling https://github.com/Electric-Coin-Company/zashi-ios/issues/1408 return .send(.exchangeRateSetupChanged) } diff --git a/modules/Sources/Features/TransactionList/TransactionListStore.swift b/modules/Sources/Features/TransactionList/TransactionListStore.swift index 1c60f99b..561e8e5e 100644 --- a/modules/Sources/Features/TransactionList/TransactionListStore.swift +++ b/modules/Sources/Features/TransactionList/TransactionListStore.swift @@ -72,10 +72,14 @@ public struct TransactionList { case .onAppear: state.requiredTransactionConfirmations = zcashSDKEnvironment.requiredTransactionConfirmations do { - let abContacts = try addressBook.allLocalContacts() + let result = try addressBook.allLocalContacts() + let abContacts = result.contacts + if let remoteStoreResult = result.remoteStoreResult, remoteStoreResult == .failure { + // TODO: [#1408] error handling https://github.com/Electric-Coin-Company/zashi-ios/issues/1408 + } state.addressBookContacts = abContacts } catch { - // TODO: [#1408] erro handling https://github.com/Electric-Coin-Company/zashi-ios/issues/1408 + // TODO: [#1408] error handling https://github.com/Electric-Coin-Company/zashi-ios/issues/1408 } return .merge(