Merge pull request #6143 from ethereum/fix-unimplemented-assert-5659

Check base contracts for abi encoder compatibility

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:

liblangutil/ErrorReporter.cpp

@@ -118,6 +118,12 @@ bool ErrorReporter::checkForExcessiveErrors(Error::Type _type)
 	return false;
 }
 
+void ErrorReporter::fatalError(Error::Type _type, SourceLocation const& _location, SecondarySourceLocation const& _secondaryLocation, string const& _description)
+{
+	error(_type, _location, _secondaryLocation, _description);
+	BOOST_THROW_EXCEPTION(FatalError());
+}
+
 void ErrorReporter::fatalError(Error::Type _type, SourceLocation const& _location, string const& _description)
 {
 	error(_type, _location, _description);
@@ -207,6 +213,15 @@ void ErrorReporter::typeError(SourceLocation const& _location, string const& _de
 	);
 }
 
+void ErrorReporter::fatalTypeError(SourceLocation const& _location, SecondarySourceLocation const& _secondaryLocation, string const& _description)
+{
+	fatalError(
+		Error::Type::TypeError,
+		_location,
+		_secondaryLocation,
+		_description
+	);
+}
 
 void ErrorReporter::fatalTypeError(SourceLocation const& _location, string const& _description)
 {

liblangutil/ErrorReporter.h

@@ -104,6 +104,7 @@ public:
 	}
 
 	void fatalTypeError(SourceLocation const& _location, std::string const& _description);
+	void fatalTypeError(SourceLocation const& _location, SecondarySourceLocation const& _secondLocation, std::string const& _description);
 
 	void docstringParsingError(std::string const& _description);
 
@@ -118,12 +119,20 @@ public:
 	}
 
 private:
-	void error(Error::Type _type,
+	void error(
+		Error::Type _type,
 		SourceLocation const& _location,
 		SecondarySourceLocation const& _secondaryLocation,
 		std::string const& _description = std::string());
 
-	void fatalError(Error::Type _type,
+	void fatalError(
+		Error::Type _type,
+		SourceLocation const& _location,
+		SecondarySourceLocation const& _secondaryLocation,
+		std::string const& _description = std::string());
+
+	void fatalError(
+		Error::Type _type,
 		SourceLocation const& _location = SourceLocation(),
 		std::string const& _description = std::string());
 

libsolidity/analysis/ContractLevelChecker.cpp

@@ -22,6 +22,7 @@
 #include <libsolidity/analysis/ContractLevelChecker.h>
 
 #include <libsolidity/ast/AST.h>
+#include <libsolidity/analysis/TypeChecker.h>
 #include <liblangutil/ErrorReporter.h>
 #include <boost/range/adaptor/reversed.hpp>
 
@@ -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."
+		);
+
+}

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;
 };

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;
 }
 

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;

libsolidity/ast/AST.h

@@ -429,7 +429,6 @@ private:
 	std::vector<ASTPointer<ASTNode>> m_subNodes;
 	ContractKind m_contractKind;
 
-	std::vector<ContractDefinition const*> m_linearizedBaseContracts;
 	mutable std::unique_ptr<std::vector<std::pair<FixedHash<4>, FunctionTypePointer>>> m_interfaceFunctionList;
 	mutable std::unique_ptr<std::vector<EventDefinition const*>> m_interfaceEvents;
 	mutable std::unique_ptr<std::vector<Declaration const*>> m_inheritableMembers;
@@ -589,6 +588,7 @@ public:
 	}
 
 	std::vector<ASTPointer<VariableDeclaration>> const& parameters() const { return m_parameters->parameters(); }
+	std::vector<ASTPointer<VariableDeclaration>> const& returnParameters() const { return m_returnParameters->parameters(); }
 	ParameterList const& parameterList() const { return *m_parameters; }
 	ASTPointer<ParameterList> const& returnParameterList() const { return m_returnParameters; }
 
@@ -629,7 +629,6 @@ public:
 	bool isFallback() const { return !m_isConstructor && name().empty(); }
 	bool isPayable() const { return m_stateMutability == StateMutability::Payable; }
 	std::vector<ASTPointer<ModifierInvocation>> const& modifiers() const { return m_functionModifiers; }
-	std::vector<ASTPointer<VariableDeclaration>> const& returnParameters() const { return m_returnParameters->parameters(); }
 	Block const& body() const { solAssert(m_body, ""); return *m_body; }
 	bool isVisibleInContract() const override
 	{

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()
 
 }