From 20750c6362176a795a47022f11bf10528ba792a0 Mon Sep 17 00:00:00 2001 From: Dmitry Bespalov Date: Wed, 2 Feb 2022 11:45:37 +0100 Subject: [PATCH] gh-1869 Check for failing tx before executing it --- .../TransactionEstimationController.swift | 11 ++++-- .../TransactionExecutionController.swift | 19 +++++++++-- .../Ethereum/Sources/Ethereum/EthRpc1.swift | 34 ++++++++++++++++--- 3 files changed, 53 insertions(+), 11 deletions(-) diff --git a/Multisig/UI/Transaction/Execution/TransactionEstimationController.swift b/Multisig/UI/Transaction/Execution/TransactionEstimationController.swift index 56ec38273..bb583314b 100644 --- a/Multisig/UI/Transaction/Execution/TransactionEstimationController.swift +++ b/Multisig/UI/Transaction/Execution/TransactionEstimationController.swift @@ -36,7 +36,7 @@ class TransactionEstimationController { self.rpcClient = JsonRpc2.Client(transport: JsonRpc2.ClientHTTPTransport(url: rpcUri), serializer: JsonRpc2.DefaultSerializer()) } - typealias EstimateCompletion = (Result<(gas: Result, transactionCount: Result, gasPrice: Result), Error>) -> Void + typealias EstimateCompletion = (Result<(gas: Result, transactionCount: Result, gasPrice: Result, ethCall: Result), Error>) -> Void func estimateTransactionWithRpc(tx: EthTransaction, completion: @escaping EstimateCompletion) -> URLSessionTask? { // check if we have hint from the chain configuration about the gas price. For now support only fixed. @@ -61,19 +61,23 @@ class TransactionEstimationController { let getPrice = EthRpc1.eth_gasPrice() + let ethCall = EthRpc1.eth_call(transaction: EthRpc1.Transaction(tx), block: .tag(.pending)) + let batch: JsonRpc2.BatchRequest let getEstimateRequest: JsonRpc2.Request let getTransactionCountRequest: JsonRpc2.Request let getPriceRequest: JsonRpc2.Request + let ethCallRequest: JsonRpc2.Request do { getEstimateRequest = try usingLegacyGasApi ? getEstimateLegacy.request(id: .int(1)) : getEstimateNew.request(id: .int(1)) getTransactionCountRequest = try getTransactionCount.request(id: .int(2)) getPriceRequest = try getPrice.request(id: .int(3)) + ethCallRequest = try ethCall.request(id: .int(4)) batch = try JsonRpc2.BatchRequest(requests: [ - getEstimateRequest, getTransactionCountRequest, getPriceRequest + getEstimateRequest, getTransactionCountRequest, getPriceRequest, ethCallRequest ]) } catch { dispatchOnMainThread(completion(.failure(error))) @@ -118,8 +122,9 @@ class TransactionEstimationController { : result(request: getEstimateRequest, method: getEstimateNew, responses: responses).map(\.storage) let txCountResult = result(request: getTransactionCountRequest, method: getTransactionCount, responses: responses).map(\.storage) let priceResult = fixedGasPrice.map { .success($0) } ?? result(request: getPriceRequest, method: getPrice, responses: responses).map(\.storage) + let callResult = result(request: ethCallRequest, method: ethCall, responses: responses).map(\.storage) - dispatchOnMainThread(completion(.success((gasResult, txCountResult, priceResult)))) + dispatchOnMainThread(completion(.success((gasResult, txCountResult, priceResult, callResult)))) } } return task diff --git a/Multisig/UI/Transaction/Execution/TransactionExecutionController.swift b/Multisig/UI/Transaction/Execution/TransactionExecutionController.swift index 41fb5a935..27f3c2172 100644 --- a/Multisig/UI/Transaction/Execution/TransactionExecutionController.swift +++ b/Multisig/UI/Transaction/Execution/TransactionExecutionController.swift @@ -192,19 +192,32 @@ class TransactionExecutionController { var gasPrice: Sol.UInt256? do { - gas = try partialResults.gas.get() + txCount = try partialResults.transactionCount.get() } catch { errors.append(error) } do { - txCount = try partialResults.transactionCount.get() + gasPrice = try partialResults.gasPrice.get() } catch { errors.append(error) } do { - gasPrice = try partialResults.gasPrice.get() + // WARN: this only works for the 'execTransaction' call + // since it depends on the output from the smart contract function. + // + // If the call will be reverted, then the eth_call will fail and also gas estimation will fail. + // so we might get duplicated errors. + // + // to avoid that, we handle both cases in one do-catch block. + gas = try partialResults.gas.get() + + let execTransactionSuccess = try partialResults.ethCall.get() + let success = try Sol.Bool(execTransactionSuccess).storage + guard success else { + throw TransactionExecutionError(code: -7, message: "Internal Gnosis Safe transaction fails. Please double check whether this transaction is valid with the current state of the Safe.") + } } catch { errors.append(error) } diff --git a/Packages/Ethereum/Sources/Ethereum/EthRpc1.swift b/Packages/Ethereum/Sources/Ethereum/EthRpc1.swift index 5b7102456..ea3405d13 100644 --- a/Packages/Ethereum/Sources/Ethereum/EthRpc1.swift +++ b/Packages/Ethereum/Sources/Ethereum/EthRpc1.swift @@ -475,15 +475,18 @@ extension EthRpc1 { } /// Executes a new message call immediately without creating a transaction on the block chain. - public struct eth_call: JsonRpc2Method, EthRpc1TransactionParams { + public struct eth_call: JsonRpc2Method { /// Transaction. NOTE: `from` field MUST be present. public var transaction: Transaction + /// Block number or tag + public var block: EthRpc1.BlockSpecifier /// Return data public typealias Return = EthRpc1.Data - public init(transaction: EthRpc1.Transaction) { + public init(transaction: EthRpc1.Transaction, block: EthRpc1.BlockSpecifier) { self.transaction = transaction + self.block = block } } @@ -608,6 +611,21 @@ extension EthRpc1TransactionParams { } } +extension EthRpc1.eth_call: Codable { + public init(from decoder: Decoder) throws { + var container = try decoder.unkeyedContainer() + let transaction = try container.decode(EthRpc1.Transaction.self) + let block = try container.decode(EthRpc1.BlockSpecifier.self) + self.init(transaction: transaction, block: block) + } + + public func encode(to encoder: Encoder) throws { + var container = encoder.unkeyedContainer() + try container.encode(transaction) + try container.encode(block) + } +} + public protocol EthEstimateGasAbi {} extension EthEstimateGasAbi where Self: JsonRpc2Method, Self: EthRpc1TransactionParams { @@ -1334,14 +1352,20 @@ extension Eth.TransactionEip1559: EthRawTransaction { } extension EthRpc1.eth_estimateGas { + public init(_ tx: EthTransaction) { + self.init(transaction: EthRpc1.Transaction(tx)) + } +} + +extension EthRpc1.Transaction { public init(_ tx: EthTransaction) { switch tx { case let eip1559 as Eth.TransactionEip1559: - self.init(transaction: .eip1559(EthRpc1.Transaction1559(eip1559))) + self = .eip1559(EthRpc1.Transaction1559(eip1559)) case let eip2930 as Eth.TransactionEip2930: - self.init(transaction: .eip2930(EthRpc1.Transaction2930(eip2930))) + self = .eip2930(EthRpc1.Transaction2930(eip2930)) case let legacy as Eth.TransactionLegacy: - self.init(transaction: .legacy(EthRpc1.TransactionLegacy(legacy))) + self = .legacy(EthRpc1.TransactionLegacy(legacy)) default: fatalError("Not implemented") }