Skip to content

Commit

Permalink
refactor: optimize calldatasize check (#3104)
Browse files Browse the repository at this point in the history
this changes the calldatasize check from a global/contract-entry check
to a function-local check - the calldatasize check only needs to be
present if a selector is 0. this maintains the invariant protecting
against the bug in #1603 while being more efficient.

Co-authored-by: Charles Cooper <cooper.charles.m@gmail.com>
  • Loading branch information
emc415 and charles-cooper authored Jan 20, 2023
1 parent deb3378 commit 02339df
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 9 deletions.
11 changes: 6 additions & 5 deletions tests/parser/features/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@ def __init__(a: uint256):
assert c.val() == 123

# Make sure the init code does not access calldata
opcodes = vyper.compile_code(code, ["opcodes"])["opcodes"].split(" ")
ir_return_idx = opcodes.index("JUMP")
assembly = vyper.compile_code(code, ["asm"])["asm"].split(" ")
ir_return_idx_start = assembly.index("{")
ir_return_idx_end = assembly.index("}")

assert "CALLDATALOAD" in opcodes
assert "CALLDATACOPY" not in opcodes[:ir_return_idx]
assert "CALLDATALOAD" not in opcodes[:ir_return_idx]
assert "CALLDATALOAD" in assembly
assert "CALLDATACOPY" not in assembly[:ir_return_idx_start] + assembly[ir_return_idx_end:]
assert "CALLDATALOAD" not in assembly[:ir_return_idx_start] + assembly[ir_return_idx_end:]


def test_init_calls_internal(get_contract, assert_compile_failed, assert_tx_failed):
Expand Down
19 changes: 18 additions & 1 deletion vyper/codegen/function_definitions/external_function.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,24 @@ def handler_for(calldata_kwargs, default_kwargs):

ret.append(["goto", sig.external_function_base_entry_label])

ret = ["if", ["eq", "_calldata_method_id", method_id], ret]
method_id_check = ["eq", "_calldata_method_id", method_id]

# if there is a function whose selector is 0, it won't be distinguished
# from the case where nil calldata is supplied, b/c calldataload loads
# 0s past the end of physical calldata (cf. yellow paper).
# since supplying 0 calldata is expected to trigger the fallback fn,
# we check that calldatasize > 0, which distinguishes the 0 selector
# from the fallback function "selector"
# (equiv. to "all selectors not in the selector table").

# note: cases where not enough calldata is supplied (besides
# calldatasize==0) are not addressed here b/c a calldatasize
# well-formedness check is already present in the function body
# as part of abi validation
if method_id.value == 0:
method_id_check = ["and", ["gt", "calldatasize", 0], method_id_check]

ret = ["if", method_id_check, ret]
return ret

ret = ["seq"]
Expand Down
3 changes: 0 additions & 3 deletions vyper/codegen/module.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,6 @@ def _runtime_ir(runtime_functions, all_sigs, global_ctx):

runtime = [
"seq",
# check that calldatasize is at least 4, otherwise
# calldataload will load zeros (cf. yellow paper).
["if", ["lt", "calldatasize", 4], ["goto", "fallback"]],
["with", "_calldata_method_id", shr(224, ["calldataload", 0]), selector_section],
close_selector_section,
["label", "fallback", ["var_list"], fallback_ir],
Expand Down

0 comments on commit 02339df

Please # to comment.