From 5a7fa8cfa9edf880073cb37b97670e9ba4222787 Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Wed, 22 Apr 2015 18:56:47 -0700 Subject: [PATCH] Reduce STAmount public interface (RIPD-867): The STAmount class includes a number of functions which serve as thin wrappers, which are unused or used only in one place, or which break encapsulation by exposing internal implemenation details. Removing such functions simplifies the interface of the class and ensures consistency. * getSNValue and getNValue are now free functions * canonicalizeRound is no longer exposed * Removed addRound and subRound * Removed overloads of multiply, mulRound, divide and divRound --- src/ripple/app/book/impl/Quality.cpp | 6 +- src/ripple/app/book/impl/Taker.cpp | 6 +- src/ripple/app/misc/NetworkOPs.cpp | 12 +- src/ripple/app/paths/Pathfinder.cpp | 2 +- .../app/paths/cursor/DeliverNodeForward.cpp | 13 +- .../app/paths/cursor/DeliverNodeReverse.cpp | 11 +- .../cursor/ForwardLiquidityForAccount.cpp | 1 + src/ripple/app/transactors/CreateOffer.cpp | 2 +- src/ripple/app/transactors/CreateTicket.cpp | 2 +- src/ripple/app/transactors/Payment.cpp | 4 +- src/ripple/app/transactors/SetTrust.cpp | 6 +- src/ripple/app/tx/TransactionEngine.cpp | 3 +- src/ripple/legacy/0.27/CreateOffer27.cpp | 2 +- .../legacy/0.27/book/impl/Quality27.cpp | 6 +- src/ripple/legacy/0.27/book/impl/Taker27.cpp | 8 +- src/ripple/protocol/STAmount.h | 89 +----- src/ripple/protocol/impl/STAmount.cpp | 299 ++++-------------- src/ripple/protocol/tests/STAmount.test.cpp | 16 - 18 files changed, 110 insertions(+), 378 deletions(-) diff --git a/src/ripple/app/book/impl/Quality.cpp b/src/ripple/app/book/impl/Quality.cpp index 8a1f9bde07..92e6c41f92 100644 --- a/src/ripple/app/book/impl/Quality.cpp +++ b/src/ripple/app/book/impl/Quality.cpp @@ -73,7 +73,7 @@ Quality::ceil_in (Amounts const& amount, Amount const& limit) const if (amount.in > limit) { Amounts result (limit, divRound ( - limit, rate(), amount.out, true)); + limit, rate(), amount.out.issue (), true)); // Clamp out if (result.out > amount.out) result.out = amount.out; @@ -90,7 +90,7 @@ Quality::ceil_out (Amounts const& amount, Amount const& limit) const if (amount.out > limit) { Amounts result (mulRound ( - limit, rate(), amount.in, true), limit); + limit, rate(), amount.in.issue (), true), limit); // Clamp in if (result.in > amount.in) result.in = amount.in; @@ -110,7 +110,7 @@ composed_quality (Quality const& lhs, Quality const& rhs) Amount const rhs_rate (rhs.rate ()); assert (rhs_rate != zero); - Amount const rate (mulRound (lhs_rate, rhs_rate, true)); + Amount const rate (mulRound (lhs_rate, rhs_rate, lhs_rate.issue (), true)); std::uint64_t const stored_exponent (rate.exponent () + 100); std::uint64_t const stored_mantissa (rate.mantissa()); diff --git a/src/ripple/app/book/impl/Taker.cpp b/src/ripple/app/book/impl/Taker.cpp index ab98ed6c91..0c5a027e05 100644 --- a/src/ripple/app/book/impl/Taker.cpp +++ b/src/ripple/app/book/impl/Taker.cpp @@ -136,14 +136,14 @@ BasicTaker::remaining_offer () const // We scale the output based on the remaining input: return Amounts (remaining_.in, divRound ( - remaining_.in, quality_.rate (), remaining_.out, true)); + remaining_.in, quality_.rate (), issue_out_, true)); } assert (remaining_.out > zero); // We scale the input based on the remaining output: return Amounts (mulRound ( - remaining_.out, quality_.rate (), remaining_.in, true), remaining_.out); + remaining_.out, quality_.rate (), issue_in_, true), remaining_.out); } Amounts const& @@ -327,7 +327,7 @@ BasicTaker::do_cross (Amounts offer, Quality quality, Account const& owner) if (cross_type_ == CrossType::XrpToIou) { - result = flow_xrp_to_iou (offer, quality, owner_funds, taker_funds, + result = flow_xrp_to_iou (offer, quality, owner_funds, taker_funds, out_rate (owner, account ())); } else if (cross_type_ == CrossType::IouToXrp) diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 5b730abf4c..3f48d0c799 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -3007,7 +3007,9 @@ void NetworkOPsImp::getBookPage ( // Need to charge a transfer fee to offer owner. uOfferRate = uTransferRate; saOwnerFundsLimit = divide ( - saOwnerFunds, STAmount (noIssue(), uOfferRate, -9)); + saOwnerFunds, + STAmount (noIssue(), uOfferRate, -9), + saOwnerFunds.issue ()); // TODO(tom): why -9? } else @@ -3030,7 +3032,7 @@ void NetworkOPsImp::getBookPage ( saTakerGetsFunded.setJson (jvOffer[jss::taker_gets_funded]); std::min ( saTakerPays, multiply ( - saTakerGetsFunded, saDirRate, saTakerPays)).setJson + saTakerGetsFunded, saDirRate, saTakerPays.issue ())).setJson (jvOffer[jss::taker_pays_funded]); } @@ -3040,8 +3042,8 @@ void NetworkOPsImp::getBookPage ( saOwnerFunds, multiply ( saTakerGetsFunded, - STAmount (noIssue(), - uOfferRate, -9))); + STAmount (noIssue(), uOfferRate, -9), + saTakerGetsFunded.issue ())); umBalance[uOfferOwnerID] = saOwnerFunds - saOwnerPays; @@ -3196,7 +3198,7 @@ void NetworkOPsImp::getBookPage ( // TOOD(tom): The result of this expression is not used - what's // going on here? std::min (saTakerPays, multiply ( - saTakerGetsFunded, saDirRate, saTakerPays)).setJson ( + saTakerGetsFunded, saDirRate, saTakerPays.issue ())).setJson ( jvOffer[jss::taker_pays_funded]); } diff --git a/src/ripple/app/paths/Pathfinder.cpp b/src/ripple/app/paths/Pathfinder.cpp index 8a41f995b2..112913f2c7 100644 --- a/src/ripple/app/paths/Pathfinder.cpp +++ b/src/ripple/app/paths/Pathfinder.cpp @@ -430,7 +430,7 @@ namespace { // total number of paths we have to evaluate. STAmount smallestUsefulAmount (STAmount const& amount, int maxPaths) { - return divide (amount, STAmount (maxPaths + 2), amount); + return divide (amount, STAmount (maxPaths + 2), amount.issue ()); } } // namespace diff --git a/src/ripple/app/paths/cursor/DeliverNodeForward.cpp b/src/ripple/app/paths/cursor/DeliverNodeForward.cpp index 702de7ff60..e5fe9e841f 100644 --- a/src/ripple/app/paths/cursor/DeliverNodeForward.cpp +++ b/src/ripple/app/paths/cursor/DeliverNodeForward.cpp @@ -102,11 +102,12 @@ TER PathCursor::deliverNodeForward ( auto saInFunded = mulRound ( saOutPassFunded, node().saOfrRate, - node().saTakerPays, + node().saTakerPays.issue (), true); // Offer maximum in with fees. - auto saInTotal = mulRound (saInFunded, saInFeeRate, true); + auto saInTotal = mulRound (saInFunded, saInFeeRate, + saInFunded.issue (), true); auto saInRemaining = saInReq - saInAct - saInFees; if (saInRemaining < zero) @@ -118,11 +119,11 @@ TER PathCursor::deliverNodeForward ( // In without fees. auto saInPassAct = std::min ( node().saTakerPays, divRound ( - saInSum, saInFeeRate, true)); + saInSum, saInFeeRate, saInSum.issue (), true)); // Out limited by in remaining. auto outPass = divRound ( - saInPassAct, node().saOfrRate, node().saTakerGets, true); + saInPassAct, node().saOfrRate, node().saTakerGets.issue (), true); STAmount saOutPassMax = std::min (saOutPassFunded, outPass); STAmount saInPassFeesMax = saInSum - saInPassAct; @@ -238,10 +239,10 @@ TER PathCursor::deliverNodeForward ( assert (saOutPassAct < saOutPassMax); auto inPassAct = mulRound ( - saOutPassAct, node().saOfrRate, saInReq, true); + saOutPassAct, node().saOfrRate, saInReq.issue (), true); saInPassAct = std::min (node().saTakerPays, inPassAct); auto inPassFees = mulRound ( - saInPassAct, saInFeeRate, true); + saInPassAct, saInFeeRate, saInPassAct.issue (), true); saInPassFees = std::min (saInPassFeesMax, inPassFees); } diff --git a/src/ripple/app/paths/cursor/DeliverNodeReverse.cpp b/src/ripple/app/paths/cursor/DeliverNodeReverse.cpp index a29d998d75..484596364b 100644 --- a/src/ripple/app/paths/cursor/DeliverNodeReverse.cpp +++ b/src/ripple/app/paths/cursor/DeliverNodeReverse.cpp @@ -154,7 +154,7 @@ TER PathCursor::deliverNodeReverse ( // // Round down: prefer liquidity rather than microscopic fees. STAmount saOutPlusFees = mulRound ( - saOutPassAct, saOutFeeRate, false); + saOutPassAct, saOutFeeRate, saOutPassAct.issue (), false); // Offer out with fees. WriteLog (lsTRACE, RippleCalc) @@ -174,7 +174,8 @@ TER PathCursor::deliverNodeReverse ( // Round up: prefer liquidity rather than microscopic fees. But, // limit by requested. - auto fee = divRound (saOutPlusFees, saOutFeeRate, true); + auto fee = divRound (saOutPlusFees, saOutFeeRate, + saOutPlusFees.issue (), true); saOutPassAct = std::min (saOutPassReq, fee); WriteLog (lsTRACE, RippleCalc) @@ -186,7 +187,7 @@ TER PathCursor::deliverNodeReverse ( // Compute portion of input needed to cover actual output. auto outputFee = mulRound ( - saOutPassAct, node().saOfrRate, node().saTakerPays, true); + saOutPassAct, node().saOfrRate, node().saTakerPays.issue (), true); STAmount saInPassReq = std::min (node().saTakerPays, outputFee); STAmount saInPassAct; @@ -251,10 +252,10 @@ TER PathCursor::deliverNodeReverse ( { // Adjust output to conform to limited input. auto outputRequirements = divRound ( - saInPassAct, node().saOfrRate, node().saTakerGets, true); + saInPassAct, node().saOfrRate, node().saTakerGets.issue (), true); saOutPassAct = std::min (saOutPassReq, outputRequirements); auto outputFees = mulRound ( - saOutPassAct, saOutFeeRate, true); + saOutPassAct, saOutFeeRate, saOutPassAct.issue (), true); saOutPlusFees = std::min (node().saOfferFunds, outputFees); WriteLog (lsTRACE, RippleCalc) diff --git a/src/ripple/app/paths/cursor/ForwardLiquidityForAccount.cpp b/src/ripple/app/paths/cursor/ForwardLiquidityForAccount.cpp index 9e2c4c67a9..7add9a7484 100644 --- a/src/ripple/app/paths/cursor/ForwardLiquidityForAccount.cpp +++ b/src/ripple/app/paths/cursor/ForwardLiquidityForAccount.cpp @@ -158,6 +158,7 @@ TER PathCursor::forwardLiquidityForAccount () const : mulRound ( previousNode().saFwdIssue, STAmount (noIssue(), uQualityIn, -9), + previousNode().saFwdIssue.issue (), true); // Amount to credit. // Amount to credit. Credit for less than received as a surcharge. diff --git a/src/ripple/app/transactors/CreateOffer.cpp b/src/ripple/app/transactors/CreateOffer.cpp index 0fa1aaddb3..842b6a3e48 100644 --- a/src/ripple/app/transactors/CreateOffer.cpp +++ b/src/ripple/app/transactors/CreateOffer.cpp @@ -774,7 +774,7 @@ public: return tesSUCCESS; } - if (mPriorBalance.getNValue () < getAccountReserve (sleCreator)) + if (mPriorBalance < getAccountReserve (sleCreator)) { // If we are here, the signing account had an insufficient reserve // *prior* to our processing. If something actually crossed, then diff --git a/src/ripple/app/transactors/CreateTicket.cpp b/src/ripple/app/transactors/CreateTicket.cpp index e27ee0108c..04336445e7 100644 --- a/src/ripple/app/transactors/CreateTicket.cpp +++ b/src/ripple/app/transactors/CreateTicket.cpp @@ -67,7 +67,7 @@ public: auto const accountReserve (mEngine->getLedger ()->getReserve ( mTxnAccount->getFieldU32 (sfOwnerCount) + 1)); - if (mPriorBalance.getNValue () < accountReserve) + if (mPriorBalance < accountReserve) return tecINSUFFICIENT_RESERVE; std::uint32_t expiration (0); diff --git a/src/ripple/app/transactors/Payment.cpp b/src/ripple/app/transactors/Payment.cpp index fe6f575c20..97a5bdc4c1 100644 --- a/src/ripple/app/transactors/Payment.cpp +++ b/src/ripple/app/transactors/Payment.cpp @@ -218,7 +218,7 @@ public: // transaction would succeed. return telNO_DST_PARTIAL; } - else if (saDstAmount.getNValue () < mEngine->getLedger ()->getReserve (0)) + else if (saDstAmount < mEngine->getLedger ()->getReserve (0)) { // getReserve() is the minimum amount that an account can have. // Reserve is not scaled by load. @@ -344,7 +344,7 @@ public: // // Make sure have enough reserve to send. Allow final spend to use // reserve for fee. - auto const mmm = std::max(uReserve, mTxn.getTransactionFee ().getNValue ()); + auto const mmm = std::max(uReserve, getNValue (mTxn.getTransactionFee ())); if (mPriorBalance < saDstAmount + mmm) { // Vote no. diff --git a/src/ripple/app/transactors/SetTrust.cpp b/src/ripple/app/transactors/SetTrust.cpp index 4c2dd2d52b..0af339c414 100644 --- a/src/ripple/app/transactors/SetTrust.cpp +++ b/src/ripple/app/transactors/SetTrust.cpp @@ -379,8 +379,8 @@ public: terResult = mEngine->view ().trustDelete (sleRippleState, uLowAccountID, uHighAccountID); } - else if (bReserveIncrease - && mPriorBalance.getNValue () < uReserveCreate) // Reserve is not scaled by load. + // Reserve is not scaled by load. + else if (bReserveIncrease && mPriorBalance < uReserveCreate) { m_journal.trace << "Delay transaction: Insufficent reserve to add trust line."; @@ -405,7 +405,7 @@ public: "Redundant: Setting non-existent ripple line to defaults."; return tecNO_LINE_REDUNDANT; } - else if (mPriorBalance.getNValue () < uReserveCreate) // Reserve is not scaled by load. + else if (mPriorBalance < uReserveCreate) // Reserve is not scaled by load. { m_journal.trace << "Delay transaction: Line does not exist. Insufficent reserve to create line."; diff --git a/src/ripple/app/tx/TransactionEngine.cpp b/src/ripple/app/tx/TransactionEngine.cpp index dc6cd07ded..19fadb8f67 100644 --- a/src/ripple/app/tx/TransactionEngine.cpp +++ b/src/ripple/app/tx/TransactionEngine.cpp @@ -242,8 +242,7 @@ TransactionEngine::applyTransaction ( } // Charge whatever fee they specified. - STAmount saPaid = txn.getTransactionFee (); - mLedger->destroyCoins (saPaid.getNValue ()); + mLedger->destroyCoins (getNValue (txn.getTransactionFee ())); } } diff --git a/src/ripple/legacy/0.27/CreateOffer27.cpp b/src/ripple/legacy/0.27/CreateOffer27.cpp index d9971768f2..737d336540 100644 --- a/src/ripple/legacy/0.27/CreateOffer27.cpp +++ b/src/ripple/legacy/0.27/CreateOffer27.cpp @@ -466,7 +466,7 @@ CreateOffer::doApply() { // Complete as is. } - else if (mPriorBalance.getNValue () < accountReserve) + else if (mPriorBalance < accountReserve) { // If we are here, the signing account had an insufficient reserve // *prior* to our processing. We use the prior balance to simplify diff --git a/src/ripple/legacy/0.27/book/impl/Quality27.cpp b/src/ripple/legacy/0.27/book/impl/Quality27.cpp index c4ca287171..2b14c007f8 100644 --- a/src/ripple/legacy/0.27/book/impl/Quality27.cpp +++ b/src/ripple/legacy/0.27/book/impl/Quality27.cpp @@ -74,7 +74,7 @@ Quality::ceil_in (Amounts const& amount, Amount const& limit) const if (amount.in > limit) { Amounts result (limit, divRound ( - limit, rate(), amount.out, true)); + limit, rate(), amount.out.issue (), true)); // Clamp out if (result.out > amount.out) result.out = amount.out; @@ -91,7 +91,7 @@ Quality::ceil_out (Amounts const& amount, Amount const& limit) const if (amount.out > limit) { Amounts result (mulRound ( - limit, rate(), amount.in, true), limit); + limit, rate(), amount.in.issue (), true), limit); // Clamp in if (result.in > amount.in) result.in = amount.in; @@ -111,7 +111,7 @@ composed_quality (Quality const& lhs, Quality const& rhs) Amount const rhs_rate (rhs.rate ()); assert (rhs_rate != zero); - Amount const rate (mulRound (lhs_rate, rhs_rate, true)); + Amount const rate (mulRound (lhs_rate, rhs_rate, lhs_rate.issue (), true)); std::uint64_t const stored_exponent (rate.exponent () + 100); std::uint64_t const stored_mantissa (rate.mantissa()); diff --git a/src/ripple/legacy/0.27/book/impl/Taker27.cpp b/src/ripple/legacy/0.27/book/impl/Taker27.cpp index 500bb10221..bcf4c74fe8 100644 --- a/src/ripple/legacy/0.27/book/impl/Taker27.cpp +++ b/src/ripple/legacy/0.27/book/impl/Taker27.cpp @@ -60,14 +60,14 @@ Taker::remaining_offer () const // We scale the output based on the remaining input: return Amounts (m_remain.in, divRound ( - m_remain.in, m_quality.rate (), m_remain.out, true)); + m_remain.in, m_quality.rate (), m_remain.out.issue (), true)); } assert (m_remain.out > zero); // We scale the input based on the remaining output: return Amounts (mulRound ( - m_remain.out, m_quality.rate (), m_remain.in, true), m_remain.out); + m_remain.out, m_quality.rate (), m_remain.in.issue (), true), m_remain.out); } /** Calculate the amount particular user could get through an offer. @@ -96,7 +96,7 @@ Taker::flow (Amounts amount, Offer const& offer, Account const& taker) { Amount const taker_charge (amountFromRate (taker_charge_rate)); amount = offer.quality ().ceil_in (amount, - divide (taker_funds, taker_charge)); + divide (taker_funds, taker_charge, taker_funds.issue ())); } // Best flow the owner can get. @@ -120,7 +120,7 @@ Taker::flow (Amounts amount, Offer const& offer, Account const& taker) { Amount const owner_charge (amountFromRate (owner_charge_rate)); owner_amount = offer.quality ().ceil_out (owner_amount, - divide (owner_funds, owner_charge)); + divide (owner_funds, owner_charge, owner_funds.issue ())); } // Calculate the amount that will flow through the offer diff --git a/src/ripple/protocol/STAmount.h b/src/ripple/protocol/STAmount.h index 2353a2b901..7670276799 100644 --- a/src/ripple/protocol/STAmount.h +++ b/src/ripple/protocol/STAmount.h @@ -165,14 +165,6 @@ public: return STAmount (mIssue); } - // When the currency is XRP, the value in raw unsigned units. - std::uint64_t - getNValue() const; - - // When the currency is XRP, the value in raw signed units. - std::int64_t - getSNValue() const; - // VFALCO TODO This can be a free function or just call the // member on the issue. std::string @@ -197,10 +189,7 @@ public: STAmount& operator+= (STAmount const&); STAmount& operator-= (STAmount const&); - STAmount& operator+= (std::uint64_t); - STAmount& operator-= (std::uint64_t); - STAmount& operator= (std::uint64_t); STAmount& operator= (beast::Zero) { clear(); @@ -359,97 +348,29 @@ STAmount operator- (STAmount const& value); STAmount divide (STAmount const& v1, STAmount const& v2, Issue const& issue); -inline -STAmount -divide (STAmount const& v1, STAmount const& v2, STAmount const& saUnit) -{ - return divide (v1, v2, saUnit.issue()); -} - -inline -STAmount -divide (STAmount const& v1, STAmount const& v2) -{ - return divide (v1, v2, v1); -} - STAmount multiply (STAmount const& v1, STAmount const& v2, Issue const& issue); -inline -STAmount -multiply (STAmount const& v1, STAmount const& v2, STAmount const& saUnit) -{ - return multiply (v1, v2, saUnit.issue()); -} - -inline -STAmount -multiply (STAmount const& v1, STAmount const& v2) -{ - return multiply (v1, v2, v1); -} - -void -canonicalizeRound (bool native, std::uint64_t& mantissa, - int& exponent, bool roundUp); - -/* addRound, subRound can end up rounding if the amount subtracted is too small - to make a change. Consder (X-d) where d is very small relative to X. - If you ask to round down, then (X-d) should not be X unless d is zero. - If you ask to round up, (X+d) should never be X unless d is zero. (Assuming X and d are positive). -*/ -// Add, subtract, multiply, or divide rounding result in specified direction -STAmount -addRound (STAmount const& v1, STAmount const& v2, bool roundUp); - -STAmount -subRound (STAmount const& v1, STAmount const& v2, bool roundUp); +// multiply, or divide rounding result in specified direction STAmount mulRound (STAmount const& v1, STAmount const& v2, Issue const& issue, bool roundUp); -inline -STAmount -mulRound (STAmount const& v1, STAmount const& v2, - STAmount const& saUnit, bool roundUp) -{ - return mulRound (v1, v2, saUnit.issue(), roundUp); -} - -inline -STAmount -mulRound (STAmount const& v1, STAmount const& v2, bool roundUp) -{ - return mulRound (v1, v2, v1.issue(), roundUp); -} - STAmount divRound (STAmount const& v1, STAmount const& v2, Issue const& issue, bool roundUp); -inline -STAmount -divRound (STAmount const& v1, STAmount const& v2, - STAmount const& saUnit, bool roundUp) -{ - return divRound (v1, v2, saUnit.issue(), roundUp); -} - -inline -STAmount -divRound (STAmount const& v1, STAmount const& v2, bool roundUp) -{ - return divRound (v1, v2, v1.issue(), roundUp); -} - // Someone is offering X for Y, what is the rate? // Rate: smaller is better, the taker wants the most out: in/out // VFALCO TODO Return a Quality object std::uint64_t getRate (STAmount const& offerOut, STAmount const& offerIn); +// When the currency is XRP, the value in raw unsigned units. +std::uint64_t +getNValue(STAmount const& amount); + //------------------------------------------------------------------------------ inline bool isXRP(STAmount const& amount) diff --git a/src/ripple/protocol/impl/STAmount.cpp b/src/ripple/protocol/impl/STAmount.cpp index 3dd91cc469..3d76a573c5 100644 --- a/src/ripple/protocol/impl/STAmount.cpp +++ b/src/ripple/protocol/impl/STAmount.cpp @@ -41,6 +41,34 @@ STAmount const saZero (noIssue(), 0u); STAmount const saOne (noIssue(), 1u); //------------------------------------------------------------------------------ +static +std::int64_t +getSNValue (STAmount const& amount) +{ + if (!amount.native ()) + throw std::runtime_error ("amount is not native!"); + + auto ret = static_cast(amount.mantissa ()); + + assert (static_cast(ret) == amount.mantissa ()); + + if (amount.negative ()) + ret = -ret; + + return ret; +} + +std::uint64_t +getNValue (STAmount const& amount) +{ + if (!amount.native ()) + throw std::runtime_error ("amount is not native!"); + + if (amount.negative ()) + throw std::runtime_error ("amount is negative!"); + + return amount.mantissa (); +} STAmount::STAmount(SerialIter& sit, SField const& name) : STBase(name) @@ -83,7 +111,7 @@ STAmount::STAmount(SerialIter& sit, SField const& name) throw std::runtime_error ("invalid native account"); // 10 bits for the offset, sign and "not native" flag - int offset = static_cast (value >> (64 - 10)); + int offset = static_cast(value >> (64 - 10)); value &= ~ (1023ull << (64 - 10)); @@ -223,8 +251,8 @@ STAmount STAmount::createFromInt64 (SField const& name, std::int64_t value) { return value >= 0 - ? STAmount (name, static_cast (value), false) - : STAmount (name, static_cast (-value), true); + ? STAmount (name, static_cast(value), false) + : STAmount (name, static_cast(-value), true); } //------------------------------------------------------------------------------ @@ -262,41 +290,6 @@ STAmount& STAmount::operator-= (STAmount const& a) return *this; } -STAmount& STAmount::operator+= (std::uint64_t v) -{ - assert (mIsNative); - if (!mIsNative) - throw std::runtime_error ("not native"); - // VFALCO TODO The cast looks dangerous, is it needed? - setSNValue (getSNValue () + static_cast (v)); - return *this; -} - -STAmount& STAmount::operator-= (std::uint64_t v) -{ - assert (mIsNative); - - if (!mIsNative) - throw std::runtime_error ("not native"); - - // VFALCO TODO The cast looks dangerous, is it needed? - setSNValue (getSNValue () - static_cast (v)); - return *this; -} - -STAmount& STAmount::operator= (std::uint64_t v) -{ - // Does not copy name, does not change currency type. - mOffset = 0; - mValue = v; - mIsNegative = false; - if (!mIsNative) - canonicalize (); - return *this; -} - - - STAmount operator+ (STAmount const& v1, STAmount const& v2) { v1.throwComparable (v2); @@ -312,11 +305,11 @@ STAmount operator+ (STAmount const& v1, STAmount const& v2) } if (v1.mIsNative) - return STAmount (v1.getFName (), v1.getSNValue () + v2.getSNValue ()); + return STAmount (v1.getFName (), getSNValue (v1) + getSNValue (v2)); int ov1 = v1.mOffset, ov2 = v2.mOffset; - std::int64_t vv1 = static_cast (v1.mValue); - std::int64_t vv2 = static_cast (v2.mValue); + std::int64_t vv1 = static_cast(v1.mValue); + std::int64_t vv2 = static_cast(v2.mValue); if (v1.mIsNegative) vv1 = -vv1; @@ -360,12 +353,12 @@ STAmount operator- (STAmount const& v1, STAmount const& v2) // XXX This could be better, check for overflow and that maximum range // is covered. return STAmount::createFromInt64 ( - v1.getFName (), v1.getSNValue () - v2.getSNValue ()); + v1.getFName (), getSNValue (v1) - getSNValue (v2)); } int ov1 = v1.mOffset, ov2 = v2.mOffset; - auto vv1 = static_cast (v1.mValue); - auto vv2 = static_cast (v2.mValue); + auto vv1 = static_cast(v1.mValue); + auto vv2 = static_cast(v2.mValue); if (v1.mIsNegative) vv1 = -vv1; @@ -488,27 +481,6 @@ STAmount::setIssue (Issue const& issue) mIsNative = isXRP (*this); } -std::uint64_t -STAmount::getNValue () const -{ - if (!mIsNative) - throw std::runtime_error ("not native"); - return mValue; -} - -std::int64_t -STAmount::getSNValue () const -{ - // signed native value - if (!mIsNative) - throw std::runtime_error ("not native"); - - if (mIsNegative) - return - static_cast (mValue); - - return static_cast (mValue); -} - std::string STAmount::getHumanCurrency () const { return to_string (mIssue.currency); @@ -522,12 +494,12 @@ STAmount::setSNValue (std::int64_t v) if (v > 0) { mIsNegative = false; - mValue = static_cast (v); + mValue = static_cast(v); } else { mIsNegative = true; - mValue = static_cast (-v); + mValue = static_cast(-v); } } @@ -591,14 +563,12 @@ void STAmount::roundSelf () if (valueDigits == 1) { mValue -= 1; - if (mValue < cMinValue) canonicalize (); } else if (valueDigits == 999999999ull) { mValue += 1; - if (mValue > cMaxValue) canonicalize (); } @@ -748,9 +718,9 @@ STAmount::add (Serializer& s) const if (*this == zero) s.add64 (cNotNative); else if (mIsNegative) // 512 = not native - s.add64 (mValue | (static_cast (mOffset + 512 + 97) << (64 - 10))); + s.add64 (mValue | (static_cast(mOffset + 512 + 97) << (64 - 10))); else // 256 = positive - s.add64 (mValue | (static_cast (mOffset + 512 + 256 + 97) << (64 - 10))); + s.add64 (mValue | (static_cast(mOffset + 512 + 256 + 97) << (64 - 10))); s.add160 (mIssue.currency); s.add160 (mIssue.account); @@ -847,12 +817,12 @@ void STAmount::set (std::int64_t v) if (v < 0) { mIsNegative = true; - mValue = static_cast (-v); + mValue = static_cast(-v); } else { mIsNegative = false; - mValue = static_cast (v); + mValue = static_cast(v); } } @@ -871,7 +841,7 @@ amountFromQuality (std::uint64_t rate) return STAmount (noIssue()); std::uint64_t mantissa = rate & ~ (255ull << (64 - 8)); - int exponent = static_cast (rate >> (64 - 8)) - 100; + int exponent = static_cast(rate >> (64 - 8)) - 100; return STAmount (noIssue(), mantissa, exponent); } @@ -1092,28 +1062,28 @@ bool operator< (STAmount const& lhs, std::uint64_t rhs) { // VFALCO Why the cast? - return lhs.getSNValue() < static_cast (rhs); + return getSNValue (lhs) < static_cast(rhs); } bool operator> (STAmount const& lhs, std::uint64_t rhs) { // VFALCO Why the cast? - return lhs.getSNValue() > static_cast (rhs); + return getSNValue (lhs) > static_cast(rhs); } bool operator<= (STAmount const& lhs, std::uint64_t rhs) { // VFALCO TODO The cast looks dangerous, is it needed? - return lhs.getSNValue () <= static_cast (rhs); + return getSNValue (lhs) <= static_cast(rhs); } bool operator>= (STAmount const& lhs, std::uint64_t rhs) { // VFALCO TODO The cast looks dangerous, is it needed? - return lhs.getSNValue() >= static_cast (rhs); + return getSNValue (lhs) >= static_cast(rhs); } STAmount @@ -1121,7 +1091,7 @@ operator+ (STAmount const& lhs, std::uint64_t rhs) { // VFALCO TODO The cast looks dangerous, is it needed? return STAmount (lhs.getFName (), - lhs.getSNValue () + static_cast (rhs)); + getSNValue (lhs) + static_cast(rhs)); } STAmount @@ -1129,7 +1099,7 @@ operator- (STAmount const& lhs, std::uint64_t rhs) { // VFALCO TODO The cast looks dangerous, is it needed? return STAmount (lhs.getFName (), - lhs.getSNValue () - static_cast (rhs)); + getSNValue (lhs) - static_cast(rhs)); } STAmount @@ -1208,10 +1178,10 @@ multiply (STAmount const& v1, STAmount const& v2, Issue const& issue) if (v1.native() && v2.native() && isXRP (issue)) { - std::uint64_t const minV = v1.getSNValue () < v2.getSNValue () - ? v1.getSNValue () : v2.getSNValue (); - std::uint64_t const maxV = v1.getSNValue () < v2.getSNValue () - ? v2.getSNValue () : v1.getSNValue (); + std::uint64_t const minV = getSNValue (v1) < getSNValue (v2) + ? getSNValue (v1) : getSNValue (v2); + std::uint64_t const maxV = getSNValue (v1) < getSNValue (v2) + ? getSNValue (v2) : getSNValue (v1); if (minV > 3000000000ull) // sqrt(cMaxNative) throw std::runtime_error ("Native value overflow"); @@ -1264,9 +1234,9 @@ multiply (STAmount const& v1, STAmount const& v2, Issue const& issue) offset1 + offset2 + 14, v1.negative() != v2.negative()); } +static void -canonicalizeRound (bool isNative, std::uint64_t& value, - int& offset, bool roundUp) +canonicalizeRound (bool native, std::uint64_t& value, int& offset, bool roundUp) { if (!roundUp) // canonicalize already rounds down return; @@ -1274,7 +1244,7 @@ canonicalizeRound (bool isNative, std::uint64_t& value, WriteLog (lsTRACE, STAmount) << "canonicalizeRound< " << value << ":" << offset; - if (isNative) + if (native) { if (offset < 0) { @@ -1309,153 +1279,6 @@ canonicalizeRound (bool isNative, std::uint64_t& value, << "canonicalizeRound> " << value << ":" << offset; } -STAmount -addRound (STAmount const& v1, STAmount const& v2, bool roundUp) -{ - v1.throwComparable (v2); - - if (v2.mantissa() == 0) - return v1; - - if (v1.mantissa() == 0) - return STAmount (v1.getFName (), v1.issue(), v2.mantissa(), - v2.exponent(), v2.negative()); - - if (v1.native()) - return STAmount (v1.getFName (), v1.getSNValue () + v2.getSNValue ()); - - int ov1 = v1.exponent(), ov2 = v2.exponent(); - auto vv1 = static_cast (v1.mantissa()); - auto vv2 = static_cast (v2.mantissa()); - - if (v1.negative()) - vv1 = -vv1; - - if (v2.negative()) - vv2 = -vv2; - - if (ov1 < ov2) - { - while (ov1 < (ov2 - 1)) - { - vv1 /= 10; - ++ov1; - } - - if (roundUp) - vv1 += 9; - - vv1 /= 10; - ++ov1; - } - - if (ov2 < ov1) - { - while (ov2 < (ov1 - 1)) - { - vv2 /= 10; - ++ov2; - } - - if (roundUp) - vv2 += 9; - - vv2 /= 10; - ++ov2; - } - - std::int64_t fv = vv1 + vv2; - - if ((fv >= -10) && (fv <= 10)) - return STAmount (v1.getFName (), v1.issue()); - else if (fv >= 0) - { - std::uint64_t v = static_cast (fv); - canonicalizeRound (false, v, ov1, roundUp); - return STAmount (v1.getFName (), v1.issue(), v, ov1, false); - } - else - { - std::uint64_t v = static_cast (-fv); - canonicalizeRound (false, v, ov1, !roundUp); - return STAmount (v1.getFName (), v1.issue(), v, ov1, true); - } -} - -STAmount -subRound (STAmount const& v1, STAmount const& v2, bool roundUp) -{ - v1.throwComparable (v2); - - if (v2.mantissa() == 0) - return v1; - - if (v1.mantissa() == 0) - return STAmount (v1.getFName (), v1.issue(), v2.mantissa(), - v2.exponent(), !v2.negative()); - - if (v1.native()) - return STAmount (v1.getFName (), v1.getSNValue () - v2.getSNValue ()); - - int ov1 = v1.exponent(), ov2 = v2.exponent(); - auto vv1 = static_cast (v1.mantissa()); - auto vv2 = static_cast (v2.mantissa()); - - if (v1.negative()) - vv1 = -vv1; - - if (!v2.negative()) - vv2 = -vv2; - - if (ov1 < ov2) - { - while (ov1 < (ov2 - 1)) - { - vv1 /= 10; - ++ov1; - } - - if (roundUp) - vv1 += 9; - - vv1 /= 10; - ++ov1; - } - - if (ov2 < ov1) - { - while (ov2 < (ov1 - 1)) - { - vv2 /= 10; - ++ov2; - } - - if (roundUp) - vv2 += 9; - - vv2 /= 10; - ++ov2; - } - - std::int64_t fv = vv1 + vv2; - - if ((fv >= -10) && (fv <= 10)) - return STAmount (v1.getFName (), v1.issue()); - - if (fv >= 0) - { - std::uint64_t v = static_cast (fv); - canonicalizeRound (false, v, ov1, roundUp); - return STAmount (v1.getFName (), v1.issue(), v, ov1, false); - } - else - { - std::uint64_t v = static_cast (-fv); - canonicalizeRound (false, v, ov1, !roundUp); - return STAmount (v1.getFName (), v1.issue(), v, ov1, true); - } -} - STAmount mulRound (STAmount const& v1, STAmount const& v2, Issue const& issue, bool roundUp) @@ -1465,10 +1288,10 @@ mulRound (STAmount const& v1, STAmount const& v2, if (v1.native() && v2.native() && isXRP (issue)) { - std::uint64_t minV = (v1.getSNValue () < v2.getSNValue ()) ? - v1.getSNValue () : v2.getSNValue (); - std::uint64_t maxV = (v1.getSNValue () < v2.getSNValue ()) ? - v2.getSNValue () : v1.getSNValue (); + std::uint64_t minV = (getSNValue (v1) < getSNValue (v2)) ? + getSNValue (v1) : getSNValue (v2); + std::uint64_t maxV = (getSNValue (v1) < getSNValue (v2)) ? + getSNValue (v2) : getSNValue (v1); if (minV > 3000000000ull) // sqrt(cMaxNative) throw std::runtime_error ("Native value overflow"); diff --git a/src/ripple/protocol/tests/STAmount.test.cpp b/src/ripple/protocol/tests/STAmount.test.cpp index 125e39b9f1..5de29bd979 100644 --- a/src/ripple/protocol/tests/STAmount.test.cpp +++ b/src/ripple/protocol/tests/STAmount.test.cpp @@ -96,20 +96,6 @@ public: { pass (); } - - aa = a; - prod1 = multiply (aa, bb, noIssue()); - - if (prod1 != prod2) - { - WriteLog (lsWARNING, STAmount) << "n(" << aa.getFullText () << " * " << bb.getFullText () << ") = " << prod1.getFullText () - << " not " << prod2.getFullText (); - fail ("Multiplication result is not exact"); - } - else - { - pass (); - } } //-------------------------------------------------------------------------- @@ -542,9 +528,7 @@ public: WriteLog (lsINFO, STAmount) << oneB; WriteLog (lsINFO, STAmount) << oneC; - STAmount fourThirdsA = addRound (twoThird2, twoThird2, false); STAmount fourThirdsB = twoThird2 + twoThird2; - STAmount fourThirdsC = addRound (twoThird2, twoThird2, true); WriteLog (lsINFO, STAmount) << fourThirdsA; WriteLog (lsINFO, STAmount) << fourThirdsB; WriteLog (lsINFO, STAmount) << fourThirdsC;