improved doc, code, naming and implement LocalizedError

Author: albertopeam

Sources/Web3Core/KeystoreManager/BIP44.swift

@@ -7,66 +7,72 @@ import Foundation
 
 public protocol BIP44 {
     /**
-    Derive an `HDNode` based on the provided `path`. The function will throws `BIP44Error.warning` if it was invoked with `warns` as true and the root key doesn't have a previous child with at least one transaction, using false the child node will be derived directly not throwing. This function needs to query the blockchain history when `warns`is true,  so it can throw network errors.
-    - Parameter path: valid BIP32 path.
-    - Parameter warns: true to be warned about following BIP44 standard, false otherwise.
-    - Throws: `BIP44Error.warning` if the child key shouldn't be used according to BIP44 standard.
-    - Returns: an HDNode child key for the provided `path` if it can be created, otherwise nil
+    Derive an ``HDNode`` based on the provided path. The function will throw ``BIP44Error.warning`` if it was invoked with throwOnError equal to`true` and the root key doesn't have a previous child with at least one transaction. If it is invoked with throwOnError equal to `false` the child node will be derived directly using the derive function of ``HDNode``. This function needs to query the blockchain history when throwOnError is `true`,  so it can throw network errors.
+    - Parameter path: valid BIP44 path.
+    - Parameter throwOnError: `true` to use [Account Discovery](https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki#account-discovery) standard,  otherwise it will dervive the key using the derive function of ``HDNode``.
+    - Throws: ``BIP44Error.warning`` if the child key shouldn't be used according to [Account Discovery](https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki#account-discovery) standard.
+    - Returns: an ``HDNode`` child key for the provided `path` if it can be created, otherwise `nil`
     */
-    func derive(path: String, warns: Bool, transactionChecker: TransactionChecker) async throws -> HDNode?
+    func derive(path: String, throwOnError: Bool, transactionChecker: TransactionChecker) async throws -> HDNode?
 }
 
