Skip to content

Commit

Permalink
feat: publish immutables layout (vyperlang#2768)
Browse files Browse the repository at this point in the history
add immutables layout to -f layout. this is a breaking change to the
`layout` output, since it adds a top-level key to distinguish between
storage and immutable variables.

this also blocks immutables from colliding with storage variables.
immutables could have the same name as storage variables, depending on
the order they occur in. for instance, this was allowed
```
x: uint256
x: immutable(uint256)
```

but, this was not:
```
x: immutable(uint256)
x: uint256
```

this commit ensures both cases are blocked.
  • Loading branch information
charles-cooper authored Apr 30, 2022
1 parent df29609 commit d366899
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 27 deletions.
36 changes: 29 additions & 7 deletions tests/cli/outputs/test_storage_layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,41 @@ def public_foo3():
output_formats=["layout"],
)

assert out["layout"] == {
"nonreentrant.foo": {"type": "nonreentrant lock", "location": "storage", "slot": 0},
"nonreentrant.bar": {"type": "nonreentrant lock", "location": "storage", "slot": 1},
assert out["layout"]["storage_layout"] == {
"nonreentrant.foo": {"type": "nonreentrant lock", "slot": 0},
"nonreentrant.bar": {"type": "nonreentrant lock", "slot": 1},
"foo": {
"type": "HashMap[address, uint256]",
"location": "storage",
"slot": 2,
},
"arr": {
"type": "DynArray[uint256, 3]",
"location": "storage",
"slot": 3,
},
"baz": {"type": "Bytes[65]", "location": "storage", "slot": 7},
"bar": {"type": "uint256", "location": "storage", "slot": 11},
"baz": {"type": "Bytes[65]", "slot": 7},
"bar": {"type": "uint256", "slot": 11},
}


def test_storage_and_immutables_layout():
code = """
name: String[32]
SYMBOL: immutable(String[32])
DECIMALS: immutable(uint8)
@external
def __init__():
SYMBOL = "VYPR"
DECIMALS = 18
"""

expected_layout = {
"code_layout": {
"DECIMALS": {"length": 32, "offset": 64, "type": "uint8"},
"SYMBOL": {"length": 64, "offset": 0, "type": "String[32]"},
},
"storage_layout": {"name": {"slot": 0, "type": "String[32]"}},
}

out = compile_code(code, output_formats=["layout"])
assert out["layout"] == expected_layout
27 changes: 15 additions & 12 deletions tests/cli/outputs/test_storage_layout_overrides.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,17 @@ def test_storage_layout_overrides():
b: uint256"""

storage_layout_overrides = {
"a": {"type": "uint256", "location": "storage", "slot": 1},
"b": {"type": "uint256", "location": "storage", "slot": 0},
"a": {"type": "uint256", "slot": 1},
"b": {"type": "uint256", "slot": 0},
}

expected_output = {"storage_layout": storage_layout_overrides, "code_layout": {}}

out = compile_code(
code, output_formats=["layout"], storage_layout_override=storage_layout_overrides
)

assert out["layout"] == storage_layout_overrides
assert out["layout"] == expected_output


def test_storage_layout_for_more_complex():
Expand Down Expand Up @@ -57,22 +59,23 @@ def public_foo3():
"""

storage_layout_override = {
"nonreentrant.foo": {"type": "nonreentrant lock", "location": "storage", "slot": 8},
"nonreentrant.bar": {"type": "nonreentrant lock", "location": "storage", "slot": 7},
"nonreentrant.foo": {"type": "nonreentrant lock", "slot": 8},
"nonreentrant.bar": {"type": "nonreentrant lock", "slot": 7},
"foo": {
"type": "HashMap[address, uint256]",
"location": "storage",
"slot": 1,
},
"baz": {"type": "Bytes[65]", "location": "storage", "slot": 2},
"bar": {"type": "uint256", "location": "storage", "slot": 6},
"baz": {"type": "Bytes[65]", "slot": 2},
"bar": {"type": "uint256", "slot": 6},
}

expected_output = {"storage_layout": storage_layout_override, "code_layout": {}}

out = compile_code(
code, output_formats=["layout"], storage_layout_override=storage_layout_override
)

assert out["layout"] == storage_layout_override
assert out["layout"] == expected_output


def test_simple_collision():
Expand All @@ -81,8 +84,8 @@ def test_simple_collision():
symbol: public(String[32])"""

storage_layout_override = {
"name": {"location": "storage", "slot": 0, "type": "String[64]"},
"symbol": {"location": "storage", "slot": 1, "type": "String[32]"},
"name": {"slot": 0, "type": "String[64]"},
"symbol": {"slot": 1, "type": "String[32]"},
}

with pytest.raises(
Expand All @@ -101,7 +104,7 @@ def test_incomplete_overrides():
symbol: public(String[32])"""

storage_layout_override = {
"name": {"location": "storage", "slot": 0, "type": "String[64]"},
"name": {"slot": 0, "type": "String[64]"},
}

with pytest.raises(
Expand Down
29 changes: 21 additions & 8 deletions vyper/semantics/validation/data_positions.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@ def set_data_positions(
vyper_module : vy_ast.Module
Top-level Vyper AST node that has already been annotated with type data.
"""
set_code_offsets(vyper_module)
return (
code_offsets = set_code_offsets(vyper_module)
storage_slots = (
set_storage_slots_with_overrides(vyper_module, storage_layout_overrides)
if storage_layout_overrides is not None
else set_storage_slots(vyper_module)
)

return {"storage_layout": storage_slots, "code_layout": code_offsets}


class StorageAllocator:
"""
Expand Down Expand Up @@ -102,7 +104,6 @@ def set_storage_slots_with_overrides(

ret[variable_name] = {
"type": "nonreentrant lock",
"location": "storage",
"slot": reentrant_slot,
}
else:
Expand Down Expand Up @@ -131,7 +132,7 @@ def set_storage_slots_with_overrides(
reserved_slots.reserve_slot_range(var_slot, storage_length, node.target.id)
type_.set_position(StorageSlot(var_slot))

ret[node.target.id] = {"type": str(type_), "location": "storage", "slot": var_slot}
ret[node.target.id] = {"type": str(type_), "slot": var_slot}
else:
raise StorageLayoutException(
f"Could not find storage_slot for {node.target.id}. "
Expand Down Expand Up @@ -174,7 +175,6 @@ def set_storage_slots(vyper_module: vy_ast.Module) -> StorageLayout:
# we nail down the format better
ret[variable_name] = {
"type": "nonreentrant lock",
"location": "storage",
"slot": storage_slot,
}

Expand All @@ -193,7 +193,7 @@ def set_storage_slots(vyper_module: vy_ast.Module) -> StorageLayout:

# this could have better typing but leave it untyped until
# we understand the use case better
ret[node.target.id] = {"type": str(type_), "location": "storage", "slot": storage_slot}
ret[node.target.id] = {"type": str(type_), "slot": storage_slot}

# CMC 2021-07-23 note that HashMaps get assigned a slot here.
# I'm not sure if it's safe to avoid allocating that slot
Expand All @@ -212,13 +212,26 @@ def set_memory_offsets(fn_node: vy_ast.FunctionDef) -> None:
pass


def set_code_offsets(vyper_module: vy_ast.Module) -> None:
def set_code_offsets(vyper_module: vy_ast.Module) -> Dict:

ret = {}
offset = 0
for node in vyper_module.get_children(
vy_ast.AnnAssign, filters={"annotation.func.id": "immutable"}
):
type_ = node._metadata["type"]
type_.set_position(CodeOffset(offset))

offset += math.ceil(type_.size_in_bytes / 32) * 32
len_ = math.ceil(type_.size_in_bytes / 32) * 32

# this could have better typing but leave it untyped until
# we understand the use case better
ret[node.target.id] = {
"type": str(type_),
"offset": offset,
"length": len_,
}

offset += len_

return ret
5 changes: 5 additions & 0 deletions vyper/semantics/validation/module.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,11 @@ def visit_AnnAssign(self, node):

if is_immutable:
try:
# block immutable if storage variable already exists
if name in self.namespace["self"].members:
raise NamespaceCollision(
f"Value '{name}' has already been declared", node
) from None
self.namespace[name] = type_definition
except VyperException as exc:
raise exc.with_annotation(node) from None
Expand Down

0 comments on commit d366899

Please sign in to comment.