From 9919670ddd520c3d6fdc1ce1400c081cc6eb7fe4 Mon Sep 17 00:00:00 2001 From: Mathias Baumann Date: Mon, 4 Mar 2019 14:33:46 +0100 Subject: [PATCH] Check base contracts for abi encoder compatibility --- Changelog.md | 1 + libsolidity/analysis/ContractLevelChecker.cpp | 49 ++++++ libsolidity/analysis/ContractLevelChecker.h | 2 + libsolidity/analysis/TypeChecker.cpp | 9 +- libsolidity/analysis/TypeChecker.h | 2 + test/libsolidity/Imports.cpp | 147 ++++++++++++++++++ 6 files changed, 203 insertions(+), 7 deletions(-) diff --git a/Changelog.md b/Changelog.md index d1384095b..3ff3c2cd0 100644 --- a/Changelog.md +++ b/Changelog.md @@ -7,6 +7,7 @@ Compiler Features: Bugfixes: + * Type system: Detect if a contract's base uses types that require the experimental abi encoder while the contract still uses the old encoder Build System: diff --git a/libsolidity/analysis/ContractLevelChecker.cpp b/libsolidity/analysis/ContractLevelChecker.cpp index 96b9611e5..1c6383b8b 100644 --- a/libsolidity/analysis/ContractLevelChecker.cpp +++ b/libsolidity/analysis/ContractLevelChecker.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -44,6 +45,7 @@ bool ContractLevelChecker::check(ContractDefinition const& _contract) checkExternalTypeClashes(_contract); checkHashCollisions(_contract); checkLibraryRequirements(_contract); + checkBaseABICompatibility(_contract); return Error::containsOnlyWarnings(m_errorReporter.errors()); } @@ -460,3 +462,50 @@ void ContractLevelChecker::checkLibraryRequirements(ContractDefinition const& _c if (!var->isConstant()) m_errorReporter.typeError(var->location(), "Library cannot have non-constant state variables"); } + +void ContractLevelChecker::checkBaseABICompatibility(ContractDefinition const& _contract) +{ + if (_contract.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::ABIEncoderV2)) + return; + + if (_contract.isLibrary()) + { + solAssert( + _contract.baseContracts().empty() || m_errorReporter.hasErrors(), + "Library is not allowed to inherit" + ); + return; + } + + SecondarySourceLocation errors; + + // interfaceFunctionList contains all inherited functions as well + for (auto const& func: _contract.interfaceFunctionList()) + { + solAssert(func.second->hasDeclaration(), "Function has no declaration?!"); + + if (!func.second->declaration().sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::ABIEncoderV2)) + continue; + + auto const& currentLoc = func.second->declaration().location(); + + for (TypePointer const& paramType: func.second->parameterTypes() + func.second->parameterTypes()) + if (!TypeChecker::typeSupportedByOldABIEncoder(*paramType, false)) + { + errors.append("Type only supported by the new experimental ABI encoder", currentLoc); + break; + } + } + + if (!errors.infos.empty()) + m_errorReporter.fatalTypeError( + _contract.location(), + errors, + std::string("Contract \"") + + _contract.name() + + "\" does not use the new experimental ABI encoder but wants to inherit from a contract " + + "which uses types that require it. " + + "Use \"pragma experimental ABIEncoderV2;\" for the inheriting contract as well to enable the feature." + ); + +} diff --git a/libsolidity/analysis/ContractLevelChecker.h b/libsolidity/analysis/ContractLevelChecker.h index d754687ae..f2cd9887f 100644 --- a/libsolidity/analysis/ContractLevelChecker.h +++ b/libsolidity/analysis/ContractLevelChecker.h @@ -78,6 +78,8 @@ private: void checkHashCollisions(ContractDefinition const& _contract); /// Checks that all requirements for a library are fulfilled if this is a library. void checkLibraryRequirements(ContractDefinition const& _contract); + /// Checks base contracts for ABI compatibility + void checkBaseABICompatibility(ContractDefinition const& _contract); langutil::ErrorReporter& m_errorReporter; }; diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 5378f15fb..0109a7b03 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -45,10 +45,7 @@ using namespace dev; using namespace langutil; using namespace dev::solidity; -namespace -{ - -bool typeSupportedByOldABIEncoder(Type const& _type, bool _isLibraryCall) +bool TypeChecker::typeSupportedByOldABIEncoder(Type const& _type, bool _isLibraryCall) { if (_isLibraryCall && _type.dataStoredIn(DataLocation::Storage)) return true; @@ -64,9 +61,6 @@ bool typeSupportedByOldABIEncoder(Type const& _type, bool _isLibraryCall) return true; } -} - - bool TypeChecker::checkTypeRequirements(ASTNode const& _contract) { _contract.accept(*this); @@ -94,6 +88,7 @@ bool TypeChecker::visit(ContractDefinition const& _contract) for (auto const& n: _contract.subNodes()) n->accept(*this); + return false; } diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h index 09879322e..8f532a89c 100644 --- a/libsolidity/analysis/TypeChecker.h +++ b/libsolidity/analysis/TypeChecker.h @@ -63,6 +63,8 @@ public: /// (this can happen for variables with non-explicit types before their types are resolved) TypePointer const& type(VariableDeclaration const& _variable) const; + static bool typeSupportedByOldABIEncoder(Type const& _type, bool _isLibraryCall); + private: bool visit(ContractDefinition const& _contract) override; diff --git a/test/libsolidity/Imports.cpp b/test/libsolidity/Imports.cpp index abd659b61..ebdb46545 100644 --- a/test/libsolidity/Imports.cpp +++ b/test/libsolidity/Imports.cpp @@ -320,6 +320,153 @@ BOOST_AUTO_TEST_CASE(shadowing_builtins_with_alias) } } +BOOST_AUTO_TEST_CASE(inheritance_abi_encoder_mismatch_1) +{ + CompilerStack c; + c.addSource("A.sol", R"( + pragma solidity >=0.0; + pragma experimental ABIEncoderV2; + + contract A + { + struct S { uint a; } + S public s; + function f(S memory _s) returns (S memory,S memory) { } + } + )"); + + c.addSource("B.sol", R"( + pragma solidity >=0.0; + pragma experimental ABIEncoderV2; + + import "./A.sol"; + contract B is A { } + )"); + + c.addSource("C.sol", R"( + pragma solidity >=0.0; + + import "./B.sol"; + contract C is B { } + )"); + + c.setEVMVersion(dev::test::Options::get().evmVersion()); + BOOST_CHECK(!c.compile()); + + int typeErrors = 0; + + // Sometimes we get the prerelease warning, sometimes not. + for (auto const& e: c.errors()) + { + if (e->type() != langutil::Error::Type::TypeError) + continue; + + typeErrors++; + + string const* msg = e->comment(); + BOOST_REQUIRE(msg); + BOOST_CHECK_EQUAL(*msg, std::string("Contract \"C\" does not use the new experimental ABI encoder but wants to inherit from a contract which uses types that require it. Use \"pragma experimental ABIEncoderV2;\" for the inheriting contract as well to enable the feature.")); + } + BOOST_CHECK_EQUAL(typeErrors, 1); +} + +BOOST_AUTO_TEST_CASE(inheritance_abi_encoder_mismatch_2) +{ + CompilerStack c; + c.addSource("A.sol", R"( + pragma solidity >=0.0; + pragma experimental ABIEncoderV2; + + contract A + { + struct S { uint a; } + S public s; + function f(S memory _s) returns (S memory,S memory) { } + } + )"); + + c.addSource("B.sol", R"( + pragma solidity >=0.0; + + import "./A.sol"; + contract B is A { } + )"); + + c.addSource("C.sol", R"( + pragma solidity >=0.0; + + import "./B.sol"; + contract C is B { } + )"); + + c.setEVMVersion(dev::test::Options::get().evmVersion()); + BOOST_CHECK(!c.compile()); + + int typeErrors = 0; + + // Sometimes we get the prerelease warning, sometimes not. + for (auto const& e: c.errors()) + { + if (e->type() != langutil::Error::Type::TypeError) + continue; + + typeErrors++; + + string const* msg = e->comment(); + BOOST_REQUIRE(msg); + BOOST_CHECK_EQUAL(*msg, std::string("Contract \"B\" does not use the new experimental ABI encoder but wants to inherit from a contract which uses types that require it. Use \"pragma experimental ABIEncoderV2;\" for the inheriting contract as well to enable the feature.")); + } + BOOST_CHECK_EQUAL(typeErrors, 1); +} + +BOOST_AUTO_TEST_CASE(inheritance_abi_encoder_match) +{ + CompilerStack c; + c.addSource("A.sol", R"( + pragma solidity >=0.0; + pragma experimental ABIEncoderV2; + + contract A + { + struct S { uint a; } + S public s; + function f(S memory _s) public returns (S memory,S memory) { } + } + )"); + + c.addSource("B.sol", R"( + pragma solidity >=0.0; + pragma experimental ABIEncoderV2; + + import "./A.sol"; + contract B is A { } + )"); + + c.addSource("C.sol", R"( + pragma solidity >=0.0; + pragma experimental ABIEncoderV2; + + import "./B.sol"; + contract C is B { } + )"); + + c.setEVMVersion(dev::test::Options::get().evmVersion()); + BOOST_CHECK(c.compile()); + + int typeErrors = 0; + + // Sometimes we get the prerelease warning, sometimes not. + for (auto const& e: c.errors()) + { + if (e->type() != langutil::Error::Type::TypeError) + continue; + + typeErrors++; + } + + BOOST_CHECK_EQUAL(typeErrors, 0); +} + BOOST_AUTO_TEST_SUITE_END() }