Merge pull request #4720 from ethereum/abi-rational-type-identifier

Fix type identifiers for RationalNumberType (on negative numbers)

4 files changed, 48 insertions, 1 deletions

diff --git a/Changelog.md b/Changelog.md
index 037451ed..dbb44983 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -79,6 +79,7 @@ Compiler Features:
 Bugfixes:
  * Tests: Fix chain parameters to make ipc tests work with newer versions of cpp-ethereum.
  * Code Generator: Fix allocation of byte arrays (zeroed out too much memory).
+ * Code Generator: Properly handle negative number literals in ABIEncoderV2.
  * Commandline Interface: Correctly handle paths with backslashes on windows.
  * Fix NatSpec json output for `@notice` and `@dev` tags on contract definitions.
  * References Resolver: Do not crash on using ``_slot`` and ``_offset`` suffixes on their own.
diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp
index 7bcae812..eff31b8d 100644
--- a/libsolidity/ast/Types.cpp
+++ b/libsolidity/ast/Types.cpp
@@ -1146,7 +1146,14 @@ TypePointer RationalNumberType::binaryOperatorResult(Token::Value _operator, Typ
 
 string RationalNumberType::richIdentifier() const
 {
-	return "t_rational_" + m_value.numerator().str() + "_by_" + m_value.denominator().str();
+	// rational seemingly will put the sign always on the numerator,
+	// but let just make it deterministic here.
+	bigint numerator = abs(m_value.numerator());
+	bigint denominator = abs(m_value.denominator());
+	if (m_value < 0)
+		return "t_rational_minus_" + numerator.str() + "_by_" + denominator.str();
+	else
+		return "t_rational_" + numerator.str() + "_by_" + denominator.str();
 }
 
 bool RationalNumberType::operator==(Type const& _other) const
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index 9c287e5e..bd1d9de7 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -12527,6 +12527,42 @@ BOOST_AUTO_TEST_CASE(abi_encode_empty_string_v2)
 		0x00
 	));
 }
+
+BOOST_AUTO_TEST_CASE(abi_encode_rational)
+{
+	char const* sourceCode = R"(
+		// Tests that rational numbers (even negative ones) are encoded properly.
+		contract C {
+			function f() public pure returns (bytes memory) {
+				return abi.encode(1, -2);
+			}
+		}
+	)";
+	compileAndRun(sourceCode, 0, "C");
+	ABI_CHECK(callContractFunction("f()"), encodeArgs(
+		0x20,
+		0x40, u256(1), u256(-2)
+	));
+}
+
+BOOST_AUTO_TEST_CASE(abi_encode_rational_v2)
+{
+	char const* sourceCode = R"(
+		// Tests that rational numbers (even negative ones) are encoded properly.
+		pragma experimental ABIEncoderV2;
+		contract C {
+			function f() public pure returns (bytes memory) {
+				return abi.encode(1, -2);
+			}
+		}
+	)";
+	compileAndRun(sourceCode, 0, "C");
+	ABI_CHECK(callContractFunction("f()"), encodeArgs(
+		0x20,
+		0x40, u256(1), u256(-2)
+	));
+}
+
 BOOST_AUTO_TEST_CASE(abi_encode_call)
 {
 	char const* sourceCode = R"T(
diff --git a/test/libsolidity/SolidityTypes.cpp b/test/libsolidity/SolidityTypes.cpp
index b8b537e5..db0e3b66 100644
--- a/test/libsolidity/SolidityTypes.cpp
+++ b/test/libsolidity/SolidityTypes.cpp
@@ -157,6 +157,9 @@ BOOST_AUTO_TEST_CASE(type_identifiers)
 	BOOST_CHECK_EQUAL(RationalNumberType(rational(7, 1)).identifier(), "t_rational_7_by_1");
 	BOOST_CHECK_EQUAL(RationalNumberType(rational(200, 77)).identifier(), "t_rational_200_by_77");
 	BOOST_CHECK_EQUAL(RationalNumberType(rational(2 * 200, 2 * 77)).identifier(), "t_rational_200_by_77");
+	BOOST_CHECK_EQUAL(RationalNumberType(rational(-2 * 200, -2 * 77)).identifier(), "t_rational_200_by_77");
+	BOOST_CHECK_EQUAL(RationalNumberType(rational(-2 * 200, 2 * 77)).identifier(), "t_rational_minus_200_by_77");
+	BOOST_CHECK_EQUAL(RationalNumberType(rational(2 * 200, -2 * 77)).identifier(), "t_rational_minus_200_by_77");
 	BOOST_CHECK_EQUAL(
 		StringLiteralType(Literal(SourceLocation{}, Token::StringLiteral, make_shared<string>("abc - def"))).identifier(),
 		 "t_stringliteral_196a9142ee0d40e274a6482393c762b16dd8315713207365e1e13d8d85b74fc4"