major refactor templates for trustTransfer and lock

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

@@ -242,25 +242,38 @@ EscrowCreate::doApply()
 
         // check if the escrow is capable of being
         // finished before we allow it to be created
-        if (TER result = 
+        {
-            trustTransferAllowed(
+            TER result = 
-                ctx_.view(),
+                trustTransferAllowed(
-                {account, ctx_.tx[sfDestination]},
+                    ctx_.view(),
-                amount.issue());
+                    {account, ctx_.tx[sfDestination]},
-            result != tesSUCCESS)
+                    amount.issue());
-            return result;
+            std::cout 
+                << "EscrowCreate::doApply trustTransferAllowed result="
+                << result
+                << "\n";
+            if (!isTesSuccess(result))
+                return result;
+        }
 
         // perform the lock as a dry run before
         // we modify anything on-ledger
         sleLine = ctx_.view().peek(keylet::line(account, amount.getIssuer(), amount.getCurrency()));
-        if (TER result = 
+
+        {
+            TER result = 
                 trustAdjustLockedBalance(
                     ctx_.view(),
                     sleLine,
                     amount,
-                    true);
+                    DryRun);
-                result != tesSUCCESS)
+                std::cout 
-            return result;
+                    << "EscrowCreate::doApply trustAdjustLockedBalance (dry) result="
+                    << result
+                    << "\n";
+            if (!isTesSuccess(result))
+                return result;
+        }
     }
 
     // Check destination account
@@ -329,9 +342,14 @@ EscrowCreate::doApply()
                 ctx_.view(),
                 sleLine,
                 amount,
-                false);
+                WetRun);
+                
+        std::cout 
+            << "EscrowCreate::doApply trustAdjustLockedBalance (wet) result="
+            << result
+            << "\n";
 
-        if (result != tesSUCCESS)
+        if (!isTesSuccess(result))
             return result;
     }
 
@@ -553,10 +571,15 @@ EscrowFinish::doApply()
                 sled,       // dst account
                 amount,     // xfer amount
                 ctx_.journal,
-                true       // dry run
+                DryRun      // dry run
             );
 
-        if (result != tesSUCCESS)
+        std::cout
+            << "EscrowFinish::doApply trustTransferLockedBalance (dry) result="
+            << result
+            << "\n";
+
+        if (!isTesSuccess(result))
             return result;
     }
 
@@ -599,10 +622,15 @@ EscrowFinish::doApply()
                 sled,       // dst account
                 amount,     // xfer amount
                 ctx_.journal,
-                false       // wet run;
+                WetRun      // wet run;
             );
 
-        if (result != tesSUCCESS)
+        std::cout
+            << "EscrowFinish::doApply trustTransferLockedBalance (wet) result="
+            << result
+            << "\n";
+
+        if (isTesSuccess(result))
             return result;
     }
 
@@ -683,7 +711,7 @@ EscrowCancel::doApply()
                 ctx_.view(),
                 sleLine,
                 -amount,
-                true);
+                DryRun);
             result != tesSUCCESS)
             return result;
     }
@@ -727,8 +755,12 @@ EscrowCancel::doApply()
                 ctx_.view(),
                 sleLine,
                 -amount,
-                false);
+                WetRun);
-        if (result != tesSUCCESS)
+        std::cout 
+            << "EscrowCancel::doApply trustAdjustLockedBalance (wet) result="
+            << result
+            << "\n";
+        if (!isTesSuccess(result))
             return result;
     }
 

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

