Merge pull request #15058 from ethereum/refactor-require-or-assert-fu…

…nction Refactor requireOrAssert to use revertWithError
ethereum · Apr 26, 2024 · 9e4d260 · 9e4d260
2 parents 938c505 + c5bfea2
commit 9e4d260
Show file tree

Hide file tree

Showing 7 changed files with 324 additions and 32 deletions.
diff --git a/libsolidity/codegen/YulUtilFunctions.cpp b/libsolidity/codegen/YulUtilFunctions.cpp
@@ -252,7 +252,7 @@ std::string YulUtilFunctions::revertWithError(
 	return templ.render();
 }
 
-std::string YulUtilFunctions::requireOrAssertFunction(bool _assert, Type const* _messageType)
+std::string YulUtilFunctions::requireOrAssertFunction(bool _assert, Type const* _messageType, ASTPointer<Expression const> _stringArgumentExpression)
 {
 	std::string functionName =
 		std::string(_assert ? "assert_helper" : "require_helper") +
@@ -271,34 +271,19 @@ std::string YulUtilFunctions::requireOrAssertFunction(bool _assert, Type const*
 			("functionName", functionName)
 			.render();
 
-		int const hashHeaderSize = 4;
-		u256 const errorHash = util::selectorFromSignatureU256("Error(string)");
-
-		std::string const encodeFunc = ABIFunctions(m_evmVersion, m_revertStrings, m_functionCollector)
-			.tupleEncoder(
-				{_messageType},
-				{TypeProvider::stringMemory()}
-			);
+		solAssert(_stringArgumentExpression, "Require with string must have a string argument");
+		solAssert(_stringArgumentExpression->annotation().type);
+		std::vector<std::string> functionParameterNames = IRVariable(*_stringArgumentExpression).stackSlots();
 
 		return Whiskers(R"(
-			function <functionName>(condition <messageVars>) {
-				if iszero(condition) {
-					let memPtr := <allocateUnbounded>()
-					mstore(memPtr, <errorHash>)
-					let end := <abiEncodeFunc>(add(memPtr, <hashHeaderSize>) <messageVars>)
-					revert(memPtr, sub(end, memPtr))
-				}
+			function <functionName>(condition <functionParameterNames>) {
+				if iszero(condition)
+					<revertWithError>
 			}
 		)")
 		("functionName", functionName)
-		("allocateUnbounded", allocateUnboundedFunction())
-		("errorHash", formatNumber(errorHash))
-		("abiEncodeFunc", encodeFunc)
-		("hashHeaderSize", std::to_string(hashHeaderSize))
-		("messageVars",
-			(_messageType->sizeOnStack() > 0 ? ", " : "") +
-			suffixedVariableNameList("message_", 1, 1 + _messageType->sizeOnStack())
-		)
+		("revertWithError", revertWithError("Error(string)", {_messageType}, {_stringArgumentExpression}))
+		("functionParameterNames", joinHumanReadablePrefixed(functionParameterNames))
 		.render();
 	});
 }

diff --git a/libsolidity/codegen/YulUtilFunctions.h b/libsolidity/codegen/YulUtilFunctions.h
@@ -104,7 +104,11 @@ class YulUtilFunctions
 
 	// @returns the name of a function that has the equivalent logic of an
 	// `assert` or `require` call.
-	std::string requireOrAssertFunction(bool _assert, Type const* _messageType = nullptr);
+	std::string requireOrAssertFunction(
+		bool _assert,
+		Type const* _messageType = nullptr,
+		ASTPointer<Expression const> _stringArgumentExpression = nullptr
+	);
 
 	// @returns function that has equivalent logic of a require function, but with a custom
 	// error constructor parameter.

diff --git a/libsolidity/codegen/ir/IRGeneratorForStatements.cpp b/libsolidity/codegen/ir/IRGeneratorForStatements.cpp
@@ -1166,9 +1166,11 @@ void IRGeneratorForStatements::endVisit(FunctionCall const& _functionCall)
 		}
 		else
 		{
+			ASTPointer<Expression const> stringArgumentExpression = messageArgumentType ? arguments[1] : nullptr;
 			std::string requireOrAssertFunction = m_utils.requireOrAssertFunction(
 				functionType->kind() == FunctionType::Kind::Assert,
-				messageArgumentType
+				messageArgumentType,
+				stringArgumentExpression
 			);
 			appendCode() << std::move(requireOrAssertFunction) << "(" << IRVariable(*arguments[0]).name();
 			if (messageArgumentType && messageArgumentType->sizeOnStack() > 0)

diff --git a/test/cmdlineTests/require_with_error_ir/input.sol b/test/cmdlineTests/require_with_error_ir/input.sol
@@ -3,15 +3,12 @@ pragma solidity >=0.0;
 
 error CustomError(uint256, string);
 
-contract C
-{
-    function f(bool condition) external pure
-    {
+contract C {
+    function f(bool condition) external pure {
         require(condition, CustomError(1, "two"));
     }
 
-    function g(bool condition) external pure
-    {
+    function g(bool condition) external pure {
         require(condition, CustomError(2, "three"));
     }
 }
diff --git a/test/cmdlineTests/require_with_string_ir/args b/test/cmdlineTests/require_with_string_ir/args
@@ -0,0 +1 @@
+--ir --debug-info none
diff --git a/test/cmdlineTests/require_with_string_ir/input.sol b/test/cmdlineTests/require_with_string_ir/input.sol
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-3.0
+pragma solidity >=0.0;
+
+contract C {
+    function f(bool condition) external pure {
+        string memory message = "Condition must be satisfied";
+        require(condition, message);
+    }
+
+    function g(bool condition) external pure {
+        require(condition, "Condition must be satisfied");
+    }
+}