Solidity getInterfaceFunctions is now a map of hash to Function

- Also introduced dependency between libsolidity and libdevcrypto

- Compler's appendFunctionSelector now has a first version of using
  function signature hash instead of index

6 files changed, 45 insertions, 42 deletions

diff --git a/AST.cpp b/AST.cpp
index 0c56cb7a..300303ac 100644
--- a/AST.cpp
+++ b/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,16 @@ 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)
-	{
-		return _a->getName().compare(_b->getName()) < 0;
-	};
+		{
+			FixedHash<4> hash(dev::sha3(f->getCanonicalSignature()));
+			exportedFunctions[hash] = f.get();
+		}
 
-	sort(exportedFunctions.begin(), exportedFunctions.end(), compareNames);
 	return exportedFunctions;
 }
 
diff --git a/AST.h b/AST.h
index 8493d432..95121d4c 100644
--- a/AST.h
+++ b/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.
-	std::vector<FunctionDefinition const*> getInterfaceFunctions() const;
+	/// @returns a map of canonical function signatures to FunctionDefinitions
+	/// 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;
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 0a0b62bd..9c0b5077 100644
--- a/CMakeLists.txt
+++ b/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} )
diff --git a/Compiler.cpp b/Compiler.cpp
index 394ae5f8..83fd69f5 100644
--- a/Compiler.cpp
+++ b/Compiler.cpp
@@ -118,34 +118,30 @@ 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;
 
-	if (interfaceFunctions.size() > 255)
-		BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("More than 255 public functions for contract."));
+	if (interfaceFunctions.size() > 4294967295) // 2 ** 32
+		BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("More than 4294967295 public functions for contract."));
 
-	// retrieve the first byte of the call data, which determines the called function
-	// @todo This code had a jump table in a previous version which was more efficient but also
-	// error prone (due to the optimizer and variable length tag addresses)
-	m_context << u256(1) << u256(0) // some constants
-			  << eth::dupInstruction(1) << eth::Instruction::CALLDATALOAD
-			  << eth::dupInstruction(2) << eth::Instruction::BYTE
-			  << eth::dupInstruction(2);
+	// retrieve the first function signature hash from the calldata
+	m_context << u256(1) << u256(0) << u256(2.6959947e+67) // some constants
+			  << eth::dupInstruction(2) << eth::Instruction::CALLDATALOAD
+			  << eth::Instruction::DIV;
 