@@ -120,6 +120,35 @@ closeChannel(
     beast::Journal j)
 {
     AccountID const src = (*slep)[sfAccount];
+    auto const amount = (*slep)[sfAmount] - (*slep)[sfBalance];
+
+    std::shared_ptr<SLE> sleLine;
+
+    if (!isXRP(amount))
+    {
+        if (!view.rules().enabled(featurePaychanAndEscrowForTokens))
+            return tefINTERNAL;
+
+        sleLine =
+            view.peek(keylet::line(src, amount.getIssuer(), amount.getCurrency()));
+    
+        // dry run
+        TER result = 
+            trustAdjustLockedBalance(
+                view,
+                sleLine,
+                -amount,
+                DryRun);
+
+        std::cout
+            << "closeChannel: trustAdjustLockedBalance(dry) result="
+            << result
+            << "\n";
+
+        if (!isTesSuccess(result))
+            return result;
+    }
+
     // Remove PayChan from owner directory
     {
         auto const page = (*slep)[sfOwnerNode];
@@ -150,25 +179,24 @@ closeChannel(
         return tefINTERNAL;
 
     assert((*slep)[sfAmount] >= (*slep)[sfBalance]);
-    auto const amount = (*slep)[sfAmount] - (*slep)[sfBalance];
 
     if (isXRP(amount))
-    {
         (*sle)[sfBalance] = (*sle)[sfBalance] + amount;
-    }
     else
     {
-        if (view.rules().enabled(featurePaychanAndEscrowForTokens))
-            return tefINTERNAL;
-
-        auto line = view.peek(keylet::line(src, amount.getIssuer(), amount.getCurrency()));
         TER result = 
             trustAdjustLockedBalance(
                 view,
-                line,
+                sleLine,
                 -amount,
-                false);
+                WetRun);
-        if (result != tesSUCCESS)
+
+        std::cout
+            << "closeChannel: trustAdjustLockedBalance(wet) result="
+            << result
+            << "\n";
+
+        if (!isTesSuccess(result))
             return result;
     }
 
@@ -260,29 +288,38 @@ PayChanCreate::preclaim(PreclaimContext const& ctx)
 
         // check for any possible bars to a channel existing
         // between these accounts for this asset
-        if (TER result = 
+        {
+            TER result = 
                 trustTransferAllowed(
                     ctx.view,
                     {account, dst},
                     amount.issue());
-            result != tesSUCCESS)
+            std::cout
+                << "PayChanCreate::preclaim trustTransferAllowed result="
+                << result
+                << "\n";
+            if (!isTesSuccess(result))
                 return result;
+        }
 
         // check if the amount can be locked
-        auto sleLine = 
+        {
-            ctx.view.read(
+            auto sleLine = 
-                keylet::line(account, amount.getIssuer(), amount.getCurrency()));
+                ctx.view.read(
-
+                    keylet::line(account, amount.getIssuer(), amount.getCurrency()));
-        if (TER result = 
+            TER result = 
                 trustAdjustLockedBalance(
                     ctx.view,
                     sleLine,
                     amount,
-                    true);
+                    DryRun);
-            result != tesSUCCESS)
+            std::cout
-            return result;
+                << "PayChanCreate::preclaim trustAdjustLockedBalance(dry) result="
+                << result;
 
-        // all good!
+            if (!isTesSuccess(result))
+                return result;
+        }
     }
 
     {
@@ -378,9 +415,14 @@ PayChanCreate::doApply()
                 ctx_.view(),
                 sleLine,
                 amount,
-                false);
+                WetRun);
 
-        if (result != tesSUCCESS)
+        std::cout
+            << "PayChanCreate::doApply trustAdjustLockedBalance(wet) result="
+            << result
+            << "\n";
+
+        if (!isTesSuccess(result))
             return tefINTERNAL;
     }
     
@@ -451,20 +493,27 @@ PayChanFund::doApply()
     // if this is a Fund operation on an IOU then perform a dry run here
     if (!isXRP(amount) &&
             ctx_.view().rules().enabled(featurePaychanAndEscrowForTokens))
+    {
         sleLine = ctx_.view().peek(
             keylet::line(
                 (*slep)[sfAccount], 
                 amount.getIssuer(),
                 amount.getCurrency()));
 
-    if (TER result =
+        TER result =
             trustAdjustLockedBalance(
                 ctx_.view(),
                 sleLine,
                 amount,
-                true);
+                DryRun);
-        result != tesSUCCESS)
+
+        std::cout
+            << "PayChanFund::doApply trustAdjustLockedBalance(dry) result="
+            << result
+            << "\n";
+        if (!isTesSuccess(result))
             return result;
