Merge pull request #1882 from gnosis/gh-1869/exec-call-check

gh-1869 Check for failing tx before executing it

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<Sol.UInt64, Error>, transactionCount: Result<Sol.UInt64, Error>, gasPrice: Result<Sol.UInt256, Error>), Error>) -> Void
+    typealias EstimateCompletion = (Result<(gas: Result<Sol.UInt64, Error>, transactionCount: Result<Sol.UInt64, Error>, gasPrice: Result<Sol.UInt256, Error>, ethCall: Result<Data, Error>), 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

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)
                 }

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")
         }