From 7b0d3c496c34acd0f37278513f4ff2384e22d916 Mon Sep 17 00:00:00 2001 From: emizzle Date: Tue, 15 Sep 2020 18:53:56 +1000 Subject: [PATCH] fix: exception handling in mutli-threaded transactions Currently, exceptions thrown during transactions or gas estimation that were spawned in another thread are not being propagated, due to a limitation in nim (see https://nim-lang.org/docs/manual_experimental.html#parallel-amp-spawn). This means any exceptions from status-go were not propagated correctly and would cause the app to crash. This includes entering the wrong password when trying to send a transaction. The issue was addressed by passing a `success` variable by reference, which is set to false if an exception was thrown by status-go. --- src/app/chat/view.nim | 20 ++++----- src/app/profile/views/ens_manager.nim | 18 ++++---- src/app/wallet/view.nim | 43 +++++++++---------- src/status/ens.nim | 16 +++---- src/status/libstatus/eth/eth.nim | 22 +++++++--- src/status/libstatus/eth/methods.nim | 27 ++++++++---- src/status/stickers.nim | 17 +++----- src/status/wallet.nim | 38 +++++++--------- .../ChatComponents/SignTransactionModal.qml | 10 ++--- .../components/StickerPackPurchaseModal.qml | 6 +-- .../Profile/Sections/Ens/RegisterENSModal.qml | 6 +-- ui/app/AppLayouts/Wallet/SendModal.qml | 10 ++--- 12 files changed, 115 insertions(+), 118 deletions(-) diff --git a/src/app/chat/view.nim b/src/app/chat/view.nim index 8a0f228eae..d80a499b9a 100644 --- a/src/app/chat/view.nim +++ b/src/app/chat/view.nim @@ -107,26 +107,24 @@ QtObject: read = getStickerMarketAddress proc buyPackGasEstimate*(self: ChatsView, packId: int, address: string, price: string): int {.slot.} = - try: - result = self.status.stickers.estimateGas(packId, address, price) - except: + var success: bool + result = self.status.stickers.estimateGas(packId, address, price, success) + if not success: result = 325000 proc transactionWasSent*(self: ChatsView, txResult: string) {.signal.} proc transactionCompleted*(self: ChatsView, success: bool, txHash: string, revertReason: string = "") {.signal.} proc buyStickerPack*(self: ChatsView, packId: int, address: string, price: string, gas: string, gasPrice: string, password: string): string {.slot.} = - try: - let response = self.status.stickers.buyPack(packId, address, price, gas, gasPrice, password) + var success: bool + let response = self.status.stickers.buyPack(packId, address, price, gas, gasPrice, password, success) + # TODO: + # check if response["error"] is not null and handle the error + result = $(%* { "result": %response, "success": %success }) + if success: self.stickerPacks.updateStickerPackInList(packId, false, true) - result = $(%* { "result": %response }) self.transactionWasSent(response) - # TODO: - # check if response["error"] is not null and handle the error - except RpcException as e: - result = $(%* { "error": %* { "message": %e.msg }}) - proc obtainAvailableStickerPacks*(self: ChatsView) = spawnAndSend(self, "setAvailableStickerPacks") do: let availableStickerPacks = status_stickers.getAvailableStickerPacks() diff --git a/src/app/profile/views/ens_manager.nim b/src/app/profile/views/ens_manager.nim index 4f8d1fa546..929546c508 100644 --- a/src/app/profile/views/ens_manager.nim +++ b/src/app/profile/views/ens_manager.nim @@ -207,17 +207,18 @@ QtObject: read = getEnsRegisterAddress proc registerENSGasEstimate(self: EnsManager, ensUsername: string, address: string): int {.slot.} = + var success: bool let pubKey = status_settings.getSetting[string](Setting.PublicKey, "0x0") - try: - result = registerUsernameEstimateGas(ensUsername, address, pubKey) - except: + result = registerUsernameEstimateGas(ensUsername, address, pubKey, success) + if not success: result = 325000 proc registerENS*(self: EnsManager, username: string, address: string, gas: string, gasPrice: string, password: string): string {.slot.} = - try: - let pubKey = status_settings.getSetting[string](Setting.PublicKey, "0x0") - let response = registerUsername(username, pubKey, address, gas, gasPrice, password) - result = $(%* { "result": %response }) + var success: bool + let pubKey = status_settings.getSetting[string](Setting.PublicKey, "0x0") + let response = registerUsername(username, pubKey, address, gas, gasPrice, password, success) + result = $(%* { "result": %response, "success": %success }) + if success: self.transactionWasSent(response) # TODO: handle transaction failure @@ -225,9 +226,6 @@ QtObject: self.pendingUsernames.incl(ensUsername) self.add ensUsername - except RpcException as e: - result = $(%* { "error": %* { "message": %e.msg }}) - proc setPubKey(self: EnsManager, username: string, password: string) {.slot.} = let pubKey = status_settings.getSetting[string](Setting.PublicKey, "0x0") let address = status_wallet.getWalletAccounts()[0].address diff --git a/src/app/wallet/view.nim b/src/app/wallet/view.nim index f4ab8b906d..c6f15a3e6c 100644 --- a/src/app/wallet/view.nim +++ b/src/app/wallet/view.nim @@ -265,35 +265,32 @@ QtObject: notify = accountListChanged proc estimateGas*(self: WalletView, from_addr: string, to: string, assetAddress: string, value: string): string {.slot.} = - try: - var response: int - if assetAddress != ZERO_ADDRESS and not assetAddress.isEmptyOrWhitespace: - response = self.status.wallet.estimateTokenGas(from_addr, to, assetAddress, value) - else: - response = self.status.wallet.estimateGas(from_addr, to, value) - result = $(%* { "result": %response }) - except RpcException as e: - result = $(%* { "error": %* { "message": %e.msg }}) + var + response: int + success: bool + if assetAddress != ZERO_ADDRESS and not assetAddress.isEmptyOrWhitespace: + response = self.status.wallet.estimateTokenGas(from_addr, to, assetAddress, value, success) + else: + response = self.status.wallet.estimateGas(from_addr, to, value, success) + result = $(%* { "result": %response, "success": %success }) proc transactionWasSent*(self: WalletView, txResult: string) {.signal.} proc transactionSent(self: WalletView, txResult: string) {.slot.} = self.transactionWasSent(txResult) - - proc sendTransaction*(self: WalletView, from_addr: string, to: string, assetAddress: string, value: string, gas: string, gasPrice: string, password: string): string {.slot.} = - try: - let wallet = self.status.wallet - if assetAddress != ZERO_ADDRESS and not assetAddress.isEmptyOrWhitespace: - spawnAndSend(self, "transactionSent") do: - let response = wallet.sendTokenTransaction(from_addr, to, assetAddress, value, gas, gasPrice, password) - $(%* { "result": %response }) - else: - spawnAndSend(self, "transactionSent") do: - let response = wallet.sendTransaction(from_addr, to, value, gas, gasPrice, password) - $(%* { "result": %response }) - except RpcException as e: - result = $(%* { "error": %* { "message": %e.msg }}) + proc sendTransaction*(self: WalletView, from_addr: string, to: string, assetAddress: string, value: string, gas: string, gasPrice: string, password: string) {.slot.} = + let wallet = self.status.wallet + if assetAddress != ZERO_ADDRESS and not assetAddress.isEmptyOrWhitespace: + spawnAndSend(self, "transactionSent") do: + var success: bool + let response = wallet.sendTokenTransaction(from_addr, to, assetAddress, value, gas, gasPrice, password, success) + $(%* { "result": %response, "success": %success }) + else: + spawnAndSend(self, "transactionSent") do: + var success: bool + let response = wallet.sendTransaction(from_addr, to, value, gas, gasPrice, password, success) + $(%* { "result": %response, "success": %success }) proc getDefaultAccount*(self: WalletView): string {.slot.} = self.currentAccount.address diff --git a/src/status/ens.nim b/src/status/ens.nim index 96e11200e3..bd728fadaf 100644 --- a/src/status/ens.nim +++ b/src/status/ens.nim @@ -145,7 +145,7 @@ proc getPrice*(): Stuint[256] = proc extractCoordinates*(pubkey: string):tuple[x: string, y:string] = result = ("0x" & pubkey[4..67], "0x" & pubkey[68..131]) -proc registerUsernameEstimateGas*(username: string, address: string, pubKey: string): int = +proc registerUsernameEstimateGas*(username: string, address: string, pubKey: string, success: var bool): int = let label = fromHex(FixedBytes[32], label(username)) coordinates = extractCoordinates(pubkey) @@ -163,13 +163,11 @@ proc registerUsernameEstimateGas*(username: string, address: string, pubKey: str var tx = transactions.buildTokenTransaction(parseAddress(address), sntContract.address, "", "") - try: - let response = sntContract.methods["approveAndCall"].estimateGas(tx, approveAndCallObj) + let response = sntContract.methods["approveAndCall"].estimateGas(tx, approveAndCallObj, success) + if success: result = fromHex[int](response) - except RpcException as e: - raise -proc registerUsername*(username, pubKey, address, gas, gasPrice, password: string): string = +proc registerUsername*(username, pubKey, address, gas, gasPrice, password: string, success: var bool): string = let label = fromHex(FixedBytes[32], label(username)) coordinates = extractCoordinates(pubkey) @@ -186,11 +184,9 @@ proc registerUsername*(username, pubKey, address, gas, gasPrice, password: stri var tx = transactions.buildTokenTransaction(parseAddress(address), sntContract.address, gas, gasPrice) - try: - result = sntContract.methods["approveAndCall"].send(tx, approveAndCallObj, password) + result = sntContract.methods["approveAndCall"].send(tx, approveAndCallObj, password, success) + if success: trackPendingTransaction(result, address, $sntContract.address, PendingTransactionType.RegisterENS, username & domain) - except RpcException as e: - raise proc setPubKey*(username:string, address: EthAddress, pubKey: string, password: string): string = var hash = namehash(username) diff --git a/src/status/libstatus/eth/eth.nim b/src/status/libstatus/eth/eth.nim index e93d7882a3..14a0dcfb8e 100644 --- a/src/status/libstatus/eth/eth.nim +++ b/src/status/libstatus/eth/eth.nim @@ -1,10 +1,20 @@ import transactions, ../types -proc sendTransaction*(tx: var EthSend, password: string): string = - let response = transactions.sendTransaction(tx, password) - result = response.result +proc sendTransaction*(tx: var EthSend, password: string, success: var bool): string = + success = true + try: + let response = transactions.sendTransaction(tx, password) + result = response.result + except RpcException as e: + success = false + result = e.msg -proc estimateGas*(tx: var EthSend): string = - let response = transactions.estimateGas(tx) - result = response.result \ No newline at end of file +proc estimateGas*(tx: var EthSend, success: var bool): string = + success = true + try: + let response = transactions.estimateGas(tx) + result = response.result + except RpcException as e: + success = false + result = e.msg \ No newline at end of file diff --git a/src/status/libstatus/eth/methods.nim b/src/status/libstatus/eth/methods.nim index 913b87c434..a6cc14bc3a 100644 --- a/src/status/libstatus/eth/methods.nim +++ b/src/status/libstatus/eth/methods.nim @@ -38,18 +38,27 @@ proc encodeAbi*(self: Method, obj: object = RootObj()): string = result &= encoded.data result &= data -proc estimateGas*(self: Method, tx: var EthSend, methodDescriptor: object): string = +proc estimateGas*(self: Method, tx: var EthSend, methodDescriptor: object, success: var bool): string = + success = true tx.data = self.encodeAbi(methodDescriptor) - let response = transactions.estimateGas(tx) - result = response.result # gas estimate in hex + try: + let response = transactions.estimateGas(tx) + result = response.result # gas estimate in hex + except RpcException as e: + success = false + result = e.msg -proc send*(self: Method, tx: var EthSend, methodDescriptor: object, password: string): string = +proc send*(self: Method, tx: var EthSend, methodDescriptor: object, password: string, success: var bool): string = tx.data = self.encodeAbi(methodDescriptor) - result = eth.sendTransaction(tx, password) - # result = coder.decodeContractResponse[string](response.result) - # result = response.result + result = eth.sendTransaction(tx, password, success) -proc call*[T](self: Method, tx: var EthSend, methodDescriptor: object): T = +proc call*[T](self: Method, tx: var EthSend, methodDescriptor: object, success: var bool): T = + success = true tx.data = self.encodeAbi(methodDescriptor) - let response = transactions.call(tx) + let response: RpcResponse + try: + response = transactions.call(tx) + except RpcException as e: + success = false + result = e.msg result = coder.decodeContractResponse[T](response.result) \ No newline at end of file diff --git a/src/status/stickers.nim b/src/status/stickers.nim index 1491899c9f..26871a902d 100644 --- a/src/status/stickers.nim +++ b/src/status/stickers.nim @@ -52,7 +52,7 @@ proc buildTransaction(self: StickersModel, packId: Uint256, address: EthAddress, approveAndCall = ApproveAndCall[100](to: stickerMktContract.address, value: price, data: DynamicBytes[100].fromHex(buyTxAbiEncoded)) transactions.buildTokenTransaction(address, sntContract.address, gas, gasPrice) -proc estimateGas*(self: StickersModel, packId: int, address: string, price: string): int = +proc estimateGas*(self: StickersModel, packId: int, address: string, price: string, success: var bool): int = var approveAndCall: ApproveAndCall[100] sntContract = status_contracts.getContract("snt") @@ -63,13 +63,11 @@ proc estimateGas*(self: StickersModel, packId: int, address: string, price: stri approveAndCall, sntContract ) - try: - let response = sntContract.methods["approveAndCall"].estimateGas(tx, approveAndCall) + let response = sntContract.methods["approveAndCall"].estimateGas(tx, approveAndCall, success) + if success: result = fromHex[int](response) - except RpcException as e: - raise -proc buyPack*(self: StickersModel, packId: int, address, price, gas, gasPrice, password: string): string = +proc buyPack*(self: StickersModel, packId: int, address, price, gas, gasPrice, password: string, success: var bool): string = var sntContract: Contract approveAndCall: ApproveAndCall[100] @@ -82,11 +80,10 @@ proc buyPack*(self: StickersModel, packId: int, address, price, gas, gasPrice, p gas, gasPrice ) - try: - result = sntContract.methods["approveAndCall"].send(tx, approveAndCall, password) + + result = sntContract.methods["approveAndCall"].send(tx, approveAndCall, password, success) + if success: trackPendingTransaction(result, address, $sntContract.address, PendingTransactionType.BuyStickerPack, $packId) - except RpcException as e: - raise proc getStickerMarketAddress*(self: StickersModel): EthAddress = result = status_contracts.getContract("sticker-market").address diff --git a/src/status/wallet.nim b/src/status/wallet.nim index 3d6b53e852..0eab0fab25 100644 --- a/src/status/wallet.nim +++ b/src/status/wallet.nim @@ -10,7 +10,7 @@ import libstatus/wallet as status_wallet import libstatus/accounts/constants as constants import libstatus/eth/[eth, contracts] from libstatus/core import getBlockByNumber -from libstatus/types import PendingTransactionType, GeneratedAccount, DerivedAccount, Transaction, Setting, GasPricePrediction, EthSend, Quantity, `%`, StatusGoException, Network, RpcResponse, RpcException +from libstatus/types import PendingTransactionType, GeneratedAccount, DerivedAccount, Transaction, Setting, GasPricePrediction, EthSend, Quantity, `%`, StatusGoException, Network, RpcResponse, RpcException, `$` from libstatus/utils as libstatus_utils import eth2Wei, gwei2Wei, first, toUInt64 import wallet/[balance_manager, account, collectibles] import transactions @@ -67,17 +67,14 @@ proc buildTokenTransaction(self: WalletModel, source, to, assetAddress: EthAddre transfer = Transfer(to: to, value: eth2Wei(value, token["decimals"].getInt)) transactions.buildTokenTransaction(source, assetAddress, gas, gasPrice) -proc estimateGas*(self: WalletModel, source, to, value: string): int = +proc estimateGas*(self: WalletModel, source, to, value: string, success: var bool): int = var tx = transactions.buildTransaction( parseAddress(source), eth2Wei(parseFloat(value), 18) ) tx.to = parseAddress(to).some - try: - let response = eth.estimateGas(tx) - result = fromHex[int](response) - except RpcException as e: - raise + let response = eth.estimateGas(tx, success) + result = fromHex[int](response) proc getTransactionReceipt*(self: WalletModel, transactionHash: string): JsonNode = result = status_wallet.getTransactionReceipt(transactionHash).parseJSON()["result"] @@ -102,7 +99,7 @@ proc checkPendingTransactions*(self: WalletModel) = proc checkPendingTransactions*(self: WalletModel, address: string, blockNumber: int) = self.confirmTransactionStatus(status_wallet.getPendingOutboundTransactionsByAddress(address).parseJson["result"], blockNumber) -proc estimateTokenGas*(self: WalletModel, source, to, assetAddress, value: string): int = +proc estimateTokenGas*(self: WalletModel, source, to, assetAddress, value: string, success: var bool): int = var transfer: Transfer contract: Contract @@ -114,26 +111,22 @@ proc estimateTokenGas*(self: WalletModel, source, to, assetAddress, value: strin transfer, contract ) - try: - let response = contract.methods["transfer"].estimateGas(tx, transfer) - result = fromHex[int](response) - except RpcException as e: - raise -proc sendTransaction*(self: WalletModel, source, to, value, gas, gasPrice, password: string): string = + let response = contract.methods["transfer"].estimateGas(tx, transfer, success) + result = fromHex[int](response) + +proc sendTransaction*(self: WalletModel, source, to, value, gas, gasPrice, password: string, success: var bool): string = var tx = transactions.buildTransaction( parseAddress(source), eth2Wei(parseFloat(value), 18), gas, gasPrice ) tx.to = parseAddress(to).some - try: - result = eth.sendTransaction(tx, password) + result = eth.sendTransaction(tx, password, success) + if success: trackPendingTransaction(result, $source, $to, PendingTransactionType.WalletTransfer, "") - except RpcException as e: - raise -proc sendTokenTransaction*(self: WalletModel, source, to, assetAddress, value, gas, gasPrice, password: string): string = +proc sendTokenTransaction*(self: WalletModel, source, to, assetAddress, value, gas, gasPrice, password: string, success: var bool): string = var transfer: Transfer contract: Contract @@ -147,11 +140,10 @@ proc sendTokenTransaction*(self: WalletModel, source, to, assetAddress, value, g gas, gasPrice ) - try: - result = contract.methods["transfer"].send(tx, transfer, password) + + result = contract.methods["transfer"].send(tx, transfer, password, success) + if success: trackPendingTransaction(result, $source, $to, PendingTransactionType.WalletTransfer, "") - except RpcException as e: - raise proc getDefaultCurrency*(self: WalletModel): string = # TODO: this should come from a model? It is going to be used too in the diff --git a/ui/app/AppLayouts/Chat/ChatColumn/ChatComponents/SignTransactionModal.qml b/ui/app/AppLayouts/Chat/ChatColumn/ChatComponents/SignTransactionModal.qml index 4bd231288a..51119c424a 100644 --- a/ui/app/AppLayouts/Chat/ChatColumn/ChatComponents/SignTransactionModal.qml +++ b/ui/app/AppLayouts/Chat/ChatColumn/ChatComponents/SignTransactionModal.qml @@ -40,12 +40,12 @@ ModalPopup { transactionSigner.enteredPassword) let response = JSON.parse(responseStr) - if (response.error) { - if (response.error.message.includes("could not decrypt key with given password")){ + if (!response.success) { + if (response.result.includes("could not decrypt key with given password")){ transactionSigner.validationError = qsTr("Wrong password") return } - sendingError.text = response.error.message + sendingError.text = response.result return sendingError.open() } } @@ -90,8 +90,8 @@ ModalPopup { root.selectedAsset.address, root.selectedAmount)) - if (gasEstimate.error) { - console.warn(qsTr("Error estimating gas: %1").arg(gasEstimate.error.message)) + if (!gasEstimate.success) { + console.warn(qsTr("Error estimating gas: %1").arg(gasEstimate.result)) return } selectedGasLimit = gasEstimate.result diff --git a/ui/app/AppLayouts/Chat/components/StickerPackPurchaseModal.qml b/ui/app/AppLayouts/Chat/components/StickerPackPurchaseModal.qml index d8188ffea3..98863b44ad 100644 --- a/ui/app/AppLayouts/Chat/components/StickerPackPurchaseModal.qml +++ b/ui/app/AppLayouts/Chat/components/StickerPackPurchaseModal.qml @@ -33,12 +33,12 @@ ModalPopup { transactionSigner.enteredPassword) let response = JSON.parse(responseStr) - if (response.error) { - if (response.error.message.includes("could not decrypt key with given password")){ + if (!response.success) { + if (response.result.includes("could not decrypt key with given password")){ transactionSigner.validationError = qsTr("Wrong password") return } - sendingError.text = response.error.message + sendingError.text = response.result return sendingError.open() } } diff --git a/ui/app/AppLayouts/Profile/Sections/Ens/RegisterENSModal.qml b/ui/app/AppLayouts/Profile/Sections/Ens/RegisterENSModal.qml index baa0daf2c5..5eb7cc65cd 100644 --- a/ui/app/AppLayouts/Profile/Sections/Ens/RegisterENSModal.qml +++ b/ui/app/AppLayouts/Profile/Sections/Ens/RegisterENSModal.qml @@ -42,12 +42,12 @@ ModalPopup { transactionSigner.enteredPassword) let response = JSON.parse(responseStr) - if (response.error) { - if (response.error.message.includes("could not decrypt key with given password")){ + if (!response.success) { + if (response.result.includes("could not decrypt key with given password")){ transactionSigner.validationError = qsTr("Wrong password") return } - sendingError.text = response.error.message + sendingError.text = response.result return sendingError.open() } diff --git a/ui/app/AppLayouts/Wallet/SendModal.qml b/ui/app/AppLayouts/Wallet/SendModal.qml index 4259c0d09b..e6729c2bfd 100644 --- a/ui/app/AppLayouts/Wallet/SendModal.qml +++ b/ui/app/AppLayouts/Wallet/SendModal.qml @@ -128,8 +128,8 @@ ModalPopup { txtAmount.selectedAsset.address, txtAmount.selectedAmount)) - if (gasEstimate.error) { - console.warn(qsTr("Error estimating gas: %1").arg(gasEstimate.error.message)) + if (!gasEstimate.success) { + console.warn(qsTr("Error estimating gas: %1").arg(gasEstimate.result)) return } selectedGasLimit = gasEstimate.result @@ -256,12 +256,12 @@ ModalPopup { stack.currentGroup.isPending = false let response = JSON.parse(txResult) - if (response.error) { - if (response.error.message.includes("could not decrypt key with given password")){ + if (!response.success) { + if (response.result.includes("could not decrypt key with given password")){ transactionSigner.validationError = qsTr("Wrong password") return } - sendingError.text = response.error.message + sendingError.text = response.result return sendingError.open() }