From bfb69664ffae3c6f5fc7b6e07cd68b7a58010867 Mon Sep 17 00:00:00 2001 From: Arthur Britto Date: Tue, 26 Mar 2013 21:19:46 -0700 Subject: [PATCH] Major pathfinding fixes. --- src/cpp/ripple/Pathfinder.cpp | 40 ++++++++++++++++++++++++------- src/cpp/ripple/RPCHandler.cpp | 11 +++++---- src/cpp/ripple/RippleCalc.cpp | 45 +++++++++++++++++++++++++---------- src/cpp/ripple/RippleCalc.h | 4 ++-- src/js/network.js | 2 +- 5 files changed, 73 insertions(+), 29 deletions(-) diff --git a/src/cpp/ripple/Pathfinder.cpp b/src/cpp/ripple/Pathfinder.cpp index fd623ae7fb..ab829d2512 100644 --- a/src/cpp/ripple/Pathfinder.cpp +++ b/src/cpp/ripple/Pathfinder.cpp @@ -119,6 +119,10 @@ bool Pathfinder::bDefaultPath(const STPath& spPath) bool bDefault; LedgerEntrySet lesActive(mLedger, tapNONE); + cLog(lsTRACE) << boost::str(boost::format("bDefaultPath> mSrcAmount=%s mDstAmount=%s") + % mSrcAmount.getFullText() + % mDstAmount.getFullText()); + // Expand the current path. pspCurrent->setExpanded(lesActive, spPath, mDstAccountID, mSrcAccountID); @@ -126,8 +130,8 @@ bool Pathfinder::bDefaultPath(const STPath& spPath) // When path is a default (implied). Don't need to add it to return set. bDefault = pspCurrent->vpnNodes == mPsDefault->vpnNodes; - cLog(lsTRACE) << "findPaths: expanded path: " << pspCurrent->getJson(); - cLog(lsTRACE) << "findPaths: default path: indirect: " << spPath.getJson(0); + cLog(lsTRACE) << "bDefaultPath: expanded path: " << pspCurrent->getJson(); + cLog(lsTRACE) << "bDefaultPath: default path: indirect: " << spPath.getJson(0); return bDefault; } @@ -162,6 +166,10 @@ Pathfinder::Pathfinder(Ledger::ref ledger, LedgerEntrySet lesActive(mLedger, tapNONE); + cLog(lsTRACE) << boost::str(boost::format("Pathfinder> mSrcAmount=%s mDstAmount=%s") + % mSrcAmount.getFullText() + % mDstAmount.getFullText()); + psDefault->setExpanded(lesActive, STPath(), mDstAccountID, mSrcAccountID); if (tesSUCCESS == psDefault->terStatus) @@ -296,6 +304,12 @@ bool Pathfinder::findPaths(const unsigned int iMaxSteps, const unsigned int iMax if (sLog(lsTRACE)) { + cLog(lsTRACE) << boost::str(boost::format("findPaths: spe: %s/%s: %s amt: %s") + % RippleAddress::createHumanAccountID(speEnd.mAccountID) + % RippleAddress::createHumanAccountID(speEnd.mIssuerID) + % RippleAddress::createHumanAccountID(mDstAccountID) + % RippleAddress::createHumanAccountID(mDstAmount.getIssuer())); + cLog(lsTRACE) << "findPaths: finish? account: " << (speEnd.mAccountID == mDstAccountID); cLog(lsTRACE) << "findPaths: finish? currency: " << (speEnd.mCurrencyID == mDstAmount.getCurrency()); cLog(lsTRACE) << "findPaths: finish? issuer: " @@ -406,7 +420,7 @@ bool Pathfinder::findPaths(const unsigned int iMaxSteps, const unsigned int iMax SLE::pointer sleEnd = lesActive.entryCache(ltACCOUNT_ROOT, Ledger::getAccountRootIndex(speEnd.mAccountID)); tLog(!sleEnd, lsDEBUG) - << boost::str(boost::format("findPaths: order book: %s/%s : ") + << boost::str(boost::format("findPaths: tail: %s/%s : ") % RippleAddress::createHumanAccountID(speEnd.mAccountID) % RippleAddress::createHumanAccountID(speEnd.mIssuerID)); @@ -443,11 +457,14 @@ bool Pathfinder::findPaths(const unsigned int iMaxSteps, const unsigned int iMax { // Path has no credit left. Ignore it. cLog(lsTRACE) << - boost::str(boost::format("findPaths: No credit: %s/%s -> %s/%s") + boost::str(boost::format("findPaths: No credit: %s/%s -> %s/%s balance=%s limit=%s") % RippleAddress::createHumanAccountID(speEnd.mAccountID) % STAmount::createHumanCurrency(speEnd.mCurrencyID) % RippleAddress::createHumanAccountID(uPeerID) - % STAmount::createHumanCurrency(speEnd.mCurrencyID)); + % STAmount::createHumanCurrency(speEnd.mCurrencyID) + % rspEntry->getBalance().getFullText() + % rspEntry->getLimitPeer().getFullText() + ); } else { @@ -461,11 +478,14 @@ bool Pathfinder::findPaths(const unsigned int iMaxSteps, const unsigned int iMax bContinued = true; cLog(lsTRACE) << - boost::str(boost::format("findPaths: push explore: %s/%s -> %s/%s") + boost::str(boost::format("findPaths: push explore: %s/%s -> %s/%s balance=%s limit=%s limit_peer=%s") % STAmount::createHumanCurrency(speEnd.mCurrencyID) % RippleAddress::createHumanAccountID(speEnd.mAccountID) % STAmount::createHumanCurrency(speEnd.mCurrencyID) - % RippleAddress::createHumanAccountID(uPeerID)); + % RippleAddress::createHumanAccountID(uPeerID) + % rspEntry->getBalance().getFullText() + % rspEntry->getLimit().getFullText() + % rspEntry->getLimitPeer().getFullText()); } } } @@ -473,7 +493,7 @@ bool Pathfinder::findPaths(const unsigned int iMaxSteps, const unsigned int iMax // Every book that wants the source currency. std::vector books; - // XXX Flip argument order to norm. + // XXX Need to flip getBooks argument order to be in normal order: currency then issuer. theApp->getOrderBookDB().getBooks(speEnd.mIssuerID, speEnd.mCurrencyID, books); BOOST_FOREACH(OrderBook::ref book, books) @@ -533,8 +553,10 @@ bool Pathfinder::findPaths(const unsigned int iMaxSteps, const unsigned int iMax TER terResult; try { + LedgerEntrySet lesSandbox(lesActive.duplicate()); + terResult = RippleCalc::rippleCalc( - lesActive, + lesSandbox, saMaxAmountAct, saDstAmountAct, vpsExpanded, diff --git a/src/cpp/ripple/RPCHandler.cpp b/src/cpp/ripple/RPCHandler.cpp index 7fdf59e00d..f4b7bb3588 100644 --- a/src/cpp/ripple/RPCHandler.cpp +++ b/src/cpp/ripple/RPCHandler.cpp @@ -1234,7 +1234,6 @@ Json::Value RPCHandler::doRipplePathFind(Json::Value jvRequest, int& cost) cost = rpcCOST_EXPENSIVE; Ledger::pointer lSnapShot = boost::make_shared(boost::ref(*lpLedger), false); - LedgerEntrySet lesSnapshot(lSnapShot, tapNONE); ScopedUnlock su(theApp->getMasterLock()); // As long as we have a locked copy of the ledger, we can unlock. @@ -1242,6 +1241,7 @@ Json::Value RPCHandler::doRipplePathFind(Json::Value jvRequest, int& cost) for (unsigned int i=0; i != jvSrcCurrencies.size(); ++i) { Json::Value jvSource = jvSrcCurrencies[i]; + uint160 uSrcCurrencyID; uint160 uSrcIssuerID; @@ -1276,7 +1276,7 @@ Json::Value RPCHandler::doRipplePathFind(Json::Value jvRequest, int& cost) if (!pf.findPaths(theConfig.PATH_SEARCH_SIZE, 3, spsComputed)) { - cLog(lsDEBUG) << "ripple_path_find: No paths found."; + cLog(lsWARNING) << "ripple_path_find: No paths found."; } else { @@ -1293,9 +1293,11 @@ Json::Value RPCHandler::doRipplePathFind(Json::Value jvRequest, int& cost) 1); saMaxAmount.negate(); + LedgerEntrySet lesSandbox(lSnapShot, tapNONE); + TER terResult = RippleCalc::rippleCalc( - lesSnapshot, + lesSandbox, saMaxAmountAct, // <-- saDstAmountAct, // <-- vpsExpanded, // <-- @@ -1313,8 +1315,7 @@ Json::Value RPCHandler::doRipplePathFind(Json::Value jvRequest, int& cost) // cLog(lsDEBUG) << "ripple_path_find: PATHS IN: " << spsComputed.size() << " : " << spsComputed.getJson(0); // cLog(lsDEBUG) << "ripple_path_find: PATHS EXP: " << vpsExpanded.size(); - - cLog(lsDEBUG) + cLog(lsWARNING) << boost::str(boost::format("ripple_path_find: saMaxAmount=%s saDstAmount=%s saMaxAmountAct=%s saDstAmountAct=%s") % saMaxAmount % saDstAmount diff --git a/src/cpp/ripple/RippleCalc.cpp b/src/cpp/ripple/RippleCalc.cpp index bf94a47839..a8fe832076 100644 --- a/src/cpp/ripple/RippleCalc.cpp +++ b/src/cpp/ripple/RippleCalc.cpp @@ -250,9 +250,15 @@ TER PathState::pushNode( if (tesSUCCESS == terResult) { STAmount saOwed = lesEntries.rippleOwed(pnCur.uAccountID, pnBck.uAccountID, pnCur.uCurrencyID); + STAmount saLimit; - if (!saOwed.isPositive() && -saOwed >= lesEntries.rippleLimit(pnCur.uAccountID, pnBck.uAccountID, pnCur.uCurrencyID)) + if (!saOwed.isPositive() + && -saOwed >= (saLimit = lesEntries.rippleLimit(pnCur.uAccountID, pnBck.uAccountID, pnCur.uCurrencyID))) { + cLog(lsWARNING) << boost::str(boost::format("pushNode: dry: saOwed=%s saLimit=%s") + % saOwed.getFullText() + % saLimit.getFullText()); + terResult = tecPATH_DRY; } } @@ -323,6 +329,8 @@ void PathState::setExpanded( const uint160 uOutIssuerID = saOutReq.getIssuer(); const uint160 uSenderIssuerID = !!uMaxCurrencyID ? uSenderID : ACCOUNT_XRP; // Sender is always issuer for non-XRP. + // cLog(lsDEBUG) << boost::str(boost::format("setExpanded>")); + lesEntries = lesSource.duplicate(); terStatus = tesSUCCESS; @@ -344,7 +352,7 @@ void PathState::setExpanded( uMaxCurrencyID, // Max specifes the currency. uSenderIssuerID); - cLog(lsDEBUG) << boost::str(boost::format("PathState: pushed: account=%s currency=%s issuer=%s") + cLog(lsDEBUG) << boost::str(boost::format("setExpanded: pushed: account=%s currency=%s issuer=%s") % RippleAddress::createHumanAccountID(uSenderID) % STAmount::createHumanCurrency(uMaxCurrencyID) % RippleAddress::createHumanAccountID(uSenderIssuerID)); @@ -366,8 +374,10 @@ void PathState::setExpanded( : uOutIssuerID // Use implied node. : ACCOUNT_XRP; - cLog(lsDEBUG) << boost::str(boost::format("PathState: implied check: uNxtCurrencyID=%s uNxtAccountID=%s") - % RippleAddress::createHumanAccountID(uNxtCurrencyID) + cLog(lsDEBUG) << boost::str(boost::format("setExpanded: implied check: uMaxIssuerID=%s uSenderIssuerID=%s uNxtCurrencyID=%s uNxtAccountID=%s") + % RippleAddress::createHumanAccountID(uMaxIssuerID) + % RippleAddress::createHumanAccountID(uSenderIssuerID) + % STAmount::createHumanCurrency(uNxtCurrencyID) % RippleAddress::createHumanAccountID(uNxtAccountID)); // Can't just use push implied, because it can't compensate for next account. @@ -375,9 +385,9 @@ void PathState::setExpanded( || uMaxCurrencyID != uNxtCurrencyID // Next is different currency, offer next... || uMaxIssuerID != uNxtAccountID) // Next is not implied issuer { - cLog(lsDEBUG) << boost::str(boost::format("PathState: sender implied: account=%s currency=%s issuer=%s") + cLog(lsDEBUG) << boost::str(boost::format("setExpanded: sender implied: account=%s currency=%s issuer=%s") % RippleAddress::createHumanAccountID(uMaxIssuerID) - % RippleAddress::createHumanAccountID(uMaxCurrencyID) + % STAmount::createHumanCurrency(uMaxCurrencyID) % RippleAddress::createHumanAccountID(uMaxIssuerID)); // Add account implied by SendMax. terStatus = pushNode( @@ -394,7 +404,7 @@ void PathState::setExpanded( { if (tesSUCCESS == terStatus) { - cLog(lsDEBUG) << boost::str(boost::format("PathState: element in path:")); + cLog(lsDEBUG) << boost::str(boost::format("setExpanded: element in path:")); terStatus = pushNode(speElement.getNodeType(), speElement.getAccountID(), speElement.getCurrency(), speElement.getIssuerID()); } } @@ -408,9 +418,9 @@ void PathState::setExpanded( || pnPrv.uAccountID != uOutIssuerID)) // Need the implied issuer. { // Add implied account. - cLog(lsDEBUG) << boost::str(boost::format("PathState: receiver implied: account=%s currency=%s issuer=%s") + cLog(lsDEBUG) << boost::str(boost::format("setExpanded: receiver implied: account=%s currency=%s issuer=%s") % RippleAddress::createHumanAccountID(uOutIssuerID) - % RippleAddress::createHumanAccountID(uOutCurrencyID) + % STAmount::createHumanCurrency(uOutCurrencyID) % RippleAddress::createHumanAccountID(uOutIssuerID)); terStatus = pushNode( !!uOutCurrencyID @@ -449,7 +459,7 @@ void PathState::setExpanded( if (!umForward.insert(std::make_pair(boost::make_tuple(pnCur.uAccountID, pnCur.uCurrencyID, pnCur.uIssuerID), uNode)).second) { // Failed to insert. Have a loop. - cLog(lsDEBUG) << boost::str(boost::format("PathState: loop detected: %s") + cLog(lsDEBUG) << boost::str(boost::format("setExpanded: loop detected: %s") % getJson()); terStatus = temBAD_PATH_LOOP; @@ -457,7 +467,7 @@ void PathState::setExpanded( } } - cLog(lsDEBUG) << boost::str(boost::format("PathState: in=%s/%s out=%s/%s %s") + cLog(lsDEBUG) << boost::str(boost::format("setExpanded: in=%s/%s out=%s/%s %s") % STAmount::createHumanCurrency(uMaxCurrencyID) % RippleAddress::createHumanAccountID(uMaxIssuerID) % STAmount::createHumanCurrency(uOutCurrencyID) @@ -2612,6 +2622,10 @@ TER RippleCalc::rippleCalc( { RippleCalc rc(lesActive, bOpenLedger); + cLog(lsTRACE) << boost::str(boost::format("rippleCalc> saMaxAmountReq=%s saDstAmountReq=%s") + % saMaxAmountReq.getFullText() + % saDstAmountReq.getFullText()); + TER terResult = temUNCERTAIN; // YYY Might do basic checks on src and dst validity as per doPayment. @@ -2669,6 +2683,12 @@ int iIndex = 0; if (!pspExpanded) return temUNKNOWN; + cLog(lsTRACE) << boost::str(boost::format("rippleCalc: EXPAND: saDstAmountReq=%s saMaxAmountReq=%s uDstAccountID=%s uSrcAccountID=%s") + % saDstAmountReq.getFullText() + % saMaxAmountReq.getFullText() + % RippleAddress::createHumanAccountID(uDstAccountID) + % RippleAddress::createHumanAccountID(uSrcAccountID)); + pspExpanded->setExpanded(lesActive, spPath, uDstAccountID, uSrcAccountID); cLog(lsDEBUG) << boost::str(boost::format("rippleCalc: Build path: %d: status: %s") @@ -2764,6 +2784,7 @@ int iPass = 0; } } } + if (sLog(lsDEBUG)) { cLog(lsDEBUG) << boost::str(boost::format("rippleCalc: Summary: Pass: %d Dry: %d Paths: %d") % ++iPass % iDry % vpsExpanded.size()); @@ -3003,7 +3024,7 @@ void TransactionEngine::calcOfferBridgeNext( { // Offer must be redeeming IOUs. - // No additional + // No additional // XXX Broken } diff --git a/src/cpp/ripple/RippleCalc.h b/src/cpp/ripple/RippleCalc.h index 8df0eebba9..c23f2be837 100644 --- a/src/cpp/ripple/RippleCalc.h +++ b/src/cpp/ripple/RippleCalc.h @@ -209,8 +209,8 @@ public: const bool bPartialPayment, const bool bLimitQuality, const bool bNoRippleDirect, - const bool bStandAlone, - const bool bOpenLedger = true + const bool bStandAlone, // --> True, not to affect accounts. + const bool bOpenLedger = true // --> What kind of errors to return. ); static void setCanonical(STPathSet& spsDst, const std::vector& vpsExpanded, bool bKeepDefault); diff --git a/src/js/network.js b/src/js/network.js index 714aa335f1..3f7697a995 100644 --- a/src/js/network.js +++ b/src/js/network.js @@ -48,7 +48,7 @@ Network.protocol.start = function () { }; -// Target state: disconnectted +// Target state: disconnect Network.protocol.stop = function () { };