fix: implements check for indexed event arguments (#3570)

prior to this commit, implementing an interface with wrong indexed
arguments for an event would pass the implements check. this commit
fixes the behavior.

chainsec june 2023 review 5.12

---------

Co-authored-by: tserg <[email protected]>

tests/parser/functions/test_interfaces.py

@@ -67,7 +67,6 @@ def test_basic_interface_implements(assert_compile_failed):
 
 implements: ERC20
 
-
 @external
 def test() -> bool:
     return True
@@ -146,6 +145,7 @@ def bar() -> uint256:
     )
 
 
+# check that event types match
 def test_malformed_event(assert_compile_failed):
     interface_code = """
 event Foo:
@@ -173,6 +173,64 @@ def bar() -> uint256:
     )
 
 
+# check that event non-indexed arg needs to match interface
+def test_malformed_events_indexed(assert_compile_failed):
+    interface_code = """
+event Foo:
+    a: uint256
+    """
+
+    interface_codes = {"FooBarInterface": {"type": "vyper", "code": interface_code}}
+
+    not_implemented_code = """
+import a as FooBarInterface
+
+implements: FooBarInterface
+
+# a should not be indexed
+event Foo:
+    a: indexed(uint256)
+
+@external
+def bar() -> uint256:
+    return 1
+    """
+
+    assert_compile_failed(
+        lambda: compile_code(not_implemented_code, interface_codes=interface_codes),
+        InterfaceViolation,
+    )
+
+
+# check that event indexed arg needs to match interface
+def test_malformed_events_indexed2(assert_compile_failed):
+    interface_code = """
+event Foo:
+    a: indexed(uint256)
+    """
+
+    interface_codes = {"FooBarInterface": {"type": "vyper", "code": interface_code}}
+
+    not_implemented_code = """
+import a as FooBarInterface
+
+implements: FooBarInterface
+
+# a should be indexed
+event Foo:
+    a: uint256
+
+@external
+def bar() -> uint256:
+    return 1
+    """
+
+    assert_compile_failed(
+        lambda: compile_code(not_implemented_code, interface_codes=interface_codes),
+        InterfaceViolation,
+    )
+
+
 VALID_IMPORT_CODE = [
     # import statement, import path without suffix
     ("import a as Foo", "a"),

tests/parser/syntax/test_interfaces.py

@@ -134,6 +134,82 @@ def f(a: uint256): # visibility is nonpayable instead of view
     """,
         InterfaceViolation,
     ),
+    (
+        # `receiver` of `Transfer` event should be indexed
+        """
+from vyper.interfaces import ERC20
+
+implements: ERC20
+
+event Transfer:
+    sender: indexed(address)
+    receiver: address
+    value: uint256
+
+event Approval:
+    owner: indexed(address)
+    spender: indexed(address)
+    value: uint256
+
+name: public(String[32])
+symbol: public(String[32])
+decimals: public(uint8)
+balanceOf: public(HashMap[address, uint256])
+allowance: public(HashMap[address, HashMap[address, uint256]])
+totalSupply: public(uint256)
+
+@external
+def transfer(_to : address, _value : uint256) -> bool:
+    return True
+
+@external
+def transferFrom(_from : address, _to : address, _value : uint256) -> bool:
+    return True
+
+@external
+def approve(_spender : address, _value : uint256) -> bool:
+    return True
+    """,
+        InterfaceViolation,
+    ),
+    (
+        # `value` of `Transfer` event should not be indexed
+        """
+from vyper.interfaces import ERC20
+
+implements: ERC20
+
+event Transfer:
+    sender: indexed(address)
+    receiver: indexed(address)
+    value: indexed(uint256)
+
+event Approval:
+    owner: indexed(address)
+    spender: indexed(address)
+    value: uint256
+
+name: public(String[32])
+symbol: public(String[32])
+decimals: public(uint8)
+balanceOf: public(HashMap[address, uint256])
+allowance: public(HashMap[address, HashMap[address, uint256]])
+totalSupply: public(uint256)
+
+@external
+def transfer(_to : address, _value : uint256) -> bool:
+    return True
+
+@external
+def transferFrom(_from : address, _to : address, _value : uint256) -> bool:
+    return True
+
+@external
+def approve(_spender : address, _value : uint256) -> bool:
+    return True
+    """,
+        InterfaceViolation,
+    ),
 ]
 
 

vyper/builtins/interfaces/ERC721.py

@@ -2,18 +2,18 @@
 # Events
 
 event Transfer:
-    _from: address
-    _to: address
-    _tokenId: uint256
+    _from: indexed(address)
+    _to: indexed(address)
+    _tokenId: indexed(uint256)
 
 event Approval:
-    _owner: address
-    _approved: address
-    _tokenId: uint256
+    _owner: indexed(address)
+    _approved: indexed(address)
+    _tokenId: indexed(uint256)
 
 event ApprovalForAll:
-    _owner: address
-    _operator: address
+    _owner: indexed(address)
+    _operator: indexed(address)
     _approved: bool
 
 # Functions

vyper/semantics/types/user.py

@@ -164,6 +164,7 @@ def __init__(self, name: str, arguments: dict, indexed: list) -> None:
         super().__init__(members=arguments)
         self.name = name
         self.indexed = indexed
+        assert len(self.indexed) == len(self.arguments)
         self.event_id = int(keccak256(self.signature.encode()).hex(), 16)
 
     # backward compatible
@@ -172,8 +173,13 @@ def arguments(self):
         return self.members
 
     def __repr__(self):
-        arg_types = ",".join(repr(a) for a in self.arguments.values())
-        return f"event {self.name}({arg_types})"
+        args = []
+        for is_indexed, (_, argtype) in zip(self.indexed, self.arguments.items()):
+            argtype_str = repr(argtype)
+            if is_indexed:
+                argtype_str = f"indexed({argtype_str})"
+            args.append(f"{argtype_str}")
+        return f"event {self.name}({','.join(args)})"
 
     # TODO rename to abi_signature
     @property
@@ -337,12 +343,17 @@ def _is_function_implemented(fn_name, fn_type):
 
         # check for missing events
         for name, event in self.events.items():
+            if name not in namespace:
+                unimplemented.append(name)
+                continue
+
+            if not isinstance(namespace[name], EventT):
+                unimplemented.append(f"{name} is not an event!")
             if (
-                name not in namespace
-                or not isinstance(namespace[name], EventT)
-                or namespace[name].event_id != event.event_id
+                namespace[name].event_id != event.event_id
+                or namespace[name].indexed != event.indexed
             ):
-                unimplemented.append(name)
+                unimplemented.append(f"{name} is not implemented! (should be {event})")
 
         if len(unimplemented) > 0:
             # TODO: improve the error message for cases where the