Disallow modifier declarations and definitions in interfaces

Author: cameel

Changelog.md

@@ -13,9 +13,10 @@ Compiler Features:
 
 
 Bugfixes:
+ * SMTChecker: Fix false negative caused by ``push`` on storage array references returned by internal functions.
  * SMTChecker: Fix false positive in external calls from constructors.
  * SMTChecker: Fix internal error on some multi-source uses of ``abi.*``, cryptographic functions and constants.
- * SMTChecker: Fix false negative caused by ``push`` on storage array references returned by internal functions.
+ * Type Checker: Disallow modifier declarations and definitions in interfaces.
 
 
 

docs/contracts/interfaces.rst

@@ -6,12 +6,14 @@
 Interfaces
 **********
 
-Interfaces are similar to abstract contracts, but they cannot have any functions implemented. There are further restrictions:
+Interfaces are similar to abstract contracts, but they cannot have any functions implemented.
+There are further restrictions:
 
 - They cannot inherit from other contracts, but they can inherit from other interfaces.
 - All declared functions must be external.
 - They cannot declare a constructor.
 - They cannot declare state variables.
+- They cannot declare modifiers.
 
 Some of these restrictions might be lifted in the future.
 

libsolidity/analysis/TypeChecker.cpp

@@ -314,14 +314,22 @@ void TypeChecker::endVisit(InheritanceSpecifier const& _inheritance)
 
 void TypeChecker::endVisit(ModifierDefinition const& _modifier)
 {
-	if (_modifier.virtualSemantics())
-		if (auto const* contractDef = dynamic_cast<ContractDefinition const*>(_modifier.scope()))
-			if (contractDef->isLibrary())
-				m_errorReporter.typeError(
-					3275_error,
-					_modifier.location(),
-					"Modifiers in a library cannot be virtual."
-				);
+	if (auto const* contractDef = dynamic_cast<ContractDefinition const*>(_modifier.scope()))
+	{
+		if (_modifier.virtualSemantics() && contractDef->isLibrary())
+			m_errorReporter.typeError(
+				3275_error,
+				_modifier.location(),
+				"Modifiers in a library cannot be virtual."
+			);
+
+		if (contractDef->isInterface())
+			m_errorReporter.typeError(
+				6408_error,
+				_modifier.location(),
+				"Modifiers cannot be defined or declared in interfaces."
+			);
+	}
 
 	if (!_modifier.isImplemented() && !_modifier.virtualSemantics())
 		m_errorReporter.typeError(8063_error, _modifier.location(), "Modifiers without implementation must be marked virtual.");

test/libsolidity/analysis/FunctionCallGraph.cpp

@@ -1114,7 +1114,6 @@ BOOST_AUTO_TEST_CASE(interfaces_and_abstract_contracts)
 	unique_ptr<CompilerStack> compilerStack = parseAndAnalyzeContracts(R"(
 		interface I {
 			event Ev(uint);
-			modifier m() virtual;
 
 			function ext1() external;
 			function ext2() external;
@@ -1126,6 +1125,8 @@ BOOST_AUTO_TEST_CASE(interfaces_and_abstract_contracts)
 		}
 
 		abstract contract C is J {
+			modifier m() virtual;
+
 			function ext3() external override virtual;
 			function ext4() external { inr2();}
 			function inr1() internal virtual;
@@ -1167,7 +1168,7 @@ BOOST_AUTO_TEST_CASE(interfaces_and_abstract_contracts)
 			{"Entry", "function C.ext4()"},
 			{"function C.ext4()", "function C.inr2()"},
 			{"function C.inr2()", "function C.inr1()"},
-			{"function C.inr2()", "modifier I.m"},
+			{"function C.inr2()", "modifier C.m"},
 		}},
 		{"D", {
 			{"Entry", "function D.ext1()"},

test/libsolidity/syntaxTests/modifiers/definition_in_contract.sol

@@ -0,0 +1,11 @@
+contract C {
+    modifier m { _; }
+    modifier mv virtual { _; }
+}
+
+abstract contract A {
+    modifier m { _; }
+    modifier mv virtual { _; }
+    modifier muv virtual;
+}
+// ----

test/libsolidity/syntaxTests/modifiers/definition_in_contract_unimplemented.sol

@@ -0,0 +1,7 @@
+contract C {
+    modifier mu;
+    modifier muv virtual;
+}
+// ----
+// TypeError 3656: (0-57): Contract "C" should be marked as abstract.
+// TypeError 8063: (17-29): Modifiers without implementation must be marked virtual.

test/libsolidity/syntaxTests/modifiers/definition_in_interface.sol

@@ -0,0 +1,12 @@
+interface I {
+    modifier m { _; }
+    modifier mu;
+    modifier mv virtual { _; }
+    modifier muv virtual;
+}
+// ----
+// TypeError 6408: (18-35): Modifiers cannot be defined or declared in interfaces.
+// TypeError 6408: (40-52): Modifiers cannot be defined or declared in interfaces.
+// TypeError 8063: (40-52): Modifiers without implementation must be marked virtual.
+// TypeError 6408: (57-83): Modifiers cannot be defined or declared in interfaces.
+// TypeError 6408: (88-109): Modifiers cannot be defined or declared in interfaces.

test/libsolidity/syntaxTests/modifiers/definition_in_library.sol

@@ -0,0 +1,5 @@
+library L {
+    modifier mv virtual { _; }
+}
+// ----
+// TypeError 3275: (16-42): Modifiers in a library cannot be virtual.

test/libsolidity/syntaxTests/modifiers/definition_in_library_unimplemented.sol

@@ -0,0 +1,7 @@
+library L {
+    modifier mu;
+    modifier muv virtual;
+}
+// ----
+// TypeError 8063: (16-28): Modifiers without implementation must be marked virtual.
+// TypeError 3275: (33-54): Modifiers in a library cannot be virtual.

test/libsolidity/syntaxTests/modifiers/definition_in_library_virtual.sol

@@ -0,0 +1,4 @@
+library L {
+    modifier m { _; }
+}
+// ----