From 4bfe09897e3346038b95eb10d74dd0d8c9a8f40e Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 25 Aug 2016 23:53:03 +0200 Subject: [PATCH 1/4] Make fallback function throw by default. --- libsolidity/analysis/TypeChecker.cpp | 2 ++ libsolidity/codegen/ContractCompiler.cpp | 5 +---- test/libsolidity/Assembly.cpp | 2 +- test/libsolidity/SolidityEndToEndTest.cpp | 2 ++ 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 235fcabd0..bc03da01f 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -92,6 +92,8 @@ bool TypeChecker::visit(ContractDefinition const& _contract) else { fallbackFunction = function; + if (_contract.isLibrary()) + typeError(fallbackFunction->location(), "Libraries cannot have fallback functions."); if (!fallbackFunction->parameters().empty()) typeError(fallbackFunction->parameterList().location(), "Fallback function cannot take parameters."); if (!fallbackFunction->returnParameters().empty()) diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 715852be8..9d77ccdca 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -247,11 +247,8 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac m_context << returnTag; appendReturnValuePacker(FunctionType(*fallback).returnParameterTypes(), _contract.isLibrary()); } - else if (_contract.isLibrary()) - // Reject invalid library calls and ether sent to a library. - m_context.appendJumpTo(m_context.errorTag()); else - m_context << Instruction::STOP; // function not found + m_context.appendJumpTo(m_context.errorTag()); for (auto const& it: interfaceFunctions) { diff --git a/test/libsolidity/Assembly.cpp b/test/libsolidity/Assembly.cpp index 557d496a1..81332f4fb 100644 --- a/test/libsolidity/Assembly.cpp +++ b/test/libsolidity/Assembly.cpp @@ -114,7 +114,7 @@ BOOST_AUTO_TEST_CASE(location_test) shared_ptr n = make_shared("source"); AssemblyItems items = compileContract(sourceCode); vector locations = - vector(17, SourceLocation(2, 75, n)) + + vector(18, SourceLocation(2, 75, n)) + vector(28, SourceLocation(20, 72, n)) + vector{SourceLocation(42, 51, n), SourceLocation(65, 67, n)} + vector(4, SourceLocation(58, 67, n)) + diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 8a61907ac..14bf28a95 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -2053,6 +2053,7 @@ BOOST_AUTO_TEST_CASE(contracts_as_addresses) { char const* sourceCode = R"( contract helper { + function() { } // can receive ether } contract test { helper h; @@ -5943,6 +5944,7 @@ BOOST_AUTO_TEST_CASE(reject_ether_sent_to_library) function f(address x) returns (bool) { return x.send(1); } + function () {} } )"; compileAndRun(sourceCode, 0, "lib"); From 546aca2a3ec11df5b54d5a3d01948ebc3d830163 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 26 Aug 2016 16:56:36 +0200 Subject: [PATCH 2/4] Test cases. --- test/libsolidity/SolidityEndToEndTest.cpp | 13 +++++++++++++ test/libsolidity/SolidityNameAndTypeResolution.cpp | 10 ++++++++++ 2 files changed, 23 insertions(+) diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 14bf28a95..a370aafa2 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -2541,6 +2541,19 @@ BOOST_AUTO_TEST_CASE(inherited_fallback_function) BOOST_CHECK(callContractFunction("getData()") == encodeArgs(1)); } +BOOST_AUTO_TEST_CASE(default_fallback_throws) +{ + char const* sourceCode = R"( + contract A { + function f() returns (bool) { + return this.call(); + } + } + )"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("f()") == encodeArgs(0)); +} + BOOST_AUTO_TEST_CASE(event) { char const* sourceCode = R"( diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index f0ab07c51..fdd8d7f48 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -1102,6 +1102,16 @@ BOOST_AUTO_TEST_CASE(fallback_function_with_arguments) BOOST_CHECK(expectError(text) == Error::Type::TypeError); } +BOOST_AUTO_TEST_CASE(fallback_function_in_library) +{ + char const* text = R"( + library C { + function() {} + } + )"; + BOOST_CHECK(expectError(text) == Error::Type::TypeError); +} + BOOST_AUTO_TEST_CASE(fallback_function_with_return_parameters) { char const* text = R"( From eb241ff1b30ca32aaba0a455c0aa74cbb868c405 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 26 Aug 2016 17:00:26 +0200 Subject: [PATCH 3/4] Documentation. --- docs/contracts.rst | 5 +++++ docs/security-considerations.rst | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/contracts.rst b/docs/contracts.rst index d3a89c1e2..234ab46a0 100644 --- a/docs/contracts.rst +++ b/docs/contracts.rst @@ -447,6 +447,11 @@ In particular, the following operations will consume more gas than the stipend p Please ensure you test your fallback function thoroughly to ensure the execution cost is less than 2300 gas before deploying a contract. +.. warning:: + The default fallback function will throw an exception (this was different + before Solidity v0.4.0), so if you want your contract to receive Ether, + you have to implement a fallback function. + :: contract Test { diff --git a/docs/security-considerations.rst b/docs/security-considerations.rst index f8d099e4a..7e846674c 100644 --- a/docs/security-considerations.rst +++ b/docs/security-considerations.rst @@ -103,7 +103,9 @@ and stall those. Please be explicit about such cases in the documentation of you Sending and Receiving Ether =========================== -- If a contract receives Ether (without a function being called), the fallback function is executed. The contract can only rely +- If a contract receives Ether (without a function being called), the fallback function is executed. + If it does not have a fallback function, the Ether will be rejected (by throwing an exception). + During the execution of the fallback function, the contract can only rely on the "gas stipend" (2300 gas) being available to it at that time. This stipend is not enough to access storage in any way. To be sure that your contract can receive Ether in that way, check the gas requirements of the fallback function (for example in the "details" section in browser-solidity). From 9a91bd80abc79b82de9e3e7881cccab43e6dde9a Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 30 Aug 2016 15:37:10 +0200 Subject: [PATCH 4/4] Clarify warning. --- docs/contracts.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/contracts.rst b/docs/contracts.rst index 234ab46a0..a22a35447 100644 --- a/docs/contracts.rst +++ b/docs/contracts.rst @@ -448,8 +448,9 @@ In particular, the following operations will consume more gas than the stipend p Please ensure you test your fallback function thoroughly to ensure the execution cost is less than 2300 gas before deploying a contract. .. warning:: - The default fallback function will throw an exception (this was different - before Solidity v0.4.0), so if you want your contract to receive Ether, + Contracts that receive Ether but do not define a fallback function + throw an exception, sending back the Ether (this was different + before Solidity v0.4.0). So if you want your contract to receive Ether, you have to implement a fallback function. ::