Merged in changes from chriseth/payable

4 files changed, 37 insertions, 33 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<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();
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<FunctionType>(
-					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<FunctionType>(
+						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<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)
 	{