Better numerical stability for deferred credits:

Before this change, the deferred credits algorithm took the current balance and subtracted the recorded credits. Conceptually, this is the same as taking the original balance, adding all the credits, subtracting all the debits, and subtracting all the credits. The new algorithm records the original balance and subtracts the debits. This prevents errors that occur when the original balance and the recorded credits have large differences in magnitude. Additionally, XRP credits were recorded incorrectly in the deferred credits table (the line was between the sender and receiver, rather than the root account).
2025-12-06 17:27:52 +00:00 · 2016-03-31 11:06:13 -04:00
parent 4124850481
commit 4b8d227922
16 changed files with 349 additions and 139 deletions
--- a/src/ripple/app/paths/Flow.cpp
+++ b/src/ripple/app/paths/Flow.cpp
@@ -66,9 +66,13 @@ flow (
    boost::optional<STAmount> const& sendMax,
    beast::Journal j)
 {
-
-    Issue const srcIssue =
-        sendMax ? sendMax->issue () : Issue (deliver.issue ().currency, src);
+    Issue const srcIssue = [&] {
+        if (sendMax)
+            return sendMax->issue ();
+        if (!isXRP (deliver.issue ().currency))
+            return Issue (deliver.issue ().currency, src);
+        return xrpIssue ();
+    }();

    Issue const dstIssue = deliver.issue ();

--- a/src/ripple/app/paths/impl/BookStep.cpp
+++ b/src/ripple/app/paths/impl/BookStep.cpp
@@ -102,6 +102,12 @@ public:
        return EitherAmount (cache_->out);
    }

+    boost::optional<Book>
+    bookStepBook () const override
+    {
+        return book_;
+    }
+
    std::pair<TIn, TOut>
    revImp (
        PaymentSandbox& sb,
@@ -258,7 +264,7 @@ forEachOffer (
            break;
    }

-    return {offers.toRemove (), counter.count()};
+    return {offers.permToRemove (), counter.count()};
 }

 template <class TIn, class TOut>
@@ -546,11 +552,16 @@ BookStep<TIn, TOut>::check(StrandContext const& ctx) const
        JLOG (j_.debug()) << "Book: currency is inconsistent with issuer." << *this;
        return temBAD_PATH;
    }
-    if (!ctx.seenBooks.insert (book_).second)
+
+    // Do not allow two books to output the same issue. This may cause offers on
+    // one step to unfund offers in another step.
+    if (!ctx.seenBookOuts.insert (book_.out).second ||
+        ctx.seenDirectIssues[0].count (book_.out))
    {
        JLOG (j_.debug()) << "BookStep: loop detected: " << *this;
        return temBAD_PATH_LOOP;
    }
+
    return tesSUCCESS;
 }

--- a/src/ripple/app/paths/impl/DirectStep.cpp
+++ b/src/ripple/app/paths/impl/DirectStep.cpp
@@ -109,6 +109,12 @@ class DirectStepI : public StepImp<IOUAmount, IOUAmount, DirectStepI>
        return EitherAmount (cache_->out);
    }

