Merge pull request #754 from LefterisJP/sol_abiFunctionHash

Calculation of ABI Function Identifier Hash

AST.cpp

@@ -27,6 +27,8 @@
 #include <libsolidity/Exceptions.h>
 #include <libsolidity/AST_accept.h>
 
+#include <libdevcrypto/SHA3.h>
+
 using namespace std;
 
 namespace dev
@@ -50,18 +52,17 @@ void ContractDefinition::checkTypeRequirements()
 		function->checkTypeRequirements();
 }
 
-vector<FunctionDefinition const*> ContractDefinition::getInterfaceFunctions() const
+map<FixedHash<4>, FunctionDefinition const*> ContractDefinition::getInterfaceFunctions() const
 {
-	vector<FunctionDefinition const*> exportedFunctions;
+	map<FixedHash<4>, FunctionDefinition const*> exportedFunctions;
 	for (ASTPointer<FunctionDefinition> const& f: m_definedFunctions)
 		if (f->isPublic() && f->getName() != getName())
-			exportedFunctions.push_back(f.get());
+		{
-	auto compareNames = [](FunctionDefinition const* _a, FunctionDefinition const* _b)
+			FixedHash<4> hash(dev::sha3(f->getCanonicalSignature()));
-	{
+			auto res = exportedFunctions.insert(std::make_pair(hash,f.get()));
-		return _a->getName().compare(_b->getName()) < 0;
+			solAssert(res.second, "Hash collision at Function Definition Hash calculation");
-	};
+		}
 
-	sort(exportedFunctions.begin(), exportedFunctions.end(), compareNames);
 	return exportedFunctions;
 }
 

AST.h

@@ -183,8 +183,9 @@ public:
 	/// Can contain a nullptr in which case indicates absence of documentation
 	ASTPointer<ASTString> const& getDocumentation() const { return m_documentation; }
 
-	/// Returns the functions that make up the calling interface in the intended order.
+	/// @returns a map of canonical function signatures to FunctionDefinitions
-	std::vector<FunctionDefinition const*> getInterfaceFunctions() const;
+	/// as intended for use by the ABI.
+	std::map<FixedHash<4>, FunctionDefinition const*> getInterfaceFunctions() const;
 
 	/// Returns the constructor or nullptr if no constructor was specified
 	FunctionDefinition const* getConstructor() const;

CMakeLists.txt

@@ -28,6 +28,7 @@ endif()
 target_link_libraries(${EXECUTABLE} ${JSONCPP_LIBRARIES})
 target_link_libraries(${EXECUTABLE} evmcore)
 target_link_libraries(${EXECUTABLE} devcore)
+target_link_libraries(${EXECUTABLE} devcrypto)
 
 install( TARGETS ${EXECUTABLE} ARCHIVE DESTINATION lib LIBRARY DESTINATION lib )
 install( FILES ${HEADERS} DESTINATION include/${EXECUTABLE} )

Compiler.cpp

@@ -100,7 +100,7 @@ void Compiler::appendConstructorCall(FunctionDefinition const& _constructor)
 	{
 		m_context << u256(argumentSize);
 		m_context.appendProgramSize();
-		m_context << u256(1); // copy it to byte one as expected for ABI calls
+		m_context << u256(CompilerUtils::dataStartOffset); // copy it to byte four as expected for ABI calls
 		m_context << eth::Instruction::CODECOPY;
 		appendCalldataUnpacker(_constructor, true);
 	}
