Merge pull request #3364 from ethereum/revertWithReason

Revert with reason

6 files changed, 76 insertions, 12 deletions

diff --git a/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp
index 47333046..a35eea73 100644
--- a/libsolidity/codegen/CompilerContext.cpp
+++ b/libsolidity/codegen/CompilerContext.cpp
@@ -262,12 +262,20 @@ CompilerContext& CompilerContext::appendRevert()
 	return *this << u256(0) << u256(0) << Instruction::REVERT;
 }
 
-CompilerContext& CompilerContext::appendConditionalRevert()
-{
-	*this << Instruction::ISZERO;
-	eth::AssemblyItem afterTag = appendConditionalJump();
-	appendRevert();
-	*this << afterTag;
+CompilerContext& CompilerContext::appendConditionalRevert(bool _forwardReturnData)
+{
+	if (_forwardReturnData && m_evmVersion.supportsReturndata())
+		appendInlineAssembly(R"({
+			if condition {
+				returndatacopy(0, 0, returndatasize())
+				revert(0, returndatasize())
+			}
+		})", {"condition"});
+	else
+		appendInlineAssembly(R"({
+			if condition { revert(0, 0) }
+		})", {"condition"});
+	*this << Instruction::POP;
 	return *this;
 }
 
diff --git a/libsolidity/codegen/CompilerContext.h b/libsolidity/codegen/CompilerContext.h
index 7b663277..098472f7 100644
--- a/libsolidity/codegen/CompilerContext.h
+++ b/libsolidity/codegen/CompilerContext.h
@@ -156,8 +156,11 @@ public:
 	CompilerContext& appendConditionalInvalid();
 	/// Appends a REVERT(0, 0) call
 	CompilerContext& appendRevert();
-	/// Appends a conditional REVERT(0, 0) call
-	CompilerContext& appendConditionalRevert();
+	/// Appends a conditional REVERT-call, either forwarding the RETURNDATA or providing the
+	/// empty string. Consumes the condition.
+	/// If the current EVM version does not support RETURNDATA, uses REVERT but does not forward
+	/// the data.
+	CompilerContext& appendConditionalRevert(bool _forwardReturnData = false);
 	/// Appends a JUMP to a specific tag
 	CompilerContext& appendJumpTo(eth::AssemblyItem const& _tag) { m_asm->appendJump(_tag); return *this; }
 	/// Appends pushing of a new tag and @returns the new tag.
diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp
index 79aef7b0..b4550153 100644
--- a/libsolidity/codegen/CompilerUtils.cpp
+++ b/libsolidity/codegen/CompilerUtils.cpp
@@ -78,6 +78,20 @@ void CompilerUtils::toSizeAfterFreeMemoryPointer()
 	m_context << Instruction::SWAP1;
 }
 
