Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat[lang]: add revert_on_failure kwarg for create builtins #3844

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 43 additions & 23 deletions tests/functional/builtins/codegen/test_create_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,17 @@ def test2(a: uint256) -> Bytes[100]:
assert receipt["gasUsed"] < GAS_SENT


def test_create_minimal_proxy_to_create2(get_contract, create2_address_of, keccak, tx_failed):
code = """
@pytest.mark.parametrize("revert_on_failure", [True, False, None])
def test_create_minimal_proxy_to_create2(
get_contract, create2_address_of, keccak, tx_failed, revert_on_failure
):
revert_arg = "" if revert_on_failure is None else f", revert_on_failure={revert_on_failure}"
code = f"""
main: address

@external
def test(_salt: bytes32) -> address:
self.main = create_minimal_proxy_to(self, salt=_salt)
self.main = create_minimal_proxy_to(self, salt=_salt{revert_arg})
charles-cooper marked this conversation as resolved.
Show resolved Hide resolved
return self.main
"""

Expand All @@ -129,16 +133,28 @@ def test(_salt: bytes32) -> address:

c.test(salt, transact={})
# revert on collision
with tx_failed():
c.test(salt, transact={})
if revert_on_failure is False:
assert not c.test(salt)
else:
with tx_failed():
c.test(salt, transact={})


# test blueprints with various prefixes - 0xfe would block calls to the blueprint
# contract, and 0xfe7100 is ERC5202 magic
@pytest.mark.parametrize("blueprint_prefix", [b"", b"\xfe", ERC5202_PREFIX])
@pytest.mark.parametrize("revert_on_failure", [True, False, None])
def test_create_from_blueprint(
get_contract, deploy_blueprint_for, w3, keccak, create2_address_of, tx_failed, blueprint_prefix
get_contract,
deploy_blueprint_for,
w3,
keccak,
create2_address_of,
tx_failed,
blueprint_prefix,
revert_on_failure,
):
revert_arg = "" if revert_on_failure is None else f", revert_on_failure={revert_on_failure}"
code = """
@external
def foo() -> uint256:
Expand All @@ -151,14 +167,16 @@ def foo() -> uint256:

@external
def test(target: address):
self.created_address = create_from_blueprint(target, code_offset={prefix_len})
self.created_address = create_from_blueprint(target, code_offset={prefix_len}{revert_arg})

