From 999701e3840dbfde1a75452a8e5a4d53a299535e Mon Sep 17 00:00:00 2001 From: seelabs Date: Wed, 4 Nov 2015 16:18:43 -0500 Subject: [PATCH] Fix underflow rounding issue: Very small payment could fail when STAmount::mulRound underflowed and returned zero, when it should have rounded up to the smallest representable value. --- .../app/paths/cursor/DeliverNodeForward.cpp | 19 +++++--- .../app/paths/cursor/DeliverNodeReverse.cpp | 27 +++++++++--- .../cursor/ForwardLiquidityForAccount.cpp | 5 ++- .../app/paths/cursor/RippleLiquidity.cpp | 15 +++++-- src/ripple/app/tests/Offer.test.cpp | 39 ++++++++++++++++ src/ripple/app/tests/Taker.test.cpp | 3 +- src/ripple/app/tx/impl/CreateOffer.cpp | 20 ++++++--- src/ripple/app/tx/impl/CreateOffer.h | 3 +- src/ripple/app/tx/impl/Taker.cpp | 8 ++-- src/ripple/app/tx/impl/Taker.h | 2 +- src/ripple/protocol/Quality.h | 9 ++-- src/ripple/protocol/STAmount.h | 23 ++++++++-- src/ripple/protocol/impl/Quality.cpp | 15 ++++--- src/ripple/protocol/impl/STAmount.cpp | 44 +++++++++++++++++-- src/ripple/protocol/tests/Quality.test.cpp | 18 +++++--- 15 files changed, 200 insertions(+), 50 deletions(-) diff --git a/src/ripple/app/paths/cursor/DeliverNodeForward.cpp b/src/ripple/app/paths/cursor/DeliverNodeForward.cpp index 4e9a629dc..b27274333 100644 --- a/src/ripple/app/paths/cursor/DeliverNodeForward.cpp +++ b/src/ripple/app/paths/cursor/DeliverNodeForward.cpp @@ -43,6 +43,9 @@ TER PathCursor::deliverNodeForward ( // Zeroed in reverse pass. node().directory.restart(multiQuality_); + STAmountCalcSwitchovers amountCalcSwitchovers ( + rippleCalc_.view.info ().parentCloseTime); + saInAct.clear (saInReq); saInFees.clear (saInReq); @@ -107,11 +110,11 @@ TER PathCursor::deliverNodeForward ( saOutPassFunded, node().saOfrRate, node().saTakerPays.issue (), - true); + true, amountCalcSwitchovers); // Offer maximum in with fees. auto saInTotal = mulRound (saInFunded, saInFeeRate, - saInFunded.issue (), true); + saInFunded.issue (), true, amountCalcSwitchovers); auto saInRemaining = saInReq - saInAct - saInFees; if (saInRemaining < zero) @@ -123,11 +126,13 @@ TER PathCursor::deliverNodeForward ( // In without fees. auto saInPassAct = std::min ( node().saTakerPays, divRound ( - saInSum, saInFeeRate, saInSum.issue (), true)); + saInSum, saInFeeRate, saInSum.issue (), true, + amountCalcSwitchovers)); // Out limited by in remaining. auto outPass = divRound ( - saInPassAct, node().saOfrRate, node().saTakerGets.issue (), true); + saInPassAct, node().saOfrRate, node().saTakerGets.issue (), true, + amountCalcSwitchovers); STAmount saOutPassMax = std::min (saOutPassFunded, outPass); STAmount saInPassFeesMax = saInSum - saInPassAct; @@ -243,10 +248,12 @@ TER PathCursor::deliverNodeForward ( assert (saOutPassAct < saOutPassMax); auto inPassAct = mulRound ( - saOutPassAct, node().saOfrRate, saInReq.issue (), true); + saOutPassAct, node().saOfrRate, saInReq.issue (), true, + amountCalcSwitchovers); saInPassAct = std::min (node().saTakerPays, inPassAct); auto inPassFees = mulRound ( - saInPassAct, saInFeeRate, saInPassAct.issue (), true); + saInPassAct, saInFeeRate, saInPassAct.issue (), true, + amountCalcSwitchovers); saInPassFees = std::min (saInPassFeesMax, inPassFees); } diff --git a/src/ripple/app/paths/cursor/DeliverNodeReverse.cpp b/src/ripple/app/paths/cursor/DeliverNodeReverse.cpp index 420a559f8..12c2e9859 100644 --- a/src/ripple/app/paths/cursor/DeliverNodeReverse.cpp +++ b/src/ripple/app/paths/cursor/DeliverNodeReverse.cpp @@ -45,6 +45,9 @@ TER PathCursor::deliverNodeReverseImpl ( { TER resultCode = tesSUCCESS; + STAmountCalcSwitchovers amountCalcSwitchovers ( + rippleCalc_.view.info ().parentCloseTime); + // Accumulation of what the previous node must deliver. // Possible optimization: Note this gets zeroed on each increment, ideally // only on first increment, then it could be a limit on the forward pass. @@ -157,7 +160,10 @@ TER PathCursor::deliverNodeReverseImpl ( // // Round down: prefer liquidity rather than microscopic fees. STAmount saOutPlusFees = mulRound ( - saOutPassAct, saOutFeeRate, saOutPassAct.issue (), false); + saOutPassAct, saOutFeeRate, saOutPassAct.issue (), false, + amountCalcSwitchovers); + + // Offer out with fees. JLOG (j_.trace) @@ -178,7 +184,7 @@ TER PathCursor::deliverNodeReverseImpl ( // Round up: prefer liquidity rather than microscopic fees. But, // limit by requested. auto fee = divRound (saOutPlusFees, saOutFeeRate, - saOutPlusFees.issue (), true); + saOutPlusFees.issue (), true, amountCalcSwitchovers); saOutPassAct = std::min (saOutPassReq, fee); JLOG (j_.trace) @@ -190,7 +196,16 @@ TER PathCursor::deliverNodeReverseImpl ( // Compute portion of input needed to cover actual output. auto outputFee = mulRound ( - saOutPassAct, node().saOfrRate, node().saTakerPays.issue (), true); + saOutPassAct, node().saOfrRate, node().saTakerPays.issue (), true, + amountCalcSwitchovers); + if (!amountCalcSwitchovers.enableUnderflowFix () && !outputFee) + { + JLOG (j_.fatal) + << "underflow computing outputFee " + << "saOutPassAct: " << saOutPassAct + << " saOfrRate: " << node ().saOfrRate; + return telFAILED_PROCESSING; + } STAmount saInPassReq = std::min (node().saTakerPays, outputFee); STAmount saInPassAct; @@ -255,10 +270,12 @@ TER PathCursor::deliverNodeReverseImpl ( { // Adjust output to conform to limited input. auto outputRequirements = divRound ( - saInPassAct, node().saOfrRate, node().saTakerGets.issue (), true); + saInPassAct, node ().saOfrRate, node ().saTakerGets.issue (), true, + amountCalcSwitchovers); saOutPassAct = std::min (saOutPassReq, outputRequirements); auto outputFees = mulRound ( - saOutPassAct, saOutFeeRate, saOutPassAct.issue (), true); + saOutPassAct, saOutFeeRate, saOutPassAct.issue (), true, + amountCalcSwitchovers); saOutPlusFees = std::min (node().saOfferFunds, outputFees); JLOG (j_.trace) diff --git a/src/ripple/app/paths/cursor/ForwardLiquidityForAccount.cpp b/src/ripple/app/paths/cursor/ForwardLiquidityForAccount.cpp index 137a0701f..d61b9a64e 100644 --- a/src/ripple/app/paths/cursor/ForwardLiquidityForAccount.cpp +++ b/src/ripple/app/paths/cursor/ForwardLiquidityForAccount.cpp @@ -150,6 +150,9 @@ TER PathCursor::forwardLiquidityForAccount () const << " previousNode.saFwdRedeem:" << previousNode().saFwdRedeem << " previousNode.saFwdIssue:" << previousNode().saFwdIssue; + STAmountCalcSwitchovers amountCalcSwitchovers ( + rippleCalc_.view.info ().parentCloseTime); + // Last node. Accept all funds. Calculate amount actually to credit. auto& saCurReceive = pathState_.outPass(); @@ -159,7 +162,7 @@ TER PathCursor::forwardLiquidityForAccount () const previousNode().saFwdIssue, amountFromRate (uQualityIn), previousNode().saFwdIssue.issue (), - true); // Amount to credit. + true, amountCalcSwitchovers); // Amount to credit. // Amount to credit. Credit for less than received as a surcharge. pathState_.setOutPass (previousNode().saFwdRedeem + saIssueCrd); diff --git a/src/ripple/app/paths/cursor/RippleLiquidity.cpp b/src/ripple/app/paths/cursor/RippleLiquidity.cpp index a2b99af45..aaad07597 100644 --- a/src/ripple/app/paths/cursor/RippleLiquidity.cpp +++ b/src/ripple/app/paths/cursor/RippleLiquidity.cpp @@ -135,16 +135,21 @@ void rippleLiquidity ( // If the next rate is at least as good as the current rate, process. if (!uRateMax || uRate <= uRateMax) { + STAmountCalcSwitchovers amountCalcSwitchovers ( + rippleCalc.view.info ().parentCloseTime); + auto currency = saCur.getCurrency (); auto uCurIssuerID = saCur.getIssuer (); // current actual = current request * (quality out / quality in). auto numerator = mulRound ( - saCur, uQualityOut, {currency, uCurIssuerID}, true); + saCur, uQualityOut, {currency, uCurIssuerID}, true, + amountCalcSwitchovers); // True means "round up" to get best flow. STAmount saCurIn = divRound ( - numerator, uQualityIn, {currency, uCurIssuerID}, true); + numerator, uQualityIn, {currency, uCurIssuerID}, true, + amountCalcSwitchovers); JLOG (rippleCalc.j_.trace) << "rippleLiquidity:" @@ -173,11 +178,13 @@ void rippleLiquidity ( Issue issue{currency, uCurIssuerID}; auto numerator = mulRound ( - saPrv, uQualityIn, issue, true); + saPrv, uQualityIn, issue, true, + amountCalcSwitchovers); // A part of current. All of previous. (Cur is the driver // variable.) STAmount saCurOut = divRound ( - numerator, uQualityOut, issue, true); + numerator, uQualityOut, issue, true, + amountCalcSwitchovers); JLOG (rippleCalc.j_.trace) << "rippleLiquidity:4: saCurReq=" << saCurReq; diff --git a/src/ripple/app/tests/Offer.test.cpp b/src/ripple/app/tests/Offer.test.cpp index 4f2ab79dd..b0ba1bbb5 100644 --- a/src/ripple/app/tests/Offer.test.cpp +++ b/src/ripple/app/tests/Offer.test.cpp @@ -121,10 +121,49 @@ public: expect (isOffer (env, "alice", XRP (300), USD (100)) && isOffer (env, "alice", XRP (400), USD (200))); } + void testTinyPayment () + { + // Regression test for tiny payments + using namespace jtx; + auto const alice = Account ("alice"); + auto const bob = Account ("bob"); + auto const carol = Account ("carol"); + auto const gw = Account ("gw"); + + auto const USD = gw["USD"]; + auto const EUR = gw["EUR"]; + + Env env (*this); + + + env.fund (XRP (10000), alice, bob, carol, gw); + env.trust (USD (1000), alice, bob, carol); + env.trust (EUR (1000), alice, bob, carol); + env (pay (gw, alice, USD (100))); + env (pay (gw, carol, EUR (100))); + + // Create more offers than the loop max count in DeliverNodeReverse + for (int i=0;i<101;++i) + env (offer (carol, USD (1), EUR (2))); + + auto const switchoverTime = STAmountCalcSwitchovers::enableUnderflowFixCloseTime (); + + for (auto timeDelta : {-10, 10}){ + auto const closeTime = switchoverTime + timeDelta; + STAmountCalcSwitchovers switchover (closeTime); + env.close (NetClock::time_point (std::chrono::seconds (closeTime))); + // Will fail without the underflow fix + auto expectedResult = switchover.enableUnderflowFix () ? + tesSUCCESS : tecPATH_PARTIAL; + env (pay ("alice", "bob", EUR (epsilon)), path (~EUR), + sendmax (USD (100)), ter (expectedResult)); + } + } void run () { testCanceledOffer (); testRmFundedOffer (); + testTinyPayment (); } }; diff --git a/src/ripple/app/tests/Taker.test.cpp b/src/ripple/app/tests/Taker.test.cpp index 7c35e4ad9..bd888e3eb 100644 --- a/src/ripple/app/tests/Taker.test.cpp +++ b/src/ripple/app/tests/Taker.test.cpp @@ -84,7 +84,8 @@ class Taker_test : public beast::unit_test::suite cross (Amounts offer1, Quality quality1, Amounts offer2, Quality quality2) { /* check if composed quality should be rejected */ - Quality const quality (composed_quality (quality1, quality2)); + Quality const quality (composed_quality ( + quality1, quality2, STAmountCalcSwitchovers{false})); if (reject (quality)) return std::make_pair( diff --git a/src/ripple/app/tx/impl/CreateOffer.cpp b/src/ripple/app/tx/impl/CreateOffer.cpp index d81d1500e..55ea77419 100644 --- a/src/ripple/app/tx/impl/CreateOffer.cpp +++ b/src/ripple/app/tx/impl/CreateOffer.cpp @@ -272,7 +272,8 @@ CreateOffer::dry_offer (ApplyView& view, Offer const& offer) std::pair CreateOffer::select_path ( bool have_direct, OfferStream const& direct, - bool have_bridge, OfferStream const& leg1, OfferStream const& leg2) + bool have_bridge, OfferStream const& leg1, OfferStream const& leg2, + STAmountCalcSwitchovers const& amountCalcSwitchovers) { // If we don't have any viable path, why are we here?! assert (have_direct || have_bridge); @@ -282,7 +283,7 @@ CreateOffer::select_path ( return std::make_pair (true, direct.tip ().quality ()); Quality const bridged_quality (composed_quality ( - leg1.tip ().quality (), leg2.tip ().quality ())); + leg1.tip ().quality (), leg2.tip ().quality (), amountCalcSwitchovers)); if (have_direct) { @@ -349,6 +350,9 @@ CreateOffer::bridged_cross ( auto viewJ = ctx_.app.journal ("View"); + STAmountCalcSwitchovers amountCalcSwitchovers ( + view.info ().parentCloseTime); + // Modifying the order or logic of the operations in the loop will cause // a protocol breaking change. while (have_direct || have_bridge) @@ -362,7 +366,9 @@ CreateOffer::bridged_cross ( std::tie (use_direct, quality) = select_path ( have_direct, offers_direct, - have_bridge, offers_leg1, offers_leg2); + have_bridge, offers_leg1, offers_leg2, + amountCalcSwitchovers); + // We are always looking at the best quality; we are done with // crossing as soon as we cross the quality boundary. @@ -465,7 +471,7 @@ CreateOffer::bridged_cross ( Throw ("bridged crossing: nothing was fully consumed."); } - return std::make_pair(cross_result, taker.remaining_offer ()); + return std::make_pair(cross_result, taker.remaining_offer (amountCalcSwitchovers)); } std::pair @@ -546,7 +552,8 @@ CreateOffer::direct_cross ( Throw ("direct crossing: nothing was fully consumed."); } - return std::make_pair(cross_result, taker.remaining_offer ()); + STAmountCalcSwitchovers amountCalcSwitchovers (view.info ().parentCloseTime); + return std::make_pair(cross_result, taker.remaining_offer (amountCalcSwitchovers)); } // Step through the stream for as long as possible, skipping any offers @@ -590,6 +597,7 @@ CreateOffer::cross ( Taker taker (cross_type_, view, account_, taker_amount, ctx_.tx.getFlags(), beast::Journal (takerSink)); + STAmountCalcSwitchovers amountCalcSwitchovers (view.info ().parentCloseTime); try { if (cross_type_ == CrossType::IouToIou) @@ -600,7 +608,7 @@ CreateOffer::cross ( catch (std::exception const& e) { j_.error << "Exception during offer crossing: " << e.what (); - return std::make_pair (tecINTERNAL, taker.remaining_offer ()); + return std::make_pair (tecINTERNAL, taker.remaining_offer (amountCalcSwitchovers)); } } diff --git a/src/ripple/app/tx/impl/CreateOffer.h b/src/ripple/app/tx/impl/CreateOffer.h index e04435e79..dbc212438 100644 --- a/src/ripple/app/tx/impl/CreateOffer.h +++ b/src/ripple/app/tx/impl/CreateOffer.h @@ -77,7 +77,8 @@ private: std::pair select_path ( bool have_direct, OfferStream const& direct, - bool have_bridge, OfferStream const& leg1, OfferStream const& leg2); + bool have_bridge, OfferStream const& leg1, OfferStream const& leg2, + STAmountCalcSwitchovers const& amountCalcSwitchovers); std::pair bridged_cross ( diff --git a/src/ripple/app/tx/impl/Taker.cpp b/src/ripple/app/tx/impl/Taker.cpp index 399a2cd10..c6113b4bf 100644 --- a/src/ripple/app/tx/impl/Taker.cpp +++ b/src/ripple/app/tx/impl/Taker.cpp @@ -140,7 +140,7 @@ BasicTaker::done () const } Amounts -BasicTaker::remaining_offer () const +BasicTaker::remaining_offer (STAmountCalcSwitchovers const& amountCalcSwitchovers) const { // If the taker is done, then there's no offer to place. if (done ()) @@ -156,14 +156,16 @@ BasicTaker::remaining_offer () const // We scale the output based on the remaining input: return Amounts (remaining_.in, divRound ( - remaining_.in, quality_.rate (), issue_out_, true)); + remaining_.in, quality_.rate (), issue_out_, true, + amountCalcSwitchovers)); } assert (remaining_.out > zero); // We scale the input based on the remaining output: return Amounts (mulRound ( - remaining_.out, quality_.rate (), issue_in_, true), remaining_.out); + remaining_.out, quality_.rate (), issue_in_, true, amountCalcSwitchovers), + remaining_.out); } Amounts const& diff --git a/src/ripple/app/tx/impl/Taker.h b/src/ripple/app/tx/impl/Taker.h index e6db44997..b5033a35a 100644 --- a/src/ripple/app/tx/impl/Taker.h +++ b/src/ripple/app/tx/impl/Taker.h @@ -167,7 +167,7 @@ public: It is always at the original offer quality (quality_) */ Amounts - remaining_offer () const; + remaining_offer (STAmountCalcSwitchovers const& amountCalcSwitchovers) const; /** Returns the amount that the offer was originally placed at. */ Amounts const& diff --git a/src/ripple/protocol/Quality.h b/src/ripple/protocol/Quality.h index 60b564a0d..9f6c40285 100644 --- a/src/ripple/protocol/Quality.h +++ b/src/ripple/protocol/Quality.h @@ -131,14 +131,16 @@ public: to prevent money creation. */ Amounts - ceil_in (Amounts const& amount, STAmount const& limit) const; + ceil_in (Amounts const& amount, STAmount const& limit, + STAmountCalcSwitchovers const& switchovers) const; /** Returns the scaled amount with out capped. Math is avoided if the result is exact. The input is clamped to prevent money creation. */ Amounts - ceil_out (Amounts const& amount, STAmount const& limit) const; + ceil_out (Amounts const& amount, STAmount const& limit, + STAmountCalcSwitchovers const& switchovers) const; /** Returns `true` if lhs is lower quality than `rhs`. Lower quality means the taker receives a worse deal. @@ -179,7 +181,8 @@ public: @param rhs The second leg of the path: intermediate to output. */ Quality -composed_quality (Quality const& lhs, Quality const& rhs); +composed_quality (Quality const& lhs, Quality const& rhs, + STAmountCalcSwitchovers const& switchovers); } diff --git a/src/ripple/protocol/STAmount.h b/src/ripple/protocol/STAmount.h index aa47f9d62..3a75bd339 100644 --- a/src/ripple/protocol/STAmount.h +++ b/src/ripple/protocol/STAmount.h @@ -377,15 +377,32 @@ divide (STAmount const& v1, STAmount const& v2, Issue const& issue); STAmount multiply (STAmount const& v1, STAmount const& v2, Issue const& issue); -// multiply, or divide rounding result in specified direction +/** Control when bugfixes that require switchover dates are enabled */ +class STAmountCalcSwitchovers +{ + bool enableUnderflowFix_ {false}; + public: + STAmountCalcSwitchovers () = delete; + explicit + STAmountCalcSwitchovers (std::uint32_t parentCloseTime); + explicit + STAmountCalcSwitchovers (bool enableAll) + : enableUnderflowFix_ (enableAll) {} + bool enableUnderflowFix () const; + // for tests + static std::uint32_t enableUnderflowFixCloseTime (); +}; +// multiply, or divide rounding result in specified direction STAmount mulRound (STAmount const& v1, STAmount const& v2, - Issue const& issue, bool roundUp); + Issue const& issue, bool roundUp, + STAmountCalcSwitchovers const& switchovers); STAmount divRound (STAmount const& v1, STAmount const& v2, - Issue const& issue, bool roundUp); + Issue const& issue, bool roundUp, + STAmountCalcSwitchovers const& switchovers); // Someone is offering X for Y, what is the rate? // Rate: smaller is better, the taker wants the most out: in/out diff --git a/src/ripple/protocol/impl/Quality.cpp b/src/ripple/protocol/impl/Quality.cpp index fefcb7802..4fd7ffc8e 100644 --- a/src/ripple/protocol/impl/Quality.cpp +++ b/src/ripple/protocol/impl/Quality.cpp @@ -67,12 +67,13 @@ Quality::operator-- (int) } Amounts -Quality::ceil_in (Amounts const& amount, STAmount const& limit) const +Quality::ceil_in (Amounts const& amount, STAmount const& limit, + STAmountCalcSwitchovers const& switchovers) const { if (amount.in > limit) { Amounts result (limit, divRound ( - limit, rate(), amount.out.issue (), true)); + limit, rate(), amount.out.issue (), true, switchovers)); // Clamp out if (result.out > amount.out) result.out = amount.out; @@ -84,12 +85,13 @@ Quality::ceil_in (Amounts const& amount, STAmount const& limit) const } Amounts -Quality::ceil_out (Amounts const& amount, STAmount const& limit) const +Quality::ceil_out (Amounts const& amount, STAmount const& limit, + STAmountCalcSwitchovers const& switchovers) const { if (amount.out > limit) { Amounts result (mulRound ( - limit, rate(), amount.in.issue (), true), limit); + limit, rate(), amount.in.issue (), true, switchovers), limit); // Clamp in if (result.in > amount.in) result.in = amount.in; @@ -101,7 +103,8 @@ Quality::ceil_out (Amounts const& amount, STAmount const& limit) const } Quality -composed_quality (Quality const& lhs, Quality const& rhs) +composed_quality (Quality const& lhs, Quality const& rhs, + STAmountCalcSwitchovers const& switchovers) { STAmount const lhs_rate (lhs.rate ()); assert (lhs_rate != zero); @@ -110,7 +113,7 @@ composed_quality (Quality const& lhs, Quality const& rhs) assert (rhs_rate != zero); STAmount const rate (mulRound ( - lhs_rate, rhs_rate, lhs_rate.issue (), true)); + lhs_rate, rhs_rate, lhs_rate.issue (), true, switchovers)); std::uint64_t const stored_exponent (rate.exponent () + 100); std::uint64_t const stored_mantissa (rate.mantissa()); diff --git a/src/ripple/protocol/impl/STAmount.cpp b/src/ripple/protocol/impl/STAmount.cpp index 4746ccebf..4b623329c 100644 --- a/src/ripple/protocol/impl/STAmount.cpp +++ b/src/ripple/protocol/impl/STAmount.cpp @@ -1140,7 +1140,8 @@ canonicalizeRound (bool native, std::uint64_t& value, int& offset, bool roundUp) STAmount mulRound (STAmount const& v1, STAmount const& v2, - Issue const& issue, bool roundUp) + Issue const& issue, bool roundUp, + STAmountCalcSwitchovers const& switchovers) { if (v1 == zero || v2 == zero) return {issue}; @@ -1203,12 +1204,21 @@ mulRound (STAmount const& v1, STAmount const& v2, int offset = offset1 + offset2 + 14; canonicalizeRound ( isXRP (issue), amount, offset, resultNegative != roundUp); - return STAmount (issue, amount, offset, resultNegative); + STAmount result (issue, amount, offset, resultNegative); + if (switchovers.enableUnderflowFix () && roundUp && !resultNegative && !result) + { + // return the smallest value above zero + amount = STAmount::cMinValue; + offset = STAmount::cMinOffset; + return STAmount (issue, amount, offset, resultNegative); + } + return result; } STAmount divRound (STAmount const& num, STAmount const& den, - Issue const& issue, bool roundUp) + Issue const& issue, bool roundUp, + STAmountCalcSwitchovers const& switchovers) { if (den == zero) Throw ("division by zero"); @@ -1254,7 +1264,15 @@ divRound (STAmount const& num, STAmount const& den, int offset = numOffset - denOffset - 17; canonicalizeRound ( isXRP (issue), amount, offset, resultNegative != roundUp); - return STAmount (issue, amount, offset, resultNegative); + STAmount result (issue, amount, offset, resultNegative); + if (switchovers.enableUnderflowFix () && roundUp && !resultNegative && !result) + { + // return the smallest value above zero + amount = STAmount::cMinValue; + offset = STAmount::cMinOffset; + return STAmount (issue, amount, offset, resultNegative); + } + return result; } // compute (value)*(mul)/(div) - avoid overflow but keep precision @@ -1291,4 +1309,22 @@ mulDivNoThrow(std::uint64_t value, std::uint64_t mul, std::uint64_t div) } } +std::uint32_t +STAmountCalcSwitchovers::enableUnderflowFixCloseTime () +{ + // Mon Dec 28 10:00:00am PST + return 504'640'800; +} + +STAmountCalcSwitchovers::STAmountCalcSwitchovers (std::uint32_t parentCloseTime) +{ + enableUnderflowFix_ = parentCloseTime > enableUnderflowFixCloseTime(); +} + + +bool STAmountCalcSwitchovers::enableUnderflowFix () const +{ + return enableUnderflowFix_; +} + } // ripple diff --git a/src/ripple/protocol/tests/Quality.test.cpp b/src/ripple/protocol/tests/Quality.test.cpp index 959008cfd..2ee21cb02 100644 --- a/src/ripple/protocol/tests/Quality.test.cpp +++ b/src/ripple/protocol/tests/Quality.test.cpp @@ -70,7 +70,8 @@ public: In1 in, Out1 out, Int limit, In2 in_expected, Out2 out_expected) { auto expect_result (amounts (in_expected, out_expected)); - auto actual_result (q.ceil_in (amounts(in, out), amount(limit))); + auto actual_result (q.ceil_in ( + amounts (in, out), amount (limit), STAmountCalcSwitchovers{false})); expect (actual_result == expect_result); } @@ -81,7 +82,8 @@ public: In1 in, Out1 out, Int limit, In2 in_expected, Out2 out_expected) { auto const expect_result (amounts (in_expected, out_expected)); - auto const actual_result (q.ceil_out (amounts(in, out), amount(limit))); + auto const actual_result (q.ceil_out ( + amounts (in, out), amount (limit), STAmountCalcSwitchovers{false})); expect (actual_result == expect_result); } @@ -230,7 +232,8 @@ public: raw (2755280000000000ull, -15)); // 2.75528 STAmount const limit ( raw (4131113916555555, -16)); // .4131113916555555 - Amounts const result (q.ceil_out (value, limit)); + Amounts const result ( + q.ceil_out (value, limit, STAmountCalcSwitchovers{false})); expect (result.in != zero); } } @@ -273,10 +276,13 @@ public: Quality const q21 (Amounts (amount2, amount1)); Quality const q31 (Amounts (amount3, amount1)); - expect (composed_quality (q12, q21) == q11); + expect ( + composed_quality (q12, q21, STAmountCalcSwitchovers{false}) == q11); - Quality const q13_31 (composed_quality (q13, q31)); - Quality const q31_13 (composed_quality (q31, q13)); + Quality const q13_31 ( + composed_quality (q13, q31, STAmountCalcSwitchovers{false})); + Quality const q31_13 ( + composed_quality (q31, q13, STAmountCalcSwitchovers{false})); expect (q13_31 == q31_13); expect (q13_31 == q11);