+    }
 
     AccountID const src = (*slep)[sfAccount];
     auto const txAccount = ctx_.tx[sfAccount];
@@ -536,8 +585,14 @@ PayChanFund::doApply()
                 ctx_.view(),
                 sleLine,
                 amount,
-                false);
+                WetRun);
-        if (result != tesSUCCESS)
+        
+        std::cout
+            << "PayChanFund::doApply trustAdjustLockedBalance(wet) result="
+            << result
+            << "\n";
+
+        if (!isTesSuccess(result))
             return tefINTERNAL;
     }
 
@@ -707,22 +762,28 @@ PayChanClaim::doApply()
         else 
         {
             // xfer locked tokens to satisfy claim
+            // RH NOTE: there's no ledger modification before this point so
+            // no reason to do a dry run first
             if (!ctx_.view().rules().enabled(featurePaychanAndEscrowForTokens))
                 return tefINTERNAL;
 
             auto sleSrcAcc = ctx_.view().peek(keylet::account(src));
             TER result =
-                trustTransferLockedBalance
+                trustTransferLockedBalance(
-                (
                     ctx_.view(),
                     txAccount,
                     sleSrcAcc,
                     sled,
                     reqDelta,
                     ctx_.journal,
-                    false);
+                    WetRun);
+            
+            std::cout
+                << "PayChanClaim::doApply trustTransferLockedBalance(wet) result="
+                << result
+                << "\n";
 
-            if (result != tesSUCCESS)
+            if (!isTesSuccess(result))
                 return result;
         }
 

src/ripple/ledger/View.h

@@ -301,17 +301,119 @@ trustDelete(
     AccountID const& uHighAccountID,
     beast::Journal j);
 
-bool isTrustDefault(
-    std::shared_ptr<SLE> const& acc,
-    std::shared_ptr<SLE> const& line);
 
