Merged in changes from chriseth/payable

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<ContractDefinition const&>(*_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<VariableDeclaration> 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();

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",
+			if (!m_declaration || m_isPayable) // If this is an actual function, it has to be payable
-				make_shared<FunctionType>(
+				members.push_back(MemberList::Member(
-					parseElementaryTypeVector({"uint"}),
+					"value",
-					TypePointers{copyAndSetGasOrValue(false, true)},
+					make_shared<FunctionType>(
-					strings(),
+						parseElementaryTypeVector({"uint"}),
-					strings(),
+						TypePointers{copyAndSetGasOrValue(false, true)},
-					Location::SetValue,
+						strings(),
-					false,
+						strings(),
-					nullptr,
+						Location::SetValue,
-					m_gasSet,
+						false,
-					m_valueSet
+						nullptr,
-				)
+						m_gasSet,
-			));
+						m_valueSet
+					)
+				));
+		}
 		if (m_location != Location::Creation)
 			members.push_back(MemberList::Member(
 				"gas",

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()));

libsolidity/parsing/Parser.cpp

@@ -305,6 +305,7 @@ ASTPointer<FunctionDefinition> Parser::parseFunctionDefinition(ASTString const*
 	options.allowLocationSpecifier = true;
 	ASTPointer<ParameterList> parameters(parseParameterList(options));
 	bool isDeclaredConst = false;
+	bool isPayable = false;
 	Declaration::Visibility visibility(Declaration::Visibility::Default);
 	vector<ASTPointer<ModifierInvocation>> modifiers;
 	while (true)
@@ -315,6 +316,11 @@ ASTPointer<FunctionDefinition> 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<FunctionDefinition> Parser::parseFunctionDefinition(ASTString const*
 		else
 			break;
 	}
-	bool isPayable = false;
-	if (m_scanner->currentToken() == Token::Payable)
-	{
-	        isPayable = true;
-	        m_scanner->next();
-	}
 	ASTPointer<ParameterList> returnParameters;
 	if (m_scanner->currentToken() == Token::Returns)
 	{

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);

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;

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);

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("outOfGas()") == bytes());
-	BOOST_REQUIRE(callContractFunction("checkState()", 5) == encodeArgs(false, 20 - 5));
+	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_function)
-BOOST_AUTO_TEST_CASE(payable_accept_explicit)
 {
 	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()
 
 }

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)

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()
 
 }