+    boost::optional<AccountID>
+    directStepSrcAcct () const override
+    {
+        return src_;
+    }
+
    std::pair<IOUAmount, IOUAmount>
    revImp (
        PaymentSandbox& sb,
@@ -565,21 +571,43 @@ TER DirectStepI::check (StrandContext const& ctx) const

    if (ctx.prevStep)
    {
-        if (auto pds = dynamic_cast<DirectStepI const*> (ctx.prevStep))
+        if (auto prevSrc = ctx.prevStep->directStepSrcAcct ())
        {
-            auto const ter = checkNoRipple (
-                ctx.view, pds->src (), src_, dst_, currency_, j_);
+            auto const ter =
+                checkNoRipple (ctx.view, *prevSrc, src_, dst_, currency_, j_);
            if (ter != tesSUCCESS)
                return ter;
        }
    }

-    if (!ctx.seenDirectIssues[0].insert (Issue{currency_, src_}).second ||
-        !ctx.seenDirectIssues[1].insert (Issue{currency_, dst_}).second)
    {
-        JLOG (j_.debug()) << "DirectStepI: loop detected: Index: "
-                        << ctx.strandSize << ' ' << *this;
-        return temBAD_PATH_LOOP;
+        Issue const srcIssue{currency_, src_};
+        Issue const dstIssue{currency_, dst_};
+
+        if (ctx.seenBookOuts.count (srcIssue))
+        {
+            if (!ctx.prevStep)
+            {
+                assert(0); // prev seen book without a prev step!?!
+                return temBAD_PATH_LOOP;
+            }
+
+            // This is OK if the previous step is a book step that outputs this issue
+            if (auto book = ctx.prevStep->bookStepBook())
+            {
+                if (book->out != srcIssue)
+                    return temBAD_PATH_LOOP;
+            }
+        }
+
+        if (!ctx.seenDirectIssues[0].insert (srcIssue).second ||
+            !ctx.seenDirectIssues[1].insert (dstIssue).second)
+        {
+            JLOG (j_.debug ())
+                << "DirectStepI: loop detected: Index: " << ctx.strandSize
+                << ' ' << *this;
+            return temBAD_PATH_LOOP;
+        }
    }

    {
--- a/src/ripple/app/paths/impl/PaySteps.cpp
+++ b/src/ripple/app/paths/impl/PaySteps.cpp
@@ -226,14 +226,14 @@ toStrand (
    */
    std::array<boost::container::flat_set<Issue>, 2> seenDirectIssues;
    // A strand may not include the same offer book more than once
-    boost::container::flat_set<Book> seenBooks;
+    boost::container::flat_set<Issue> seenBookOuts;
    seenDirectIssues[0].reserve (pes.size());
    seenDirectIssues[1].reserve (pes.size());
-    seenBooks.reserve (pes.size());
+    seenBookOuts.reserve (pes.size());
    auto ctx = [&](bool isLast = false)
    {
        return StrandContext{view, result, strandSrc, strandDst, isLast,
-            seenDirectIssues, seenBooks, j};
+            seenDirectIssues, seenBookOuts, j};
    };

    for (int i = 0; i < pes.size () - 1; ++i)
@@ -456,7 +456,7 @@ StrandContext::StrandContext (
    AccountID strandDst_,
    bool isLast_,
    std::array<boost::container::flat_set<Issue>, 2>& seenDirectIssues_,
-    boost::container::flat_set<Book>& seenBooks_,
+    boost::container::flat_set<Issue>& seenBookOuts_,
    beast::Journal j_)
        : view (view_)
        , strandSrc (strandSrc_)
@@ -467,7 +467,7 @@ StrandContext::StrandContext (
        , prevStep (!strand_.empty () ? strand_.back ().get ()
                     : nullptr)
        , seenDirectIssues(seenDirectIssues_)
-        , seenBooks(seenBooks_)
+        , seenBookOuts(seenBookOuts_)
        , j (j_)
 {
 }
--- a/src/ripple/app/paths/impl/Steps.h
+++ b/src/ripple/app/paths/impl/Steps.h
@@ -54,13 +54,14 @@ class ApplyView;
   when executing `fwd` or `rev`, but all those offers will be from the same
   quality directory.

-   A step may not have enough liquidity to transform the entire requested amount. Both
-   `fwd` and `rev` return a pair of amounts (one for input amount, one for output amount)
-   that show how much of the requested amount the step was actually able use.
+   A step may not have enough liquidity to transform the entire requested
+   amount. Both `fwd` and `rev` return a pair of amounts (one for input amount,
+   one for output amount) that show how much of the requested amount the step
+   was actually able to use.
 */
 class Step
 {
-  public:
+public:
    virtual ~Step () = default;

    /**
@@ -109,6 +110,25 @@ class Step
    boost::optional<EitherAmount>
    cachedOut () const = 0;

+    /**
+       If this step is DirectStepI (IOU->IOU direct step), return the src
+       account. This is needed for checkNoRipple.
+    */
+    virtual boost::optional<AccountID>
+    directStepSrcAcct () const
+    {
+        return boost::none;
+    }
+
+    /**
+       If this step is a BookStep, return the book.
+    */
+    virtual boost::optional<Book>
+    bookStepBook () const
+    {
+        return boost::none;
+    }
+
    /**
       Check if amount is zero
    */
@@ -122,6 +142,10 @@ class Step
        EitherAmount const& lhs,
        EitherAmount const& rhs) const = 0;

+    virtual bool equalIn (
+        EitherAmount const& lhs,
+        EitherAmount const& rhs) const = 0;
+
    /**
       Check that the step can correctly execute in the forward direction

@@ -267,18 +291,22 @@ struct StepImp : public Step
    }

    bool
-    equalOut (
-        EitherAmount const& lhs,
-        EitherAmount const& rhs) const override
+    equalOut (EitherAmount const& lhs, EitherAmount const& rhs) const override
    {
-        return get<TOut> (lhs) == get <TOut> (rhs);
+        return get<TOut> (lhs) == get<TOut> (rhs);
+    }
+
+    bool
+    equalIn (EitherAmount const& lhs, EitherAmount const& rhs) const override
+    {
+        return get<TIn> (lhs) == get<TIn> (rhs);
    }
 };

 // Thrown when unexpected errors occur
 class FlowException : public std::runtime_error
 {
-  public:
+public:
    TER ter;
    std::string msg;

@@ -312,14 +340,14 @@ struct StrandContext
    bool const isLast = false;
    size_t const strandSize;
    // The previous step in the strand. Needed to check the no ripple constraint
-    Step const * const prevStep = nullptr;
+    Step const* const prevStep = nullptr;
    // A strand may not include the same account node more than once
    // in the same currency. In a direct step, an account will show up
    // at most twice: once as a src and once as a dst (hence the two element array).
    // The strandSrc and strandDst will only show up once each.
    std::array<boost::container::flat_set<Issue>, 2>& seenDirectIssues;
-    // A strand may not include the same offer book more than once
-    boost::container::flat_set<Book>& seenBooks;
+    // A strand may not include an offer that output the same issue more than once
+    boost::container::flat_set<Issue>& seenBookOuts;
    beast::Journal j;

    StrandContext (ReadView const& view_,
@@ -330,12 +358,11 @@ struct StrandContext
        AccountID strandDst_,
        bool isLast_,
        std::array<boost::container::flat_set<Issue>, 2>& seenDirectIssues_,
-        boost::container::flat_set<Book>& seenBooks_,
+        boost::container::flat_set<Issue>& seenBookOuts_,
        beast::Journal j);
 };

-namespace test
-{
+namespace test {
 // Needed for testing
 bool directStepEqual (Step const& step,
    AccountID const& src,
--- a/src/ripple/app/paths/impl/StrandFlow.h
+++ b/src/ripple/app/paths/impl/StrandFlow.h
@@ -181,7 +181,19 @@ flow (
        {
            EitherAmount stepIn (limitStepOut);
            for (auto i = limitingStep + 1; i < s; ++i)
-                stepIn = strand[i]->fwd (*sb, *afView, ofrsToRm, stepIn).second;
+            {
+                auto const r = strand[i]->fwd (*sb, *afView, ofrsToRm, stepIn);
+                if (strand[i]->dry (r.second) ||
+                    !strand[i]->equalIn (r.first, stepIn))
+                {
+                    // The limits should already have been found, so executing a strand forward
+                    // from the limiting step should not find a new limit
+                    JLOG (j.fatal()) << "Re-executed forward pass failed";
+                    assert (0);
+                    return {telFAILED_PROCESSING, std::move (ofrsToRm)};
+                }
+                stepIn = r.second;
+            }
        }

        auto const strandIn = *strand.front ()->cachedIn ();
--- a/src/ripple/app/tests/Flow_test.cpp
+++ b/src/ripple/app/tests/Flow_test.cpp
@@ -274,6 +274,30 @@ struct Flow_test : public beast::unit_test::suite
            test (env, EUR, USD.issue (), STPath ({IPE (EUR), IPE (USD), IPE (EUR)}),
                  temBAD_PATH_LOOP);
        }
+
+        {
+            // cannot have more than one offer with the same output issue
+
+            using namespace jtx;
+            Env env (*this, features (featureFlowV2));
+
+            env.fund (XRP (10000), alice, bob, carol, gw);
+            env.trust (USD (10000), alice, bob, carol);
+            env.trust (EUR (10000), alice, bob, carol);
+
+            env (pay (gw, bob, USD (100)));
+            env (pay (gw, bob, EUR (100)));
+
+            env (offer (bob, XRP (100), USD (100)));
+            env (offer (bob, USD (100), EUR (100)), txflags (tfPassive));
+            env (offer (bob, EUR (100), USD (100)), txflags (tfPassive));
+
+            // payment path: XRP -> XRP/USD -> USD/EUR -> EUR/USD
+            env (pay (alice, carol, USD (100)), path (~USD, ~EUR, ~USD),
+                sendmax (XRP (200)), txflags (tfNoRippleDirect),
+                ter (temBAD_PATH_LOOP));
+        }
+
        {
            Env env (*this, features(featureFlowV2));
            env.fund (XRP (10000), alice, bob, noripple (gw));
--- a/src/ripple/app/tests/Offer.test.cpp
+++ b/src/ripple/app/tests/Offer.test.cpp
@@ -41,6 +41,15 @@ class Offer_test : public beast::unit_test::suite
        return env.current()->info().parentCloseTime.time_since_epoch().count();
    }

+    static auto
+    xrpMinusFee (jtx::Env const& env, std::int64_t xrpAmount)
+        -> jtx::PrettyAmount
+    {
+        using namespace jtx;
+        auto feeDrops = env.current ()->fees ().base;
+        return drops (dropsPerXRP<std::int64_t>::value * xrpAmount - feeDrops);
+    };
+
 public:
    void testRmFundedOffer ()
    {
@@ -344,15 +353,6 @@ public:
            env (pay (alice, carol, USD2 (50)), path (~USD1, bob),
                sendmax (XRP (50)), txflags (tfNoRippleDirect));

-            auto xrpMinusFee = [](jtx::Env const& env,
-                std::int64_t xrpAmount) -> jtx::PrettyAmount
-            {
-                using namespace jtx;
-                auto feeDrops = env.current ()->fees ().base;
-                return drops (
-                    dropsPerXRP<std::int64_t>::value * xrpAmount - feeDrops);
-            };
-
            env.require (balance (alice, xrpMinusFee (env, 10000 - 50)));
            env.require (balance (bob, USD1 (100)));
            env.require (balance (bob, USD2 (0)));
--- a/src/ripple/app/tx/impl/OfferStream.cpp
+++ b/src/ripple/app/tx/impl/OfferStream.cpp
@@ -216,7 +216,7 @@ OfferStream::permRmOffer (std::shared_ptr<SLE> const& sle)
 template<class TIn, class TOut>
 void FlowOfferStream<TIn, TOut>::permRmOffer (std::shared_ptr<SLE> const& sle)
 {
-    toRemove_.insert (sle->key());
+    permToRemove_.insert (sle->key());
 }

 template class FlowOfferStream<STAmount, STAmount>;
--- a/src/ripple/app/tx/impl/OfferStream.h
+++ b/src/ripple/app/tx/impl/OfferStream.h
@@ -85,6 +85,7 @@ protected:
    virtual
    void
    permRmOffer (std::shared_ptr<SLE> const& sle) = 0;
+
 public:
    TOfferStreamBase (ApplyView& view, ApplyView& cancelView,
        Book const& book, NetClock::time_point when,
@@ -149,11 +150,11 @@ public:
    The `view_' ` `ApplyView` accumulates changes to the ledger.
    The `cancelView_` is used to determine if an offer is found
    unfunded or became unfunded.
-    The `toRemove` collection identifies offers that should be
+    The `permToRemove` collection identifies offers that should be
    removed even if the strand associated with this OfferStream
    is not applied.

-    Certain invalid offers are added to the `toRemove` collection:
+    Certain invalid offers are added to the `permToRemove` collection:
    - Offers with missing ledger entries
    - Offers that expired
    - Offers found unfunded:
@@ -165,7 +166,7 @@ template <class TIn, class TOut>
 class FlowOfferStream : public TOfferStreamBase<TIn, TOut>
 {
 private:
-    boost::container::flat_set<uint256> toRemove_;
+    boost::container::flat_set<uint256> permToRemove_;
 protected:
    void
    permRmOffer (std::shared_ptr<SLE> const& sle) override;
@@ -173,9 +174,9 @@ protected:
 public:
    using TOfferStreamBase<TIn, TOut>::TOfferStreamBase;

-    boost::container::flat_set<uint256> const& toRemove () const
+    boost::container::flat_set<uint256> const& permToRemove () const
    {
-        return toRemove_;
+        return permToRemove_;
    };
 };
 }
--- a/src/ripple/ledger/ApplyView.h
+++ b/src/ripple/ledger/ApplyView.h
@@ -201,11 +201,11 @@ public:

    // Called when a credit is made to an account
    // This is required to support PaymentSandbox
-    virtual
-    void
+    virtual void
    creditHook (AccountID const& from,
        AccountID const& to,
-            STAmount const& amount)
+        STAmount const& amount,
+        STAmount const& preCreditBalance)
    {
    }
 };
--- a/src/ripple/ledger/PaymentSandbox.h
+++ b/src/ripple/ledger/PaymentSandbox.h
@@ -36,31 +36,38 @@ namespace detail {
 class DeferredCredits
 {
 public:
-    // Get the adjusted balance of main for the
-    // balance between main and other.
-    STAmount
-    adjustedBalance (AccountID const& main,
+    struct Adjustment
+    {
+        Adjustment (STAmount const& d, STAmount const& c, STAmount const& b)
+            : debits (d), credits (c), origBalance (b) {}
+        STAmount debits;
+        STAmount credits;
+        STAmount origBalance;
+    };
+
+    // Get the adjustments for the balance between main and other.
+    // Returns the debits, credits and the original balance
+    boost::optional<Adjustment> adjustments (
+        AccountID const& main,
        AccountID const& other,
-            STAmount const& curBalance) const;
+        Currency const& currency) const;

    void credit (AccountID const& sender,
-                 AccountID const& receiver,
-                 STAmount const& amount);
+        AccountID const& receiver,
+        STAmount const& amount,
+        STAmount const& preCreditSenderBalance);

    void apply (DeferredCredits& to);
-
-    // VFALCO Is this needed?
-    // DEPRECATED
-    void clear ();
-
 private:
    // lowAccount, highAccount
    using Key = std::tuple<
        AccountID, AccountID, Currency>;
-
-    // lowAccountCredits, highAccountCredits
-    using Value = std::tuple<
-        STAmount, STAmount>;
+    struct Value
+    {
+        STAmount lowAcctCredits;
+        STAmount highAcctCredits;
+        STAmount lowAcctOrigBalance;
+    };

    static
    Key
@@ -145,7 +152,8 @@ public:
    void
    creditHook (AccountID const& from,
        AccountID const& to,
-            STAmount const& amount) override;
+            STAmount const& amount,
+                STAmount const& preCreditBalance) override;

    /** Apply changes to base view.

@@ -154,8 +162,7 @@ public:
        behavior will result.
    */
    /** @{ */
-    void
-    apply (RawView& to);
+    void apply (RawView& to);

    void
    apply (PaymentSandbox& to);
--- a/src/ripple/ledger/View.h
+++ b/src/ripple/ledger/View.h
@@ -328,6 +328,7 @@ transferXRP (ApplyView& view,
            STAmount const& amount,
                beast::Journal j);

+NetClock::time_point const& flowV2SoTime ();
 bool flowV2Switchover (NetClock::time_point const closeTime);

 } // ripple
--- a/src/ripple/ledger/impl/PaymentSandbox.cpp
+++ b/src/ripple/ledger/impl/PaymentSandbox.cpp
@@ -19,16 +19,19 @@

 #include <BeastConfig.h>
 #include <ripple/ledger/PaymentSandbox.h>
+#include <ripple/ledger/View.h>
+
+#include <boost/optional.hpp>
+
 #include <cassert>

 namespace ripple {

 namespace detail {

-auto
-DeferredCredits::makeKey (AccountID const& a1,
-    AccountID const& a2, Currency const& c) ->
-        Key
+auto DeferredCredits::makeKey (AccountID const& a1,
+    AccountID const& a2,
+    Currency const& c) -> Key
 {
    if (a1 < a2)
        return std::make_tuple(a1, a2, c);
@@ -36,12 +39,12 @@ DeferredCredits::makeKey (AccountID const& a1,
        return std::make_tuple(a2, a1, c);
 }

-void DeferredCredits::credit (AccountID const& sender,
-                              AccountID const& receiver,
-                              STAmount const& amount)
+void
+DeferredCredits::credit (AccountID const& sender,
+    AccountID const& receiver,
+    STAmount const& amount,
+    STAmount const& preCreditSenderBalance)
 {
-    using std::get;
-
    assert (sender != receiver);
    assert (!amount.negative());

@@ -53,52 +56,55 @@ void DeferredCredits::credit (AccountID const& sender,

        if (sender < receiver)
        {
-            get<1> (v) = amount;
-            get<0> (v) = amount.zeroed ();
+            v.highAcctCredits = amount;
+            v.lowAcctCredits = amount.zeroed ();
+            v.lowAcctOrigBalance = preCreditSenderBalance;
        }
        else
        {
-            get<1> (v) = amount.zeroed ();
-            get<0> (v) = amount;
+            v.highAcctCredits = amount.zeroed ();
+            v.lowAcctCredits = amount;
+            v.lowAcctOrigBalance = -preCreditSenderBalance;
        }

        map_[k] = v;
    }
    else
    {
+        // only record the balance the first time, do not record it here
        auto& v = i->second;
        if (sender < receiver)
-            get<1> (v) += amount;
+            v.highAcctCredits += amount;
        else
-            get<0> (v) += amount;
+            v.lowAcctCredits += amount;
    }
 }

-// Get the adjusted balance of main for the
-// balance between main and other.
-STAmount DeferredCredits::adjustedBalance (AccountID const& main,
-                                           AccountID const& other,
-                                           STAmount const& curBalance) const
+// Get the adjustments for the balance between main and other.
+auto
+DeferredCredits::adjustments (AccountID const& main,
+    AccountID const& other,
+    Currency const& currency) const -> boost::optional<Adjustment>
 {
-    using std::get;
-    STAmount result (curBalance);
+    boost::optional<Adjustment> result;

-    Key const k = makeKey (main, other, curBalance.getCurrency ());
+    Key const k = makeKey (main, other, currency);
    auto i = map_.find (k);
-    if (i != map_.end ())
-    {
-        auto const& v = i->second;
-        if (main < other)
-        {
-            result -= get<0> (v);
-        }
-        else
-        {
-            result -= get<1> (v);
-        }
-    }
+    if (i == map_.end ())
+        return result;

-    return result;
+    auto const& v = i->second;
+
+    if (main < other)
+    {
+        result.emplace (v.highAcctCredits, v.lowAcctCredits, v.lowAcctOrigBalance);
+        return result;
+    }
+    else
+    {
+        result.emplace (v.lowAcctCredits, v.highAcctCredits, -v.lowAcctOrigBalance);
+        return result;
+    }
 }

 void DeferredCredits::apply(
@@ -110,18 +116,15 @@ void DeferredCredits::apply(
            to.map_.emplace(p);
        if (! r.second)
        {
-            using std::get;
-            get<0>(r.first->second) += get<0>(p.second);
-            get<1>(r.first->second) += get<1>(p.second);
+            auto& toVal = r.first->second;
+            auto const& fromVal = p.second;
+            toVal.lowAcctCredits += fromVal.lowAcctCredits;
+            toVal.highAcctCredits += fromVal.highAcctCredits;
+            // Do not update the orig balance, it's already correct
        }
    }
 }

-void DeferredCredits::clear ()
-{
-    map_.clear ();
-}
-
 } // detail

 STAmount
@@ -129,19 +132,57 @@ PaymentSandbox::balanceHook (AccountID const& account,
    AccountID const& issuer,
        STAmount const& amount) const
 {
-    if (ps_)
-        return tab_.adjustedBalance (
-            account, issuer, ps_->balanceHook (account, issuer, amount));
-    return tab_.adjustedBalance(
-        account, issuer, amount);
+    /*
+    There are two algorithms here. The pre-switchover algorithm takes the
+    current amount and subtracts the recorded credits. The post-switchover
+    algorithm remembers the original balance, and subtracts the debits. The
+    post-switchover algorithm should be more numerically stable. Consider a
+    large credit with a small initial balance. The pre-switchover algorithm
+    computes (B+C)-C (where B+C will the the amount passed in). The
+    post-switchover algorithm returns B. When B and C differ by large
+    magnitudes, (B+C)-C may not equal B.
+    */
+
+    auto const currency = amount.getCurrency ();
+    auto const switchover = flowV2Switchover (info ().parentCloseTime);
+
+    auto adjustedAmt = amount;
+    if (switchover)
+    {
+        auto delta = amount.zeroed ();
+        auto lastBal = amount;
+        for (auto curSB = this; curSB; curSB = curSB->ps_)
+        {
+            if (auto adj = curSB->tab_.adjustments (account, issuer, currency))
+            {
+                delta += adj->debits;
+                lastBal = adj->origBalance;
+            }
+        }
+        adjustedAmt = std::min(amount, lastBal - delta);
+    }
+    else
+    {
+        for (auto curSB = this; curSB; curSB = curSB->ps_)
+        {
+            if (auto adj = curSB->tab_.adjustments (account, issuer, currency))
+            {
+                adjustedAmt -= adj->credits;
+            }
+        }
+    }
+
+    assert (!isXRP(issuer) || adjustedAmt >= beast::zero);
+    return adjustedAmt;
 }

 void
 PaymentSandbox::creditHook (AccountID const& from,
    AccountID const& to,
-        STAmount const& amount)
+        STAmount const& amount,
+            STAmount const& preCreditBalance)
 {
-    tab_.credit(from, to, amount);
+    tab_.credit (from, to, amount, preCreditBalance);
 }

 void
--- a/src/ripple/ledger/impl/View.cpp
+++ b/src/ripple/ledger/impl/View.cpp
@@ -31,12 +31,17 @@

 namespace ripple {

-bool flowV2Switchover (NetClock::time_point const closeTime)
+NetClock::time_point const& flowV2SoTime ()
 {
    using namespace std::chrono_literals;
    // Mon March 28, 2016 10:00:00am PST
    static NetClock::time_point const soTime{512503200s};
-    return closeTime > soTime;
+    return soTime;
+}
+
+bool flowV2Switchover (NetClock::time_point const closeTime)
+{
+    return closeTime > flowV2SoTime();
 }

 // VFALCO NOTE A copy of the other one for now
@@ -1092,7 +1097,7 @@ trustCreate (ApplyView& view,
        sleRippleState->setFieldAmount (sfBalance, bSetHigh ? -saBalance : saBalance);

        view.creditHook (uSrcAccountID,
-            uDstAccountID, saBalance);
+            uDstAccountID, saBalance, saBalance.zeroed());
    }

    return terResult;
@@ -1239,14 +1244,14 @@ rippleCredit (ApplyView& view,
    }
    else
    {
-        view.creditHook (uSenderID,
-            uReceiverID, saAmount);
-
        STAmount saBalance   = sleRippleState->getFieldAmount (sfBalance);

        if (bSenderHigh)
            saBalance.negate ();    // Put balance in sender terms.

+        view.creditHook (uSenderID,
+            uReceiverID, saAmount, saBalance);
+
        STAmount    saBefore    = saBalance;

        saBalance   -= saAmount;
@@ -1432,8 +1437,12 @@ accountSend (ApplyView& view,
        return rippleSend (view, uSenderID, uReceiverID, saAmount, saActual, j);
    }

-    view.creditHook (uSenderID,
-        uReceiverID, saAmount);
+    auto const fv2Switch = flowV2Switchover (view.info ().parentCloseTime);
+    if (!fv2Switch)
+    {
+        auto const dummyBalance = saAmount.zeroed();
+        view.creditHook (uSenderID, uReceiverID, saAmount, dummyBalance);
+    }

    /* XRP send which does not check reserve and can do pure adjustment.
     * Note that sender or receiver may be null and this not a mistake; this
@@ -1479,9 +1488,12 @@ accountSend (ApplyView& view,
        }
        else
        {
+            auto const sndBal = sender->getFieldAmount (sfBalance);
+            if (fv2Switch)
+                view.creditHook (uSenderID, xrpAccount (), saAmount, sndBal);
+
            // Decrement XRP balance.
-            sender->setFieldAmount (sfBalance,
-                sender->getFieldAmount (sfBalance) - saAmount);
+            sender->setFieldAmount (sfBalance, sndBal - saAmount);
            view.update (sender);
        }
    }
@@ -1489,8 +1501,12 @@ accountSend (ApplyView& view,
    if (tesSUCCESS == terResult && receiver)
    {
        // Increment XRP balance.
-        receiver->setFieldAmount (sfBalance,
-            receiver->getFieldAmount (sfBalance) + saAmount);
+        auto const rcvBal = receiver->getFieldAmount (sfBalance);
+        receiver->setFieldAmount (sfBalance, rcvBal + saAmount);
+
+        if (fv2Switch)
+            view.creditHook (xrpAccount (), uReceiverID, saAmount, -rcvBal);
+
        view.update (receiver);
    }

@@ -1619,12 +1635,11 @@ issueIOU (ApplyView& view,
    auto const must_delete = updateTrustLine(view, state, bSenderHigh, issue.account,
        start_balance, final_balance, j);

+    view.creditHook (issue.account, account, amount, start_balance);
+
    if (bSenderHigh)
        final_balance.negate ();

-    view.creditHook (issue.account,
-        account, amount);
-
    // Adjust the balance on the trust line if necessary. We do this even if we
    // are going to delete the line to reflect the correct balance at the time
    // of deletion.
@@ -1687,11 +1702,11 @@ redeemIOU (ApplyView& view,
    auto const must_delete = updateTrustLine (view, state, bSenderHigh, account,
        start_balance, final_balance, j);

+    view.creditHook (account, issue.account, amount, start_balance);
+
    if (bSenderHigh)
        final_balance.negate ();

-    view.creditHook (account,
-        issue.account, amount);

    // Adjust the balance on the trust line if necessary. We do this even if we
    // are going to delete the line to reflect the correct balance at the time
--- a/src/ripple/ledger/tests/PaymentSandbox_test.cpp
+++ b/src/ripple/ledger/tests/PaymentSandbox_test.cpp
@@ -21,6 +21,7 @@
 #include <ripple/ledger/ApplyViewImpl.h>
 #include <ripple/ledger/PaymentSandbox.h>
 #include <ripple/ledger/tests/PathSet.h>
+#include <ripple/ledger/View.h>

 namespace ripple {
 namespace test {
@@ -253,11 +254,49 @@ class PaymentSandbox_test : public beast::unit_test::suite
        }
    }

+    void testTinyBalance ()
+    {
+        testcase ("Tiny balance");
+
+        // Add and subtract a huge credit from a tiny balance, expect the tiny
+        // balance back. Numerical stability problems could cause the balance to
+        // be zero.
+
+        using namespace jtx;
+
+        Env env (*this);
+
+        Account const gw ("gw");
+        Account const alice ("alice");
+        auto const USD = gw["USD"];
+
+        auto const issue = USD.issue ();
+        STAmount tinyAmt (issue, STAmount::cMinValue, STAmount::cMinOffset + 1,
+            false, false, STAmount::unchecked{});
+        STAmount hugeAmt (issue, STAmount::cMaxValue, STAmount::cMaxOffset - 1,
+            false, false, STAmount::unchecked{});
+
+        for (auto timeDelta : {-env.closed ()->info ().closeTimeResolution,
+                 env.closed ()->info ().closeTimeResolution})
+        {
+            auto const closeTime = flowV2SoTime () + timeDelta;
+            env.close (closeTime);
+            ApplyViewImpl av (&*env.current (), tapNONE);
+            PaymentSandbox pv (&av);
+            pv.creditHook (gw, alice, hugeAmt, -tinyAmt);
+            if (closeTime > flowV2SoTime ())
+                expect (pv.balanceHook (alice, gw, hugeAmt) == tinyAmt);
+            else
+                expect (pv.balanceHook (alice, gw, hugeAmt) != tinyAmt);
+        }
+    }
+
 public:
    void run ()
    {
        testSelfFunding ();
        testSubtractCredits ();
+        testTinyBalance ();
    }
 };