From 17fc0f54b5dbf6b49e0500e3fca7b681316071f6 Mon Sep 17 00:00:00 2001 From: chriseth Date: Sat, 30 Dec 2017 13:44:09 +0100 Subject: [PATCH 01/19] Use FunctionTypePointer (adds ``const``). --- libsolidity/ast/AST.cpp | 7 ++++--- libsolidity/ast/AST.h | 8 ++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/libsolidity/ast/AST.cpp b/libsolidity/ast/AST.cpp index d8ad009d15c8..80f5d642454c 100644 --- a/libsolidity/ast/AST.cpp +++ b/libsolidity/ast/AST.cpp @@ -297,7 +297,7 @@ ContractDefinition::ContractKind FunctionDefinition::inContractKind() const return contractDef->contractKind(); } -shared_ptr FunctionDefinition::functionType(bool _internal) const +FunctionTypePointer FunctionDefinition::functionType(bool _internal) const { if (_internal) { @@ -338,6 +338,7 @@ shared_ptr FunctionDefinition::functionType(bool _internal) const TypePointer FunctionDefinition::type() const { + solAssert(visibility() != Declaration::Visibility::External, ""); return make_shared(*this); } @@ -379,7 +380,7 @@ TypePointer EventDefinition::type() const return make_shared(*this); } -std::shared_ptr EventDefinition::functionType(bool _internal) const +FunctionTypePointer EventDefinition::functionType(bool _internal) const { if (_internal) return make_shared(*this); @@ -484,7 +485,7 @@ TypePointer VariableDeclaration::type() const return annotation().type; } -shared_ptr VariableDeclaration::functionType(bool _internal) const +FunctionTypePointer VariableDeclaration::functionType(bool _internal) const { if (_internal) return {}; diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h index ae253f0c8436..b648e08b2bf2 100644 --- a/libsolidity/ast/AST.h +++ b/libsolidity/ast/AST.h @@ -218,7 +218,7 @@ class Declaration: public ASTNode, public Scopable /// @param _internal false indicates external interface is concerned, true indicates internal interface is concerned. /// @returns null when it is not accessible as a function. - virtual std::shared_ptr functionType(bool /*_internal*/) const { return {}; } + virtual FunctionTypePointer functionType(bool /*_internal*/) const { return {}; } protected: virtual Visibility defaultVisibility() const { return Visibility::Public; } @@ -634,7 +634,7 @@ class FunctionDefinition: public CallableDeclaration, public Documented, public /// @param _internal false indicates external interface is concerned, true indicates internal interface is concerned. /// @returns null when it is not accessible as a function. - virtual std::shared_ptr functionType(bool /*_internal*/) const override; + virtual FunctionTypePointer functionType(bool /*_internal*/) const override; virtual FunctionDefinitionAnnotation& annotation() const override; @@ -703,7 +703,7 @@ class VariableDeclaration: public Declaration /// @param _internal false indicates external interface is concerned, true indicates internal interface is concerned. /// @returns null when it is not accessible as a function. - virtual std::shared_ptr functionType(bool /*_internal*/) const override; + virtual FunctionTypePointer functionType(bool /*_internal*/) const override; virtual VariableDeclarationAnnotation& annotation() const override; @@ -805,7 +805,7 @@ class EventDefinition: public CallableDeclaration, public Documented bool isAnonymous() const { return m_anonymous; } virtual TypePointer type() const override; - virtual std::shared_ptr functionType(bool /*_internal*/) const override; + virtual FunctionTypePointer functionType(bool /*_internal*/) const override; virtual EventDefinitionAnnotation& annotation() const override; From f00bb4359375cd03e9ff6373b5efa41a37ce2876 Mon Sep 17 00:00:00 2001 From: chriseth Date: Sat, 30 Dec 2017 13:46:53 +0100 Subject: [PATCH 02/19] Allow function overloads involving MagicVariableDeclarations. --- libsolidity/analysis/DeclarationContainer.cpp | 8 +++++++- libsolidity/analysis/NameAndTypeResolver.cpp | 5 +++-- libsolidity/analysis/TypeChecker.cpp | 7 +++---- libsolidity/ast/AST.h | 5 +++++ 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/libsolidity/analysis/DeclarationContainer.cpp b/libsolidity/analysis/DeclarationContainer.cpp index c7ba78d60947..786272e4505b 100644 --- a/libsolidity/analysis/DeclarationContainer.cpp +++ b/libsolidity/analysis/DeclarationContainer.cpp @@ -45,7 +45,8 @@ Declaration const* DeclarationContainer::conflictingDeclaration( if ( dynamic_cast(&_declaration) || - dynamic_cast(&_declaration) + dynamic_cast(&_declaration) || + dynamic_cast(&_declaration) ) { // check that all other declarations with the same name are functions or a public state variable or events. @@ -68,6 +69,11 @@ Declaration const* DeclarationContainer::conflictingDeclaration( !dynamic_cast(declaration) ) return declaration; + if ( + dynamic_cast(&_declaration) && + !dynamic_cast(declaration) + ) + return declaration; // Or, continue. } } diff --git a/libsolidity/analysis/NameAndTypeResolver.cpp b/libsolidity/analysis/NameAndTypeResolver.cpp index 2f6751351aef..5f9e0e995ce0 100644 --- a/libsolidity/analysis/NameAndTypeResolver.cpp +++ b/libsolidity/analysis/NameAndTypeResolver.cpp @@ -202,8 +202,9 @@ vector NameAndTypeResolver::cleanedDeclarations( solAssert( dynamic_cast(declaration) || dynamic_cast(declaration) || - dynamic_cast(declaration), - "Found overloading involving something not a function or a variable." + dynamic_cast(declaration) || + dynamic_cast(declaration), + "Found overloading involving something not a function, event or a (magic) variable." ); FunctionTypePointer functionType { declaration->functionType(false) }; diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 8b57fc158bfa..4b1ef55b9dfd 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -2124,10 +2124,9 @@ bool TypeChecker::visit(Identifier const& _identifier) for (Declaration const* declaration: annotation.overloadedDeclarations) { - TypePointer function = declaration->type(); - solAssert(!!function, "Requested type not present."); - auto const* functionType = dynamic_cast(function.get()); - if (functionType && functionType->canTakeArguments(*annotation.argumentTypes)) + FunctionTypePointer functionType = declaration->functionType(true); + solAssert(!!functionType, "Requested type not present."); + if (functionType->canTakeArguments(*annotation.argumentTypes)) candidates.push_back(declaration); } if (candidates.empty()) diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h index b648e08b2bf2..a53987bf563e 100644 --- a/libsolidity/ast/AST.h +++ b/libsolidity/ast/AST.h @@ -831,6 +831,11 @@ class MagicVariableDeclaration: public Declaration solAssert(false, "MagicVariableDeclaration used inside real AST."); } + virtual FunctionTypePointer functionType(bool) const override + { + solAssert(m_type->category() == Type::Category::Function, ""); + return std::dynamic_pointer_cast(m_type); + } virtual TypePointer type() const override { return m_type; } private: From 8ab7dc036aafd5781118fc16f05df5b0a8a5550e Mon Sep 17 00:00:00 2001 From: chriseth Date: Sat, 30 Dec 2017 13:47:15 +0100 Subject: [PATCH 03/19] Register overload for ``revert()`` that can receive a reason string. --- libsolidity/analysis/GlobalContext.cpp | 1 + libsolidity/analysis/NameAndTypeResolver.cpp | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/libsolidity/analysis/GlobalContext.cpp b/libsolidity/analysis/GlobalContext.cpp index 6a858d36db33..822674aff3ed 100644 --- a/libsolidity/analysis/GlobalContext.cpp +++ b/libsolidity/analysis/GlobalContext.cpp @@ -52,6 +52,7 @@ m_magicVariables(vector>{ make_shared("now", make_shared(256)), make_shared("require", make_shared(strings{"bool"}, strings{}, FunctionType::Kind::Require, false, StateMutability::Pure)), make_shared("revert", make_shared(strings(), strings(), FunctionType::Kind::Revert, false, StateMutability::Pure)), + make_shared("revert", make_shared(strings{"string memory"}, strings(), FunctionType::Kind::Revert, false, StateMutability::Pure)), make_shared("ripemd160", make_shared(strings(), strings{"bytes20"}, FunctionType::Kind::RIPEMD160, true, StateMutability::Pure)), make_shared("selfdestruct", make_shared(strings{"address"}, strings{}, FunctionType::Kind::Selfdestruct)), make_shared("sha256", make_shared(strings(), strings{"bytes32"}, FunctionType::Kind::SHA256, true, StateMutability::Pure)), diff --git a/libsolidity/analysis/NameAndTypeResolver.cpp b/libsolidity/analysis/NameAndTypeResolver.cpp index 5f9e0e995ce0..0a356f04e8a7 100644 --- a/libsolidity/analysis/NameAndTypeResolver.cpp +++ b/libsolidity/analysis/NameAndTypeResolver.cpp @@ -47,7 +47,9 @@ NameAndTypeResolver::NameAndTypeResolver( if (!m_scopes[nullptr]) m_scopes[nullptr].reset(new DeclarationContainer()); for (Declaration const* declaration: _globals) - m_scopes[nullptr]->registerDeclaration(*declaration); + { + solAssert(m_scopes[nullptr]->registerDeclaration(*declaration), "Unable to register global declaration."); + } } bool NameAndTypeResolver::registerDeclarations(SourceUnit& _sourceUnit, ASTNode const* _currentScope) From 012ab37fe3984a692dcdda6ef516e4588a8721b3 Mon Sep 17 00:00:00 2001 From: chriseth Date: Sat, 30 Dec 2017 13:47:51 +0100 Subject: [PATCH 04/19] Code generator for revert with reason string. --- libsolidity/codegen/ExpressionCompiler.cpp | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index 57d49ac66ad4..dc9fae21a644 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -680,8 +680,25 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) m_context << Instruction::SELFDESTRUCT; break; case FunctionType::Kind::Revert: - m_context.appendRevert(); + { + if (!arguments.empty()) + { + solAssert(arguments.size() == 1, ""); + solAssert(function.parameterTypes().size() == 1, ""); + m_context << u256(0); + arguments.front()->accept(*this); + utils().fetchFreeMemoryPointer(); + utils().abiEncode( + {make_shared(256), arguments.front()->annotation().type}, + {make_shared(256), make_shared(DataLocation::Memory, true)} + ); + utils().toSizeAfterFreeMemoryPointer(); + m_context << Instruction::REVERT; + } + else + m_context.appendRevert(); break; + } case FunctionType::Kind::SHA3: { TypePointers argumentTypes; From a06249c98484f8ca3772b09f66bc68dd30c13c4d Mon Sep 17 00:00:00 2001 From: chriseth Date: Sat, 30 Dec 2017 13:48:07 +0100 Subject: [PATCH 05/19] Tests for revert with reason string. --- test/libsolidity/SolidityEndToEndTest.cpp | 37 +++++++++++++++++++ .../SolidityNameAndTypeResolution.cpp | 17 ++++++++- 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index f7f1062d1745..82ad2917dcde 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -10419,6 +10419,43 @@ BOOST_AUTO_TEST_CASE(revert) ABI_CHECK(callContractFunction("a()"), encodeArgs(u256(42))); } +BOOST_AUTO_TEST_CASE(revert_with_cause) +{ + char const* sourceCode = R"( + contract D { + function f() public { + revert("test123"); + } + function g() public { + revert("test1234567890123456789012345678901234567890"); + } + } + contract C { + D d = new D(); + function forward(address target, bytes data) internal returns (bool success, bytes retval) { + uint retsize; + assembly { + success := call(not(0), target, 0, add(data, 0x20), mload(data), 0, 0) + retsize := returndatasize() + } + retval = new bytes(retsize); + assembly { + returndatacopy(add(retval, 0x20), 0, returndatasize()) + } + } + function f() public returns (bool, bytes) { + return forward(address(d), msg.data); + } + function g() public returns (bool, bytes) { + return forward(address(d), msg.data); + } + } + )"; + compileAndRun(sourceCode, 0, "C"); + ABI_CHECK(callContractFunction("f()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 7, "test123")); + ABI_CHECK(callContractFunction("g()"), encodeArgs(0, 0x40, 0xa0, 0, 0x40, 44, "test1234567890123456789012345678901234567890")); +} + BOOST_AUTO_TEST_CASE(negative_stack_height) { // This code was causing negative stack height during code generation diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 6b6c86a1eaa9..2bf718867f6f 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -5987,7 +5987,22 @@ BOOST_AUTO_TEST_CASE(bare_revert) } } )"; - CHECK_WARNING(text, "Statement has no effect."); + CHECK_ERROR(text, TypeError, "No matching declaration found"); +} + +BOOST_AUTO_TEST_CASE(revert_with_reason) +{ + char const* text = R"( + contract C { + function f(uint x) pure public { + if (x > 7) + revert("abc"); + else + revert(); + } + } + )"; + CHECK_SUCCESS_NO_WARNINGS(text); } BOOST_AUTO_TEST_CASE(bare_others) From 3da16b3e8af41a1d743a94d2e19822e82440a63d Mon Sep 17 00:00:00 2001 From: chriseth Date: Sat, 30 Dec 2017 13:46:02 +0100 Subject: [PATCH 06/19] Documentation for revert with reason string. --- Changelog.md | 1 + docs/control-structures.rst | 26 ++++++++++++++++++++++++-- docs/miscellaneous.rst | 1 + docs/units-and-global-variables.rst | 2 ++ 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/Changelog.md b/Changelog.md index 6288e8488969..60f28db4010c 100644 --- a/Changelog.md +++ b/Changelog.md @@ -5,6 +5,7 @@ Features: * Code Generator: More specialized and thus optimized implementation for ``x.push(...)`` * Commandline interface: Error when missing or inaccessible file detected. Suppress it with the ``--ignore-missing`` flag. * Constant Evaluator: Fix evaluation of single element tuples. + * General: Allow providing reason string for ``revert()``. * General: Limit the number of errors output in a single run to 256. * General: Support accessing dynamic return data in post-byzantium EVMs. * Interfaces: Allow overriding external functions in interfaces with public in an implementing contract. diff --git a/docs/control-structures.rst b/docs/control-structures.rst index 40070a20771e..18a02572529f 100644 --- a/docs/control-structures.rst +++ b/docs/control-structures.rst @@ -455,8 +455,9 @@ The ``require`` function should be used to ensure valid conditions, such as inpu If used properly, analysis tools can evaluate your contract to identify the conditions and function calls which will reach a failing ``assert``. Properly functioning code should never reach a failing assert statement; if this happens there is a bug in your contract which you should fix. There are two other ways to trigger exceptions: The ``revert`` function can be used to flag an error and -revert the current call. In the future it might be possible to also include details about the error -in a call to ``revert``. The ``throw`` keyword can also be used as an alternative to ``revert()``. +revert the current call. It is possible to provide a string message containing details about the error +that will be passed back to the caller. +The ``throw`` keyword can also be used as an alternative to ``revert()``, but is deprecated. .. note:: From version 0.4.13 the ``throw`` keyword is deprecated and will be phased out in the future. @@ -515,3 +516,24 @@ the EVM to revert all changes made to the state. The reason for reverting is tha did not occur. Because we want to retain the atomicity of transactions, the safest thing to do is to revert all changes and make the whole transaction (or at least call) without effect. Note that ``assert``-style exceptions consume all gas available to the call, while ``require``-style exceptions will not consume any gas starting from the Metropolis release. + +The following example shows how an error string can be used together with revert: + +:: + + pragma solidity ^0.4.0; + + contract VendingMachine { + function buy(uint amount) payable { + if (amount > msg.value / 2 ether) + revert("Not enough Ether provided."); + // Perform the purchase. + } + } + +The provided string will be abi-encoded together with a uint value that will always be zero. +This means that if a string ``x`` is provided, ``(0, x)`` will be encoded as if a function with arguments +``(uint256, string)`` would be called. This zero is used as a version identifier. Future extensions +of this feature might provide actual exception payload of dynamic type and this number could be used +to encode the type or also as a simple numeric error code. Until this is specified, a zero will +be used in all cases. \ No newline at end of file diff --git a/docs/miscellaneous.rst b/docs/miscellaneous.rst index 20400aa26a59..c5178b5119f1 100644 --- a/docs/miscellaneous.rst +++ b/docs/miscellaneous.rst @@ -334,6 +334,7 @@ Global Variables - ``assert(bool condition)``: abort execution and revert state changes if condition is ``false`` (use for internal error) - ``require(bool condition)``: abort execution and revert state changes if condition is ``false`` (use for malformed input or error in external component) - ``revert()``: abort execution and revert state changes +- ``revert(string)``: abort execution and revert state changes providing an explanatory string - ``blockhash(uint blockNumber) returns (bytes32)``: hash of the given block - only works for 256 most recent blocks - ``keccak256(...) returns (bytes32)``: compute the Ethereum-SHA-3 (Keccak-256) hash of the :ref:`(tightly packed) arguments ` - ``sha3(...) returns (bytes32)``: an alias to ``keccak256`` diff --git a/docs/units-and-global-variables.rst b/docs/units-and-global-variables.rst index e7f41ed15f66..aad00ba2db40 100644 --- a/docs/units-and-global-variables.rst +++ b/docs/units-and-global-variables.rst @@ -101,6 +101,8 @@ Error Handling throws if the condition is not met - to be used for errors in inputs or external components. ``revert()``: abort execution and revert state changes +``revert(string reason)``: + abort execution and revert state changes, providing an explanatory string .. index:: keccak256, ripemd160, sha256, ecrecover, addmod, mulmod, cryptography, From ae1d040285d97c2be0eb9d3e94a983975459f879 Mon Sep 17 00:00:00 2001 From: chriseth Date: Sat, 30 Dec 2017 14:35:45 +0100 Subject: [PATCH 07/19] Allow error string for ``require``. --- libsolidity/analysis/GlobalContext.cpp | 1 + libsolidity/codegen/ExpressionCompiler.cpp | 26 ++++++++++ test/libsolidity/SolidityEndToEndTest.cpp | 56 ++++++++++++++++++++++ 3 files changed, 83 insertions(+) diff --git a/libsolidity/analysis/GlobalContext.cpp b/libsolidity/analysis/GlobalContext.cpp index 822674aff3ed..c58d99fb37ce 100644 --- a/libsolidity/analysis/GlobalContext.cpp +++ b/libsolidity/analysis/GlobalContext.cpp @@ -51,6 +51,7 @@ m_magicVariables(vector>{ make_shared("mulmod", make_shared(strings{"uint256", "uint256", "uint256"}, strings{"uint256"}, FunctionType::Kind::MulMod, false, StateMutability::Pure)), make_shared("now", make_shared(256)), make_shared("require", make_shared(strings{"bool"}, strings{}, FunctionType::Kind::Require, false, StateMutability::Pure)), + make_shared("require", make_shared(strings{"bool", "string memory"}, strings{}, FunctionType::Kind::Require, false, StateMutability::Pure)), make_shared("revert", make_shared(strings(), strings(), FunctionType::Kind::Revert, false, StateMutability::Pure)), make_shared("revert", make_shared(strings{"string memory"}, strings(), FunctionType::Kind::Revert, false, StateMutability::Pure)), make_shared("ripemd160", make_shared(strings(), strings{"bytes20"}, FunctionType::Kind::RIPEMD160, true, StateMutability::Pure)), diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index dc9fae21a644..cb92b030b578 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -917,16 +917,42 @@ 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: // 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) + { + m_context << u256(0); + utils().moveIntoStack(arguments.at(1)->annotation().type->sizeOnStack(), 1); + utils().fetchFreeMemoryPointer(); + utils().abiEncode( + {make_shared(256), arguments.at(1)->annotation().type}, + {make_shared(256), make_shared(DataLocation::Memory, true)} + ); + utils().toSizeAfterFreeMemoryPointer(); + m_context << Instruction::REVERT; + m_context.adjustStackOffset(arguments.at(1)->annotation().type->sizeOnStack()); + } 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::GasLeft: diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 82ad2917dcde..b6b26f4961e6 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -10456,6 +10456,62 @@ BOOST_AUTO_TEST_CASE(revert_with_cause) ABI_CHECK(callContractFunction("g()"), encodeArgs(0, 0x40, 0xa0, 0, 0x40, 44, "test1234567890123456789012345678901234567890")); } +BOOST_AUTO_TEST_CASE(require_with_message) +{ + char const* sourceCode = R"( + contract D { + bool flag = false; + string storageError = "abc"; + function f(uint x) public { + require(x > 7, "failed"); + } + function g() public { + // As a side-effect of internalFun, the flag will be set to true + // (even if the condition is true), + // but it will only throw in the next evaluation. + bool flagCopy = flag; + require(flagCopy == false, internalFun()); + } + function internalFun() returns (string) { + flag = true; + return "only on second run"; + } + function h() public { + require(false, storageError); + } + } + contract C { + D d = new D(); + function forward(address target, bytes data) internal returns (bool success, bytes retval) { + uint retsize; + assembly { + success := call(not(0), target, 0, add(data, 0x20), mload(data), 0, 0) + retsize := returndatasize() + } + retval = new bytes(retsize); + assembly { + returndatacopy(add(retval, 0x20), 0, returndatasize()) + } + } + function f(uint x) public returns (bool, bytes) { + return forward(address(d), msg.data); + } + function g() public returns (bool, bytes) { + return forward(address(d), msg.data); + } + function h() public returns (bool, bytes) { + return forward(address(d), msg.data); + } + } + )"; + compileAndRun(sourceCode, 0, "C"); + ABI_CHECK(callContractFunction("f(uint256)", 8), encodeArgs(1, 0x40, 0)); + ABI_CHECK(callContractFunction("f(uint256)", 5), encodeArgs(0, 0x40, 0x80, 0, 0x40, 6, "failed")); + ABI_CHECK(callContractFunction("g()"), encodeArgs(1, 0x40, 0)); + ABI_CHECK(callContractFunction("g()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 18 , "only on second run")); + ABI_CHECK(callContractFunction("h()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 3, "abc")); +} + BOOST_AUTO_TEST_CASE(negative_stack_height) { // This code was causing negative stack height during code generation From 7a9ee69e986cf58c8b2a6cabec2c59b0eb2fbb57 Mon Sep 17 00:00:00 2001 From: chriseth Date: Sat, 30 Dec 2017 20:13:41 +0100 Subject: [PATCH 08/19] Bubble up error messages. --- libsolidity/codegen/CompilerContext.cpp | 20 ++++++++---- libsolidity/codegen/CompilerContext.h | 5 +-- libsolidity/codegen/CompilerUtils.cpp | 1 + libsolidity/codegen/ContractCompiler.cpp | 2 ++ libsolidity/codegen/ExpressionCompiler.cpp | 9 ++++-- test/libsolidity/SolidityEndToEndTest.cpp | 37 ++++++++++++++++++++++ 6 files changed, 63 insertions(+), 11 deletions(-) diff --git a/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp index 47333046673b..2cd256f3dd81 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) + 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 7b6632774f03..c6f2f3be5c27 100644 --- a/libsolidity/codegen/CompilerContext.h +++ b/libsolidity/codegen/CompilerContext.h @@ -156,8 +156,9 @@ class CompilerContext 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. + 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 79aef7b06ba5..34337d7d79b5 100644 --- a/libsolidity/codegen/CompilerUtils.cpp +++ b/libsolidity/codegen/CompilerUtils.cpp @@ -691,6 +691,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/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 5cb371035fd7..0889ac7ce375 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 cb92b030b578..3f521f2d6b68 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -608,7 +608,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; @@ -670,8 +671,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: @@ -1823,6 +1825,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; } @@ -1865,7 +1868,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); diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index b6b26f4961e6..41ebe4628ea0 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -10512,6 +10512,43 @@ BOOST_AUTO_TEST_CASE(require_with_message) ABI_CHECK(callContractFunction("h()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 3, "abc")); } +BOOST_AUTO_TEST_CASE(bubble_up_error_messages) +{ + char const* sourceCode = R"( + contract D { + function f() public { + revert("message"); + } + function g() public { + this.f(); + } + } + contract C { + D d = new D(); + function forward(address target, bytes data) internal returns (bool success, bytes retval) { + uint retsize; + assembly { + success := call(not(0), target, 0, add(data, 0x20), mload(data), 0, 0) + retsize := returndatasize() + } + retval = new bytes(retsize); + assembly { + returndatacopy(add(retval, 0x20), 0, returndatasize()) + } + } + function f() public returns (bool, bytes) { + return forward(address(d), msg.data); + } + function g() public returns (bool, bytes) { + return forward(address(d), msg.data); + } + } + )"; + compileAndRun(sourceCode, 0, "C"); + ABI_CHECK(callContractFunction("f()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 3, "message")); + ABI_CHECK(callContractFunction("g()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 3, "message")); +} + BOOST_AUTO_TEST_CASE(negative_stack_height) { // This code was causing negative stack height during code generation From aa715f8759934ac68b76a2bef84c460b68be636a Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 3 Jan 2018 15:07:25 +0100 Subject: [PATCH 09/19] Tests about error bubbling for create and transfer. --- test/libsolidity/SolidityEndToEndTest.cpp | 72 ++++++++++++++++++++++- 1 file changed, 70 insertions(+), 2 deletions(-) diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 41ebe4628ea0..c7d8f917522c 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -10545,8 +10545,76 @@ BOOST_AUTO_TEST_CASE(bubble_up_error_messages) } )"; compileAndRun(sourceCode, 0, "C"); - ABI_CHECK(callContractFunction("f()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 3, "message")); - ABI_CHECK(callContractFunction("g()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 3, "message")); + ABI_CHECK(callContractFunction("f()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 7, "message")); + ABI_CHECK(callContractFunction("g()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 7, "message")); +} + +BOOST_AUTO_TEST_CASE(bubble_up_error_messages_through_transfer) +{ + char const* sourceCode = R"( + contract D { + function() public payable { + revert("message"); + } + function f() public { + this.transfer(0); + } + } + contract C { + D d = new D(); + function forward(address target, bytes data) internal returns (bool success, bytes retval) { + uint retsize; + assembly { + success := call(not(0), target, 0, add(data, 0x20), mload(data), 0, 0) + retsize := returndatasize() + } + retval = new bytes(retsize); + assembly { + returndatacopy(add(retval, 0x20), 0, returndatasize()) + } + } + function f() public returns (bool, bytes) { + return forward(address(d), msg.data); + } + } + )"; + compileAndRun(sourceCode, 0, "C"); + ABI_CHECK(callContractFunction("f()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 7, "message")); +} + +BOOST_AUTO_TEST_CASE(bubble_up_error_messages_through_create) +{ + char const* sourceCode = R"( + contract E { + function E() { + revert("message"); + } + } + contract D { + function f() public { + var x = new E(); + } + } + contract C { + D d = new D(); + function forward(address target, bytes data) internal returns (bool success, bytes retval) { + uint retsize; + assembly { + success := call(not(0), target, 0, add(data, 0x20), mload(data), 0, 0) + retsize := returndatasize() + } + retval = new bytes(retsize); + assembly { + returndatacopy(add(retval, 0x20), 0, returndatasize()) + } + } + function f() public returns (bool, bytes) { + return forward(address(d), msg.data); + } + } + )"; + compileAndRun(sourceCode, 0, "C"); + ABI_CHECK(callContractFunction("f()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 7, "message")); } BOOST_AUTO_TEST_CASE(negative_stack_height) From 344a388d4461abd7369ea44b123f5afe549dc8f7 Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 3 Jan 2018 15:30:01 +0100 Subject: [PATCH 10/19] Update documentation. --- docs/common-patterns.rst | 20 +++++++-- docs/contracts.rst | 10 ++++- docs/control-structures.rst | 16 +++++-- docs/miscellaneous.rst | 3 +- docs/solidity-by-example.rst | 66 +++++++++++++++++++---------- docs/structure-of-a-contract.rst | 5 ++- docs/types.rst | 5 ++- docs/units-and-global-variables.rst | 2 + 8 files changed, 92 insertions(+), 35 deletions(-) diff --git a/docs/common-patterns.rst b/docs/common-patterns.rst index c62b5acabbf2..233bdc4e9731 100644 --- a/docs/common-patterns.rst +++ b/docs/common-patterns.rst @@ -147,7 +147,10 @@ restrictions highly readable. // a certain address. modifier onlyBy(address _account) { - require(msg.sender == _account); + require( + msg.sender == _account, + "Sender not authorized." + ); // Do not forget the "_;"! It will // be replaced by the actual function // body when the modifier is used. @@ -164,7 +167,10 @@ restrictions highly readable. } modifier onlyAfter(uint _time) { - require(now >= _time); + require( + now >= _time, + "Function called too early." + ); _; } @@ -186,7 +192,10 @@ restrictions highly readable. // This was dangerous before Solidity version 0.4.0, // where it was possible to skip the part after `_;`. modifier costs(uint _amount) { - require(msg.value >= _amount); + require( + msg.value >= _amount, + "Not enough Ether provided." + ); _; if (msg.value > _amount) msg.sender.send(msg.value - _amount); @@ -290,7 +299,10 @@ function finishes. uint public creationTime = now; modifier atStage(Stages _stage) { - require(stage == _stage); + require( + stage == _stage, + "Function cannot be called at this time." + ); _; } diff --git a/docs/contracts.rst b/docs/contracts.rst index 0dd9845c0555..0c697dd6b5cf 100644 --- a/docs/contracts.rst +++ b/docs/contracts.rst @@ -315,7 +315,10 @@ inheritable properties of contracts and may be overridden by derived contracts. // function is executed and otherwise, an exception is // thrown. modifier onlyOwner { - require(msg.sender == owner); + require( + msg.sender == owner, + "Only owner can call this function." + ); _; } } @@ -360,7 +363,10 @@ inheritable properties of contracts and may be overridden by derived contracts. contract Mutex { bool locked; modifier noReentrancy() { - require(!locked); + require( + !locked, + "Reentrant call." + ); locked = true; _; locked = false; diff --git a/docs/control-structures.rst b/docs/control-structures.rst index 18a02572529f..7e3027a0621d 100644 --- a/docs/control-structures.rst +++ b/docs/control-structures.rst @@ -472,13 +472,16 @@ of an exception instead of "bubbling up". Catching exceptions is not yet possible. In the following example, you can see how ``require`` can be used to easily check conditions on inputs -and how ``assert`` can be used for internal error checking:: +and how ``assert`` can be used for internal error checking. Note that you can optionally provide +a message string for require, but not for assert. + +:: pragma solidity ^0.4.0; contract Sharer { function sendHalf(address addr) public payable returns (uint balance) { - require(msg.value % 2 == 0); // Only allow even numbers + require(msg.value % 2 == 0, "Even value required."); uint balanceBeforeTransfer = this.balance; addr.transfer(msg.value / 2); // Since transfer throws an exception on failure and @@ -517,7 +520,7 @@ did not occur. Because we want to retain the atomicity of transactions, the safe (or at least call) without effect. Note that ``assert``-style exceptions consume all gas available to the call, while ``require``-style exceptions will not consume any gas starting from the Metropolis release. -The following example shows how an error string can be used together with revert: +The following example shows how an error string can be used together with revert and require: :: @@ -527,13 +530,18 @@ The following example shows how an error string can be used together with revert function buy(uint amount) payable { if (amount > msg.value / 2 ether) revert("Not enough Ether provided."); + // Alternative way to do it: + require( + amount <= msg.value / 2 ether, + "Not enough Ether provided." + ); // Perform the purchase. } } The provided string will be abi-encoded together with a uint value that will always be zero. This means that if a string ``x`` is provided, ``(0, x)`` will be encoded as if a function with arguments -``(uint256, string)`` would be called. This zero is used as a version identifier. Future extensions +``(uint256, string)`` was called. This zero is used as a version identifier. Future extensions of this feature might provide actual exception payload of dynamic type and this number could be used to encode the type or also as a simple numeric error code. Until this is specified, a zero will be used in all cases. \ No newline at end of file diff --git a/docs/miscellaneous.rst b/docs/miscellaneous.rst index c5178b5119f1..8270727f9188 100644 --- a/docs/miscellaneous.rst +++ b/docs/miscellaneous.rst @@ -333,8 +333,9 @@ Global Variables - ``tx.origin`` (``address``): sender of the transaction (full call chain) - ``assert(bool condition)``: abort execution and revert state changes if condition is ``false`` (use for internal error) - ``require(bool condition)``: abort execution and revert state changes if condition is ``false`` (use for malformed input or error in external component) +- ``require(bool condition, string message)``: abort execution and revert state changes if condition is ``false`` (use for malformed input or error in external component). Also provide error message. - ``revert()``: abort execution and revert state changes -- ``revert(string)``: abort execution and revert state changes providing an explanatory string +- ``revert(string message)``: abort execution and revert state changes providing an explanatory string - ``blockhash(uint blockNumber) returns (bytes32)``: hash of the given block - only works for 256 most recent blocks - ``keccak256(...) returns (bytes32)``: compute the Ethereum-SHA-3 (Keccak-256) hash of the :ref:`(tightly packed) arguments ` - ``sha3(...) returns (bytes32)``: an alias to ``keccak256`` diff --git a/docs/solidity-by-example.rst b/docs/solidity-by-example.rst index 3636a3328c4e..3cbfcd666be5 100644 --- a/docs/solidity-by-example.rst +++ b/docs/solidity-by-example.rst @@ -87,17 +87,25 @@ of votes. // Give `voter` the right to vote on this ballot. // May only be called by `chairperson`. function giveRightToVote(address voter) public { - // If the argument of `require` evaluates to `false`, - // it terminates and reverts all changes to - // the state and to Ether balances. - // This consumes all gas in old EVM versions, but not anymore. - // It is often a good idea to use this if functions are - // called incorrectly. + // If the first argument of `require` evaluates + // to `false`, execution terminates and all + // changes to the state and to Ether balances + // are reverted. + // This used to consume all gas in old EVM versions, but + // not anymore. + // It is often a good idea to use `require` to check if + // functions are called correctly. + // As a second argument, you can also provide an + // explanation about what went wrong. require( - (msg.sender == chairperson) && - !voters[voter].voted && - (voters[voter].weight == 0) + msg.sender == chairperson, + "Only chairperson can give right to vote." ); + require( + !voters[voter].voted, + "The voter already voted." + ); + require(voters[voter].weight == 0); voters[voter].weight = 1; } @@ -105,10 +113,9 @@ of votes. function delegate(address to) public { // assigns reference Voter storage sender = voters[msg.sender]; - require(!sender.voted); + require(!sender.voted, "You already voted."); - // Self-delegation is not allowed. - require(to != msg.sender); + require(to != msg.sender, "Self-delegation is disallowed."); // Forward the delegation as long as // `to` also delegated. @@ -122,7 +129,7 @@ of votes. to = voters[to].delegate; // We found a loop in the delegation, not allowed. - require(to != msg.sender); + require(to != msg.sender, "Found loop in delegation."); } // Since `sender` is a reference, this @@ -145,7 +152,7 @@ of votes. /// to proposal `proposals[proposal].name`. function vote(uint proposal) public { Voter storage sender = voters[msg.sender]; - require(!sender.voted); + require(!sender.voted, "Already voted."); sender.voted = true; sender.vote = proposal; @@ -270,11 +277,17 @@ activate themselves. // Revert the call if the bidding // period is over. - require(now <= auctionEnd); + require( + now <= auctionEnd, + "Auction already ended." + ); // If the bid is not higher, send the // money back. - require(msg.value > highestBid); + require( + msg.value > highestBid, + "There already is a higher bid." + ); if (highestBid != 0) { // Sending back the money by simply using @@ -324,8 +337,8 @@ activate themselves. // external contracts. // 1. Conditions - require(now >= auctionEnd); // auction did not yet end - require(!ended); // this function has already been called + require(now >= auctionEnd, "Auction not yet ended."); + require(!ended, "auctionEnd has already been called."); // 2. Effects ended = true; @@ -543,7 +556,7 @@ Safe Remote Purchase function Purchase() public payable { seller = msg.sender; value = msg.value / 2; - require((2 * value) == msg.value); + require((2 * value) == msg.value, "Value has to be even."); } modifier condition(bool _condition) { @@ -552,17 +565,26 @@ Safe Remote Purchase } modifier onlyBuyer() { - require(msg.sender == buyer); + require( + msg.sender == buyer, + "Only buyer can call this." + ); _; } modifier onlySeller() { - require(msg.sender == seller); + require( + msg.sender == seller, + "Only seller can call this." + ); _; } modifier inState(State _state) { - require(state == _state); + require( + state == _state, + "Invalid state." + ); _; } diff --git a/docs/structure-of-a-contract.rst b/docs/structure-of-a-contract.rst index df40b1d018b2..9e5eacbbdf41 100644 --- a/docs/structure-of-a-contract.rst +++ b/docs/structure-of-a-contract.rst @@ -68,7 +68,10 @@ Function modifiers can be used to amend the semantics of functions in a declarat address public seller; modifier onlySeller() { // Modifier - require(msg.sender == seller); + require( + msg.sender == seller, + "Only seller can call this." + ); _; } diff --git a/docs/types.rst b/docs/types.rst index 5de6d07e9f36..07421bdfb4a3 100644 --- a/docs/types.rst +++ b/docs/types.rst @@ -495,7 +495,10 @@ Another example that uses external function types:: oracle.query("USD", this.oracleResponse); } function oracleResponse(bytes response) public { - require(msg.sender == address(oracle)); + require( + msg.sender == address(oracle), + "Only oracle can call this." + ); // Use the data } } diff --git a/docs/units-and-global-variables.rst b/docs/units-and-global-variables.rst index aad00ba2db40..9d5821d58a95 100644 --- a/docs/units-and-global-variables.rst +++ b/docs/units-and-global-variables.rst @@ -99,6 +99,8 @@ Error Handling throws if the condition is not met - to be used for internal errors. ``require(bool condition)``: throws if the condition is not met - to be used for errors in inputs or external components. +``require(bool condition, string message)``: + throws if the condition is not met - to be used for errors in inputs or external components. Also provides an error message. ``revert()``: abort execution and revert state changes ``revert(string reason)``: From 43b1dd758b3f3194907fad4be1c35bbc374cdd71 Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 3 Jan 2018 15:30:35 +0100 Subject: [PATCH 11/19] Changelog entry. --- Changelog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index 60f28db4010c..4011270ee20f 100644 --- a/Changelog.md +++ b/Changelog.md @@ -5,7 +5,7 @@ Features: * Code Generator: More specialized and thus optimized implementation for ``x.push(...)`` * Commandline interface: Error when missing or inaccessible file detected. Suppress it with the ``--ignore-missing`` flag. * Constant Evaluator: Fix evaluation of single element tuples. - * General: Allow providing reason string for ``revert()``. + * General: Allow providing reason string for ``revert()`` and ``require()``. * General: Limit the number of errors output in a single run to 256. * General: Support accessing dynamic return data in post-byzantium EVMs. * Interfaces: Allow overriding external functions in interfaces with public in an implementing contract. From 167ee2fcbb1000e6387142892c4252f8597a4481 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 4 Jan 2018 14:24:45 +0100 Subject: [PATCH 12/19] Update source location tests. --- test/libsolidity/Assembly.cpp | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/test/libsolidity/Assembly.cpp b/test/libsolidity/Assembly.cpp index 5519ae0d9fde..59993f66f490 100644 --- a/test/libsolidity/Assembly.cpp +++ b/test/libsolidity/Assembly.cpp @@ -158,13 +158,24 @@ BOOST_AUTO_TEST_CASE(location_test) } )"; shared_ptr n = make_shared(""); + shared_ptr codegen = make_shared("--CODEGEN--:8-17"); AssemblyItems items = compileContract(sourceCode); vector locations = - vector(24, SourceLocation(2, 75, n)) + - vector(32, SourceLocation(20, 72, n)) + - vector{SourceLocation(42, 51, n), SourceLocation(65, 67, n)} + - vector(2, SourceLocation(58, 67, n)) + - vector(2, SourceLocation(20, 72, n)); + vector(24, SourceLocation(2, 75, make_shared(""))) + + vector(2, SourceLocation(20, 72, make_shared(""))) + + vector(1, SourceLocation(8, 17, make_shared("--CODEGEN--"))) + + vector(3, SourceLocation(5, 7, make_shared("--CODEGEN--"))) + + vector(1, SourceLocation(30, 31, make_shared("--CODEGEN--"))) + + vector(1, SourceLocation(27, 28, make_shared("--CODEGEN--"))) + + vector(1, SourceLocation(20, 32, make_shared("--CODEGEN--"))) + + vector(1, SourceLocation(5, 7, make_shared("--CODEGEN--"))) + + vector(24, SourceLocation(20, 72, make_shared(""))) + + vector(1, SourceLocation(42, 51, make_shared(""))) + + vector(1, SourceLocation(65, 67, make_shared(""))) + + vector(2, SourceLocation(58, 67, make_shared(""))) + + vector(2, SourceLocation(20, 72, make_shared(""))); + + checkAssemblyLocations(items, locations); } From 42c4c78390b6330e8bc4558b4be5d580251abcba Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 4 Jan 2018 17:08:47 +0100 Subject: [PATCH 13/19] Adjust tests. --- test/libsolidity/JSONCompiler.cpp | 8 ++++---- .../SolidityNameAndTypeResolution.cpp | 5 +++-- test/libsolidity/StandardCompiler.cpp | 19 ++++++++++++------- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/test/libsolidity/JSONCompiler.cpp b/test/libsolidity/JSONCompiler.cpp index cdcc22a650a8..2b3df3a73dad 100644 --- a/test/libsolidity/JSONCompiler.cpp +++ b/test/libsolidity/JSONCompiler.cpp @@ -111,7 +111,7 @@ BOOST_AUTO_TEST_CASE(basic_compilation) BOOST_CHECK(contract["bytecode"].isString()); BOOST_CHECK_EQUAL( dev::test::bytecodeSansMetadata(contract["bytecode"].asString()), - "60806040523415600e57600080fd5b603580601b6000396000f3006080604052600080fd00" + "6080604052348015600f57600080fd5b50603580601d6000396000f3006080604052600080fd00" ); BOOST_CHECK(contract["runtimeBytecode"].isString()); BOOST_CHECK_EQUAL( @@ -122,7 +122,7 @@ BOOST_AUTO_TEST_CASE(basic_compilation) BOOST_CHECK(contract["gasEstimates"].isObject()); BOOST_CHECK_EQUAL( dev::jsonCompactPrint(contract["gasEstimates"]), - "{\"creation\":[61,10600],\"external\":{},\"internal\":{}}" + "{\"creation\":[66,10600],\"external\":{},\"internal\":{}}" ); BOOST_CHECK(contract["metadata"].isString()); BOOST_CHECK(dev::test::isValidMetadata(contract["metadata"].asString())); @@ -153,7 +153,7 @@ BOOST_AUTO_TEST_CASE(single_compilation) BOOST_CHECK(contract["bytecode"].isString()); BOOST_CHECK_EQUAL( dev::test::bytecodeSansMetadata(contract["bytecode"].asString()), - "60806040523415600e57600080fd5b603580601b6000396000f3006080604052600080fd00" + "6080604052348015600f57600080fd5b50603580601d6000396000f3006080604052600080fd00" ); BOOST_CHECK(contract["runtimeBytecode"].isString()); BOOST_CHECK_EQUAL( @@ -164,7 +164,7 @@ BOOST_AUTO_TEST_CASE(single_compilation) BOOST_CHECK(contract["gasEstimates"].isObject()); BOOST_CHECK_EQUAL( dev::jsonCompactPrint(contract["gasEstimates"]), - "{\"creation\":[61,10600],\"external\":{},\"internal\":{}}" + "{\"creation\":[66,10600],\"external\":{},\"internal\":{}}" ); BOOST_CHECK(contract["metadata"].isString()); BOOST_CHECK(dev::test::isValidMetadata(contract["metadata"].asString())); diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 2bf718867f6f..5688267a90cf 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -6009,7 +6009,8 @@ BOOST_AUTO_TEST_CASE(bare_others) { CHECK_WARNING("contract C { function f() pure public { selfdestruct; } }", "Statement has no effect."); CHECK_WARNING("contract C { function f() pure public { assert; } }", "Statement has no effect."); - CHECK_WARNING("contract C { function f() pure public { require; } }", "Statement has no effect."); + // This is different because it does have overloads. + CHECK_ERROR("contract C { function f() pure public { require; } }", TypeError, "No matching declaration found after variable lookup."); CHECK_WARNING("contract C { function f() pure public { suicide; } }", "Statement has no effect."); } @@ -6508,7 +6509,7 @@ BOOST_AUTO_TEST_CASE(does_not_error_transfer_regular_function) CHECK_SUCCESS_NO_WARNINGS(text); } -BOOST_AUTO_TEST_CASE(returndatacopy_as_variable) +BOOST_AUTO_TEST_CASE(returndatasize_as_variable) { char const* text = R"( contract c { function f() public { uint returndatasize; assembly { returndatasize }}} diff --git a/test/libsolidity/StandardCompiler.cpp b/test/libsolidity/StandardCompiler.cpp index b285a2a0e99f..74bf01b2f919 100644 --- a/test/libsolidity/StandardCompiler.cpp +++ b/test/libsolidity/StandardCompiler.cpp @@ -261,19 +261,24 @@ BOOST_AUTO_TEST_CASE(basic_compilation) BOOST_CHECK(contract["evm"]["bytecode"]["object"].isString()); BOOST_CHECK_EQUAL( dev::test::bytecodeSansMetadata(contract["evm"]["bytecode"]["object"].asString()), - "60806040523415600e57600080fd5b603580601b6000396000f3006080604052600080fd00" + "6080604052348015600f57600080fd5b50603580601d6000396000f3006080604052600080fd00" ); BOOST_CHECK(contract["evm"]["assembly"].isString()); BOOST_CHECK(contract["evm"]["assembly"].asString().find( - " /* \"fileA\":0:14 contract A { } */\n mstore(0x40, 0x80)\n jumpi(tag_1, iszero(callvalue))\n" - " 0x0\n dup1\n revert\ntag_1:\n dataSize(sub_0)\n dup1\n dataOffset(sub_0)\n 0x0\n codecopy\n 0x0\n" - " return\nstop\n\nsub_0: assembly {\n /* \"fileA\":0:14 contract A { } */\n" - " mstore(0x40, 0x80)\n 0x0\n dup1\n revert\n\n" - " auxdata: 0xa165627a7a7230582") == 0); + " /* \"fileA\":0:14 contract A { } */\n mstore(0x40, 0x80)\n " + "callvalue\n /* \"--CODEGEN--\":8:17 */\n dup1\n " + "/* \"--CODEGEN--\":5:7 */\n iszero\n tag_1\n jumpi\n " + "/* \"--CODEGEN--\":30:31 */\n 0x0\n /* \"--CODEGEN--\":27:28 */\n " + "dup1\n /* \"--CODEGEN--\":20:32 */\n revert\n /* \"--CODEGEN--\":5:7 */\n" + "tag_1:\n /* \"fileA\":0:14 contract A { } */\n pop\n dataSize(sub_0)\n dup1\n " + "dataOffset(sub_0)\n 0x0\n codecopy\n 0x0\n return\nstop\n\nsub_0: assembly {\n " + "/* \"fileA\":0:14 contract A { } */\n mstore(0x40, 0x80)\n 0x0\n " + "dup1\n revert\n\n auxdata: 0xa165627a7a72305820" + ) == 0); BOOST_CHECK(contract["evm"]["gasEstimates"].isObject()); BOOST_CHECK_EQUAL( dev::jsonCompactPrint(contract["evm"]["gasEstimates"]), - "{\"creation\":{\"codeDepositCost\":\"10600\",\"executionCost\":\"61\",\"totalCost\":\"10661\"}}" + "{\"creation\":{\"codeDepositCost\":\"10600\",\"executionCost\":\"66\",\"totalCost\":\"10666\"}}" ); BOOST_CHECK(contract["metadata"].isString()); BOOST_CHECK(dev::test::isValidMetadata(contract["metadata"].asString())); From fcb7a2721636a9a2f05659610fc05fa8513f745d Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 2 Mar 2018 17:26:16 +0100 Subject: [PATCH 14/19] Only forward returndata if EVM version supports it. --- libsolidity/codegen/CompilerContext.cpp | 2 +- libsolidity/codegen/CompilerContext.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp index 2cd256f3dd81..a35eea7387ce 100644 --- a/libsolidity/codegen/CompilerContext.cpp +++ b/libsolidity/codegen/CompilerContext.cpp @@ -264,7 +264,7 @@ CompilerContext& CompilerContext::appendRevert() CompilerContext& CompilerContext::appendConditionalRevert(bool _forwardReturnData) { - if (_forwardReturnData) + if (_forwardReturnData && m_evmVersion.supportsReturndata()) appendInlineAssembly(R"({ if condition { returndatacopy(0, 0, returndatasize()) diff --git a/libsolidity/codegen/CompilerContext.h b/libsolidity/codegen/CompilerContext.h index c6f2f3be5c27..098472f7b178 100644 --- a/libsolidity/codegen/CompilerContext.h +++ b/libsolidity/codegen/CompilerContext.h @@ -158,6 +158,8 @@ class CompilerContext CompilerContext& appendRevert(); /// 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; } From e133b1a0cd78acebb0db5448ec62e62ae0060fa2 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 6 Mar 2018 20:09:52 +0100 Subject: [PATCH 15/19] Adjust expectations in case of homestead VM. --- test/libsolidity/SolidityEndToEndTest.cpp | 27 ++++++++++++++--------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index c7d8f917522c..600757f10f1d 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -10452,8 +10452,9 @@ BOOST_AUTO_TEST_CASE(revert_with_cause) } )"; compileAndRun(sourceCode, 0, "C"); - ABI_CHECK(callContractFunction("f()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 7, "test123")); - ABI_CHECK(callContractFunction("g()"), encodeArgs(0, 0x40, 0xa0, 0, 0x40, 44, "test1234567890123456789012345678901234567890")); + bool const haveReturndata = dev::test::Options::get().evmVersion().supportsReturndata(); + ABI_CHECK(callContractFunction("f()"), haveReturndata ? encodeArgs(0, 0x40, 0x80, 0, 0x40, 7, "test123") : bytes()); + ABI_CHECK(callContractFunction("g()"), haveReturndata ? encodeArgs(0, 0x40, 0xa0, 0, 0x40, 44, "test1234567890123456789012345678901234567890") : bytes()); } BOOST_AUTO_TEST_CASE(require_with_message) @@ -10505,11 +10506,12 @@ BOOST_AUTO_TEST_CASE(require_with_message) } )"; compileAndRun(sourceCode, 0, "C"); - ABI_CHECK(callContractFunction("f(uint256)", 8), encodeArgs(1, 0x40, 0)); - ABI_CHECK(callContractFunction("f(uint256)", 5), encodeArgs(0, 0x40, 0x80, 0, 0x40, 6, "failed")); - ABI_CHECK(callContractFunction("g()"), encodeArgs(1, 0x40, 0)); - ABI_CHECK(callContractFunction("g()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 18 , "only on second run")); - ABI_CHECK(callContractFunction("h()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 3, "abc")); + bool const haveReturndata = dev::test::Options::get().evmVersion().supportsReturndata(); + ABI_CHECK(callContractFunction("f(uint256)", 8), haveReturndata ? encodeArgs(1, 0x40, 0) : bytes()); + ABI_CHECK(callContractFunction("f(uint256)", 5), haveReturndata ? encodeArgs(0, 0x40, 0x80, 0, 0x40, 6, "failed") : bytes()); + ABI_CHECK(callContractFunction("g()"), haveReturndata ? encodeArgs(1, 0x40, 0) : bytes()); + ABI_CHECK(callContractFunction("g()"), haveReturndata ? encodeArgs(0, 0x40, 0x80, 0, 0x40, 18 , "only on second run") : bytes()); + ABI_CHECK(callContractFunction("h()"), haveReturndata ? encodeArgs(0, 0x40, 0x80, 0, 0x40, 3, "abc") : bytes()); } BOOST_AUTO_TEST_CASE(bubble_up_error_messages) @@ -10545,8 +10547,9 @@ BOOST_AUTO_TEST_CASE(bubble_up_error_messages) } )"; compileAndRun(sourceCode, 0, "C"); - ABI_CHECK(callContractFunction("f()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 7, "message")); - ABI_CHECK(callContractFunction("g()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 7, "message")); + bool const haveReturndata = dev::test::Options::get().evmVersion().supportsReturndata(); + ABI_CHECK(callContractFunction("f()"), haveReturndata ? encodeArgs(0, 0x40, 0x80, 0, 0x40, 7, "message") : bytes()); + ABI_CHECK(callContractFunction("g()"), haveReturndata ? encodeArgs(0, 0x40, 0x80, 0, 0x40, 7, "message") : bytes()); } BOOST_AUTO_TEST_CASE(bubble_up_error_messages_through_transfer) @@ -10579,7 +10582,8 @@ BOOST_AUTO_TEST_CASE(bubble_up_error_messages_through_transfer) } )"; compileAndRun(sourceCode, 0, "C"); - ABI_CHECK(callContractFunction("f()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 7, "message")); + bool const haveReturndata = dev::test::Options::get().evmVersion().supportsReturndata(); + ABI_CHECK(callContractFunction("f()"), haveReturndata ? encodeArgs(0, 0x40, 0x80, 0, 0x40, 7, "message") : bytes()); } BOOST_AUTO_TEST_CASE(bubble_up_error_messages_through_create) @@ -10614,7 +10618,8 @@ BOOST_AUTO_TEST_CASE(bubble_up_error_messages_through_create) } )"; compileAndRun(sourceCode, 0, "C"); - ABI_CHECK(callContractFunction("f()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 7, "message")); + bool const haveReturndata = dev::test::Options::get().evmVersion().supportsReturndata(); + ABI_CHECK(callContractFunction("f()"), haveReturndata ? encodeArgs(0, 0x40, 0x80, 0, 0x40, 7, "message") : bytes()); } BOOST_AUTO_TEST_CASE(negative_stack_height) From 338a875134f2e41e9a7e254cc3f7d87c7f4d462e Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 4 Apr 2018 17:59:45 +0200 Subject: [PATCH 16/19] Update expectation. --- test/libsolidity/SolidityCompiler.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/libsolidity/SolidityCompiler.cpp b/test/libsolidity/SolidityCompiler.cpp index e87ab6030fe7..90540f3ef825 100644 --- a/test/libsolidity/SolidityCompiler.cpp +++ b/test/libsolidity/SolidityCompiler.cpp @@ -47,8 +47,8 @@ BOOST_AUTO_TEST_CASE(does_not_include_creation_time_only_internal_functions) BOOST_REQUIRE_MESSAGE(m_compiler.compile(), "Compiling contract failed"); bytes const& creationBytecode = m_compiler.object("C").bytecode; bytes const& runtimeBytecode = m_compiler.runtimeObject("C").bytecode; - BOOST_CHECK(creationBytecode.size() >= 120); - BOOST_CHECK(creationBytecode.size() <= 150); + BOOST_CHECK(creationBytecode.size() >= 130); + BOOST_CHECK(creationBytecode.size() <= 160); BOOST_CHECK(runtimeBytecode.size() >= 50); BOOST_CHECK(runtimeBytecode.size() <= 70); } From 4faa839813ce76fc87f99b002aad6cadd2b784e1 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 6 Apr 2018 15:14:55 +0200 Subject: [PATCH 17/19] Use error signature for revert data. --- libsolidity/codegen/CompilerUtils.cpp | 14 +++++++++++++ libsolidity/codegen/CompilerUtils.h | 7 +++++++ libsolidity/codegen/ExpressionCompiler.cpp | 23 +++------------------- test/libsolidity/SolidityEndToEndTest.cpp | 23 +++++++++++++--------- 4 files changed, 38 insertions(+), 29 deletions(-) diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp index 34337d7d79b5..b4550153e920 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: + abiEncode({_argumentType.shared_from_this()}, {make_shared(DataLocation::Memory, true)}); + toSizeAfterFreeMemoryPointer(); + m_context << Instruction::REVERT; + m_context.adjustStackOffset(_argumentType.sizeOnStack()); +} + unsigned CompilerUtils::loadFromMemory( unsigned _offset, Type const& _type, diff --git a/libsolidity/codegen/CompilerUtils.h b/libsolidity/codegen/CompilerUtils.h index a32c5c6e1df7..476a7559d62e 100644 --- a/libsolidity/codegen/CompilerUtils.h +++ b/libsolidity/codegen/CompilerUtils.h @@ -54,6 +54,13 @@ class CompilerUtils /// Stack post: 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/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index 3f521f2d6b68..b67e7b688f4d 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -685,17 +685,11 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) { if (!arguments.empty()) { + // function-sel(Error(string)) + encoding solAssert(arguments.size() == 1, ""); solAssert(function.parameterTypes().size() == 1, ""); - m_context << u256(0); arguments.front()->accept(*this); - utils().fetchFreeMemoryPointer(); - utils().abiEncode( - {make_shared(256), arguments.front()->annotation().type}, - {make_shared(256), make_shared(DataLocation::Memory, true)} - ); - utils().toSizeAfterFreeMemoryPointer(); - m_context << Instruction::REVERT; + utils().revertWithStringData(*arguments.front()->annotation().type); } else m_context.appendRevert(); @@ -937,18 +931,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) // condition was not met, flag an error m_context.appendInvalid(); else if (arguments.size() > 1) - { - m_context << u256(0); - utils().moveIntoStack(arguments.at(1)->annotation().type->sizeOnStack(), 1); - utils().fetchFreeMemoryPointer(); - utils().abiEncode( - {make_shared(256), arguments.at(1)->annotation().type}, - {make_shared(256), make_shared(DataLocation::Memory, true)} - ); - utils().toSizeAfterFreeMemoryPointer(); - m_context << Instruction::REVERT; - m_context.adjustStackOffset(arguments.at(1)->annotation().type->sizeOnStack()); - } + utils().revertWithStringData(*arguments.at(1)->annotation().type); else m_context.appendRevert(); // the success branch diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 600757f10f1d..c71d6b5942e8 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -10453,8 +10453,9 @@ BOOST_AUTO_TEST_CASE(revert_with_cause) )"; compileAndRun(sourceCode, 0, "C"); bool const haveReturndata = dev::test::Options::get().evmVersion().supportsReturndata(); - ABI_CHECK(callContractFunction("f()"), haveReturndata ? encodeArgs(0, 0x40, 0x80, 0, 0x40, 7, "test123") : bytes()); - ABI_CHECK(callContractFunction("g()"), haveReturndata ? encodeArgs(0, 0x40, 0xa0, 0, 0x40, 44, "test1234567890123456789012345678901234567890") : bytes()); + bytes const errorSignature = bytes{0x08, 0xc3, 0x79, 0xa0}; + ABI_CHECK(callContractFunction("f()"), haveReturndata ? encodeArgs(0, 0x40, 0x64) + errorSignature + encodeArgs(0x20, 7, "test123") + bytes(28, 0) : bytes()); + ABI_CHECK(callContractFunction("g()"), haveReturndata ? encodeArgs(0, 0x40, 0x84) + errorSignature + encodeArgs(0x20, 44, "test1234567890123456789012345678901234567890") + bytes(28, 0): bytes()); } BOOST_AUTO_TEST_CASE(require_with_message) @@ -10507,11 +10508,12 @@ BOOST_AUTO_TEST_CASE(require_with_message) )"; compileAndRun(sourceCode, 0, "C"); bool const haveReturndata = dev::test::Options::get().evmVersion().supportsReturndata(); + bytes const errorSignature = bytes{0x08, 0xc3, 0x79, 0xa0}; ABI_CHECK(callContractFunction("f(uint256)", 8), haveReturndata ? encodeArgs(1, 0x40, 0) : bytes()); - ABI_CHECK(callContractFunction("f(uint256)", 5), haveReturndata ? encodeArgs(0, 0x40, 0x80, 0, 0x40, 6, "failed") : bytes()); + ABI_CHECK(callContractFunction("f(uint256)", 5), haveReturndata ? encodeArgs(0, 0x40, 0x64) + errorSignature + encodeArgs(0x20, 6, "failed") + bytes(28, 0) : bytes()); ABI_CHECK(callContractFunction("g()"), haveReturndata ? encodeArgs(1, 0x40, 0) : bytes()); - ABI_CHECK(callContractFunction("g()"), haveReturndata ? encodeArgs(0, 0x40, 0x80, 0, 0x40, 18 , "only on second run") : bytes()); - ABI_CHECK(callContractFunction("h()"), haveReturndata ? encodeArgs(0, 0x40, 0x80, 0, 0x40, 3, "abc") : bytes()); + ABI_CHECK(callContractFunction("g()"), haveReturndata ? encodeArgs(0, 0x40, 0x64) + errorSignature + encodeArgs(0x20, 18, "only on second run") + bytes(28, 0) : bytes()); + ABI_CHECK(callContractFunction("h()"), haveReturndata ? encodeArgs(0, 0x40, 0x64) + errorSignature + encodeArgs(0x20, 3, "abc") + bytes(28, 0): bytes()); } BOOST_AUTO_TEST_CASE(bubble_up_error_messages) @@ -10548,8 +10550,9 @@ BOOST_AUTO_TEST_CASE(bubble_up_error_messages) )"; compileAndRun(sourceCode, 0, "C"); bool const haveReturndata = dev::test::Options::get().evmVersion().supportsReturndata(); - ABI_CHECK(callContractFunction("f()"), haveReturndata ? encodeArgs(0, 0x40, 0x80, 0, 0x40, 7, "message") : bytes()); - ABI_CHECK(callContractFunction("g()"), haveReturndata ? encodeArgs(0, 0x40, 0x80, 0, 0x40, 7, "message") : bytes()); + bytes const errorSignature = bytes{0x08, 0xc3, 0x79, 0xa0}; + ABI_CHECK(callContractFunction("f()"), haveReturndata ? encodeArgs(0, 0x40, 0x64) + errorSignature + encodeArgs(0x20, 7, "message") + bytes(28, 0) : bytes()); + ABI_CHECK(callContractFunction("g()"), haveReturndata ? encodeArgs(0, 0x40, 0x64) + errorSignature + encodeArgs(0x20, 7, "message") + bytes(28, 0) : bytes()); } BOOST_AUTO_TEST_CASE(bubble_up_error_messages_through_transfer) @@ -10583,7 +10586,8 @@ BOOST_AUTO_TEST_CASE(bubble_up_error_messages_through_transfer) )"; compileAndRun(sourceCode, 0, "C"); bool const haveReturndata = dev::test::Options::get().evmVersion().supportsReturndata(); - ABI_CHECK(callContractFunction("f()"), haveReturndata ? encodeArgs(0, 0x40, 0x80, 0, 0x40, 7, "message") : bytes()); + bytes const errorSignature = bytes{0x08, 0xc3, 0x79, 0xa0}; + ABI_CHECK(callContractFunction("f()"), haveReturndata ? encodeArgs(0, 0x40, 0x64) + errorSignature + encodeArgs(0x20, 7, "message") + bytes(28, 0) : bytes()); } BOOST_AUTO_TEST_CASE(bubble_up_error_messages_through_create) @@ -10619,7 +10623,8 @@ BOOST_AUTO_TEST_CASE(bubble_up_error_messages_through_create) )"; compileAndRun(sourceCode, 0, "C"); bool const haveReturndata = dev::test::Options::get().evmVersion().supportsReturndata(); - ABI_CHECK(callContractFunction("f()"), haveReturndata ? encodeArgs(0, 0x40, 0x80, 0, 0x40, 7, "message") : bytes()); + bytes const errorSignature = bytes{0x08, 0xc3, 0x79, 0xa0}; + ABI_CHECK(callContractFunction("f()"), haveReturndata ? encodeArgs(0, 0x40, 0x64) + errorSignature + encodeArgs(0x20, 7, "message") + bytes(28, 0) : bytes()); } BOOST_AUTO_TEST_CASE(negative_stack_height) From b25598126e57fca73058edd722eef7c681460557 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 6 Apr 2018 16:11:52 +0200 Subject: [PATCH 18/19] Update documentation and minor changes. --- docs/control-structures.rst | 18 +++++++++++------- test/libsolidity/Assembly.cpp | 1 - 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/docs/control-structures.rst b/docs/control-structures.rst index 7e3027a0621d..7024a68401e9 100644 --- a/docs/control-structures.rst +++ b/docs/control-structures.rst @@ -473,7 +473,7 @@ Catching exceptions is not yet possible. In the following example, you can see how ``require`` can be used to easily check conditions on inputs and how ``assert`` can be used for internal error checking. Note that you can optionally provide -a message string for require, but not for assert. +a message string for ``require``, but not for ``assert``. :: @@ -539,9 +539,13 @@ The following example shows how an error string can be used together with revert } } -The provided string will be abi-encoded together with a uint value that will always be zero. -This means that if a string ``x`` is provided, ``(0, x)`` will be encoded as if a function with arguments -``(uint256, string)`` was called. This zero is used as a version identifier. Future extensions -of this feature might provide actual exception payload of dynamic type and this number could be used -to encode the type or also as a simple numeric error code. Until this is specified, a zero will -be used in all cases. \ No newline at end of file +The provided string will be :ref:`abi-encoded ` as if it were a call to a function ``Error(string)``. +In the above example, ``revert("Not enough Ether provided.");`` will cause the following hexadecimal data be +set as error return data: + +.. code:: + + 0x08c379a0 // Function selector for Error(string) + 0x0000000000000000000000000000000000000000000000000000000000000020 // Data offset + 0x000000000000000000000000000000000000000000000000000000000000001a // String length + 0x4e6f7420656e6f7567682045746865722070726f76696465642e000000000000 // String data diff --git a/test/libsolidity/Assembly.cpp b/test/libsolidity/Assembly.cpp index 59993f66f490..bdb7a1073d35 100644 --- a/test/libsolidity/Assembly.cpp +++ b/test/libsolidity/Assembly.cpp @@ -158,7 +158,6 @@ BOOST_AUTO_TEST_CASE(location_test) } )"; shared_ptr n = make_shared(""); - shared_ptr codegen = make_shared("--CODEGEN--:8-17"); AssemblyItems items = compileContract(sourceCode); vector locations = vector(24, SourceLocation(2, 75, make_shared(""))) + From 966367305ad511900bedfd9af08114a0b1307399 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 12 Apr 2018 20:13:16 +0200 Subject: [PATCH 19/19] Remove dead code and clarify throw. --- docs/control-structures.rst | 2 +- test/libsolidity/Assembly.cpp | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/control-structures.rst b/docs/control-structures.rst index 7024a68401e9..879e26f7a427 100644 --- a/docs/control-structures.rst +++ b/docs/control-structures.rst @@ -457,7 +457,7 @@ If used properly, analysis tools can evaluate your contract to identify the cond There are two other ways to trigger exceptions: The ``revert`` function can be used to flag an error and revert the current call. It is possible to provide a string message containing details about the error that will be passed back to the caller. -The ``throw`` keyword can also be used as an alternative to ``revert()``, but is deprecated. +The deprecated keyword ``throw`` can also be used as an alternative to ``revert()`` (but only without error message). .. note:: From version 0.4.13 the ``throw`` keyword is deprecated and will be phased out in the future. diff --git a/test/libsolidity/Assembly.cpp b/test/libsolidity/Assembly.cpp index bdb7a1073d35..77ca363a80a3 100644 --- a/test/libsolidity/Assembly.cpp +++ b/test/libsolidity/Assembly.cpp @@ -157,7 +157,6 @@ BOOST_AUTO_TEST_CASE(location_test) } } )"; - shared_ptr n = make_shared(""); AssemblyItems items = compileContract(sourceCode); vector locations = vector(24, SourceLocation(2, 75, make_shared(""))) +