diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index d05eaf83e..1736469eb 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -88,7 +88,7 @@ void ContractDefinition::checkTypeRequirements() if (hashes.count(hash)) BOOST_THROW_EXCEPTION(createTypeError( std::string("Function signature hash collision for ") + - it.second->getCanonicalSignature())); + it.second->externalSignature())); hashes.insert(hash); } } @@ -192,7 +192,7 @@ vector, FunctionTypePointer>> const& ContractDefinition::getIn if (functionsSeen.count(f->getName()) == 0 && f->isPartOfExternalInterface()) { functionsSeen.insert(f->getName()); - FixedHash<4> hash(dev::sha3(f->getCanonicalSignature())); + FixedHash<4> hash(dev::sha3(f->externalSignature())); m_interfaceFunctionList->push_back(make_pair(hash, make_shared(*f, false))); } @@ -200,8 +200,9 @@ vector, FunctionTypePointer>> const& ContractDefinition::getIn if (functionsSeen.count(v->getName()) == 0 && v->isPartOfExternalInterface()) { FunctionType ftype(*v); + solAssert(v->getType().get(), ""); functionsSeen.insert(v->getName()); - FixedHash<4> hash(dev::sha3(ftype.getCanonicalSignature(v->getName()))); + FixedHash<4> hash(dev::sha3(ftype.externalSignature(v->getName()))); m_interfaceFunctionList->push_back(make_pair(hash, make_shared(*v))); } } @@ -305,8 +306,12 @@ TypePointer FunctionDefinition::getType(ContractDefinition const*) const void FunctionDefinition::checkTypeRequirements() { for (ASTPointer const& var: getParameters() + getReturnParameters()) + { if (!var->getType()->canLiveOutsideStorage()) BOOST_THROW_EXCEPTION(var->createTypeError("Type is required to live outside storage.")); + if (!(var->getType()->externalType()) && getVisibility() >= Visibility::Public) + BOOST_THROW_EXCEPTION(var->createTypeError("Internal type is not allowed for function with external visibility")); + } for (ASTPointer const& modifier: m_functionModifiers) modifier->checkTypeRequirements(isConstructor() ? dynamic_cast(*getScope()).getBaseContracts() : @@ -315,9 +320,9 @@ void FunctionDefinition::checkTypeRequirements() m_body->checkTypeRequirements(); } -string FunctionDefinition::getCanonicalSignature() const +string FunctionDefinition::externalSignature() const { - return FunctionType(*this).getCanonicalSignature(getName()); + return FunctionType(*this).externalSignature(getName()); } bool VariableDeclaration::isLValue() const @@ -342,8 +347,11 @@ void VariableDeclaration::checkTypeRequirements() if (!m_value) return; if (m_type) + { m_value->expectType(*m_type); - else + if (m_isStateVariable && !m_type->externalType() && getVisibility() >= Visibility::Public) + BOOST_THROW_EXCEPTION(createTypeError("Internal type is not allowed for state variables.")); + } else { // no type declared and no previous assignment, infer the type m_value->checkTypeRequirements(); @@ -422,6 +430,8 @@ void EventDefinition::checkTypeRequirements() numIndexed++; if (!var->getType()->canLiveOutsideStorage()) BOOST_THROW_EXCEPTION(var->createTypeError("Type is required to live outside storage.")); + if (!var->getType()->externalType()) + BOOST_THROW_EXCEPTION(var->createTypeError("Internal type is not allowed as event parameter type.")); } if (numIndexed > 3) BOOST_THROW_EXCEPTION(createTypeError("More than 3 indexed arguments for event.")); diff --git a/libsolidity/AST.h b/libsolidity/AST.h index 335b9a880..00ad33073 100644 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -421,10 +421,10 @@ public: /// Checks that all parameters have allowed types and calls checkTypeRequirements on the body. void checkTypeRequirements(); - /// @returns the canonical signature of the function + /// @returns the external signature of the function /// That consists of the name of the function followed by the types of the /// arguments separated by commas all enclosed in parentheses without any spaces. - std::string getCanonicalSignature() const; + std::string externalSignature() const; private: bool m_isConstructor; diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index da762147a..90568767b 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -544,7 +544,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) } if (!event.isAnonymous()) { - m_context << u256(h256::Arith(dev::sha3(function.getCanonicalSignature(event.getName())))); + m_context << u256(h256::Arith(dev::sha3(function.externalSignature(event.getName())))); ++numIndexed; } solAssert(numIndexed <= 4, "Too many indexed arguments."); diff --git a/libsolidity/InterfaceHandler.cpp b/libsolidity/InterfaceHandler.cpp index 406d1e24a..2f35a96f6 100644 --- a/libsolidity/InterfaceHandler.cpp +++ b/libsolidity/InterfaceHandler.cpp @@ -129,7 +129,7 @@ std::unique_ptr 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[it.second->getCanonicalSignature()] = user; + methods[it.second->externalSignature()] = user; } } } @@ -185,7 +185,7 @@ std::unique_ptr InterfaceHandler::getDevDocumentation(ContractDefin method["return"] = m_return; if (!method.empty()) // add the function, only if we have any documentation to add - methods[it.second->getCanonicalSignature()] = method; + methods[it.second->externalSignature()] = method; } } doc["methods"] = methods; diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 016f0b236..5fd7d24a6 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -742,6 +742,23 @@ string ArrayType::toString() const return ret + "]"; } +TypePointer ArrayType::externalType() const +{ + if (m_location != Location::CallData) + return TypePointer(); + if (m_isByteArray) + return shared_from_this(); + if (!m_baseType->externalType()) + return TypePointer(); + if (m_baseType->getCategory() == Category::Array && m_baseType->isDynamicallySized()) + return TypePointer(); + + if (isDynamicallySized()) + return std::make_shared(Location::CallData, m_baseType->externalType()); + else + return std::make_shared(Location::CallData, m_baseType->externalType(), m_length); +} + shared_ptr ArrayType::copyForLocation(ArrayType::Location _location) const { auto copy = make_shared(_location); @@ -1081,6 +1098,19 @@ unsigned FunctionType::getSizeOnStack() const return size; } +TypePointer FunctionType::externalType() const +{ + TypePointers paramTypes; + TypePointers retParamTypes; + + for (auto it = m_parameterTypes.cbegin(); it != m_parameterTypes.cend(); ++it) + paramTypes.push_back((*it)->externalType()); + for (auto it = m_returnParameterTypes.cbegin(); it != m_returnParameterTypes.cend(); ++it) + retParamTypes.push_back((*it)->externalType()); + + return make_shared(paramTypes, retParamTypes, m_location, m_arbitraryParameters); +} + MemberList const& FunctionType::getMembers() const { switch (m_location) @@ -1110,7 +1140,7 @@ MemberList const& FunctionType::getMembers() const } } -string FunctionType::getCanonicalSignature(std::string const& _name) const +string FunctionType::externalSignature(std::string const& _name) const { std::string funcName = _name; if (_name == "") @@ -1120,8 +1150,12 @@ string FunctionType::getCanonicalSignature(std::string const& _name) const } string ret = funcName + "("; - for (auto it = m_parameterTypes.cbegin(); it != m_parameterTypes.cend(); ++it) - ret += (*it)->toString() + (it + 1 == m_parameterTypes.cend() ? "" : ","); + TypePointers externalParameterTypes = dynamic_cast(*externalType()).getParameterTypes(); + for (auto it = externalParameterTypes.cbegin(); it != externalParameterTypes.cend(); ++it) + { + solAssert(!!(*it), "Parameter should have external type"); + ret += (*it)->toString() + (it + 1 == externalParameterTypes.cend() ? "" : ","); + } return ret + ")"; } diff --git a/libsolidity/Types.h b/libsolidity/Types.h index c90aabda1..99fd878f0 100644 --- a/libsolidity/Types.h +++ b/libsolidity/Types.h @@ -187,6 +187,10 @@ public: "for type without literals.")); } + /// @returns a type suitable for outside of Solidity, i.e. for contract types it returns address. + /// If there is no such type, returns an empty shared pointer. + virtual TypePointer externalType() const { return TypePointer(); } + protected: /// Convenience object used when returning an empty member list. static const MemberList EmptyMemberList; @@ -217,10 +221,12 @@ public: virtual unsigned getStorageBytes() const override { return m_bits / 8; } virtual bool isValueType() const override { return true; } - virtual MemberList const& getMembers() const { return isAddress() ? AddressMemberList : EmptyMemberList; } + virtual MemberList const& getMembers() const override { return isAddress() ? AddressMemberList : EmptyMemberList; } virtual std::string toString() const override; + virtual TypePointer externalType() const override { return shared_from_this(); } + int getNumBits() const { return m_bits; } bool isAddress() const { return m_modifier == Modifier::Address; } bool isSigned() const { return m_modifier == Modifier::Signed; } @@ -292,6 +298,7 @@ public: virtual std::string toString() const override { return "bytes" + dev::toString(m_bytes); } virtual u256 literalValue(Literal const* _literal) const override; + virtual TypePointer externalType() const override { return shared_from_this(); } int getNumBytes() const { return m_bytes; } @@ -311,12 +318,13 @@ public: virtual TypePointer unaryOperatorResult(Token::Value _operator) const override; virtual TypePointer binaryOperatorResult(Token::Value _operator, TypePointer const& _other) const override; - virtual unsigned getCalldataEncodedSize(bool _padded) const { return _padded ? 32 : 1; } + virtual unsigned getCalldataEncodedSize(bool _padded) const override{ return _padded ? 32 : 1; } virtual unsigned getStorageBytes() const override { return 1; } virtual bool isValueType() const override { return true; } virtual std::string toString() const override { return "bool"; } virtual u256 literalValue(Literal const* _literal) const override; + virtual TypePointer externalType() const override { return shared_from_this(); } }; /** @@ -361,6 +369,7 @@ public: virtual unsigned getSizeOnStack() const override; virtual std::string toString() const override; virtual MemberList const& getMembers() const override { return s_arrayTypeMemberList; } + virtual TypePointer externalType() const override; Location getLocation() const { return m_location; } bool isByteArray() const { return m_isByteArray; } @@ -400,6 +409,7 @@ public: virtual std::string toString() const override; virtual MemberList const& getMembers() const override; + virtual TypePointer externalType() const override { return std::make_shared(160, IntegerType::Modifier::Address); } bool isSuper() const { return m_super; } ContractDefinition const& getContractDefinition() const { return m_contract; } @@ -468,6 +478,7 @@ public: virtual bool isValueType() const override { return true; } virtual bool isExplicitlyConvertibleTo(Type const& _convertTo) const override; + virtual TypePointer externalType() const override { return std::make_shared(8 * int(getStorageBytes())); } EnumDefinition const& getEnumDefinition() const { return m_enum; } /// @returns the value that the string has in the Enum @@ -500,6 +511,9 @@ public: Bare }; virtual Category getCategory() const override { return Category::Function; } + + virtual TypePointer externalType() const override; + explicit FunctionType(FunctionDefinition const& _function, bool _isInternal = true); explicit FunctionType(VariableDeclaration const& _varDecl); explicit FunctionType(EventDefinition const& _event); @@ -539,10 +553,10 @@ public: virtual MemberList const& getMembers() const override; Location const& getLocation() const { return m_location; } - /// @returns the canonical signature of this function type given the function name + /// @returns the external signature of this function type given the function name /// If @a _name is not provided (empty string) then the @c m_declaration member of the /// function type is used - std::string getCanonicalSignature(std::string const& _name = "") const; + std::string externalSignature(std::string const& _name = "") const; Declaration const& getDeclaration() const { solAssert(m_declaration, "Requested declaration from a FunctionType that has none"); diff --git a/mix/QFunctionDefinition.cpp b/mix/QFunctionDefinition.cpp index fed4e8add..13dbd4821 100644 --- a/mix/QFunctionDefinition.cpp +++ b/mix/QFunctionDefinition.cpp @@ -28,7 +28,7 @@ using namespace dev::solidity; using namespace dev::mix; -QFunctionDefinition::QFunctionDefinition(QObject* _parent, dev::solidity::FunctionTypePointer const& _f): QBasicNodeDefinition(_parent, &_f->getDeclaration()), m_hash(dev::sha3(_f->getCanonicalSignature())) +QFunctionDefinition::QFunctionDefinition(QObject* _parent, dev::solidity::FunctionTypePointer const& _f): QBasicNodeDefinition(_parent, &_f->getDeclaration()), m_hash(dev::sha3(_f->externalSignature())) { auto paramNames = _f->getParameterNames(); auto paramTypes = _f->getParameterTypes(); diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index 591cf053a..3bec70a2b 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include "TestHelper.h" using namespace std; @@ -48,16 +49,28 @@ ASTPointer parseTextAndResolveNames(std::string const& _source) ASTPointer sourceUnit = parser.parse(std::make_shared(CharStream(_source))); NameAndTypeResolver resolver({}); resolver.registerDeclarations(*sourceUnit); + std::shared_ptr globalContext = make_shared(); + for (ASTPointer const& node: sourceUnit->getNodes()) if (ContractDefinition* contract = dynamic_cast(node.get())) + { + globalContext->setCurrentContract(*contract); + resolver.updateDeclaration(*globalContext->getCurrentThis()); + resolver.updateDeclaration(*globalContext->getCurrentSuper()); resolver.resolveNamesAndTypes(*contract); + } for (ASTPointer const& node: sourceUnit->getNodes()) if (ContractDefinition* contract = dynamic_cast(node.get())) + { + globalContext->setCurrentContract(*contract); + resolver.updateDeclaration(*globalContext->getCurrentThis()); resolver.checkTypeRequirements(*contract); + } return sourceUnit; } + static ContractDefinition const* retrieveContract(ASTPointer _source, unsigned index) { ContractDefinition* contract; @@ -359,7 +372,7 @@ BOOST_AUTO_TEST_CASE(function_canonical_signature) if (ContractDefinition* contract = dynamic_cast(node.get())) { auto functions = contract->getDefinedFunctions(); - BOOST_CHECK_EQUAL("foo(uint256,uint64,bool)", functions[0]->getCanonicalSignature()); + BOOST_CHECK_EQUAL("foo(uint256,uint64,bool)", functions[0]->externalSignature()); } } @@ -376,8 +389,93 @@ BOOST_AUTO_TEST_CASE(function_canonical_signature_type_aliases) if (ContractDefinition* contract = dynamic_cast(node.get())) { auto functions = contract->getDefinedFunctions(); - BOOST_CHECK_EQUAL("boo(uint256,bytes32,address)", functions[0]->getCanonicalSignature()); + if (functions.empty()) + continue; + BOOST_CHECK_EQUAL("boo(uint256,bytes32,address)", functions[0]->externalSignature()); + } +} + +BOOST_AUTO_TEST_CASE(function_external_types) +{ + ASTPointer sourceUnit; + char const* text = R"( + contract C { + uint a; + } + contract Test { + function boo(uint arg2, bool arg3, bytes8 arg4, bool[2] pairs, uint[] dynamic, C carg) external returns (uint ret) { + ret = 5; + } + })"; + ETH_TEST_REQUIRE_NO_THROW(sourceUnit = parseTextAndResolveNames(text), "Parsing and name Resolving failed"); + for (ASTPointer const& node: sourceUnit->getNodes()) + if (ContractDefinition* contract = dynamic_cast(node.get())) + { + auto functions = contract->getDefinedFunctions(); + if (functions.empty()) + continue; + BOOST_CHECK_EQUAL("boo(uint256,bool,bytes8,bool[2],uint256[],address)", functions[0]->externalSignature()); + } +} + +BOOST_AUTO_TEST_CASE(function_external_call_allowed_conversion) +{ + char const* text = R"( + contract C {} + contract Test { + function externalCall() { + C arg; + this.g(arg); + } + function g (C c) external {} + })"; + BOOST_CHECK_NO_THROW(parseTextAndResolveNames(text)); +} + +BOOST_AUTO_TEST_CASE(function_external_call_not_allowed_conversion) +{ + char const* text = R"( + contract C {} + contract Test { + function externalCall() { + address arg; + this.g(arg); + } + function g (C c) external {} + })"; + BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError); +} + +BOOST_AUTO_TEST_CASE(function_internal_allowed_conversion) +{ + char const* text = R"( + contract C { + uint a; } + contract Test { + C a; + function g (C c) {} + function internalCall() { + g(a); + } + })"; + BOOST_CHECK_NO_THROW(parseTextAndResolveNames(text)); +} + +BOOST_AUTO_TEST_CASE(function_internal_not_allowed_conversion) +{ + char const* text = R"( + contract C { + uint a; + } + contract Test { + address a; + function g (C c) {} + function internalCall() { + g(a); + } + })"; + BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError); } BOOST_AUTO_TEST_CASE(hash_collision_in_interface)