From 86198a914c89a36ea18aedf57e1ac6c584d636f8 Mon Sep 17 00:00:00 2001
From: Uxio Fuentefria <uxio@safe.global>
Date: Wed, 29 Jun 2022 12:44:18 +0200
Subject: [PATCH] Log and raise a better exception for Ethereum RPC errors

---
 gnosis/eth/ethereum_client.py | 56 +++++++++++++++++++++++++++--------
 1 file changed, 43 insertions(+), 13 deletions(-)

diff --git a/gnosis/eth/ethereum_client.py b/gnosis/eth/ethereum_client.py
index 612628d15..b86ccee25 100644
--- a/gnosis/eth/ethereum_client.py
+++ b/gnosis/eth/ethereum_client.py
@@ -154,6 +154,28 @@ def with_exception_handling(*args, **kwargs):
     return with_exception_handling
 
 
+def parse_rpc_result_or_raise(
+    result: Dict[str, Any], eth_fn: str, arguments: Any
+) -> Any:
+    """
+    Responses from RPC should return a dictionary with `result` key and they are always 200 OK
+    even if errored. If not, raise an error
+
+    :param result:
+    :param eth_fn: RPC function called (for more info if exception is raised)
+    :param arguments: Arguments for the RPC function (for more info if exception is raised)
+    :return: `result["result"]` if key exists
+    :raises: ValueError
+    """
+
+    if "result" not in result:
+        message = f"Problem calling `{eth_fn}` on {arguments}, result={result}"
+        logger.error(message)
+        raise ValueError(message)
+
+    return result["result"]
+
+
 class EthereumTxSent(NamedTuple):
     tx_hash: bytes
     tx: TxParams
@@ -1095,11 +1117,7 @@ def trace_blocks(
         results = sorted(response.json(), key=lambda x: x["id"])
         traces = []
         for block_identifier, result in zip(block_identifiers, results):
-            if "result" not in result:
-                message = f"Problem calling batch `trace_block` on block={block_identifier}, result={result}"
-                logger.error(message)
-                raise ValueError(message)
-            raw_tx = result["result"]
+            raw_tx = parse_rpc_result_or_raise(result, "trace_block", block_identifier)
             if raw_tx:
                 try:
                     decoded_traces = self._decode_traces(raw_tx)
@@ -1152,8 +1170,8 @@ def trace_transactions(
             raise ValueError(message)
         results = sorted(response.json(), key=lambda x: x["id"])
         traces = []
-        for result in results:
-            raw_tx = result["result"]
+        for tx_hash, result in zip(tx_hashes, results):
+            raw_tx = parse_rpc_result_or_raise(result, "trace_transaction", tx_hash)
             if raw_tx:
                 try:
                     decoded_traces = self._decode_traces(raw_tx)
@@ -1663,8 +1681,10 @@ def get_transactions(self, tx_hashes: List[EthereumHash]) -> List[Optional[TxDat
             self.ethereum_node_url, json=payload, timeout=self.slow_timeout
         ).json()
         txs = []
-        for result in sorted(results, key=lambda x: x["id"]):
-            raw_tx = result["result"]
+        for tx_hash, result in zip(tx_hashes, sorted(results, key=lambda x: x["id"])):
+            raw_tx = parse_rpc_result_or_raise(
+                result, "eth_getTransactionByHash", tx_hash
+            )
             if raw_tx:
                 txs.append(transaction_result_formatter(raw_tx))
             else:
@@ -1712,8 +1732,10 @@ def get_transaction_receipts(
             self.ethereum_node_url, json=payload, timeout=self.slow_timeout
         ).json()
         receipts = []
-        for result in sorted(results, key=lambda x: x["id"]):
-            tx_receipt = result["result"]
+        for tx_hash, result in zip(tx_hashes, sorted(results, key=lambda x: x["id"])):
+            tx_receipt = parse_rpc_result_or_raise(
+                result, "eth_getTransactionReceipt", tx_hash
+            )
             # Parity returns tx_receipt even is tx is still pending, so we check `blockNumber` is not None
             if tx_receipt and tx_receipt["blockNumber"] is not None:
                 receipts.append(receipt_formatter(tx_receipt))
@@ -1764,8 +1786,16 @@ def get_blocks(
             self.ethereum_node_url, json=payload, timeout=self.slow_timeout
         ).json()
         blocks = []
-        for result in sorted(results, key=lambda x: x["id"]):
-            raw_block = result["result"]
+        for block_identifier, result in zip(
+            block_identifiers, sorted(results, key=lambda x: x["id"])
+        ):
+            raw_block = parse_rpc_result_or_raise(
+                result,
+                "eth_getBlockByNumber"
+                if isinstance(block_identifier, int)
+                else "eth_getBlockByHash",
+                block_identifier,
+            )
             if raw_block:
                 if "extraData" in raw_block:
                     del raw_block[