Merge pull request #3031 from wadeAlexC/develop

Improves checksum error message

9 files changed, 92 insertions, 17 deletions

diff --git a/Changelog.md b/Changelog.md
index 45521f3e..2487b87c 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -2,6 +2,7 @@
 
 Features:
  * Syntax Checker: Turn the usage of ``callcode`` into an error as experimental 0.5.0 feature.
+ * Type Checker: Improve address checksum warning.
  * Type Checker: More detailed errors for invalid array lengths (such as division by zero).
 
 Bugfixes:
diff --git a/libdevcore/CommonData.cpp b/libdevcore/CommonData.cpp
index db11e61c..445d11cd 100644
--- a/libdevcore/CommonData.cpp
+++ b/libdevcore/CommonData.cpp
@@ -21,6 +21,7 @@
 
 #include <libdevcore/CommonData.h>
 #include <libdevcore/Exceptions.h>
+#include <libdevcore/Assertions.h>
 #include <libdevcore/SHA3.h>
 
 #include <boost/algorithm/string.hpp>
@@ -86,20 +87,26 @@ bool dev::passesAddressChecksum(string const& _str, bool _strict)
 	))
 		return true;
 
+	return _str == dev::getChecksummedAddress(_str);
+}
+
+string dev::getChecksummedAddress(string const& _addr)
+{
+	string s = _addr.substr(0, 2) == "0x" ? _addr.substr(2) : _addr;
+	assertThrow(s.length() == 40, InvalidAddress, "");
+	assertThrow(s.find_first_not_of("0123456789abcdefABCDEF") == string::npos, InvalidAddress, "");
+
 	h256 hash = keccak256(boost::algorithm::to_lower_copy(s, std::locale::classic()));
+
+	string ret = "0x";
 	for (size_t i = 0; i < 40; ++i)
 	{
 		char addressCharacter = s[i];
-		bool lowerCase;
-		if ('a' <= addressCharacter && addressCharacter <= 'f')
-			lowerCase = true;
-		else if ('A' <= addressCharacter && addressCharacter <= 'F')
-			lowerCase = false;
-		else
-			continue;
 		unsigned nibble = (unsigned(hash[i / 2]) >> (4 * (1 - (i % 2)))) & 0xf;
-		if ((nibble >= 8) == lowerCase)
-			return false;
+		if (nibble >= 8)
+			ret += toupper(addressCharacter);
+		else
+			ret += tolower(addressCharacter);
 	}
-	return true;
+	return ret;
 }
diff --git a/libdevcore/CommonData.h b/libdevcore/CommonData.h
index 765707f8..e76a0949 100644
--- a/libdevcore/CommonData.h
+++ b/libdevcore/CommonData.h
@@ -209,4 +209,8 @@ bool contains(T const& _t, V const& _v)
 /// are considered valid.
 bool passesAddressChecksum(std::string const& _str, bool _strict);
 
+/// @returns the checksummed version of an address
+/// @param hex strings that look like an address
+std::string getChecksummedAddress(std::string const& _addr);
+
 }
diff --git a/libdevcore/Exceptions.h b/libdevcore/Exceptions.h
index a3e638bf..cfe72fbf 100644
--- a/libdevcore/Exceptions.h
+++ b/libdevcore/Exceptions.h
@@ -44,6 +44,7 @@ private:
 
 #define DEV_SIMPLE_EXCEPTION(X) struct X: virtual Exception { const char* what() const noexcept override { return #X; } }
 
+DEV_SIMPLE_EXCEPTION(InvalidAddress);
 DEV_SIMPLE_EXCEPTION(BadHexCharacter);
 DEV_SIMPLE_EXCEPTION(FileError);
 
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index 746e762e..73047e76 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -1122,7 +1122,7 @@ bool TypeChecker::visit(VariableDeclarationStatement const& _statement)
 						var.annotation().type->toString() +
 						". Try converting to type " +
 						valueComponentType->mobileType()->toString() +
-						" or use an explicit conversion." 
+						" or use an explicit conversion."
 					);
 				else
 					m_errorReporter.typeError(
@@ -1320,7 +1320,7 @@ bool TypeChecker::visit(TupleExpression const& _tuple)
 		_tuple.annotation().isPure = isPure;
 		if (_tuple.isInlineArray())
 		{
-			if (!inlineArrayType) 
+			if (!inlineArrayType)
 				m_errorReporter.fatalTypeError(_tuple.location(), "Unable to deduce common type for array elements.");
 			_tuple.annotation().type = make_shared<ArrayType>(DataLocation::Memory, inlineArrayType, types.size());
 		}
@@ -2000,7 +2000,9 @@ void TypeChecker::endVisit(Literal const& _literal)
 			m_errorReporter.warning(
 				_literal.location(),
 				"This looks like an address but has an invalid checksum. "
-				"If this is not used as an address, please prepend '00'."
+				"If this is not used as an address, please prepend '00'. " +
+				(!_literal.getChecksummedAddress().empty() ? "Correct checksummed address: '" + _literal.getChecksummedAddress() + "'. " : "") +
+				"For more information please see https://solidity.readthedocs.io/en/develop/types.html#address-literals"
 			);
 	}
 	if (!_literal.annotation().type)
diff --git a/libsolidity/ast/AST.cpp b/libsolidity/ast/AST.cpp
index 1048b610..8da6964e 100644
--- a/libsolidity/ast/AST.cpp
+++ b/libsolidity/ast/AST.cpp
@@ -583,3 +583,14 @@ bool Literal::passesAddressChecksum() const
 	solAssert(isHexNumber(), "Expected hex number");
 	return dev::passesAddressChecksum(value(), true);
 }
