Return correct error code during unfunded offer cross (RIPD-1082):

When placing an offer that sells XRP, if the account's balance was
low enough that paying the transaction fee would drop the balance
below the reserve, the transaction should return tecUNFUNDED_OFFER.

The existing implementation returned a tesSUCCESS instead. Although
the net result is the same as far as the transaction's effects are
concerned (the offer is not placed on the books and the transaction
fee is charged) the incorrect result code makes deciphering metadata
difficult.

Add unit test that verifies the new behavior.

src/ripple/app/tests/Offer.test.cpp

@@ -630,6 +630,67 @@ public:
             owners ("bob", 1));
     }
 
+    void
+    testUnfundedCross()
+    {
+        testcase ("Unfunded Crossing");
+
+        using namespace jtx;
+
+        auto const gw = Account("gateway");
+        auto const USD = gw["USD"];
+
+        auto const usdOffer = USD(1000);
+        auto const xrpOffer = XRP(1000);
+
+        Env env(*this);
+        env.fund (XRP(1000000), gw);
+
+        // The fee that's charged for transactions
+        auto const f = env.current ()->fees ().base;
+
+        // Account is at the reserve, and will dip below once
+        // fees are subtracted.
+        env.fund (reserve (env, 0), "alice");
+        env (offer ("alice", usdOffer, xrpOffer),     ter(tecUNFUNDED_OFFER));
+        env.require (
+            balance ("alice", reserve (env, 0) - f),
+            owners ("alice", 0));
+
+        // Account has just enough for the reserve and the
+        // fee.
+        env.fund (reserve (env, 0) + f, "bob");
+        env (offer ("bob", usdOffer, xrpOffer),       ter(tecUNFUNDED_OFFER));
+        env.require (
+            balance ("bob", reserve (env, 0)),
+            owners ("bob", 0));
+
+        // Account has enough for the reserve, the fee and
+        // the offer, and a bit more, but not enough for the
+        // reserve after the offer is placed.
+        env.fund (reserve (env, 0) + f + XRP(1), "carol");
+        env (offer ("carol", usdOffer, xrpOffer),     ter(tecINSUF_RESERVE_OFFER));
+        env.require (
+            balance ("carol", reserve (env, 0) + XRP(1)),
+            owners ("carol", 0));
+
+        // Account has enough for the reserve plus one
+        // offer, and the fee.
+        env.fund (reserve (env, 1) + f, "dan");
+        env (offer ("dan", usdOffer, xrpOffer),       ter(tesSUCCESS));
+        env.require (
+            balance ("dan", reserve (env, 1)),
+            owners ("dan", 1));
+
+        // Account has enough for the reserve plus one
+        // offer, the fee and the entire offer amount.
+        env.fund (reserve (env, 1) + f + xrpOffer, "eve");
+        env (offer ("eve", usdOffer, xrpOffer),       ter(tesSUCCESS));
+        env.require (
+            balance ("eve", reserve (env, 1) + xrpOffer),
+            owners ("eve", 1));
+    }
+
     void run ()
     {
         testCanceledOffer ();
@@ -640,6 +701,7 @@ public:
         testFillModes ();
         testMalformed ();
         testExpiration ();
+        testUnfundedCross ();
     }
 };
 

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

@@ -575,10 +575,9 @@ CreateOffer::step_account (OfferStream& stream, Taker const& taker, Logs& logs)
     return false;
 }
 
