diff options
| -rw-r--r-- | libsolidity/analysis/TypeChecker.cpp | 12 | ||||
| -rw-r--r-- | libsolidity/ast/Types.cpp | 62 | ||||
| -rw-r--r-- | libsolidity/ast/Types.h | 24 | ||||
| -rw-r--r-- | libsolidity/codegen/ContractCompiler.cpp | 6 | ||||
| -rw-r--r-- | libsolidity/codegen/ExpressionCompiler.cpp | 2 | ||||
| -rw-r--r-- | test/libsolidity/Assembly.cpp | 2 | ||||
| -rw-r--r-- | test/libsolidity/SolidityEndToEndTest.cpp | 59 | ||||
| -rw-r--r-- | test/libsolidity/SolidityNameAndTypeResolution.cpp | 18 |
8 files changed, 123 insertions, 62 deletions
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index c1781397..c948b90b 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -349,7 +349,7 @@ void TypeChecker::endVisit(InheritanceSpecifier const& _inheritance) typeError(_inheritance.location(), "Libraries cannot be inherited from."); auto const& arguments = _inheritance.arguments(); - TypePointers parameterTypes = ContractType(*base).constructorType()->parameterTypes(); + TypePointers parameterTypes = ContractType(*base).newExpressionType()->parameterTypes(); if (!arguments.empty() && parameterTypes.size() != arguments.size()) { typeError( @@ -1264,15 +1264,7 @@ void TypeChecker::endVisit(NewExpression const& _newExpression) "Circular reference for contract creation (cannot create instance of derived or same contract)." ); - auto contractType = make_shared<ContractType>(*contract); - TypePointers parameterTypes = contractType->constructorType()->parameterTypes(); - _newExpression.annotation().type = make_shared<FunctionType>( - parameterTypes, - TypePointers{contractType}, - strings(), - strings(), - FunctionType::Location::Creation - ); + _newExpression.annotation().type = FunctionType::newExpressionType(*contract); } else if (type->category() == Type::Category::Array) { diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index ca8490f2..a0c1626d 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -362,8 +362,8 @@ MemberList::MemberMap IntegerType::nativeMembers(ContractDefinition const*) cons if (isAddress()) return { {"balance", make_shared<IntegerType >(256)}, - {"call", make_shared<FunctionType>(strings(), strings{"bool"}, FunctionType::Location::Bare, true)}, - {"callcode", make_shared<FunctionType>(strings(), strings{"bool"}, FunctionType::Location::BareCallCode, true)}, + {"call", make_shared<FunctionType>(strings(), strings{"bool"}, FunctionType::Location::Bare, true, false, true)}, + {"callcode", make_shared<FunctionType>(strings(), strings{"bool"}, FunctionType::Location::BareCallCode, true, false, true)}, {"delegatecall", make_shared<FunctionType>(strings(), strings{"bool"}, FunctionType::Location::BareDelegateCall, true)}, {"send", make_shared<FunctionType>(strings{"uint"}, strings{"bool"}, FunctionType::Location::Send)} }; @@ -1329,16 +1329,10 @@ MemberList::MemberMap ContractType::nativeMembers(ContractDefinition const*) con return members; } -shared_ptr<FunctionType const> const& ContractType::constructorType() const +shared_ptr<FunctionType const> const& ContractType::newExpressionType() const { if (!m_constructorType) - { - FunctionDefinition const* constructor = m_contract.constructor(); - if (constructor) - m_constructorType = make_shared<FunctionType>(*constructor); - else - m_constructorType = make_shared<FunctionType>(TypePointers(), TypePointers()); - } + m_constructorType = FunctionType::newExpressionType(m_contract); return m_constructorType; } @@ -1738,7 +1732,7 @@ FunctionType::FunctionType(VariableDeclaration const& _varDecl): swap(retParamNames, m_returnParameterNames); } -FunctionType::FunctionType(const EventDefinition& _event): +FunctionType::FunctionType(EventDefinition const& _event): m_location(Location::Event), m_isConstant(true), m_declaration(&_event) { TypePointers params; @@ -1754,6 +1748,35 @@ FunctionType::FunctionType(const EventDefinition& _event): swap(paramNames, m_parameterNames); } +FunctionTypePointer FunctionType::newExpressionType(ContractDefinition const& _contract) +{ + FunctionDefinition const* constructor = _contract.constructor(); + TypePointers parameters; + strings parameterNames; + bool payable = false; + + if (constructor) + { + for (ASTPointer<VariableDeclaration> const& var: constructor->parameters()) + { + parameterNames.push_back(var->name()); + parameters.push_back(var->annotation().type); + } + payable = constructor->isPayable(); + } + return make_shared<FunctionType>( + parameters, + TypePointers{make_shared<ContractType>(_contract)}, + parameterNames, + strings{""}, + Location::Creation, + false, + nullptr, + false, + payable + ); +} + vector<string> FunctionType::parameterNames() const { if (!bound()) @@ -1872,7 +1895,12 @@ FunctionTypePointer FunctionType::interfaceFunctionType() const if (variable && retParamTypes.empty()) return FunctionTypePointer(); - return make_shared<FunctionType>(paramTypes, retParamTypes, m_parameterNames, m_returnParameterNames, m_location, m_arbitraryParameters); + return make_shared<FunctionType>( + paramTypes, retParamTypes, + m_parameterNames, m_returnParameterNames, + m_location, m_arbitraryParameters, + m_declaration, m_isConstant, m_isPayable + ); } MemberList::MemberMap FunctionType::nativeMembers(ContractDefinition const*) const @@ -1891,7 +1919,7 @@ MemberList::MemberMap FunctionType::nativeMembers(ContractDefinition const*) con MemberList::MemberMap members; if (m_location != Location::BareDelegateCall && m_location != Location::DelegateCall) { - if (!m_declaration || m_isPayable) // If this is an actual function, it has to be payable + if (m_isPayable) members.push_back(MemberList::Member( "value", make_shared<FunctionType>( @@ -1902,6 +1930,8 @@ MemberList::MemberMap FunctionType::nativeMembers(ContractDefinition const*) con Location::SetValue, false, nullptr, + false, + false, m_gasSet, m_valueSet ) @@ -1918,6 +1948,8 @@ MemberList::MemberMap FunctionType::nativeMembers(ContractDefinition const*) con Location::SetGas, false, nullptr, + false, + false, m_gasSet, m_valueSet ) @@ -2023,6 +2055,8 @@ TypePointer FunctionType::copyAndSetGasOrValue(bool _setGas, bool _setValue) con m_location, m_arbitraryParameters, m_declaration, + m_isConstant, + m_isPayable, m_gasSet || _setGas, m_valueSet || _setValue, m_bound @@ -2068,6 +2102,8 @@ FunctionTypePointer FunctionType::asMemberFunction(bool _inLibrary, bool _bound) location, m_arbitraryParameters, m_declaration, + m_isConstant, + m_isPayable, m_gasSet, m_valueSet, _bound diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h index c57f8db1..9173f39a 100644 --- a/libsolidity/ast/Types.h +++ b/libsolidity/ast/Types.h @@ -640,9 +640,8 @@ public: bool isSuper() const { return m_super; } ContractDefinition const& contractDefinition() const { return m_contract; } - /// Returns the function type of the constructor. Note that the location part of the function type - /// is not used, as this type cannot be the type of a variable or expression. - FunctionTypePointer const& constructorType() const; + /// Returns the function type of the constructor modified to return an object of the contract's type. + FunctionTypePointer const& newExpressionType() const; /// @returns the identifier of the function with the given name or Invalid256 if such a name does /// not exist. @@ -820,21 +819,32 @@ public: explicit FunctionType(VariableDeclaration const& _varDecl); /// Creates the function type of an event. explicit FunctionType(EventDefinition const& _event); + /// Function type constructor to be used for a plain type (not derived from a declaration). FunctionType( strings const& _parameterTypes, strings const& _returnParameterTypes, Location _location = Location::Internal, - bool _arbitraryParameters = false + bool _arbitraryParameters = false, + bool _constant = false, + bool _payable = false ): FunctionType( parseElementaryTypeVector(_parameterTypes), parseElementaryTypeVector(_returnParameterTypes), strings(), strings(), _location, - _arbitraryParameters + _arbitraryParameters, + nullptr, + _constant, + _payable ) { } + + /// @returns the type of the "new Contract" function, i.e. basically the constructor. + static FunctionTypePointer newExpressionType(ContractDefinition const& _contract); + + /// Detailed constructor, use with care. FunctionType( TypePointers const& _parameterTypes, TypePointers const& _returnParameterTypes, @@ -843,6 +853,8 @@ public: Location _location = Location::Internal, bool _arbitraryParameters = false, Declaration const* _declaration = nullptr, + bool _isConstant = false, + bool _isPayable = false, bool _gasSet = false, bool _valueSet = false, bool _bound = false @@ -856,6 +868,8 @@ public: m_gasSet(_gasSet), m_valueSet(_valueSet), m_bound(_bound), + m_isConstant(_isConstant), + m_isPayable(_isPayable), m_declaration(_declaration) {} diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 58e6847c..33571bc0 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -242,6 +242,12 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac m_context << notFound; if (fallback) { + if (!fallback->isPayable()) + { + // Throw if function is not payable but call contained ether. + m_context << Instruction::CALLVALUE; + m_context.appendConditionalJumpTo(m_context.errorTag()); + } eth::AssemblyItem returnTag = m_context.pushNewTag(); fallback->accept(*this); m_context << returnTag; diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index 4a81e27d..de937b42 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -583,6 +583,8 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) Location::Bare, false, nullptr, + false, + false, true, true ), diff --git a/test/libsolidity/Assembly.cpp b/test/libsolidity/Assembly.cpp index 81332f4f..8d7a3540 100644 --- a/test/libsolidity/Assembly.cpp +++ b/test/libsolidity/Assembly.cpp @@ -115,7 +115,7 @@ BOOST_AUTO_TEST_CASE(location_test) AssemblyItems items = compileContract(sourceCode); vector<SourceLocation> locations = vector<SourceLocation>(18, SourceLocation(2, 75, n)) + - vector<SourceLocation>(28, SourceLocation(20, 72, n)) + + vector<SourceLocation>(31, SourceLocation(20, 72, n)) + vector<SourceLocation>{SourceLocation(42, 51, n), SourceLocation(65, 67, n)} + vector<SourceLocation>(4, SourceLocation(58, 67, n)) + vector<SourceLocation>(3, SourceLocation(20, 72, n)); diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index c1f1b148..006d41c2 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -1526,8 +1526,10 @@ BOOST_AUTO_TEST_CASE(convert_uint_to_fixed_bytes_greater_size) } })"; compileAndRun(sourceCode); - BOOST_CHECK(callContractFunction("UintToBytes(uint16)", u256("0x6162")) == - encodeArgs(string("\0\0\0\0\0\0ab", 8))); + BOOST_CHECK( + callContractFunction("UintToBytes(uint16)", u256("0x6162")) == + encodeArgs(string("\0\0\0\0\0\0ab", 8)) + ); } BOOST_AUTO_TEST_CASE(send_ether) @@ -2053,7 +2055,7 @@ BOOST_AUTO_TEST_CASE(contracts_as_addresses) { char const* sourceCode = R"( contract helper { - function() { } // can receive ether + function() payable { } // can receive ether } contract test { helper h; @@ -2065,6 +2067,7 @@ BOOST_AUTO_TEST_CASE(contracts_as_addresses) } )"; compileAndRun(sourceCode, 20); + BOOST_CHECK_EQUAL(balanceAt(m_contractAddress), 20 - 5); BOOST_REQUIRE(callContractFunction("getBalance()") == encodeArgs(u256(20 - 5), u256(5))); } @@ -2938,11 +2941,10 @@ BOOST_AUTO_TEST_CASE(generic_delegatecall) uint public received; address public sender; uint public value; - function sender() payable {} function doSend(address rec) payable { bytes4 signature = bytes4(bytes32(sha3("receive(uint256)"))); - rec.delegatecall(signature, 23); + if (rec.delegatecall(signature, 23)) {} } } )**"; @@ -5961,7 +5963,7 @@ BOOST_AUTO_TEST_CASE(reject_ether_sent_to_library) function f(address x) returns (bool) { return x.send(1); } - function () {} + function () payable {} } )"; compileAndRun(sourceCode, 0, "lib"); @@ -6879,7 +6881,7 @@ BOOST_AUTO_TEST_CASE(skip_dynamic_types_for_structs) BOOST_AUTO_TEST_CASE(failed_create) { char const* sourceCode = R"( - contract D { } + contract D { function D() payable {} } contract C { uint public x; function f(uint amount) returns (address) { @@ -7029,7 +7031,7 @@ BOOST_AUTO_TEST_CASE(mutex) else return fund.withdrawUnprotected(10); } - function() { + function() payable { callDepth++; if (callDepth < 4) attackInternal(); @@ -7104,55 +7106,47 @@ BOOST_AUTO_TEST_CASE(payable_function) { char const* sourceCode = R"( contract C { + uint public a; function f() payable returns (uint) { return msg.value; } - function() payable returns (uint) { - return msg.value; + function() payable { + a = msg.value + 1; } } )"; compileAndRun(sourceCode, 0, "C"); BOOST_CHECK(callContractFunctionWithValue("f()", 27) == encodeArgs(u256(27))); - BOOST_CHECK(callContractFunctionWithValue("", 27) == encodeArgs(u256(27))); -} - -BOOST_AUTO_TEST_CASE(non_payable_throw_constructor) -{ - char const* sourceCode = R"( - contract C { - function C() { } - } - contract D { - function D() payable {} - function f() returns (uint) { - (new C).value(2)(); - return 2; - } - } - )"; - compileAndRun(sourceCode, 27, "D"); - BOOST_CHECK(callContractFunction("f()") == encodeArgs()); BOOST_CHECK_EQUAL(balanceAt(m_contractAddress), 27); + BOOST_CHECK(callContractFunctionWithValue("", 27) == encodeArgs()); + BOOST_CHECK_EQUAL(balanceAt(m_contractAddress), 27 + 27); + BOOST_CHECK(callContractFunction("a()") == encodeArgs(u256(28))); + BOOST_CHECK_EQUAL(balanceAt(m_contractAddress), 27 + 27); } BOOST_AUTO_TEST_CASE(non_payable_throw) { char const* sourceCode = R"( contract C { - string public a; + uint public a; function f() returns (uint) { return msg.value; } - function() returns (uint) { - return msg.value; + function() { + a = msg.value + 1; } } )"; compileAndRun(sourceCode, 0, "C"); BOOST_CHECK(callContractFunctionWithValue("f()", 27) == encodeArgs()); + BOOST_CHECK_EQUAL(balanceAt(m_contractAddress), 0); + BOOST_CHECK(callContractFunction("") == encodeArgs()); + BOOST_CHECK(callContractFunction("a()") == encodeArgs(u256(1))); BOOST_CHECK(callContractFunctionWithValue("", 27) == encodeArgs()); + BOOST_CHECK_EQUAL(balanceAt(m_contractAddress), 0); + BOOST_CHECK(callContractFunction("a()") == encodeArgs(u256(1))); BOOST_CHECK(callContractFunctionWithValue("a()", 27) == encodeArgs()); + BOOST_CHECK_EQUAL(balanceAt(m_contractAddress), 0); } BOOST_AUTO_TEST_CASE(no_nonpayable_circumvention_by_modifier) @@ -7169,6 +7163,7 @@ BOOST_AUTO_TEST_CASE(no_nonpayable_circumvention_by_modifier) )"; compileAndRun(sourceCode); BOOST_CHECK(callContractFunctionWithValue("f()", 27) == encodeArgs()); + BOOST_CHECK_EQUAL(balanceAt(m_contractAddress), 0); } diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index e193745d..3adf4612 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -3911,7 +3911,23 @@ BOOST_AUTO_TEST_CASE(calling_nonpayable) char const* text = R"( contract receiver { function nopay() {} } contract test { - function_argument_mem_to_storage f() { (new receiver()).nopay.value(10)(); } + function f() { (new receiver()).nopay.value(10)(); } + } + )"; + BOOST_CHECK(expectError(text) == Error::Type::TypeError); +} + +BOOST_AUTO_TEST_CASE(non_payable_constructor) +{ + char const* text = R"( + contract C { + function C() { } + } + contract D { + function f() returns (uint) { + (new C).value(2)(); + return 2; + } } )"; BOOST_CHECK(expectError(text) == Error::Type::TypeError); |