Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improves checksum error message #3031

Merged
merged 6 commits into from
Nov 17, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
27 changes: 17 additions & 10 deletions libdevcore/CommonData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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;
}
4 changes: 4 additions & 0 deletions libdevcore/CommonData.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

}
1 change: 1 addition & 0 deletions libdevcore/Exceptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ struct Exception: virtual std::exception, virtual boost::exception

#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);

Expand Down
8 changes: 5 additions & 3 deletions libsolidity/analysis/TypeChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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)
Expand Down
11 changes: 11 additions & 0 deletions libsolidity/ast/AST.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
2 changes: 2 additions & 0 deletions libsolidity/ast/AST.h
Original file line number Diff line number Diff line change
Expand Up @@ -1613,6 +1613,8 @@ class Literal: public PrimaryExpression
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;
Expand Down
34 changes: 34 additions & 0 deletions test/libdevcore/Checksum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
*/

#include <libdevcore/CommonData.h>
#include <libdevcore/Exceptions.h>


#include "../TestHelper.h"

Expand All @@ -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));
Expand Down
21 changes: 17 additions & 4 deletions test/libsolidity/SolidityNameAndTypeResolution.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand All @@ -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)
Expand Down