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

@@ -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));

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?!
+                saOwnerFundsLimit = divide (saOwnerFunds,
-                STAmount amount (noIssue(), uOfferRate, -9);
+                    amountFromRate (uOfferRate));
-                saOwnerFundsLimit = divide (saOwnerFunds, amount);
             }
             else
             {
@@ -3084,11 +3082,9 @@ void NetworkOPsImp::getBookPage (
 
             STAmount saOwnerPays = (uOfferRate == QUALITY_ONE)
                 ? saTakerGetsFunded
-                    : std::min (saOwnerFunds,
+                : std::min (
-                                multiply (
+                    saOwnerFunds,
-                                    saTakerGetsFunded, STAmount (
+                    multiply (saTakerGetsFunded, amountFromRate (uOfferRate)));
-                                        noIssue(), uOfferRate,
-                                        -9)));
 
             umBalance[uOfferOwnerID]    = saOwnerFunds - saOwnerPays;
 

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;

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);

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.
 

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 (
+        return STAmount (mEngine->getLedger ()->getReserve (
-            account->getFieldU32 (sfOwnerCount) + 1);
+            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 ||
+        if (saTakerPays.native () != !uPaysIssuerID ||
-            saTakerGets.isNative () != !uGetsIssuerID)
+            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 pays_xrp = txn.getFieldAmount (sfTakerPays).native ();
-    bool const gets_xrp = txn.getFieldAmount (sfTakerGets).isNative ();
+    bool const gets_xrp = txn.getFieldAmount (sfTakerGets).native ();
 
     if (pays_xrp && !gets_xrp)
         cross_type = CrossType::IouToXrp;

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
+        // A ticket counts against the reserve of the issuing account, but we
-        // the starting balance because we want to allow dipping into the reserve to
+        // check the starting balance because we want to allow dipping into the
-        // pay fees.
+        // reserve to pay fees.
-        auto const accountReserve (mEngine->getLedger ()->getReserve (
+        if (mPriorBalance < getAccountReserve (mTxnAccount))
-            mTxnAccount->getFieldU32 (sfOwnerCount) + 1));
-
-        if (mPriorBalance < accountReserve)
             return tecINSUFFICIENT_RESERVE;
 
         std::uint32_t expiration (0);

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
+            // uOwnerCount is the number of entries in this legder for this
-            // that require a reserve.
+            // account that require a reserve.
-
+            auto const uOwnerCount = mTxnAccount->getFieldU32 (sfOwnerCount);
-            std::uint32_t 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 =
-            std::uint64_t const uReserve (mEngine->getLedger ()->getReserve (uOwnerCount));
+                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.
+                // Vote no. However the transaction might succeed, if applied in
-                // However, transaction might succeed, if applied in a different order.
+                // 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
+                // The source account does have enough money, so do the
-                // for the transfer and make the ledger change.
+                // arithmetic for the transfer and make the ledger change.
-                mTxnAccount->setFieldAmount (sfBalance, mSourceBalance - saDstAmount);
+                mTxnAccount->setFieldAmount (sfBalance,
-                sleDst->setFieldAmount (sfBalance, sleDst->getFieldAmount (sfBalance) + saDstAmount);
+                    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))

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.";

src/ripple/app/tx/impl/Taker.cpp

@@ -360,10 +360,10 @@ BasicTaker::do_cross (
 {
     assert (!done ());
 
-    assert (!offer1.in.isNative ());
+    assert (!offer1.in.native ());
-    assert (offer1.out.isNative ());
+    assert (offer1.out.native ());
-    assert (offer2.in.isNative ());
+    assert (offer2.in.native ());
-    assert (!offer2.out.isNative ());
+    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.

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 () ?
+    if (mSigningPubKey.getAccountPublic ().empty ())
-        checkMultiSign () : checkSingleSign ();
+        return checkMultiSign ();
-#else
+#endif
-    TER const signingTER = checkSingleSign ();
-#endif // RIPPLE_ENABLE_MULTI_SIGN
 
-    return signingTER;
+    return checkSingleSign ();
 }
 
 TER Transactor::checkSingleSign ()

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);
+        return amountFromString (issue, amount);
-        expect (result.setValue (amount), amount);
-        return result;
     }
 
     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;
     }
 

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)

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);
+    // Do not call canonicalize
-
-    // Calls canonicalize
-    STAmount (SField const& name, Issue const& issue,
-        mantissa_type mantissa, exponent_type exponent,
-            bool native, bool negative);
-
-    // Does 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,
+    STAmount (Issue const& issue, std::uint64_t mantissa = 0, int exponent = 0,
-        std::uint64_t mantissa = 0, int exponent = 0, bool negative = false);
+        bool negative = false);
 
     // VFALCO Is this needed when we have the previous signature?
