Merge pull request #2703 from ethereum/warnAboutLargeStorageArrays

Warn about large storage structures.

4 files changed, 97 insertions, 0 deletions

diff --git a/Changelog.md b/Changelog.md
index b4ca745f..4f5615a9 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -3,6 +3,7 @@
 Features:
  * Parser: Display previous visibility specifier in error if multiple are found.
  * Syntax Checker: Support ``pragma experimental <feature>;`` to turn on experimental features.
+ * Static Analyzer: Warn about large storage structures.
  * Metadata: Store experimental flag in metadata CBOR.
  * Type Checker: More detailed error message for invalid overrides.
 
diff --git a/libsolidity/analysis/StaticAnalyzer.cpp b/libsolidity/analysis/StaticAnalyzer.cpp
index 46477e1e..ab1cbb52 100644
--- a/libsolidity/analysis/StaticAnalyzer.cpp
+++ b/libsolidity/analysis/StaticAnalyzer.cpp
@@ -92,6 +92,17 @@ bool StaticAnalyzer::visit(VariableDeclaration const& _variable)
 			// This is not a no-op, the entry might pre-exist.
 			m_localVarUseCount[&_variable] += 0;
 	}
+	else if (_variable.isStateVariable())
+	{
+		set<StructDefinition const*> structsSeen;
+		if (structureSizeEstimate(*_variable.type(), structsSeen) >= bigint(1) << 64)
+			m_errorReporter.warning(
+				_variable.location(),
+				"Variable covers a large part of storage and thus makes collisions likely. "
+				"Either use mappings or dynamic arrays and allow their size to be increased only "
+				"in small quantities per transaction."
+			);
+	}
 	return true;
 }
 
@@ -160,3 +171,34 @@ bool StaticAnalyzer::visit(InlineAssembly const& _inlineAssembly)
 
 	return true;
 }
+
+bigint StaticAnalyzer::structureSizeEstimate(Type const& _type, set<StructDefinition const*>& _structsSeen)
+{
+	switch (_type.category())
+	{
+	case Type::Category::Array:
+	{
+		auto const& t = dynamic_cast<ArrayType const&>(_type);
+		return structureSizeEstimate(*t.baseType(), _structsSeen) * (t.isDynamicallySized() ? 1 : t.length());
+	}
+	case Type::Category::Struct:
+	{
+		auto const& t = dynamic_cast<StructType const&>(_type);
+		bigint size = 1;
+		if (!_structsSeen.count(&t.structDefinition()))
+		{
+			_structsSeen.insert(&t.structDefinition());
+			for (auto const& m: t.members(nullptr))
+				size += structureSizeEstimate(*m.type, _structsSeen);
+		}
+		return size;
+	}
+	case Type::Category::Mapping:
+	{
+		return structureSizeEstimate(*dynamic_cast<MappingType const&>(_type).valueType(), _structsSeen);
+	}
+	default:
+		break;
+	}
+	return bigint(1);
+}
diff --git a/libsolidity/analysis/StaticAnalyzer.h b/libsolidity/analysis/StaticAnalyzer.h
index 21a487df..a3080b42 100644
--- a/libsolidity/analysis/StaticAnalyzer.h
+++ b/libsolidity/analysis/StaticAnalyzer.h
@@ -65,6 +65,9 @@ private:
 	virtual bool visit(MemberAccess const& _memberAccess) override;
 	virtual bool visit(InlineAssembly const& _inlineAssembly) override;
 
+	/// @returns the size of this type in storage, including all sub-types.
+	static bigint structureSizeEstimate(Type const& _type, std::set<StructDefinition const*>& _structsSeen);
+
 	ErrorReporter& m_errorReporter;
 
 	/// Flag that indicates whether the current contract definition is a library.
diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp
index 49a23e13..13843afa 100644
--- a/test/libsolidity/SolidityNameAndTypeResolution.cpp
+++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp
@@ -6545,6 +6545,57 @@ BOOST_AUTO_TEST_CASE(constructor_without_implementation)
 	CHECK_ERROR(text, TypeError, "Constructor must be implemented if declared.");
 }
 
+BOOST_AUTO_TEST_CASE(large_storage_array_fine)
+{
+	char const* text = R"(
+		contract C {
+			uint[2**64 - 1] x;
+		}
+	)";
+	CHECK_SUCCESS_NO_WARNINGS(text);
+}
+
+BOOST_AUTO_TEST_CASE(large_storage_array_simple)
+{
+	char const* text = R"(
+		contract C {
+			uint[2**64] x;
+		}
+	)";
+	CHECK_WARNING(text, "covers a large part of storage and thus makes collisions likely");
+}
+
+BOOST_AUTO_TEST_CASE(large_storage_arrays_combined)
+{
+	char const* text = R"(
+		contract C {
+			uint[200][200][2**30][][2**30] x;
+		}
+	)";
+	CHECK_WARNING(text, "covers a large part of storage and thus makes collisions likely");
+}
+
+BOOST_AUTO_TEST_CASE(large_storage_arrays_struct)
+{
+	char const* text = R"(
+		contract C {
+			struct S { uint[2**30] x; uint[2**50] y; }
+			S[2**20] x;
+		}
+	)";
+	CHECK_WARNING(text, "covers a large part of storage and thus makes collisions likely");
+}
+
+BOOST_AUTO_TEST_CASE(large_storage_array_mapping)
+{
+	char const* text = R"(
+		contract C {
+			mapping(uint => uint[2**100]) x;
+		}
+	)";
+	CHECK_WARNING(text, "covers a large part of storage and thus makes collisions likely");
+}
+
 BOOST_AUTO_TEST_CASE(library_function_without_implementation)
 {
 	char const* text = R"(