+void CompilerUtils::revertWithStringData(Type const& _argumentType)
+{
+	solAssert(_argumentType.isImplicitlyConvertibleTo(*Type::fromElementaryTypeName("string memory")), "");
+	fetchFreeMemoryPointer();
+	m_context << (u256(FixedHash<4>::Arith(FixedHash<4>(dev::keccak256("Error(string)")))) << (256 - 32));
+	m_context << Instruction::DUP2 << Instruction::MSTORE;
+	m_context << u256(4) << Instruction::ADD;
+	// Stack: <string data> <mem pos of encoding start>
+	abiEncode({_argumentType.shared_from_this()}, {make_shared<ArrayType>(DataLocation::Memory, true)});
+	toSizeAfterFreeMemoryPointer();
+	m_context << Instruction::REVERT;
+	m_context.adjustStackOffset(_argumentType.sizeOnStack());
+}
+
 unsigned CompilerUtils::loadFromMemory(
 	unsigned _offset,
 	Type const& _type,
@@ -691,6 +705,7 @@ void CompilerUtils::convertType(
 			solAssert(enumType.numberOfMembers() > 0, "empty enum should have caused a parser error.");
 			m_context << u256(enumType.numberOfMembers() - 1) << Instruction::DUP2 << Instruction::GT;
 			if (_asPartOfArgumentDecoding)
+				// TODO: error message?
 				m_context.appendConditionalRevert();
 			else
 				m_context.appendConditionalInvalid();
diff --git a/libsolidity/codegen/CompilerUtils.h b/libsolidity/codegen/CompilerUtils.h
index a32c5c6e..476a7559 100644
--- a/libsolidity/codegen/CompilerUtils.h
+++ b/libsolidity/codegen/CompilerUtils.h
@@ -54,6 +54,13 @@ public:
 	/// Stack post: <size> <mem_start>
 	void toSizeAfterFreeMemoryPointer();
 
+	/// Appends code that performs a revert, providing the given string data.
+	/// Will also append an error signature corresponding to Error(string).
+	/// @param _argumentType the type of the string argument, will be converted to memory string.
+	/// Stack pre: string data
+	/// Stack post:
+	void revertWithStringData(Type const& _argumentType);
+
 	/// Loads data from memory to the stack.
 	/// @param _offset offset in memory (or calldata)
 	/// @param _type data type to load
diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp
index 5cb37103..0889ac7c 100644
--- a/libsolidity/codegen/ContractCompiler.cpp
+++ b/libsolidity/codegen/ContractCompiler.cpp
@@ -128,6 +128,7 @@ void ContractCompiler::appendCallValueCheck()
 {
 	// Throw if function is not payable but call contained ether.
 	m_context << Instruction::CALLVALUE;
+	// TODO: error message?
 	m_context.appendConditionalRevert();
 }
 
@@ -327,6 +328,7 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac
 		m_context << Instruction::STOP;
 	}
 	else
+		// TODO: error message here?
 		m_context.appendRevert();
 
 	for (auto const& it: interfaceFunctions)
diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp
index 051db536..ed5af42e 100644
--- a/libsolidity/codegen/ExpressionCompiler.cpp
+++ b/libsolidity/codegen/ExpressionCompiler.cpp
@@ -610,7 +610,8 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 			m_context << Instruction::CREATE;
 			// Check if zero (out of stack or not enough balance).
 			m_context << Instruction::DUP1 << Instruction::ISZERO;
-			m_context.appendConditionalRevert();
+			// TODO: Can we bubble up here? There might be different reasons for failure, I think.
+			m_context.appendConditionalRevert(true);
 			if (function.valueSet())
 				m_context << swapInstruction(1) << Instruction::POP;
 			break;
@@ -672,8 +673,9 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 			if (function.kind() == FunctionType::Kind::Transfer)
 			{
 				// Check if zero (out of stack or not enough balance).
+				// TODO: bubble up here, but might also be different error.
 				m_context << Instruction::ISZERO;
-				m_context.appendConditionalRevert();
+				m_context.appendConditionalRevert(true);
 			}
 			break;
 		case FunctionType::Kind::Selfdestruct:
@@ -682,8 +684,19 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 			m_context << Instruction::SELFDESTRUCT;
 			break;
 		case FunctionType::Kind::Revert:
-			m_context.appendRevert();
+		{
+			if (!arguments.empty())
+			{
+				// function-sel(Error(string)) + encoding
+				solAssert(arguments.size() == 1, "");
+				solAssert(function.parameterTypes().size() == 1, "");
+				arguments.front()->accept(*this);
+				utils().revertWithStringData(*arguments.front()->annotation().type);
+			}
+			else
+				m_context.appendRevert();
 			break;
+		}
 		case FunctionType::Kind::SHA3:
 		{
 			TypePointers argumentTypes;
@@ -902,16 +915,31 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 		{
 			arguments.front()->accept(*this);
 			utils().convertType(*arguments.front()->annotation().type, *function.parameterTypes().front(), false);
+			if (arguments.size() > 1)
+			{
+				// Users probably expect the second argument to be evaluated
+				// even if the condition is false, as would be the case for an actual
+				// function call.
+				solAssert(arguments.size() == 2, "");
+				solAssert(function.kind() == FunctionType::Kind::Require, "");
+				arguments.at(1)->accept(*this);
+				utils().moveIntoStack(1, arguments.at(1)->annotation().type->sizeOnStack());
+			}
+			// Stack: <error string (unconverted)> <condition>
 			// jump if condition was met
 			m_context << Instruction::ISZERO << Instruction::ISZERO;
 			auto success = m_context.appendConditionalJump();
 			if (function.kind() == FunctionType::Kind::Assert)
 				// condition was not met, flag an error
 				m_context.appendInvalid();
+			else if (arguments.size() > 1)
+				utils().revertWithStringData(*arguments.at(1)->annotation().type);
 			else
 				m_context.appendRevert();
 			// the success branch
 			m_context << success;
+			if (arguments.size() > 1)
+				utils().popStackElement(*arguments.at(1)->annotation().type);
 			break;
 		}
 		case FunctionType::Kind::ABIEncode:
@@ -1882,6 +1910,7 @@ void ExpressionCompiler::appendExternalFunctionCall(
 	if (funKind == FunctionType::Kind::External || funKind == FunctionType::Kind::CallCode || funKind == FunctionType::Kind::DelegateCall)
 	{
 		m_context << Instruction::DUP1 << Instruction::EXTCODESIZE << Instruction::ISZERO;
+		// TODO: error message?
 		m_context.appendConditionalRevert();
 		existenceChecked = true;
 	}
@@ -1924,7 +1953,7 @@ void ExpressionCompiler::appendExternalFunctionCall(
 	{
 		//Propagate error condition (if CALL pushes 0 on stack).
 		m_context << Instruction::ISZERO;
-		m_context.appendConditionalRevert();
+		m_context.appendConditionalRevert(true);
 	}
 
 	utils().popStackSlots(remainsSize);