Review comments addressed

CHANGELOG.md

@@ -22,8 +22,8 @@ and this library adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `SDKSynchronizer.getAccountBalance -> AccountBalance?` into `SDKSynchronizer.getAccountsBalances -> [AccountUUID: AccountBalance]`
 
 ## Removed
-- `SDKSynchronizer.sendToAddress`, deperacted in 2.1
-- `SDKSynchronizer.shieldFunds`, deperacted in 2.1
+- `SDKSynchronizer.sendToAddress`, deprecated in 2.1
+- `SDKSynchronizer.shieldFunds`, deprecated in 2.1
 
 ## Checkpoints
 

Package.swift

@@ -16,7 +16,7 @@ let package = Package(
     dependencies: [
         .package(url: "https://github.com/grpc/grpc-swift.git", from: "1.24.2"),
         .package(url: "https://github.com/stephencelis/SQLite.swift.git", from: "0.15.3"),
-        .package(url: "https://github.com/Electric-Coin-Company/zcash-light-client-ffi", from: "0.12.0")
+        .package(url: "https://github.com/Electric-Coin-Company/zcash-light-client-ffi", exact: "0.12.0")
     ],
     targets: [
         .target(

Sources/ZcashLightClientKit/Constants/ZcashSDK.swift

@@ -220,7 +220,7 @@ public enum ZcashSDKTestnetConstants: NetworkConstants {
     public static let defaultDbNamePrefix = "ZcashSdk_testnet_"
 }
 
-/// Used when importing an aaccount `importAccount(..., purpose: AccountPurpose)`
+/// Used when importing an account `importAccount(..., purpose: AccountPurpose)`
 public enum AccountPurpose: UInt32, Equatable {
     case spending = 0
     case viewOnly

Sources/ZcashLightClientKit/Error/ZcashError.swift

@@ -384,6 +384,14 @@ public enum ZcashError: Equatable, Error {
     /// - `rustError` contains error generated by the rust layer.
     /// ZRUST0071
     case rustExtractAndStoreTxFromPCZT(_ rustError: String)
+    /// Error from rust layer when calling ZcashRustBackend.getAccount
+    /// - `rustError` contains error generated by the rust layer.
+    /// ZRUST0072
+    case rustUUIDAccountNotFound(_ rustError: String)
+    /// Error from rust layer when calling ZcashRustBackend.extractAndStoreTxFromPCZT
+    /// - `rustError` contains error generated by the rust layer.
+    /// ZRUST0073
+    case rustTxidPtrIncorrectLength(_ rustError: String)
     /// SQLite query failed when fetching all accounts from the database.
     /// - `sqliteError` is error produced by SQLite library.
     /// ZADAO0001
@@ -757,6 +765,8 @@ public enum ZcashError: Equatable, Error {
         case .rustCreatePCZTFromProposal: return "Error from rust layer when calling ZcashRustBackend.createPCZTFromProposal"
         case .rustAddProofsToPCZT: return "Error from rust layer when calling ZcashRustBackend.addProofsToPCZT"
         case .rustExtractAndStoreTxFromPCZT: return "Error from rust layer when calling ZcashRustBackend.extractAndStoreTxFromPCZT"
+        case .rustUUIDAccountNotFound: return "Error from rust layer when calling ZcashRustBackend.getAccount"
+        case .rustTxidPtrIncorrectLength: return "Error from rust layer when calling ZcashRustBackend.extractAndStoreTxFromPCZT"
         case .accountDAOGetAll: return "SQLite query failed when fetching all accounts from the database."
         case .accountDAOGetAllCantDecode: return "Fetched accounts from SQLite but can't decode them."
         case .accountDAOFindBy: return "SQLite query failed when seaching for accounts in the database."
@@ -947,6 +957,8 @@ public enum ZcashError: Equatable, Error {
         case .rustCreatePCZTFromProposal: return .rustCreatePCZTFromProposal
         case .rustAddProofsToPCZT: return .rustAddProofsToPCZT
         case .rustExtractAndStoreTxFromPCZT: return .rustExtractAndStoreTxFromPCZT
+        case .rustUUIDAccountNotFound: return .rustUUIDAccountNotFound
+        case .rustTxidPtrIncorrectLength: return .rustTxidPtrIncorrectLength
         case .accountDAOGetAll: return .accountDAOGetAll
         case .accountDAOGetAllCantDecode: return .accountDAOGetAllCantDecode
         case .accountDAOFindBy: return .accountDAOFindBy

Sources/ZcashLightClientKit/Error/ZcashErrorCode.swift

@@ -205,6 +205,10 @@ public enum ZcashErrorCode: String {
     case rustAddProofsToPCZT = "ZRUST0070"
     /// Error from rust layer when calling ZcashRustBackend.extractAndStoreTxFromPCZT
     case rustExtractAndStoreTxFromPCZT = "ZRUST0071"
+    /// Error from rust layer when calling ZcashRustBackend.getAccount
+    case rustUUIDAccountNotFound = "ZRUST0072"
+    /// Error from rust layer when calling ZcashRustBackend.extractAndStoreTxFromPCZT
+    case rustTxidPtrIncorrectLength = "ZRUST0073"
     /// SQLite query failed when fetching all accounts from the database.
     case accountDAOGetAll = "ZADAO0001"
     /// Fetched accounts from SQLite but can't decode them.

Sources/ZcashLightClientKit/Error/ZcashErrorCodeDefinition.swift

@@ -406,8 +406,15 @@ enum ZcashErrorDefinition {
     /// - `rustError` contains error generated by the rust layer.
     // sourcery: code="ZRUST0071"
     case rustExtractAndStoreTxFromPCZT(_ rustError: String)
+    /// Error from rust layer when calling ZcashRustBackend.getAccount
+    /// - `rustError` contains error generated by the rust layer.
+    // sourcery: code="ZRUST0072"
+    case rustUUIDAccountNotFound(_ rustError: String)
+    /// Error from rust layer when calling ZcashRustBackend.extractAndStoreTxFromPCZT
+    /// - `rustError` contains error generated by the rust layer.
+    // sourcery: code="ZRUST0073"
+    case rustTxidPtrIncorrectLength(_ rustError: String)
 
-
     // MARK: - Account DAO
 
     /// SQLite query failed when fetching all accounts from the database.

Sources/ZcashLightClientKit/Rust/ZcashKeyDerivationBackendWelding.swift

@@ -41,8 +41,8 @@ protocol ZcashKeyDerivationBackendWelding {
     func isValidUnifiedFullViewingKey(_ ufvk: String) -> Bool
 
     /// Derives and returns a UnifiedAddress from a UnifiedFullViewingKey
-    /// - Parameter ufvk: UTF-8 encoded String to validate
-    /// - Returns: true `UnifiedAddress`
+    /// - Parameter ufvk: UTF-8 encoded String containing a valid UFVK
+    /// - Returns: the corresponding default `UnifiedAddress`
     func deriveUnifiedAddressFrom(ufvk: String) throws -> UnifiedAddress
 
     /// Derives and returns a unified spending key from the given seed and ZIP 32 account index.

Sources/ZcashLightClientKit/Rust/ZcashRustBackend.swift

@@ -111,12 +111,16 @@ struct ZcashRustBackend: ZcashRustBackendWelding {
         )
 
         guard let accountPtr else {
-            throw ZcashError.rustImportAccountUfvk(lastErrorMessage(fallback: "`importAccount` failed with unknown error"))
+            throw ZcashError.rustImportAccountUfvk(lastErrorMessage(fallback: "`getAccount` failed with unknown error"))
         }
 
         defer { zcashlc_free_account(accountPtr) }
 
-        return accountPtr.pointee.unsafeToAccount()
+        guard let validAccount = accountPtr.pointee.unsafeToAccount() else {
+            throw ZcashError.rustUUIDAccountNotFound(lastErrorMessage(fallback: "`getAccount` failed with unknown error"))
+        }
+
+        return validAccount
     }
 
     // swiftlint:disable:next function_parameter_count
@@ -358,7 +362,7 @@ struct ZcashRustBackend: ZcashRustBackendWelding {
         pcztWithProofs: Pczt,
         pcztWithSigs: Pczt
     ) async throws -> Data {
-        let pcztPtr: UnsafeMutablePointer<FfiBoxedSlice>? = pcztWithProofs.withUnsafeBytes { pcztWithProofsBuffer in
+        let txidPtr: UnsafeMutablePointer<FfiBoxedSlice>? = pcztWithProofs.withUnsafeBytes { pcztWithProofsBuffer in
             guard let pcztWithProofsBufferPtr = pcztWithProofsBuffer.baseAddress?.assumingMemoryBound(to: UInt8.self) else {
                 return nil
             }
@@ -384,15 +388,19 @@ struct ZcashRustBackend: ZcashRustBackendWelding {
             }
         }
 
-        guard let pcztPtr else {
+        guard let txidPtr else {
             throw ZcashError.rustExtractAndStoreTxFromPCZT(lastErrorMessage(fallback: "`extractAndStoreTxFromPCZT` failed with unknown error"))
         }
 
-        defer { zcashlc_free_boxed_slice(pcztPtr) }
+        guard txidPtr.pointee.len == 32 else {
+            throw ZcashError.rustTxidPtrIncorrectLength(lastErrorMessage(fallback: "`extractAndStoreTxFromPCZT` failed with unknown error"))
+        }
+
+        defer { zcashlc_free_boxed_slice(txidPtr) }
 
         return Data(
-            bytes: pcztPtr.pointee.ptr,
-            count: Int(pcztPtr.pointee.len)
+            bytes: txidPtr.pointee.ptr,
+            count: Int(txidPtr.pointee.len)
         )
     }
 
@@ -1163,8 +1171,25 @@ extension FfiAccount {
 
     /// converts an [`FfiAccount`] into a [`Account`]
     /// - Note: This does not check that the converted value actually holds a valid Account
-    func unsafeToAccount() -> Account {
-        .init(
+    func unsafeToAccount() -> Account? {
+        // Invalid UUID check
+        guard uuidArray != [UInt8](repeating: 0, count: 16) else {
+            return nil
+        }
+
+        // Invalid ZIP 32 account index
+        if hd_account_index == UInt32.max {
+            return .init(
+                id: AccountUUID(id: uuidArray),
+                name: account_name != nil ? String(cString: account_name) : nil,
+                keySource: key_source != nil ? String(cString: key_source) : nil,
+                seedFingerprint: nil,
+                hdAccountIndex: nil
+            )
+        }
+
+        // Valid ZIP32 account index
+        return .init(
             id: AccountUUID(id: uuidArray),
             name: account_name != nil ? String(cString: account_name) : nil,
             keySource: key_source != nil ? String(cString: key_source) : nil,

Sources/ZcashLightClientKit/Rust/ZcashRustBackendWelding.swift

@@ -38,6 +38,15 @@ protocol ZcashRustBackendWelding {
     /// Returns a list of the accounts in the wallet.
     func listAccounts() async throws -> [Account]
 
+    /// Adds a new account to the wallet by importing the UFVK that will be used to detect incoming
+    /// payments.
+    ///
+    /// Derivation metadata may optionally be included. To indicate that no derivation metadata is
+    /// available, `seedFingerprint` and `zip32AccountIndex` should be set to `nil`. Derivation
+    /// metadata will not be stored unless both the seed fingerprint and the HD account index are
+    /// provided.
+    ///
+    /// - Returns: the globally unique identifier for the account.
     // swiftlint:disable:next function_parameter_count
     func importAccount(
         ufvk: String,

Sources/ZcashLightClientKit/Synchronizer/SDKSynchronizer.swift

@@ -425,12 +425,12 @@ public class SDKSynchronizer: Synchronizer {
             logger: logger
         )
 
-        let txIds = try await initializer.rustBackend.extractAndStoreTxFromPCZT(
+        let txId = try await initializer.rustBackend.extractAndStoreTxFromPCZT(
             pcztWithProofs: pcztWithProofs,
             pcztWithSigs: pcztWithSigs
         )
 
-        let transactions = try await transactionEncoder.createTransactionsFromTxIds([txIds])
+        let transactions = try await transactionEncoder.fetchTransactionsForTxIds([txId])
 
         return submitTransactions(transactions)
     }

Sources/ZcashLightClientKit/Tool/DerivationTool.swift

@@ -18,8 +18,8 @@ public protocol KeyValidation {
 
 public protocol KeyDeriving {
     /// Derives and returns a UnifiedAddress from a UnifiedFullViewingKey
-    /// - Parameter ufvk: UTF-8 encoded String to validate
-    /// - Returns: true `UnifiedAddress`
+    /// - Parameter ufvk: UTF-8 encoded String containing a valid UFVK
+    /// - Returns: the corresponding default `UnifiedAddress`
     func deriveUnifiedAddressFrom(ufvk: String) throws -> UnifiedAddress
 
     /// Given the seed bytes and ZIP 32 account index, return the corresponding UnifiedSpendingKey.

Sources/ZcashLightClientKit/Transaction/TransactionEncoder.swift

@@ -91,7 +91,9 @@ protocol TransactionEncoder {
     /// - Parameter transaction: a transaction overview
     func submit(transaction: EncodedTransaction) async throws
 
-    func createTransactionsFromTxIds(_ txIds: [Data]) async throws -> [ZcashTransaction.Overview]
+    /// Tries to fetch the transaction for the given transction ids.
+    /// - Parameter txIds: an array of transaction ids to be fetched from the DB.
+    func fetchTransactionsForTxIds(_ txIds: [Data]) async throws -> [ZcashTransaction.Overview]
 
     func closeDBConnection()
 }

Sources/ZcashLightClientKit/Transaction/WalletTransactionEncoder.swift

@@ -111,14 +111,10 @@ class WalletTransactionEncoder: TransactionEncoder {
             usk: spendingKey
         )
 
-        return try await createTransactionsFromTxIds(txIds)
+        return try await fetchTransactionsForTxIds(txIds)
     }
 
-    func createTransactionsFromTxIds(_ txIds: [Data]) async throws -> [ZcashTransaction.Overview] {
-        guard ensureParams(spend: self.spendParamsURL, output: self.outputParamsURL) else {
-            throw ZcashError.walletTransEncoderCreateTransactionMissingSaplingParams
-        }
-
+    func fetchTransactionsForTxIds(_ txIds: [Data]) async throws -> [ZcashTransaction.Overview] {
         logger.debug("transaction ids: \(txIds)")
 
         var txs: [ZcashTransaction.Overview] = []

Tests/PerformanceTests/SynchronizerTests.swift

@@ -47,7 +47,7 @@ class SynchronizerTests: ZcashTestCase {
         rustBackend = nil
     }
 
-    // TODO: [#1518] Fix the test, https://github.com/Electric-Coin-Company/zcash-swift-wallet-sdk/issues/1518
+    // TODO: [#1521] Fix the test, https://github.com/Electric-Coin-Company/zcash-swift-wallet-sdk/issues/1521
     func _testHundredBlocksSync() async throws {
         guard let seedData = Data(base64Encoded: "9VDVOZZZOWWHpZtq1Ebridp3Qeux5C+HwiRR0g7Oi7HgnMs8Gfln83+/Q1NnvClcaSwM4ADFL1uZHxypEWlWXg==") else {
             XCTFail("seedData expected to be successfuly instantiated.")

Tests/TestUtils/TestsData.swift

@@ -125,5 +125,6 @@ class TestsData {
 }
 
 extension TestsData {
-    static let mockedAccountUUID = AccountUUID(id: Array<UInt8>(repeating: 0, count: 16))
+    /// `mockedAccountUUID` is used to mock the rust backend for check only purposes. It's never passed to it.
+    static let mockedAccountUUID = AccountUUID(id: [UInt8](repeating: 0, count: 16))
 }