From 6f5d8bba2d3be19535e5e20e95003cfe2d07f67c Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Sun, 17 May 2015 10:29:47 -0700 Subject: [PATCH] Reduce STAmount public interface (RIPD-867): * Implement subtraction as addition to the additive inverse * Do not allow comparison with, addition to or subtraction from integers * Remove unused functions * Convert member functions to free functions * Isolate unit-test specific code into the unit test --- src/ripple/app/ledger/LedgerEntrySet.cpp | 15 +- src/ripple/app/misc/NetworkOPs.cpp | 20 +- src/ripple/app/paths/PathRequest.cpp | 2 +- src/ripple/app/paths/Pathfinder.cpp | 4 +- .../cursor/ForwardLiquidityForAccount.cpp | 2 +- src/ripple/app/tx/impl/CreateOffer.cpp | 20 +- src/ripple/app/tx/impl/CreateTicket.cpp | 19 +- src/ripple/app/tx/impl/Payment.cpp | 48 +- src/ripple/app/tx/impl/SetTrust.cpp | 10 +- src/ripple/app/tx/impl/Taker.cpp | 8 +- src/ripple/app/tx/impl/Transactor.cpp | 12 +- src/ripple/app/tx/tests/Taker.test.cpp | 10 +- src/ripple/app/tx/tests/common_transactor.cpp | 2 +- src/ripple/protocol/STAmount.h | 112 ++-- src/ripple/protocol/impl/STAmount.cpp | 496 ++++++------------ src/ripple/protocol/impl/STParsedJSON.cpp | 6 - src/ripple/protocol/tests/STAmount.test.cpp | 100 ++-- src/ripple/rpc/handlers/AccountLines.cpp | 2 +- src/ripple/rpc/impl/TransactionSign.cpp | 4 +- 19 files changed, 351 insertions(+), 541 deletions(-) diff --git a/src/ripple/app/ledger/LedgerEntrySet.cpp b/src/ripple/app/ledger/LedgerEntrySet.cpp index e3fc0fcdd..3bb148da9 100644 --- a/src/ripple/app/ledger/LedgerEntrySet.cpp +++ b/src/ripple/app/ledger/LedgerEntrySet.cpp @@ -1163,22 +1163,23 @@ STAmount LedgerEntrySet::accountHolds ( std::uint64_t uReserve = mLedger->getReserve ( sleAccount->getFieldU32 (sfOwnerCount)); - STAmount saBalance = sleAccount->getFieldAmount (sfBalance); + STAmount const saBalance = sleAccount->getFieldAmount (sfBalance); + STAmount const saReserve (uReserve); - if (saBalance < uReserve) + if (saBalance < saReserve) { saAmount.clear (); } else { - saAmount = saBalance - uReserve; + saAmount = saBalance - saReserve; } WriteLog (lsTRACE, LedgerEntrySet) << "accountHolds:" << " account=" << to_string (account) << " saAmount=" << saAmount.getFullText () << " saBalance=" << saBalance.getFullText () << - " uReserve=" << uReserve; + " saReserve=" << saReserve.getFullText (); return adjustedBalance(account, issuer, saAmount); } @@ -1248,7 +1249,7 @@ STAmount LedgerEntrySet::accountFunds ( { STAmount saFunds; - if (!saDefault.isNative () && saDefault.getIssuer () == account) + if (!saDefault.native () && saDefault.getIssuer () == account) { saFunds = saDefault; @@ -1684,7 +1685,7 @@ TER LedgerEntrySet::accountSend ( if (!saAmount || (uSenderID == uReceiverID)) return tesSUCCESS; - if (!saAmount.isNative ()) + if (!saAmount.native ()) { STAmount saActual; @@ -1972,7 +1973,7 @@ TER LedgerEntrySet::transfer_xrp ( assert (from != beast::zero); assert (to != beast::zero); assert (from != to); - assert (amount.isNative ()); + assert (amount.native ()); SLE::pointer sender = entryCache (ltACCOUNT_ROOT, getAccountRootIndex (from)); diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 3c4ec7b83..e05dd2c96 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -2888,9 +2888,8 @@ void NetworkOPsImp::getBookPage ( uOfferRate = uTransferRate; saOwnerFundsLimit = divide ( saOwnerFunds, - STAmount (noIssue(), uOfferRate, -9), + amountFromRate (uOfferRate), saOwnerFunds.issue ()); - // TODO(tom): why -9? } else { @@ -2922,7 +2921,7 @@ void NetworkOPsImp::getBookPage ( saOwnerFunds, multiply ( saTakerGetsFunded, - STAmount (noIssue(), uOfferRate, -9), + amountFromRate (uOfferRate), saTakerGetsFunded.issue ())); umBalance[uOfferOwnerID] = saOwnerFunds - saOwnerPays; @@ -3053,9 +3052,8 @@ void NetworkOPsImp::getBookPage ( { // Need to charge a transfer fee to offer owner. uOfferRate = uTransferRate; - // TODO(tom): where does -9 come from?! - STAmount amount (noIssue(), uOfferRate, -9); - saOwnerFundsLimit = divide (saOwnerFunds, amount); + saOwnerFundsLimit = divide (saOwnerFunds, + amountFromRate (uOfferRate)); } else { @@ -3083,12 +3081,10 @@ void NetworkOPsImp::getBookPage ( } STAmount saOwnerPays = (uOfferRate == QUALITY_ONE) - ? saTakerGetsFunded - : std::min (saOwnerFunds, - multiply ( - saTakerGetsFunded, STAmount ( - noIssue(), uOfferRate, - -9))); + ? saTakerGetsFunded + : std::min ( + saOwnerFunds, + multiply (saTakerGetsFunded, amountFromRate (uOfferRate))); umBalance[uOfferOwnerID] = saOwnerFunds - saOwnerPays; diff --git a/src/ripple/app/paths/PathRequest.cpp b/src/ripple/app/paths/PathRequest.cpp index cc0e994f6..d6e705465 100644 --- a/src/ripple/app/paths/PathRequest.cpp +++ b/src/ripple/app/paths/PathRequest.cpp @@ -171,7 +171,7 @@ bool PathRequest::isValid (RippleLineCache::ref crCache) // no destination account jvDestCur.append (Json::Value ("XRP")); - if (!saDstAmount.isNative ()) + if (!saDstAmount.native ()) { // only XRP can be send to a non-existent account bValid = false; diff --git a/src/ripple/app/paths/Pathfinder.cpp b/src/ripple/app/paths/Pathfinder.cpp index 112913f2c..c1fe0937a 100644 --- a/src/ripple/app/paths/Pathfinder.cpp +++ b/src/ripple/app/paths/Pathfinder.cpp @@ -297,7 +297,7 @@ bool Pathfinder::findPaths (int searchLevel) return false; } - auto reserve = mLedger->getReserve (0); + auto const reserve = STAmount (mLedger->getReserve (0)); if (mDstAmount < reserve) { WriteLog (lsDEBUG, Pathfinder) @@ -908,7 +908,7 @@ void Pathfinder::addLink ( // add accounts if (bOnXRP) { - if (mDstAmount.isNative () && !currentPath.empty ()) + if (mDstAmount.native () && !currentPath.empty ()) { // non-default path to XRP destination WriteLog (lsTRACE, Pathfinder) << "complete path found ax: " << currentPath.getJson(0); diff --git a/src/ripple/app/paths/cursor/ForwardLiquidityForAccount.cpp b/src/ripple/app/paths/cursor/ForwardLiquidityForAccount.cpp index b46b30e41..04e68f22d 100644 --- a/src/ripple/app/paths/cursor/ForwardLiquidityForAccount.cpp +++ b/src/ripple/app/paths/cursor/ForwardLiquidityForAccount.cpp @@ -156,7 +156,7 @@ TER PathCursor::forwardLiquidityForAccount () const ? previousNode().saFwdIssue // No fee. : mulRound ( previousNode().saFwdIssue, - STAmount (noIssue(), uQualityIn, -9), + amountFromRate (uQualityIn), previousNode().saFwdIssue.issue (), true); // Amount to credit. diff --git a/src/ripple/app/tx/impl/CreateOffer.cpp b/src/ripple/app/tx/impl/CreateOffer.cpp index e45b33829..d96768718 100644 --- a/src/ripple/app/tx/impl/CreateOffer.cpp +++ b/src/ripple/app/tx/impl/CreateOffer.cpp @@ -430,7 +430,7 @@ private: { std::string txt = amount.getText (); txt += "/"; - txt += amount.getHumanCurrency (); + txt += to_string (amount.issue().currency); return txt; } @@ -451,11 +451,11 @@ public: } /** Returns the reserve the account would have if an offer was added. */ - std::uint32_t + STAmount getAccountReserve (SLE::pointer account) { - return mEngine->getLedger ()->getReserve ( - account->getFieldU32 (sfOwnerCount) + 1); + return STAmount (mEngine->getLedger ()->getReserve ( + account->getFieldU32 (sfOwnerCount) + 1)); } TER @@ -504,7 +504,7 @@ public: if (!isLegalNet (saTakerPays) || !isLegalNet (saTakerGets)) return temBAD_AMOUNT; - if (saTakerPays.isNative () && saTakerGets.isNative ()) + if (saTakerPays.native () && saTakerGets.native ()) { if (m_journal.debug) m_journal.warning << "Malformed offer: XRP for XRP"; @@ -537,8 +537,8 @@ public: return temBAD_CURRENCY; } - if (saTakerPays.isNative () != !uPaysIssuerID || - saTakerGets.isNative () != !uGetsIssuerID) + if (saTakerPays.native () != !uPaysIssuerID || + saTakerGets.native () != !uGetsIssuerID) { if (m_journal.warning) m_journal.warning << "Malformed offer: bad issuer"; @@ -660,7 +660,7 @@ public: } // Make sure that we are authorized to hold what the taker will pay us. - if (result == tesSUCCESS && !saTakerPays.isNative ()) + if (result == tesSUCCESS && !saTakerPays.native ()) result = checkAcceptAsset (Issue (uPaysCurrency, uPaysIssuerID)); bool const bOpenLedger (mParams & tapOPEN_LEDGER); @@ -858,8 +858,8 @@ transact_CreateOffer ( { CrossType cross_type = CrossType::IouToIou; - bool const pays_xrp = txn.getFieldAmount (sfTakerPays).isNative (); - bool const gets_xrp = txn.getFieldAmount (sfTakerGets).isNative (); + bool const pays_xrp = txn.getFieldAmount (sfTakerPays).native (); + bool const gets_xrp = txn.getFieldAmount (sfTakerGets).native (); if (pays_xrp && !gets_xrp) cross_type = CrossType::IouToXrp; diff --git a/src/ripple/app/tx/impl/CreateTicket.cpp b/src/ripple/app/tx/impl/CreateTicket.cpp index 9a797ab98..c76ead5cf 100644 --- a/src/ripple/app/tx/impl/CreateTicket.cpp +++ b/src/ripple/app/tx/impl/CreateTicket.cpp @@ -57,17 +57,22 @@ public: return Transactor::preCheck (); } + /** Returns the reserve the account would have if an offer was added. */ + STAmount + getAccountReserve (SLE::pointer account) + { + return STAmount (mEngine->getLedger ()->getReserve ( + account->getFieldU32 (sfOwnerCount) + 1)); + } + TER doApply () override { assert (mTxnAccount); - // A ticket counts against the reserve of the issuing account, but we check - // the starting balance because we want to allow dipping into the reserve to - // pay fees. - auto const accountReserve (mEngine->getLedger ()->getReserve ( - mTxnAccount->getFieldU32 (sfOwnerCount) + 1)); - - if (mPriorBalance < accountReserve) + // A ticket counts against the reserve of the issuing account, but we + // check the starting balance because we want to allow dipping into the + // reserve to pay fees. + if (mPriorBalance < getAccountReserve (mTxnAccount)) return tecINSUFFICIENT_RESERVE; std::uint32_t expiration (0); diff --git a/src/ripple/app/tx/impl/Payment.cpp b/src/ripple/app/tx/impl/Payment.cpp index 5ad555189..bbeef9965 100644 --- a/src/ripple/app/tx/impl/Payment.cpp +++ b/src/ripple/app/tx/impl/Payment.cpp @@ -73,7 +73,7 @@ public: if (bMax) maxSourceAmount = mTxn.getFieldAmount (sfSendMax); - else if (saDstAmount.isNative ()) + else if (saDstAmount.native ()) maxSourceAmount = saDstAmount; else maxSourceAmount = STAmount ( @@ -178,7 +178,7 @@ public: STAmount maxSourceAmount; if (bMax) maxSourceAmount = mTxn.getFieldAmount (sfSendMax); - else if (saDstAmount.isNative ()) + else if (saDstAmount.native ()) maxSourceAmount = saDstAmount; else maxSourceAmount = STAmount ( @@ -197,7 +197,7 @@ public: if (!sleDst) { // Destination account does not exist. - if (!saDstAmount.isNative ()) + if (!saDstAmount.native ()) { m_journal.trace << "Delay transaction: Destination account does not exist."; @@ -218,7 +218,7 @@ public: // transaction would succeed. return telNO_DST_PARTIAL; } - else if (saDstAmount < mEngine->getLedger ()->getReserve (0)) + else if (saDstAmount < STAmount (mEngine->getLedger ()->getReserve (0))) { // getReserve() is the minimum amount that an account can have. // Reserve is not scaled by load. @@ -260,7 +260,7 @@ public: TER terResult; - bool const bRipple = bPaths || bMax || !saDstAmount.isNative (); + bool const bRipple = bPaths || bMax || !saDstAmount.native (); // XXX Should bMax be sufficient to imply ripple? if (bRipple) @@ -331,37 +331,39 @@ public: { // Direct XRP payment. - // uOwnerCount is the number of entries in this legder for this account - // that require a reserve. - - std::uint32_t const uOwnerCount (mTxnAccount->getFieldU32 (sfOwnerCount)); + // uOwnerCount is the number of entries in this legder for this + // account that require a reserve. + auto const uOwnerCount = mTxnAccount->getFieldU32 (sfOwnerCount); // This is the total reserve in drops. - // TODO(tom): there should be a class for this. - std::uint64_t const uReserve (mEngine->getLedger ()->getReserve (uOwnerCount)); + std::uint64_t const uReserve = + mEngine->getLedger ()->getReserve (uOwnerCount); + + // mPriorBalance is the balance on the sending account BEFORE the + // fees were charged. We want to make sure we have enough reserve + // to send. Allow final spend to use reserve for fee. + auto const mmm = std::max(mTxn.getTransactionFee (), + STAmount (uReserve)); - // mPriorBalance is the balance on the sending account BEFORE the fees were charged. - // - // Make sure have enough reserve to send. Allow final spend to use - // reserve for fee. - auto const mmm = std::max(uReserve, getNValue (mTxn.getTransactionFee ())); if (mPriorBalance < saDstAmount + mmm) { - // Vote no. - // However, transaction might succeed, if applied in a different order. + // Vote no. However the transaction might succeed, if applied in + // a different order. m_journal.trace << "Delay transaction: Insufficient funds: " << " " << mPriorBalance.getText () << - " / " << (saDstAmount + uReserve).getText () << + " / " << (saDstAmount + mmm).getText () << " (" << uReserve << ")"; terResult = tecUNFUNDED_PAYMENT; } else { - // The source account does have enough money, so do the arithmetic - // for the transfer and make the ledger change. - mTxnAccount->setFieldAmount (sfBalance, mSourceBalance - saDstAmount); - sleDst->setFieldAmount (sfBalance, sleDst->getFieldAmount (sfBalance) + saDstAmount); + // The source account does have enough money, so do the + // arithmetic for the transfer and make the ledger change. + mTxnAccount->setFieldAmount (sfBalance, + mSourceBalance - saDstAmount); + sleDst->setFieldAmount (sfBalance, + sleDst->getFieldAmount (sfBalance) + saDstAmount); // Re-arm the password change fee if we can and need to. if ((sleDst->getFlags () & lsfPasswordSpent)) diff --git a/src/ripple/app/tx/impl/SetTrust.cpp b/src/ripple/app/tx/impl/SetTrust.cpp index c158aeefb..7c688345d 100644 --- a/src/ripple/app/tx/impl/SetTrust.cpp +++ b/src/ripple/app/tx/impl/SetTrust.cpp @@ -58,7 +58,7 @@ public: if (!isLegalNet (saLimitAmount)) return temBAD_AMOUNT; - if (saLimitAmount.isNative ()) + if (saLimitAmount.native ()) { if (m_journal.trace) m_journal.trace << "Malformed transaction: specifies native limit " << @@ -121,9 +121,9 @@ public: // to fund accounts in a way where there's no incentive to trick them // into creating an account you have no intention of using. - std::uint64_t const uReserveCreate = (uOwnerCount < 2) + STAmount const reserveCreate ((uOwnerCount < 2) ? 0 - : mEngine->getLedger ()->getReserve (uOwnerCount + 1); + : mEngine->getLedger ()->getReserve (uOwnerCount + 1)); std::uint32_t uQualityIn (bQualityIn ? mTxn.getFieldU32 (sfQualityIn) : 0); std::uint32_t uQualityOut (bQualityOut ? mTxn.getFieldU32 (sfQualityOut) : 0); @@ -380,7 +380,7 @@ public: terResult = mEngine->view ().trustDelete (sleRippleState, uLowAccountID, uHighAccountID); } // Reserve is not scaled by load. - else if (bReserveIncrease && mPriorBalance < uReserveCreate) + else if (bReserveIncrease && mPriorBalance < reserveCreate) { 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 < uReserveCreate) // Reserve is not scaled by load. + else if (mPriorBalance < reserveCreate) // 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/impl/Taker.cpp b/src/ripple/app/tx/impl/Taker.cpp index 8236f085a..916ff57ea 100644 --- a/src/ripple/app/tx/impl/Taker.cpp +++ b/src/ripple/app/tx/impl/Taker.cpp @@ -360,10 +360,10 @@ BasicTaker::do_cross ( { assert (!done ()); - assert (!offer1.in.isNative ()); - assert (offer1.out.isNative ()); - assert (offer2.in.isNative ()); - assert (!offer2.out.isNative ()); + assert (!offer1.in.native ()); + assert (offer1.out.native ()); + assert (offer2.in.native ()); + assert (!offer2.out.native ()); // If the taker owns the first leg of the offer, then the taker's available // funds aren't the limiting factor for the input - the offer itself is. diff --git a/src/ripple/app/tx/impl/Transactor.cpp b/src/ripple/app/tx/impl/Transactor.cpp index 976d54155..289d05d09 100644 --- a/src/ripple/app/tx/impl/Transactor.cpp +++ b/src/ripple/app/tx/impl/Transactor.cpp @@ -126,7 +126,7 @@ TER Transactor::payFee () return telINSUF_FEE_P; } - if (saPaid < zero || !saPaid.isNative ()) + if (saPaid < zero || !saPaid.native ()) return temBAD_FEE; if (!saPaid) @@ -307,13 +307,11 @@ TER Transactor::checkSign () { #if RIPPLE_ENABLE_MULTI_SIGN // If the mSigningPubKey is empty, then we must be multi-signing. - TER const signingTER = mSigningPubKey.getAccountPublic ().empty () ? - checkMultiSign () : checkSingleSign (); -#else - TER const signingTER = checkSingleSign (); -#endif // RIPPLE_ENABLE_MULTI_SIGN + if (mSigningPubKey.getAccountPublic ().empty ()) + return checkMultiSign (); +#endif - return signingTER; + return checkSingleSign (); } TER Transactor::checkSingleSign () diff --git a/src/ripple/app/tx/tests/Taker.test.cpp b/src/ripple/app/tx/tests/Taker.test.cpp index 7834489f0..97ef30abe 100644 --- a/src/ripple/app/tx/tests/Taker.test.cpp +++ b/src/ripple/app/tx/tests/Taker.test.cpp @@ -128,9 +128,7 @@ private: STAmount parse_amount (std::string const& amount, Issue const& issue) { - STAmount result (issue); - expect (result.setValue (amount), amount); - return result; + return amountFromString (issue, amount); } Amounts parse_amounts ( @@ -162,7 +160,7 @@ private: { std::string txt = amount.getText (); txt += "/"; - txt += amount.getHumanCurrency (); + txt += to_string (amount.issue().currency); return txt; } @@ -212,7 +210,7 @@ private: Amounts const expected (parse_amounts ( flow.in, issue_in, flow.out, issue_out)); - + expect (expected == result, name + (sell ? " (s)" : " (b)")); if (expected != result) @@ -233,7 +231,7 @@ private: public: // Notation for clamp scenario descriptions: - // + // // IN:OUT (with the last in the list being limiting factor) // N = Nothing // T = Taker Offer Balance diff --git a/src/ripple/app/tx/tests/common_transactor.cpp b/src/ripple/app/tx/tests/common_transactor.cpp index 9939f2a56..d5942d306 100644 --- a/src/ripple/app/tx/tests/common_transactor.cpp +++ b/src/ripple/app/tx/tests/common_transactor.cpp @@ -362,7 +362,7 @@ void payInDrops (TestLedger& ledger, std::uint64_t getNativeBalance(TestLedger& ledger, UserAccount& acct) { - return getNValue(ledger.getAccountState(acct)->getBalance()); + return ledger.getAccountState(acct)->getBalance().mantissa (); } std::uint32_t getOwnerCount(TestLedger& ledger, UserAccount& acct) diff --git a/src/ripple/protocol/STAmount.h b/src/ripple/protocol/STAmount.h index 767027679..c65f5ea95 100644 --- a/src/ripple/protocol/STAmount.h +++ b/src/ripple/protocol/STAmount.h @@ -70,21 +70,24 @@ public: static std::uint64_t const uRateOne; //-------------------------------------------------------------------------- + STAmount(SerialIter& sit, SField const& name); struct unchecked { }; - STAmount(SerialIter& sit, SField const& name); - - // Calls canonicalize - STAmount (SField const& name, Issue const& issue, - mantissa_type mantissa, exponent_type exponent, - bool native, bool negative); - - // Does not call canonicalize + // Do not call canonicalize STAmount (SField const& name, Issue const& issue, mantissa_type mantissa, exponent_type exponent, bool native, bool negative, unchecked); + STAmount (Issue const& issue, + mantissa_type mantissa, exponent_type exponent, + bool native, bool negative, unchecked); + + // Call canonicalize + STAmount (SField const& name, Issue const& issue, + mantissa_type mantissa, exponent_type exponent, + bool native, bool negative); + STAmount (SField const& name, std::int64_t mantissa); STAmount (SField const& name, @@ -95,12 +98,12 @@ public: STAmount (std::uint64_t mantissa = 0, bool negative = false); - STAmount (Issue const& issue, - std::uint64_t mantissa = 0, int exponent = 0, bool negative = false); + STAmount (Issue const& issue, std::uint64_t mantissa = 0, int exponent = 0, + bool negative = false); // VFALCO Is this needed when we have the previous signature? - STAmount (Issue const& issue, - std::uint32_t mantissa, int exponent = 0, bool negative = false); + STAmount (Issue const& issue, std::uint32_t mantissa, int exponent = 0, + bool negative = false); STAmount (Issue const& issue, std::int64_t mantissa, int exponent = 0); @@ -125,14 +128,10 @@ private: std::unique_ptr construct (SerialIter&, SField const& name); - void - setSNValue (std::int64_t); + void set (std::int64_t v); + void canonicalize(); public: - static - STAmount - createFromInt64 (SField const& n, std::int64_t v); - //-------------------------------------------------------------------------- // // Observers @@ -148,7 +147,6 @@ public: // These three are deprecated Currency const& getCurrency() const { return mIssue.currency; } Account const& getIssuer() const { return mIssue.account; } - bool isNative() const { return mIsNative; } int signum() const noexcept @@ -165,11 +163,6 @@ public: return STAmount (mIssue); } - // VFALCO TODO This can be a free function or just call the - // member on the issue. - std::string - getHumanCurrency() const; - void setJson (Json::Value&) const; @@ -184,9 +177,6 @@ public: return *this != zero; } - bool isComparable (STAmount const&) const; - void throwComparable (STAmount const&) const; - STAmount& operator+= (STAmount const&); STAmount& operator-= (STAmount const&); @@ -196,18 +186,12 @@ public: return *this; } - friend STAmount operator+ (STAmount const& v1, STAmount const& v2); - friend STAmount operator- (STAmount const& v1, STAmount const& v2); - //-------------------------------------------------------------------------- // // Modification // //-------------------------------------------------------------------------- - // VFALCO TODO Remove this, it is only called from the unit test - void roundSelf(); - void negate() { if (*this != zero) @@ -216,7 +200,8 @@ public: void clear() { - // VFALCO: Why -100? + // The -100 is used to allow 0 to sort less than a small positive values + // which have a negative exponent. mOffset = mIsNative ? 0 : -100; mValue = 0; mIsNegative = false; @@ -243,10 +228,6 @@ public: /** Set the Issue for this amount and update mIsNative. */ void setIssue (Issue const& issue); - // VFALCO TODO Rename to setValueOnly (it only sets mantissa and exponent) - // Make this private - bool setValue (std::string const& sAmount); - //-------------------------------------------------------------------------- // // STBase @@ -279,9 +260,6 @@ public: { return (mValue == 0) && mIsNative; } - - void canonicalize(); - void set (std::int64_t v); }; //------------------------------------------------------------------------------ @@ -290,15 +268,18 @@ public: // //------------------------------------------------------------------------------ +STAmount +amountFromRate (std::uint64_t uRate); + // VFALCO TODO The parameter type should be Quality not uint64_t STAmount amountFromQuality (std::uint64_t rate); STAmount -amountFromJson (SField const& name, Json::Value const& v); +amountFromString (Issue const& issue, std::string const& amount); STAmount -amountFromRate (std::uint64_t uRate); +amountFromJson (SField const& name, Json::Value const& v); bool amountFromJsonNoThrow (STAmount& result, Json::Value const& jvSource); @@ -323,20 +304,36 @@ isLegalNet (STAmount const& value) //------------------------------------------------------------------------------ bool operator== (STAmount const& lhs, STAmount const& rhs); -bool operator!= (STAmount const& lhs, STAmount const& rhs); bool operator< (STAmount const& lhs, STAmount const& rhs); -bool operator> (STAmount const& lhs, STAmount const& rhs); -bool operator<= (STAmount const& lhs, STAmount const& rhs); -bool operator>= (STAmount const& lhs, STAmount const& rhs); -// native currency only -bool operator< (STAmount const& lhs, std::uint64_t rhs); -bool operator> (STAmount const& lhs, std::uint64_t rhs); -bool operator<= (STAmount const& lhs, std::uint64_t rhs); -bool operator>= (STAmount const& lhs, std::uint64_t rhs); +inline +bool +operator!= (STAmount const& lhs, STAmount const& rhs) +{ + return !(lhs == rhs); +} + +inline +bool +operator> (STAmount const& lhs, STAmount const& rhs) +{ + return rhs < lhs; +} + +inline +bool +operator<= (STAmount const& lhs, STAmount const& rhs) +{ + return !(rhs < lhs); +} + +inline +bool +operator>= (STAmount const& lhs, STAmount const& rhs) +{ + return !(lhs < rhs); +} -STAmount operator+ (STAmount const& lhs, std::uint64_t rhs); -STAmount operator- (STAmount const& lhs, std::uint64_t rhs); STAmount operator- (STAmount const& value); //------------------------------------------------------------------------------ @@ -345,6 +342,9 @@ STAmount operator- (STAmount const& value); // //------------------------------------------------------------------------------ +STAmount operator+ (STAmount const& v1, STAmount const& v2); +STAmount operator- (STAmount const& v1, STAmount const& v2); + STAmount divide (STAmount const& v1, STAmount const& v2, Issue const& issue); @@ -367,10 +367,6 @@ divRound (STAmount const& v1, STAmount const& v2, 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 3d76a573c..4f32250e5 100644 --- a/src/ripple/protocol/impl/STAmount.cpp +++ b/src/ripple/protocol/impl/STAmount.cpp @@ -37,8 +37,8 @@ static const std::uint64_t tenTo14 = 100000000000000ull; static const std::uint64_t tenTo14m1 = tenTo14 - 1; static const std::uint64_t tenTo17 = tenTo14 * 1000; -STAmount const saZero (noIssue(), 0u); -STAmount const saOne (noIssue(), 1u); +STAmount const saZero (noIssue(), 0); +STAmount const saOne (noIssue(), 1); //------------------------------------------------------------------------------ static @@ -58,16 +58,12 @@ getSNValue (STAmount const& amount) return ret; } -std::uint64_t -getNValue (STAmount const& amount) +static +bool +areComparable (STAmount const& v1, STAmount const& v2) { - if (!amount.native ()) - throw std::runtime_error ("amount is not native!"); - - if (amount.negative ()) - throw std::runtime_error ("amount is negative!"); - - return amount.mantissa (); + return v1.native() == v2.native() && + v1.issue().currency == v2.issue().currency; } STAmount::STAmount(SerialIter& sit, SField const& name) @@ -146,6 +142,30 @@ STAmount::STAmount(SerialIter& sit, SField const& name) canonicalize(); } +STAmount::STAmount (SField const& name, Issue const& issue, + mantissa_type mantissa, exponent_type exponent, + bool native, bool negative, unchecked) + : STBase (name) + , mIssue (issue) + , mValue (mantissa) + , mOffset (exponent) + , mIsNative (native) + , mIsNegative (negative) +{ +} + +STAmount::STAmount (Issue const& issue, + mantissa_type mantissa, exponent_type exponent, + bool native, bool negative, unchecked) + : mIssue (issue) + , mValue (mantissa) + , mOffset (exponent) + , mIsNative (native) + , mIsNegative (negative) +{ +} + + STAmount::STAmount (SField const& name, Issue const& issue, mantissa_type mantissa, exponent_type exponent, bool native, bool negative) @@ -159,18 +179,6 @@ STAmount::STAmount (SField const& name, Issue const& issue, canonicalize(); } -STAmount::STAmount (SField const& name, Issue const& issue, - mantissa_type mantissa, exponent_type exponent, - bool native, bool negative, unchecked) - : STBase (name) - , mIssue (issue) - , mValue (mantissa) - , mOffset (exponent) - , mIsNative (native) - , mIsNegative (negative) -{ -} - STAmount::STAmount (SField const& name, std::int64_t mantissa) : STBase (name) , mOffset (0) @@ -247,37 +255,12 @@ STAmount::construct (SerialIter& sit, SField const& name) return std::make_unique(sit, name); } -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); -} - //------------------------------------------------------------------------------ // // Operators // //------------------------------------------------------------------------------ -bool STAmount::isComparable (STAmount const& t) const -{ - // are these two STAmount instances in the same currency - if (mIsNative) return t.mIsNative; - - if (t.mIsNative) return false; - - return mIssue.currency == t.mIssue.currency; -} - -void STAmount::throwComparable (STAmount const& t) const -{ - // throw an exception if these two STAmount instances are incomparable - if (!isComparable (t)) - throw std::runtime_error ("amounts are not comparable"); -} - STAmount& STAmount::operator+= (STAmount const& a) { *this = *this + a; @@ -292,7 +275,8 @@ STAmount& STAmount::operator-= (STAmount const& a) STAmount operator+ (STAmount const& v1, STAmount const& v2) { - v1.throwComparable (v2); + if (!areComparable (v1, v2)) + throw std::runtime_error ("Can't add amounts that are't comparable!"); if (v2 == zero) return v1; @@ -300,21 +284,21 @@ STAmount operator+ (STAmount const& v1, STAmount const& v2) if (v1 == zero) { // Result must be in terms of v1 currency and issuer. - return STAmount (v1.getFName (), v1.mIssue, - v2.mValue, v2.mOffset, v2.mIsNegative); + return { v1.getFName (), v1.issue (), + v2.mantissa (), v2.exponent (), v2.negative () }; } - if (v1.mIsNative) - return STAmount (v1.getFName (), getSNValue (v1) + getSNValue (v2)); + if (v1.native ()) + return { 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); + int ov1 = v1.exponent (), ov2 = v2.exponent (); + std::int64_t vv1 = static_cast(v1.mantissa ()); + std::int64_t vv2 = static_cast(v2.mantissa ()); - if (v1.mIsNegative) + if (v1.negative ()) vv1 = -vv1; - if (v2.mIsNegative) + if (v2.negative ()) vv2 = -vv2; while (ov1 < ov2) @@ -335,145 +319,25 @@ STAmount operator+ (STAmount const& v1, STAmount const& v2) std::int64_t fv = vv1 + vv2; if ((fv >= -10) && (fv <= 10)) - return STAmount (v1.getFName (), v1.mIssue); + return { v1.getFName (), v1.issue () }; + if (fv >= 0) - return STAmount (v1.getFName (), v1.mIssue, fv, ov1, false); - return STAmount (v1.getFName (), v1.mIssue, -fv, ov1, true); + return STAmount { v1.getFName (), v1.issue (), + static_cast(fv), ov1, false }; + + return STAmount { v1.getFName (), v1.issue (), + static_cast(-fv), ov1, true }; } STAmount operator- (STAmount const& v1, STAmount const& v2) { - v1.throwComparable (v2); - - if (v2 == zero) - return v1; - - if (v2.mIsNative) - { - // XXX This could be better, check for overflow and that maximum range - // is covered. - return STAmount::createFromInt64 ( - 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); - - if (v1.mIsNegative) - vv1 = -vv1; - - if (v2.mIsNegative) - vv2 = -vv2; - - while (ov1 < ov2) - { - vv1 /= 10; - ++ov1; - } - - while (ov2 < ov1) - { - vv2 /= 10; - ++ov2; - } - - // this subtraction cannot overflow an std::int64_t, it can overflow an STAmount and the constructor will throw - - std::int64_t fv = vv1 - vv2; - - if ((fv >= -10) && (fv <= 10)) - return STAmount (v1.getFName (), v1.mIssue); - if (fv >= 0) - return STAmount (v1.getFName (), v1.mIssue, fv, ov1, false); - return STAmount (v1.getFName (), v1.mIssue, -fv, ov1, true); + return v1 + (-v2); } //------------------------------------------------------------------------------ std::uint64_t const STAmount::uRateOne = getRate (STAmount (1), STAmount (1)); -// Note: mIsNative and mIssue.currency must be set already! -bool -STAmount::setValue (std::string const& amount) -{ - static boost::regex const reNumber ( - "^" // the beginning of the string - "([-+]?)" // (optional) + or - character - "(0|[1-9][0-9]*)" // a number (no leading zeroes, unless 0) - "(\\.([0-9]+))?" // (optional) period followed by any number - "([eE]([+-]?)([0-9]+))?" // (optional) E, optional + or -, any number - "$", - boost::regex_constants::optimize); - - boost::smatch match; - - if (!boost::regex_match (amount, match, reNumber)) - { - WriteLog (lsWARNING, STAmount) << - "Number not valid: \"" << amount << "\""; - return false; - } - - // Match fields: - // 0 = whole input - // 1 = sign - // 2 = integer portion - // 3 = whole fraction (with '.') - // 4 = fraction (without '.') - // 5 = whole exponent (with 'e') - // 6 = exponent sign - // 7 = exponent number - - try - { - // CHECKME: Why 32? Shouldn't this be 16? - if ((match[2].length () + match[4].length ()) > 32) - { - WriteLog (lsWARNING, STAmount) << "Overlong number: " << amount; - return false; - } - - mIsNegative = (match[1].matched && (match[1] == "-")); - - // Can't specify XRP using fractional representation - if (mIsNative && match[3].matched) - return false; - - if (!match[4].matched) // integer only - { - mValue = beast::lexicalCastThrow (std::string (match[2])); - mOffset = 0; - } - else - { - // integer and fraction - mValue = beast::lexicalCastThrow (match[2] + match[4]); - mOffset = -(match[4].length ()); - } - - if (match[5].matched) - { - // we have an exponent - if (match[6].matched && (match[6] == "-")) - mOffset -= beast::lexicalCastThrow (std::string (match[7])); - else - mOffset += beast::lexicalCastThrow (std::string (match[7])); - } - - canonicalize (); - - WriteLog (lsTRACE, STAmount) << - "Canonicalized \"" << amount << "\" to " << mValue << " : " << mOffset; - } - catch (...) - { - return false; - } - - return true; -} - void STAmount::setIssue (Issue const& issue) { @@ -481,28 +345,6 @@ STAmount::setIssue (Issue const& issue) mIsNative = isXRP (*this); } -std::string STAmount::getHumanCurrency () const -{ - return to_string (mIssue.currency); -} - -void -STAmount::setSNValue (std::int64_t v) -{ - if (!mIsNative) throw std::runtime_error ("not native"); - - if (v > 0) - { - mIsNegative = false; - mValue = static_cast(v); - } - else - { - mIsNegative = true; - mValue = static_cast(-v); - } -} - // Convert an offer into an index amount so they sort by rate. // A taker will take the best, lowest, rate first. // (e.g. a taker will prefer pay 1 get 3 over pay 1 get 2. @@ -543,7 +385,7 @@ void STAmount::setJson (Json::Value& elem) const // It is an error for currency or issuer not to be specified for valid // json. elem[jss::value] = getText (); - elem[jss::currency] = getHumanCurrency (); + elem[jss::currency] = to_string (mIssue.currency); elem[jss::issuer] = to_string (mIssue.account); } else @@ -552,28 +394,6 @@ void STAmount::setJson (Json::Value& elem) const } } -// VFALCO What does this perplexing function do? -void STAmount::roundSelf () -{ - if (mIsNative) - return; - - std::uint64_t valueDigits = mValue % 1000000000ull; - - if (valueDigits == 1) - { - mValue -= 1; - if (mValue < cMinValue) - canonicalize (); - } - else if (valueDigits == 999999999ull) - { - mValue += 1; - if (mValue > cMaxValue) - canonicalize (); - } -} - //------------------------------------------------------------------------------ // // STBase @@ -586,7 +406,7 @@ STAmount::getFullText () const std::string ret; ret.reserve(64); - ret = getText () + "/" + getHumanCurrency (); + ret = getText () + "/" + to_string (mIssue.currency); if (!mIsNative) { @@ -831,7 +651,7 @@ void STAmount::set (std::int64_t v) STAmount amountFromRate (std::uint64_t uRate) { - return STAmount (noIssue(), uRate, -9, false); + return { noIssue(), uRate, -9, false }; } STAmount @@ -846,6 +666,70 @@ amountFromQuality (std::uint64_t rate) return STAmount (noIssue(), mantissa, exponent); } +STAmount +amountFromString (Issue const& issue, std::string const& amount) +{ + static boost::regex const reNumber ( + "^" // the beginning of the string + "([-+]?)" // (optional) + or - character + "(0|[1-9][0-9]*)" // a number (no leading zeroes, unless 0) + "(\\.([0-9]+))?" // (optional) period followed by any number + "([eE]([+-]?)([0-9]+))?" // (optional) E, optional + or -, any number + "$", + boost::regex_constants::optimize); + + boost::smatch match; + + if (!boost::regex_match (amount, match, reNumber)) + throw std::runtime_error ("Number '" + amount + "' is not valid"); + + // Match fields: + // 0 = whole input + // 1 = sign + // 2 = integer portion + // 3 = whole fraction (with '.') + // 4 = fraction (without '.') + // 5 = whole exponent (with 'e') + // 6 = exponent sign + // 7 = exponent number + + // CHECKME: Why 32? Shouldn't this be 16? + if ((match[2].length () + match[4].length ()) > 32) + throw std::runtime_error ("Number '" + amount + "' is overlong"); + + bool negative = (match[1].matched && (match[1] == "-")); + + // Can't specify XRP using fractional representation + if (isXRP(issue) && match[3].matched) + throw std::runtime_error ("XRP must be specified in integral drops."); + + std::uint64_t mantissa; + int exponent; + + if (!match[4].matched) // integer only + { + mantissa = beast::lexicalCastThrow (std::string (match[2])); + exponent = 0; + } + else + { + // integer and fraction + mantissa = beast::lexicalCastThrow (match[2] + match[4]); + exponent = -(match[4].length ()); + } + + if (match[5].matched) + { + // we have an exponent + if (match[6].matched && (match[6] == "-")) + exponent -= beast::lexicalCastThrow (std::string (match[7])); + else + exponent += beast::lexicalCastThrow (std::string (match[7])); + } + + return { issue, mantissa, exponent, negative }; +} + STAmount amountFromJson (SField const& name, Json::Value const& v) { @@ -906,6 +790,7 @@ amountFromJson (SField const& name, Json::Value const& v) { if (v.isObject ()) throw std::runtime_error ("XRP may not be specified as an object"); + issue = xrpIssue (); } else { @@ -939,35 +824,18 @@ amountFromJson (SField const& name, Json::Value const& v) } else if (value.isString ()) { - if (native) - { - std::int64_t val = beast::lexicalCastThrow ( - value.asString ()); + auto const ret = amountFromString (issue, value.asString ()); - if (val >= 0) - { - mantissa = val; - } - else - { - mantissa = -val; - negative = true; - } - } - else - { - STAmount amount (name, issue, mantissa, exponent, - native, negative, STAmount::unchecked{}); - amount.setValue (value.asString ()); - return amount; - } + mantissa = ret.mantissa (); + exponent = ret.exponent (); + negative = ret.negative (); } else { throw std::runtime_error ("invalid amount type"); } - return STAmount (name, issue, mantissa, exponent, native, negative); + return { name, issue, mantissa, exponent, native, negative }; } bool @@ -992,114 +860,42 @@ amountFromJsonNoThrow (STAmount& result, Json::Value const& jvSource) // //------------------------------------------------------------------------------ -static -int -compare (STAmount const& lhs, STAmount const& rhs) -{ - // Compares the value of a to the value of this STAmount, amounts must be comparable - if (lhs.negative() != rhs.negative()) - return lhs.negative() ? -1 : 1; - if (lhs.mantissa() == 0) - { - if (rhs.negative()) - return 1; - return (rhs.mantissa() != 0) ? -1 : 0; - } - if (rhs.mantissa() == 0) return 1; - if (lhs.exponent() > rhs.exponent()) return lhs.negative() ? -1 : 1; - if (lhs.exponent() < rhs.exponent()) return lhs.negative() ? 1 : -1; - if (lhs.mantissa() > rhs.mantissa()) return lhs.negative() ? -1 : 1; - if (lhs.mantissa() < rhs.mantissa()) return lhs.negative() ? 1 : -1; - return 0; -} - bool operator== (STAmount const& lhs, STAmount const& rhs) { - return lhs.isComparable (rhs) && lhs.negative() == rhs.negative() && - lhs.exponent() == rhs.exponent() && lhs.mantissa() == rhs.mantissa(); -} - -bool -operator!= (STAmount const& lhs, STAmount const& rhs) -{ - return lhs.exponent() != rhs.exponent() || - lhs.mantissa() != rhs.mantissa() || - lhs.negative() != rhs.negative() || ! lhs.isComparable (rhs); + return areComparable (lhs, rhs) && + lhs.negative() == rhs.negative() && + lhs.exponent() == rhs.exponent() && + lhs.mantissa() == rhs.mantissa(); } bool operator< (STAmount const& lhs, STAmount const& rhs) { - lhs.throwComparable (rhs); - return compare (lhs, rhs) < 0; -} + if (!areComparable (lhs, rhs)) + throw std::runtime_error ("Can't compare amounts that are't comparable!"); -bool -operator> (STAmount const& lhs, STAmount const& rhs) -{ - lhs.throwComparable (rhs); - return compare (lhs, rhs) > 0; -} + if (lhs.negative() != rhs.negative()) + return lhs.negative(); -bool -operator<= (STAmount const& lhs, STAmount const& rhs) -{ - lhs.throwComparable (rhs); - return compare (lhs, rhs) <= 0; -} + if (lhs.mantissa() == 0) + { + if (rhs.negative()) + return false; + return rhs.mantissa() != 0; + } -bool -operator>= (STAmount const& lhs, STAmount const& rhs) -{ - lhs.throwComparable (rhs); - return compare (lhs, rhs) >= 0; -} + // We know that lhs is non-zero and both sides have the same sign. Since + // rhs is zero (and thus not negative), lhs must, therefore, be strictly + // greater than zero. So if rhs is zero, the comparison must be false. + if (rhs.mantissa() == 0) return false; -// native currency only + if (lhs.exponent() > rhs.exponent()) return lhs.negative(); + if (lhs.exponent() < rhs.exponent()) return ! lhs.negative(); + if (lhs.mantissa() > rhs.mantissa()) return lhs.negative(); + if (lhs.mantissa() < rhs.mantissa()) return ! lhs.negative(); -bool -operator< (STAmount const& lhs, std::uint64_t rhs) -{ - // VFALCO Why the cast? - return getSNValue (lhs) < static_cast(rhs); -} - -bool -operator> (STAmount const& lhs, std::uint64_t rhs) -{ - // VFALCO Why the cast? - 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 getSNValue (lhs) <= static_cast(rhs); -} - -bool -operator>= (STAmount const& lhs, std::uint64_t rhs) -{ - // VFALCO TODO The cast looks dangerous, is it needed? - return getSNValue (lhs) >= static_cast(rhs); -} - -STAmount -operator+ (STAmount const& lhs, std::uint64_t rhs) -{ - // VFALCO TODO The cast looks dangerous, is it needed? - return STAmount (lhs.getFName (), - getSNValue (lhs) + static_cast(rhs)); -} - -STAmount -operator- (STAmount const& lhs, std::uint64_t rhs) -{ - // VFALCO TODO The cast looks dangerous, is it needed? - return STAmount (lhs.getFName (), - getSNValue (lhs) - static_cast(rhs)); + return false; } STAmount diff --git a/src/ripple/protocol/impl/STParsedJSON.cpp b/src/ripple/protocol/impl/STParsedJSON.cpp index 8224b2b55..a94b75497 100644 --- a/src/ripple/protocol/impl/STParsedJSON.cpp +++ b/src/ripple/protocol/impl/STParsedJSON.cpp @@ -138,12 +138,6 @@ static Json::Value singleton_expected (std::string const& object, "]' must be an object with a single key/object value."); } -static Json::Value serialization_error (SField const& sField) -{ - return RPC::make_error (rpcINVALID_PARAMS, - "Object '" + sField.getName () + "' failed to serialize."); -} - static Json::Value template_mismatch (SField const& sField) { return RPC::make_error (rpcINVALID_PARAMS, diff --git a/src/ripple/protocol/tests/STAmount.test.cpp b/src/ripple/protocol/tests/STAmount.test.cpp index 5de29bd97..2bef5007e 100644 --- a/src/ripple/protocol/tests/STAmount.test.cpp +++ b/src/ripple/protocol/tests/STAmount.test.cpp @@ -38,41 +38,68 @@ public: } //-------------------------------------------------------------------------- + STAmount roundSelf (STAmount const& amount) + { + if (amount.native ()) + return amount; - bool roundTest (int n, int d, int m) + std::uint64_t mantissa = amount.mantissa (); + std::uint64_t valueDigits = mantissa % 1000000000; + + if (valueDigits == 1) + { + mantissa--; + + if (mantissa < STAmount::cMinValue) + return { amount.issue (), mantissa, amount.exponent (), + amount.negative () }; + + return { amount.issue (), mantissa, amount.exponent (), + amount.native(), amount.negative (), STAmount::unchecked {} }; + } + + if (valueDigits == 999999999) + { + mantissa++; + + if (mantissa > STAmount::cMaxValue) + return { amount.issue (), mantissa, amount.exponent (), + amount.negative () }; + + return { amount.issue (), mantissa, amount.exponent (), + amount.native(), amount.negative (), STAmount::unchecked {} }; + } + + return amount; + } + + void roundTest (int n, int d, int m) { // check STAmount rounding STAmount num (noIssue(), n); STAmount den (noIssue(), d); STAmount mul (noIssue(), m); STAmount quot = divide (n, d, noIssue()); - STAmount res = multiply (quot, mul, noIssue()); + STAmount res = roundSelf (multiply (quot, mul, noIssue())); - expect (! res.isNative (), "Product should not be native"); - - res.roundSelf (); + expect (! res.native (), "Product should not be native"); STAmount cmp (noIssue(), (n * m) / d); - expect (! cmp.isNative (), "Comparison amount should not be native"); + expect (! cmp.native (), "Comparison amount should not be native"); + + expect (cmp.issue().currency == res.issue().currency, + "Product and result should be comparable"); if (res != cmp) { - cmp.throwComparable (res); - - WriteLog (lsWARNING, STAmount) << "(" << num.getText () << "/" << den.getText () << ") X " << mul.getText () << " = " - << res.getText () << " not " << cmp.getText (); - + log << + "(" << num.getText () << "/" << den.getText () << + ") X " << mul.getText () << " = " << res.getText () << + " not " << cmp.getText (); fail ("Rounding"); - - return false; + return; } - else - { - pass (); - } - - return true; } void mulTest (int a, int b) @@ -81,21 +108,17 @@ public: STAmount bb (noIssue(), b); STAmount prod1 (multiply (aa, bb, noIssue())); - expect (! prod1.isNative ()); + expect (! prod1.native ()); STAmount prod2 (noIssue(), static_cast (a) * static_cast (b)); if (prod1 != prod2) { - WriteLog (lsWARNING, STAmount) << "nn(" << aa.getFullText () << " * " << bb.getFullText () << ") = " << prod1.getFullText () - << " not " << prod2.getFullText (); - + log << + "nn(" << aa.getFullText () << " * " << bb.getFullText () << + ") = " << prod1.getFullText () << " not " << prod2.getFullText (); fail ("Multiplication result is not exact"); } - else - { - pass (); - } } //-------------------------------------------------------------------------- @@ -103,14 +126,15 @@ public: void testSetValue ( std::string const& value, Issue const& issue, bool success = true) { - STAmount amount (issue); - - bool const result = amount.setValue (value); - - expect (result == success, "parse " + value); - - if (success) + try + { + STAmount const amount = amountFromString (issue, value); expect (amount.getText () == value, "format " + value); + } + catch (std::exception const& e) + { + expect (!success, "parse " + value + " should fail"); + } } void testSetValue () @@ -187,8 +211,8 @@ public: unexpected (serializeAndDeserialize (zeroSt) != zeroSt, "STAmount fail"); unexpected (serializeAndDeserialize (one) != one, "STAmount fail"); unexpected (serializeAndDeserialize (hundred) != hundred, "STAmount fail"); - unexpected (!zeroSt.isNative (), "STAmount fail"); - unexpected (!hundred.isNative (), "STAmount fail"); + unexpected (!zeroSt.native (), "STAmount fail"); + unexpected (!hundred.native (), "STAmount fail"); unexpected (zeroSt != zero, "STAmount fail"); unexpected (one == zero, "STAmount fail"); unexpected (hundred == zero, "STAmount fail"); @@ -269,8 +293,8 @@ public: unexpected (serializeAndDeserialize (zeroSt) != zeroSt, "STAmount fail"); unexpected (serializeAndDeserialize (one) != one, "STAmount fail"); unexpected (serializeAndDeserialize (hundred) != hundred, "STAmount fail"); - unexpected (zeroSt.isNative (), "STAmount fail"); - unexpected (hundred.isNative (), "STAmount fail"); + unexpected (zeroSt.native (), "STAmount fail"); + unexpected (hundred.native (), "STAmount fail"); unexpected (zeroSt != zero, "STAmount fail"); unexpected (one == zero, "STAmount fail"); unexpected (hundred == zero, "STAmount fail"); diff --git a/src/ripple/rpc/handlers/AccountLines.cpp b/src/ripple/rpc/handlers/AccountLines.cpp index d9840026a..9be10e3ab 100644 --- a/src/ripple/rpc/handlers/AccountLines.cpp +++ b/src/ripple/rpc/handlers/AccountLines.cpp @@ -45,7 +45,7 @@ void addLine (Json::Value& jsonLines, RippleState const& line) // Amount reported is negative if other account holds current // account's IOUs. jPeer[jss::balance] = saBalance.getText (); - jPeer[jss::currency] = saBalance.getHumanCurrency (); + jPeer[jss::currency] = to_string (saBalance.issue ().currency); jPeer[jss::limit] = saLimit.getText (); jPeer[jss::limit_peer] = saLimitPeer.getText (); jPeer[jss::quality_in] diff --git a/src/ripple/rpc/impl/TransactionSign.cpp b/src/ripple/rpc/impl/TransactionSign.cpp index 7408c56b3..cc10730fc 100644 --- a/src/ripple/rpc/impl/TransactionSign.cpp +++ b/src/ripple/rpc/impl/TransactionSign.cpp @@ -400,7 +400,7 @@ static Json::Value checkPayment( saSendMax.setIssuer (raSrcAddressID.getAccountID ()); } - if (saSendMax.isNative () && amount.isNative ()) + if (saSendMax.native () && amount.native ()) return RPC::make_error (rpcINVALID_PARAMS, "Cannot build XRP to XRP paths."); @@ -486,7 +486,7 @@ checkTxJsonFields ( // Check for current ledger. if (verify && !getConfig ().RUN_STANDALONE && - (apiFacade.getValidatedLedgerAge() > + (apiFacade.getValidatedLedgerAge() > Tuning::maxValidatedLedgerAge)) { ret.first = rpcError (rpcNO_CURRENT);