Merge pull request #7041 from AlphaWallet/walletconnect-eth_signtransaction-didnt-return-a-response-and-erroneously-sent-the-transaction-after-signing

[WalletConnect] eth_signTransaction didn't return a response and (erroneously) sent the transaction after signing

AlphaWallet/ActiveWalletCoordinator.swift

@@ -1196,7 +1196,6 @@ extension ActiveWalletCoordinator: DappBrowserCoordinatorDelegate {
                                 requester: RequesterViewModel?,
                                 transaction: UnconfirmedTransaction,
                                 configuration: TransactionType.Configuration) -> AnyPublisher<Data, PromiseError> {
-
         infoLog("[\(source)] signTransaction: \(transaction) type: \(configuration.confirmType)")
 
         return firstly {
@@ -1222,7 +1221,7 @@ extension ActiveWalletCoordinator: DappBrowserCoordinatorDelegate {
         }.then { shouldSend -> Promise<ConfirmResult> in
             guard shouldSend else { return .init(error: JsonRpcError.requestRejected) }
             let prompt = R.string.localizable.keystoreAccessKeySign()
-            let sender = SendTransaction(session: session, keystore: self.keystore, confirmType: .signThenSend, config: session.config, analytics: self.analytics, prompt: prompt)
+            let sender = SignMaySendTransaction(session: session, keystore: self.keystore, confirmType: .signThenSend, config: session.config, analytics: self.analytics, prompt: prompt)
             return Promise {
                 try await sender.send(rawTransaction: transaction)
             }

AlphaWallet/Transfer/Coordinators/TransactionConfirmationCoordinator.swift

@@ -148,16 +148,31 @@ extension TransactionConfirmationCoordinator: TransactionConfirmationViewControl
         rootViewController.set(state: .pending)
 
         Task { @MainActor in
-            do {
-                let result = try await sendTransaction()
-                handleSendTransactionSuccessfully(result: result)
-                logCompleteActionSheetForTransactionConfirmationSuccessfully()
-            } catch {
-                logActionSheetForTransactionConfirmationFailed()
-                //TODO remove delay which is currently needed because the starting animation may not have completed and internal state (whether animation is running) is in correct
-                DispatchQueue.main.asyncAfter(deadline: .now() + 0.4) {
-                    self.rootViewController.set(state: .done(withError: true)) {
-                        self.handleSendTransactionError(error)
+            switch configuration.confirmType {
+            case .sign:
+                do {
+                    let result = try await signTransaction()
+                    handleTransactionSuccessfully(result: result)
+                } catch {
+                    //TODO remove delay which is currently needed because the starting animation may not have completed and internal state (whether animation is running) is in correct
+                    DispatchQueue.main.asyncAfter(deadline: .now() + 0.4) {
+                        self.rootViewController.set(state: .done(withError: true)) {
+                            self.handleTransactionError(error)
+                        }
+                    }
+                }
+            case .signThenSend:
+                do {
+                    let result = try await sendTransaction()
+                    handleTransactionSuccessfully(result: result)
+                    logCompleteActionSheetForTransactionConfirmationSuccessfully()
+                } catch {
+                    logActionSheetForTransactionConfirmationFailed()
+                    //TODO remove delay which is currently needed because the starting animation may not have completed and internal state (whether animation is running) is in correct
+                    DispatchQueue.main.asyncAfter(deadline: .now() + 0.4) {
+                        self.rootViewController.set(state: .done(withError: true)) {
+                            self.handleTransactionError(error)
+                        }
                     }
                 }
             }
@@ -169,17 +184,25 @@ extension TransactionConfirmationCoordinator: TransactionConfirmationViewControl
 
     private func sendTransaction() async throws -> ConfirmResult {
         let prompt = R.string.localizable.keystoreAccessKeySign()
-        let sender = SendTransaction(session: configurator.session, keystore: keystore, confirmType: configuration.confirmType, config: configurator.session.config, analytics: analytics, prompt: prompt)
+        let sender = SignMaySendTransaction(session: configurator.session, keystore: keystore, confirmType: configuration.confirmType, config: configurator.session.config, analytics: analytics, prompt: prompt)
         let transaction = configurator.formUnsignedTransaction()
         infoLog("[TransactionConfirmation] form unsigned transaction: \(transaction)")
         if configurator.session.config.development.shouldNotSendTransactions {
             throw DevelopmentForcedError(message: "Did not send transaction because of development flag")
         } else {
-            return try await sender.send(transaction: transaction)
+            return try await sender.signMaybeSend(transaction: transaction)
         }
     }
 
-    private func handleSendTransactionSuccessfully(result: ConfirmResult) {
+    private func signTransaction() async throws -> ConfirmResult {
+        let prompt = R.string.localizable.keystoreAccessKeySign()
+        let signer = SignMaySendTransaction(session: configurator.session, keystore: keystore, confirmType: configuration.confirmType, config: configurator.session.config, analytics: analytics, prompt: prompt)
+        let transaction = configurator.formUnsignedTransaction()
+        infoLog("[TransactionConfirmation] form unsigned transaction: \(transaction)")
+        return try await signer.signMaybeSend(transaction: transaction)
+    }
+
+    private func handleTransactionSuccessfully(result: ConfirmResult) {
         switch result {
         case .sentTransaction(let tx):
             delegate?.didSendTransaction(tx, inCoordinator: self)
@@ -193,7 +216,7 @@ extension TransactionConfirmationCoordinator: TransactionConfirmationViewControl
         }
     }
 
-    private func handleSendTransactionError(_ error: Error) {
+    private func handleTransactionError(_ error: Error) {
         switch error {
         case let e as SendTransactionNotRetryableError where !server.isTestnet:
             let errorViewController = SendTransactionErrorViewController(analytics: analytics, viewModel: .init(server: server, error: e))

modules/AlphaWalletFoundation/AlphaWalletFoundation/Transfer/SignMaySendTransaction.swift

@@ -4,7 +4,7 @@ import Foundation
 import AlphaWalletLogger
 import BigInt
 
-public class SendTransaction {
+public class SignMaySendTransaction {
     private let keystore: Keystore
     private let session: WalletSession
     private let confirmType: ConfirmType
@@ -12,13 +12,7 @@ public class SendTransaction {
     private let analytics: AnalyticsLogger
     private let prompt: String
 
-    public init(session: WalletSession,
-                keystore: Keystore,
-                confirmType: ConfirmType,
-                config: Config,
-                analytics: AnalyticsLogger,
-                prompt: String) {
-
+    public init(session: WalletSession, keystore: Keystore, confirmType: ConfirmType, config: Config, analytics: AnalyticsLogger, prompt: String) {
         self.prompt = prompt
         self.session = session
         self.keystore = keystore
@@ -38,17 +32,26 @@ public class SendTransaction {
         }
     }
 
-    public func send(transaction: UnsignedTransaction) async throws -> ConfirmResult {
-        if transaction.nonce >= 0 {
+    public func signMaybeSend(transaction: UnsignedTransaction) async throws -> ConfirmResult {
+        switch confirmType {
+        case .sign:
+            return try await sign(transaction: transaction)
+        case .signThenSend:
             return try await signAndSend(transaction: transaction)
+        }
+    }
+
+    private func signAndSend(transaction: UnsignedTransaction) async throws -> ConfirmResult {
+        if transaction.nonce >= 0 {
+            return try await _signAndSend(transaction: transaction)
         } else {
             let nonce = try await session.blockchainProvider.nextNonce(wallet: session.account.address).first
             let transaction = transaction.updating(nonce: nonce)
-            return try await signAndSend(transaction: transaction)
+            return try await _signAndSend(transaction: transaction)
         }
     }
 
-    private func signAndSend(transaction: UnsignedTransaction) async throws -> ConfirmResult {
+    private func _signAndSend(transaction: UnsignedTransaction) async throws -> ConfirmResult {
         do {
             switch try await keystore.signTransaction(transaction, prompt: prompt) {
             case .failure(let error):
@@ -64,6 +67,22 @@ public class SendTransaction {
         }
     }
 
+    private func sign(transaction: UnsignedTransaction) async throws -> ConfirmResult {
+        do {
+            switch try await keystore.signTransaction(transaction, prompt: prompt) {
+            case .failure(let error):
+                throw error
+            case .success(let data):
+                infoLog("Sign transaction")
+                return .signedTransaction(data)
+            }
+        } catch {
+            //TODO should rename the function (since we are only signing, not sending), or have a different function
+            logSelectSendError(error)
+            throw error
+        }
+    }
+
     private func logSelectSendError(_ error: Error) {
         guard let error = error as? SendTransactionNotRetryableError else { return }
         switch error.type {