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

util: Enhance Fraction class overflow resistance #2735

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
41 changes: 41 additions & 0 deletions src/test/util_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1385,6 +1385,24 @@ BOOST_AUTO_TEST_CASE(util_Fraction_addition_with_internal_simplification)
BOOST_CHECK_EQUAL(sum.IsSimplified(), true);
}

BOOST_AUTO_TEST_CASE(util_Fraction_addition_with_internal_gcd_simplification)
{
Fraction lhs(1, 6);
Fraction rhs(2, 15);

// gcd(6, 15) = 3, so this really is
//
// 1 * (15/3) + 2 * (6/3) 1 * 5 + 2 * 2 3
// ---------------------- = ------------- = --
// 3 * (6/3) * (15/3) 3 * 2 * 5 10

Fraction sum = lhs + rhs;

BOOST_CHECK_EQUAL(sum.GetNumerator(), 3);
BOOST_CHECK_EQUAL(sum.GetDenominator(), 10);
BOOST_CHECK_EQUAL(sum.IsSimplified(), true);
}

BOOST_AUTO_TEST_CASE(util_Fraction_subtraction)
{
Fraction lhs(2, 3);
Expand Down Expand Up @@ -1421,6 +1439,29 @@ BOOST_AUTO_TEST_CASE(util_Fraction_multiplication_with_internal_simplification)
BOOST_CHECK_EQUAL(product.IsSimplified(), true);
}

BOOST_AUTO_TEST_CASE(util_Fraction_multiplication_with_cross_simplification_overflow_resistance)
{

Fraction lhs(std::numeric_limits<int64_t>::max() - 3, std::numeric_limits<int64_t>::max() - 1, false);
Fraction rhs((std::numeric_limits<int64_t>::max() - 1) / (int64_t) 2, (std::numeric_limits<int64_t>::max() - 3) / (int64_t) 2);

Fraction product;

// This should NOT overflow
bool overflow = false;
try {
product = lhs * rhs;
} catch (std::overflow_error& e) {
overflow = true;
}

BOOST_CHECK_EQUAL(overflow, false);

if (!overflow) {
BOOST_CHECK(product == Fraction(1));
}
}

BOOST_AUTO_TEST_CASE(util_Fraction_division_with_internal_simplification)
{
Fraction lhs(-2, 3);
Expand Down
90 changes: 84 additions & 6 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -356,10 +356,60 @@ class Fraction {
return Fraction(overflow_add(slhs.GetNumerator(), srhs.GetNumerator()), slhs.GetDenominator(), true);
}

// Otherwise do the full pattern of getting a common denominator and adding, then simplify...
return Fraction(overflow_add(overflow_mult(slhs.GetNumerator(), srhs.GetDenominator()),
overflow_mult(slhs.GetDenominator(), srhs.GetNumerator())),
overflow_mult(slhs.GetDenominator(), srhs.GetDenominator()),
// Now the more complex case. In general, fraction addition follows this pattern:
//
// a c a * (d/g) + c * (b/g)
// - + - , g = gcd(b, d) => --------------------- where {(b/g), (d/g)} will be elements of the counting numbers.
// b d g * (b/g) * (d/g)
//
// (b/g) and (d/g) are divisible with no remainders precisely because of the definition of gcd.
//
// We have already covered the trivial common denominator case above before bothering to compute the gcd of the
// denominator.
int64_t denom_gcd = std::gcd(slhs.GetDenominator(), srhs.GetDenominator());

// We have two special cases. One is where g = b (i.e. d is actually a multiple of b). In this case,
// the expression simplifies to
//
// a * (d/b) + c
// -------------
// d
if (denom_gcd == slhs.GetDenominator()) {
return Fraction(overflow_add(overflow_mult(slhs.GetNumerator(), srhs.GetDenominator() / slhs.GetDenominator()),
srhs.GetNumerator()),
srhs.GetDenominator(),
true);
}

// The other is where g = d (i.e. b is actually a multiple of d). In this case,
// the expression simplifies to
//
// a + c * (b/d)
// -------------
// b
if (denom_gcd == srhs.GetDenominator()) {
return Fraction(overflow_add(overflow_mult(srhs.GetNumerator(), slhs.GetDenominator() / srhs.GetDenominator()),
slhs.GetNumerator()),
slhs.GetDenominator(),
true);
}

// Otherwise do the full pattern of getting a common denominator (pulling out the gcd of the denominators),
// and adding, then simplify...
//
// This approach is more complex than
//
// a * d + c * b
// -------------
// b * d
//
// but has the advantage of being more resistant to overflows, especially when the two denominators are related by a large
// gcd. In particular in Gridcoin's application with Allocations, the largest denominator of the allocations is 10000, so
// every allocation denominator in reduced form must be divisible evenly into 10000. This means the majority of fraction
// additions will be the two simpler cases above.
return Fraction(overflow_add(overflow_mult(slhs.GetNumerator(), srhs.GetDenominator() / denom_gcd),
overflow_mult(srhs.GetNumerator(), slhs.GetDenominator() / denom_gcd)),
overflow_mult(denom_gcd, overflow_mult(slhs.GetDenominator() / denom_gcd, srhs.GetDenominator() / denom_gcd)),
true);
}

Expand All @@ -385,8 +435,36 @@ class Fraction {
Fraction slhs(*this, true);
Fraction srhs(rhs, true);

return Fraction(overflow_mult(slhs.GetNumerator(), srhs.GetNumerator()),
overflow_mult(slhs.GetDenominator(), srhs.GetDenominator()),
// Gcd's can be used in multiplication for better overflow resistance as well.
//
// Consider
// a c
// - * -, where a/b and c/d are already simplified (i.e. gcd(a, b) = gcd(c, d) = 1.
// b d
//
// We can have g = gcd(a, d) and h = gcd(c, b), which is with the numerators reversed, since multiplication is
// commutative. This means we have
//
// (c / h) (a / g)
// ------- * ------- .
// (b / h) (d / g)
//
// If we form Fraction(c, b, true) and Fraction(a, d, true), the simplification will determine and divide the numerator and
// denominator by h and g respectively.
//
// A specific example is instructive.
//
// 1998 1000 999 1000 1000 999 1 1
// ---- * ---- = ---- * ---- = ---- * --- = - * -
// 2000 999 1000 999 1000 999 1 1
//
// This is a formal form of what grade school teachers called factor cancellation. :).

Fraction sxlhs(srhs.GetNumerator(), slhs.GetDenominator(), true);
Fraction sxrhs(slhs.GetNumerator(), srhs.GetDenominator(), true);

return Fraction(overflow_mult(sxlhs.GetNumerator(), sxrhs.GetNumerator()),
overflow_mult(sxlhs.GetDenominator(), sxrhs.GetDenominator()),
true);
}

Expand Down
Loading