fix: skip trace contract-logic errors during proxy-detection and more…

… error-handling in eth_call (#2207)

src/ape_ethereum/provider.py

@@ -462,7 +462,9 @@ def send_call(
         skip_trace = kwargs.pop("skip_trace", False)
         arguments = self._prepare_call(txn, **kwargs)
         if skip_trace:
-            return self._eth_call(arguments, raise_on_revert=txn.raise_on_revert)
+            return self._eth_call(
+                arguments, raise_on_revert=txn.raise_on_revert, skip_trace=skip_trace
+            )
 
         show_gas = kwargs.pop("show_gas_report", False)
         show_trace = kwargs.pop("show_trace", False)
@@ -476,7 +478,7 @@ def send_call(
 
         needs_trace = track_gas or track_coverage or show_gas or show_trace
         if not needs_trace:
-            return self._eth_call(arguments, raise_on_revert=raise_on_revert)
+            return self._eth_call(arguments, raise_on_revert=raise_on_revert, skip_trace=skip_trace)
 
         # The user is requesting information related to a call's trace,
         # such as gas usage data.
@@ -517,7 +519,9 @@ def send_call(
 
         return HexBytes(trace.return_value)
 
-    def _eth_call(self, arguments: list, raise_on_revert: bool = True) -> HexBytes:
+    def _eth_call(
+        self, arguments: list, raise_on_revert: bool = True, skip_trace: bool = False
+    ) -> HexBytes:
         # Force the usage of hex-type to support a wider-range of nodes.
         txn_dict = copy(arguments[0])
         if isinstance(txn_dict.get("type"), int):
@@ -530,14 +534,27 @@ def _eth_call(self, arguments: list, raise_on_revert: bool = True) -> HexBytes:
         try:
             result = self.make_request("eth_call", arguments)
         except Exception as err:
-            trace = CallTrace(tx=arguments[0], arguments=arguments[1:], use_tokens_for_symbols=True)
-            contract_address = arguments[0]["to"]
-            contract_type = self.chain_manager.contracts.get(contract_address)
-            method_id = arguments[0].get("data", "")[:10] or None
+            trace = None
             tb = None
-            if contract_type and method_id:
-                if contract_src := self.local_project._create_contract_source(contract_type):
-                    tb = SourceTraceback.create(contract_src, trace, method_id)
+            contract_address = arguments[0].get("to")
+            if not skip_trace:
+                if address := contract_address:
+                    try:
+                        contract_type = self.chain_manager.contracts.get(address)
+                    except RecursionError:
+                        # Occurs when already in the middle of fetching this contract.
+                        contract_type = None
+                else:
+                    contract_type = None
+
+                trace = CallTrace(
+                    tx=arguments[0], arguments=arguments[1:], use_tokens_for_symbols=True
+                )
+                method_id = arguments[0].get("data", "")[:10] or None
+                tb = None
+                if contract_type and method_id:
+                    if contract_src := self.local_project._create_contract_source(contract_type):
+                        tb = SourceTraceback.create(contract_src, trace, method_id)
 
             vm_err = self.get_virtual_machine_error(
                 err, trace=trace, contract_address=contract_address, source_traceback=tb

tests/functional/geth/test_provider.py

@@ -19,6 +19,7 @@
     ProviderError,
     TransactionError,
     TransactionNotFoundError,
+    VirtualMachineError,
 )
 from ape.utils import to_int
 from ape_ethereum.ecosystem import Block
@@ -40,6 +41,21 @@ def web3_factory(mocker):
     return mocker.patch("ape_ethereum.provider._create_web3")
 
 
+@pytest.fixture
+def tx_for_call(geth_contract):
+    return DynamicFeeTransaction.model_validate(
+        {
+            "chainId": 1337,
+            "to": geth_contract.address,
+            "gas": 4716984,
+            "value": 0,
+            "data": HexBytes(keccak(text="myNumber()")[:4]),
+            "type": 2,
+            "accessList": [],
+        }
+    )
+
+
 @geth_process_test
 def test_uri(geth_provider):
     assert geth_provider.http_uri == GETH_URI
@@ -428,22 +444,81 @@ def test_send_transaction_when_no_error_and_receipt_fails(
 
 
 @geth_process_test
-def test_send_call(geth_provider, ethereum, geth_contract):
-    txn = DynamicFeeTransaction.model_validate(
-        {
-            "chainId": 1337,
-            "to": geth_contract.address,
-            "gas": 4716984,
-            "value": 0,
-            "data": HexBytes(keccak(text="myNumber()")[:4]),
-            "type": 2,
-            "accessList": [],
-        }
-    )
-    actual = geth_provider.send_call(txn)
+def test_send_call(geth_provider, ethereum, tx_for_call):
+    actual = geth_provider.send_call(tx_for_call)
     assert to_int(actual) == 0
 
 
+@geth_process_test
+def test_send_call_base_class_block_id(networks, ethereum, mocker):
+    """
+    Testing a case where was a bug in the base class for most providers.
+    Note: can't use ape-node as-is, as it overrides `send_call()`.
+    """
+
+    provider = mocker.MagicMock()
+    provider.network.name = "mainnet"
+
+    def hacked_send_call(*args, **kwargs):
+        return EthereumNodeProvider.send_call(provider, *args, **kwargs)
+
+    provider.send_call = hacked_send_call
+    tx = ethereum.create_transaction()
+    block_id = 567
+
+    orig = networks.active_provider
+    networks.active_provider = provider
+    _ = provider.send_call(tx, block_id=block_id, skip_trace=True) == HexStr("0x")
+    networks.active_provider = orig  # put back ASAP
+
+    actual = provider._prepare_call.call_args[-1]["block_identifier"]
+    assert actual == block_id
+
+
+@geth_process_test
+def test_send_call_handles_contract_type_failure(mocker, geth_provider, tx_for_call, mock_web3):
+    """
+    Fixes an issue where we would get a recursion error during
+    handling a CALL failure, which would happen during proxy detection.
+    """
+    orig_web3 = geth_provider._web3
+
+    def sfx(rpc, arguments, *args, **kwargs):
+        if rpc == "eth_call" and arguments[0]:
+            raise ContractLogicError()
+
+        return orig_web3.provider.make_request(rpc, arguments, *args, **kwargs)
+
+    # Do this to trigger re-entrancy.
+    mock_get = mocker.MagicMock()
+    mock_get.side_effect = RecursionError
+    orig = geth_provider.chain_manager.contracts.get
+    geth_provider.chain_manager.contracts.get = mock_get
+
+    mock_web3.provider.make_request.side_effect = sfx
+    geth_provider._web3 = mock_web3
+    try:
+        with pytest.raises(VirtualMachineError):
+            geth_provider.send_call(tx_for_call)
+    finally:
+        geth_provider._web3 = orig_web3
+        geth_provider.chain_manager.contracts.get = orig
+
+
+@geth_process_test
+def test_send_call_skip_trace(mocker, geth_provider, ethereum, tx_for_call):
+    """
+    When we pass skip_trace=True to `send_call` (as proxy-checking des), we should
+    also not bother with any traces in exception handling for that call, as proxy-
+    checks fail consistently and getting their traces is unnecessary.
+    """
+    eth_call_spy = mocker.spy(geth_provider, "_eth_call")
+    get_contract_spy = mocker.spy(geth_provider.chain_manager.contracts, "get")
+    geth_provider.send_call(tx_for_call, skip_trace=True)
+    assert eth_call_spy.call_args[1]["skip_trace"] is True
+    assert get_contract_spy.call_count == 0
+
+
 @geth_process_test
 def test_network_choice(geth_provider):
     actual = geth_provider.network_choice
@@ -569,32 +644,6 @@ def test_create_access_list(geth_provider, geth_contract, geth_account):
     assert len(actual[0].storage_keys) > 0
 
 
-@geth_process_test
-def test_send_call_base_class_block_id(networks, ethereum, mocker):
-    """
-    Testing a case where was a bug in the base class for most providers.
-    Note: can't use ape-node as-is, as it overrides `send_call()`.
-    """
-
-    provider = mocker.MagicMock()
-    provider.network.name = "mainnet"
-
-    def hacked_send_call(*args, **kwargs):
-        return EthereumNodeProvider.send_call(provider, *args, **kwargs)
-
-    provider.send_call = hacked_send_call
-    tx = ethereum.create_transaction()
-    block_id = 567
-
-    orig = networks.active_provider
-    networks.active_provider = provider
-    _ = provider.send_call(tx, block_id=block_id, skip_trace=True) == HexStr("0x")
-    networks.active_provider = orig  # put back ASAP
-
-    actual = provider._prepare_call.call_args[-1]["block_identifier"]
-    assert actual == block_id
-
-
 @geth_process_test
 def test_get_virtual_machine_error(geth_provider):
     expected = "__EXPECTED__"