Remove recursions from computeLiquidity

src/ripple/module/app/paths/Calculators.h

@@ -80,12 +80,10 @@ void pathNext (
 // Many of these routines use recursion to loop over all nodes in a path.
 // TODO(tom): replace this recursion with a loop.
 
-TER computeReverseLiqudity (
-    RippleCalc&,
-    unsigned int nodeIndex, PathState& pathState, bool bMultiQuality);
-TER computeForwardLiqudity (
-    RippleCalc&,
-    unsigned int nodeIndex, PathState& pathState, bool bMultiQuality);
+TER computeReverseLiquidity (
+    RippleCalc&, PathState& pathState, bool bMultiQuality);
+TER computeForwardLiquidity (
+    RippleCalc&, PathState& pathState, bool bMultiQuality);
 TER computeReverseLiquidityForOffer (
     RippleCalc&,
     unsigned int nodeIndex, PathState& pathState, bool bMultiQuality);

src/ripple/module/app/paths/ComputeLiquidity.cpp

@@ -27,32 +27,35 @@
 namespace ripple {
 namespace path {
 
-TER computeForwardLiqudity (
-    RippleCalc& rippleCalc,
-    const unsigned int nodeIndex, PathState& pathState,
-    const bool bMultiQuality)
+TER computeForwardLiquidity (
+    RippleCalc& rippleCalc, PathState& pathState, const bool bMultiQuality)
 {
-    auto const& node = pathState.nodes()[nodeIndex];
-    WriteLog (lsTRACE, RippleCalc)
-        << "computeForwardLiqudity> nodeIndex=" << nodeIndex;
+    TER resultCode;
+    for (auto nodeIndex = 0; nodeIndex < pathState.nodes().size(); ++nodeIndex)
+    {
+        auto const& node = pathState.nodes()[nodeIndex];
+        WriteLog (lsTRACE, RippleCalc)
+            << "computeForwardLiquidity> nodeIndex=" << nodeIndex;
 
-    TER resultCode = node.isAccount()
-        ? computeForwardLiquidityForAccount (
-            rippleCalc, nodeIndex, pathState, bMultiQuality)
-        : computeForwardLiquidityForOffer (
-              rippleCalc, nodeIndex, pathState, bMultiQuality);
+        resultCode = node.isAccount()
+            ? computeForwardLiquidityForAccount (
+                rippleCalc, nodeIndex, pathState, bMultiQuality)
+            : computeForwardLiquidityForOffer (
+                  rippleCalc, nodeIndex, pathState, bMultiQuality);
 
-    if (resultCode == tesSUCCESS && nodeIndex + 1 != pathState.nodes().size ())
-        resultCode = computeForwardLiqudity (rippleCalc, nodeIndex + 1, pathState, bMultiQuality);
+        bool success = (resultCode == tesSUCCESS);
 
-    if (resultCode == tesSUCCESS && !(pathState.inPass() && pathState.outPass()))
-        resultCode = tecPATH_DRY;
+        if (success && !(pathState.inPass() && pathState.outPass()))
+            resultCode = tecPATH_DRY;
 
-    WriteLog (lsTRACE, RippleCalc)
-        << "computeForwardLiqudity<"
-        << " nodeIndex:" << nodeIndex
-        << " resultCode:" << resultCode;
+        WriteLog (lsTRACE, RippleCalc)
+            << "computeForwardLiquidity<"
+            << " nodeIndex:" << nodeIndex
+            << " resultCode:" << resultCode;
 
+        if (!success)
+            return resultCode;
+    }
     return resultCode;
 }
 
@@ -65,7 +68,8 @@ TER computeForwardLiqudity (
 // the amount of liquidity available.
 //
 // This is just a controlling loop that sets things up and then hands the work
-// off to either computeReverseLiquidityForAccount or computeReverseLiquidityForOffer.
+// off to either computeReverseLiquidityForAccount or
+// computeReverseLiquidityForOffer.
 //
 // Later on the result of this will be used to work forward, figuring out how
 // much can actually be delivered.
@@ -77,49 +81,47 @@ TER computeForwardLiqudity (
 //     --> [all]saAccount
 //     <-> [0]saWanted.mAmount : --> limit, <-- actual
 
-TER computeReverseLiqudity (
+TER computeReverseLiquidity (
     RippleCalc& rippleCalc,
-    const unsigned int nodeIndex, PathState& pathState,
+    PathState& pathState,
     const bool bMultiQuality)
 {
-    auto& node = pathState.nodes()[nodeIndex];
-
-    // Every account has a transfer rate for its issuances.
-
-    // TOMOVE: The account charges
-    // a fee when third parties transfer that account's own issuances.
-
-    // node.transferRate_ caches the output transfer rate for this node.
-    node.transferRate_ = STAmount::saFromRate (
-        rippleCalc.mActiveLedger.rippleTransferRate (node.issuer_));
-
-    WriteLog (lsTRACE, RippleCalc)
-        << "computeReverseLiqudity>"
-        << " nodeIndex=" << nodeIndex
-        << " issuer_=" << to_string (node.issuer_)
-        << " transferRate_=" << node.transferRate_;
-
-    auto resultCode = node.isAccount()
-        ? computeReverseLiquidityForAccount (
-            rippleCalc, nodeIndex, pathState, bMultiQuality)
-        : computeReverseLiquidityForOffer (
-            rippleCalc, nodeIndex, pathState, bMultiQuality);
-
-    // Do previous.
-    if (resultCode == tesSUCCESS && nodeIndex)
+    for (int nodeIndex = pathState.nodes().size(); nodeIndex--; )
     {
-        // Continue in reverse.  TODO(tom): remove unnecessary recursion.
-        resultCode = computeReverseLiqudity (
-            rippleCalc, nodeIndex - 1, pathState, bMultiQuality);
+        auto& node = pathState.nodes()[nodeIndex];
+
+        // Every account has a transfer rate for its issuances.
+
+        // TOMOVE: The account charges
+        // a fee when third parties transfer that account's own issuances.
+
+        // node.transferRate_ caches the output transfer rate for this node.
+        node.transferRate_ = STAmount::saFromRate (
+            rippleCalc.mActiveLedger.rippleTransferRate (node.issuer_));
+
+        WriteLog (lsTRACE, RippleCalc)
+            << "computeReverseLiquidity>"
+            << " nodeIndex=" << nodeIndex
+            << " issuer_=" << to_string (node.issuer_)
+            << " transferRate_=" << node.transferRate_;
+
+        auto resultCode = node.isAccount()
+            ? computeReverseLiquidityForAccount (
+                rippleCalc, nodeIndex, pathState, bMultiQuality)
+            : computeReverseLiquidityForOffer (
+                rippleCalc, nodeIndex, pathState, bMultiQuality);
+
+        WriteLog (lsTRACE, RippleCalc)
+            << "computeReverseLiquidity< "
+            << "nodeIndex=" << nodeIndex
+            << " resultCode=%s" << transToken (resultCode)
+            << "/" << resultCode;
+
+        if (resultCode != tesSUCCESS)
+            return resultCode;
+
     }
-
-    WriteLog (lsTRACE, RippleCalc)
-        << "computeReverseLiqudity< "
-        << "nodeIndex=" << nodeIndex
-        << " resultCode=%s" << transToken (resultCode)
-        << "/" << resultCode;
-
-    return resultCode;
+    return tesSUCCESS;
 }
 
 } // path

src/ripple/module/app/paths/PathNext.cpp

@@ -42,7 +42,6 @@ void pathNext (
 {
     // The next state is what is available in preference order.
     // This is calculated when referenced accounts changed.
-    const unsigned int  lastNodeIndex = pathState.nodes().size () - 1;
     pathState.clear();
 
     WriteLog (lsTRACE, RippleCalc)
@@ -62,7 +61,8 @@ void pathNext (
         node.saFwdDeliver.clear ();
     }
 
-    pathState.setStatus(computeReverseLiqudity (rippleCalc, lastNodeIndex, pathState, bMultiQuality));
+    pathState.setStatus(computeReverseLiquidity (
+        rippleCalc, pathState, bMultiQuality));
 
     WriteLog (lsTRACE, RippleCalc)
         << "pathNext: Path after reverse: " << pathState.getJson ();
@@ -72,13 +72,14 @@ void pathNext (
         // Do forward.
         lesCurrent = lesCheckpoint.duplicate ();   // Restore from checkpoint.
 
-        pathState.setStatus(computeForwardLiqudity (rippleCalc, 0, pathState, bMultiQuality));
+        pathState.setStatus(computeForwardLiquidity (
+            rippleCalc, pathState, bMultiQuality));
     }
 
     if (tesSUCCESS == pathState.status())
     {
         CondLog (!pathState.inPass() || !pathState.outPass(), lsDEBUG, RippleCalc)
-            << "pathNext: Error computeForwardLiqudity reported success for nothing:"
+            << "pathNext: Error computeForwardLiquidity reported success for nothing:"
             << " saOutPass=" << pathState.outPass()
             << " inPass()=" << pathState.inPass();
 

src/ripple/module/app/paths/PathState.h

@@ -79,7 +79,7 @@ class PathState : public CountedObject <PathState>
     void setQuality (std::uint64_t q) { uQuality = q; }
 
     bool allLiquidityConsumed() const { return allLiquidityConsumed_; }
-    void consumeAllLiqudity () { allLiquidityConsumed_ = true; }
+    void consumeAllLiquidity () { allLiquidityConsumed_ = true; }
 
     void setIndex (int i) { mIndex  = i; }
     int index() const { return mIndex; }

src/ripple/module/app/paths/RippleCalc.cpp

@@ -58,27 +58,28 @@ TER rippleCalculate (
     //      XRP: xrpIssuer()
     //  non-XRP: uSrcAccountID (for any issuer) or another account with trust
     //           node.
-    const STAmount&     saMaxAmountReq,             // --> -1 = no limit.
+    STAmount const&     saMaxAmountReq,             // --> -1 = no limit.
 
     // Issuer:
     //      XRP: xrpIssuer()
     //  non-XRP: uDstAccountID (for any issuer) or another account with trust
     //           node.
-    const STAmount&     saDstAmountReq,
+    STAmount const&     saDstAmountReq,
 
     Account const&      uDstAccountID,
     Account const&      uSrcAccountID,
 
     // A set of paths that are included in the transaction that we'll explore
     // for liquidity.
-    const STPathSet&    spsPaths,
-    const bool          bPartialPayment,
-    const bool          bLimitQuality,
-    const bool          bNoRippleDirect,
-    const bool          bStandAlone,
+    STPathSet const&    spsPaths,
+
+    bool const bPartialPayment,
+    bool const bLimitQuality,
+    bool const bNoRippleDirect,
+    bool const bStandAlone,
 
     // True, not to delete unfundeds.
-    const bool          bOpenLedger)
+    bool const          bOpenLedger)
 {
     assert (activeLedger.isValid ());
     RippleCalc  rc (activeLedger, bOpenLedger);