From 2e72bd163a149183c119ca9664b98b0c5473da41 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 24 Jan 2017 12:44:49 +0100 Subject: [PATCH 01/16] Allow structs as part of function interfaces. --- libsolidity/ast/Types.cpp | 2 +- libsolidity/interface/ABI.cpp | 58 ++++++-- libsolidity/interface/ABI.h | 4 + test/libsolidity/SolidityABIJSON.cpp | 136 +++++++++++++++++- test/libsolidity/SolidityEndToEndTest.cpp | 41 ++++++ .../SolidityNameAndTypeResolution.cpp | 50 +++++++ 6 files changed, 280 insertions(+), 11 deletions(-) diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index 705d0f7f2..3a93b74e2 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -1770,7 +1770,7 @@ TypePointer StructType::interfaceType(bool _inLibrary) const if (_inLibrary && location() == DataLocation::Storage) return shared_from_this(); else - return TypePointer(); + return copyForLocation(DataLocation::Memory, true); } TypePointer StructType::copyForLocation(DataLocation _location, bool _isPointer) const diff --git a/libsolidity/interface/ABI.cpp b/libsolidity/interface/ABI.cpp index 49df843d8..7c7496cdd 100644 --- a/libsolidity/interface/ABI.cpp +++ b/libsolidity/interface/ABI.cpp @@ -86,12 +86,12 @@ Json::Value ABI::generate(ContractDefinition const& _contractDef) Json::Value params(Json::arrayValue); for (auto const& p: it->parameters()) { - solAssert(!!p->annotation().type->interfaceType(false), ""); + auto type = p->annotation().type->interfaceType(false); + solAssert(type, ""); Json::Value input; - input["name"] = p->name(); - input["type"] = p->annotation().type->interfaceType(false)->canonicalName(false); - input["indexed"] = p->isIndexed(); - params.append(input); + auto param = formatType(p->name(), *type, false); + param["indexed"] = p->isIndexed(); + params.append(param); } event["inputs"] = params; abi.append(event); @@ -111,10 +111,50 @@ Json::Value ABI::formatTypeList( for (unsigned i = 0; i < _names.size(); ++i) { solAssert(_types[i], ""); - Json::Value param; - param["name"] = _names[i]; - param["type"] = _types[i]->canonicalName(_forLibrary); - params.append(param); + params.append(formatType(_names[i], *_types[i], _forLibrary)); } return params; } + +Json::Value ABI::formatType(string const& _name, Type const& _type, bool _forLibrary) +{ + Json::Value ret; + ret["name"] = _name; + if (_type.isValueType() || (_forLibrary && _type.dataStoredIn(DataLocation::Storage))) + ret["type"] = _type.canonicalName(_forLibrary); + else if (ArrayType const* arrayType = dynamic_cast(&_type)) + { + if (arrayType->isByteArray()) + ret["type"] = _type.canonicalName(_forLibrary); + else + { + string suffix; + if (arrayType->isDynamicallySized()) + suffix = "[]"; + else + suffix = string("[") + arrayType->length().str() + "]"; + solAssert(arrayType->baseType(), ""); + Json::Value subtype = formatType("", *arrayType->baseType(), _forLibrary); + if (subtype["type"].isString() && !subtype.isMember("subtype")) + ret["type"] = subtype["type"].asString() + suffix; + else + { + ret["type"] = suffix; + solAssert(!subtype.isMember("subtype"), ""); + ret["subtype"] = subtype["type"]; + } + } + } + else if (StructType const* structType = dynamic_cast(&_type)) + { + ret["type"] = Json::arrayValue; + for (auto const& member: structType->members(nullptr)) + { + solAssert(member.type, ""); + ret["type"].append(formatType(member.name, *member.type, _forLibrary)); + } + } + else + solAssert(false, "Invalid type."); + return ret; +} diff --git a/libsolidity/interface/ABI.h b/libsolidity/interface/ABI.h index 95b162a95..7e42909b6 100644 --- a/libsolidity/interface/ABI.h +++ b/libsolidity/interface/ABI.h @@ -50,6 +50,10 @@ private: std::vector const& _types, bool _forLibrary ); + /// @returns a Json object with "name", "type" and potentially "subtype" keys, according + /// to the ABI specification. + /// If it is possible to express the type as a single string, it is allowed to return a single string. + static Json::Value formatType(std::string const& _name, Type const& _type, bool _forLibrary); }; } diff --git a/test/libsolidity/SolidityABIJSON.cpp b/test/libsolidity/SolidityABIJSON.cpp index 4b9223de3..7f03285d3 100644 --- a/test/libsolidity/SolidityABIJSON.cpp +++ b/test/libsolidity/SolidityABIJSON.cpp @@ -48,7 +48,7 @@ public: Json::Value generatedInterface = m_compilerStack.contractABI(""); Json::Value expectedInterface; - m_reader.parse(_expectedInterfaceString, expectedInterface); + BOOST_REQUIRE(m_reader.parse(_expectedInterfaceString, expectedInterface)); BOOST_CHECK_MESSAGE( expectedInterface == generatedInterface, "Expected:\n" << expectedInterface.toStyledString() << @@ -939,6 +939,140 @@ BOOST_AUTO_TEST_CASE(function_type) checkInterface(sourceCode, interface); } +BOOST_AUTO_TEST_CASE(return_structs) +{ + char const* text = R"( + contract C { + struct S { uint a; T[] sub; } + struct T { uint[2] x; } + function f() returns (uint x, S s) { + } + } + )"; + char const* interface = R"( + [ + { + "constant" : false, + "payable": false, + "inputs": [], + "name": "f", + "outputs": [{ + "name": "x", + "type": "uint256" + }, { + "name": "s", + "type": [{ + "name": "a", + "type": "uint256" + }, { + "name": "sub", + "subtype": [{ + "name": "x", + "type": "uint256[2]" + }], + "type": "[]" + }] + }], + "type" : "function" + } + ] + )"; + checkInterface(text, interface); +} + +BOOST_AUTO_TEST_CASE(event_structs) +{ + char const* text = R"( + contract C { + struct S { uint a; T[] sub; bytes b; } + struct T { uint[2] x; } + event E(T t, S s); + } + )"; + char const* interface = R"( + [{ + "anonymous" : false, + "inputs" : [{ + "indexed" : false, + "name" : "t", + "type" : [{ + "name" : "x", + "type" : "uint256[2]" + }] + }, { + "indexed" : false, + "name" : "s", + "type" : [{ + "name" : "a", + "type" : "uint256" + }, { + "name" : "sub", + "subtype" : [{ + "name" : "x", + "type" : "uint256[2]" + }], + "type" : "[]" + }, { + "name" : "b", + "type" : "bytes" + }] + }], + "name" : "E", + "type" : "event" + }] + )"; + checkInterface(text, interface); +} + +BOOST_AUTO_TEST_CASE(structs_in_libraries) +{ + char const* text = R"( + library L { + struct S { uint a; T[] sub; bytes b; } + struct T { uint[2] x; } + function f(L.S storage s) {} + function g(L.S s) {} + } + )"; + char const* interface = R"( + [{ + "constant" : false, + "inputs" : [{ + "name" : "s", + "type" : [{ + "name" : "a", + "type" : "uint256" + }, { + "name" : "sub", + "subtype" : [{ + "name" : "x", + "type" : "uint256[2]" + }], + "type" : "[]" + }, { + "name" : "b", + "type" : "bytes" + }] + }], + "name" : "g", + "outputs" : [], + "payable" : false, + "type" : "function" + }, { + "constant" : false, + "inputs" : [{ + "name" : "s", + "type" : "L.S storage" + }], + "name" : "f", + "outputs" : [], + "payable" : false, + "type" : "function" + }] + )"; + checkInterface(text, interface); +} + BOOST_AUTO_TEST_SUITE_END() } diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index ea924fcb9..6ea026738 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -9682,6 +9682,47 @@ BOOST_AUTO_TEST_CASE(contracts_separated_with_comment) compileAndRun(sourceCode, 0, "C2"); } +BOOST_AUTO_TEST_CASE(return_structs) +{ + char const* sourceCode = R"( + contract C { + struct S { uint a; T[] sub; } + struct T { uint[2] x; } + function f() returns (uint x, S s) { + x = 7; + s.a = 8; + s.sub = new S[](3); + s.sub[0][0] = 9; + s.sub[1][0] = 10; + s.sub[2][1] = 11; + } + } + )"; + compileAndRun(sourceCode, 0, "C"); + +// Will calculate the exact encoding later. +// BOOST_CHECK(callContractFunction("f()") == encodeArgs( +// u256(7), u256(0x40), +// u256(8), u256(0x40), +// u256(3), +// // s.sub[0] +// u256(9), u256(0xc0), +// // s.sub[1] +// u256(10), u256(0xe0), +// // s.sub[2] +// u256(11), u256(0x100), +// // s.sub[0].sub +// u256(2) +// // s.sub[0].sub[0].a +// u256(8), +// u256() +// // s.sub[1].sub +// u256(0) +// // s.sub[2].sub +// u256(2) +// )); +} + BOOST_AUTO_TEST_CASE(include_creation_bytecode_only_once) { char const* sourceCode = R"( diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 8c271fe10..037bc9a0c 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -5396,6 +5396,56 @@ BOOST_AUTO_TEST_CASE(constructible_internal_constructor) success(text); } +BOOST_AUTO_TEST_CASE(return_structs) +{ + char const* text = R"( + contract C { + struct S { uint a; T[] sub; } + struct T { uint[] x; } + function f() returns (uint x, S s) { + } + } + )"; + success(text); +} + +BOOST_AUTO_TEST_CASE(return_recursive_structs) +{ + char const* text = R"( + contract C { + struct S { uint a; S[] sub; } + function f() returns (uint x, S s) { + } + } + )"; + success(text); +} + +BOOST_AUTO_TEST_CASE(return_recursive_structs2) +{ + char const* text = R"( + contract C { + struct S { uint a; S[2] sub; } + function f() returns (uint x, S s) { + } + } + )"; + CHECK_ERROR(text, TypeError, "recursive data types in external functions."); +} + +BOOST_AUTO_TEST_CASE(return_recursive_structs3) +{ + char const* text = R"( + contract C { + struct S { uint a; S sub; } + struct T { S s; } + function f() returns (uint x, T t) { + } + } + )"; + CHECK_ERROR(text, TypeError, "recursive data types in external functions."); +} + BOOST_AUTO_TEST_CASE(address_checksum_type_deduction) { char const* text = R"( From 59ea19b3b957949fc53bfb5dc4e199d2196f8d18 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 1 Jun 2017 11:48:38 +0200 Subject: [PATCH 02/16] Check for recursive structs. --- libsolidity/analysis/TypeChecker.cpp | 6 +++--- libsolidity/ast/Types.cpp | 27 ++++++++++++++++++++++++++- libsolidity/ast/Types.h | 4 ++++ 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 26529c229..fe4207a3a 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -546,7 +546,7 @@ bool TypeChecker::visit(FunctionDefinition const& _function) if (!type(*var)->canLiveOutsideStorage()) m_errorReporter.typeError(var->location(), "Type is required to live outside storage."); if (_function.visibility() >= FunctionDefinition::Visibility::Public && !(type(*var)->interfaceType(isLibraryFunction))) - m_errorReporter.fatalTypeError(var->location(), "Internal type is not allowed for public or external functions."); + m_errorReporter.fatalTypeError(var->location(), "Internal or recursive type is not allowed for public or external functions."); var->accept(*this); } @@ -641,7 +641,7 @@ bool TypeChecker::visit(VariableDeclaration const& _variable) _variable.visibility() >= VariableDeclaration::Visibility::Public && !FunctionType(_variable).interfaceFunctionType() ) - m_errorReporter.typeError(_variable.location(), "Internal type is not allowed for public state variables."); + m_errorReporter.typeError(_variable.location(), "Internal or recursive type is not allowed for public state variables."); if (varType->category() == Type::Category::Array) if (auto arrayType = dynamic_cast(varType.get())) @@ -728,7 +728,7 @@ bool TypeChecker::visit(EventDefinition const& _eventDef) if (!type(*var)->canLiveOutsideStorage()) m_errorReporter.typeError(var->location(), "Type is required to live outside storage."); if (!type(*var)->interfaceType(false)) - m_errorReporter.typeError(var->location(), "Internal type is not allowed as event parameter type."); + m_errorReporter.typeError(var->location(), "Internal or recursive type is not allowed as event parameter type."); } if (_eventDef.isAnonymous() && numIndexed > 4) m_errorReporter.typeError(_eventDef.location(), "More than 4 indexed arguments for anonymous event."); diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index 3a93b74e2..443164030 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -1769,8 +1769,10 @@ TypePointer StructType::interfaceType(bool _inLibrary) const { if (_inLibrary && location() == DataLocation::Storage) return shared_from_this(); - else + else if (!recursive()) return copyForLocation(DataLocation::Memory, true); + else + return TypePointer(); } TypePointer StructType::copyForLocation(DataLocation _location, bool _isPointer) const @@ -1836,6 +1838,29 @@ set StructType::membersMissingInMemory() const return missing; } +bool StructType::recursive() const +{ + set structsSeen; + function check = [&](StructType const* t) -> bool + { + StructDefinition const* str = &t->structDefinition(); + if (structsSeen.count(str)) + return true; + structsSeen.insert(str); + for (ASTPointer const& variable: str->members()) + { + Type const* memberType = variable->annotation().type.get(); + while (dynamic_cast(memberType)) + memberType = dynamic_cast(memberType)->baseType().get(); + if (StructType const* innerStruct = dynamic_cast(memberType)) + if (check(innerStruct)) + return true; + } + return false; + }; + return check(this); +} + TypePointer EnumType::unaryOperatorResult(Token::Value _operator) const { return _operator == Token::Delete ? make_shared() : TypePointer(); diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h index d4d6da690..e6d3a7b1c 100644 --- a/libsolidity/ast/Types.h +++ b/libsolidity/ast/Types.h @@ -758,6 +758,10 @@ public: /// @returns the set of all members that are removed in the memory version (typically mappings). std::set membersMissingInMemory() const; + /// @returns true if the same struct is used recursively in one of its members. Only + /// analyses the "memory" representation, i.e. mappings are ignored in all structs. + bool recursive() const; + private: StructDefinition const& m_struct; }; From 22f85d5af371bb621f8735957b7519d14e3b40c9 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 1 Jun 2017 12:26:13 +0200 Subject: [PATCH 03/16] Update tests and error messages. --- libsolidity/codegen/ContractCompiler.cpp | 2 +- test/libsolidity/SolidityEndToEndTest.cpp | 8 ++++---- .../SolidityNameAndTypeResolution.cpp | 18 +++++++++--------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 514963681..92782b8d7 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -333,7 +333,7 @@ void ContractCompiler::appendCalldataUnpacker(TypePointers const& _typeParameter { // stack: v1 v2 ... v(k-1) base_offset current_offset TypePointer type = parameterType->decodingType(); - solAssert(type, "No decoding type found."); + solUnimplementedAssert(type, "No decoding type found."); if (type->category() == Type::Category::Array) { auto const& arrayType = dynamic_cast(*type); diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 6ea026738..3b657e39d 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -9691,10 +9691,10 @@ BOOST_AUTO_TEST_CASE(return_structs) function f() returns (uint x, S s) { x = 7; s.a = 8; - s.sub = new S[](3); - s.sub[0][0] = 9; - s.sub[1][0] = 10; - s.sub[2][1] = 11; + s.sub = new T[](3); + s.sub[0].x[0] = 9; + s.sub[1].x[0] = 10; + s.sub[2].x[1] = 11; } } )"; diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 037bc9a0c..162190bf7 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -3283,7 +3283,7 @@ BOOST_AUTO_TEST_CASE(library_memory_struct) function f() public returns (S ) {} } )"; - CHECK_ERROR(text, TypeError, "Internal type is not allowed for public or external functions."); + CHECK_SUCCESS(text); } BOOST_AUTO_TEST_CASE(using_for_arbitrary_mismatch) @@ -5402,7 +5402,7 @@ BOOST_AUTO_TEST_CASE(return_structs) contract C { struct S { uint a; T[] sub; } struct T { uint[] x; } - function f() returns (uint x, S s) { + function f() returns (uint, S) { } } )"; @@ -5414,36 +5414,36 @@ BOOST_AUTO_TEST_CASE(return_recursive_structs) char const* text = R"( contract C { struct S { uint a; S[] sub; } - function f() returns (uint x, S s) { + function f() returns (uint, S) { } } )"; - success(text); + CHECK_ERROR(text, TypeError, "Internal or recursive type is not allowed for public or external functions."); } BOOST_AUTO_TEST_CASE(return_recursive_structs2) { char const* text = R"( contract C { - struct S { uint a; S[2] sub; } - function f() returns (uint x, S s) { + struct S { uint a; S[2][] sub; } + function f() returns (uint, S) { } } )"; - CHECK_ERROR(text, TypeError, "recursive data types in external functions."); + CHECK_ERROR(text, TypeError, "Internal or recursive type is not allowed for public or external functions."); } BOOST_AUTO_TEST_CASE(return_recursive_structs3) { char const* text = R"( contract C { - struct S { uint a; S sub; } + struct S { uint a; S[][][] sub; } struct T { S s; } function f() returns (uint x, T t) { } } )"; - CHECK_ERROR(text, TypeError, "recursive data types in external functions."); + CHECK_ERROR(text, TypeError, "Internal or recursive type is not allowed for public or external functions."); } BOOST_AUTO_TEST_CASE(address_checksum_type_deduction) From 080be885f835462f0074b05c617fa670402b067a Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 8 Jun 2017 11:14:58 +0200 Subject: [PATCH 04/16] Function signatures containing structs. --- libsolidity/ast/Types.cpp | 90 +++++++++++++------ libsolidity/ast/Types.h | 28 ++++-- libsolidity/interface/ABI.cpp | 5 +- .../SolidityNameAndTypeResolution.cpp | 20 +++-- 4 files changed, 98 insertions(+), 45 deletions(-) diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index 443164030..9fdfe6320 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include @@ -1470,7 +1471,7 @@ string ArrayType::toString(bool _short) const return ret; } -string ArrayType::canonicalName(bool _addDataLocation) const +string ArrayType::canonicalName() const { string ret; if (isString()) @@ -1479,16 +1480,29 @@ string ArrayType::canonicalName(bool _addDataLocation) const ret = "bytes"; else { - ret = baseType()->canonicalName(false) + "["; + ret = baseType()->canonicalName() + "["; if (!isDynamicallySized()) ret += length().str(); ret += "]"; } - if (_addDataLocation && location() == DataLocation::Storage) - ret += " storage"; return ret; } +string ArrayType::signatureInExternalFunction(bool _structsByName) const +{ + if (isByteArray()) + return canonicalName(); + else + { + solAssert(baseType(), ""); + return + baseType()->signatureInExternalFunction(_structsByName) + + "[" + + (isDynamicallySized() ? "" : length().str()) + + "]"; + } +} + MemberList::MemberMap ArrayType::nativeMembers(ContractDefinition const*) const { MemberList::MemberMap members; @@ -1597,7 +1611,7 @@ string ContractType::toString(bool) const m_contract.name(); } -string ContractType::canonicalName(bool) const +string ContractType::canonicalName() const { return m_contract.annotation().canonicalName; } @@ -1727,9 +1741,8 @@ bool StructType::isDynamicallyEncoded() const u256 StructType::memorySize() const { u256 size; - for (auto const& member: members(nullptr)) - if (member.type->canLiveOutsideStorage()) - size += member.type->memoryHeadSize(); + for (auto const& t: memoryMemberTypes()) + size += t->memoryHeadSize(); return size; } @@ -1782,12 +1795,25 @@ TypePointer StructType::copyForLocation(DataLocation _location, bool _isPointer) return copy; } -string StructType::canonicalName(bool _addDataLocation) const +string StructType::signatureInExternalFunction(bool _structsByName) const { - string ret = m_struct.annotation().canonicalName; - if (_addDataLocation && location() == DataLocation::Storage) - ret += " storage"; - return ret; + if (_structsByName) + return canonicalName(); + else + { + TypePointers memberTypes = memoryMemberTypes(); + auto memberTypeStrings = memberTypes | boost::adaptors::transformed([&](TypePointer _t) -> string + { + solAssert(_t, "Parameter should have external type."); + return _t->signatureInExternalFunction(_structsByName); + }); + return "(" + boost::algorithm::join(memberTypeStrings, ",") + ")"; + } +} + +string StructType::canonicalName() const +{ + return m_struct.annotation().canonicalName; } FunctionTypePointer StructType::constructorType() const @@ -1829,6 +1855,15 @@ u256 StructType::memoryOffsetOfMember(string const& _name) const return 0; } +TypePointers StructType::memoryMemberTypes() const +{ + TypePointers types; + for (ASTPointer const& variable: m_struct.members()) + if (variable->annotation().type->canLiveOutsideStorage()) + types.push_back(variable->annotation().type); + return types; +} + set StructType::membersMissingInMemory() const { set missing; @@ -1893,7 +1928,7 @@ string EnumType::toString(bool) const return string("enum ") + m_enum.annotation().canonicalName; } -string EnumType::canonicalName(bool) const +string EnumType::canonicalName() const { return m_enum.annotation().canonicalName; } @@ -2320,7 +2355,7 @@ TypePointer FunctionType::binaryOperatorResult(Token::Value _operator, TypePoint return TypePointer(); } -string FunctionType::canonicalName(bool) const +string FunctionType::canonicalName() const { solAssert(m_kind == Kind::External, ""); return "function"; @@ -2580,20 +2615,19 @@ string FunctionType::externalSignature() const solAssert(m_declaration != nullptr, "External signature of function needs declaration"); solAssert(!m_declaration->name().empty(), "Fallback function has no signature."); - bool _inLibrary = dynamic_cast(*m_declaration->scope()).isLibrary(); - - string ret = m_declaration->name() + "("; - + bool const inLibrary = dynamic_cast(*m_declaration->scope()).isLibrary(); FunctionTypePointer external = interfaceFunctionType(); solAssert(!!external, "External function type requested."); - TypePointers externalParameterTypes = external->parameterTypes(); - for (auto it = externalParameterTypes.cbegin(); it != externalParameterTypes.cend(); ++it) + auto parameterTypes = external->parameterTypes(); + auto typeStrings = parameterTypes | boost::adaptors::transformed([&](TypePointer _t) -> string { - solAssert(!!(*it), "Parameter should have external type"); - ret += (*it)->canonicalName(_inLibrary) + (it + 1 == externalParameterTypes.cend() ? "" : ","); - } - - return ret + ")"; + solAssert(_t, "Parameter should have external type."); + string typeName = _t->signatureInExternalFunction(inLibrary); + if (inLibrary && _t->dataStoredIn(DataLocation::Storage)) + typeName += " storage"; + return typeName; + }); + return m_declaration->name() + "(" + boost::algorithm::join(typeStrings, ",") + ")"; } u256 FunctionType::externalIdentifier() const @@ -2724,9 +2758,9 @@ string MappingType::toString(bool _short) const return "mapping(" + keyType()->toString(_short) + " => " + valueType()->toString(_short) + ")"; } -string MappingType::canonicalName(bool) const +string MappingType::canonicalName() const { - return "mapping(" + keyType()->canonicalName(false) + " => " + valueType()->canonicalName(false) + ")"; + return "mapping(" + keyType()->canonicalName() + " => " + valueType()->canonicalName() + ")"; } string TypeType::identifier() const diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h index e6d3a7b1c..0713f5276 100644 --- a/libsolidity/ast/Types.h +++ b/libsolidity/ast/Types.h @@ -245,9 +245,15 @@ public: virtual std::string toString(bool _short) const = 0; std::string toString() const { return toString(false); } - /// @returns the canonical name of this type for use in function signatures. - /// @param _addDataLocation if true, includes data location for reference types if it is "storage". - virtual std::string canonicalName(bool /*_addDataLocation*/) const { return toString(true); } + /// @returns the canonical name of this type for use in library function signatures. + virtual std::string canonicalName() const { return toString(true); } + /// @returns the signature of this type in external functions, i.e. `uint256` for integers + /// or `(uint256, bytes8)[2]` for an array of structs. If @a _structsByName, + /// structs are given by canonical name like `ContractName.StructName[2]`. + virtual std::string signatureInExternalFunction(bool /*_structsByName*/) const + { + return canonicalName(); + } virtual u256 literalValue(Literal const*) const { solAssert(false, "Literal value requested for type without literals."); @@ -619,7 +625,8 @@ public: virtual bool canLiveOutsideStorage() const override { return m_baseType->canLiveOutsideStorage(); } virtual unsigned sizeOnStack() const override; virtual std::string toString(bool _short) const override; - virtual std::string canonicalName(bool _addDataLocation) const override; + virtual std::string canonicalName() const override; + virtual std::string signatureInExternalFunction(bool _structsByName) const override; virtual MemberList::MemberMap nativeMembers(ContractDefinition const* _currentScope) const override; virtual TypePointer encodingType() const override; virtual TypePointer decodingType() const override; @@ -677,7 +684,7 @@ public: virtual unsigned sizeOnStack() const override { return m_super ? 0 : 1; } virtual bool isValueType() const override { return true; } virtual std::string toString(bool _short) const override; - virtual std::string canonicalName(bool _addDataLocation) const override; + virtual std::string canonicalName() const override; virtual MemberList::MemberMap nativeMembers(ContractDefinition const* _currentScope) const override; virtual TypePointer encodingType() const override @@ -744,7 +751,8 @@ public: TypePointer copyForLocation(DataLocation _location, bool _isPointer) const override; - virtual std::string canonicalName(bool _addDataLocation) const override; + virtual std::string canonicalName() const override; + virtual std::string signatureInExternalFunction(bool _structsByName) const override; /// @returns a function that peforms the type conversion between a list of struct members /// and a memory struct of this type. @@ -755,6 +763,8 @@ public: StructDefinition const& structDefinition() const { return m_struct; } + /// @returns the vector of types of members available in memory. + TypePointers memoryMemberTypes() const; /// @returns the set of all members that are removed in the memory version (typically mappings). std::set membersMissingInMemory() const; @@ -784,7 +794,7 @@ public: virtual unsigned storageBytes() const override; virtual bool canLiveOutsideStorage() const override { return true; } virtual std::string toString(bool _short) const override; - virtual std::string canonicalName(bool _addDataLocation) const override; + virtual std::string canonicalName() const override; virtual bool isValueType() const override { return true; } virtual bool isExplicitlyConvertibleTo(Type const& _convertTo) const override; @@ -955,7 +965,7 @@ public: virtual bool isExplicitlyConvertibleTo(Type const& _convertTo) const override; virtual TypePointer unaryOperatorResult(Token::Value _operator) const override; virtual TypePointer binaryOperatorResult(Token::Value, TypePointer const&) const override; - virtual std::string canonicalName(bool /*_addDataLocation*/) const override; + virtual std::string canonicalName() const override; virtual std::string toString(bool _short) const override; virtual unsigned calldataEncodedSize(bool _padded) const override; virtual bool canBeStored() const override { return m_kind == Kind::Internal || m_kind == Kind::External; } @@ -1057,7 +1067,7 @@ public: virtual std::string identifier() const override; virtual bool operator==(Type const& _other) const override; virtual std::string toString(bool _short) const override; - virtual std::string canonicalName(bool _addDataLocation) const override; + virtual std::string canonicalName() const override; virtual bool canLiveOutsideStorage() const override { return false; } virtual TypePointer binaryOperatorResult(Token::Value, TypePointer const&) const override { return TypePointer(); } virtual TypePointer encodingType() const override diff --git a/libsolidity/interface/ABI.cpp b/libsolidity/interface/ABI.cpp index 7c7496cdd..c04de57ec 100644 --- a/libsolidity/interface/ABI.cpp +++ b/libsolidity/interface/ABI.cpp @@ -120,12 +120,13 @@ Json::Value ABI::formatType(string const& _name, Type const& _type, bool _forLib { Json::Value ret; ret["name"] = _name; + string suffix = (_forLibrary && _type.dataStoredIn(DataLocation::Storage)) ? " storage" : ""; if (_type.isValueType() || (_forLibrary && _type.dataStoredIn(DataLocation::Storage))) - ret["type"] = _type.canonicalName(_forLibrary); + ret["type"] = _type.canonicalName() + suffix; else if (ArrayType const* arrayType = dynamic_cast(&_type)) { if (arrayType->isByteArray()) - ret["type"] = _type.canonicalName(_forLibrary); + ret["type"] = _type.canonicalName() + suffix; else { string suffix; diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 162190bf7..3cea8d603 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -599,6 +599,14 @@ BOOST_AUTO_TEST_CASE(enum_external_type) } } +BOOST_AUTO_TEST_CASE(external_structs) +{ + BOOST_FAIL("This should test external structs"); + // More test ideas: + // external function type as part of a struct and array + // external function type taking structs and arrays... +} + BOOST_AUTO_TEST_CASE(function_external_call_allowed_conversion) { char const* text = R"( @@ -980,24 +988,24 @@ BOOST_AUTO_TEST_CASE(state_variable_accessors) FunctionTypePointer function = retrieveFunctionBySignature(*contract, "foo()"); BOOST_REQUIRE(function && function->hasDeclaration()); auto returnParams = function->returnParameterTypes(); - BOOST_CHECK_EQUAL(returnParams.at(0)->canonicalName(false), "uint256"); + BOOST_CHECK_EQUAL(returnParams.at(0)->canonicalName(), "uint256"); BOOST_CHECK(function->stateMutability() == StateMutability::View); function = retrieveFunctionBySignature(*contract, "map(uint256)"); BOOST_REQUIRE(function && function->hasDeclaration()); auto params = function->parameterTypes(); - BOOST_CHECK_EQUAL(params.at(0)->canonicalName(false), "uint256"); + BOOST_CHECK_EQUAL(params.at(0)->canonicalName(), "uint256"); returnParams = function->returnParameterTypes(); - BOOST_CHECK_EQUAL(returnParams.at(0)->canonicalName(false), "bytes4"); + BOOST_CHECK_EQUAL(returnParams.at(0)->canonicalName(), "bytes4"); BOOST_CHECK(function->stateMutability() == StateMutability::View); function = retrieveFunctionBySignature(*contract, "multiple_map(uint256,uint256)"); BOOST_REQUIRE(function && function->hasDeclaration()); params = function->parameterTypes(); - BOOST_CHECK_EQUAL(params.at(0)->canonicalName(false), "uint256"); - BOOST_CHECK_EQUAL(params.at(1)->canonicalName(false), "uint256"); + BOOST_CHECK_EQUAL(params.at(0)->canonicalName(), "uint256"); + BOOST_CHECK_EQUAL(params.at(1)->canonicalName(), "uint256"); returnParams = function->returnParameterTypes(); - BOOST_CHECK_EQUAL(returnParams.at(0)->canonicalName(false), "bytes4"); + BOOST_CHECK_EQUAL(returnParams.at(0)->canonicalName(), "bytes4"); BOOST_CHECK(function->stateMutability() == StateMutability::View); } From 823e67bf4014d20c6c83d509264e1464d9578f99 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 9 Jun 2017 18:28:13 +0200 Subject: [PATCH 05/16] Tests for external signatures. --- .../SolidityNameAndTypeResolution.cpp | 32 ++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 3cea8d603..9dfbea210 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -601,12 +601,36 @@ BOOST_AUTO_TEST_CASE(enum_external_type) BOOST_AUTO_TEST_CASE(external_structs) { - BOOST_FAIL("This should test external structs"); - // More test ideas: - // external function type as part of a struct and array - // external function type taking structs and arrays... + ASTPointer sourceUnit; + char const* text = R"( + contract Test { + enum ActionChoices { GoLeft, GoRight, GoStraight, Sit } + struct Empty {} + struct Nested { X[2][] a; mapping(uint => uint) m; uint y; } + struct X { bytes32 x; Test t; Empty[] e; } + function f(ActionChoices, uint, Empty) external {} + function g(Nested) external {} + function h(function(Nested) external returns (uint)[]) external {} + function i(Nested[]) external {} + } + )"; + ETH_TEST_REQUIRE_NO_THROW(sourceUnit = parseAndAnalyse(text), "Parsing and name Resolving failed"); + for (ASTPointer const& node: sourceUnit->nodes()) + if (ContractDefinition* contract = dynamic_cast(node.get())) + { + auto functions = contract->definedFunctions(); + BOOST_REQUIRE(!functions.empty()); + for (auto const& f: functions) + cout << f->externalSignature() << endl; + BOOST_CHECK_EQUAL("f(uint8,uint256,())", functions[0]->externalSignature()); + BOOST_CHECK_EQUAL("g(((bytes32,address,()[])[2][],uint256))", functions[1]->externalSignature()); + BOOST_CHECK_EQUAL("h(function[])", functions[2]->externalSignature()); + BOOST_CHECK_EQUAL("i(((bytes32,address,()[])[2][],uint256)[])", functions[3]->externalSignature()); + } } +// TODO: Add a test that checks the signature of library functions taking structs + BOOST_AUTO_TEST_CASE(function_external_call_allowed_conversion) { char const* text = R"( From 36a90289e65b06c54326a1c254baa5fa6029f766 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 13 Jun 2017 10:51:49 +0200 Subject: [PATCH 06/16] Fix interface type conversion internal to structs. --- libsolidity/ast/Types.cpp | 6 +++- libsolidity/interface/ABI.cpp | 4 ++- test/libsolidity/SolidityABIJSON.cpp | 36 +++++++++++++++++++ .../SolidityNameAndTypeResolution.cpp | 6 ++-- 4 files changed, 46 insertions(+), 6 deletions(-) diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index 9fdfe6320..14b30df82 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -1783,6 +1783,8 @@ TypePointer StructType::interfaceType(bool _inLibrary) const if (_inLibrary && location() == DataLocation::Storage) return shared_from_this(); else if (!recursive()) + // TODO this might not be enough, we have to convert all members to + // their interfaceType return copyForLocation(DataLocation::Memory, true); else return TypePointer(); @@ -1805,7 +1807,9 @@ string StructType::signatureInExternalFunction(bool _structsByName) const auto memberTypeStrings = memberTypes | boost::adaptors::transformed([&](TypePointer _t) -> string { solAssert(_t, "Parameter should have external type."); - return _t->signatureInExternalFunction(_structsByName); + auto t = _t->interfaceType(_structsByName); + solAssert(t, ""); + return t->signatureInExternalFunction(_structsByName); }); return "(" + boost::algorithm::join(memberTypeStrings, ",") + ")"; } diff --git a/libsolidity/interface/ABI.cpp b/libsolidity/interface/ABI.cpp index c04de57ec..0e28d0102 100644 --- a/libsolidity/interface/ABI.cpp +++ b/libsolidity/interface/ABI.cpp @@ -152,7 +152,9 @@ Json::Value ABI::formatType(string const& _name, Type const& _type, bool _forLib for (auto const& member: structType->members(nullptr)) { solAssert(member.type, ""); - ret["type"].append(formatType(member.name, *member.type, _forLibrary)); + auto t = member.type->interfaceType(_forLibrary); + solAssert(t, ""); + ret["type"].append(formatType(member.name, *t, _forLibrary)); } } else diff --git a/test/libsolidity/SolidityABIJSON.cpp b/test/libsolidity/SolidityABIJSON.cpp index 7f03285d3..3de8b732c 100644 --- a/test/libsolidity/SolidityABIJSON.cpp +++ b/test/libsolidity/SolidityABIJSON.cpp @@ -980,6 +980,42 @@ BOOST_AUTO_TEST_CASE(return_structs) checkInterface(text, interface); } +BOOST_AUTO_TEST_CASE(return_structs_with_contracts) +{ + char const* text = R"( + contract C { + struct S { C[] x; C y; } + function f() returns (S s, C c) { + } + } + )"; + char const* interface = R"( + [ + { + "constant" : false, + "payable": false, + "inputs": [], + "name": "f", + "outputs" : [{ + "name" : "s", + "type" : [{ + "name" : "x", + "type" : "address[]" + }, { + "name" : "y", + "type" : "address" + }] + }, { + "name" : "c", + "type" : "address" + }], + "type" : "function" + } + ] + )"; + checkInterface(text, interface); +} + BOOST_AUTO_TEST_CASE(event_structs) { char const* text = R"( diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 9dfbea210..dcab8fb00 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -609,7 +609,7 @@ BOOST_AUTO_TEST_CASE(external_structs) struct Nested { X[2][] a; mapping(uint => uint) m; uint y; } struct X { bytes32 x; Test t; Empty[] e; } function f(ActionChoices, uint, Empty) external {} - function g(Nested) external {} + function g(Test, Nested) external {} function h(function(Nested) external returns (uint)[]) external {} function i(Nested[]) external {} } @@ -620,10 +620,8 @@ BOOST_AUTO_TEST_CASE(external_structs) { auto functions = contract->definedFunctions(); BOOST_REQUIRE(!functions.empty()); - for (auto const& f: functions) - cout << f->externalSignature() << endl; BOOST_CHECK_EQUAL("f(uint8,uint256,())", functions[0]->externalSignature()); - BOOST_CHECK_EQUAL("g(((bytes32,address,()[])[2][],uint256))", functions[1]->externalSignature()); + BOOST_CHECK_EQUAL("g(address,((bytes32,address,()[])[2][],uint256))", functions[1]->externalSignature()); BOOST_CHECK_EQUAL("h(function[])", functions[2]->externalSignature()); BOOST_CHECK_EQUAL("i(((bytes32,address,()[])[2][],uint256)[])", functions[3]->externalSignature()); } From 7e1b9c16528b08a8924f85c57f3b08b6cec70584 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 30 Jun 2017 10:22:35 +0200 Subject: [PATCH 07/16] Structure type json using "components". --- libsolidity/interface/ABI.cpp | 16 +- test/libsolidity/SolidityABIJSON.cpp | 249 +++++++++++++++------------ 2 files changed, 151 insertions(+), 114 deletions(-) diff --git a/libsolidity/interface/ABI.cpp b/libsolidity/interface/ABI.cpp index 0e28d0102..9af7cdc37 100644 --- a/libsolidity/interface/ABI.cpp +++ b/libsolidity/interface/ABI.cpp @@ -136,25 +136,25 @@ Json::Value ABI::formatType(string const& _name, Type const& _type, bool _forLib suffix = string("[") + arrayType->length().str() + "]"; solAssert(arrayType->baseType(), ""); Json::Value subtype = formatType("", *arrayType->baseType(), _forLibrary); - if (subtype["type"].isString() && !subtype.isMember("subtype")) - ret["type"] = subtype["type"].asString() + suffix; - else + if (subtype.isMember("components")) { - ret["type"] = suffix; - solAssert(!subtype.isMember("subtype"), ""); - ret["subtype"] = subtype["type"]; + ret["type"] = subtype["type"].asString() + suffix; + ret["components"] = subtype["components"]; } + else + ret["type"] = subtype["type"].asString() + suffix; } } else if (StructType const* structType = dynamic_cast(&_type)) { - ret["type"] = Json::arrayValue; + ret["type"] = string(); + ret["components"] = Json::arrayValue; for (auto const& member: structType->members(nullptr)) { solAssert(member.type, ""); auto t = member.type->interfaceType(_forLibrary); solAssert(t, ""); - ret["type"].append(formatType(member.name, *t, _forLibrary)); + ret["components"].append(formatType(member.name, *t, _forLibrary)); } } else diff --git a/test/libsolidity/SolidityABIJSON.cpp b/test/libsolidity/SolidityABIJSON.cpp index 3de8b732c..5ddd28ad2 100644 --- a/test/libsolidity/SolidityABIJSON.cpp +++ b/test/libsolidity/SolidityABIJSON.cpp @@ -950,32 +950,39 @@ BOOST_AUTO_TEST_CASE(return_structs) } )"; char const* interface = R"( - [ - { + [{ "constant" : false, - "payable": false, - "inputs": [], - "name": "f", - "outputs": [{ - "name": "x", - "type": "uint256" - }, { - "name": "s", - "type": [{ - "name": "a", - "type": "uint256" - }, { - "name": "sub", - "subtype": [{ - "name": "x", - "type": "uint256[2]" - }], - "type": "[]" - }] - }], + "inputs" : [], + "name" : "f", + "outputs" : [ + { + "name" : "x", + "type" : "uint256" + }, + { + "components" : [ + { + "name" : "a", + "type" : "uint256" + }, + { + "components" : [ + { + "name" : "x", + "type" : "uint256[2]" + } + ], + "name" : "sub", + "type" : "[]" + } + ], + "name" : "s", + "type" : "" + } + ], + "payable" : false, "type" : "function" - } - ] + }] )"; checkInterface(text, interface); } @@ -990,28 +997,33 @@ BOOST_AUTO_TEST_CASE(return_structs_with_contracts) } )"; char const* interface = R"( - [ - { - "constant" : false, - "payable": false, + [{ + "constant": false, "inputs": [], "name": "f", - "outputs" : [{ - "name" : "s", - "type" : [{ - "name" : "x", - "type" : "address[]" - }, { - "name" : "y", - "type" : "address" - }] - }, { - "name" : "c", - "type" : "address" - }], - "type" : "function" - } - ] + "outputs": [ + { + "components": [ + { + "name": "x", + "type": "address[]" + }, + { + "name": "y", + "type": "address" + } + ], + "name": "s", + "type": "" + }, + { + "name": "c", + "type": "address" + } + ], + "payable": false, + "type": "function" + }] )"; checkInterface(text, interface); } @@ -1025,36 +1037,49 @@ BOOST_AUTO_TEST_CASE(event_structs) event E(T t, S s); } )"; - char const* interface = R"( - [{ - "anonymous" : false, - "inputs" : [{ - "indexed" : false, - "name" : "t", - "type" : [{ - "name" : "x", - "type" : "uint256[2]" - }] - }, { - "indexed" : false, - "name" : "s", - "type" : [{ - "name" : "a", - "type" : "uint256" - }, { - "name" : "sub", - "subtype" : [{ - "name" : "x", - "type" : "uint256[2]" - }], - "type" : "[]" - }, { - "name" : "b", - "type" : "bytes" - }] - }], - "name" : "E", - "type" : "event" + char const *interface = R"( + [{ + "anonymous": false, + "inputs": [ + { + "components": [ + { + "name": "x", + "type": "uint256[2]" + } + ], + "indexed": false, + "name": "t", + "type": "" + }, + { + "components": [ + { + "name": "a", + "type": "uint256" + }, + { + "components": [ + { + "name": "x", + "type": "uint256[2]" + } + ], + "name": "sub", + "type": "[]" + }, + { + "name": "b", + "type": "bytes" + } + ], + "indexed": false, + "name": "s", + "type": "" + } + ], + "name": "E", + "type": "event" }] )"; checkInterface(text, interface); @@ -1072,38 +1097,50 @@ BOOST_AUTO_TEST_CASE(structs_in_libraries) )"; char const* interface = R"( [{ - "constant" : false, - "inputs" : [{ - "name" : "s", - "type" : [{ - "name" : "a", - "type" : "uint256" - }, { - "name" : "sub", - "subtype" : [{ - "name" : "x", - "type" : "uint256[2]" - }], - "type" : "[]" - }, { - "name" : "b", - "type" : "bytes" - }] - }], - "name" : "g", - "outputs" : [], - "payable" : false, - "type" : "function" - }, { - "constant" : false, - "inputs" : [{ - "name" : "s", - "type" : "L.S storage" - }], - "name" : "f", - "outputs" : [], - "payable" : false, - "type" : "function" + "constant": false, + "inputs": [ + { + "components": [ + { + "name": "a", + "type": "uint256" + }, + { + "components": [ + { + "name": "x", + "type": "uint256[2]" + } + ], + "name": "sub", + "type": "[]" + }, + { + "name": "b", + "type": "bytes" + } + ], + "name": "s", + "type": "" + } + ], + "name": "g", + "outputs": [], + "payable": false, + "type": "function" + }, + { + "constant": false, + "inputs": [ + { + "name": "s", + "type": "L.S storage" + } + ], + "name": "f", + "outputs": [], + "payable": false, + "type": "function" }] )"; checkInterface(text, interface); From e4bb767dcdab6121f00933b5d6af64e029a9e93c Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 30 Jun 2017 10:49:19 +0200 Subject: [PATCH 08/16] Document structs in ABI --- docs/abi-spec.rst | 92 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 89 insertions(+), 3 deletions(-) diff --git a/docs/abi-spec.rst b/docs/abi-spec.rst index f75a6885c..6df9f1d53 100644 --- a/docs/abi-spec.rst +++ b/docs/abi-spec.rst @@ -289,14 +289,16 @@ In effect, a log entry using this ABI is described as: JSON ==== -The JSON format for a contract's interface is given by an array of function and/or event descriptions. A function description is a JSON object with the fields: +The JSON format for a contract's interface is given by an array of function and/or event descriptions. +A function description is a JSON object with the fields: - ``type``: ``"function"``, ``"constructor"``, or ``"fallback"`` (the :ref:`unnamed "default" function `); - ``name``: the name of the function; - ``inputs``: an array of objects, each of which contains: * ``name``: the name of the parameter; - * ``type``: the canonical type of the parameter. + * ``type``: the canonical type of the parameter (more below). + * ``components``: used for tuple types (more below). - ``outputs``: an array of objects similar to ``inputs``, can be omitted if function doesn't return anything; - ``payable``: ``true`` if function accepts ether, defaults to ``false``; @@ -316,7 +318,8 @@ An event description is a JSON object with fairly similar fields: - ``inputs``: an array of objects, each of which contains: * ``name``: the name of the parameter; - * ``type``: the canonical type of the parameter. + * ``type``: the canonical type of the parameter (more below). + * ``components``: used for tuple types (more below). * ``indexed``: ``true`` if the field is part of the log's topics, ``false`` if it one of the log's data segment. - ``anonymous``: ``true`` if the event was declared as ``anonymous``. @@ -353,3 +356,86 @@ would result in the JSON: "name":"foo", "outputs": [] }] + +Use of Structs in Types +----------------------- + +If structs are part of the type, we still want to know the name of the components. Because of that, +the json structure gets arbitrarily nested in the following way: + +An object with members ``name``, ``type`` and potentially ``components`` describes a typed variable. +The canonical type is determined until a struct type is reached and the string description up +to that point is stored in ``type``, i.e. it will be a sequence of ``[]`` and ``[k]`` with +integers ``k``. The components of the struct are then stored in the member ``components``, +which is of array type and has the same structure as the top-level object except that +``indexed`` is not allowed there. + +As an example, the code + +:: + + contract Test { + struct S { uint a; uint[] b; T[] c; } + struct T { uint x; uint y; } + function f(S s, T t, uint a) { } + } + +would result in the JSON: + +.. code:: json + + [ + { + "name": "f", + "type": "function", + "inputs": [ + { + "name": "s", + "type": "", + "components": [ + { + "name": "a", + "type": "uint256" + }, + { + "name": "b", + "type": "uint256[]" + }, + { + "name": "c", + "type": "[]", + "components": [ + { + "name": "x", + "type": "uint256" + }, + { + "name": "y", + "type": "uint256" + } + ] + } + ] + }, + { + "name": "t", + "type": "", + "components": [ + { + "name": "x", + "type": "uint256" + }, + { + "name": "y", + "type": "uint256" + } + ] + }, + { + "name": "a", + "type": "uint256" + } + ], + "outputs": [] + } + ] From 44825d1c1ecef6ea3fa27da6330c12dcf4279fb3 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 30 Jun 2017 14:30:30 +0200 Subject: [PATCH 09/16] Expect test to fail until implemented. --- test/libsolidity/SolidityEndToEndTest.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 3b657e39d..393d3c645 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -9698,7 +9698,10 @@ BOOST_AUTO_TEST_CASE(return_structs) } } )"; - compileAndRun(sourceCode, 0, "C"); + // This will throw "unimplemented" until it is implemented. + BOOST_CHECK_THROW( + compileAndRun(sourceCode, 0, "C"), + Exception); // Will calculate the exact encoding later. // BOOST_CHECK(callContractFunction("f()") == encodeArgs( From 6385641f6e461a1964da793956fbe0ef8cddadd3 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 24 Aug 2017 15:08:31 +0200 Subject: [PATCH 10/16] Fix tests. --- libsolidity/ast/Types.h | 2 +- libsolidity/interface/ABI.h | 2 +- test/libsolidity/SolidityABIJSON.cpp | 4 ++++ test/libsolidity/SolidityEndToEndTest.cpp | 1 + test/libsolidity/SolidityNameAndTypeResolution.cpp | 8 ++++---- 5 files changed, 11 insertions(+), 6 deletions(-) diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h index 0713f5276..dd50c5735 100644 --- a/libsolidity/ast/Types.h +++ b/libsolidity/ast/Types.h @@ -248,7 +248,7 @@ public: /// @returns the canonical name of this type for use in library function signatures. virtual std::string canonicalName() const { return toString(true); } /// @returns the signature of this type in external functions, i.e. `uint256` for integers - /// or `(uint256, bytes8)[2]` for an array of structs. If @a _structsByName, + /// or `(uint256,bytes8)[2]` for an array of structs. If @a _structsByName, /// structs are given by canonical name like `ContractName.StructName[2]`. virtual std::string signatureInExternalFunction(bool /*_structsByName*/) const { diff --git a/libsolidity/interface/ABI.h b/libsolidity/interface/ABI.h index 7e42909b6..db70729db 100644 --- a/libsolidity/interface/ABI.h +++ b/libsolidity/interface/ABI.h @@ -50,7 +50,7 @@ private: std::vector const& _types, bool _forLibrary ); - /// @returns a Json object with "name", "type" and potentially "subtype" keys, according + /// @returns a Json object with "name", "type" and potentially "components" keys, according /// to the ABI specification. /// If it is possible to express the type as a single string, it is allowed to return a single string. static Json::Value formatType(std::string const& _name, Type const& _type, bool _forLibrary); diff --git a/test/libsolidity/SolidityABIJSON.cpp b/test/libsolidity/SolidityABIJSON.cpp index 5ddd28ad2..e4ab54ec1 100644 --- a/test/libsolidity/SolidityABIJSON.cpp +++ b/test/libsolidity/SolidityABIJSON.cpp @@ -981,6 +981,7 @@ BOOST_AUTO_TEST_CASE(return_structs) } ], "payable" : false, + "stateMutability" : "nonpayable", "type" : "function" }] )"; @@ -1022,6 +1023,7 @@ BOOST_AUTO_TEST_CASE(return_structs_with_contracts) } ], "payable": false, + "stateMutability" : "nonpayable", "type": "function" }] )"; @@ -1127,6 +1129,7 @@ BOOST_AUTO_TEST_CASE(structs_in_libraries) "name": "g", "outputs": [], "payable": false, + "stateMutability": "nonpayable", "type": "function" }, { @@ -1140,6 +1143,7 @@ BOOST_AUTO_TEST_CASE(structs_in_libraries) "name": "f", "outputs": [], "payable": false, + "stateMutability": "nonpayable", "type": "function" }] )"; diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 393d3c645..2bdcbd483 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -9685,6 +9685,7 @@ BOOST_AUTO_TEST_CASE(contracts_separated_with_comment) BOOST_AUTO_TEST_CASE(return_structs) { char const* sourceCode = R"( + pragma experimental ABIEncoderV2; contract C { struct S { uint a; T[] sub; } struct T { uint[2] x; } diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index dcab8fb00..778c12c5c 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -1102,7 +1102,7 @@ BOOST_AUTO_TEST_CASE(struct_accessor_one_array_only) Data public data; } )"; - CHECK_ERROR(sourceCode, TypeError, "Internal type is not allowed for public state variables."); + CHECK_ERROR(sourceCode, TypeError, "Internal or recursive type is not allowed for public state variables."); } BOOST_AUTO_TEST_CASE(base_class_state_variable_internal_member) @@ -4904,7 +4904,7 @@ BOOST_AUTO_TEST_CASE(internal_function_as_external_parameter) } } )"; - CHECK_ERROR(text, TypeError, "Internal type is not allowed for public or external functions."); + CHECK_ERROR(text, TypeError, "Internal or recursive type is not allowed for public or external functions."); } BOOST_AUTO_TEST_CASE(internal_function_returned_from_public_function) @@ -4916,7 +4916,7 @@ BOOST_AUTO_TEST_CASE(internal_function_returned_from_public_function) } } )"; - CHECK_ERROR(text, TypeError, "Internal type is not allowed for public or external functions."); + CHECK_ERROR(text, TypeError, "Internal or recursive type is not allowed for public or external functions."); } BOOST_AUTO_TEST_CASE(internal_function_as_external_parameter_in_library_internal) @@ -4938,7 +4938,7 @@ BOOST_AUTO_TEST_CASE(internal_function_as_external_parameter_in_library_external } } )"; - CHECK_ERROR(text, TypeError, "Internal type is not allowed for public or external functions."); + CHECK_ERROR(text, TypeError, "Internal or recursive type is not allowed for public or external functions."); } BOOST_AUTO_TEST_CASE(function_type_arrays) From 70d70e78160069d28a6b4931c995d0b24c2b09d5 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 24 Aug 2017 15:20:49 +0200 Subject: [PATCH 11/16] Implement struct encoder. --- libsolidity/ast/Types.cpp | 10 +- libsolidity/ast/Types.h | 2 +- libsolidity/codegen/ABIFunctions.cpp | 128 +++++++++++++++++++++- libsolidity/codegen/ABIFunctions.h | 7 ++ libsolidity/codegen/CompilerUtils.cpp | 2 +- test/libsolidity/ABIEncoderTests.cpp | 36 ++++++ test/libsolidity/SolidityEndToEndTest.cpp | 44 -------- 7 files changed, 177 insertions(+), 52 deletions(-) diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index 14b30df82..c7843b903 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -1735,7 +1735,15 @@ unsigned StructType::calldataEncodedSize(bool _padded) const bool StructType::isDynamicallyEncoded() const { - solAssert(false, "Structs are not yet supported in the ABI."); + solAssert(!recursive(), ""); + for (auto t: memoryMemberTypes()) + { + solAssert(t, "Parameter should have external type."); + t = t->interfaceType(false); + if (t->isDynamicallyEncoded()) + return true; + } + return false; } u256 StructType::memorySize() const diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h index dd50c5735..cb4396939 100644 --- a/libsolidity/ast/Types.h +++ b/libsolidity/ast/Types.h @@ -745,7 +745,7 @@ public: virtual MemberList::MemberMap nativeMembers(ContractDefinition const* _currentScope) const override; virtual TypePointer encodingType() const override { - return location() == DataLocation::Storage ? std::make_shared(256) : TypePointer(); + return location() == DataLocation::Storage ? std::make_shared(256) : shared_from_this(); } virtual TypePointer interfaceType(bool _inLibrary) const override; diff --git a/libsolidity/codegen/ABIFunctions.cpp b/libsolidity/codegen/ABIFunctions.cpp index 3a9f1a486..9f6c55ba6 100644 --- a/libsolidity/codegen/ABIFunctions.cpp +++ b/libsolidity/codegen/ABIFunctions.cpp @@ -404,9 +404,11 @@ string ABIFunctions::abiEncodingFunction( else solAssert(false, ""); } - else if (dynamic_cast(&to)) + else if (auto const* toStruct = dynamic_cast(&to)) { - solUnimplementedAssert(false, "Structs not yet implemented."); + StructType const* fromStruct = dynamic_cast(&_from); + solAssert(fromStruct, ""); + return abiEncodingFunctionStruct(*fromStruct, *toStruct, _encodeAsLibraryTypes); } else if (_from.category() == Type::Category::Function) return abiEncodingFunctionFunctionType( @@ -534,7 +536,7 @@ string ABIFunctions::abiEncodingFunctionSimpleArray( for { let i := 0 } lt(i, length) { i := add(i, 1) } { mstore(pos, sub(tail, headStart)) - tail := ((srcPtr), tail) + tail := (, tail) srcPtr := (srcPtr) pos := add(pos, ) } @@ -549,7 +551,7 @@ string ABIFunctions::abiEncodingFunctionSimpleArray( let srcPtr := (value) for { let i := 0 } lt(i, length) { i := add(i, 1) } { - ((srcPtr), pos) + (, pos) srcPtr := (srcPtr) pos := add(pos, ) } @@ -573,7 +575,7 @@ string ABIFunctions::abiEncodingFunctionSimpleArray( _encodeAsLibraryTypes, true )); - templ("arrayElementAccess", inMemory ? "mload" : "sload"); + templ("arrayElementAccess", inMemory ? "mload(srcPtr)" : _from.baseType()->isValueType() ? "sload(srcPtr)" : "srcPtr" ); templ("nextArrayElement", nextArrayElementFunction(_from)); return templ.render(); }); @@ -726,6 +728,122 @@ string ABIFunctions::abiEncodingFunctionCompactStorageArray( }); } +string ABIFunctions::abiEncodingFunctionStruct( + StructType const& _from, + StructType const& _to, + bool _encodeAsLibraryTypes +) +{ + string functionName = + "abi_encode_" + + _from.identifier() + + "_to_" + + _to.identifier() + + (_encodeAsLibraryTypes ? "_library" : ""); + + solUnimplementedAssert(!_from.dataStoredIn(DataLocation::CallData), ""); + solAssert(&_from.structDefinition() == &_to.structDefinition(), ""); + + return createFunction(functionName, [&]() { + bool fromStorage = _from.location() == DataLocation::Storage; + bool dynamic = _to.isDynamicallyEncoded(); + Whiskers templ(R"( + function (value, pos) { + let tail := add(pos, ) + + <#members> + { + // + + } + + + } + )"); + templ("functionName", functionName); + templ("return", dynamic ? " -> end " : ""); + templ("assignEnd", dynamic ? "end := tail" : ""); + // to avoid multiple loads from the same slot for subsequent members + templ("init", fromStorage ? "let slotValue := 0" : ""); + u256 previousSlotOffset(-1); + u256 encodingOffset = 0; + vector> members; + for (auto const& member: _to.members(nullptr)) + { + solAssert(member.type, ""); + if (!member.type->canLiveOutsideStorage()) + continue; + solUnimplementedAssert( + member.type->mobileType() && + member.type->mobileType()->interfaceType(_encodeAsLibraryTypes) && + member.type->mobileType()->interfaceType(_encodeAsLibraryTypes)->encodingType(), + "Encoding type \"" + member.type->toString() + "\" not yet implemented." + ); + auto memberTypeTo = member.type->mobileType()->interfaceType(_encodeAsLibraryTypes)->encodingType(); + auto memberTypeFrom = _from.memberType(member.name); + solAssert(memberTypeFrom, ""); + bool dynamicMember = memberTypeTo->isDynamicallyEncoded(); + if (dynamicMember) + solAssert(dynamic, ""); + Whiskers memberTempl(R"( + + let memberValue := + )" + ( + dynamicMember ? + string(R"( + mstore(add(pos, ), sub(tail, pos)) + tail := (memberValue, tail) + )") : + string(R"( + (memberValue, add(pos, )) + )") + ) + ); + if (fromStorage) + { + solAssert(memberTypeFrom->isValueType() == memberTypeTo->isValueType(), ""); + u256 storageSlotOffset; + size_t intraSlotOffset; + tie(storageSlotOffset, intraSlotOffset) = _from.storageOffsetsOfMember(member.name); + if (memberTypeFrom->isValueType()) + { + if (storageSlotOffset != previousSlotOffset) + { + memberTempl("preprocess", "slotValue := sload(add(value, " + toCompactHexWithPrefix(storageSlotOffset) + "))"); + previousSlotOffset = storageSlotOffset; + } + else + memberTempl("preprocess", ""); + memberTempl("retrieveValue", shiftRightFunction(intraSlotOffset * 8, false) + "(slotValue)"); + } + else + { + solAssert(memberTypeFrom->dataStoredIn(DataLocation::Storage), ""); + solAssert(intraSlotOffset == 0, ""); + memberTempl("preprocess", ""); + memberTempl("retrieveValue", "add(value, " + toCompactHexWithPrefix(storageSlotOffset) + ")"); + } + } + else + { + memberTempl("preprocess", ""); + string sourceOffset = toCompactHexWithPrefix(_from.memoryOffsetOfMember(member.name)); + memberTempl("retrieveValue", "mload(add(value, " + sourceOffset + "))"); + } + memberTempl("encodingOffset", toCompactHexWithPrefix(encodingOffset)); + encodingOffset += dynamicMember ? 0x20 : memberTypeTo->calldataEncodedSize(); + memberTempl("abiEncode", abiEncodingFunction(*memberTypeFrom, *memberTypeTo, _encodeAsLibraryTypes, false)); + + members.push_back({}); + members.back()["encode"] = memberTempl.render(); + members.back()["memberName"] = member.name; + } + templ("members", members); + templ("headSize", toCompactHexWithPrefix(encodingOffset)); + return templ.render(); + }); +} + string ABIFunctions::abiEncodingFunctionStringLiteral( Type const& _from, Type const& _to, diff --git a/libsolidity/codegen/ABIFunctions.h b/libsolidity/codegen/ABIFunctions.h index 5bbd842f8..de2a140ae 100644 --- a/libsolidity/codegen/ABIFunctions.h +++ b/libsolidity/codegen/ABIFunctions.h @@ -123,6 +123,13 @@ private: bool _encodeAsLibraryTypes ); + /// Part of @a abiEncodingFunction for struct types. + std::string abiEncodingFunctionStruct( + StructType const& _givenType, + StructType const& _targetType, + bool _encodeAsLibraryTypes + ); + // @returns the name of the ABI encoding function with the given type // and queues the generation of the function to the requested functions. // Case for _givenType being a string literal diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp index 1e623357a..37aa1aea6 100644 --- a/libsolidity/codegen/CompilerUtils.cpp +++ b/libsolidity/codegen/CompilerUtils.cpp @@ -121,7 +121,7 @@ void CompilerUtils::storeInMemoryDynamic(Type const& _type, bool _padToWordBound { if (auto ref = dynamic_cast(&_type)) { - solAssert(ref->location() == DataLocation::Memory, ""); + solUnimplementedAssert(ref->location() == DataLocation::Memory, ""); storeInMemoryDynamic(IntegerType(256), _padToWordBoundaries); } else if (auto str = dynamic_cast(&_type)) diff --git a/test/libsolidity/ABIEncoderTests.cpp b/test/libsolidity/ABIEncoderTests.cpp index 4ddf17ce7..05158601f 100644 --- a/test/libsolidity/ABIEncoderTests.cpp +++ b/test/libsolidity/ABIEncoderTests.cpp @@ -424,7 +424,43 @@ BOOST_AUTO_TEST_CASE(function_name_collision) ) } +BOOST_AUTO_TEST_CASE(structs) +{ + string sourceCode = R"( + contract C { + struct S { uint16 a; uint16 b; T[] sub; uint16 c; } + struct T { uint64[2] x; } + S s; + event e(uint16, S); + function f() returns (uint, S) { + uint16 x = 7; + s.a = 8; + s.b = 9; + s.c = 10; + s.sub.length = 3; + s.sub[0].x[0] = 11; + s.sub[1].x[0] = 12; + s.sub[2].x[1] = 13; + e(x, s); + return (x, s); + } + } + )"; + NEW_ENCODER( + compileAndRun(sourceCode, 0, "C"); + bytes encoded = encodeArgs( + u256(7), 0x40, + 8, 9, 0x80, 10, + 3, + 11, 0, + 12, 0, + 0, 13 + ); + BOOST_CHECK(callContractFunction("f()") == encoded); + REQUIRE_LOG_DATA(encoded); + ) +} BOOST_AUTO_TEST_SUITE_END() diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 2bdcbd483..f885b0d31 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -9682,50 +9682,6 @@ BOOST_AUTO_TEST_CASE(contracts_separated_with_comment) compileAndRun(sourceCode, 0, "C2"); } -BOOST_AUTO_TEST_CASE(return_structs) -{ - char const* sourceCode = R"( - pragma experimental ABIEncoderV2; - contract C { - struct S { uint a; T[] sub; } - struct T { uint[2] x; } - function f() returns (uint x, S s) { - x = 7; - s.a = 8; - s.sub = new T[](3); - s.sub[0].x[0] = 9; - s.sub[1].x[0] = 10; - s.sub[2].x[1] = 11; - } - } - )"; - // This will throw "unimplemented" until it is implemented. - BOOST_CHECK_THROW( - compileAndRun(sourceCode, 0, "C"), - Exception); - -// Will calculate the exact encoding later. -// BOOST_CHECK(callContractFunction("f()") == encodeArgs( -// u256(7), u256(0x40), -// u256(8), u256(0x40), -// u256(3), -// // s.sub[0] -// u256(9), u256(0xc0), -// // s.sub[1] -// u256(10), u256(0xe0), -// // s.sub[2] -// u256(11), u256(0x100), -// // s.sub[0].sub -// u256(2) -// // s.sub[0].sub[0].a -// u256(8), -// u256() -// // s.sub[1].sub -// u256(0) -// // s.sub[2].sub -// u256(2) -// )); -} BOOST_AUTO_TEST_CASE(include_creation_bytecode_only_once) { From c5063d315583270e88a01a0a82a84a68190f6ba1 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 1 Sep 2017 13:37:40 +0200 Subject: [PATCH 12/16] Use "tuple" for struct types in ABI JSON. Only use tuple as a type in the ABI (and remove all "anonymous struct" references too) --- docs/abi-spec.rst | 30 +++++++++--------- libsolidity/interface/ABI.cpp | 2 +- test/libsolidity/SolidityABIJSON.cpp | 16 +++++----- .../SolidityNameAndTypeResolution.cpp | 31 +++++++++++++++++-- 4 files changed, 52 insertions(+), 27 deletions(-) diff --git a/docs/abi-spec.rst b/docs/abi-spec.rst index 6df9f1d53..f4822be7d 100644 --- a/docs/abi-spec.rst +++ b/docs/abi-spec.rst @@ -68,13 +68,12 @@ The following non-fixed-size types exist: - ``[]``: a variable-length array of the given fixed-length type. -Types can be combined to anonymous structs by enclosing a finite non-negative number +Types can be combined to a tuple by enclosing a finite non-negative number of them inside parentheses, separated by commas: -- ``(T1,T2,...,Tn)``: anonymous struct (ordered tuple) consisting of the types ``T1``, ..., ``Tn``, ``n >= 0`` - -It is possible to form structs of structs, arrays of structs and so on. +- ``(T1,T2,...,Tn)``: tuple consisting of the types ``T1``, ..., ``Tn``, ``n >= 0`` +It is possible to form tuples of tuples, arrays of tuples and so on. Formal Specification of the Encoding ==================================== @@ -133,7 +132,7 @@ on the type of ``X`` being ``enc(X) = enc((X[0], ..., X[k-1]))`` - i.e. it is encoded as if it were an anonymous struct with ``k`` elements + i.e. it is encoded as if it were a tuple with ``k`` elements of the same type. - ``T[]`` where ``X`` has ``k`` elements (``k`` is assumed to be of type ``uint256``): @@ -176,7 +175,7 @@ and the return values ``v_1, ..., v_k`` of ``f`` are encoded as ``enc((v_1, ..., v_k))`` -i.e. the values are combined into an anonymous struct and encoded. +i.e. the values are combined into a tuple and encoded. Examples ======== @@ -357,16 +356,17 @@ would result in the JSON: "outputs": [] }] -Use of Structs in Types ------------------------ +Handling tuple types +-------------------- -If structs are part of the type, we still want to know the name of the components. Because of that, +If tuples are part of the type, we still want to know the name of the components. Because of that, the json structure gets arbitrarily nested in the following way: An object with members ``name``, ``type`` and potentially ``components`` describes a typed variable. -The canonical type is determined until a struct type is reached and the string description up -to that point is stored in ``type``, i.e. it will be a sequence of ``[]`` and ``[k]`` with -integers ``k``. The components of the struct are then stored in the member ``components``, +The canonical type is determined until a tuple type is reached and the string description up +to that point is stored in ``type`` prefix with the word ``tuple``, i.e. it will be ``tuple`` followed by +a sequence of ``[]`` and ``[k]`` with +integers ``k``. The components of the tuple are then stored in the member ``components``, which is of array type and has the same structure as the top-level object except that ``indexed`` is not allowed there. @@ -391,7 +391,7 @@ would result in the JSON: "inputs": [ { "name": "s", - "type": "", + "type": "tuple", "components": [ { "name": "a", @@ -403,7 +403,7 @@ would result in the JSON: }, { "name": "c", - "type": "[]", + "type": "tuple[]", "components": [ { "name": "x", @@ -419,7 +419,7 @@ would result in the JSON: }, { "name": "t", - "type": "", + "type": "tuple", "components": [ { "name": "x", diff --git a/libsolidity/interface/ABI.cpp b/libsolidity/interface/ABI.cpp index 9af7cdc37..aefb34afa 100644 --- a/libsolidity/interface/ABI.cpp +++ b/libsolidity/interface/ABI.cpp @@ -147,7 +147,7 @@ Json::Value ABI::formatType(string const& _name, Type const& _type, bool _forLib } else if (StructType const* structType = dynamic_cast(&_type)) { - ret["type"] = string(); + ret["type"] = "tuple"; ret["components"] = Json::arrayValue; for (auto const& member: structType->members(nullptr)) { diff --git a/test/libsolidity/SolidityABIJSON.cpp b/test/libsolidity/SolidityABIJSON.cpp index e4ab54ec1..e5d9e99c1 100644 --- a/test/libsolidity/SolidityABIJSON.cpp +++ b/test/libsolidity/SolidityABIJSON.cpp @@ -973,11 +973,11 @@ BOOST_AUTO_TEST_CASE(return_structs) } ], "name" : "sub", - "type" : "[]" + "type" : "tuple[]" } ], "name" : "s", - "type" : "" + "type" : "tuple" } ], "payable" : false, @@ -1015,7 +1015,7 @@ BOOST_AUTO_TEST_CASE(return_structs_with_contracts) } ], "name": "s", - "type": "" + "type": "tuple" }, { "name": "c", @@ -1052,7 +1052,7 @@ BOOST_AUTO_TEST_CASE(event_structs) ], "indexed": false, "name": "t", - "type": "" + "type": "tuple" }, { "components": [ @@ -1068,7 +1068,7 @@ BOOST_AUTO_TEST_CASE(event_structs) } ], "name": "sub", - "type": "[]" + "type": "tuple[]" }, { "name": "b", @@ -1077,7 +1077,7 @@ BOOST_AUTO_TEST_CASE(event_structs) ], "indexed": false, "name": "s", - "type": "" + "type": "tuple" } ], "name": "E", @@ -1115,7 +1115,7 @@ BOOST_AUTO_TEST_CASE(structs_in_libraries) } ], "name": "sub", - "type": "[]" + "type": "tuple[]" }, { "name": "b", @@ -1123,7 +1123,7 @@ BOOST_AUTO_TEST_CASE(structs_in_libraries) } ], "name": "s", - "type": "" + "type": "tuple" } ], "name": "g", diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 778c12c5c..5ab824d3e 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -601,7 +601,6 @@ BOOST_AUTO_TEST_CASE(enum_external_type) BOOST_AUTO_TEST_CASE(external_structs) { - ASTPointer sourceUnit; char const* text = R"( contract Test { enum ActionChoices { GoLeft, GoRight, GoStraight, Sit } @@ -614,7 +613,7 @@ BOOST_AUTO_TEST_CASE(external_structs) function i(Nested[]) external {} } )"; - ETH_TEST_REQUIRE_NO_THROW(sourceUnit = parseAndAnalyse(text), "Parsing and name Resolving failed"); + SourceUnit const* sourceUnit = parseAndAnalyse(text); for (ASTPointer const& node: sourceUnit->nodes()) if (ContractDefinition* contract = dynamic_cast(node.get())) { @@ -627,7 +626,33 @@ BOOST_AUTO_TEST_CASE(external_structs) } } -// TODO: Add a test that checks the signature of library functions taking structs +BOOST_AUTO_TEST_CASE(external_structs_in_libraries) +{ + char const* text = R"( + library Test { + enum ActionChoices { GoLeft, GoRight, GoStraight, Sit } + struct Empty {} + struct Nested { X[2][] a; mapping(uint => uint) m; uint y; } + struct X { bytes32 x; Test t; Empty[] e; } + function f(ActionChoices, uint, Empty) external {} + function g(Test, Nested) external {} + function h(function(Nested) external returns (uint)[]) external {} + function i(Nested[]) external {} + } + )"; + SourceUnit const* sourceUnit = parseAndAnalyse(text); + for (ASTPointer const& node: sourceUnit->nodes()) + if (ContractDefinition* contract = dynamic_cast(node.get())) + { + auto functions = contract->definedFunctions(); + BOOST_REQUIRE(!functions.empty()); + BOOST_CHECK_EQUAL("f(Test.ActionChoices,uint256,Test.Empty)", functions[0]->externalSignature()); + BOOST_CHECK_EQUAL("g(Test,Test.Nested)", functions[1]->externalSignature()); + BOOST_CHECK_EQUAL("h(function[])", functions[2]->externalSignature()); + BOOST_CHECK_EQUAL("i(Test.Nested[])", functions[3]->externalSignature()); + } +} + BOOST_AUTO_TEST_CASE(function_external_call_allowed_conversion) { From 923373b41efed30839cfc26e903e32b0dddd9cb5 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Mon, 11 Sep 2017 15:03:49 +0100 Subject: [PATCH 13/16] Clarify ABI & Solidity types --- docs/abi-spec.rst | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/abi-spec.rst b/docs/abi-spec.rst index f4822be7d..97320c7f5 100644 --- a/docs/abi-spec.rst +++ b/docs/abi-spec.rst @@ -75,6 +75,9 @@ of them inside parentheses, separated by commas: It is possible to form tuples of tuples, arrays of tuples and so on. +.. note:: + Solidity supports all the types presented above with the same names with the exception of tuples. The ABI tuple type is utilised for encoding Solidity ``structs``. + Formal Specification of the Encoding ==================================== @@ -359,8 +362,8 @@ would result in the JSON: Handling tuple types -------------------- -If tuples are part of the type, we still want to know the name of the components. Because of that, -the json structure gets arbitrarily nested in the following way: +Despite that names are intentionally not part of the ABI encoding they do make a lot of sense to be included +in the JSON to enable displaying it to the end user. The structure is nested in the following way: An object with members ``name``, ``type`` and potentially ``components`` describes a typed variable. The canonical type is determined until a tuple type is reached and the string description up From b687d74c47c5814ce159bf16a2da2158c1a15879 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Wed, 13 Sep 2017 19:09:31 +0100 Subject: [PATCH 14/16] Add changelog --- Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Changelog.md b/Changelog.md index 0b9295d06..376494e0d 100644 --- a/Changelog.md +++ b/Changelog.md @@ -5,6 +5,7 @@ Features: * Code Generator: Added ``.selector`` member on external function types to retrieve their signature. * Code Generator: Keep a single copy of encoding functions when using the experimental "ABIEncoderV2". * Optimizer: Add new optimization step to remove unused ``JUMPDEST``s. + * Code Generator: Support passing ``structs`` as arguments and return parameters (requires ``pragma experimental ABIEncoderV2`` for now). * Syntax Checker: Warn if no visibility is specified on contract functions. * Type Checker: Display helpful warning for unused function arguments/return parameters. * Type Checker: Do not show the same error multiple times for events. From 06965458080dcaff6a8c486acc500b64b26a078f Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 14 Sep 2017 14:51:14 +0200 Subject: [PATCH 15/16] Check for interface types of members and cache recursion check. --- libsolidity/ast/Types.cpp | 67 ++++++++++++++++++++++++++------------- libsolidity/ast/Types.h | 4 +++ 2 files changed, 49 insertions(+), 22 deletions(-) diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index c7843b903..83a5b465a 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -1788,14 +1788,33 @@ MemberList::MemberMap StructType::nativeMembers(ContractDefinition const*) const TypePointer StructType::interfaceType(bool _inLibrary) const { + if (!canBeUsedExternally(_inLibrary)) + return TypePointer(); + + // Has to fulfill canBeUsedExternally(_inLibrary) == !!interfaceType(_inLibrary) if (_inLibrary && location() == DataLocation::Storage) return shared_from_this(); - else if (!recursive()) - // TODO this might not be enough, we have to convert all members to - // their interfaceType - return copyForLocation(DataLocation::Memory, true); else - return TypePointer(); + return copyForLocation(DataLocation::Memory, true); +} + +bool StructType::canBeUsedExternally(bool _inLibrary) const +{ + if (_inLibrary && location() == DataLocation::Storage) + return true; + else if (recursive()) + return false; + else + { + // Check that all members have interface types. + // We pass "false" to canBeUsedExternally (_inLibrary), because this struct will be + // passed by value and thus the encoding does not differ, but it will disallow + // mappings. + for (auto const& var: m_struct.members()) + if (!var->annotation().type->canBeUsedExternally(false)) + return false; + } + return true; } TypePointer StructType::copyForLocation(DataLocation _location, bool _isPointer) const @@ -1887,25 +1906,29 @@ set StructType::membersMissingInMemory() const bool StructType::recursive() const { - set structsSeen; - function check = [&](StructType const* t) -> bool + if (!m_recursive.is_initialized()) { - StructDefinition const* str = &t->structDefinition(); - if (structsSeen.count(str)) - return true; - structsSeen.insert(str); - for (ASTPointer const& variable: str->members()) + set structsSeen; + function check = [&](StructType const* t) -> bool { - Type const* memberType = variable->annotation().type.get(); - while (dynamic_cast(memberType)) - memberType = dynamic_cast(memberType)->baseType().get(); - if (StructType const* innerStruct = dynamic_cast(memberType)) - if (check(innerStruct)) - return true; - } - return false; - }; - return check(this); + StructDefinition const* str = &t->structDefinition(); + if (structsSeen.count(str)) + return true; + structsSeen.insert(str); + for (ASTPointer const& variable: str->members()) + { + Type const* memberType = variable->annotation().type.get(); + while (dynamic_cast(memberType)) + memberType = dynamic_cast(memberType)->baseType().get(); + if (StructType const* innerStruct = dynamic_cast(memberType)) + if (check(innerStruct)) + return true; + } + return false; + }; + m_recursive = check(this); + } + return *m_recursive; } TypePointer EnumType::unaryOperatorResult(Token::Value _operator) const diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h index cb4396939..8ba555215 100644 --- a/libsolidity/ast/Types.h +++ b/libsolidity/ast/Types.h @@ -32,6 +32,7 @@ #include #include +#include #include #include @@ -748,6 +749,7 @@ public: return location() == DataLocation::Storage ? std::make_shared(256) : shared_from_this(); } virtual TypePointer interfaceType(bool _inLibrary) const override; + virtual bool canBeUsedExternally(bool _inLibrary) const override; TypePointer copyForLocation(DataLocation _location, bool _isPointer) const override; @@ -774,6 +776,8 @@ public: private: StructDefinition const& m_struct; + /// Cache for the recursive() function. + mutable boost::optional m_recursive; }; /** From c001903cdc6db97f437150375a9b5343c70c3656 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 14 Sep 2017 17:03:59 +0200 Subject: [PATCH 16/16] Fixed tests with mappings in structs and added some more. --- .../SolidityNameAndTypeResolution.cpp | 92 ++++++++++++++++++- 1 file changed, 90 insertions(+), 2 deletions(-) diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 5ab824d3e..d9e6a63d0 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -605,7 +605,7 @@ BOOST_AUTO_TEST_CASE(external_structs) contract Test { enum ActionChoices { GoLeft, GoRight, GoStraight, Sit } struct Empty {} - struct Nested { X[2][] a; mapping(uint => uint) m; uint y; } + struct Nested { X[2][] a; uint y; } struct X { bytes32 x; Test t; Empty[] e; } function f(ActionChoices, uint, Empty) external {} function g(Test, Nested) external {} @@ -632,7 +632,7 @@ BOOST_AUTO_TEST_CASE(external_structs_in_libraries) library Test { enum ActionChoices { GoLeft, GoRight, GoStraight, Sit } struct Empty {} - struct Nested { X[2][] a; mapping(uint => uint) m; uint y; } + struct Nested { X[2][] a; uint y; } struct X { bytes32 x; Test t; Empty[] e; } function f(ActionChoices, uint, Empty) external {} function g(Test, Nested) external {} @@ -653,6 +653,94 @@ BOOST_AUTO_TEST_CASE(external_structs_in_libraries) } } +BOOST_AUTO_TEST_CASE(struct_with_mapping_in_library) +{ + char const* text = R"( + library Test { + struct Nested { mapping(uint => uint)[2][] a; uint y; } + struct X { Nested n; } + function f(X storage x) external {} + } + )"; + SourceUnit const* sourceUnit = parseAndAnalyse(text); + for (ASTPointer const& node: sourceUnit->nodes()) + if (ContractDefinition* contract = dynamic_cast(node.get())) + { + auto functions = contract->definedFunctions(); + BOOST_REQUIRE(!functions.empty()); + BOOST_CHECK_EQUAL("f(Test.X storage)", functions[0]->externalSignature()); + } +} + +BOOST_AUTO_TEST_CASE(functions_with_identical_structs_in_interface) +{ + char const* text = R"( + pragma experimental ABIEncoderV2; + + contract C { + struct S1 { } + struct S2 { } + function f(S1) pure {} + function f(S2) pure {} + } + )"; + CHECK_ERROR(text, TypeError, "Function overload clash during conversion to external types for arguments"); +} + +BOOST_AUTO_TEST_CASE(functions_with_different_structs_in_interface) +{ + char const* text = R"( + pragma experimental ABIEncoderV2; + + contract C { + struct S1 { function() external a; } + struct S2 { bytes24 a; } + function f(S1) pure {} + function f(S2) pure {} + } + )"; + CHECK_SUCCESS(text); +} + +BOOST_AUTO_TEST_CASE(functions_with_stucts_of_non_external_types_in_interface) +{ + char const* text = R"( + pragma experimental ABIEncoderV2; + + contract C { + struct S { function() internal a; } + function f(S) {} + } + )"; + CHECK_ERROR(text, TypeError, "Internal or recursive type is not allowed for public or external functions."); +} + +BOOST_AUTO_TEST_CASE(functions_with_stucts_of_non_external_types_in_interface_2) +{ + char const* text = R"( + pragma experimental ABIEncoderV2; + + contract C { + struct S { mapping(uint => uint) a; } + function f(S) {} + } + )"; + CHECK_ERROR(text, TypeError, "Internal or recursive type is not allowed for public or external functions."); +} + +BOOST_AUTO_TEST_CASE(functions_with_stucts_of_non_external_types_in_interface_nested) +{ + char const* text = R"( + pragma experimental ABIEncoderV2; + + contract C { + struct T { mapping(uint => uint) a; } + struct S { T[][2] b; } + function f(S) {} + } + )"; + CHECK_ERROR(text, TypeError, "Internal or recursive type is not allowed for public or external functions."); +} BOOST_AUTO_TEST_CASE(function_external_call_allowed_conversion) {