change WalletCache to only throw to avoid complexities of checking bo…

…ol and catch
kukai-wallet · Jan 22, 2024 · 0dec578 · 0dec578
1 parent 4ec1921
commit 0dec578
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 31 deletions.
diff --git 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<T: Wallet>(wallet: T, childOfIndex: Int?, backedUp: Bool) throws -> Bool {
+	public func cache<T: Wallet>(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
@@ -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,21 +222,29 @@ 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
 		let _ = list.set(hdWalletGroupName: "Blah 1", forAddress: hdWallet1.address)
 		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() {