From 3ad373921847319b1738abaf73ec35c0d7c950ae Mon Sep 17 00:00:00 2001 From: Malik Al-Jabr Date: Mon, 18 Jan 2021 14:44:33 +0200 Subject: [PATCH] fix: gas estimate error fixes #935 A bug occurs when someone requests a large amount of funds from you since the gas estimation will fail and there isn't a way of handling errors in the source yet. This PR handles the error appropriatley for both `estimateGas` and `estimateTokenGas` where the response is only converted from hex to int if the RPC call was successful. Otherwise return the error message as the response and let the UI decide how to display it. Currently the error for gas estimation in transaction bubbles is displayed in a popup however, ive come to realize that 2 popups open instead of one. This is a new bug of which I can't pinpoint the root cause at the moment and have opted to file a separate issue for it. --- src/app/wallet/view.nim | 9 +++++++-- src/status/wallet.nim | 10 ++++------ .../ChatColumn/ChatComponents/SignTransactionModal.qml | 9 ++++----- .../TransactionComponents/AcceptTransaction.qml | 8 ++++++++ 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/app/wallet/view.nim b/src/app/wallet/view.nim index 0ce024ec11..a46877a5ba 100644 --- a/src/app/wallet/view.nim +++ b/src/app/wallet/view.nim @@ -273,13 +273,18 @@ QtObject: proc estimateGas*(self: WalletView, from_addr: string, to: string, assetAddress: string, value: string, data: string = ""): string {.slot.} = var - response: int + response: string 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, data, success) - result = $(%* { "result": %response, "success": %success }) + + if success == true: + let res = fromHex[int](response) + result = $(%* { "result": %res, "success": %success }) + else: + result = $(%* { "result": "-1", "success": %success, "error": { "message": %response } }) proc transactionWasSent*(self: WalletView, txResult: string) {.signal.} diff --git a/src/status/wallet.nim b/src/status/wallet.nim index bddd12e5c9..e171f81abf 100644 --- a/src/status/wallet.nim +++ b/src/status/wallet.nim @@ -72,15 +72,14 @@ proc buildTokenTransaction(self: WalletModel, source, to, assetAddress: Address, proc getKnownTokenContract*(self: WalletModel, address: Address): Erc20Contract = getErc20Contracts().concat(getCustomTokens()).getErc20ContractByAddress(address) -proc estimateGas*(self: WalletModel, source, to, value, data: string, success: var bool): int = +proc estimateGas*(self: WalletModel, source, to, value, data: string, success: var bool): string = var tx = transactions.buildTransaction( parseAddress(source), eth2Wei(parseFloat(value), 18), data = data ) tx.to = parseAddress(to).some - let response = eth.estimateGas(tx, success) - result = fromHex[int](response) + result = eth.estimateGas(tx, success) proc getTransactionReceipt*(self: WalletModel, transactionHash: string): JsonNode = result = status_wallet.getTransactionReceipt(transactionHash).parseJSON()["result"] @@ -110,7 +109,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, success: var bool): int = +proc estimateTokenGas*(self: WalletModel, source, to, assetAddress, value: string, success: var bool): string = var transfer: Transfer contract: Erc20Contract @@ -123,8 +122,7 @@ proc estimateTokenGas*(self: WalletModel, source, to, assetAddress, value: strin contract ) - let response = contract.methods["transfer"].estimateGas(tx, transfer, success) - result = fromHex[int](response) + result = contract.methods["transfer"].estimateGas(tx, transfer, success) proc sendTransaction*(self: WalletModel, source, to, value, gas, gasPrice, password: string, success: var bool, data = ""): string = var tx = transactions.buildTransaction( diff --git a/ui/app/AppLayouts/Chat/ChatColumn/ChatComponents/SignTransactionModal.qml b/ui/app/AppLayouts/Chat/ChatColumn/ChatComponents/SignTransactionModal.qml index eb5d1e2a10..0995be05ad 100644 --- a/ui/app/AppLayouts/Chat/ChatColumn/ChatComponents/SignTransactionModal.qml +++ b/ui/app/AppLayouts/Chat/ChatColumn/ChatComponents/SignTransactionModal.qml @@ -31,10 +31,6 @@ ModalPopup { root.close() } - function estimateGas(){ - gasSelector.estimateGas() - } - id: root //% "Send" @@ -139,7 +135,10 @@ ModalPopup { if (!gasEstimate.success) { //% "Error estimating gas: %1" - console.warn(qsTrId("error-estimating-gas---1").arg(gasEstimate.error.message)) + let message = qsTrId("error-estimating-gas---1").arg(gasEstimate.error.message) + console.warn(message) + gasEstimateErrorPopup.confirmationText = message + qsTr(". The transaction will probably fail.") + gasEstimateErrorPopup.open() return } selectedGasLimit = gasEstimate.result diff --git a/ui/app/AppLayouts/Chat/ChatColumn/MessageComponents/TransactionComponents/AcceptTransaction.qml b/ui/app/AppLayouts/Chat/ChatColumn/MessageComponents/TransactionComponents/AcceptTransaction.qml index 2eaec8eb48..76ad8f4f71 100644 --- a/ui/app/AppLayouts/Chat/ChatColumn/MessageComponents/TransactionComponents/AcceptTransaction.qml +++ b/ui/app/AppLayouts/Chat/ChatColumn/MessageComponents/TransactionComponents/AcceptTransaction.qml @@ -77,6 +77,14 @@ Item { } } + ConfirmationDialog { + id: gasEstimateErrorPopup + height: 220 + onConfirmButtonClicked: { + gasEstimateErrorPopup.close(); + } + } + Loader { id: signTransactionModal function open() {