From 49b1da8a09ec73997689a96244d71766922e4d99 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Thu, 11 Apr 2013 13:56:44 -0700 Subject: [PATCH 1/8] Add destination account. --- src/cpp/ripple/RPCHandler.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cpp/ripple/RPCHandler.cpp b/src/cpp/ripple/RPCHandler.cpp index a649e11848..52462a39e2 100644 --- a/src/cpp/ripple/RPCHandler.cpp +++ b/src/cpp/ripple/RPCHandler.cpp @@ -1242,6 +1242,7 @@ Json::Value RPCHandler::doRipplePathFind(Json::Value jvRequest, int& cost) jvDestCur.append(STAmount::createHumanCurrency(uCurrency)); jvResult["destination_currencies"] = jvDestCur; + jvResult["destination_account"] = raDst.humanAccountID(); Json::Value jvArray(Json::arrayValue); From 2e39dfc15a4c4eb0d97f49f9ce82eb0ea5720f4c Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 12 Apr 2013 16:03:57 -0700 Subject: [PATCH 2/8] Don't explore paths if we don't have enough path length left to use them. --- src/cpp/ripple/Pathfinder.cpp | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/cpp/ripple/Pathfinder.cpp b/src/cpp/ripple/Pathfinder.cpp index d58d951dab..f3134e9047 100644 --- a/src/cpp/ripple/Pathfinder.cpp +++ b/src/cpp/ripple/Pathfinder.cpp @@ -369,7 +369,9 @@ bool Pathfinder::findPaths(const unsigned int iMaxSteps, const unsigned int iMax continue; } - else if (!speEnd.mCurrencyID) + bool isLast = (spPath.mPath.size() == (iMaxSteps - 1)); + + if (!speEnd.mCurrencyID) { // XXX Might restrict the number of times bridging through XRP. @@ -381,7 +383,10 @@ bool Pathfinder::findPaths(const unsigned int iMaxSteps, const unsigned int iMax // New end is an order book with the currency and issuer. if (!spPath.hasSeen(ACCOUNT_XRP, book->getCurrencyOut(), book->getIssuerOut()) && - !matchesOrigin(book->getCurrencyOut(), book->getIssuerOut())) + !matchesOrigin(book->getCurrencyOut(), book->getIssuerOut()) && + (!isLast || + (book->getCurrencyOut() == mDstAmount.getCurrency() && + book->getIssuerOut() == mDstAccountID))) { // Not a order book already in path. STPath spNew(spPath); @@ -453,6 +458,10 @@ bool Pathfinder::findPaths(const unsigned int iMaxSteps, const unsigned int iMax % RippleAddress::createHumanAccountID(uPeerID) % STAmount::createHumanCurrency(speEnd.mCurrencyID)); } + else if (isLast && (!dstCurrency || (uPeerID != mDstAccountID))) + { + nothing(); + } else if (!rspEntry->getBalance().isPositive() // No IOUs to send. && (!rspEntry->getLimitPeer() // Peer does not extend credit. || -rspEntry->getBalance() >= rspEntry->getLimitPeer() // No credit left. @@ -522,7 +531,10 @@ bool Pathfinder::findPaths(const unsigned int iMaxSteps, const unsigned int iMax BOOST_FOREACH(OrderBook::ref book, books) { if (!spPath.hasSeen(ACCOUNT_XRP, book->getCurrencyOut(), book->getIssuerOut()) && - !matchesOrigin(book->getCurrencyOut(), book->getIssuerOut())) + !matchesOrigin(book->getCurrencyOut(), book->getIssuerOut()) && + (!isLast || + (book->getCurrencyOut() == mDstAmount.getCurrency() && + book->getIssuerOut() == mDstAccountID))) { // A book we haven't seen before. Add it. STPath spNew(spPath); From 0359ab5d462393b0d10acd6157757f86e22fa7a3 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 12 Apr 2013 16:11:51 -0700 Subject: [PATCH 3/8] Don't count forced issuer nodes after exchanges because we didn't have to find them. Drop the default maximum path length to four because exchanges to non-XRP aren't overcounted. --- rippled-example.cfg | 2 +- src/cpp/ripple/Pathfinder.cpp | 20 +++++++++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/rippled-example.cfg b/rippled-example.cfg index 2b04f763b3..638a383ac7 100644 --- a/rippled-example.cfg +++ b/rippled-example.cfg @@ -251,7 +251,7 @@ # When searching for paths, the maximum number of nodes allowed. This can take # exponentially more resources as the size is increased. # -# The default is: 5 +# The default is: 4 # # [rpc_startup] # Specify a list of RPC commands to run at startup. diff --git a/src/cpp/ripple/Pathfinder.cpp b/src/cpp/ripple/Pathfinder.cpp index f3134e9047..323d2d22ed 100644 --- a/src/cpp/ripple/Pathfinder.cpp +++ b/src/cpp/ripple/Pathfinder.cpp @@ -118,7 +118,7 @@ bool Pathfinder::bDefaultPath(const STPath& spPath) } typedef std::pair candidate_t; -bool candCmp(uint32 seq, const candidate_t& first, const candidate_t& second) +static bool candCmp(uint32 seq, const candidate_t& first, const candidate_t& second) { if (first.first < second.first) return false; @@ -127,6 +127,17 @@ bool candCmp(uint32 seq, const candidate_t& first, const candidate_t& second) return (first.first ^ seq) < (second.first ^ seq); } +static int getEffectiveLength(const STPath& spPath) +{ // don't count exchanges to non-XRP currencies twice (only count the forced issuer account node) + int length = 0; + for (std::vector::const_iterator it = spPath.begin(); it != spPath.end(); ++it) + { + if (it->isAccount() || it->getCurrency().isZero()) + ++length; + } + return length; +} + Pathfinder::Pathfinder(Ledger::ref ledger, const RippleAddress& uSrcAccountID, const RippleAddress& uDstAccountID, const uint160& uSrcCurrencyID, const uint160& uSrcIssuerID, const STAmount& saDstAmount, bool& bValid) @@ -360,7 +371,8 @@ bool Pathfinder::findPaths(const unsigned int iMaxSteps, const unsigned int iMax % STAmount::createHumanCurrency(speEnd.mCurrencyID) % RippleAddress::createHumanAccountID(speEnd.mIssuerID)); - if (spPath.mPath.size() >= iMaxSteps) + int length = getEffectiveLength(spPath.mPath); + if (length >= iMaxSteps) { // Path is at maximum size. Don't want to add more. @@ -369,12 +381,10 @@ bool Pathfinder::findPaths(const unsigned int iMaxSteps, const unsigned int iMax continue; } - bool isLast = (spPath.mPath.size() == (iMaxSteps - 1)); + bool isLast = (length == (iMaxSteps - 1)); if (!speEnd.mCurrencyID) { - // XXX Might restrict the number of times bridging through XRP. - // Cursor is for XRP, continue with qualifying books: XRP -> non-XRP std::vector xrpBooks; theApp->getOrderBookDB().getBooksByTakerPays(ACCOUNT_XRP, CURRENCY_XRP, xrpBooks); From 455c73ce4d707edd21b296a802a04eca379adacb Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 12 Apr 2013 18:35:49 -0700 Subject: [PATCH 4/8] Misuse of scoped lock. --- src/cpp/ripple/InstanceCounter.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpp/ripple/InstanceCounter.h b/src/cpp/ripple/InstanceCounter.h index 5a8fe1c612..b4c37c5ebb 100644 --- a/src/cpp/ripple/InstanceCounter.h +++ b/src/cpp/ripple/InstanceCounter.h @@ -82,7 +82,7 @@ public: } int getCount() { - boost::mutex::scoped_lock(mLock); + boost::mutex::scoped_lock sl(mLock); return mInstances; } const std::string& getName() From 3c3f9d3fea87fa2cebe6908b5cb43876dcf45c89 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 12 Apr 2013 18:36:03 -0700 Subject: [PATCH 5/8] Add a way to test if the master lock is held by someone else. --- src/cpp/ripple/Application.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/cpp/ripple/Application.h b/src/cpp/ripple/Application.h index 2a848ed7c5..c8492e02a4 100644 --- a/src/cpp/ripple/Application.h +++ b/src/cpp/ripple/Application.h @@ -149,6 +149,13 @@ public: void run(); void stop(); void sweep(); + +#ifdef DEBUG + void mustHaveMasterLock() { bool tl = mMasterLock.try_lock(); assert(tl); mMasterLock.unlock(); } +#else + void mustHaveMasterLock() { ; } +#endif + }; extern Application* theApp; From e5fdc99dd02bbb4a2d2a7c9fa112631fd4a3b0d1 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 12 Apr 2013 18:36:31 -0700 Subject: [PATCH 6/8] Grr! These are way too easy to do by mistake. The following code does nothing useful: ScopedLock(theApp->getMasterLock()); Must be: ScopedLock sl(theApp->getMasterLock()); --- src/cpp/ripple/NetworkOPs.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpp/ripple/NetworkOPs.cpp b/src/cpp/ripple/NetworkOPs.cpp index 769b48c6f9..41006ac865 100644 --- a/src/cpp/ripple/NetworkOPs.cpp +++ b/src/cpp/ripple/NetworkOPs.cpp @@ -600,7 +600,7 @@ void NetworkOPs::checkState(const boost::system::error_code& result) } { - ScopedLock(theApp->getMasterLock()); + ScopedLock sl(theApp->getMasterLock()); std::vector peerList = theApp->getConnectionPool().getPeerVector(); From bd06b22384e8f6a3e61b7ba63a6505c8ef78273f Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Sat, 13 Apr 2013 02:52:58 -0700 Subject: [PATCH 7/8] Use the fees that apply, not the transfer rate. --- src/cpp/ripple/RippleCalc.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpp/ripple/RippleCalc.cpp b/src/cpp/ripple/RippleCalc.cpp index 359454dbc8..d9dc9bb663 100644 --- a/src/cpp/ripple/RippleCalc.cpp +++ b/src/cpp/ripple/RippleCalc.cpp @@ -1440,7 +1440,7 @@ TER RippleCalc::calcNodeDeliverFwd( STAmount saOutFunded = std::min(saOfferFunds, saTakerGets); // Offer maximum out - If there are no out fees. STAmount saInFunded = STAmount::mulRound(saOutFunded, saOfrRate, saTakerPays, true); // Offer maximum in - Limited by by payout. - STAmount saInTotal = STAmount::mulRound(saInFunded, saInTransRate, true); // Offer maximum in with fees. + STAmount saInTotal = STAmount::mulRound(saInFunded, saInFeeRate, true); // Offer maximum in with fees. STAmount saInSum = std::min(saInTotal, saInReq-saInAct-saInFees); // In limited by remaining. STAmount saInPassAct = STAmount::divRound(saInSum, saInFeeRate, true); // In without fees. STAmount saOutPassMax = STAmount::divRound(saInPassAct, saOfrRate, saTakerGets, true); // Out limited by in remaining. From ef499920bae18ca0614e8fb3d0d92b634f56752b Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Sat, 13 Apr 2013 03:06:06 -0700 Subject: [PATCH 8/8] Remove old debug code. --- src/cpp/ripple/AmountRound.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/cpp/ripple/AmountRound.cpp b/src/cpp/ripple/AmountRound.cpp index e25eba9f19..3b77d26b8e 100644 --- a/src/cpp/ripple/AmountRound.cpp +++ b/src/cpp/ripple/AmountRound.cpp @@ -271,9 +271,6 @@ STAmount STAmount::divRound(const STAmount& num, const STAmount& den, --denOffset; } - cLog(lsINFO) << "num: " << numVal << " " << numOffset; - cLog(lsINFO) << "den: " << denVal << " " << denOffset; - bool resultNegative = num.mIsNegative != num.mIsNegative; // Compute (numerator * 10^17) / denominator CBigNum v;