Merge pull request #13135 from ethereum/foreign-event-access-revert-and-selector-cleanup

cameel · web-flow · commit dccc06cc29dd · 2022-06-14T18:14:30.000+02:00
Revert access to foreign events and clean up event/error selector tests/docs
diff --git a/Changelog.md b/Changelog.md
@@ -6,7 +6,6 @@ Important Bugfixes:
 
 Language Features:
  * Add `E.selector` for a non-anonymous event `E` to access the 32-byte selector topic.
- * Errors and Events allow qualified access from other contracts.
 
 
 Compiler Features:
diff --git a/docs/abi-spec.rst b/docs/abi-spec.rst
@@ -19,7 +19,7 @@ We assume that all contracts will have the interface definitions of any contract
 This specification does not address contracts whose interface is dynamic or otherwise known only at run-time.
 
 .. _abi_function_selector:
-.. index:: selector
+.. index:: ! selector; of a function
 
 Function Selector
 =================
@@ -503,6 +503,7 @@ efficient search and arbitrary legibility by defining events with two arguments
 indexed, one not — intended to hold the same value.
 
 .. _abi_errors:
+.. index:: error, selector; of an error
 
 Errors
 ======
diff --git a/docs/assembly.rst b/docs/assembly.rst
@@ -116,7 +116,7 @@ efficient code, for example:
         }
     }
 
-
+.. index:: selector; of a function
 
 Access to External Variables, Functions and Libraries
 -----------------------------------------------------
diff --git a/docs/contracts/errors.rst b/docs/contracts/errors.rst
@@ -1,5 +1,4 @@
-.. index:: ! error, revert
-
+.. index:: ! error, revert, ! selector; of an error
 .. _errors:
 
 *******************************
@@ -80,3 +79,8 @@ of the built-in type ``Panic(uint256)``.
     by default. This means that an inner call
     can "forge" revert data that looks like it could have come from the
     contract that called it.
+
+Members of Errors
+=================
+
+- ``error.selector``: A ``bytes4`` value containing the error selector.
diff --git a/docs/contracts/events.rst b/docs/contracts/events.rst
@@ -1,4 +1,4 @@
-.. index:: ! event
+.. index:: ! event, ! event; anonymous, ! event; indexed, ! event; topic
 
 .. _events:
 
@@ -73,6 +73,8 @@ four indexed arguments rather than three.
     In particular, it is possible to "fake" the signature of another event
     using an anonymous event.
 
+.. index:: ! selector; of an event
+
 Members of Events
 =================
 
@@ -147,7 +149,7 @@ The output of the above looks like the following (trimmed):
     }
 
 Additional Resources for Understanding Events
-==============================================
+=============================================
 
 - `Javascript documentation <https://github.com/ethereum/web3.js/blob/1.x/docs/web3-eth-contract.rst#events>`_
 - `Example usage of events <https://github.com/ethchange/smart-exchange/blob/master/lib/contracts/SmartExchange.sol>`_
diff --git a/docs/contracts/libraries.rst b/docs/contracts/libraries.rst
@@ -215,7 +215,7 @@ In comparison to contracts, libraries are restricted in the following ways:
 (These might be lifted at a later point.)
 
 .. _library-selectors:
-.. index:: selector
+.. index:: ! selector; of a library function
 
 Function Signatures and Selectors in Libraries
 ==============================================
diff --git a/docs/introduction-to-smart-contracts.rst b/docs/introduction-to-smart-contracts.rst
@@ -548,7 +548,7 @@ these **create calls** and normal message calls is that the payload data is
 executed and the result stored as code and the caller / creator
 receives the address of the new contract on the stack.
 
-.. index:: selfdestruct, self-destruct, deactivate
+.. index:: ! selfdestruct, deactivate
 
 Deactivate and Self-destruct
 ============================
diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h
@@ -1216,7 +1216,7 @@ class EventDefinition: public CallableDeclaration, public StructurallyDocumented
 	FunctionTypePointer functionType(bool /*_internal*/) const override;
 
 	bool isVisibleInDerivedContracts() const override { return true; }
-	bool isVisibleViaContractTypeAccess() const override { return true; }
+	bool isVisibleViaContractTypeAccess() const override { return false; /* TODO */ }
 
 	EventDefinitionAnnotation& annotation() const override;
 
