refactor[tool]: refactor storage layout export

[charles-cooper]

What I did

refactor the storage layout export code
cc @ritzdorf @trocher

fixes #3996, #3517, #3989

How I did it

How to verify it

Commit message

refactor storage layout allocator. separate concerns of allocating the
storage layout and exporting the storage layout into separate functions.
this is intended to make it easier to add features to the storage layout
export in the future

fix several bugs in storage layout overrides, including:
- handle stateful modules
- add a sanity check that the override file roundtrips
- ignore non-storage variables in override files
- set nonreentrant lock properly for all functions instead of panicking

misc:
add `n_slots` to each storage layout item in the export

Description for the changelog

Cute Animal Picture

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 95.52239% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 90.21%. Comparing base (abf795a) to head (a2ee205).

❗ Current head a2ee205 differs from pull request most recent head 1042b90

Please upload reports for the commit 1042b90 to get more accurate results.

Files	Patch %	Lines
vyper/semantics/analysis/data_positions.py	95.08%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3789      +/-   ##
==========================================
- Coverage   90.82%   90.21%   -0.62%     
==========================================
  Files         106      106              
  Lines       15309    15333      +24     
  Branches     3366     3378      +12     
==========================================
- Hits        13904    13832      -72     
- Misses        966     1036      +70     
- Partials      439      465      +26     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[fubuloubu]

Is this is a breaking change for tooling?

[charles-cooper]

Is this is a breaking change for tooling?

in theory, no. still ironing out some bugs though

[Leminkay]

with overrides, non-reentrant key will be in storage, but the layout (-f) will return that key in the transient_storage_layout

[charles-cooper]

right, so you're saying the output of -f layout won't match the override file right?

[Leminkay]

thats right

[charles-cooper]

b9acf98, do you think this addresses it appropriately?

[cyberthirst]

i think that overrides don't handle reentrancy lock properly:

override file:

{
  "$.nonreentrant_key": {
    "type": "uint256",
    "slot": 1
  }
}

# main.vy

c: transient(uint256)

@external
@nonreentrant
def foo():
    self.c = 0

and run with cancun:

 vyper tests/custom/main.vy -f layout --evm cancun --storage-layout-file tests/custom/override.json

outputs:

{
  "transient_storage_layout": {
    "c": {
      "type": "uint256",
      "n_slots": 1,
      "slot": 1
    },
    "$.nonreentrant_key": {
      "type": "nonreentrant lock",
      "slot": 1,
      "n_slots": 1
    }
  }
}

EDIT - adding also the corresponding IR

[label,
  external 0 foo()_common,
  var_list,
  [seq,
    [seq, [assert, [ne, 1, [tload, 1]]], [tstore, 1, 1]],
    [seq,
      [seq, [unique_symbol, tstore_1], [tstore, 1 <self.c>, 0 <0>]],
      [exit_to, external 0 foo()_cleanup],
      pass]]],
[label, external 0 foo()_cleanup, var_list, [seq, [tstore, 1, 0], stop]]]]]],

[charles-cooper]

i added a sanity check that the storage layout roundtrips in b9acf98

[cyberthirst]

function not exported, but -f layout includes the lock:

# main.vy
import lib1

initializes: lib1

@deploy
def __init__():
    lib1.counter = 99
    
# lib1.vy
counter: uint256

@external
@nonreentrant
def foo() -> uint256:
    return self.counter

Layout:

{
  "storage_layout": {
    "lib1": {
      "counter": {
        "type": "uint256",
        "n_slots": 1,
        "slot": 0
      }
    }
  },
  "transient_storage_layout": {
    "$.nonreentrant_key": {
      "type": "nonreentrant lock",
      "slot": 0,
      "n_slots": 1
    }
  }
}

EDIT: updated main.vy

[cyberthirst]

var_name unused

vyper/vyper/semantics/analysis/data_positions.py

Line 65 in 4fb1653

def allocate_slot(self, n, var_name, node=None):

[cyberthirst]

rather nothing, but reporting for completness

lib1: uint256

@external
def foo():
    pass

{
  "lib1": {
    "slot": {
      "slot": 20,
      "type": "uint256",
      "n_slots": 1
    }
  }
}

panics w:

Error compiling: tests/custom/main.vy
TypeError: unsupported operand type(s) for +: 'int' and 'dict'

[charles-cooper]

var_name unused

vyper/vyper/semantics/analysis/data_positions.py

Line 65 in 4fb1653

def allocate_slot(self, n, var_name, node=None):

cdcfb81

[cyberthirst]

this part won't be fun with large arrays - in the future we should try to rewrite the logic to use interval intersections

vyper/vyper/semantics/analysis/data_positions.py

Lines 133 to 148 in 1042b90

	def _reserve_slots(self, slots: list[int], var_name: str) -> None:
	for slot in slots:
	self._reserve_slot(slot, var_name)
	def _reserve_slot(self, slot: int, var_name: str) -> None:
	if slot < 0 or slot >= 2**256:
	raise StorageLayoutException(
	f"Invalid storage slot for var {var_name}, out of bounds: {slot}"
	)
	if slot in self.occupied_slots:
	collided_var = self.occupied_slots[slot]
	raise StorageLayoutException(
	f"Storage collision! Tried to assign '{var_name}' to slot {slot} but it has "
	f"already been reserved by '{collided_var}'"
	)
	self.occupied_slots[slot] = var_name

EDIT: even for moderately-sized DynArrays (say ~2**30) we'll run out of memory on the host

[charles-cooper]

this part won't be fun with large arrays - in the future we should try to rewrite the logic to use interval intersections

vyper/vyper/semantics/analysis/data_positions.py

Lines 133 to 148 in 1042b90

	def _reserve_slots(self, slots: list[int], var_name: str) -> None:
	for slot in slots:
	self._reserve_slot(slot, var_name)
	def _reserve_slot(self, slot: int, var_name: str) -> None:
	if slot < 0 or slot >= 2**256:
	raise StorageLayoutException(
	f"Invalid storage slot for var {var_name}, out of bounds: {slot}"
	)
	if slot in self.occupied_slots:
	collided_var = self.occupied_slots[slot]
	raise StorageLayoutException(
	f"Storage collision! Tried to assign '{var_name}' to slot {slot} but it has "
	f"already been reserved by '{collided_var}'"
	)
	self.occupied_slots[slot] = var_name

EDIT: even for moderately-sized DynArrays (say ~2**30) we'll run out of memory on the host

dup of / tracked in #3966