Autobridging Pt. 4

src/BeastConfig.h

@@ -205,11 +205,11 @@ This determines whether to add any features to the proposed transaction set.
 #define RIPPLE_PROPOSE_AMENDMENTS 0
 #endif
 
-/** Config: RIPPLE_USE_OLD_CREATE_TRANSACTOR
-This determines whether ripple uses the legacy OfferCreate transactor.
+/** Config: RIPPLE_ENABLE_AUTOBRIDGING
+This determines whether ripple implements offer autobridging via XRP.
 */
-#ifndef RIPPLE_USE_OLD_CREATE_TRANSACTOR
-#define RIPPLE_USE_OLD_CREATE_TRANSACTOR 1
+#ifndef RIPPLE_ENABLE_AUTOBRIDGING
+#define RIPPLE_ENABLE_AUTOBRIDGING 0
 #endif
 
 #endif

src/ripple/module/app/book/Offer.h

@@ -80,7 +80,8 @@ class Offer
     Amounts const
     amount() const
     {
-        return Amounts (m_entry->getFieldAmount (sfTakerPays),
+        return Amounts (
+            m_entry->getFieldAmount (sfTakerPays),
             m_entry->getFieldAmount (sfTakerGets));
     }
 

src/ripple/module/app/book/Taker.h

@@ -29,7 +29,6 @@
 #include <beast/utility/noexcept.h>
 
 #include <functional>
-//#include <utility>
 
 namespace ripple {
 namespace core {
@@ -80,6 +79,9 @@ class Taker
     fill (Offer const& leg1, Amounts const& amount1,
         Offer const& leg2, Amounts const& amount2);
 
+    void
+    consume (Offer const& offer, Amounts const& consumed) const;
+
 public:
     Taker (LedgerView& view, Account const& account,
         Amounts const& amount, Options const& options);

src/ripple/module/app/book/impl/Quality.cpp

@@ -47,7 +47,7 @@ Quality
 Quality::operator++ (int)
 {
     Quality prev (*this);
-    --*this;
+    ++*this;
     return prev;
 }
 
@@ -63,7 +63,7 @@ Quality
 Quality::operator-- (int)
 {
     Quality prev (*this);
-    ++*this;
+    --*this;
     return prev;
 }
 
@@ -77,8 +77,10 @@ Quality::ceil_in (Amounts const& amount, Amount const& limit) const
         // Clamp out
         if (result.out > amount.out)
             result.out = amount.out;
+        assert (result.in == limit);
         return result;
     }
+    assert (amount.in <= limit);
     return amount;
 }
 
@@ -92,8 +94,10 @@ Quality::ceil_out (Amounts const& amount, Amount const& limit) const
         // Clamp in
         if (result.in > amount.in)
             result.in = amount.in;
+        assert (result.out == limit);
         return result;
     }
+    assert (amount.out <= limit);
     return amount;
 }
 

src/ripple/module/app/book/impl/Taker.cpp

@@ -19,8 +19,6 @@
 
 #include <ripple/module/app/book/Taker.h>
 