-    STAmount (Issue const& issue,
+    STAmount (Issue const& issue, std::uint32_t mantissa, int exponent = 0,
-        std::uint32_t mantissa, int exponent = 0, bool negative = false);
+        bool negative = false);
 
     STAmount (Issue const& issue, std::int64_t mantissa, int exponent = 0);
 
@@ -125,14 +128,10 @@ private:
     std::unique_ptr<STAmount>
     construct (SerialIter&, SField const& name);
 
-    void
+    void set (std::int64_t v);
-    setSNValue (std::int64_t);
+    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
+inline
-bool operator<  (STAmount const& lhs, std::uint64_t rhs);
+bool
-bool operator>  (STAmount const& lhs, std::uint64_t rhs);
+operator!= (STAmount const& lhs, STAmount const& rhs)
-bool operator<= (STAmount const& lhs, std::uint64_t rhs);
+{
-bool operator>= (STAmount const& lhs, std::uint64_t 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)

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 saZero (noIssue(), 0);
-STAmount const saOne (noIssue(), 1u);
+STAmount const saOne (noIssue(), 1);
 
 //------------------------------------------------------------------------------
 static
@@ -58,16 +58,12 @@ getSNValue (STAmount const& amount)
     return ret;
 }
 
-std::uint64_t
+static
-getNValue (STAmount const& amount)
+bool
+areComparable (STAmount const& v1, STAmount const& v2)
 {
-    if (!amount.native ())
+    return v1.native() == v2.native() &&
-        throw std::runtime_error ("amount is not native!");
+        v1.issue().currency == v2.issue().currency;
-
-    if (amount.negative ())
-        throw std::runtime_error ("amount is negative!");
-
-    return amount.mantissa ();
 }
 
 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<STAmount>(sit, name);
 }
 