-public enum BIP44Error: Error, Equatable {
+public enum BIP44Error: Error, Equatable, LocalizedError {
     /// The selected path doesn't fulfill BIP44 standard, you can derive the root key anyway
     case warning
+    
+    public var errorDescription: String? {
+        switch self {
+        case .warning:
+            return "Couldn't derive key as it doesn't have a previous account with at least one transaction"
+        }
+    }
 }
 
 public protocol TransactionChecker {
     /**
-    It verifies if the provided `address` has at least one transaction
+    It verifies if the provided address has at least one transaction
     - Parameter address: to be queried
     - Throws: any error related to query the blockchain provider
-    - Returns: true if the `address` has at least one transaction, false otherwise
+    - Returns: `true` if the address has at least one transaction, `false` otherwise
     */
     func hasTransactions(address: String) async throws -> Bool
 }
 
 extension HDNode: BIP44 {
-    public func derive(path: String, warns: Bool = true, transactionChecker: TransactionChecker) async throws -> HDNode? {
-        if warns {
-            guard let account = path.accountFromPath else {
-                return nil
-            }
-            if account == 0 {
-                return derive(path: path, derivePrivateKey: true)
-            } else {
-                for searchAccount in 0..<account {
-                    let maxUnusedAddressIndexes = 20
-                    var hasTransactions = false
-                    for searchAddressIndex in 0..<maxUnusedAddressIndexes {
-                        if let searchPath = path.newPath(account: searchAccount, addressIndex: searchAddressIndex),
-                        let childNode = derive(path: searchPath, derivePrivateKey: true),
-                        let ethAddress = Utilities.publicToAddress(childNode.publicKey) {
-                            hasTransactions = try await transactionChecker.hasTransactions(address: ethAddress.address)
-                            if hasTransactions {
-                                break
-                            }
+    public func derive(path: String, throwOnError: Bool = true, transactionChecker: TransactionChecker) async throws -> HDNode? {
+        guard throwOnError else {
+            return derive(path: path, derivePrivateKey: true)
+        }
+        guard let account = path.accountFromPath else {
+            return nil
+        }
+        if account == 0 {
+            return derive(path: path, derivePrivateKey: true)
+        } else {
+            for searchAccount in 0..<account {
+                let maxUnusedAddressIndexes = 20
+                var hasTransactions = false
+                for searchAddressIndex in 0..<maxUnusedAddressIndexes {
+                    if let searchPath = path.newPath(account: searchAccount, addressIndex: searchAddressIndex),
+                    let childNode = derive(path: searchPath, derivePrivateKey: true),
+                    let ethAddress = Utilities.publicToAddress(childNode.publicKey) {
+                        hasTransactions = try await transactionChecker.hasTransactions(address: ethAddress.address)
+                        if hasTransactions {
+                            break
                         }
                     }
-                    if !hasTransactions {
-                        throw BIP44Error.warning
-                    }
                 }
-                return derive(path: path, derivePrivateKey: true)
+                if !hasTransactions {
+                    throw BIP44Error.warning
+                }
             }
-        } else {
             return derive(path: path, derivePrivateKey: true)
         }
     }
 }
 
 extension String {
-    /// Verifies if current value matches BIP44 path standard
+    /// Verifies if self matches BIP44 path
     var isBip44Path: Bool {
         do {
             let pattern = "^m/44'/\\d+'/\\d+'/[0-1]/\\d+$"
@@ -78,7 +84,7 @@ extension String {
         }
     }
 
-    /// Returns the account from the path if the string contains a well formed BIP44 path
+    /// Returns the account from the path if self contains a well formed BIP44 path
     var accountFromPath: Int? {
         guard isBip44Path else {
             return nil
@@ -93,10 +99,10 @@ extension String {
     }
 
     /**
-    Transforms a bip44 path into a new one changing `account` & `index`. The resulting one will have the change value equal to `0` to represent external chain. Format `m/44'/coin_type'/account'/change/address_index`
-    - Parameter account: the new `account` to use
-    - Parameter addressIndex: the new `addressIndex` to use
-    - Returns: a valid bip44 path with the provided `account`, `addressIndex`and external `change` or nil otherwise
+    Transforms a bip44 path into a new one changing account & index. The resulting one will have the change value equal to `0` to represent the external chain. The format will be `m/44'/coin_type'/account'/change/address_index`
+    - Parameter account: the new account to use
+    - Parameter addressIndex: the new addressIndex to use
+    - Returns: a valid bip44 path with the provided account, addressIndex and external change or `nil` otherwise
     */
     func newPath(account: Int, addressIndex: Int) -> String? {
         guard isBip44Path else {

Tests/web3swiftTests/localTests/BIP44Tests.swift

@@ -60,7 +60,7 @@ final class BIP44Tests: XCTestCase {
     func testDeriveNoWarn() async throws {
         let rootNode = try rootNode()
 
-        let childNode = try await rootNode.derive(path: "m/44'/60'/8096'/0/1", warns: false, transactionChecker: mockTransactionChecker)
+        let childNode = try await rootNode.derive(path: "m/44'/60'/8096'/0/1", throwOnError: false, transactionChecker: mockTransactionChecker)
 
         XCTAssertEqual(try XCTUnwrap(childNode).publicKey.toHexString(), "035785d4918449c87892371c0f9ccf6e4eda40a7fb0f773f1254c064d3bba64026")
         XCTAssertEqual(mockTransactionChecker.addresses.count, 0)
@@ -69,7 +69,7 @@ final class BIP44Tests: XCTestCase {
     func testDeriveInvalidPath() async throws {
         let rootNode = try rootNode()
 
-        let childNode = try? await rootNode.derive(path: "", warns: true, transactionChecker: mockTransactionChecker)
+        let childNode = try? await rootNode.derive(path: "", throwOnError: true, transactionChecker: mockTransactionChecker)
 
         XCTAssertNil(childNode)
         XCTAssertEqual(mockTransactionChecker.addresses.count, 0)
@@ -80,7 +80,7 @@ final class BIP44Tests: XCTestCase {
     func testAccountZeroCanBeDerivedAlways() async throws {
         let rootNode = try rootNode()
 
-        let childNode = try await rootNode.derive(path: "m/44'/60'/0'/0/255", warns: true, transactionChecker: mockTransactionChecker)
+        let childNode = try await rootNode.derive(path: "m/44'/60'/0'/0/255", throwOnError: true, transactionChecker: mockTransactionChecker)
 
         XCTAssertEqual(try XCTUnwrap(childNode).publicKey.toHexString(), "0262fba1af8f149258123265318114066decf50d16c1222a9d657b7de2296c2734")
         XCTAssertEqual(mockTransactionChecker.addresses.count, 0)
@@ -94,7 +94,7 @@ final class BIP44Tests: XCTestCase {
             results.append(true)
             mockTransactionChecker.results = results
 
-            _ = try await rootNode.derive(path: path, warns: true, transactionChecker: mockTransactionChecker)
+            _ = try await rootNode.derive(path: path, throwOnError: true, transactionChecker: mockTransactionChecker)
 
             XCTFail("Child must not be created usign warns true for the path: \(path)")
         } catch BIP44Error.warning {
@@ -111,7 +111,7 @@ final class BIP44Tests: XCTestCase {
             results.append(true)
             mockTransactionChecker.results = results
 
-            let childNode = try await rootNode.derive(path: path, warns: true, transactionChecker: mockTransactionChecker)
+            let childNode = try await rootNode.derive(path: path, throwOnError: true, transactionChecker: mockTransactionChecker)
 
             XCTAssertEqual(try XCTUnwrap(childNode).publicKey.toHexString(), "036cd8f1bad46fa7caf7a80d48528b90db2a3b7a5c9a18d74d61b286e03850abf4")
             XCTAssertEqual(mockTransactionChecker.addresses, accountZeroScannedAddresses)
@@ -129,7 +129,7 @@ final class BIP44Tests: XCTestCase {
             results.append(contentsOf: false.times(n: 20))
             mockTransactionChecker.results = results
 
-            _ = try await rootNode.derive(path: path, warns: true, transactionChecker: mockTransactionChecker)
+            _ = try await rootNode.derive(path: path, throwOnError: true, transactionChecker: mockTransactionChecker)
 
             XCTFail("Child must not be created usign warns true for the path: \(path)")
         } catch BIP44Error.warning {
@@ -149,7 +149,7 @@ final class BIP44Tests: XCTestCase {
             results.append(true)
             mockTransactionChecker.results = results
 
-            let childNode = try await rootNode.derive(path: path, warns: true, transactionChecker: mockTransactionChecker)
+            let childNode = try await rootNode.derive(path: path, throwOnError: true, transactionChecker: mockTransactionChecker)
 
             XCTAssertEqual(try XCTUnwrap(childNode).publicKey.toHexString(), "0282134e44d4c040a4b4c1a780d8302955096cf1d5e738b161c83f0ce1b863c14e")
             XCTAssertEqual(mockTransactionChecker.addresses, accountZeroScannedAddresses)
@@ -167,6 +167,13 @@ final class BIP44Tests: XCTestCase {
     }
 }
 
+class BIP44ErrorTests: XCTestCase {
+    func testLocalizedDescription() {
+        let error = BIP44Error.warning
+        XCTAssertEqual(error.localizedDescription, "Couldn't derive key as it doesn't have a previous account with at least one transaction")
+    }
+}
+
 extension Bool {
     func times(n: Int) -> [Bool] {
         var array: [Bool] = .init()