-//#include <utility>
-
 namespace ripple {
 namespace core {
 
@@ -34,6 +32,9 @@ Taker::Taker (LedgerView& view, Account const& account,
     , m_amount (amount)
     , m_remain (amount)
 {
+    assert (m_remain.in > zero);
+    assert (m_remain.out > zero);
+
     // If this is a passive order (tfPassive), this prevents
     // offers at the same quality level from being consumed.
     if (m_options.passive)
@@ -71,11 +72,12 @@ Taker::remaining_offer () const
         m_remain.out, m_quality.rate (), m_remain.in, true), m_remain.out);
 }
 
-// Calculate the amount particular user could get through an offer.
-//   @param amount the maximum flow that is available to the taker.
-//   @param offer the offer to flow through.
-//   @param taker the person taking the offer.
-//   @return the maximum amount that can flow through this offer.
+/** Calculate the amount particular user could get through an offer.
+    @param amount the maximum flow that is available to the taker.
+    @param offer the offer to flow through.
+    @param taker the person taking the offer.
+    @return the maximum amount that can flow through this offer.
+*/
 Amounts
 Taker::flow (Amounts amount, Offer const& offer, Account const& taker)
 {
@@ -129,25 +131,35 @@ Taker::flow (Amounts amount, Offer const& offer, Account const& taker)
         : amount;
 }
 
+// Adjust an offer to indicate that we are consuming some (or all) of it.
+void
+Taker::consume (Offer const& offer, Amounts const& consumed) const
+{
+    Amounts const& remaining (offer.amount ());
+
+    assert (remaining.in > zero && remaining.out > zero);
+    assert (remaining.in >= consumed.in && remaining.out >= consumed.out);
+
+    offer.entry ()->setFieldAmount (sfTakerPays, remaining.in - consumed.in);
+    offer.entry ()->setFieldAmount (sfTakerGets, remaining.out - consumed.out);
+
+    view ().entryModify (offer.entry());
+
+    assert (offer.entry ()->getFieldAmount (sfTakerPays) >= zero);
+    assert (offer.entry ()->getFieldAmount (sfTakerGets) >= zero);
+}
+
 // Fill a direct offer.
 //   @param offer the offer we are going to use.
 //   @param amount the amount to flow through the offer.
 //   @returns: tesSUCCESS if successful, or an error code otherwise.
 TER
 Taker::fill (Offer const& offer, Amounts const& amount)
 {
-    TER result (tesSUCCESS);
-
-    Amounts const remain (
-        offer.entry ()->getFieldAmount (sfTakerPays) - amount.in,
-        offer.entry ()->getFieldAmount (sfTakerGets) - amount.out);
-
-    offer.entry ()->setFieldAmount (sfTakerPays, remain.in);
-    offer.entry ()->setFieldAmount (sfTakerGets, remain.out);
-    view ().entryModify (offer.entry());
+    consume (offer, amount);
 
     // Pay the taker, then the owner
-    result = view ().accountSend (offer.account(), account(), amount.out);
+    TER result = view ().accountSend (offer.account(), account(), amount.out);
 
     if (result == tesSUCCESS)
         result = view ().accountSend (account(), offer.account(), amount.in);
@@ -168,27 +180,14 @@ Taker::fill (
 {
     assert (amount1.out == amount2.in);
 
-    TER result (tesSUCCESS);
+    consume (leg1, amount1);
+    consume (leg2, amount2);
 
-    Amounts const remain1 (
-        leg1.entry ()->getFieldAmount (sfTakerPays) - amount1.in,
-        leg1.entry ()->getFieldAmount (sfTakerGets) - amount1.out);
-
-    leg1.entry ()->setFieldAmount (sfTakerPays, remain1.in);
-    leg1.entry ()->setFieldAmount (sfTakerGets, remain1.out);
-    view ().entryModify (leg1.entry());
-
-    Amounts const remain2 (
-        leg2.entry ()->getFieldAmount (sfTakerPays) - amount2.in,
-        leg2.entry ()->getFieldAmount (sfTakerGets) - amount2.out);
-    view ().entryModify (leg2.entry ());
-
-    // Execute the payments in order:
-    // Taker pays Leg1 (amount1.in - A currency)
-    // Leg1 pays Leg2 (amount1.out == amount2.in, XRP)
-    // Leg2 pays Taker (amount2.out - B currency)
-
-    result = view ().accountSend (m_account, leg1.account (), amount1.in);
+    /* It is possible that m_account is the same as leg1.account, leg2.account
+     * or both. This could happen when bridging over one's own offer. In that
+     * case, accountSend won't actually do a send, which is what we want.
+     */
+    TER result = view ().accountSend (m_account, leg1.account (), amount1.in);
 
     if (result == tesSUCCESS)
         result = view ().accountSend (leg1.account (), leg2.account (), amount1.out);
@@ -202,17 +201,15 @@ Taker::fill (
 bool
 Taker::done () const
 {
-    if (m_options.sell)
+    if (m_options.sell && (m_remain.in <= zero))
     {
-        // With the sell option, we are finished when
-        // we have consumed all the input currency.
-        if (m_remain.in <= zero)
-            return true;
+        // Sell semantics: we consumed all the input currency
+        return true;
     }
-    else if (m_remain.out <= zero)
+
+    if (!m_options.sell && (m_remain.out <= zero))
     {
-        // With the buy option (!sell) we are finished when we
-        // have received the desired amount of output currency.
+        // Buy semantics: we received the desired amount of output currency
         return true;
     }
 
@@ -224,18 +221,25 @@ TER
 Taker::cross (Offer const& offer)
 {
     assert (!done ());
+
+    /* Before we call flow we must set the limit right; for buy semantics we
+       need to clamp the output. And we always want to clamp the input.
+     */
     Amounts limit (offer.amount());
-    if (m_options.sell)
-        limit = offer.quality().ceil_in (limit, m_remain.in);
-    else
+
+    if (! m_options.sell)
         limit = offer.quality ().ceil_out (limit, m_remain.out);
+    limit = offer.quality().ceil_in (limit, m_remain.in);
 
-    assert (limit.out <= offer.amount().out);
     assert (limit.in <= offer.amount().in);
+    assert (limit.out <= offer.amount().out);
+    assert (limit.in <= m_remain.in);
 
     Amounts const amount (flow (limit, offer, account ()));
+
     m_remain.out -= amount.out;
     m_remain.in -= amount.in;
+
     assert (m_remain.in >= zero);
     return fill (offer, amount);
 }

src/ripple/module/app/book/tests/Quality.test.cpp

@@ -222,6 +222,8 @@ class Quality_test : public beast::unit_test::suite
     void
     test_raw()
     {
+        testcase ("raw");
+
         {
             Quality q (0x5d048191fb9130daull);      // 126836389.7680090
             Amounts const value (
@@ -234,12 +236,85 @@ class Quality_test : public beast::unit_test::suite
         }
     }
 
+    void
+    test_comparisons()
+    {
+        testcase ("comparisons");
+
+        Amount const amount1 (noCurrency (), noAccount (), 231);
+        Amount const amount2 (noCurrency (), noAccount (), 462);
+        Amount const amount3 (noCurrency (), noAccount (), 924);
+
+        Quality const q11 (core::Amounts (amount1, amount1));
+        Quality const q12 (core::Amounts (amount1, amount2));
+        Quality const q13 (core::Amounts (amount1, amount3));
+        Quality const q21 (core::Amounts (amount2, amount1));
+        Quality const q31 (core::Amounts (amount3, amount1));
+
+        expect (q11 == q11);
+        expect (q11 < q12);
+        expect (q12 < q13);
+        expect (q31 < q21);
+        expect (q21 < q11);
+        expect (q31 != q21);
+    }
+
+    void
+    test_composition ()
+    {
+        testcase ("composition");
+
+        Amount const amount1 (noCurrency (), noAccount (), 231);
+        Amount const amount2 (noCurrency (), noAccount (), 462);
+        Amount const amount3 (noCurrency (), noAccount (), 924);
+
+        Quality const q11 (core::Amounts (amount1, amount1));
+        Quality const q12 (core::Amounts (amount1, amount2));
+        Quality const q13 (core::Amounts (amount1, amount3));
+        Quality const q21 (core::Amounts (amount2, amount1));
+        Quality const q31 (core::Amounts (amount3, amount1));
+
+        expect (composed_quality (q12, q21) == q11);
+
+        Quality const q13_31 (composed_quality (q13, q31));
+        Quality const q31_13 (composed_quality (q31, q13));
+
+        expect (q13_31 == q31_13);
+        expect (q13_31 == q11);
+    }
+
+    void
+    test_operations ()
+    {
+        testcase ("operations");
+
+        Quality const q11 (core::Amounts (
+            Amount (noCurrency (), noAccount (), 731),
+            Amount (noCurrency (), noAccount (), 731)));
+
+        Quality qa (q11);
+        Quality qb (q11);
+
+        expect (qa == qb);
+        expect (++qa != q11);
+        expect (qa != qb);
+        expect (--qb != q11);
+        expect (qa != qb);
+        expect (qb < qa);
+        expect (qb++ < qa);
+        expect (qb++ < qa);
+        expect (qb++ == qa);
+        expect (qa < qb);
+    }
     void
     run()
     {
+        test_comparisons ();
+        test_composition ();
+        test_operations ();
         test_ceil_in ();
         test_ceil_out ();
-        test_raw();
+        test_raw ();
     }
 };