diff --git a/docs/interfaces.rst b/docs/interfaces.rst index acc0ce91f3..9737d3c567 100644 --- a/docs/interfaces.rst +++ b/docs/interfaces.rst @@ -85,10 +85,6 @@ The ``default_return_value`` parameter can be used to handle ERC20 tokens affect extcall IERC20(USDT).transfer(msg.sender, 1, default_return_value=True) # returns True extcall IERC20(USDT).transfer(msg.sender, 1) # reverts because nothing returned -.. warning:: - - When ``skip_contract_check=True`` is used and the called function returns data (ex.: ``x: uint256 = SomeContract.foo(skip_contract_check=True)``, no guarantees are provided by the compiler as to the validity of the returned value. In other words, it is undefined behavior what happens if the called contract did not exist. In particular, the returned value might point to garbage memory. It is therefore recommended to only use ``skip_contract_check=True`` to call contracts which have been manually ensured to exist at the time of the call. - Built-in Interfaces =================== diff --git a/tests/functional/codegen/calling_convention/test_external_contract_calls.py b/tests/functional/codegen/calling_convention/test_external_contract_calls.py index f9252f0a99..d98c8d79dc 100644 --- a/tests/functional/codegen/calling_convention/test_external_contract_calls.py +++ b/tests/functional/codegen/calling_convention/test_external_contract_calls.py @@ -1441,12 +1441,18 @@ def get_lucky(gas_amount: uint256) -> int128: c2.get_lucky(50) # too little gas. -def test_skip_contract_check(get_contract): +def test_skip_contract_check(get_contract, tx_failed): contract_2 = """ @external @view def bar(): pass + +# include fallback for sanity, make sure we don't get trivially rejected in +# selector table +@external +def __default__(): + pass """ contract_1 = """ interface Bar: @@ -1454,9 +1460,10 @@ def bar() -> uint256: view def baz(): nonpayable @external -def call_bar(addr: address): - # would fail if returndatasize check were on - x: uint256 = staticcall Bar(addr).bar(skip_contract_check=True) +def call_bar(addr: address) -> uint256: + # fails during abi decoding + return staticcall Bar(addr).bar(skip_contract_check=True) + @external def call_baz(): # some address with no code @@ -1466,7 +1473,10 @@ def call_baz(): """ c1 = get_contract(contract_1) c2 = get_contract(contract_2) - c1.call_bar(c2.address) + + with tx_failed(): + c1.call_bar(c2.address) + c1.call_baz() diff --git a/vyper/codegen/external_call.py b/vyper/codegen/external_call.py index 1bc83995c5..331b991bfe 100644 --- a/vyper/codegen/external_call.py +++ b/vyper/codegen/external_call.py @@ -109,8 +109,7 @@ def _unpack_returndata(buf, fn_type, call_kwargs, contract_address, context, exp # unpack strictly if not needs_clamp(wrapped_return_t, encoding): - # revert when returndatasize is not in bounds, except when - # skip_contract_check is enabled. + # revert when returndatasize is not in bounds # NOTE: there is an optimization here: when needs_clamp is True, # make_setter (implicitly) checks returndatasize during abi # decoding. @@ -125,14 +124,13 @@ def _unpack_returndata(buf, fn_type, call_kwargs, contract_address, context, exp # another thing we could do instead once we have the machinery is to # simply always use make_setter instead of having this assertion, and # rely on memory analyser to optimize out the memory movement. - if not call_kwargs.skip_contract_check: - assertion = IRnode.from_list( - ["assert", ["ge", "returndatasize", min_return_size]], - error_msg="returndatasize too small", - ) - unpacker.append(assertion) - return_buf = buf + assertion = IRnode.from_list( + ["assert", ["ge", "returndatasize", min_return_size]], + error_msg="returndatasize too small", + ) + unpacker.append(assertion) + return_buf = buf else: return_buf = context.new_internal_variable(wrapped_return_t)