From e57f7552d9f3fd064beddb69eee63ac8fe33d4a4 Mon Sep 17 00:00:00 2001 From: Stefan Date: Wed, 17 Jul 2024 19:36:32 +0300 Subject: [PATCH] feat(dapps) make wallet connect request data human readable Notify user if he doesn't hold enough funds to make the transaction Also check fees funds including the amount to be sent Closes: #15192 --- .../service/wallet_connect/service.nim | 9 +- .../qmlTests/tests/tst_DAppsWorkflow.qml | 15 +- .../StatusQ/Core/Utils/AmountsArithmetic.qml | 10 ++ .../Wallet/panels/DAppsWorkflow.qml | 30 +--- .../services/dapps/DAppsRequestHandler.qml | 140 +++++++++++++++--- .../dapps/types/SessionRequestResolved.qml | 2 + ui/imports/shared/stores/DAppsStore.qml | 33 +++-- 7 files changed, 178 insertions(+), 61 deletions(-) diff --git a/src/app_service/service/wallet_connect/service.nim b/src/app_service/service/wallet_connect/service.nim index 879f92311e..9b04c10529 100644 --- a/src/app_service/service/wallet_connect/service.nim +++ b/src/app_service/service/wallet_connect/service.nim @@ -222,8 +222,13 @@ QtObject: else: maxFeePerGas = chainFees.maxFeePerGasL else: - let maxFeePerGasInt = parseHexInt(maxFeePerGasHex) - maxFeePerGas = maxFeePerGasInt.float + try: + let maxFeePerGasInt = parseHexInt(maxFeePerGasHex) + maxFeePerGas = maxFeePerGasInt.float + except ValueError: + error "failed to parse maxFeePerGasHex", maxFeePerGasHex + return EstimatedTime.Unknown + return self.transactions.getEstimatedTime(chainId, $(maxFeePerGas)) proc getSuggestedFees*(self: Service, chainId: int): SuggestedFeesDto = diff --git a/storybook/qmlTests/tests/tst_DAppsWorkflow.qml b/storybook/qmlTests/tests/tst_DAppsWorkflow.qml index f06c3b9842..53d5282dc7 100644 --- a/storybook/qmlTests/tests/tst_DAppsWorkflow.qml +++ b/storybook/qmlTests/tests/tst_DAppsWorkflow.qml @@ -134,9 +134,17 @@ Item { l1GasFee: 0.0, eip1559Enabled: true }) + function getSuggestedFees() { return mockedSuggestedFees } + + function hexToDec(hex) { + if (hex.length > "0xfffffffffffff".length) { + console.warn(`Beware of possible loss of precision converting ${hex}`) + } + return parseInt(hex, 16).toString() + } } } @@ -804,9 +812,9 @@ Item { } } - function mockSessionRequestEvent(tc, sdk, accountsModel, networksMdodel) { + function mockSessionRequestEvent(tc, sdk, accountsModel, networksModel) { let account = accountsModel.get(1) - let network = networksMdodel.get(1) + let network = networksModel.get(1) let method = "personal_sign" let message = "hello world" let params = [`"${Helpers.strToHex(message)}"`, `"${account.address}"`] @@ -819,7 +827,8 @@ Item { method: Constants.personal_sign, account, network, - data: message + data: message, + preparedData: message }) // Expect to have calls to getActiveSessions from service initialization let prevRequests = sdk.getActiveSessionsCallbacks.length diff --git a/ui/StatusQ/src/StatusQ/Core/Utils/AmountsArithmetic.qml b/ui/StatusQ/src/StatusQ/Core/Utils/AmountsArithmetic.qml index 8700e1e9aa..e0951c6760 100644 --- a/ui/StatusQ/src/StatusQ/Core/Utils/AmountsArithmetic.qml +++ b/ui/StatusQ/src/StatusQ/Core/Utils/AmountsArithmetic.qml @@ -140,6 +140,16 @@ QtObject { return amount1.plus(amount2) } + /*! + \qmlmethod AmountsArithmetic::sub(amount1, amount2) + \brief Returns a Big number whose value is the subtraction of amount2 from amount1. + */ + function sub(amount1, amount2) { + console.assert(amount1 instanceof Big.Big) + console.assert(amount2 instanceof Big.Big || Number.isInteger(amount2)) + return amount1.minus(amount2) + } + /*! \qmlmethod AmountsArithmetic::cmp(amount1, amount2) \brief Compares two amounts. diff --git a/ui/app/AppLayouts/Wallet/panels/DAppsWorkflow.qml b/ui/app/AppLayouts/Wallet/panels/DAppsWorkflow.qml index 0ec1e2c4ec..53846bd2db 100644 --- a/ui/app/AppLayouts/Wallet/panels/DAppsWorkflow.qml +++ b/ui/app/AppLayouts/Wallet/panels/DAppsWorkflow.qml @@ -183,33 +183,9 @@ DappsComboBox { enoughFundsForTransaction: request.enoughFunds enoughFundsForFees: request.enoughFunds - signingTransaction: !!request.method && (request.method === SessionRequest.methods.signTransaction.name || request.method === SessionRequest.methods.sendTransaction.name) - requestPayload: { - switch(request.method) { - case SessionRequest.methods.personalSign.name: - return SessionRequest.methods.personalSign.getMessageFromData(request.data) - case SessionRequest.methods.sign.name: { - return SessionRequest.methods.sign.getMessageFromData(request.data) - } - case SessionRequest.methods.signTypedData_v4.name: { - const stringPayload = SessionRequest.methods.signTypedData_v4.getMessageFromData(request.data) - return JSON.stringify(JSON.parse(stringPayload), null, 2) - } - case SessionRequest.methods.signTypedData.name: { - const stringPayload = SessionRequest.methods.signTypedData.getMessageFromData(root.payloadData) - return JSON.stringify(JSON.parse(stringPayload), null, 2) - } - case SessionRequest.methods.signTransaction.name: { - const jsonPayload = SessionRequest.methods.signTransaction.getTxObjFromData(request.data) - return JSON.stringify(jsonPayload, null, 2) - } - case SessionRequest.methods.sendTransaction.name: { - const jsonPayload = SessionRequest.methods.sendTransaction.getTxObjFromData(request.data) - return JSON.stringify(jsonPayload, null, 2) - } - } - } - + signingTransaction: !!request.method && (request.method === SessionRequest.methods.signTransaction.name + || request.method === SessionRequest.methods.sendTransaction.name) + requestPayload: request.preparedData onClosed: { Qt.callLater( () => { rejectRequest() diff --git a/ui/app/AppLayouts/Wallet/services/dapps/DAppsRequestHandler.qml b/ui/app/AppLayouts/Wallet/services/dapps/DAppsRequestHandler.qml index 9c4aeee56c..1c60045017 100644 --- a/ui/app/AppLayouts/Wallet/services/dapps/DAppsRequestHandler.qml +++ b/ui/app/AppLayouts/Wallet/services/dapps/DAppsRequestHandler.qml @@ -132,6 +132,9 @@ SQUtils.QObject { console.error("Error in event data lookup", JSON.stringify(event)) return null } + + const interpreted = d.prepareData(method, data) + let enoughFunds = !d.isTransactionMethod(method) let obj = sessionRequestComponent.createObject(null, { event, @@ -141,6 +144,7 @@ SQUtils.QObject { account, network, data, + preparedData: interpreted.preparedData, maxFeesText: "?", maxFeesEthText: "?", enoughFunds: enoughFunds, @@ -180,7 +184,7 @@ SQUtils.QObject { } let st = getEstimatedFeesStatus(data, method, obj.network.chainId, mainChainId) - let fundsStatus = checkFundsStatus(st.feesInfo.maxFees, st.feesInfo.l1GasFee, account.address, obj.network.chainId, mainNet.chainId) + let fundsStatus = checkFundsStatus(st.feesInfo.maxFees, st.feesInfo.l1GasFee, account.address, obj.network.chainId, mainNet.chainId, interpreted.value) root.maxFeesUpdated(st.fiatMaxFees.toNumber(), st.maxFeesEth, fundsStatus.haveEnoughFunds, fundsStatus.haveEnoughForFees, st.symbol, st.feesInfo) @@ -436,7 +440,7 @@ SQUtils.QObject { let feesInfo = getEstimatedMaxFees(data, method, chainId, mainNetChainId) let totalMaxFees = Math.sum(feesInfo.maxFees, feesInfo.l1GasFee) - let maxFeesEth = Math.div(totalMaxFees, Math.fromString("1000000000")) + let maxFeesEth = Math.div(totalMaxFees, Math.fromNumber(1, 9)) let maxFeesEthStr = maxFeesEth.toString() let fiatMaxFeesStr = root.currenciesStore.getFiatValue(maxFeesEthStr, Constants.ethToken) @@ -446,27 +450,24 @@ SQUtils.QObject { return {fiatMaxFees, maxFeesEth, symbol, feesInfo} } - function checkBalanceForChain(balances, address, chainId, fees) { - let Math = SQUtils.AmountsArithmetic + function getBalanceInEth(balances, address, chainId) { + const Math = SQUtils.AmountsArithmetic let accEth = SQUtils.ModelUtils.getFirstModelEntryIf(balances, (balance) => { return balance.account.toLowerCase() === address.toLowerCase() && balance.chainId === chainId }) if (!accEth) { console.error("Error balance lookup for account ", address, " on chain ", chainId) - return {haveEnoughForFees, haveEnoughFunds} + return null } let accountFundsWei = Math.fromString(accEth.balance) - let accountFundsEth = Math.div(accountFundsWei, Math.fromString("1000000000000000000")) - - let feesEth = Math.div(fees, Math.fromString("1000000000")) - return Math.cmp(accountFundsEth, feesEth) >= 0 + return Math.div(accountFundsWei, Math.fromNumber(1, 18)) } - function checkFundsStatus(maxFees, l1GasFee, address, chainId, mainNetChainId) { + // Returns {haveEnoughForFees, haveEnoughFunds} and true in case of error not to block request + function checkFundsStatus(maxFees, l1GasFee, address, chainId, mainNetChainId, valueEth) { let Math = SQUtils.AmountsArithmetic - let haveEnoughForFees = false - // TODO #15192: extract funds from transaction and check against it + let haveEnoughForFees = true let haveEnoughFunds = true let token = SQUtils.ModelUtils.getByKey(root.assetsStore.groupedAccountAssetsModel, "tokensKey", Constants.ethToken) @@ -475,13 +476,35 @@ SQUtils.QObject { return {haveEnoughForFees, haveEnoughFunds} } - if (chainId == mainNetChainId) { - const finalFees = Math.sum(maxFees, l1GasFee) - haveEnoughForFees = checkBalanceForChain(token.balances, address, chainId, finalFees) + let chainBalance = getBalanceInEth(token.balances, address, chainId) + if (!chainBalance) { + console.error("Error fetching chain balance") + return {haveEnoughForFees, haveEnoughFunds} + } + haveEnoughFunds = Math.cmp(chainBalance, valueEth) >= 0 + if (haveEnoughFunds) { + chainBalance = Math.sub(chainBalance, valueEth) + + if (chainId == mainNetChainId) { + const finalFees = Math.sum(maxFees, l1GasFee) + let feesEth = Math.div(finalFees, Math.fromNumber(1, 9)) + haveEnoughForFees = Math.cmp(chainBalance, feesEth) >= 0 + } else { + const feesChain = Math.div(maxFees, Math.fromNumber(1, 9)) + const haveEnoughOnChain = Math.cmp(chainBalance, feesChain) >= 0 + + const mainBalance = getBalanceInEth(token.balances, address, mainNetChainId) + if (!mainBalance) { + console.error("Error fetching mainnet balance") + return {haveEnoughForFees, haveEnoughFunds} + } + const feesMain = Math.div(l1GasFee, Math.fromNumber(1, 9)) + const haveEnoughOnMain = Math.cmp(mainBalance, feesMain) >= 0 + + haveEnoughForFees = haveEnoughOnChain && haveEnoughOnMain + } } else { - const haveEnoughOnChain = checkBalanceForChain(token.balances, address, chainId, maxFees) - const haveEnoughOnMain = checkBalanceForChain(token.balances, address, mainNetChainId, l1GasFee) - haveEnoughForFees = haveEnoughOnChain && haveEnoughOnMain + haveEnoughForFees = false } return {haveEnoughForFees, haveEnoughFunds} @@ -503,6 +526,87 @@ SQUtils.QObject { } return tx } + + // returns { + // preparedData, + // value // null or ETH Big number + // } + function prepareData(method, data) { + let payload = null + switch(method) { + case SessionRequest.methods.personalSign.name: { + payload = SessionRequest.methods.personalSign.getMessageFromData(data) + break + } + case SessionRequest.methods.sign.name: { + payload = SessionRequest.methods.sign.getMessageFromData(data) + break + } + case SessionRequest.methods.signTypedData_v4.name: { + const stringPayload = SessionRequest.methods.signTypedData_v4.getMessageFromData(data) + payload = JSON.stringify(JSON.parse(stringPayload), null, 2) + break + } + case SessionRequest.methods.signTypedData.name: { + const stringPayload = SessionRequest.methods.signTypedData.getMessageFromData(data) + payload = JSON.stringify(JSON.parse(stringPayload), null, 2) + break + } + default: + // For transaction we process the data in a different way + break; + } + + let value = SQUtils.AmountsArithmetic.fromNumber(0) + if (d.isTransactionMethod(method)) { + let txObj = d.getTxObject(method, data) + let tx = Object.assign({}, txObj) + if (tx.value) { + value = hexToEth(tx.value) + tx.value = value.toString() + } + if (tx.maxFeePerGas) { + tx.maxFeePerGas = hexToGwei(tx.maxFeePerGas).toString() + } + if (tx.maxPriorityFeePerGas) { + tx.maxPriorityFeePerGas = hexToGwei(tx.maxPriorityFeePerGas).toString() + } + if (tx.gasPrice) { + tx.gasPrice = hexToGwei(tx.gasPrice) + } + if (tx.gasLimit) { + tx.gasLimit = parseInt(root.store.hexToDec(tx.gasLimit)) + } + if (tx.nonce) { + tx.nonce = parseInt(root.store.hexToDec(tx.nonce)) + } + + payload = JSON.stringify(tx, null, 2) + } + return { + preparedData: payload, + value: value + } + } + + function hexToEth(value) { + return hexToEthDenomination(value, "eth") + } + function hexToGwei(value) { + return hexToEthDenomination(value, "gwei") + } + function hexToEthDenomination(value, ethUnit) { + let unitMapping = { + "gwei": 9, + "eth": 18 + } + let Math = SQUtils.AmountsArithmetic + let decValue = root.store.hexToDec(value) + if (!!decValue) { + return Math.div(Math.fromNumber(decValue), Math.fromNumber(1, unitMapping[ethUnit])) + } + return Math.fromNumber(0) + } } /// The queue is used to ensure that the events are processed in the order they are received but they could be diff --git a/ui/app/AppLayouts/Wallet/services/dapps/types/SessionRequestResolved.qml b/ui/app/AppLayouts/Wallet/services/dapps/types/SessionRequestResolved.qml index d6fb10a04c..31764c8494 100644 --- a/ui/app/AppLayouts/Wallet/services/dapps/types/SessionRequestResolved.qml +++ b/ui/app/AppLayouts/Wallet/services/dapps/types/SessionRequestResolved.qml @@ -23,6 +23,8 @@ QObject { required property var network required property var data + // Data prepared for display in a human readable format + required property var preparedData readonly property alias dappName: d.dappName readonly property alias dappUrl: d.dappUrl diff --git a/ui/imports/shared/stores/DAppsStore.qml b/ui/imports/shared/stores/DAppsStore.qml index 8507641544..5a6825af52 100644 --- a/ui/imports/shared/stores/DAppsStore.qml +++ b/ui/imports/shared/stores/DAppsStore.qml @@ -53,17 +53,28 @@ QObject { // Strip leading zeros from numbers as expected by status-go function prepareTxForStatusGo(txObj) { - let tx = {} - if (txObj.data) { tx.data = txObj.data } - if (txObj.from) { tx.from = txObj.from } - if (txObj.gasLimit) { tx.gasLimit = stripLeadingZeros(txObj.gasLimit) } - if (txObj.gas) { tx.gas = stripLeadingZeros(txObj.gas) } - if (txObj.gasPrice) { tx.gasPrice = stripLeadingZeros(txObj.gasPrice) } - if (txObj.nonce) { tx.nonce = stripLeadingZeros(txObj.nonce) } - if (txObj.maxFeePerGas) { tx.maxFeePerGas = stripLeadingZeros(txObj.maxFeePerGas) } - if (txObj.maxPriorityFeePerGas) { tx.maxPriorityFeePerGas = stripLeadingZeros(txObj.maxPriorityFeePerGas) } - if (txObj.to) { tx.to = txObj.to } - if (txObj.value) { tx.value = stripLeadingZeros(txObj.value) } + let tx = Object.assign({}, txObj) + if (txObj.gasLimit) { + tx.gasLimit = stripLeadingZeros(txObj.gasLimit) + } + if (txObj.gas) { + tx.gas = stripLeadingZeros(txObj.gas) + } + if (txObj.gasPrice) { + tx.gasPrice = stripLeadingZeros(txObj.gasPrice) + } + if (txObj.nonce) { + tx.nonce = stripLeadingZeros(txObj.nonce) + } + if (txObj.maxFeePerGas) { + tx.maxFeePerGas = stripLeadingZeros(txObj.maxFeePerGas) + } + if (txObj.maxPriorityFeePerGas) { + tx.maxPriorityFeePerGas = stripLeadingZeros(txObj.maxPriorityFeePerGas) + } + if (txObj.value) { + tx.value = stripLeadingZeros(txObj.value) + } return tx }