From ceeb8f4a2b20a54c38792c7451969b660b144246 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Tue, 15 Nov 2016 10:24:53 +0000 Subject: [PATCH 1/8] Add payable check for constructor in codegen --- libsolidity/codegen/ContractCompiler.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 2e3f5d7fe..03296cb27 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -137,6 +137,12 @@ void ContractCompiler::appendInitAndConstructorCode(ContractDefinition const& _c appendConstructor(*constructor); else if (auto c = m_context.nextConstructor(_contract)) appendBaseConstructor(*c); + else + { + // Throw if function is not payable but call contained ether. + m_context << Instruction::CALLVALUE; + m_context.appendConditionalJumpTo(m_context.errorTag()); + } } size_t ContractCompiler::packIntoContractCreator(ContractDefinition const& _contract) @@ -184,6 +190,12 @@ void ContractCompiler::appendBaseConstructor(FunctionDefinition const& _construc void ContractCompiler::appendConstructor(FunctionDefinition const& _constructor) { CompilerContext::LocationSetter locationSetter(m_context, _constructor); + if (!_constructor.isPayable()) + { + // Throw if function is not payable but call contained ether. + m_context << Instruction::CALLVALUE; + m_context.appendConditionalJumpTo(m_context.errorTag()); + } // copy constructor arguments from code to memory and then to stack, they are supplied after the actual program if (!_constructor.parameters().empty()) { From 60e9c901e9f4a314fca1acd374c13841d7be3963 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Tue, 15 Nov 2016 10:25:39 +0000 Subject: [PATCH 2/8] Include payable for the constructor in the ABI --- libsolidity/interface/InterfaceHandler.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libsolidity/interface/InterfaceHandler.cpp b/libsolidity/interface/InterfaceHandler.cpp index 0f7b6c359..9944bb22d 100644 --- a/libsolidity/interface/InterfaceHandler.cpp +++ b/libsolidity/interface/InterfaceHandler.cpp @@ -68,6 +68,7 @@ Json::Value InterfaceHandler::abiInterface(ContractDefinition const& _contractDe method["type"] = "constructor"; auto externalFunction = FunctionType(*_contractDef.constructor(), false).interfaceFunctionType(); solAssert(!!externalFunction, ""); + method["payable"] = externalFunction->isPayable(); method["inputs"] = populateParameters( externalFunction->parameterNames(), externalFunction->parameterTypeNames(_contractDef.isLibrary()) From 819da2f0cd8d5ce9086f0fea05769788011f75fa Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Tue, 15 Nov 2016 10:26:17 +0000 Subject: [PATCH 3/8] Add changelog entry for payable constructor --- Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Changelog.md b/Changelog.md index ce3343d8c..6d080556a 100644 --- a/Changelog.md +++ b/Changelog.md @@ -10,6 +10,7 @@ Features: Bugfixes: * Inline assembly: calculate stack height warning correctly even when local variables are used. + * Support the ``payable`` keyword on constructors. * Parser: disallow empty enum definitions. * Type checker: disallow conversion between different enum types. * Interface JSON: do not include trailing new line. From 7af360882e649a835619bf02ca5e63f6f6af6d9f Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Tue, 15 Nov 2016 18:40:37 +0000 Subject: [PATCH 4/8] Add missing payable constructors --- test/libsolidity/SolidityEndToEndTest.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index ed95d6877..09fba26ab 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -1337,6 +1337,7 @@ BOOST_AUTO_TEST_CASE(struct_accessor) BOOST_AUTO_TEST_CASE(balance) { char const* sourceCode = "contract test {\n" + " function test() payable {}\n" " function getBalance() returns (uint256 balance) {\n" " return address(this).balance;\n" " }\n" @@ -1348,6 +1349,7 @@ BOOST_AUTO_TEST_CASE(balance) BOOST_AUTO_TEST_CASE(blockchain) { char const* sourceCode = "contract test {\n" + " function test() payable {}\n" " function someInfo() payable returns (uint256 value, address coinbase, uint256 blockNumber) {\n" " value = msg.value;\n" " coinbase = block.coinbase;\n" @@ -1563,6 +1565,7 @@ BOOST_AUTO_TEST_CASE(convert_uint_to_fixed_bytes_greater_size) BOOST_AUTO_TEST_CASE(send_ether) { char const* sourceCode = "contract test {\n" + " function test() payable {}\n" " function a(address addr, uint amount) returns (uint ret) {\n" " addr.send(amount);\n" " return address(this).balance;\n" @@ -1675,6 +1678,7 @@ BOOST_AUTO_TEST_CASE(log_in_constructor) BOOST_AUTO_TEST_CASE(suicide) { char const* sourceCode = "contract test {\n" + " function test() payable {}\n" " function a(address receiver) returns (uint ret) {\n" " suicide(receiver);\n" " return 10;\n" @@ -1691,6 +1695,7 @@ BOOST_AUTO_TEST_CASE(suicide) BOOST_AUTO_TEST_CASE(selfdestruct) { char const* sourceCode = "contract test {\n" + " function test() payable {}\n" " function a(address receiver) returns (uint ret) {\n" " selfdestruct(receiver);\n" " return 10;\n" @@ -2992,12 +2997,14 @@ BOOST_AUTO_TEST_CASE(generic_delegatecall) uint public received; address public sender; uint public value; + function receiver() payable {} 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 sender() payable {} function doSend(address rec) payable { bytes4 signature = bytes4(bytes32(sha3("receive(uint256)"))); @@ -4818,6 +4825,7 @@ BOOST_AUTO_TEST_CASE(failing_send) } } contract Main { + function Main() payable {} function callHelper(address _a) returns (bool r, uint bal) { r = !_a.send(5); bal = this.balance; @@ -4840,6 +4848,7 @@ BOOST_AUTO_TEST_CASE(send_zero_ether) } } contract Main { + function Main() payable {} function s() returns (bool) { var r = new Receiver(); return r.send(0); @@ -6341,6 +6350,7 @@ BOOST_AUTO_TEST_CASE(reject_ether_sent_to_library) char const* sourceCode = R"( library lib {} contract c { + function c() payable {} function f(address x) returns (bool) { return x.send(1); } @@ -7279,6 +7289,7 @@ BOOST_AUTO_TEST_CASE(failed_create) contract D { function D() payable {} } contract C { uint public x; + function C() payable {} function f(uint amount) returns (address) { x++; return (new D).value(amount)(); @@ -7392,7 +7403,7 @@ BOOST_AUTO_TEST_CASE(mutex) } contract Fund is mutexed { uint shares; - function Fund() { shares = msg.value; } + function Fund() payable { shares = msg.value; } function withdraw(uint amount) protected returns (uint) { // NOTE: It is very bad practice to write this function this way. // Please refer to the documentation of how to do this properly. From 1d6fe5c4e4b01d9ef5e1527a405ae44e79b56726 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Tue, 15 Nov 2016 18:41:54 +0000 Subject: [PATCH 5/8] Add payable to constructor ABI tests --- test/libsolidity/SolidityABIJSON.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/libsolidity/SolidityABIJSON.cpp b/test/libsolidity/SolidityABIJSON.cpp index 5cabb7fa8..a8a39b0b1 100644 --- a/test/libsolidity/SolidityABIJSON.cpp +++ b/test/libsolidity/SolidityABIJSON.cpp @@ -524,6 +524,7 @@ BOOST_AUTO_TEST_CASE(constructor_abi) "type": "bool" } ], + "payable": false, "type": "constructor" } ])"; @@ -567,6 +568,7 @@ BOOST_AUTO_TEST_CASE(return_param_in_abi) "type": "uint8" } ], + "payable": false, "type": "constructor" } ] From d97eb7cc75f6a789f41a43132c7ca3b5aefe967f Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Tue, 15 Nov 2016 18:42:57 +0000 Subject: [PATCH 6/8] Add payable keyword to the multisig wallet --- test/contracts/Wallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/contracts/Wallet.cpp b/test/contracts/Wallet.cpp index ec9680583..4966b26c8 100644 --- a/test/contracts/Wallet.cpp +++ b/test/contracts/Wallet.cpp @@ -368,7 +368,7 @@ contract Wallet is multisig, multiowned, daylimit { // constructor - just pass on the owner array to the multiowned and // the limit to daylimit - function Wallet(address[] _owners, uint _required, uint _daylimit) + function Wallet(address[] _owners, uint _required, uint _daylimit) payable multiowned(_owners, _required) daylimit(_daylimit) { } From a35ca910c798e018c7cedc5a0436674a0c58ca1d Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Tue, 15 Nov 2016 18:59:07 +0000 Subject: [PATCH 7/8] Rename test contract names to capitalised --- test/libsolidity/SolidityEndToEndTest.cpp | 24 +++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 09fba26ab..4abe0894e 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -2961,24 +2961,24 @@ BOOST_AUTO_TEST_CASE(generic_call) BOOST_AUTO_TEST_CASE(generic_callcode) { char const* sourceCode = R"**( - contract receiver { + contract Receiver { uint public received; function receive(uint256 x) payable { received = x; } } - contract sender { + contract Sender { uint public received; - function sender() payable { } + function Sender() payable { } function doSend(address rec) returns (uint d) { bytes4 signature = bytes4(bytes32(sha3("receive(uint256)"))); rec.callcode.value(2)(signature, 23); - return receiver(rec).received(); + return Receiver(rec).received(); } } )**"; - compileAndRun(sourceCode, 0, "receiver"); + compileAndRun(sourceCode, 0, "Receiver"); u160 const c_receiverAddress = m_contractAddress; - compileAndRun(sourceCode, 50, "sender"); + compileAndRun(sourceCode, 50, "Sender"); u160 const c_senderAddress = m_contractAddress; BOOST_CHECK(callContractFunction("doSend(address)", c_receiverAddress) == encodeArgs(0)); BOOST_CHECK(callContractFunction("received()") == encodeArgs(23)); @@ -2993,18 +2993,18 @@ BOOST_AUTO_TEST_CASE(generic_callcode) BOOST_AUTO_TEST_CASE(generic_delegatecall) { char const* sourceCode = R"**( - contract receiver { + contract Receiver { uint public received; address public sender; uint public value; - function receiver() payable {} + function Receiver() payable {} function receive(uint256 x) payable { received = x; sender = msg.sender; value = msg.value; } } - contract sender { + contract Sender { uint public received; address public sender; uint public value; - function sender() payable {} + function Sender() payable {} function doSend(address rec) payable { bytes4 signature = bytes4(bytes32(sha3("receive(uint256)"))); @@ -3012,9 +3012,9 @@ BOOST_AUTO_TEST_CASE(generic_delegatecall) } } )**"; - compileAndRun(sourceCode, 0, "receiver"); + compileAndRun(sourceCode, 0, "Receiver"); u160 const c_receiverAddress = m_contractAddress; - compileAndRun(sourceCode, 50, "sender"); + compileAndRun(sourceCode, 50, "Sender"); u160 const c_senderAddress = m_contractAddress; BOOST_CHECK(m_sender != c_senderAddress); // just for sanity BOOST_CHECK(callContractFunctionWithValue("doSend(address)", 11, c_receiverAddress) == encodeArgs()); From 910269a29f56265e42bce73694d45481cbd5fd54 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Thu, 17 Nov 2016 17:23:31 +0000 Subject: [PATCH 8/8] Add appendCallValueCheck --- libsolidity/codegen/ContractCompiler.cpp | 33 ++++++++++-------------- libsolidity/codegen/ContractCompiler.h | 1 + 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 03296cb27..8d60d6b3f 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -102,6 +102,13 @@ void ContractCompiler::initializeContext( m_context.resetVisitedNodes(&_contract); } +void ContractCompiler::appendCallValueCheck() +{ + // Throw if function is not payable but call contained ether. + m_context << Instruction::CALLVALUE; + m_context.appendConditionalJumpTo(m_context.errorTag()); +} + void ContractCompiler::appendInitAndConstructorCode(ContractDefinition const& _contract) { // Determine the arguments that are used for the base constructors. @@ -138,11 +145,7 @@ void ContractCompiler::appendInitAndConstructorCode(ContractDefinition const& _c else if (auto c = m_context.nextConstructor(_contract)) appendBaseConstructor(*c); else - { - // Throw if function is not payable but call contained ether. - m_context << Instruction::CALLVALUE; - m_context.appendConditionalJumpTo(m_context.errorTag()); - } + appendCallValueCheck(); } size_t ContractCompiler::packIntoContractCreator(ContractDefinition const& _contract) @@ -191,11 +194,8 @@ void ContractCompiler::appendConstructor(FunctionDefinition const& _constructor) { CompilerContext::LocationSetter locationSetter(m_context, _constructor); if (!_constructor.isPayable()) - { - // Throw if function is not payable but call contained ether. - m_context << Instruction::CALLVALUE; - m_context.appendConditionalJumpTo(m_context.errorTag()); - } + appendCallValueCheck(); + // copy constructor arguments from code to memory and then to stack, they are supplied after the actual program if (!_constructor.parameters().empty()) { @@ -263,11 +263,8 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac 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()); - } + appendCallValueCheck(); + eth::AssemblyItem returnTag = m_context.pushNewTag(); fallback->accept(*this); m_context << returnTag; @@ -286,11 +283,7 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac // We have to allow this for libraries, because value of the previous // call is still visible in the delegatecall. if (!functionType->isPayable() && !_contract.isLibrary()) - { - // Throw if function is not payable but call contained ether. - m_context << Instruction::CALLVALUE; - m_context.appendConditionalJumpTo(m_context.errorTag()); - } + appendCallValueCheck(); eth::AssemblyItem returnTag = m_context.pushNewTag(); m_context << CompilerUtils::dataStartOffset; diff --git a/libsolidity/codegen/ContractCompiler.h b/libsolidity/codegen/ContractCompiler.h index 2244a3be5..10febcf72 100644 --- a/libsolidity/codegen/ContractCompiler.h +++ b/libsolidity/codegen/ContractCompiler.h @@ -80,6 +80,7 @@ private: void appendBaseConstructor(FunctionDefinition const& _constructor); void appendConstructor(FunctionDefinition const& _constructor); void appendFunctionSelector(ContractDefinition const& _contract); + void appendCallValueCheck(); /// Creates code that unpacks the arguments for the given function represented by a vector of TypePointers. /// From memory if @a _fromMemory is true, otherwise from call data. /// Expects source offset on the stack, which is removed.