-STAmount
-STAmount::createFromInt64 (SField const& name, std::int64_t value)
-{
-    return value >= 0
-           ? STAmount (name, static_cast<std::uint64_t>(value), false)
-           : STAmount (name, static_cast<std::uint64_t>(-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,
+        return { v1.getFName (), v1.issue (),
-                         v2.mValue, v2.mOffset, v2.mIsNegative);
+            v2.mantissa (), v2.exponent (), v2.negative () };
     }
 
-    if (v1.mIsNative)
+    if (v1.native ())
-        return STAmount (v1.getFName (), getSNValue (v1) + getSNValue (v2));
+        return { v1.getFName (), getSNValue (v1) + getSNValue (v2) };
 
-    int ov1 = v1.mOffset, ov2 = v2.mOffset;
+    int ov1 = v1.exponent (), ov2 = v2.exponent ();
-    std::int64_t vv1 = static_cast<std::int64_t>(v1.mValue);
+    std::int64_t vv1 = static_cast<std::int64_t>(v1.mantissa ());
-    std::int64_t vv2 = static_cast<std::int64_t>(v2.mValue);
+    std::int64_t vv2 = static_cast<std::int64_t>(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.issue (),
-    return STAmount (v1.getFName (), v1.mIssue, -fv, ov1, true);
+            static_cast<std::uint64_t>(fv), ov1, false };
+
+    return STAmount { v1.getFName (), v1.issue (),
+        static_cast<std::uint64_t>(-fv), ov1, true };
 }
 
 STAmount operator- (STAmount const& v1, STAmount const& v2)
 {
-    v1.throwComparable (v2);
+    return v1 + (-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<std::int64_t>(v1.mValue);
-    auto vv2 = static_cast<std::int64_t>(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);
 }
 
 //------------------------------------------------------------------------------
 
 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::uint64_t> (std::string (match[2]));
-            mOffset = 0;
-        }
-        else
-        {
-            // integer and fraction
-            mValue = beast::lexicalCastThrow <std::uint64_t> (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 <int> (std::string (match[7]));
-            else
-                mOffset += beast::lexicalCastThrow <int> (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<std::uint64_t>(v);
-    }
-    else
-    {
-        mIsNegative = true;
-        mValue = static_cast<std::uint64_t>(-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::uint64_t> (std::string (match[2]));
+        exponent = 0;
+    }
+    else
+    {
+        // integer and fraction
+        mantissa = beast::lexicalCastThrow <std::uint64_t> (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 <int> (std::string (match[7]));
+        else
+            exponent += beast::lexicalCastThrow <int> (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)
+        auto const ret = amountFromString (issue, value.asString ());
-        {
-            std::int64_t val = beast::lexicalCastThrow <std::int64_t> (
-                value.asString ());
 
-            if (val >= 0)
+        mantissa = ret.mantissa ();
-            {
+        exponent = ret.exponent ();
-                mantissa = val;
+        negative = ret.negative ();
-            }
-            else
-            {
-                mantissa = -val;
-                negative = true;
-            }
-        }
-        else
-        {
-            STAmount amount (name, issue, mantissa, exponent,
-                native, negative, STAmount::unchecked{});
-            amount.setValue (value.asString ());
-            return amount;
-        }
     }
     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() &&
+    return areComparable (lhs, rhs) &&
-        lhs.exponent() == rhs.exponent() && lhs.mantissa() == rhs.mantissa();
+        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);
 }
 
 bool
 operator< (STAmount const& lhs, STAmount const& rhs)
 {
-    lhs.throwComparable (rhs);
+    if (!areComparable (lhs, rhs))
-    return compare (lhs, rhs) < 0;
+        throw std::runtime_error ("Can't compare amounts that are't comparable!");
-}
 
-bool
+    if (lhs.negative() != rhs.negative())
-operator> (STAmount const& lhs, STAmount const& rhs)
+        return lhs.negative();
-{
-    lhs.throwComparable (rhs);
-    return compare (lhs, rhs) > 0;
-}
 
-bool
+    if (lhs.mantissa() == 0)
-operator<= (STAmount const& lhs, STAmount const& rhs)
+    {
-{
+        if (rhs.negative())
-    lhs.throwComparable (rhs);
+            return false;
-    return compare (lhs, rhs) <= 0;
+        return rhs.mantissa() != 0;
-}
+    }
 
-bool
+    // We know that lhs is non-zero and both sides have the same sign. Since
-operator>= (STAmount const& lhs, STAmount const& rhs)
+    // 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.
-    lhs.throwComparable (rhs);
+    if (rhs.mantissa() == 0) return false;
-    return compare (lhs, rhs) >= 0;
-}
 
-// 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
+    return false;
-operator< (STAmount const& lhs, std::uint64_t rhs)
-{
-    // VFALCO Why the cast?
-    return getSNValue (lhs) < static_cast<std::int64_t>(rhs);
-}
-
-bool
-operator> (STAmount const& lhs, std::uint64_t rhs)
-{
-    // VFALCO Why the cast?
-    return getSNValue (lhs) > static_cast<std::int64_t>(rhs);
-}
-
-bool
-operator<= (STAmount const& lhs, std::uint64_t rhs)
-{
-    // VFALCO TODO The cast looks dangerous, is it needed?
-    return getSNValue (lhs) <= static_cast<std::int64_t>(rhs);
-}
-
-bool
-operator>= (STAmount const& lhs, std::uint64_t rhs)
-{
-    // VFALCO TODO The cast looks dangerous, is it needed?
-    return getSNValue (lhs) >= static_cast<std::int64_t>(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<std::int64_t>(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<std::int64_t>(rhs));
 }
 
 STAmount

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,

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");
+        expect (! res.native (), "Product should not be native");
-
-        res.roundSelf ();
 
         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);
+            log <<
-
+                "(" << num.getText () << "/" << den.getText () <<
-            WriteLog (lsWARNING, STAmount) << "(" << num.getText () << "/" << den.getText () << ") X " << mul.getText () << " = "
+                ") X " << mul.getText () << " = " << res.getText () <<
-                                       << res.getText () << " not " << cmp.getText ();
+                " not " << cmp.getText ();
-
             fail ("Rounding");
-
+            return;
-            return false;
         }
-        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<std::uint64_t> (a) * static_cast<std::uint64_t> (b));
 
         if (prod1 != prod2)
         {
-            WriteLog (lsWARNING, STAmount) << "nn(" << aa.getFullText () << " * " << bb.getFullText () << ") = " << prod1.getFullText ()
+            log <<
-                                           << " not " << prod2.getFullText ();
+                "nn(" << aa.getFullText () << " * " << bb.getFullText () <<
-
+                ") = " << prod1.getFullText () << " not " << prod2.getFullText ();
             fail ("Multiplication result is not exact");
         }
-        else
-        {
-            pass ();
-        }
     }
 
     //--------------------------------------------------------------------------
@@ -103,15 +126,16 @@ public:
     void testSetValue (
         std::string const& value, Issue const& issue, bool success = true)
     {
-        STAmount amount (issue);
+        try
-
+        {
-        bool const result = amount.setValue (value);
+            STAmount const amount = amountFromString (issue, value);
-
-        expect (result == success, "parse " + value);
-
-        if (success)
             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 (!zeroSt.native (), "STAmount fail");
-        unexpected (!hundred.isNative (), "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 (zeroSt.native (), "STAmount fail");
-        unexpected (hundred.isNative (), "STAmount fail");
+        unexpected (hundred.native (), "STAmount fail");
         unexpected (zeroSt != zero, "STAmount fail");
         unexpected (one == zero, "STAmount fail");
         unexpected (hundred == zero, "STAmount fail");

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]

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.");