-// Fill offer as much as possible by consuming offers already on the books,
-// and adjusting account balances accordingly.
-//
-// Charges fees on top to taker.
+// Fill as much of the offer as possible by consuming offers
+// already on the books. Return the status and the amount of
+// the offer to left unfilled.
 std::pair<TER, Amounts>
 CreateOffer::cross (
     ApplyView& view,
@@ -592,6 +591,20 @@ CreateOffer::cross (
     Taker taker (cross_type_, view, account_, taker_amount,
         ctx_.tx.getFlags(), beast::Journal (takerSink));
 
+    // If the taker is unfunded before we begin crossing
+    // there's nothing to do - just return an error.
+    //
+    // We check this in preclaim, but when selling XRP
+    // charged fees can cause a user's available balance
+    // to go to 0 (by causing it to dip below the reserve)
+    // so we check this case again.
+    if (taker.unfunded ())
+    {
+        JLOG (j_.debug) <<
+            "Not crossing: taker is unfunded.";
+        return { tecUNFUNDED_OFFER, taker_amount };
+    }
+
     try
     {
         if (cross_type_ == CrossType::IouToIou)
@@ -601,8 +614,9 @@ CreateOffer::cross (
     }
     catch (std::exception const& e)
     {
-        j_.error << "Exception during offer crossing: " << e.what ();
-        return std::make_pair (tecINTERNAL, taker.remaining_offer ());
+        JLOG (j_.error) <<
+            "Exception during offer crossing: " << e.what ();
+        return { tecINTERNAL, taker.remaining_offer () };
     }
 }
 
@@ -794,7 +808,7 @@ CreateOffer::applyGuts (ApplyView& view, ApplyView& view_cancel)
     // entire operation should be aborted, with only fees paid.
     if (bFillOrKill)
     {
-        j_.trace << "Fill or Kill: offer killed";
+        JLOG (j_.trace) << "Fill or Kill: offer killed";
         return { tesSUCCESS, false };
     }
 
@@ -802,7 +816,7 @@ CreateOffer::applyGuts (ApplyView& view, ApplyView& view_cancel)
     // placed - it gets cancelled and the operation succeeds.
     if (bImmediateOrCancel)
     {
-        j_.trace << "Immediate or cancel: offer cancelled";
+        JLOG (j_.trace) << "Immediate or cancel: offer cancelled";
         return { tesSUCCESS, true };
     }
 
@@ -847,7 +861,7 @@ CreateOffer::applyGuts (ApplyView& view, ApplyView& view_cancel)
         // Update owner count.
         adjustOwnerCount(view, sleCreator, 1, viewJ);
 
-        if (j_.trace) j_.trace <<
+        JLOG (j_.trace) <<
             "adding to book: " << to_string (saTakerPays.issue ()) <<
             " : " << to_string (saTakerGets.issue ());
 

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

@@ -111,6 +111,16 @@ BasicTaker::effective_rate (
     return Rate (QUALITY_ONE);
 }
 
+bool
+BasicTaker::unfunded () const
+{
+    if (get_funds (account(), remaining_.in) > zero)
+        return false;
+
+    journal_.debug << "Unfunded: taker is out of funds.";
+    return true;
+}
+
 bool
 BasicTaker::done () const
 {
@@ -130,7 +140,7 @@ BasicTaker::done () const
     }
 
     // We are done if the taker is out of funds
-    if (get_funds (account(), remaining_.in) <= zero)
+    if (unfunded ())
     {
         journal_.debug << "Done: taker out of funds.";
         return true;
@@ -147,7 +157,7 @@ BasicTaker::remaining_offer () const
         return Amounts (remaining_.in.zeroed(), remaining_.out.zeroed());
 
     // Avoid math altogether if we didn't cross.
-   if (original_ == remaining_)
+    if (original_ == remaining_)
        return original_;
 
     if (sell_)
@@ -379,8 +389,6 @@ BasicTaker::flow_iou_to_iou (
 BasicTaker::Flow
 BasicTaker::do_cross (Amounts offer, Quality quality, AccountID const& owner)
 {
-    assert (!done ());
-
     auto const owner_funds = get_funds (owner, offer.out);
     auto const taker_funds = get_funds (account (), offer.in);
 
@@ -419,8 +427,6 @@ BasicTaker::do_cross (
     Amounts offer1, Quality quality1, AccountID const& owner1,
     Amounts offer2, Quality quality2, AccountID const& owner2)
 {
-    assert (!done ());
-
     assert (!offer1.in.native ());
     assert (offer1.out.native ());
     assert (offer2.in.native ());

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

@@ -208,6 +208,10 @@ public:
         return issue_out_;
     }
 
+    /** Returns `true` if the taker has run out of funds. */
+    bool
+    unfunded () const;
+
     /** Returns `true` if order crossing should not continue.
         Order processing is stopped if the taker's order quantities have
         been reached, or if the taker has run out of input funds.