From a96dee85d2c6936e7a8a10e263becd04feb22c16 Mon Sep 17 00:00:00 2001 From: Tom Ritchford Date: Wed, 25 Jun 2014 11:51:50 -0400 Subject: [PATCH] Remove recursions from computeLiquidity --- src/ripple/module/app/paths/Calculators.h | 10 +- .../module/app/paths/ComputeLiquidity.cpp | 120 +++++++++--------- src/ripple/module/app/paths/PathNext.cpp | 9 +- src/ripple/module/app/paths/PathState.h | 2 +- src/ripple/module/app/paths/RippleCalc.cpp | 17 +-- 5 files changed, 80 insertions(+), 78 deletions(-) diff --git a/src/ripple/module/app/paths/Calculators.h b/src/ripple/module/app/paths/Calculators.h index a7d5e3c72..43a51019a 100644 --- a/src/ripple/module/app/paths/Calculators.h +++ b/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); diff --git a/src/ripple/module/app/paths/ComputeLiquidity.cpp b/src/ripple/module/app/paths/ComputeLiquidity.cpp index 0dc42db6f..7e5aba364 100644 --- a/src/ripple/module/app/paths/ComputeLiquidity.cpp +++ b/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 diff --git a/src/ripple/module/app/paths/PathNext.cpp b/src/ripple/module/app/paths/PathNext.cpp index 5c6abd25b..7fcb112ef 100644 --- a/src/ripple/module/app/paths/PathNext.cpp +++ b/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(); diff --git a/src/ripple/module/app/paths/PathState.h b/src/ripple/module/app/paths/PathState.h index 04a2d3eec..829c2539d 100644 --- a/src/ripple/module/app/paths/PathState.h +++ b/src/ripple/module/app/paths/PathState.h @@ -79,7 +79,7 @@ class PathState : public CountedObject 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; } diff --git a/src/ripple/module/app/paths/RippleCalc.cpp b/src/ripple/module/app/paths/RippleCalc.cpp index d00f5783f..3af788397 100644 --- a/src/ripple/module/app/paths/RippleCalc.cpp +++ b/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);