ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-07-24 23:50:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

Remove premature, incorrect optimization

Browse Source 
- pushOverflow had a shortcut that if the value rounded down to kMaxRep,
  then the work was done, but neglected to consider what would happen
  back in doRoundUp if round() returned Even. Even rounded up on 1.5 in
  pushOverflow, but on 0.5 in doRoundUp, so you'd get an incorrect
  result.
- Updated tests to reflect the behavior for older modes.behavior for
  older modes.behavior for older modes.behavior for older modes.
This commit is contained in:
Ed Hennis
2026-07-10 14:14:35 -04:00
parent 38e14f8439
commit 4d19c26080
2 changed files with 11 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
src/libxrpl/basics/Number.cpp
View File

@@ -431,12 +431,6 @@ Number::Guard::pushOverflow(T mantissa)
}
}
if (mantissa == kMaxRep)
{
// If the mantissa ends up exactly kMaxRep, there's nothing more to do here.
return;
}
// The second step scales the final digit of the updated mantissa proportionally, converting
// from (kMaxRep, kMaxRepUp) to (0 to 9]. It then pushes that scaled digit onto the guard as
// if it was a digit that got removed, but doesn't actually remove it. This method should be
@@ -455,7 +449,7 @@ Number::Guard::pushOverflow(T mantissa)
auto const diff = mantissa - kMaxRep;
auto const digit = (diff * 10) / spread;
XRPL_ASSERT(
digit > 0 && digit < 10 && digit != 5,
digit >= 0 && digit < 10 && digit != 5,
"xrpl::Number::Guard::pushOverflow : valid overflow digit");
// Don't remove the digit from the mantissa, but add it to the guard as if it was.

14
src/tests/libxrpl/basics/Number.cpp
View File

@@ -2730,25 +2730,30 @@ TEST(NumberTest, number_cusp_rounding_with_fractional_parts)
auto header = [&] {
std::ostringstream log;
log << "Below: " << below << ", Above: " << above << "\n";
log << "Scale: " << to_string(mantissaScale) << ", Below: " << below
<< ", Above: " << above << "\n";
return log.str();
};
auto const zeroPointFour = Number(4, -1);
auto const zeroPointFive = Number(5, -1);
auto const zeroPointSix = Number(6, -1);
auto const onePointFour = Number(14, -1);
auto const onePointFive = Number(15, -1);
auto const onePointSix = Number(16, -1);
auto const twoPointFour = Number(24, -1);
auto const twoPointFive = Number(25, -1);
auto const twoPointSix = Number(26, -1);
auto const operands = std::to_array<Number>({
zeroPointFour,
zeroPointFive,
zeroPointSix,
onePointFour,
onePointFive,
onePointSix,
twoPointFour,
twoPointFive,
twoPointSix,
});
@@ -2767,6 +2772,7 @@ TEST(NumberTest, number_cusp_rounding_with_fractional_parts)
NumberRoundModeGuard const rg{mode};
auto const expectedValue = [&]() {
// Returns "above" by default. The checks here are for exceptions.
if (scale >= MantissaRange::MantissaScale::Large330)
{
if (mode == Number::RoundingMode::ToNearest && operand < onePointFive)
@@ -2779,7 +2785,7 @@ TEST(NumberTest, number_cusp_rounding_with_fractional_parts)
{
if (mode == Number::RoundingMode::ToNearest)
{
if (operand < zeroPointSix)
if (operand < zeroPointFive)
return below;
}
if (mode == Number::RoundingMode::TowardsZero ||
@@ -2794,9 +2800,9 @@ TEST(NumberTest, number_cusp_rounding_with_fractional_parts)
{
if (mode == Number::RoundingMode::ToNearest)
{
if (operand < zeroPointSix)
if (operand < zeroPointFive)
return below;
if (operand == zeroPointSix)
if (operand <= zeroPointSix)
return below - 7;
}
if (mode == Number::RoundingMode::TowardsZero ||