From a34f2f1a316d6093c14045ee8423d913ac01421d Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Sat, 18 Jun 2016 11:11:55 +0100 Subject: Support payable keyword for functions --- libsolidity/ast/AST.h | 4 ++++ libsolidity/ast/Types.cpp | 1 + libsolidity/ast/Types.h | 2 ++ libsolidity/codegen/ContractCompiler.cpp | 17 +++++++++++++++++ libsolidity/interface/InterfaceHandler.cpp | 2 ++ libsolidity/parsing/Parser.cpp | 7 +++++++ libsolidity/parsing/Token.h | 2 +- 7 files changed, 34 insertions(+), 1 deletion(-) diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h index 761d85fe..8fd1584d 100644 --- a/libsolidity/ast/AST.h +++ b/libsolidity/ast/AST.h @@ -540,6 +540,7 @@ public: bool _isDeclaredConst, std::vector> const& _modifiers, ASTPointer const& _returnParameters, + bool _isPayable, ASTPointer const& _body ): CallableDeclaration(_location, _name, _visibility, _parameters, _returnParameters), @@ -547,6 +548,7 @@ public: ImplementationOptional(_body != nullptr), m_isConstructor(_isConstructor), m_isDeclaredConst(_isDeclaredConst), + m_isPayable(_isPayable), m_functionModifiers(_modifiers), m_body(_body) {} @@ -556,6 +558,7 @@ public: bool isConstructor() const { return m_isConstructor; } bool isDeclaredConst() const { return m_isDeclaredConst; } + bool isPayable() const { return m_isPayable; } std::vector> const& modifiers() const { return m_functionModifiers; } std::vector> const& returnParameters() const { return m_returnParameters->parameters(); } Block const& body() const { return *m_body; } @@ -578,6 +581,7 @@ public: private: bool m_isConstructor; bool m_isDeclaredConst; + bool m_isPayable; std::vector> m_functionModifiers; ASTPointer m_body; }; diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index d86a2caf..938dced6 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -1653,6 +1653,7 @@ TypePointer TupleType::closestTemporaryType(TypePointer const& _targetType) cons FunctionType::FunctionType(FunctionDefinition const& _function, bool _isInternal): m_location(_isInternal ? Location::Internal : Location::External), m_isConstant(_function.isDeclaredConst()), + m_isPayable(_function.isPayable()), m_declaration(&_function) { TypePointers params; diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h index 1282e5d8..c57f8db1 100644 --- a/libsolidity/ast/Types.h +++ b/libsolidity/ast/Types.h @@ -905,6 +905,7 @@ public: } bool hasDeclaration() const { return !!m_declaration; } bool isConstant() const { return m_isConstant; } + bool isPayable() const { return m_isPayable; } /// @return A shared pointer of an ASTString. /// Can contain a nullptr in which case indicates absence of documentation ASTPointer documentation() const; @@ -942,6 +943,7 @@ private: bool const m_valueSet = false; ///< true iff the value to be sent is on the stack bool const m_bound = false; ///< true iff the function is called as arg1.fun(arg2, ..., argn) bool m_isConstant = false; + bool m_isPayable = false; Declaration const* m_declaration = nullptr; }; diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 9d77ccdc..eba85103 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -243,6 +243,14 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac if (fallback) { eth::AssemblyItem returnTag = m_context.pushNewTag(); + + // Reject transaction if value is not accepted, but was received + if (!fallback->isPayable()) + { + m_context << Instruction::CALLVALUE; + m_context.appendConditionalJumpTo(m_context.errorTag()); + } + fallback->accept(*this); m_context << returnTag; appendReturnValuePacker(FunctionType(*fallback).returnParameterTypes(), _contract.isLibrary()); @@ -255,8 +263,17 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac FunctionTypePointer const& functionType = it.second; solAssert(functionType->hasDeclaration(), ""); CompilerContext::LocationSetter locationSetter(m_context, functionType->declaration()); + m_context << callDataUnpackerEntryPoints.at(it.first); eth::AssemblyItem returnTag = m_context.pushNewTag(); + + // Reject transaction if value is not accepted, but was received + if (!functionType->isPayable()) + { + m_context << Instruction::CALLVALUE; + m_context.appendConditionalJumpTo(m_context.errorTag()); + } + m_context << CompilerUtils::dataStartOffset; appendCalldataUnpacker(functionType->parameterTypes()); m_context.appendJumpTo(m_context.functionEntryLabel(functionType->declaration())); diff --git a/libsolidity/interface/InterfaceHandler.cpp b/libsolidity/interface/InterfaceHandler.cpp index d39f8285..017753fc 100644 --- a/libsolidity/interface/InterfaceHandler.cpp +++ b/libsolidity/interface/InterfaceHandler.cpp @@ -52,6 +52,7 @@ string InterfaceHandler::abiInterface(ContractDefinition const& _contractDef) method["type"] = "function"; method["name"] = it.second->declaration().name(); method["constant"] = it.second->isConstant(); + method["payable"] = it.second->isPayable(); method["inputs"] = populateParameters( externalFunctionType->parameterNames(), externalFunctionType->parameterTypeNames(_contractDef.isLibrary()) @@ -72,6 +73,7 @@ string InterfaceHandler::abiInterface(ContractDefinition const& _contractDef) externalFunction->parameterNames(), externalFunction->parameterTypeNames(_contractDef.isLibrary()) ); + method["payable"] = externalFunction->isPayable(); abi.append(method); } if (_contractDef.fallbackFunction()) diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp index 1bffa5d7..1d76f80a 100644 --- a/libsolidity/parsing/Parser.cpp +++ b/libsolidity/parsing/Parser.cpp @@ -326,6 +326,12 @@ ASTPointer Parser::parseFunctionDefinition(ASTString const* else break; } + bool isPayable = false; + if (m_scanner->currentToken() == Token::Payable) + { + isPayable = true; + m_scanner->next(); + } ASTPointer returnParameters; if (m_scanner->currentToken() == Token::Returns) { @@ -354,6 +360,7 @@ ASTPointer Parser::parseFunctionDefinition(ASTString const* isDeclaredConst, modifiers, returnParameters, + isPayable, block ); } diff --git a/libsolidity/parsing/Token.h b/libsolidity/parsing/Token.h index 15d4860f..cc85b610 100644 --- a/libsolidity/parsing/Token.h +++ b/libsolidity/parsing/Token.h @@ -166,6 +166,7 @@ namespace solidity K(Memory, "memory", 0) \ K(Modifier, "modifier", 0) \ K(New, "new", 0) \ + K(Payable, "payable", 0) \ K(Public, "public", 0) \ K(Pragma, "pragma", 0) \ K(Private, "private", 0) \ @@ -229,7 +230,6 @@ namespace solidity K(Let, "let", 0) \ K(Match, "match", 0) \ K(Of, "of", 0) \ - K(Payable, "payable", 0) \ K(Relocatable, "relocatable", 0) \ K(Static, "static", 0) \ K(Switch, "switch", 0) \ -- cgit v1.2.3 From a7794b1a682d6b13bbe58e8164853d481ffd51f5 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Tue, 2 Aug 2016 20:22:26 +0100 Subject: Include ABI JSON test for payable keyword --- test/libsolidity/SolidityABIJSON.cpp | 49 ++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/test/libsolidity/SolidityABIJSON.cpp b/test/libsolidity/SolidityABIJSON.cpp index ec621104..9ee74d41 100644 --- a/test/libsolidity/SolidityABIJSON.cpp +++ b/test/libsolidity/SolidityABIJSON.cpp @@ -68,6 +68,7 @@ BOOST_AUTO_TEST_CASE(basic_test) { "name": "f", "constant": false, + "payable" : false, "type": "function", "inputs": [ { @@ -107,6 +108,7 @@ BOOST_AUTO_TEST_CASE(multiple_methods) { "name": "f", "constant": false, + "payable" : false, "type": "function", "inputs": [ { @@ -124,6 +126,7 @@ BOOST_AUTO_TEST_CASE(multiple_methods) { "name": "g", "constant": false, + "payable" : false, "type": "function", "inputs": [ { @@ -153,6 +156,7 @@ BOOST_AUTO_TEST_CASE(multiple_params) { "name": "f", "constant": false, + "payable" : false, "type": "function", "inputs": [ { @@ -188,6 +192,7 @@ BOOST_AUTO_TEST_CASE(multiple_methods_order) { "name": "c", "constant": false, + "payable" : false, "type": "function", "inputs": [ { @@ -205,6 +210,7 @@ BOOST_AUTO_TEST_CASE(multiple_methods_order) { "name": "f", "constant": false, + "payable" : false, "type": "function", "inputs": [ { @@ -235,6 +241,7 @@ BOOST_AUTO_TEST_CASE(const_function) { "name": "foo", "constant": false, + "payable" : false, "type": "function", "inputs": [ { @@ -256,6 +263,7 @@ BOOST_AUTO_TEST_CASE(const_function) { "name": "boo", "constant": true, + "payable" : false, "type": "function", "inputs": [{ "name": "a", @@ -284,6 +292,7 @@ BOOST_AUTO_TEST_CASE(events) { "name": "f", "constant": false, + "payable" : false, "type": "function", "inputs": [ { @@ -361,6 +370,7 @@ BOOST_AUTO_TEST_CASE(inherited) { "name": "baseFunction", "constant": false, + "payable" : false, "type": "function", "inputs": [{ @@ -376,6 +386,7 @@ BOOST_AUTO_TEST_CASE(inherited) { "name": "derivedFunction", "constant": false, + "payable" : false, "type": "function", "inputs": [{ @@ -429,6 +440,7 @@ BOOST_AUTO_TEST_CASE(empty_name_input_parameter_with_named_one) { "name": "f", "constant": false, + "payable" : false, "type": "function", "inputs": [ { @@ -469,6 +481,7 @@ BOOST_AUTO_TEST_CASE(empty_name_return_parameter) { "name": "f", "constant": false, + "payable" : false, "type": "function", "inputs": [ { @@ -536,6 +549,7 @@ BOOST_AUTO_TEST_CASE(return_param_in_abi) [ { "constant" : false, + "payable" : false, "inputs" : [], "name" : "ret", "outputs" : [ @@ -573,6 +587,7 @@ BOOST_AUTO_TEST_CASE(strings_and_arrays) [ { "constant" : false, + "payable" : false, "name": "f", "inputs": [ { "name": "a", "type": "string" }, @@ -600,6 +615,7 @@ BOOST_AUTO_TEST_CASE(library_function) [ { "constant" : false, + "payable" : false, "name": "f", "inputs": [ { "name": "b", "type": "test.StructType storage" }, @@ -636,6 +652,39 @@ BOOST_AUTO_TEST_CASE(include_fallback_function) checkInterface(sourceCode, interface); } +BOOST_AUTO_TEST_CASE(payable_function) +{ + char const* sourceCode = R"( + contract test { + function f() {} + function g() payable {} + } + )"; + + char const* interface = R"( + [ + { + "constant" : false, + "payable": false, + "inputs": [], + "name": "f", + "outputs": [], + "type" : "function" + }, + { + "constant" : false, + "payable": true, + "inputs": [], + "name": "g", + "outputs": [], + "type" : "function" + } + ] + )"; + checkInterface(sourceCode, interface); +} + + BOOST_AUTO_TEST_SUITE_END() } -- cgit v1.2.3 From 75d556a2cfb6ae043b6d9fa375e4a6410145e4d1 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Tue, 2 Aug 2016 20:36:54 +0100 Subject: Do not include the payable keyword for constructors --- libsolidity/interface/InterfaceHandler.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/libsolidity/interface/InterfaceHandler.cpp b/libsolidity/interface/InterfaceHandler.cpp index 017753fc..a536b0bc 100644 --- a/libsolidity/interface/InterfaceHandler.cpp +++ b/libsolidity/interface/InterfaceHandler.cpp @@ -73,7 +73,6 @@ string InterfaceHandler::abiInterface(ContractDefinition const& _contractDef) externalFunction->parameterNames(), externalFunction->parameterTypeNames(_contractDef.isLibrary()) ); - method["payable"] = externalFunction->isPayable(); abi.append(method); } if (_contractDef.fallbackFunction()) -- cgit v1.2.3 From 34a6afbd77f499ca4883a62a8de642db1c062e53 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Thu, 4 Aug 2016 16:47:46 +0100 Subject: Include EndToEnd test for payable keyword --- test/libsolidity/SolidityEndToEndTest.cpp | 57 +++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index f3fdef15..f855c2a3 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -7087,6 +7087,63 @@ BOOST_AUTO_TEST_CASE(calling_nonexisting_contract_throws) BOOST_CHECK(callContractFunction("h()") == encodeArgs(u256(7))); } +BOOST_AUTO_TEST_CASE(payable_accept_explicit_constructor) +{ + char const* sourceCode = R"( + contract C { + function () payable { } + } + )"; + compileAndRun(sourceCode, 27, "C"); +} + + +BOOST_AUTO_TEST_CASE(payable_accept_explicit) +{ + char const* sourceCode = R"( + contract C { + function f() payable returns (uint) { + return msg.value; + } + function() payable returns (uint) { + return msg.value; + } + } + )"; + 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() { } + } + )"; + compileAndRun(sourceCode, 27, "C"); +} + +BOOST_AUTO_TEST_CASE(non_payable_throw) +{ + char const* sourceCode = R"( + contract C { + string public a; + function f() returns (uint) { + return msg.value; + } + function() returns (uint) { + return msg.value; + } + } + )"; + compileAndRun(sourceCode, 0, "C"); + BOOST_CHECK(callContractFunctionWithValue("f()", 27) == encodeArgs()); + BOOST_CHECK(callContractFunctionWithValue("", 27) == encodeArgs()); + BOOST_CHECK(callContractFunctionWithValue("a()", 27) == encodeArgs()); +} + BOOST_AUTO_TEST_SUITE_END() } -- cgit v1.2.3 From 680b83b2a44a8f943d6d78028ad4326f4b3b64c2 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Fri, 5 Aug 2016 19:48:10 +0100 Subject: Mark every other test payable where neccesary in EndToEndTest --- test/libsolidity/SolidityEndToEndTest.cpp | 55 ++++++++++++++++--------------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index f855c2a3..65f81dea 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -1320,7 +1320,7 @@ BOOST_AUTO_TEST_CASE(balance) BOOST_AUTO_TEST_CASE(blockchain) { char const* sourceCode = "contract test {\n" - " function someInfo() returns (uint256 value, address coinbase, uint256 blockNumber) {\n" + " function someInfo() payable returns (uint256 value, address coinbase, uint256 blockNumber) {\n" " value = msg.value;\n" " coinbase = block.coinbase;\n" " blockNumber = block.number;\n" @@ -1342,7 +1342,7 @@ BOOST_AUTO_TEST_CASE(msg_sig) } )"; compileAndRun(sourceCode); - BOOST_CHECK(callContractFunctionWithValue("foo(uint256)", 13) == encodeArgs(asString(FixedHash<4>(dev::sha3("foo(uint256)")).asBytes()))); + BOOST_CHECK(callContractFunction("foo(uint256)") == encodeArgs(asString(FixedHash<4>(dev::sha3("foo(uint256)")).asBytes()))); } BOOST_AUTO_TEST_CASE(msg_sig_after_internal_call_is_same) @@ -1358,7 +1358,7 @@ BOOST_AUTO_TEST_CASE(msg_sig_after_internal_call_is_same) } )"; compileAndRun(sourceCode); - BOOST_CHECK(callContractFunctionWithValue("foo(uint256)", 13) == encodeArgs(asString(FixedHash<4>(dev::sha3("foo(uint256)")).asBytes()))); + BOOST_CHECK(callContractFunction("foo(uint256)") == encodeArgs(asString(FixedHash<4>(dev::sha3("foo(uint256)")).asBytes()))); } BOOST_AUTO_TEST_CASE(now) @@ -2057,7 +2057,7 @@ BOOST_AUTO_TEST_CASE(contracts_as_addresses) } contract test { helper h; - function test() { h = new helper(); h.send(5); } + function test() payable { h = new helper(); h.send(5); } function getBalance() returns (uint256 myBalance, uint256 helperBalance) { myBalance = this.balance; helperBalance = h.balance; @@ -2073,7 +2073,7 @@ BOOST_AUTO_TEST_CASE(gas_and_value_basic) char const* sourceCode = R"( contract helper { bool flag; - function getBalance() returns (uint256 myBalance) { + function getBalance() payable returns (uint256 myBalance) { return this.balance; } function setFlag() { flag = true; } @@ -2081,15 +2081,15 @@ BOOST_AUTO_TEST_CASE(gas_and_value_basic) } contract test { helper h; - function test() { h = new helper(); } - function sendAmount(uint amount) returns (uint256 bal) { + function test() payable { h = new helper(); } + function sendAmount(uint amount) payable returns (uint256 bal) { return h.getBalance.value(amount)(); } - function outOfGas() returns (bool ret) { + function outOfGas() payable returns (bool ret) { h.setFlag.gas(2)(); // should fail due to OOG return true; } - function checkState() returns (bool flagAfter, uint myBal) { + function checkState() payable returns (bool flagAfter, uint myBal) { flagAfter = h.getFlag(); myBal = this.balance; } @@ -2106,7 +2106,7 @@ BOOST_AUTO_TEST_CASE(gas_for_builtin) { char const* sourceCode = R"( contract Contract { - function test(uint g) returns (bytes32 data, bool flag) { + function test(uint g) payable returns (bytes32 data, bool flag) { data = ripemd160.gas(g)("abc"); flag = true; } @@ -2121,14 +2121,14 @@ BOOST_AUTO_TEST_CASE(value_complex) { char const* sourceCode = R"( contract helper { - function getBalance() returns (uint256 myBalance) { + function getBalance() payable returns (uint256 myBalance) { return this.balance; } } contract test { helper h; - function test() { h = new helper(); } - function sendAmount(uint amount) returns (uint256 bal) { + function test() payable { h = new helper(); } + function sendAmount(uint amount) payable returns (uint256 bal) { var x1 = h.getBalance.value(amount); uint someStackElement = 20; var x2 = x1.gas(1000); @@ -2144,14 +2144,14 @@ BOOST_AUTO_TEST_CASE(value_insane) { char const* sourceCode = R"( contract helper { - function getBalance() returns (uint256 myBalance) { + function getBalance() payable returns (uint256 myBalance) { return this.balance; } } contract test { helper h; - function test() { h = new helper(); } - function sendAmount(uint amount) returns (uint256 bal) { + function test() payable { h = new helper(); } + function sendAmount(uint amount) payable returns (uint256 bal) { var x1 = h.getBalance.value; var x2 = x1(amount).gas; var x3 = x2(1000).value; @@ -2169,7 +2169,7 @@ BOOST_AUTO_TEST_CASE(value_for_constructor) contract Helper { bytes3 name; bool flag; - function Helper(bytes3 x, bool f) { + function Helper(bytes3 x, bool f) payable { name = x; flag = f; } @@ -2178,7 +2178,7 @@ BOOST_AUTO_TEST_CASE(value_for_constructor) } contract Main { Helper h; - function Main() { + function Main() payable { h = (new Helper).value(10)("abc", true); } function getFlag() returns (bool ret) { return h.getFlag(); } @@ -2352,7 +2352,7 @@ BOOST_AUTO_TEST_CASE(function_modifier) { char const* sourceCode = R"( contract C { - function getOne() nonFree returns (uint r) { return 1; } + function getOne() payable nonFree returns (uint r) { return 1; } modifier nonFree { if (msg.value > 0) _; } } )"; @@ -2559,7 +2559,7 @@ BOOST_AUTO_TEST_CASE(event) char const* sourceCode = R"( contract ClientReceipt { event Deposit(address indexed _from, bytes32 indexed _id, uint _value); - function deposit(bytes32 _id, bool _manually) { + function deposit(bytes32 _id, bool _manually) payable { if (_manually) { bytes32 s = 0x19dacbf83c5de6658e14cbf7bcae5c15eca2eedecf1c66fbca928e4d351bea0f; log3(bytes32(msg.value), s, bytes32(msg.sender), _id); @@ -2624,7 +2624,7 @@ BOOST_AUTO_TEST_CASE(event_anonymous_with_topics) char const* sourceCode = R"( contract ClientReceipt { event Deposit(address indexed _from, bytes32 indexed _id, uint indexed _value, uint indexed _value2, bytes32 data) anonymous; - function deposit(bytes32 _id, bool _manually) { + function deposit(bytes32 _id, bool _manually) payable { Deposit(msg.sender, _id, msg.value, 2, "abc"); } } @@ -2648,7 +2648,7 @@ BOOST_AUTO_TEST_CASE(event_lots_of_data) char const* sourceCode = R"( contract ClientReceipt { event Deposit(address _from, bytes32 _id, uint _value, bool _flag); - function deposit(bytes32 _id) { + function deposit(bytes32 _id) payable { Deposit(msg.sender, _id, msg.value, true); } } @@ -2874,9 +2874,10 @@ BOOST_AUTO_TEST_CASE(generic_call) char const* sourceCode = R"**( contract receiver { uint public received; - function receive(uint256 x) { received = x; } + function receive(uint256 x) payable { received = x; } } contract sender { + function sender() payable {} function doSend(address rec) returns (uint d) { bytes4 signature = bytes4(bytes32(sha3("receive(uint256)"))); @@ -2897,10 +2898,11 @@ BOOST_AUTO_TEST_CASE(generic_callcode) char const* sourceCode = R"**( contract receiver { uint public received; - function receive(uint256 x) { received = x; } + function receive(uint256 x) payable { received = x; } } contract sender { uint public received; + function sender() payable { } function doSend(address rec) returns (uint d) { bytes4 signature = bytes4(bytes32(sha3("receive(uint256)"))); @@ -2930,13 +2932,14 @@ BOOST_AUTO_TEST_CASE(generic_delegatecall) uint public received; address public sender; uint public value; - function receive(uint256 x) { received = x; sender = msg.sender; value = msg.value; } + function receive(uint256 x) payable { received = x; sender = msg.sender; value = msg.value; } } contract sender { uint public received; address public sender; uint public value; - function doSend(address rec) + function sender() payable {} + function doSend(address rec) payable { bytes4 signature = bytes4(bytes32(sha3("receive(uint256)"))); rec.delegatecall(signature, 23); -- cgit v1.2.3 From 962531af96a8a3ed6b28462d43c69d78fa48d511 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Fri, 26 Aug 2016 19:37:10 +0100 Subject: Merged in changes from chriseth/payable --- libsolidity/analysis/TypeChecker.cpp | 14 ++++- libsolidity/ast/Types.cpp | 31 ++++++----- libsolidity/codegen/ContractCompiler.cpp | 13 +---- libsolidity/parsing/Parser.cpp | 12 ++-- test/contracts/AuctionRegistrar.cpp | 7 +-- test/contracts/FixedFeeRegistrar.cpp | 2 +- test/contracts/Wallet.cpp | 2 +- test/libsolidity/SolidityEndToEndTest.cpp | 49 +++++++++++++---- test/libsolidity/SolidityNameAndTypeResolution.cpp | 64 +++++++++++++++++++++- test/libsolidity/SolidityParser.cpp | 10 ++++ 10 files changed, 150 insertions(+), 54 deletions(-) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index bc03da01..c1781397 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -272,6 +272,7 @@ void TypeChecker::checkContractIllegalOverrides(ContractDefinition const& _contr if ( overriding->visibility() != function->visibility() || overriding->isDeclaredConst() != function->isDeclaredConst() || + overriding->isPayable() != function->isPayable() || overridingType != functionType ) typeError(overriding->location(), "Override changes extended function signature."); @@ -416,6 +417,13 @@ bool TypeChecker::visit(StructDefinition const& _struct) bool TypeChecker::visit(FunctionDefinition const& _function) { bool isLibraryFunction = dynamic_cast(*_function.scope()).isLibrary(); + if (_function.isPayable()) + { + if (isLibraryFunction) + typeError(_function.location(), "Library functions cannot be payable."); + if (!_function.isConstructor() && !_function.name().empty() && !_function.isPartOfExternalInterface()) + typeError(_function.location(), "Internal functions cannot be payable."); + } for (ASTPointer const& var: _function.parameters() + _function.returnParameters()) { if (!type(*var)->canLiveOutsideStorage()) @@ -1328,14 +1336,16 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess) fatalTypeError( _memberAccess.location(), "Member \"" + memberName + "\" not found or not visible " - "after argument-dependent lookup in " + exprType->toString() + "after argument-dependent lookup in " + exprType->toString() + + (memberName == "value" ? " - did you forget the \"payable\" modifier?" : "") ); } else if (possibleMembers.size() > 1) fatalTypeError( _memberAccess.location(), "Member \"" + memberName + "\" not unique " - "after argument-dependent lookup in " + exprType->toString() + "after argument-dependent lookup in " + exprType->toString() + + (memberName == "value" ? " - did you forget the \"payable\" modifier?" : "") ); auto& annotation = _memberAccess.annotation(); diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index 938dced6..ca8490f2 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -1890,20 +1890,23 @@ MemberList::MemberMap FunctionType::nativeMembers(ContractDefinition const*) con { MemberList::MemberMap members; if (m_location != Location::BareDelegateCall && m_location != Location::DelegateCall) - members.push_back(MemberList::Member( - "value", - make_shared( - parseElementaryTypeVector({"uint"}), - TypePointers{copyAndSetGasOrValue(false, true)}, - strings(), - strings(), - Location::SetValue, - false, - nullptr, - m_gasSet, - m_valueSet - ) - )); + { + if (!m_declaration || m_isPayable) // If this is an actual function, it has to be payable + members.push_back(MemberList::Member( + "value", + make_shared( + parseElementaryTypeVector({"uint"}), + TypePointers{copyAndSetGasOrValue(false, true)}, + strings(), + strings(), + Location::SetValue, + false, + nullptr, + m_gasSet, + m_valueSet + ) + )); + } if (m_location != Location::Creation) members.push_back(MemberList::Member( "gas", diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index eba85103..58e6847c 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -243,14 +243,6 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac if (fallback) { eth::AssemblyItem returnTag = m_context.pushNewTag(); - - // Reject transaction if value is not accepted, but was received - if (!fallback->isPayable()) - { - m_context << Instruction::CALLVALUE; - m_context.appendConditionalJumpTo(m_context.errorTag()); - } - fallback->accept(*this); m_context << returnTag; appendReturnValuePacker(FunctionType(*fallback).returnParameterTypes(), _contract.isLibrary()); @@ -265,15 +257,14 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac CompilerContext::LocationSetter locationSetter(m_context, functionType->declaration()); m_context << callDataUnpackerEntryPoints.at(it.first); - eth::AssemblyItem returnTag = m_context.pushNewTag(); - - // Reject transaction if value is not accepted, but was received if (!functionType->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(); m_context << CompilerUtils::dataStartOffset; appendCalldataUnpacker(functionType->parameterTypes()); m_context.appendJumpTo(m_context.functionEntryLabel(functionType->declaration())); diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp index 1d76f80a..0e99d1e7 100644 --- a/libsolidity/parsing/Parser.cpp +++ b/libsolidity/parsing/Parser.cpp @@ -305,6 +305,7 @@ ASTPointer Parser::parseFunctionDefinition(ASTString const* options.allowLocationSpecifier = true; ASTPointer parameters(parseParameterList(options)); bool isDeclaredConst = false; + bool isPayable = false; Declaration::Visibility visibility(Declaration::Visibility::Default); vector> modifiers; while (true) @@ -315,6 +316,11 @@ ASTPointer Parser::parseFunctionDefinition(ASTString const* isDeclaredConst = true; m_scanner->next(); } + else if (m_scanner->currentToken() == Token::Payable) + { + isPayable = true; + m_scanner->next(); + } else if (token == Token::Identifier) modifiers.push_back(parseModifierInvocation()); else if (Token::isVisibilitySpecifier(token)) @@ -326,12 +332,6 @@ ASTPointer Parser::parseFunctionDefinition(ASTString const* else break; } - bool isPayable = false; - if (m_scanner->currentToken() == Token::Payable) - { - isPayable = true; - m_scanner->next(); - } ASTPointer returnParameters; if (m_scanner->currentToken() == Token::Returns) { diff --git a/test/contracts/AuctionRegistrar.cpp b/test/contracts/AuctionRegistrar.cpp index 9161f2ce..05da3490 100644 --- a/test/contracts/AuctionRegistrar.cpp +++ b/test/contracts/AuctionRegistrar.cpp @@ -115,11 +115,6 @@ contract GlobalRegistrar is Registrar, AuctionSystem { // TODO: Populate with hall-of-fame. } - function() { - // prevent people from just sending funds to the registrar - throw; - } - function onAuctionEnd(string _name) internal { var auction = m_auctions[_name]; var record = m_toRecord[_name]; @@ -136,7 +131,7 @@ contract GlobalRegistrar is Registrar, AuctionSystem { } } - function reserve(string _name) external { + function reserve(string _name) external payable { if (bytes(_name).length == 0) throw; bool needAuction = requiresAuction(_name); diff --git a/test/contracts/FixedFeeRegistrar.cpp b/test/contracts/FixedFeeRegistrar.cpp index c37873cf..af8ee595 100644 --- a/test/contracts/FixedFeeRegistrar.cpp +++ b/test/contracts/FixedFeeRegistrar.cpp @@ -73,7 +73,7 @@ contract FixedFeeRegistrar is Registrar { modifier onlyrecordowner(string _name) { if (m_record(_name).owner == msg.sender) _; } - function reserve(string _name) { + function reserve(string _name) payable { Record rec = m_record(_name); if (rec.owner == 0 && msg.value >= c_fee) { rec.owner = msg.sender; diff --git a/test/contracts/Wallet.cpp b/test/contracts/Wallet.cpp index d3e1b8d7..b4f29a87 100644 --- a/test/contracts/Wallet.cpp +++ b/test/contracts/Wallet.cpp @@ -378,7 +378,7 @@ contract Wallet is multisig, multiowned, daylimit { } // gets called when no other function matches - function() { + function() payable { // just being sent some cash? if (msg.value > 0) Deposit(msg.sender, msg.value); diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 65f81dea..c1f1b148 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -2085,11 +2085,11 @@ BOOST_AUTO_TEST_CASE(gas_and_value_basic) function sendAmount(uint amount) payable returns (uint256 bal) { return h.getBalance.value(amount)(); } - function outOfGas() payable returns (bool ret) { + function outOfGas() returns (bool ret) { h.setFlag.gas(2)(); // should fail due to OOG return true; } - function checkState() payable returns (bool flagAfter, uint myBal) { + function checkState() returns (bool flagAfter, uint myBal) { flagAfter = h.getFlag(); myBal = this.balance; } @@ -2098,15 +2098,15 @@ BOOST_AUTO_TEST_CASE(gas_and_value_basic) compileAndRun(sourceCode, 20); BOOST_REQUIRE(callContractFunction("sendAmount(uint256)", 5) == encodeArgs(5)); // call to helper should not succeed but amount should be transferred anyway - BOOST_REQUIRE(callContractFunction("outOfGas()", 5) == bytes()); - BOOST_REQUIRE(callContractFunction("checkState()", 5) == encodeArgs(false, 20 - 5)); + BOOST_REQUIRE(callContractFunction("outOfGas()") == bytes()); + BOOST_REQUIRE(callContractFunction("checkState()") == encodeArgs(false, 20 - 5)); } BOOST_AUTO_TEST_CASE(gas_for_builtin) { char const* sourceCode = R"( contract Contract { - function test(uint g) payable returns (bytes32 data, bool flag) { + function test(uint g) returns (bytes32 data, bool flag) { data = ripemd160.gas(g)("abc"); flag = true; } @@ -2151,7 +2151,7 @@ BOOST_AUTO_TEST_CASE(value_insane) contract test { helper h; function test() payable { h = new helper(); } - function sendAmount(uint amount) payable returns (uint256 bal) { + function sendAmount(uint amount) returns (uint256 bal) { var x1 = h.getBalance.value; var x2 = x1(amount).gas; var x3 = x2(1000).value; @@ -7090,18 +7090,17 @@ BOOST_AUTO_TEST_CASE(calling_nonexisting_contract_throws) BOOST_CHECK(callContractFunction("h()") == encodeArgs(u256(7))); } -BOOST_AUTO_TEST_CASE(payable_accept_explicit_constructor) +BOOST_AUTO_TEST_CASE(payable_constructor) { char const* sourceCode = R"( contract C { - function () payable { } + function C() payable { } } )"; compileAndRun(sourceCode, 27, "C"); } - -BOOST_AUTO_TEST_CASE(payable_accept_explicit) +BOOST_AUTO_TEST_CASE(payable_function) { char const* sourceCode = R"( contract C { @@ -7122,10 +7121,19 @@ BOOST_AUTO_TEST_CASE(non_payable_throw_constructor) { char const* sourceCode = R"( contract C { - function() { } + function C() { } + } + contract D { + function D() payable {} + function f() returns (uint) { + (new C).value(2)(); + return 2; + } } )"; - compileAndRun(sourceCode, 27, "C"); + compileAndRun(sourceCode, 27, "D"); + BOOST_CHECK(callContractFunction("f()") == encodeArgs()); + BOOST_CHECK_EQUAL(balanceAt(m_contractAddress), 27); } BOOST_AUTO_TEST_CASE(non_payable_throw) @@ -7147,6 +7155,23 @@ BOOST_AUTO_TEST_CASE(non_payable_throw) BOOST_CHECK(callContractFunctionWithValue("a()", 27) == encodeArgs()); } +BOOST_AUTO_TEST_CASE(no_nonpayable_circumvention_by_modifier) +{ + char const* sourceCode = R"( + contract C { + modifier tryCircumvent { + if (false) _ // avoid the function, we should still not accept ether + } + function f() tryCircumvent returns (uint) { + return msg.value; + } + } + )"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunctionWithValue("f()", 27) == encodeArgs()); +} + + BOOST_AUTO_TEST_SUITE_END() } diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 8d4890d1..e193745d 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -3852,7 +3852,69 @@ BOOST_AUTO_TEST_CASE(modifier_without_underscore) modifier m() {} } )"; - BOOST_CHECK(expectError(text, true) == Error::Type::SyntaxError); + BOOST_CHECK(expectError(text) == Error::Type::SyntaxError); +} + +BOOST_AUTO_TEST_CASE(payable_in_library) +{ + char const* text = R"( + library test { + function f() payable {} + } + )"; + BOOST_CHECK(expectError(text) == Error::Type::TypeError); +} + +BOOST_AUTO_TEST_CASE(payable_internal) +{ + char const* text = R"( + contract test { + function f() payable internal {} + } + )"; + BOOST_CHECK(expectError(text) == Error::Type::TypeError); +} + +BOOST_AUTO_TEST_CASE(illegal_override_payable) +{ + char const* text = R"( + contract B { function f() payable {} } + contract C is B { function f() {} } + )"; + BOOST_CHECK(expectError(text) == Error::Type::TypeError); +} + +BOOST_AUTO_TEST_CASE(illegal_override_payable_nonpayable) +{ + char const* text = R"( + contract B { function f() {} } + contract C is B { function f() payable {} } + )"; + BOOST_CHECK(expectError(text) == Error::Type::TypeError); +} + +BOOST_AUTO_TEST_CASE(calling_payable) +{ + char const* text = R"( + contract receiver { function pay() payable {} } + contract test { + funciton f() { (new receiver()).pay.value(10)(); } + recevier r = new receiver(); + function g() { r.pay.value(10)(); } + } + )"; + BOOST_CHECK(success(text)); +} + +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)(); } + } + )"; + BOOST_CHECK(expectError(text) == Error::Type::TypeError); } BOOST_AUTO_TEST_CASE(warn_nonpresent_pragma) diff --git a/test/libsolidity/SolidityParser.cpp b/test/libsolidity/SolidityParser.cpp index 0c0e343b..a81a9828 100644 --- a/test/libsolidity/SolidityParser.cpp +++ b/test/libsolidity/SolidityParser.cpp @@ -1231,6 +1231,16 @@ BOOST_AUTO_TEST_CASE(invalid_fixed_conversion_leading_zeroes_check) BOOST_CHECK(!successParse(text)); } +BOOST_AUTO_TEST_CASE(payable_accessor) +{ + char const* text = R"( + contract test { + uint payable x; + } + )"; + BOOST_CHECK(!successParse(text)); +} + BOOST_AUTO_TEST_SUITE_END() } -- cgit v1.2.3 From 9c64edf11052f2918f10ccd202bbfda628005562 Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 31 Aug 2016 20:43:24 +0200 Subject: Change function type to include and propagate payable and constant modifier. --- libsolidity/analysis/TypeChecker.cpp | 12 +---- libsolidity/ast/Types.cpp | 62 +++++++++++++++++----- libsolidity/ast/Types.h | 24 +++++++-- libsolidity/codegen/ContractCompiler.cpp | 6 +++ libsolidity/codegen/ExpressionCompiler.cpp | 2 + test/libsolidity/Assembly.cpp | 2 +- test/libsolidity/SolidityEndToEndTest.cpp | 59 ++++++++++---------- 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(*contract); - TypePointers parameterTypes = contractType->constructorType()->parameterTypes(); - _newExpression.annotation().type = make_shared( - 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(256)}, - {"call", make_shared(strings(), strings{"bool"}, FunctionType::Location::Bare, true)}, - {"callcode", make_shared(strings(), strings{"bool"}, FunctionType::Location::BareCallCode, true)}, + {"call", make_shared(strings(), strings{"bool"}, FunctionType::Location::Bare, true, false, true)}, + {"callcode", make_shared(strings(), strings{"bool"}, FunctionType::Location::BareCallCode, true, false, true)}, {"delegatecall", make_shared(strings(), strings{"bool"}, FunctionType::Location::BareDelegateCall, true)}, {"send", make_shared(strings{"uint"}, strings{"bool"}, FunctionType::Location::Send)} }; @@ -1329,16 +1329,10 @@ MemberList::MemberMap ContractType::nativeMembers(ContractDefinition const*) con return members; } -shared_ptr const& ContractType::constructorType() const +shared_ptr const& ContractType::newExpressionType() const { if (!m_constructorType) - { - FunctionDefinition const* constructor = m_contract.constructor(); - if (constructor) - m_constructorType = make_shared(*constructor); - else - m_constructorType = make_shared(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 const& var: constructor->parameters()) + { + parameterNames.push_back(var->name()); + parameters.push_back(var->annotation().type); + } + payable = constructor->isPayable(); + } + return make_shared( + parameters, + TypePointers{make_shared(_contract)}, + parameterNames, + strings{""}, + Location::Creation, + false, + nullptr, + false, + payable + ); +} + vector FunctionType::parameterNames() const { if (!bound()) @@ -1872,7 +1895,12 @@ FunctionTypePointer FunctionType::interfaceFunctionType() const if (variable && retParamTypes.empty()) return FunctionTypePointer(); - return make_shared(paramTypes, retParamTypes, m_parameterNames, m_returnParameterNames, m_location, m_arbitraryParameters); + return make_shared( + 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( @@ -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 locations = vector(18, SourceLocation(2, 75, n)) + - vector(28, SourceLocation(20, 72, n)) + + vector(31, SourceLocation(20, 72, n)) + vector{SourceLocation(42, 51, n), SourceLocation(65, 67, n)} + vector(4, SourceLocation(58, 67, n)) + vector(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); -- cgit v1.2.3 From 1eb7ddbb09b2ff3c65b6329962229ffcc10eaf03 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 5 Sep 2016 13:34:57 +0200 Subject: Make constant and payable mutually exclusive. --- libsolidity/analysis/TypeChecker.cpp | 2 ++ test/libsolidity/SolidityNameAndTypeResolution.cpp | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index c948b90b..eea4619d 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -424,6 +424,8 @@ bool TypeChecker::visit(FunctionDefinition const& _function) if (!_function.isConstructor() && !_function.name().empty() && !_function.isPartOfExternalInterface()) typeError(_function.location(), "Internal functions cannot be payable."); } + if (_function.isPayable() && _function.isDeclaredConst()) + typeError(_function.location(), "Functions cannot be constant and payable at the same time."); for (ASTPointer const& var: _function.parameters() + _function.returnParameters()) { if (!type(*var)->canLiveOutsideStorage()) diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 3adf4612..29c0ccba 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -3893,6 +3893,14 @@ BOOST_AUTO_TEST_CASE(illegal_override_payable_nonpayable) BOOST_CHECK(expectError(text) == Error::Type::TypeError); } +BOOST_AUTO_TEST_CASE(payable_constant_conflict) +{ + char const* text = R"( + contract C { function f() payable constant {} } + )"; + BOOST_CHECK(expectError(text) == Error::Type::TypeError); +} + BOOST_AUTO_TEST_CASE(calling_payable) { char const* text = R"( -- cgit v1.2.3 From ff11aa192708620de538ed8676136eb616abc74e Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 5 Sep 2016 21:32:27 +0200 Subject: Change placeholder style. --- test/libsolidity/SolidityEndToEndTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 006d41c2..1ecd7a2c 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -7154,7 +7154,7 @@ BOOST_AUTO_TEST_CASE(no_nonpayable_circumvention_by_modifier) char const* sourceCode = R"( contract C { modifier tryCircumvent { - if (false) _ // avoid the function, we should still not accept ether + if (false) _; // avoid the function, we should still not accept ether } function f() tryCircumvent returns (uint) { return msg.value; -- cgit v1.2.3 From 384f189a6adffa7b1879bb06ded4453ef5f00dcb Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 6 Sep 2016 10:58:56 +0200 Subject: Tests for payable / private combination. --- libsolidity/analysis/TypeChecker.cpp | 4 ++-- test/libsolidity/SolidityNameAndTypeResolution.cpp | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index eea4619d..06e26a76 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -423,9 +423,9 @@ bool TypeChecker::visit(FunctionDefinition const& _function) typeError(_function.location(), "Library functions cannot be payable."); if (!_function.isConstructor() && !_function.name().empty() && !_function.isPartOfExternalInterface()) typeError(_function.location(), "Internal functions cannot be payable."); + if (_function.isDeclaredConst()) + typeError(_function.location(), "Functions cannot be constant and payable at the same time."); } - if (_function.isPayable() && _function.isDeclaredConst()) - typeError(_function.location(), "Functions cannot be constant and payable at the same time."); for (ASTPointer const& var: _function.parameters() + _function.returnParameters()) { if (!type(*var)->canLiveOutsideStorage()) diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 29c0ccba..ab0f9c7b 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -3865,6 +3865,16 @@ BOOST_AUTO_TEST_CASE(payable_in_library) BOOST_CHECK(expectError(text) == Error::Type::TypeError); } +BOOST_AUTO_TEST_CASE(payable_external) +{ + char const* text = R"( + contract test { + function f() payable external {} + } + )"; + BOOST_CHECK(success(text)); +} + BOOST_AUTO_TEST_CASE(payable_internal) { char const* text = R"( @@ -3875,6 +3885,16 @@ BOOST_AUTO_TEST_CASE(payable_internal) BOOST_CHECK(expectError(text) == Error::Type::TypeError); } +BOOST_AUTO_TEST_CASE(payable_private) +{ + char const* text = R"( + contract test { + function f() payable private {} + } + )"; + BOOST_CHECK(expectError(text) == Error::Type::TypeError); +} + BOOST_AUTO_TEST_CASE(illegal_override_payable) { char const* text = R"( -- cgit v1.2.3 From dff9633084ebc241d8268c5dbd35a0c5307fd6fc Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 6 Sep 2016 10:59:13 +0200 Subject: Test and fixes for payable fallback in ABI. --- libsolidity/interface/InterfaceHandler.cpp | 1 + test/libsolidity/SolidityABIJSON.cpp | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/libsolidity/interface/InterfaceHandler.cpp b/libsolidity/interface/InterfaceHandler.cpp index a536b0bc..6e3ae78f 100644 --- a/libsolidity/interface/InterfaceHandler.cpp +++ b/libsolidity/interface/InterfaceHandler.cpp @@ -82,6 +82,7 @@ string InterfaceHandler::abiInterface(ContractDefinition const& _contractDef) Json::Value method; method["type"] = "fallback"; method["constant"] = externalFunctionType->isConstant(); + method["payable"] = externalFunctionType->isPayable(); abi.append(method); } for (auto const& it: _contractDef.interfaceEvents()) diff --git a/test/libsolidity/SolidityABIJSON.cpp b/test/libsolidity/SolidityABIJSON.cpp index 9ee74d41..185ba3bf 100644 --- a/test/libsolidity/SolidityABIJSON.cpp +++ b/test/libsolidity/SolidityABIJSON.cpp @@ -645,6 +645,7 @@ BOOST_AUTO_TEST_CASE(include_fallback_function) [ { "constant" : false, + "payable": false, "type" : "fallback" } ] @@ -684,6 +685,25 @@ BOOST_AUTO_TEST_CASE(payable_function) checkInterface(sourceCode, interface); } +BOOST_AUTO_TEST_CASE(payable_fallback_unction) +{ + char const* sourceCode = R"( + contract test { + function () payable {} + } + )"; + + char const* interface = R"( + [ + { + "constant" : false, + "payable": true, + "type" : "fallback" + } + ] + )"; + checkInterface(sourceCode, interface); +} BOOST_AUTO_TEST_SUITE_END() -- cgit v1.2.3