+
+std::string Literal::getChecksummedAddress() const
+{
+	solAssert(isHexNumber(), "Expected hex number");
+	/// Pad literal to be a proper hex address.
+	string address = value().substr(2);
+	if (address.length() > 40)
+		return string();
+	address.insert(address.begin(), 40 - address.size(), '0');
+	return dev::getChecksummedAddress(address);
+}
diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h
index 733e7c78..feffde64 100644
--- a/libsolidity/ast/AST.h
+++ b/libsolidity/ast/AST.h
@@ -1613,6 +1613,8 @@ public:
 	bool looksLikeAddress() const;
 	/// @returns true if it passes the address checksum test.
 	bool passesAddressChecksum() const;
+	/// @returns the checksummed version of an address (or empty string if not valid)
+	std::string getChecksummedAddress() const;
 
 private:
 	Token::Value m_token;
diff --git a/test/libdevcore/Checksum.cpp b/test/libdevcore/Checksum.cpp
index 17a17d22..4eedbd99 100644
--- a/test/libdevcore/Checksum.cpp
+++ b/test/libdevcore/Checksum.cpp
@@ -19,6 +19,8 @@
  */
 
 #include <libdevcore/CommonData.h>
+#include <libdevcore/Exceptions.h>
+
 
 #include "../TestHelper.h"
 
@@ -31,6 +33,38 @@ namespace test
 
 BOOST_AUTO_TEST_SUITE(Checksum)
 
+BOOST_AUTO_TEST_CASE(calculate)
+{
+	BOOST_CHECK(!getChecksummedAddress("0x5aaeb6053f3e94c9b9a09f33669435e7ef1beaed").empty());
+	BOOST_CHECK(!getChecksummedAddress("0x0123456789abcdefABCDEF0123456789abcdefAB").empty());
+	// too short
+	BOOST_CHECK_THROW(getChecksummedAddress("0x5aaeb6053f3e94c9b9a09f33669435e7ef1beae"), InvalidAddress);
+	BOOST_CHECK_THROW(getChecksummedAddress("5aaeb6053f3e94c9b9a09f33669435e7ef1beae"), InvalidAddress);
+	// too long
+	BOOST_CHECK_THROW(getChecksummedAddress("0x5aaeb6053f3e94c9b9a09f33669435e7ef1beaed1"), InvalidAddress);
+	BOOST_CHECK_THROW(getChecksummedAddress("5aaeb6053f3e94c9b9a09f33669435e7ef1beaed1"), InvalidAddress);
+	// non-hex character
+	BOOST_CHECK_THROW(getChecksummedAddress("0x5aaeb6053f3e94c9b9a09f33669435e7ef1beaeK"), InvalidAddress);
+
+	// the official test suite from EIP-55
+	vector<string> cases {
+		// all upper case
+		"0x52908400098527886E0F7030069857D2E4169EE7",
+		"0x8617E340B3D01FA5F11F306F4090FD50E238070D",
+		// all lower case
+		"0xde709f2102306220921060314715629080e2fb77",
+		"0x27b1fdb04752bbc536007a920d24acb045561c26",
+		// regular
+		"0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed",
+		"0xfB6916095ca1df60bB79Ce92cE3Ea74c37c5d359",
+		"0xdbF03B407c01E7cD3CBea99509d93f8DDDC8C6FB",
+		"0xD1220A0cf47c7B9Be7A2E6BA89F429762e7b9aDb"
+	};
+
+	for (size_t i = 0; i < cases.size(); i++)
+		BOOST_REQUIRE_MESSAGE(getChecksummedAddress(cases[i]) == cases[i], cases[i]);
+}
+
 BOOST_AUTO_TEST_CASE(regular)
 {
 	BOOST_CHECK(passesAddressChecksum("0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed", true));
diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp
index 30624260..88ec58ee 100644
--- a/test/libsolidity/SolidityNameAndTypeResolution.cpp
+++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp
@@ -5766,7 +5766,7 @@ BOOST_AUTO_TEST_CASE(invalid_address_checksum)
 			}
 		}
 	)";
-	CHECK_WARNING(text, "checksum");
+	CHECK_WARNING(text, "This looks like an address but has an invalid checksum.");
 }
 
 BOOST_AUTO_TEST_CASE(invalid_address_no_checksum)
@@ -5779,10 +5779,10 @@ BOOST_AUTO_TEST_CASE(invalid_address_no_checksum)
 			}
 		}
 	)";
-	CHECK_WARNING(text, "checksum");
+	CHECK_WARNING(text, "This looks like an address but has an invalid checksum.");
 }
 
-BOOST_AUTO_TEST_CASE(invalid_address_length)
+BOOST_AUTO_TEST_CASE(invalid_address_length_short)
 {
 	char const* text = R"(
 		contract C {
@@ -5792,7 +5792,20 @@ BOOST_AUTO_TEST_CASE(invalid_address_length)
 			}
 		}
 	)";
-	CHECK_WARNING(text, "checksum");
+	CHECK_WARNING(text, "This looks like an address but has an invalid checksum.");
+}
+
+BOOST_AUTO_TEST_CASE(invalid_address_length_long)
+{
+	char const* text = R"(
+		contract C {
+			function f() pure public {
+				address x = 0xFA0bFc97E48458494Ccd857e1A85DC91F7F0046E0;
+				x;
+			}
+		}
+	)";
+	CHECK_WARNING_ALLOW_MULTI(text, "This looks like an address but has an invalid checksum.");
 }
 
 BOOST_AUTO_TEST_CASE(address_test_for_bug_in_implementation)