feat[codegen]!: check returndatasize even when skip_contract_check is set

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
 ===================
 

tests/functional/codegen/calling_convention/test_external_contract_calls.py

@@ -1441,22 +1441,29 @@ 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:
     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()
 
 

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)