Skip to content

Commit

Permalink
fix: metadata journal can rollback incorrectly (#3569)
Browse files Browse the repository at this point in the history
this commit fixes an issue where multiple writes inside of a checkpoint
lead to journal corruption on rollback. it ensures a call to
`register_update()` when the metadata dict has already been updated
inside of a given checkpoint.

note this does not change any observed functionality in the compiler
because writes to the metadata journal inside for loops only ever happen
to be written once, but it prevents a bug in case we ever add multiple
writes inside of the same checkpoint.

chainsec june review 5.3

---------

Co-authored-by: tserg <8017125+tserg@users.noreply.github.com>
  • Loading branch information
charles-cooper and tserg committed Sep 5, 2023
1 parent a854929 commit 39a2313
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 1 deletion.
82 changes: 82 additions & 0 deletions tests/ast/test_metadata_journal.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
from vyper.ast.metadata import NodeMetadata
from vyper.exceptions import VyperException


def test_metadata_journal_basic():
m = NodeMetadata()

m["x"] = 1
assert m["x"] == 1


def test_metadata_journal_commit():
m = NodeMetadata()

with m.enter_typechecker_speculation():
m["x"] = 1

assert m["x"] == 1


def test_metadata_journal_exception():
m = NodeMetadata()

m["x"] = 1
try:
with m.enter_typechecker_speculation():
m["x"] = 2
m["x"] = 3

assert m["x"] == 3
raise VyperException("dummy exception")

except VyperException:
pass

# rollback upon exception
assert m["x"] == 1


def test_metadata_journal_rollback_inner():
m = NodeMetadata()

m["x"] = 1
with m.enter_typechecker_speculation():
m["x"] = 2

try:
with m.enter_typechecker_speculation():
m["x"] = 3
m["x"] = 4 # test multiple writes

assert m["x"] == 4
raise VyperException("dummy exception")

except VyperException:
pass

assert m["x"] == 2


def test_metadata_journal_rollback_outer():
m = NodeMetadata()

m["x"] = 1
try:
with m.enter_typechecker_speculation():
m["x"] = 2

with m.enter_typechecker_speculation():
m["x"] = 3
m["x"] = 4 # test multiple writes

assert m["x"] == 4

m["x"] = 5

raise VyperException("dummy exception")

except VyperException:
pass

assert m["x"] == 1
5 changes: 4 additions & 1 deletion vyper/ast/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,11 @@ def __init__(self):
self._node_updates: list[dict[tuple[int, str, Any], NodeMetadata]] = []

def register_update(self, metadata, k):
KEY = (id(metadata), k)
if KEY in self._node_updates[-1]:
return
prev = metadata.get(k, self._NOT_FOUND)
self._node_updates[-1][(id(metadata), k)] = (metadata, prev)
self._node_updates[-1][KEY] = (metadata, prev)

@contextlib.contextmanager
def enter(self):
Expand Down

0 comments on commit 39a2313

Please sign in to comment.