diff --git a/test/libsolidity/semanticTests/error/selector.sol b/test/libsolidity/semanticTests/error/selector.sol
diff --git a/test/libsolidity/semanticTests/errors/error_selector.sol b/test/libsolidity/semanticTests/errors/error_selector.sol
diff --git a/test/libsolidity/semanticTests/events/event_selector.sol b/test/libsolidity/semanticTests/events/event_selector.sol
@@ -7,32 +7,26 @@ library S {
 library T {
     event E();
 }
-interface I {
-    event E();
-}
 
 contract D {
     event F();
 }
 
 contract C is D {
-    function test1() external pure returns (bytes32, bytes32, bytes32) {
+    function test1() external pure returns (bytes32, bytes32) {
         assert(L.E.selector == T.E.selector);
-        assert(I.E.selector == L.E.selector);
 
         assert(L.E.selector != S.E.selector);
         assert(T.E.selector != S.E.selector);
-        assert(I.E.selector != S.E.selector);
 
-        return (L.E.selector, S.E.selector, I.E.selector);
+        return (L.E.selector, S.E.selector);
     }
 
     bytes32 s1 = L.E.selector;
     bytes32 s2 = S.E.selector;
     bytes32 s3 = T.E.selector;
-    bytes32 s4 = I.E.selector;
-    function test2() external returns (bytes32, bytes32, bytes32, bytes32) {
-        return (s1, s2, s3, s4);
+    function test2() external returns (bytes32, bytes32, bytes32) {
+        return (s1, s2, s3);
     }
 
     function test3() external returns (bytes32) {
@@ -42,6 +36,6 @@ contract C is D {
 // ====
 // compileViaYul: also
 // ----
-// test1() -> 0x92bbf6e823a631f3c8e09b1c8df90f378fb56f7fbc9701827e1ff8aad7f6a028, 0x2ff0672f372fbe844b353429d4510ea5e43683af134c54f75f789ff57bc0c0, 0x92bbf6e823a631f3c8e09b1c8df90f378fb56f7fbc9701827e1ff8aad7f6a028
-// test2() -> 0x92bbf6e823a631f3c8e09b1c8df90f378fb56f7fbc9701827e1ff8aad7f6a028, 0x2ff0672f372fbe844b353429d4510ea5e43683af134c54f75f789ff57bc0c0, 0x92bbf6e823a631f3c8e09b1c8df90f378fb56f7fbc9701827e1ff8aad7f6a028, 0x92bbf6e823a631f3c8e09b1c8df90f378fb56f7fbc9701827e1ff8aad7f6a028
+// test1() -> 0x92bbf6e823a631f3c8e09b1c8df90f378fb56f7fbc9701827e1ff8aad7f6a028, 0x2ff0672f372fbe844b353429d4510ea5e43683af134c54f75f789ff57bc0c0
+// test2() -> 0x92bbf6e823a631f3c8e09b1c8df90f378fb56f7fbc9701827e1ff8aad7f6a028, 0x2ff0672f372fbe844b353429d4510ea5e43683af134c54f75f789ff57bc0c0, 0x92bbf6e823a631f3c8e09b1c8df90f378fb56f7fbc9701827e1ff8aad7f6a028
 // test3() -> 0x28811f5935c16a099486acb976b3a6b4942950a1425a74e9eb3e9b7f7135e12a
diff --git a/test/libsolidity/semanticTests/events/simple.sol b/test/libsolidity/semanticTests/events/simple.sol
@@ -2,7 +2,7 @@ contract C {
     event E();
 }
 
-contract Test {
+contract Test is C {
     event E(uint256, uint256);
     function f() public {
         emit C.E();
diff --git a/test/libsolidity/syntaxTests/errors/error_selector_syntax.sol b/test/libsolidity/syntaxTests/errors/error_selector_syntax.sol
@@ -1,48 +1,34 @@
 library L {
-    error E();
-}
-library S {
-    error E(uint);
-}
-library T {
-    error E();
+    error E(bytes4, bool, bytes);
 }
 
-error E();
+error E(bytes4, bool, bytes);
 
 interface I {
-    error E();
-    function f() external pure;
+    error E(bytes4, bool, bytes);
 }
 
-contract D {
-    error F();
+contract B {
+    error E(bytes4, bool, bytes);
 }
 
-contract C is D {
-    function test1() public pure returns (bytes4, bytes4, bytes4, bytes4) {
-        assert(L.E.selector == T.E.selector);
-        assert(L.E.selector != S.E.selector);
-        assert(E.selector == L.E.selector);
-        assert(I.E.selector == L.E.selector);
-        return (L.E.selector, S.E.selector, E.selector, I.E.selector);
-    }
+contract C is B {
+    bytes4 public librarySelector = L.E.selector;
+    bytes4 internal freeSelector = E.selector;
+    bytes4 internal contractSelector = B.E.selector;
+    bytes4 private interfaceSelector = I.E.selector;
 
-    bytes4 s1 = L.E.selector;
-    bytes4 s2 = S.E.selector;
-    bytes4 s3 = T.E.selector;
-    bytes4 s4 = I.E.selector;
-
-    function test2() view external returns (bytes4, bytes4, bytes4, bytes4) {
-        return (s1, s2, s3, s4);
-    }
+    function f(bool condition) public view {
+        assert(librarySelector == L.E.selector);
+        assert(E.selector == B.E.selector);
 
-    function test3() pure external returns (bytes4) {
-        return (F.selector);
+        if (condition)
+            revert E(E.selector, true, "123");
+        else
+            revert L.E((B.E.selector), true, "123");
     }
 }
 // ----
-// Warning 2519: (16-26): This declaration shadows an existing declaration.
-// Warning 2519: (45-59): This declaration shadows an existing declaration.
-// Warning 2519: (78-88): This declaration shadows an existing declaration.
-// Warning 2519: (122-132): This declaration shadows an existing declaration.
+// Warning 2519: (16-45): This declaration shadows an existing declaration.
+// Warning 2519: (98-127): This declaration shadows an existing declaration.
+// Warning 2519: (148-177): This declaration shadows an existing declaration.
diff --git a/test/libsolidity/syntaxTests/events/event_selector_access_foreign_contract.sol b/test/libsolidity/syntaxTests/events/event_selector_access_foreign_contract.sol
@@ -0,0 +1,11 @@
+contract D {
+    event E();
+}
+
+contract C {
+    function f() external pure returns (bytes32) {
+        return D.E.selector;
+    }
+}
+// ----
+// TypeError 9582: (110-113): Member "E" not found or not visible after argument-dependent lookup in type(contract D).
diff --git a/test/libsolidity/syntaxTests/events/event_selector_access_interface.sol b/test/libsolidity/syntaxTests/events/event_selector_access_interface.sol
@@ -0,0 +1,11 @@
+interface I {
+    event E();
+}
+
+contract C {
+    function f() external pure returns (bytes32) {
+        return I.E.selector;
+    }
+}
+// ----
+// TypeError 9582: (111-114): Member "E" not found or not visible after argument-dependent lookup in type(contract I).
diff --git a/test/libsolidity/syntaxTests/events/event_selector_access_library.sol b/test/libsolidity/syntaxTests/events/event_selector_access_library.sol
@@ -0,0 +1,10 @@
+library L {
+    event E();
+}
+
+contract C {
+    function f() external pure returns (bytes32) {
+        return L.E.selector;
+    }
+}
+// ----
diff --git a/test/libsolidity/syntaxTests/events/event_selector_library_called_inside_function.sol b/test/libsolidity/syntaxTests/events/event_selector_library_called_inside_function.sol
@@ -6,7 +6,6 @@ contract D {
     function test1() external pure returns (bytes32) {
         return Y.E.selector;
     }
-
 }
 // ----
 // TypeError 9582: (123-135): Member "selector" not found or not visible after argument-dependent lookup in function ().
diff --git a/test/libsolidity/syntaxTests/events/event_selector_syntax.sol b/test/libsolidity/syntaxTests/events/event_selector_syntax.sol
@@ -1,43 +1,20 @@
 library L {
-    event E();
-}
-library S {
-    event E(uint);
-}
-library T {
-    event E();
-}
-interface I {
-    event E();
+    event E(bytes32, bool, bytes indexed);
 }
 
-contract D {
-    event F();
+contract B {
+    event E(bytes32, bool, bytes indexed);
 }
 
-contract C is D {
-    function test1() external pure returns (bytes32, bytes32, bytes32) {
-        assert(L.E.selector == T.E.selector);
-        assert(I.E.selector == L.E.selector);
-
-        assert(L.E.selector != S.E.selector);
-        assert(T.E.selector != S.E.selector);
-        assert(I.E.selector != S.E.selector);
+contract C is B {
+    bytes32 public librarySelector = L.E.selector;
+    bytes32 inheritedSelector = E.selector;
 
-        return (L.E.selector, S.E.selector, I.E.selector);
-    }
-
-    bytes32 s1 = L.E.selector;
-    bytes32 s2 = S.E.selector;
-    bytes32 s3 = T.E.selector;
-    bytes32 s4 = I.E.selector;
-
-    function test2() view external returns (bytes32, bytes32, bytes32, bytes32) {
-        return (s1, s2, s3, s4);
-    }
+    function f() public {
+        assert(librarySelector == L.E.selector);
+        assert(E.selector == B.E.selector);
 
-    function test3() pure external returns (bytes32) {
-        return (F.selector);
+        emit E(E.selector, true, "123");
+        emit L.E((B.E.selector), true, "123");
     }
 }
-// ----

Original file line number	Diff line number	Diff line change
`@@ -116,7 +116,7 @@ efficient code, for example:`
`116`	`116`	`}`
`117`	`117`	`}`
`118`	`118`
`119`		`-`
	`119`	`+.. index:: selector; of a function`
`120`	`120`
`121`	`121`	`Access to External Variables, Functions and Libraries`
`122`	`122`	`-----------------------------------------------------`