@external
def test2(target: address, salt: bytes32):
self.created_address = create_from_blueprint(target, code_offset={prefix_len}, salt=salt)
self.created_address = create_from_blueprint(
target, code_offset={prefix_len}, salt=salt{revert_arg}
)
"""

# deploy a foo so we can compare its bytecode with factory deployed version
# deploy a foo, so we can compare its bytecode with factory deployed version
foo_contract = get_contract(code)
expected_runtime_code = w3.eth.get_code(foo_contract.address)

Expand Down Expand Up @@ -191,8 +209,11 @@ def test2(target: address, salt: bytes32):
assert HexBytes(test.address) == create2_address_of(d.address, salt, initcode)

# can't collide addresses
with tx_failed():
d.test2(f.address, salt)
if revert_on_failure is False:
assert not d.test2(f.address, salt)
else:
with tx_failed():
d.test2(f.address, salt)


# test blueprints with 0xfe7100 prefix, which is the EIP 5202 standard.
Expand Down Expand Up @@ -425,16 +446,18 @@ def should_fail(target: address, arg1: String[129], arg2: Bar):
w3.eth.send_transaction({"to": d.address, "data": f"{sig}{encoded}"})


def test_create_copy_of(get_contract, w3, keccak, create2_address_of, tx_failed):
code = """
@pytest.mark.parametrize("revert_on_failure", [True, False, None])
def test_create_copy_of(get_contract, w3, keccak, create2_address_of, tx_failed, revert_on_failure):
revert_arg = "" if revert_on_failure is None else f", revert_on_failure={revert_on_failure}"
code = f"""
created_address: public(address)
@internal
def _create_copy_of(target: address):
self.created_address = create_copy_of(target)
self.created_address = create_copy_of(target{revert_arg})

@internal
def _create_copy_of2(target: address, salt: bytes32):
self.created_address = create_copy_of(target, salt=salt)
self.created_address = create_copy_of(target, salt=salt{revert_arg})

@external
def test(target: address) -> address:
Expand Down Expand Up @@ -473,14 +496,11 @@ def test2(target: address, salt: bytes32) -> address:
assert HexBytes(test2) == create2_address_of(c.address, salt, vyper_initcode(bytecode))

# can't create2 where contract already exists
with tx_failed():
c.test2(c.address, salt, transact={})

# test single byte contract
# test2 = c.test2(b"\x01", salt)
# assert HexBytes(test2) == create2_address_of(c.address, salt, vyper_initcode(b"\x01"))
# with tx_failed():
# c.test2(bytecode, salt)
if revert_on_failure is False:
charles-cooper marked this conversation as resolved.
Show resolved Hide resolved
assert not c.test2(c.address, salt)
else:
with tx_failed():
c.test2(c.address, salt)


# XXX: these various tests to check the msize allocator for
Expand Down
20 changes: 12 additions & 8 deletions vyper/builtins/functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1584,7 +1584,7 @@ def build_IR(self, expr, context):

# create helper functions
# generates CREATE op sequence + zero check for result
def _create_ir(value, buf, length, salt, checked=True):
def _create_ir(value, buf, length, salt, revert_on_failure=True):
args = [value, buf, length]
create_op = "create"
if salt is not CREATE2_SENTINEL:
Expand All @@ -1595,7 +1595,7 @@ def _create_ir(value, buf, length, salt, checked=True):
["seq", eval_once_check(_freshname("create_builtin")), [create_op, *args]]
)

if not checked:
if not revert_on_failure:
return ret

ret = clamp_nonzero(ret)
Expand Down Expand Up @@ -1696,6 +1696,7 @@ class _CreateBase(BuiltinFunctionT):
_kwargs = {
"value": KwargSettings(UINT256_T, zero_value),
"salt": KwargSettings(BYTES32_T, empty_value),
"revert_on_failure": KwargSettings(BoolT(), True, require_literal=True),
}
_return_type = AddressT()

Expand Down Expand Up @@ -1729,7 +1730,7 @@ def _add_gas_estimate(self, args, should_use_create2):
bytecode_len = 20 + len(b) + len(c)
return _create_addl_gas_estimate(bytecode_len, should_use_create2)

def _build_create_IR(self, expr, args, context, value, salt):
def _build_create_IR(self, expr, args, context, value, salt, revert_on_failure):
target_address = args[0]

buf = context.new_internal_variable(BytesT(96))
Expand Down Expand Up @@ -1757,7 +1758,7 @@ def _build_create_IR(self, expr, args, context, value, salt):
["mstore", buf, forwarder_preamble],
["mstore", ["add", buf, preamble_length], aligned_target],
["mstore", ["add", buf, preamble_length + 20], forwarder_post],
_create_ir(value, buf, buf_len, salt=salt),
_create_ir(value, buf, buf_len, salt, revert_on_failure),
]


Expand Down Expand Up @@ -1786,7 +1787,7 @@ def _add_gas_estimate(self, args, should_use_create2):
# max possible runtime length + preamble length
return _create_addl_gas_estimate(EIP_170_LIMIT + self._preamble_len, should_use_create2)

def _build_create_IR(self, expr, args, context, value, salt):
def _build_create_IR(self, expr, args, context, value, salt, revert_on_failure):
target = args[0]

# something we can pass to scope_multi
Expand Down Expand Up @@ -1820,7 +1821,7 @@ def _build_create_IR(self, expr, args, context, value, salt):
buf = add_ofst(mem_ofst, 32 - preamble_len)
buf_len = ["add", codesize, preamble_len]

ir.append(_create_ir(value, buf, buf_len, salt))
ir.append(_create_ir(value, buf, buf_len, salt, revert_on_failure))

return b1.resolve(b2.resolve(ir))

Expand All @@ -1833,6 +1834,7 @@ class CreateFromBlueprint(_CreateBase):
"salt": KwargSettings(BYTES32_T, empty_value),
"raw_args": KwargSettings(BoolT(), False, require_literal=True),
"code_offset": KwargSettings(UINT256_T, IRnode.from_list(3, typ=UINT256_T)),
"revert_on_failure": KwargSettings(BoolT(), True, require_literal=True),
}
_has_varargs = True

Expand All @@ -1842,7 +1844,9 @@ def _add_gas_estimate(self, args, should_use_create2):
maxlen = EIP_170_LIMIT + ctor_args.typ.abi_type.size_bound()
return _create_addl_gas_estimate(maxlen, should_use_create2)

def _build_create_IR(self, expr, args, context, value, salt, code_offset, raw_args):
def _build_create_IR(
self, expr, args, context, value, salt, code_offset, raw_args, revert_on_failure
):
target = args[0]
ctor_args = args[1:]

Expand Down Expand Up @@ -1918,7 +1922,7 @@ def _build_create_IR(self, expr, args, context, value, salt, code_offset, raw_ar

length = ["add", codesize, encoded_args_len]

ir.append(_create_ir(value, mem_ofst, length, salt))
ir.append(_create_ir(value, mem_ofst, length, salt, revert_on_failure))

return b1.resolve(b2.resolve(ir))

Expand Down
Loading