-	// stack here: 1 0 <funid> 0, stack top will be counted up until it matches funid
-	for (unsigned funid = 0; funid < interfaceFunctions.size(); ++funid)
+	// stack now is: 1 0 2.6959947e+67 <funhash>
+	for (auto it = interfaceFunctions.begin(); it != interfaceFunctions.end(); ++it)
 	{
 		callDataUnpackerEntryPoints.push_back(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());
-		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)
+	unsigned funid = 0;
+	for (auto it = interfaceFunctions.begin(); it != interfaceFunctions.end(); ++it, ++funid)
 	{
-		FunctionDefinition const& function = *interfaceFunctions[funid];
+		FunctionDefinition const& function = *it->second;
 		m_context << callDataUnpackerEntryPoints[funid];
 		eth::AssemblyItem returnTag = m_context.pushNewTag();
 		appendCalldataUnpacker(function);
diff --git a/InterfaceHandler.cpp b/InterfaceHandler.cpp
index 224234cb..0843c363 100644
--- a/InterfaceHandler.cpp
+++ b/InterfaceHandler.cpp
@@ -35,8 +35,9 @@ std::unique_ptr<std::string> InterfaceHandler::getDocumentation(ContractDefiniti
 std::unique_ptr<std::string> InterfaceHandler::getABIInterface(ContractDefinition const& _contractDef)
 {
 	Json::Value methods(Json::arrayValue);
+	auto interfaceFunctions = _contractDef.getInterfaceFunctions();
 
-	for (FunctionDefinition const* f: _contractDef.getInterfaceFunctions())
+	for (auto it = interfaceFunctions.cbegin(); it != interfaceFunctions.cend(); ++it)
 	{
 		Json::Value method;
 		Json::Value inputs(Json::arrayValue);
@@ -55,10 +56,10 @@ std::unique_ptr<std::string> InterfaceHandler::getABIInterface(ContractDefinitio
 			return params;
 		};
 
-		method["name"] = f->getName();
-		method["constant"] = f->isDeclaredConst();
-		method["inputs"] = populateParameters(f->getParameters());
-		method["outputs"] = populateParameters(f->getReturnParameters());
+		method["name"] = it->second->getName();
+		method["constant"] = it->second->isDeclaredConst();
+		method["inputs"] = populateParameters(it->second->getParameters());
+		method["outputs"] = populateParameters(it->second->getReturnParameters());
 		methods.append(method);
 	}
 	return std::unique_ptr<std::string>(new std::string(m_writer.write(methods)));
@@ -68,11 +69,12 @@ std::unique_ptr<std::string> InterfaceHandler::getUserDocumentation(ContractDefi
 {
 	Json::Value doc;
 	Json::Value methods(Json::objectValue);
+	auto interfaceFunctions = _contractDef.getInterfaceFunctions();
 
-	for (FunctionDefinition const* f: _contractDef.getInterfaceFunctions())
+	for (auto it = interfaceFunctions.cbegin(); it != interfaceFunctions.cend(); ++it)
 	{
 		Json::Value user;
-		auto strPtr = f->getDocumentation();
+		auto strPtr = it->second->getDocumentation();
 		if (strPtr)
 		{
 			resetUser();
@@ -80,7 +82,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;
 			}
 		}
 	}
@@ -110,10 +112,11 @@ std::unique_ptr<std::string> InterfaceHandler::getDevDocumentation(ContractDefin
 			doc["title"] = m_title;
 	}
 
-	for (FunctionDefinition const* f: _contractDef.getInterfaceFunctions())
+	auto interfaceFunctions = _contractDef.getInterfaceFunctions();
+	for (auto it = interfaceFunctions.cbegin(); it != interfaceFunctions.cend(); ++it)
 	{
 		Json::Value method;
-		auto strPtr = f->getDocumentation();
+		auto strPtr = it->second->getDocumentation();
 		if (strPtr)
 		{
 			resetDev();
@@ -136,7 +139,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;
diff --git a/Types.cpp b/Types.cpp
index 7a4c45c6..1a04fe61 100644
--- a/Types.cpp
+++ b/Types.cpp
@@ -301,9 +301,10 @@ MemberList const& ContractType::getMembers() const
 	// We need to lazy-initialize it because of recursive references.
 	if (!m_members)
 	{
+		auto interfaceFunctions = m_contract.getInterfaceFunctions();
 		map<string, shared_ptr<Type const>> members;
-		for (FunctionDefinition const* function: m_contract.getInterfaceFunctions())
-			members[function->getName()] = make_shared<FunctionType>(*function, false);
+		for (auto it = interfaceFunctions.cbegin(); it != interfaceFunctions.cend(); ++it)
+			members[it->second->getName()] = make_shared<FunctionType>(*it->second, false);
 		m_members.reset(new MemberList(members));
 	}
 	return *m_members;
@@ -325,9 +326,10 @@ shared_ptr<FunctionType const> const& ContractType::getConstructorType() const
 unsigned ContractType::getFunctionIndex(string const& _functionName) const
 {
 	unsigned index = 0;
-	for (FunctionDefinition const* function: m_contract.getInterfaceFunctions())
+	auto interfaceFunctions = m_contract.getInterfaceFunctions();
+	for (auto it = interfaceFunctions.cbegin(); it != interfaceFunctions.cend(); ++it)
 	{
-		if (function->getName() == _functionName)
+		if (it->second->getName() == _functionName)
 			return index;
 		++index;
 	}