Fixes issue 136 - expiry height -1 on pending transactions (#139)

ZcashLightClientKit/Transaction/PersistentTransactionManager.swift

@@ -45,11 +45,14 @@ class PersistentTransactionManager: OutboundTransactionManager {
             guard let self = self else { return }
             do {
                 let encodedTransaction = try self.encoder.createTransaction(spendingKey: spendingKey, zatoshi: pendingTransaction.value, to: pendingTransaction.toAddress, memo: pendingTransaction.memo?.asZcashTransactionMemo(), from: pendingTransaction.accountIndex)
+                let transaction = try self.encoder.expandEncodedTransaction(encodedTransaction)
+                
                 var pending = pendingTransaction
                 pending.encodeAttempts = pending.encodeAttempts + 1
                 pending.raw = encodedTransaction.raw
                 pending.rawTransactionId = encodedTransaction.transactionId
-                
+                pending.expiryHeight = transaction.expiryHeight ?? BlockHeight.empty()
+                pending.minedHeight = transaction.minedHeight ?? BlockHeight.empty()
                 try self.repository.update(pending)
                 result(.success(pending))
             } catch StorageError.updateFailed {

ZcashLightClientKit/Transaction/TransactionEncoder.swift

@@ -14,6 +14,7 @@ public enum TransactionEncoderError: Error {
     case NotEncoded(transactionId: Int)
     case missingParams
     case spendingKeyWrongNetwork
+    case couldNotExpand(txId: Data)
 }
 
 protocol TransactionEncoder {
@@ -51,4 +52,11 @@ protocol TransactionEncoder {
     */
     func createTransaction(spendingKey: String, zatoshi: Int, to: String, memo: String?, from accountIndex: Int, result: @escaping TransactionEncoderResultBlock)
     
+    /**
+        Fetch the Transaction Entity from the encoded representation
+     - Parameter encodedTransaction: The encoded transaction to expand
+     - Returns: a TransactionEntity based on the given Encoded Transaction
+     - Throws: a TransactionEncoderError
+     */
+    func expandEncodedTransaction(_ encodedTransaction: EncodedTransaction) throws -> TransactionEntity
 }

ZcashLightClientKit/Transaction/WalletTransactionEncoder.swift

@@ -11,12 +11,11 @@ class WalletTransactionEncoder: TransactionEncoder {
     
     var rustBackend: ZcashRustBackendWelding.Type
     var repository: TransactionRepository
-//    var initializer: Initializer
     var queue: DispatchQueue
     private var outputParamsURL: URL
     private var spendParamsURL: URL
     private var dataDbURL: URL
-
+    
     init(rust: ZcashRustBackendWelding.Type,
          dataDb: URL,
          repository: TransactionRepository,
@@ -75,7 +74,7 @@ class WalletTransactionEncoder: TransactionEncoder {
         guard ensureParams(spend: self.spendParamsURL, output: self.spendParamsURL) else {
             throw TransactionEncoderError.missingParams
         }
-                
+        
         let txId = rustBackend.createToAddress(dbData: self.dataDbURL, account: Int32(accountIndex), extsk: spendingKey, to: address, value: Int64(zatoshi), memo: memo, spendParamsPath: self.spendParamsURL.path, outputParamsPath: self.outputParamsURL.path)
         
         guard txId > 0 else {
@@ -92,4 +91,17 @@ class WalletTransactionEncoder: TransactionEncoder {
         
         return readableSpend && readableOutput // Todo: change this to something that makes sense
     }
+    
+    /**
+     Fetch the Transaction Entity from the encoded representation
+     - Parameter encodedTransaction: The encoded transaction to expand
+     - Returns: a TransactionEntity based on the given Encoded Transaction
+     - Throws: a TransactionEncoderError
+     */
+    func expandEncodedTransaction(_ encodedTransaction: EncodedTransaction) throws -> TransactionEntity {
+        guard let t = try? repository.findBy(rawId: encodedTransaction.transactionId) else {
+            throw TransactionEncoderError.couldNotExpand(txId: encodedTransaction.transactionId)
+        }
+        return t
+    }
 }

ZcashLightClientKitTests/BalanceTests.swift

@@ -174,11 +174,11 @@ class BalanceTests: XCTestCase {
         }
         //  (previous available funds - spent note + change) equals to (previous available funds - sent amount)
         
-            self.verifiedBalanceValidation(previousBalance: presendBalance,
+        self.verifiedBalanceValidation(previousBalance: presendBalance,
-                                           spentNoteValue:  Int64(sentNote.value),
+                                       spentNoteValue:  Int64(sentNote.value),
-                                           changeValue: Int64(receivedNote.value),
+                                       changeValue: Int64(receivedNote.value),
-                                           sentAmount: Int64(self.sendAmount),
+                                       sentAmount: Int64(self.sendAmount),
-                                           currentVerifiedBalance: self.coordinator.synchronizer.initializer.getVerifiedBalance())
+                                       currentVerifiedBalance: self.coordinator.synchronizer.initializer.getVerifiedBalance())
         
     }
     
@@ -200,7 +200,7 @@ class BalanceTests: XCTestCase {
      */
     func testVerifyTotalBalanceDuringSend() throws {
         try FakeChainBuilder.buildChain(darksideWallet: coordinator.service)
-               
+        
         try coordinator.applyStaged(blockheight: defaultLatestHeight)
         
         sleep(2)
@@ -222,9 +222,9 @@ class BalanceTests: XCTestCase {
         
         var error: Error?
         coordinator.synchronizer.sendToAddress(spendingKey: spendingKey,
-                                   zatoshi: Int64(sendAmount),
+                                               zatoshi: Int64(sendAmount),
-                                   toAddress: testRecipientAddress,
+                                               toAddress: testRecipientAddress,
-                                   memo: "test send \(self.description) \(Date().description)",
+                                               memo: "test send \(self.description) \(Date().description)",
         from: 0) { result in
             switch result {
             case .failure(let e):
@@ -255,7 +255,7 @@ class BalanceTests: XCTestCase {
         XCTAssertEqual(Int64(transaction.value), self.sendAmount)
         
         XCTAssertEqual(self.coordinator.synchronizer.initializer.getBalance(), presendBalance - Int64(self.sendAmount))
-
+        
         XCTAssertNil(transaction.errorCode)
         
         let latestHeight = try coordinator.latestHeight()
@@ -356,7 +356,7 @@ class BalanceTests: XCTestCase {
         
         wait(for: [syncedExpectation], timeout: 6)
         
-     
+        
         
         let previousVerifiedBalance = coordinator.synchronizer.initializer.getVerifiedBalance()
         let previousTotalBalance = coordinator.synchronizer.initializer.getBalance()
@@ -460,17 +460,17 @@ class BalanceTests: XCTestCase {
              */
             
             self.verifiedBalanceValidation(
-                    previousBalance: previousVerifiedBalance,
+                previousBalance: previousVerifiedBalance,
-                    spentNoteValue: Int64(sentNote.value),
+                spentNoteValue: Int64(sentNote.value),
-                    changeValue: Int64(receivedNote.value),
+                changeValue: Int64(receivedNote.value),
-                    sentAmount: Int64(self.sendAmount),
+                sentAmount: Int64(self.sendAmount),
-                    currentVerifiedBalance: synchronizer.initializer.getVerifiedBalance())
+                currentVerifiedBalance: synchronizer.initializer.getVerifiedBalance())
             
             
             
             self.totalBalanceValidation(totalBalance: synchronizer.initializer.getBalance(),
-                                            previousTotalbalance: previousTotalBalance,
+                                        previousTotalbalance: previousTotalBalance,
-                                            sentAmount: Int64(self.sendAmount))
+                                        sentAmount: Int64(self.sendAmount))
             
             syncToMinedheightExpectation.fulfill()
         }, error: self.handleError)
@@ -544,41 +544,44 @@ class BalanceTests: XCTestCase {
         let expirationSyncExpectation = XCTestExpectation(description: "expiration sync expectation")
         let expiryHeight = pendingTransaction.expiryHeight
         let blockCount = abs(self.defaultLatestHeight - expiryHeight)
-        try coordinator.stageBlockCreate(height: self.defaultLatestHeight  + 1, count: blockCount)
+        try coordinator.stageBlockCreate(height: self.defaultLatestHeight + 1, count: blockCount)
-        try coordinator.applyStaged(blockheight: expiryHeight)
+        try coordinator.applyStaged(blockheight: expiryHeight + 1)
         
         sleep(2)
         try coordinator.sync(completion: { (synchronizer) in
-            
-            /*
-             Verified Balance is equal to verified balance previously shown before sending the expired transaction
-             */
-            XCTAssertEqual(synchronizer.initializer.getVerifiedBalance(), previousVerifiedBalance)
-            
-            /*
-             Total Balance is equal to total balance previously shown before sending the expired transaction
-             */
-            XCTAssertEqual(synchronizer.initializer.getBalance(), previousTotalBalance)
-            
-            let pendingRepo = PendingTransactionSQLDAO(dbProvider: SimpleConnectionProvider(path: synchronizer.initializer.pendingDbURL.absoluteString))
-            
-            guard let expiredPending = try? pendingRepo.find(by: pendingTransaction.id!),
-                let id = expiredPending.id,
-                let sentExpired = try? synchronizer.allSentTransactions().first(where: { $0.id ==  id}) else {
-                    XCTFail("expired transaction not found")
-                    return
-            }
-            /*
-             There’s a pending transaction that has expired
-             */
-            XCTAssertEqual(expiredPending.minedHeight, -1)
-            XCTAssertEqual(sentExpired.expiryHeight,expiryHeight)
-            XCTAssertEqual(Int64(sentExpired.value), self.sendAmount)
-            
             expirationSyncExpectation.fulfill()
         }, error: self.handleError)
         
-        wait(for: [expirationSyncExpectation], timeout: 10)
+        wait(for: [expirationSyncExpectation], timeout: 5)
+        
+        /*
+         Verified Balance is equal to verified balance previously shown before sending the expired transaction
+         */
+        XCTAssertEqual(coordinator.synchronizer.initializer.getVerifiedBalance(), previousVerifiedBalance)
+        
+        /*
+         Total Balance is equal to total balance previously shown before sending the expired transaction
+         */
+        XCTAssertEqual(coordinator.synchronizer.initializer.getBalance(), previousTotalBalance)
+        
+        let pendingRepo = PendingTransactionSQLDAO(dbProvider: SimpleConnectionProvider(path: coordinator.synchronizer.initializer.pendingDbURL.absoluteString))
+        
+        guard let expiredPending = try? pendingRepo.find(by: pendingTransaction.id!),
+            let id = expiredPending.id else {
+                XCTFail("pending transaction not found")
+                return
+        }
+        
+        /*
+         there no sent transaction displayed
+         */
+        
+        XCTAssertNil( try coordinator.synchronizer.allSentTransactions().first(where: { $0.id ==  id}))
+        /*
+         There’s a pending transaction that has expired
+         */
+        XCTAssertEqual(expiredPending.minedHeight, -1)
+
     }
     
     func handleError(_ error: Error?) {