From e2839a0ac9b7edbb261e9262d0068926cde0a959 Mon Sep 17 00:00:00 2001 From: antazoey Date: Mon, 5 Aug 2024 17:21:52 -0500 Subject: [PATCH] fix: skip trace contract-logic errors during proxy-detection and more error-handling in eth_call (#2207) --- src/ape_ethereum/provider.py | 37 +++++-- tests/functional/geth/test_provider.py | 127 +++++++++++++++++-------- 2 files changed, 115 insertions(+), 49 deletions(-) diff --git a/src/ape_ethereum/provider.py b/src/ape_ethereum/provider.py index 19eae807af..31c9643cd7 100644 --- a/src/ape_ethereum/provider.py +++ b/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 diff --git a/tests/functional/geth/test_provider.py b/tests/functional/geth/test_provider.py index c7ecc5d314..b960c4c3a0 100644 --- a/tests/functional/geth/test_provider.py +++ b/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__"