-template<class V, class S>
+/** Delete an offer.
+
+    Requirements:
+        The passed `sle` be obtained from a prior
+        call to view.peek()
+*/
+// [[nodiscard]] // nodiscard commented out so Flow, BookTip and others compile.
+TER
+offerDelete(ApplyView& view, std::shared_ptr<SLE> const& sle, beast::Journal j);
+
+//------------------------------------------------------------------------------
+
+//
+// Money Transfers
+//
+
+// Direct send w/o fees:
+// - Redeeming IOUs and/or sending sender's own IOUs.
+// - Create trust line of needed.
+// --> bCheckIssuer : normally require issuer to be involved.
+// [[nodiscard]] // nodiscard commented out so DirectStep.cpp compiles.
+TER
+rippleCredit(
+    ApplyView& view,
+    AccountID const& uSenderID,
+    AccountID const& uReceiverID,
+    const STAmount& saAmount,
+    bool bCheckIssuer,
+    beast::Journal j);
+
+[[nodiscard]] TER
+accountSend(
+    ApplyView& view,
+    AccountID const& from,
+    AccountID const& to,
+    const STAmount& saAmount,
+    beast::Journal j);
+
+[[nodiscard]] TER
+issueIOU(
+    ApplyView& view,
+    AccountID const& account,
+    STAmount const& amount,
+    Issue const& issue,
+    beast::Journal j);
+
+[[nodiscard]] TER
+redeemIOU(
+    ApplyView& view,
+    AccountID const& account,
+    STAmount const& amount,
+    Issue const& issue,
+    beast::Journal j);
+
+[[nodiscard]] TER
+transferXRP(
+    ApplyView& view,
+    AccountID const& from,
+    AccountID const& to,
+    STAmount const& amount,
+    beast::Journal j);
+
+//------------------------------------------------------------------------------
+
+//
+// Trustline Locking and Transfer (PaychanAndEscrowForTokens)
+//
+
+// In functions white require a `RunType` 
+// pass DryRun (don't apply changes) or WetRun (do apply changes)
+// to allow compile time evaluation of which types and calls to use
+
+// For all functions below that take a Dry/Wet run parameter
+// View may be ReadView const or ApplyView for DryRuns.
+// View *must* be ApplyView for a WetRun.
+// Passed SLEs must be non-const for WetRun.
+#define DryRun RunType<bool, true>()
+#define WetRun RunType<bool, false>()
+template <class T, T V>
+struct RunType
+{
+    //see:
+    //http://alumni.media.mit.edu/~rahimi/compile-time-flags/
+    constexpr operator T() const 
+    { 
+        static_assert(std::is_same<bool, T>::value);
+        return V;
+    }
+
+    constexpr T operator!() const
+    {
+        static_assert(std::is_same<bool, T>::value);
+        return !(V);
+    }
+};
+
+// Return true iff the acc side of line is in default state
+bool isTrustDefault(
+    std::shared_ptr<SLE> const& acc,    // side to check
+    std::shared_ptr<SLE> const& line);  // line to check
+
+/** Lock or unlock a TrustLine balance.
+    If positive deltaAmt lock the amount.
+    If negative deltaAmt unlock the amount.
+*/
+template<class V, class S, class R>
 [[nodiscard]] TER
 trustAdjustLockedBalance(
     V& view,
     S& sleLine,
     STAmount const& deltaAmt,
-    bool dryRun)
+    R dryRun)
 {
 
     static_assert(
@@ -323,7 +425,7 @@ trustAdjustLockedBalance(
     // dry runs are explicit in code, but really the view type determines
     // what occurs here, so this combination is invalid.
 
-    assert(!(std::is_same<V, ReadView const>::value && !dryRun));
+    static_assert(!(std::is_same<V, ReadView const>::value && !dryRun));
 
     if (!view.rules().enabled(featurePaychanAndEscrowForTokens))
         return tefINTERNAL;
@@ -373,7 +475,16 @@ trustAdjustLockedBalance(
     lockedBalance += deltaAmt;
 
     if (lockedBalance > balance)
+    {
+        std::cout
+            << "trustAdjustLockedBalance: "
+            << "lockedBalance("
+            << lockedBalance
+            << ") > balance("
+            << balance
+            << ") = true\n";
         return tecUNFUNDED_PAYMENT;
+    }
 
     if (lockedBalance < beast::zero)
         return tecINTERNAL;
@@ -397,11 +508,14 @@ trustAdjustLockedBalance(
 }
 
 
-// Check if movement of a particular token between 1 or more accounts
+/** Check if movement of a particular token between 1 or more accounts
-// (including unlocking) is forbidden by any flag or condition.
+    Read only, does not change any ledger object.
-// If parties contains 1 entry then noRipple is not a bar to xfer.
+    May be called with ApplyView or ReadView.
-// Part of featurePaychanAndEscrowForTokens, but can be callled without guard
+    (including unlocking) is forbidden by any flag or condition.
-
+    If parties contains 1 entry then noRipple is not a bar to xfer.
+    If parties contains more than 1 entry then any party with noRipple on issuer
+    side is a bar to xfer.
+*/
 template<class V>
 [[nodiscard]]TER
 trustTransferAllowed(
@@ -441,6 +555,9 @@ trustTransferAllowed(
 
     for (AccountID const& p: parties)
     {
+        if (p == issue.account)
+            continue;
+
         auto const line = view.read(keylet::line(p, issue.account, issue.currency));
         if (!line)
         {
@@ -529,7 +646,11 @@ trustTransferAllowed(
     return tesSUCCESS;
 }
 
-template <class V, class S>
+/** Transfer a locked balance from one TL to an unlocked balance on another
+    or create a line at the destination if the actingAcc has permission to.
+    Used for resolving payment instruments that use locked TL balances.
+*/
+template <class V, class S, class R>
 [[nodiscard]] TER
 trustTransferLockedBalance(
     V& view,
@@ -538,23 +659,23 @@ trustTransferLockedBalance(
     S& sleDstAcc,
     STAmount const& amount,     // issuer, currency are in this field
     beast::Journal const& j,
-    bool dryRun)
+    R dryRun)
 {
     
     typedef typename std::conditional<
-        std::is_same<V, ApplyView>::value,
+        std::is_same<V, ApplyView>::value && !dryRun,
         std::shared_ptr<SLE>,
         std::shared_ptr<SLE const>>::type SLEPtr;
 
     auto peek = [&](Keylet& k)
     {
-        if constexpr (std::is_same<V, ApplyView>::value)
+        if constexpr (std::is_same<V, ApplyView>::value && !dryRun)
             return const_cast<ApplyView&>(view).peek(k);
         else
             return view.read(k);
     };
 
-    assert(!(std::is_same<V, ApplyView>::value && !dryRun));
+    static_assert(std::is_same<V, ApplyView>::value || dryRun);
 
     if (!view.rules().enabled(featurePaychanAndEscrowForTokens))
         return tefINTERNAL;
@@ -582,10 +703,16 @@ trustTransferLockedBalance(
     bool dstHigh = dstAccID > issuerAccID;
 
     // check for freezing, auth, no ripple and TL sanity
-    if (TER result =
+    {
+        TER result =
             trustTransferAllowed(view, {srcAccID, dstAccID}, {currency, issuerAccID});
-            result != tesSUCCESS)
+        std::cout
-        return result;
+            << "trustTransferLockedBalance: trustTransferAlowed result="
+            << result
+            << "\n";
+        if (!isTesSuccess(result))
+            return result;
+    }
 
     // ensure source line exists
     Keylet klSrcLine { keylet::line(srcAccID, issuerAccID, currency)};
@@ -632,12 +759,15 @@ trustTransferLockedBalance(
             return tecINTERNAL;
         }
 
-        sleSrcLine->setFieldAmount(sfBalance, srcHigh ? -finalBalance : finalBalance);
+        if constexpr(!dryRun)
+        {
+            sleSrcLine->setFieldAmount(sfBalance, srcHigh ? -finalBalance : finalBalance);
 
-        if (finalLockedBalance == beast::zero)
+            if (finalLockedBalance == beast::zero)
-            sleSrcLine->makeFieldAbsent(sfLockedBalance);
+                sleSrcLine->makeFieldAbsent(sfLockedBalance);
-        else
+            else
-            sleSrcLine->setFieldAmount(sfLockedBalance, srcHigh ? -finalLockedBalance : finalLockedBalance);
+                sleSrcLine->setFieldAmount(sfLockedBalance, srcHigh ? -finalLockedBalance : finalLockedBalance);
+        }
 
     }
 
@@ -674,31 +804,27 @@ trustTransferLockedBalance(
 
         // yes we can... we will 
 
-        if (!dryRun)
+        if constexpr(!dryRun)
         {
-            if constexpr(std::is_same<V, ApplyView>::value)
+            // clang-format off
+            if (TER const ter = trustCreate(
+                    view,
+                    !dstHigh,                       // is dest low?
+                    issuerAccID,                    // source
+                    dstAccID,                       // destination
+                    klDstLine.key,                  // ledger index
+                    sleDstAcc,                      // Account to add to
+                    false,                          // authorize account
+                    (sleDstAcc->getFlags() & lsfDefaultRipple) == 0,
+                    false,                          // freeze trust line
+                    flipDstAmt ? -dstAmt : dstAmt,  // initial balance
+                    Issue(currency, dstAccID),      // limit of zero
+                    0,                              // quality in
+                    0,                              // quality out
+                    j);                             // journal
+                !isTesSuccess(ter))
             {
-
+                return ter;
-                // clang-format off
-                if (TER const ter = trustCreate(
-                        view,
-                        !dstHigh,                       // is dest low?
-                        issuerAccID,                    // source
-                        dstAccID,                       // destination
-                        klDstLine.key,                  // ledger index
-                        sleDstAcc,                      // Account to add to
-                        false,                          // authorize account
-                        (sleDstAcc->getFlags() & lsfDefaultRipple) == 0,
-                        false,                          // freeze trust line
-                        flipDstAmt ? -dstAmt : dstAmt,  // initial balance
-                        Issue(currency, dstAccID),      // limit of zero
-                        0,                              // quality in
-                        0,                              // quality out
-                        j);                             // journal
-                    !isTesSuccess(ter))
-                {
-                    return ter;
-                }
             }
         }
         // clang-format on
@@ -731,102 +857,35 @@ trustTransferLockedBalance(
             return tecPATH_DRY;
         }
 
-        sleDstLine->setFieldAmount(sfBalance, dstHigh ? -finalBalance : finalBalance);
+        if constexpr(!dryRun)
+            sleDstLine->setFieldAmount(sfBalance, dstHigh ? -finalBalance : finalBalance);
     }
 
-    if (dryRun)
+    if constexpr (!dryRun)
-        return tesSUCCESS;
-
-    static_assert(std::is_same<V, ApplyView>::value);
-
-    // check if source line ended up in default state and adjust owner count if it did
-    if (isTrustDefault(sleSrcAcc, sleSrcLine))
     {
-        uint32_t flags = sleSrcLine->getFieldU32(sfFlags);
+        static_assert(std::is_same<V, ApplyView>::value);
-        uint32_t fReserve { srcHigh ? lsfHighReserve : lsfLowReserve };
+
-        if (flags & fReserve)
+        // check if source line ended up in default state and adjust owner count if it did
+        if (isTrustDefault(sleSrcAcc, sleSrcLine))
         {
-            sleSrcLine->setFieldU32(sfFlags, flags & ~fReserve);
+            uint32_t flags = sleSrcLine->getFieldU32(sfFlags);
-            if (!dryRun)
+            uint32_t fReserve { srcHigh ? lsfHighReserve : lsfLowReserve };
+            if (flags & fReserve)
             {
+                sleSrcLine->setFieldU32(sfFlags, flags & ~fReserve);
                 adjustOwnerCount(view, sleSrcAcc, -1, j); 
                 view.update(sleSrcAcc);
             }
         }
-    }
 
-    view.update(sleSrcLine);
+        view.update(sleSrcLine);
-    
+        
-    if (sleDstLine)
-    {
         // a destination line already existed and was updated
-        view.update(sleDstLine);
+        if (sleDstLine)
+            view.update(sleDstLine);
     }
-
     return tesSUCCESS;
 }
-/** Delete an offer.
-
-    Requirements:
-        The passed `sle` be obtained from a prior
-        call to view.peek()
-*/
-// [[nodiscard]] // nodiscard commented out so Flow, BookTip and others compile.
-TER
-offerDelete(ApplyView& view, std::shared_ptr<SLE> const& sle, beast::Journal j);
-
-//------------------------------------------------------------------------------
-
-//
-// Money Transfers
-//
-
-// Direct send w/o fees:
-// - Redeeming IOUs and/or sending sender's own IOUs.
-// - Create trust line of needed.
-// --> bCheckIssuer : normally require issuer to be involved.
-// [[nodiscard]] // nodiscard commented out so DirectStep.cpp compiles.
-TER
-rippleCredit(
-    ApplyView& view,
-    AccountID const& uSenderID,
-    AccountID const& uReceiverID,
-    const STAmount& saAmount,
-    bool bCheckIssuer,
-    beast::Journal j);
-
-[[nodiscard]] TER
-accountSend(
-    ApplyView& view,
-    AccountID const& from,
-    AccountID const& to,
-    const STAmount& saAmount,
-    beast::Journal j);
-
-[[nodiscard]] TER
-issueIOU(
-    ApplyView& view,
-    AccountID const& account,
-    STAmount const& amount,
-    Issue const& issue,
-    beast::Journal j);
-
-[[nodiscard]] TER
-redeemIOU(
-    ApplyView& view,
-    AccountID const& account,
-    STAmount const& amount,
-    Issue const& issue,
-    beast::Journal j);
-
-[[nodiscard]] TER
-transferXRP(
-    ApplyView& view,
-    AccountID const& from,
-    AccountID const& to,
-    STAmount const& amount,
-    beast::Journal j);
-
 }  // namespace ripple
 
 #endif