From 0dec5782204130706ed37395b2dc568e7e7f386f Mon Sep 17 00:00:00 2001 From: Simon McLoughlin Date: Mon, 22 Jan 2024 12:49:00 +0000 Subject: [PATCH] change WalletCache to only throw to avoid complexities of checking bool and catch --- .../Services/WalletCacheService.swift | 11 +- .../Services/WalletCacheServiceTests.swift | 104 +++++++++++++----- 2 files changed, 84 insertions(+), 31 deletions(-) diff --git a/Sources/KukaiCoreSwift/Services/WalletCacheService.swift b/Sources/KukaiCoreSwift/Services/WalletCacheService.swift index 5f6a65f9..5e9f39a7 100644 --- a/Sources/KukaiCoreSwift/Services/WalletCacheService.swift +++ b/Sources/KukaiCoreSwift/Services/WalletCacheService.swift @@ -25,6 +25,8 @@ enum WalletCacheError: Error { case noPrivateKeyFound case unableToDecrypt case walletAlreadyExists + case requestedIndexTooHigh + case unableToEncryptAndWrite } @@ -83,7 +85,7 @@ public class WalletCacheService { - Parameter childOfIndex: An optional `Int` to denote the index of the HD wallet that this wallet is a child of - Returns: Bool, indicating if the storage was successful or not */ - public func cache(wallet: T, childOfIndex: Int?, backedUp: Bool) throws -> Bool { + public func cache(wallet: T, childOfIndex: Int?, backedUp: Bool) throws { guard let existingWallets = readWalletsFromDiskAndDecrypt() else { Logger.walletCache.error("cache - Unable to cache wallet, as can't decrypt existing wallets") throw WalletCacheError.unableToDecrypt @@ -101,7 +103,7 @@ public class WalletCacheService { if let index = childOfIndex { if index >= newMetadata.hdWallets.count { Logger.walletCache.error("WalletCacheService metadata insertion issue. Requested to add to HDWallet at index \"\(index)\", when there are currently only \"\(newMetadata.hdWallets.count)\" items") - return false + throw WalletCacheError.requestedIndexTooHigh } newMetadata.hdWallets[index].children.append(WalletMetadata(address: wallet.address, hdWalletGroupName: nil, walletNickname: nil, socialUsername: nil, type: wallet.type, children: [], isChild: true, isWatchOnly: false, bas58EncodedPublicKey: wallet.publicKeyBase58encoded(), backedUp: backedUp)) @@ -130,7 +132,10 @@ public class WalletCacheService { newMetadata.linearWallets.append(WalletMetadata(address: wallet.address, hdWalletGroupName: nil, walletNickname: nil, socialUsername: nil, socialType: nil, type: wallet.type, children: [], isChild: false, isWatchOnly: false, bas58EncodedPublicKey: wallet.publicKeyBase58encoded(), backedUp: backedUp)) } - return encryptAndWriteWalletsToDisk(wallets: newWallets) && encryptAndWriteMetadataToDisk(newMetadata) + + if encryptAndWriteWalletsToDisk(wallets: newWallets) && encryptAndWriteMetadataToDisk(newMetadata) == false { + throw WalletCacheError.unableToEncryptAndWrite + } } /** Cahce a watch wallet metadata obj, only. Metadata cahcing handled via wallet cache method diff --git a/Tests/KukaiCoreSwiftTests/Services/WalletCacheServiceTests.swift b/Tests/KukaiCoreSwiftTests/Services/WalletCacheServiceTests.swift index dfc86fc3..3fb821dc 100644 --- a/Tests/KukaiCoreSwiftTests/Services/WalletCacheServiceTests.swift +++ b/Tests/KukaiCoreSwiftTests/Services/WalletCacheServiceTests.swift @@ -31,12 +31,26 @@ class WalletCacheServiceTests: XCTestCase { // Check its empty to begin with XCTAssert(walletCacheService.readWalletsFromDiskAndDecrypt()?.count == 0) + // Check we can write wallet objects - XCTAssert((try? walletCacheService.cache(wallet: MockConstants.defaultLinearWallet, childOfIndex: nil, backedUp: false)) != nil) - XCTAssert((try? walletCacheService.cache(wallet: MockConstants.defaultHdWallet, childOfIndex: nil, backedUp: false)) != nil) + do { + try walletCacheService.cache(wallet: MockConstants.defaultLinearWallet, childOfIndex: nil, backedUp: false) + try walletCacheService.cache(wallet: MockConstants.defaultHdWallet, childOfIndex: nil, backedUp: false) + } catch { + XCTFail("Should not error: \(error)") + } + // Check it fails if we try add the same wallet a second time - XCTAssert((try? walletCacheService.cache(wallet: MockConstants.defaultHdWallet, childOfIndex: nil, backedUp: false)) == nil) + do { + try walletCacheService.cache(wallet: MockConstants.defaultHdWallet, childOfIndex: nil, backedUp: false) + + } catch let error as WalletCacheError { + XCTAssert(error == WalletCacheError.walletAlreadyExists) + } catch { + XCTFail("Should throw WalletCacheError.walletAlreadyExists") + } + // Check they have been stored XCTAssert(walletCacheService.readWalletsFromDiskAndDecrypt()?.count == 2) @@ -74,8 +88,12 @@ class WalletCacheServiceTests: XCTestCase { XCTAssert(walletCacheService.readWalletsFromDiskAndDecrypt()?.count == 0) // Check we can write wallet objects - XCTAssert((try? walletCacheService.cache(wallet: MockConstants.defaultLinearWallet, childOfIndex: nil, backedUp: false)) != nil) - XCTAssert((try? walletCacheService.cache(wallet: MockConstants.defaultHdWallet, childOfIndex: nil, backedUp: false)) != nil) + do { + try walletCacheService.cache(wallet: MockConstants.defaultLinearWallet, childOfIndex: nil, backedUp: false) + try walletCacheService.cache(wallet: MockConstants.defaultHdWallet, childOfIndex: nil, backedUp: false) + } catch { + XCTFail("Should not error: \(error)") + } // Check they have been stored XCTAssert(walletCacheService.readWalletsFromDiskAndDecrypt()?.count == 2) @@ -102,8 +120,12 @@ class WalletCacheServiceTests: XCTestCase { XCTAssert(walletCacheService.readWalletsFromDiskAndDecrypt()?.count == 0) // Check we can write wallet objects - XCTAssert((try? walletCacheService.cache(wallet: MockConstants.defaultLinearWallet, childOfIndex: nil, backedUp: false)) != nil) - XCTAssert((try? walletCacheService.cache(wallet: MockConstants.defaultHdWallet, childOfIndex: nil, backedUp: false)) != nil) + do { + try walletCacheService.cache(wallet: MockConstants.defaultLinearWallet, childOfIndex: nil, backedUp: false) + try walletCacheService.cache(wallet: MockConstants.defaultHdWallet, childOfIndex: nil, backedUp: false) + } catch { + XCTFail("Should not error: \(error)") + } // Rmeove Linear XCTAssert(walletCacheService.deleteWallet(withAddress: MockConstants.defaultLinearWallet.address, parentIndex: nil)) @@ -114,9 +136,13 @@ class WalletCacheServiceTests: XCTestCase { XCTAssert(walletCacheService.readWalletsFromDiskAndDecrypt()?.count == 0) // Add 2 children to the HDWallet - XCTAssert((try? walletCacheService.cache(wallet: MockConstants.defaultHdWallet, childOfIndex: nil, backedUp: false)) != nil) - XCTAssert((try? walletCacheService.cache(wallet: MockConstants.defaultHdWallet.createChild(accountIndex: 1) ?? MockConstants.defaultHdWallet, childOfIndex: 0, backedUp: false)) != nil) - XCTAssert((try? walletCacheService.cache(wallet: MockConstants.defaultHdWallet.createChild(accountIndex: 2) ?? MockConstants.defaultHdWallet, childOfIndex: 0, backedUp: false)) != nil) + do { + try walletCacheService.cache(wallet: MockConstants.defaultHdWallet, childOfIndex: nil, backedUp: false) + try walletCacheService.cache(wallet: MockConstants.defaultHdWallet.createChild(accountIndex: 1) ?? MockConstants.defaultHdWallet, childOfIndex: 0, backedUp: false) + try walletCacheService.cache(wallet: MockConstants.defaultHdWallet.createChild(accountIndex: 2) ?? MockConstants.defaultHdWallet, childOfIndex: 0, backedUp: false) + } catch { + XCTFail("Should not error: \(error)") + } // Delete the first child XCTAssert(walletCacheService.deleteWallet(withAddress: MockConstants.hdWallet.childWalletAddresses[0], parentIndex: 0)) @@ -131,7 +157,11 @@ class WalletCacheServiceTests: XCTestCase { func testDerivationPaths() { let wallet = HDWallet(withMnemonic: MockConstants.mnemonic, passphrase: MockConstants.passphrase, derivationPath: MockConstants.hdWallet_hardened_change.derivationPath)! - XCTAssert((try? walletCacheService.cache(wallet: wallet, childOfIndex: nil, backedUp: false)) != nil) + do { + try walletCacheService.cache(wallet: wallet, childOfIndex: nil, backedUp: false) + } catch { + XCTFail("Should not error: \(error)") + } let wallet1 = walletCacheService.fetchWallet(forAddress: wallet.address) as? HDWallet XCTAssert(wallet1 != nil) @@ -142,7 +172,11 @@ class WalletCacheServiceTests: XCTestCase { func testPassphrase() { let wallet = RegularWallet(withMnemonic: MockConstants.mnemonic, passphrase: MockConstants.passphrase)! - XCTAssert((try? walletCacheService.cache(wallet: wallet, childOfIndex: nil, backedUp: false)) != nil) + do { + try walletCacheService.cache(wallet: wallet, childOfIndex: nil, backedUp: false) + } catch { + XCTFail("Should not error: \(error)") + } let wallet1 = walletCacheService.fetchWallet(forAddress: wallet.address) XCTAssert(wallet1 != nil) @@ -160,16 +194,22 @@ class WalletCacheServiceTests: XCTestCase { let hdWallet4 = HDWallet(withMnemonic: mnemonic, passphrase: "abc")! - // Set 2 wallets - let _ = try? walletCacheService.cache(wallet: hdWallet1, childOfIndex: nil, backedUp: false) var list = walletCacheService.readMetadataFromDiskAndDecrypt() - let groupName1 = list.metadata(forAddress: hdWallet1.address)?.hdWalletGroupName - XCTAssert(groupName1 == "HD Wallet 1", groupName1 ?? "-") - let _ = try? walletCacheService.cache(wallet: hdWallet2, childOfIndex: nil, backedUp: false) - list = walletCacheService.readMetadataFromDiskAndDecrypt() - let groupName2 = list.metadata(forAddress: hdWallet2.address)?.hdWalletGroupName - XCTAssert(groupName2 == "HD Wallet 2", groupName2 ?? "-") + // Set 2 wallets + do { + let _ = try walletCacheService.cache(wallet: hdWallet1, childOfIndex: nil, backedUp: false) + list = walletCacheService.readMetadataFromDiskAndDecrypt() + let groupName1 = list.metadata(forAddress: hdWallet1.address)?.hdWalletGroupName + XCTAssert(groupName1 == "HD Wallet 1", groupName1 ?? "-") + + let _ = try walletCacheService.cache(wallet: hdWallet2, childOfIndex: nil, backedUp: false) + list = walletCacheService.readMetadataFromDiskAndDecrypt() + let groupName2 = list.metadata(forAddress: hdWallet2.address)?.hdWalletGroupName + XCTAssert(groupName2 == "HD Wallet 2", groupName2 ?? "-") + } catch { + XCTFail("Should not error: \(error)") + } // Update one and check @@ -182,10 +222,14 @@ class WalletCacheServiceTests: XCTestCase { // Add another to check did it reuse the name "HD Wallet 2" - let _ = try? walletCacheService.cache(wallet: hdWallet3, childOfIndex: nil, backedUp: false) - list = walletCacheService.readMetadataFromDiskAndDecrypt() - let groupName4 = list.metadata(forAddress: hdWallet3.address)?.hdWalletGroupName - XCTAssert(groupName4 == "HD Wallet 2", groupName4 ?? "-") + do { + let _ = try walletCacheService.cache(wallet: hdWallet3, childOfIndex: nil, backedUp: false) + list = walletCacheService.readMetadataFromDiskAndDecrypt() + let groupName4 = list.metadata(forAddress: hdWallet3.address)?.hdWalletGroupName + XCTAssert(groupName4 == "HD Wallet 2", groupName4 ?? "-") + } catch { + XCTFail("Should not error: \(error)") + } // Change all names and add 4th @@ -193,10 +237,14 @@ class WalletCacheServiceTests: XCTestCase { let _ = list.set(hdWalletGroupName: "Blah 3", forAddress: hdWallet3.address) let _ = walletCacheService.encryptAndWriteMetadataToDisk(list) - let _ = try? walletCacheService.cache(wallet: hdWallet4, childOfIndex: nil, backedUp: false) - list = walletCacheService.readMetadataFromDiskAndDecrypt() - let groupName5 = list.metadata(forAddress: hdWallet4.address)?.hdWalletGroupName - XCTAssert(groupName5 == "HD Wallet 4", groupName5 ?? "-") + do { + let _ = try walletCacheService.cache(wallet: hdWallet4, childOfIndex: nil, backedUp: false) + list = walletCacheService.readMetadataFromDiskAndDecrypt() + let groupName5 = list.metadata(forAddress: hdWallet4.address)?.hdWalletGroupName + XCTAssert(groupName5 == "HD Wallet 4", groupName5 ?? "-") + } catch { + XCTFail("Should not error: \(error)") + } } func testMetadata() {