@@ -118,35 +118,27 @@ set<FunctionDefinition const*> Compiler::getFunctionsNeededByConstructor(Functio
 
 void Compiler::appendFunctionSelector(ContractDefinition const& _contract)
 {
-	vector<FunctionDefinition const*> interfaceFunctions = _contract.getInterfaceFunctions();
+	map<FixedHash<4>, FunctionDefinition const*> interfaceFunctions = _contract.getInterfaceFunctions();
-	vector<eth::AssemblyItem> callDataUnpackerEntryPoints;
+	map<FixedHash<4>, const eth::AssemblyItem> callDataUnpackerEntryPoints;
 
-	if (interfaceFunctions.size() > 255)
+	// retrieve the function signature hash from the calldata
-		BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("More than 255 public functions for contract."));
+	m_context << u256(1) << u256(0);
+	CompilerUtils(m_context).loadFromMemory(0, 4, false, true);
 
-	// retrieve the first byte of the call data, which determines the called function
+	// stack now is: 1 0 <funhash>
-	// @todo This code had a jump table in a previous version which was more efficient but also
+	// for (auto it = interfaceFunctions.cbegin(); it != interfaceFunctions.cend(); ++it)
-	// error prone (due to the optimizer and variable length tag addresses)
+	for (auto const& it: interfaceFunctions)
-	m_context << u256(1) << u256(0) // some constants
-			  << eth::dupInstruction(1) << eth::Instruction::CALLDATALOAD
-			  << eth::dupInstruction(2) << eth::Instruction::BYTE
-			  << eth::dupInstruction(2);
-
-	// stack here: 1 0 <funid> 0, stack top will be counted up until it matches funid
-	for (unsigned funid = 0; funid < interfaceFunctions.size(); ++funid)
 	{
-		callDataUnpackerEntryPoints.push_back(m_context.newTag());
+		callDataUnpackerEntryPoints.insert(std::make_pair(it.first, m_context.newTag()));
-		m_context << eth::dupInstruction(2) << eth::dupInstruction(2) << eth::Instruction::EQ;
+		m_context << eth::dupInstruction(1) << u256(FixedHash<4>::Arith(it.first)) << eth::Instruction::EQ;
-		m_context.appendConditionalJumpTo(callDataUnpackerEntryPoints.back());
+		m_context.appendConditionalJumpTo(callDataUnpackerEntryPoints.at(it.first));
-		if (funid < interfaceFunctions.size() - 1)
-			m_context << eth::dupInstruction(4) << eth::Instruction::ADD;
 	}
 	m_context << eth::Instruction::STOP; // function not found
 
-	for (unsigned funid = 0; funid < interfaceFunctions.size(); ++funid)
+	for (auto const& it: interfaceFunctions)
 	{
-		FunctionDefinition const& function = *interfaceFunctions[funid];
+		FunctionDefinition const& function = *it.second;
-		m_context << callDataUnpackerEntryPoints[funid];
+		m_context << callDataUnpackerEntryPoints.at(it.first);
 		eth::AssemblyItem returnTag = m_context.pushNewTag();
 		appendCalldataUnpacker(function);
 		m_context.appendJumpTo(m_context.getFunctionEntryLabel(function));
@@ -158,7 +150,7 @@ void Compiler::appendFunctionSelector(ContractDefinition const& _contract)
 unsigned Compiler::appendCalldataUnpacker(FunctionDefinition const& _function, bool _fromMemory)
 {
 	// We do not check the calldata size, everything is zero-padded.
-	unsigned dataOffset = 1;
+	unsigned dataOffset = CompilerUtils::dataStartOffset; // the 4 bytes of the function hash signature
 	//@todo this can be done more efficiently, saving some CALLDATALOAD calls
 	for (ASTPointer<VariableDeclaration> const& var: _function.getParameters())
 	{

CompilerUtils.cpp

@@ -31,6 +31,8 @@ namespace dev
 namespace solidity
 {
 
+const unsigned int CompilerUtils::dataStartOffset = 4;
+
 void CompilerUtils::loadFromMemory(unsigned _offset, unsigned _bytes, bool _leftAligned, bool _fromCalldata)
 {
 	if (_bytes == 0)

CompilerUtils.h

@@ -58,10 +58,14 @@ public:
 	static unsigned getSizeOnStack(std::vector<T> const& _variables);
 	static unsigned getSizeOnStack(std::vector<std::shared_ptr<Type const>> const& _variableTypes);
 
+	/// Bytes we need to the start of call data.
+	///  - The size in bytes of the function (hash) identifier.
+	static const unsigned int dataStartOffset;
 private:
 	CompilerContext& m_context;
 };
 
+
 template <class T>
 unsigned CompilerUtils::getSizeOnStack(std::vector<T> const& _variables)
 {

ExpressionCompiler.cpp

@@ -390,7 +390,7 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess)
 	case Type::Category::CONTRACT:
 	{
 		ContractType const& type = dynamic_cast<ContractType const&>(*_memberAccess.getExpression().getType());
-		m_context << type.getFunctionIndex(member);
+		m_context << type.getFunctionIdentifier(member);
 		break;
 	}
 	case Type::Category::MAGIC:
@@ -645,7 +645,11 @@ void ExpressionCompiler::appendExternalFunctionCall(FunctionType const& _functio
 {
 	solAssert(_arguments.size() == _functionType.getParameterTypes().size(), "");
 
-	unsigned dataOffset = _options.bare ? 0 : 1; // reserve one byte for the function index
+	_options.obtainAddress();
+	if (!_options.bare)
+		CompilerUtils(m_context).storeInMemory(0, CompilerUtils::dataStartOffset);
+
+	unsigned dataOffset = _options.bare ? 0 : CompilerUtils::dataStartOffset; // reserve 4 bytes for the function's hash identifier
 	for (unsigned i = 0; i < _arguments.size(); ++i)
 	{
 		_arguments[i]->accept(*this);
@@ -672,12 +676,13 @@ void ExpressionCompiler::appendExternalFunctionCall(FunctionType const& _functio
 		_options.obtainValue();
 	else
 		m_context << u256(0);
-	_options.obtainAddress();
+	m_context << eth::dupInstruction(6); //copy contract address
-	if (!_options.bare)
+
-		m_context << u256(0) << eth::Instruction::MSTORE8;
 	m_context << u256(25) << eth::Instruction::GAS << eth::Instruction::SUB
 			  << eth::Instruction::CALL
-			  << eth::Instruction::POP; // @todo do not ignore failure indicator
+			  << eth::Instruction::POP // @todo do not ignore failure indicator
+			  << eth::Instruction::POP; // pop contract address
+
 	if (retSize > 0)
 	{
 		bool const leftAligned = firstType->getCategory() == Type::Category::STRING;

InterfaceHandler.cpp

@@ -39,7 +39,7 @@ std::unique_ptr<std::string> InterfaceHandler::getABIInterface(ContractDefinitio
 {
 	Json::Value methods(Json::arrayValue);
 
-	for (FunctionDefinition const* f: _contractDef.getInterfaceFunctions())
+	for (auto const& it: _contractDef.getInterfaceFunctions())
 	{
 		Json::Value method;
 		Json::Value inputs(Json::arrayValue);
@@ -58,10 +58,10 @@ std::unique_ptr<std::string> InterfaceHandler::getABIInterface(ContractDefinitio
 			return params;
 		};
 
-		method["name"] = f->getName();
+		method["name"] = it.second->getName();
-		method["constant"] = f->isDeclaredConst();
+		method["constant"] = it.second->isDeclaredConst();
-		method["inputs"] = populateParameters(f->getParameters());
+		method["inputs"] = populateParameters(it.second->getParameters());
-		method["outputs"] = populateParameters(f->getReturnParameters());
+		method["outputs"] = populateParameters(it.second->getReturnParameters());
 		methods.append(method);
 	}
 	return std::unique_ptr<std::string>(new std::string(m_writer.write(methods)));
@@ -94,10 +94,10 @@ std::unique_ptr<std::string> InterfaceHandler::getUserDocumentation(ContractDefi
 	Json::Value doc;
 	Json::Value methods(Json::objectValue);
 
-	for (FunctionDefinition const* f: _contractDef.getInterfaceFunctions())
+	for (auto const& it: _contractDef.getInterfaceFunctions())
 	{
 		Json::Value user;
-		auto strPtr = f->getDocumentation();
+		auto strPtr = it.second->getDocumentation();
 		if (strPtr)
 		{
 			resetUser();
@@ -105,7 +105,7 @@ std::unique_ptr<std::string> InterfaceHandler::getUserDocumentation(ContractDefi
 			if (!m_notice.empty())
 			{// since @notice is the only user tag if missing function should not appear
 				user["notice"] = Json::Value(m_notice);
-				methods[f->getName()] = user;
+				methods[it.second->getName()] = user;
 			}
 		}
 	}
@@ -135,10 +135,10 @@ std::unique_ptr<std::string> InterfaceHandler::getDevDocumentation(ContractDefin
 			doc["title"] = m_title;
 	}
 
-	for (FunctionDefinition const* f: _contractDef.getInterfaceFunctions())
+	for (auto const& it: _contractDef.getInterfaceFunctions())
 	{
 		Json::Value method;
-		auto strPtr = f->getDocumentation();
+		auto strPtr = it.second->getDocumentation();
 		if (strPtr)
 		{
 			resetDev();
@@ -161,7 +161,7 @@ std::unique_ptr<std::string> InterfaceHandler::getDevDocumentation(ContractDefin
 				method["return"] = m_return;
 
 			if (!method.empty()) // add the function, only if we have any documentation to add
-				methods[f->getName()] = method;
+				methods[it.second->getName()] = method;
 		}
 	}
 	doc["methods"] = methods;

Types.cpp

@@ -334,8 +334,8 @@ MemberList const& ContractType::getMembers() const
 	if (!m_members)
 	{
 		map<string, shared_ptr<Type const>> members;
-		for (FunctionDefinition const* function: m_contract.getInterfaceFunctions())
+		for (auto const& it: m_contract.getInterfaceFunctions())
-			members[function->getName()] = make_shared<FunctionType>(*function, false);
+			members[it.second->getName()] = make_shared<FunctionType>(*it.second, false);
 		m_members.reset(new MemberList(members));
 	}
 	return *m_members;
@@ -354,15 +354,13 @@ shared_ptr<FunctionType const> const& ContractType::getConstructorType() const
 	return m_constructorType;
 }
 
-unsigned ContractType::getFunctionIndex(string const& _functionName) const
+u256 ContractType::getFunctionIdentifier(string const& _functionName) const
 {
-	unsigned index = 0;
+	auto interfaceFunctions = m_contract.getInterfaceFunctions();
-	for (FunctionDefinition const* function: m_contract.getInterfaceFunctions())
+	for (auto it = interfaceFunctions.cbegin(); it != interfaceFunctions.cend(); ++it)
-	{
+		if (it->second->getName() == _functionName)
-		if (function->getName() == _functionName)
+			return FixedHash<4>::Arith(it->first);
-			return index;
+
-		++index;
-	}
 	BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_comment("Index of non-existing contract function requested."));
 }
 

Types.h

@@ -267,7 +267,7 @@ public:
 	/// is not used, as this type cannot be the type of a variable or expression.
 	std::shared_ptr<FunctionType const> const& getConstructorType() const;
 
-	unsigned getFunctionIndex(std::string const& _functionName) const;
+	u256 getFunctionIdentifier(std::string const& _functionName) const;
 
 private:
 	ContractDefinition const& m_contract;