From 3d777f3f5d9187474cc505b85cfc27ed8e622a5f Mon Sep 17 00:00:00 2001 From: Miguel Portilla Date: Wed, 29 Jul 2015 15:29:18 -0400 Subject: [PATCH] Convert all of an asset (RIPD-655) --- src/ripple/app/paths/FindPaths.cpp | 46 +- src/ripple/app/paths/FindPaths.h | 28 +- src/ripple/app/paths/PathRequest.cpp | 470 ++++++++++-------- src/ripple/app/paths/PathRequest.h | 13 +- src/ripple/app/paths/PathState.cpp | 8 +- src/ripple/app/paths/PathState.h | 6 - src/ripple/app/paths/Pathfinder.cpp | 196 ++++---- src/ripple/app/paths/Pathfinder.h | 20 +- src/ripple/app/paths/RippleCalc.cpp | 2 +- .../app/paths/cursor/DeliverNodeForward.cpp | 2 +- .../app/paths/cursor/DeliverNodeReverse.cpp | 4 +- .../cursor/ReverseLiquidityForAccount.cpp | 8 +- src/ripple/app/tests/Path_test.cpp | 468 ++++++++++------- src/ripple/ledger/impl/PaymentSandbox.cpp | 2 +- src/ripple/protocol/ErrorCodes.h | 2 + src/ripple/protocol/JsonFields.h | 2 + src/ripple/protocol/STAmount.h | 7 +- src/ripple/protocol/STPathSet.h | 6 + src/ripple/protocol/impl/STAmount.cpp | 6 +- src/ripple/rpc/RipplePathFind.h | 3 +- src/ripple/rpc/handlers/RipplePathFind.cpp | 107 +++- src/ripple/rpc/impl/TransactionSign.cpp | 32 +- src/ripple/rpc/impl/TransactionSign.h | 5 +- src/ripple/test/jtx/impl/paths.cpp | 7 +- 24 files changed, 838 insertions(+), 612 deletions(-) diff --git a/src/ripple/app/paths/FindPaths.cpp b/src/ripple/app/paths/FindPaths.cpp index 18b0c1af39..ff800f93ca 100644 --- a/src/ripple/app/paths/FindPaths.cpp +++ b/src/ripple/app/paths/FindPaths.cpp @@ -30,31 +30,32 @@ public: AccountID const& srcAccount, AccountID const& dstAccount, STAmount const& dstAmount, + boost::optional const& srcAmount, int searchLevel, unsigned int maxPaths) : cache_ (cache), srcAccount_ (srcAccount), dstAccount_ (dstAccount), dstAmount_ (dstAmount), + srcAmount_ (srcAmount), searchLevel_ (searchLevel), maxPaths_ (maxPaths) { } - - bool findPathsForIssue ( + boost::optional + findPathsForIssue ( Issue const& issue, - STPathSet& pathsInOut, + STPathSet const& paths, STPath& fullLiquidityPath) { if (auto& pathfinder = getPathFinder (issue.currency)) { - pathsInOut = pathfinder->getBestPaths ( - maxPaths_, fullLiquidityPath, pathsInOut, issue.account); - return true; + return pathfinder->getBestPaths (maxPaths_, + fullLiquidityPath, paths, issue.account); } assert (false); - return false; + return boost::none; } private: @@ -64,6 +65,7 @@ private: AccountID const srcAccount_; AccountID const dstAccount_; STAmount const dstAmount_; + boost::optional const srcAmount_; int const searchLevel_; unsigned int const maxPaths_; @@ -73,7 +75,8 @@ private: if (i != currencyMap_.end ()) return i->second; auto pathfinder = std::make_unique ( - cache_, srcAccount_, dstAccount_, currency, dstAmount_); + cache_, srcAccount_, dstAccount_, currency, + boost::none, dstAmount_, srcAmount_); if (pathfinder->findPaths (searchLevel_)) pathfinder->computePathRanks (maxPaths_); else @@ -90,24 +93,28 @@ FindPaths::FindPaths ( AccountID const& srcAccount, AccountID const& dstAccount, STAmount const& dstAmount, + boost::optional const& srcAmount, int level, unsigned int maxPaths) : impl_ (std::make_unique ( - cache, srcAccount, dstAccount, dstAmount, level, maxPaths)) + cache, srcAccount, dstAccount, + dstAmount, srcAmount, level, maxPaths)) { } FindPaths::~FindPaths() = default; -bool FindPaths::findPathsForIssue ( +boost::optional +FindPaths::findPathsForIssue ( Issue const& issue, - STPathSet& pathsInOut, + STPathSet const& paths, STPath& fullLiquidityPath) { - return impl_->findPathsForIssue (issue, pathsInOut, fullLiquidityPath); + return impl_->findPathsForIssue (issue, paths, fullLiquidityPath); } -bool findPathsForOneIssuer ( +boost::optional +findPathsForOneIssuer ( RippleLineCache::ref cache, AccountID const& srcAccount, AccountID const& dstAccount, @@ -115,7 +122,7 @@ bool findPathsForOneIssuer ( STAmount const& dstAmount, int searchLevel, unsigned int const maxPaths, - STPathSet& pathsInOut, + STPathSet const& paths, STPath& fullLiquidityPath) { Pathfinder pf ( @@ -124,14 +131,15 @@ bool findPathsForOneIssuer ( dstAccount, srcIssue.currency, srcIssue.account, - dstAmount); + dstAmount, + boost::none); - if (!pf.findPaths (searchLevel)) - return false; + if (! pf.findPaths(searchLevel)) + return boost::none; pf.computePathRanks (maxPaths); - pathsInOut = pf.getBestPaths(maxPaths, fullLiquidityPath, pathsInOut, srcIssue.account); - return true; + return pf.getBestPaths(maxPaths, fullLiquidityPath, + paths, srcIssue.account); } void initializePathfinding () diff --git a/src/ripple/app/paths/FindPaths.h b/src/ripple/app/paths/FindPaths.h index bb3b6dfefd..f7dddda34f 100644 --- a/src/ripple/app/paths/FindPaths.h +++ b/src/ripple/app/paths/FindPaths.h @@ -32,6 +32,7 @@ public: AccountID const& srcAccount, AccountID const& dstAccount, STAmount const& dstAmount, + boost::optional const& srcAmount, /** searchLevel is the maximum search level allowed in an output path. */ int searchLevel, @@ -40,15 +41,15 @@ public: unsigned int const maxPaths); ~FindPaths(); - bool findPathsForIssue ( + /** The return value will have any additional paths found. Only + non-default paths without source or destination will be added. */ + boost::optional + findPathsForIssue ( Issue const& issue, - /** On input, pathsInOut contains any paths you want to ensure are - included if still good. - - On output, pathsInOut will have any additional paths found. Only - non-default paths without source or destination will be added. */ - STPathSet& pathsInOut, + /** Contains any paths you want to ensure are + included if still good. */ + STPathSet const& paths, /** On input, fullLiquidityPath must be an empty STPath. @@ -61,7 +62,10 @@ private: std::unique_ptr impl_; }; -bool findPathsForOneIssuer ( +/** The return value will have any additional paths found. Only + non-default paths without source or destination will be added. */ +boost::optional +findPathsForOneIssuer ( RippleLineCache::ref cache, AccountID const& srcAccount, AccountID const& dstAccount, @@ -75,12 +79,10 @@ bool findPathsForOneIssuer ( pathsOut. */ unsigned int const maxPaths, - /** On input, pathsInOut contains any paths you want to ensure are included if + /** Contains any paths you want to ensure are included if still good. - - On output, pathsInOut will have any additional paths found. Only - non-default paths without source or destination will be added. */ - STPathSet& pathsInOut, + */ + STPathSet const& paths, /** On input, fullLiquidityPath must be an empty STPath. diff --git a/src/ripple/app/paths/PathRequest.cpp b/src/ripple/app/paths/PathRequest.cpp index c77c039a0e..150f1c427e 100644 --- a/src/ripple/app/paths/PathRequest.cpp +++ b/src/ripple/app/paths/PathRequest.cpp @@ -19,7 +19,6 @@ #include #include -#include #include #include #include @@ -46,7 +45,6 @@ PathRequest::PathRequest ( , mOwner (owner) , wpSubscriber (subscriber) , jvStatus (Json::objectValue) - , bValid (false) , mLastIndex (0) , mInProgress (false) , iLastLevel (0) @@ -67,7 +65,6 @@ PathRequest::PathRequest ( , mOwner (owner) , fCompletion (completion) , jvStatus (Json::objectValue) - , bValid (false) , mLastIndex (0) , mInProgress (false) , iLastLevel (0) @@ -114,12 +111,6 @@ PathRequest::~PathRequest() " total:" << get_milli_diff(ptCreated) << "ms"; } -bool PathRequest::isValid () -{ - ScopedLockType sl (mLock); - return bValid; -} - bool PathRequest::isNew () { ScopedLockType sl (mIndexLock); @@ -175,70 +166,66 @@ void PathRequest::updateComplete () bool PathRequest::isValid (RippleLineCache::ref crCache) { ScopedLockType sl (mLock); - bValid = raSrcAccount && raDstAccount && - saDstAmount > zero; - auto const& lrLedger = crCache->getLedger (); + if (! raSrcAccount || ! raDstAccount) + return false; - if (bValid) + if (! convert_all_ && (saSendMax || saDstAmount <= zero)) { - if (! crCache->getLedger()->exists( - keylet::account(*raSrcAccount))) - { - // no source account - bValid = false; - jvStatus = rpcError (rpcSRC_ACT_NOT_FOUND); - } + // If send max specified, dst amt must be -1. + jvStatus = rpcError(rpcDST_AMT_MALFORMED); + return false; } - if (bValid) + if (! crCache->getLedger()->exists( + keylet::account(*raSrcAccount))) { - auto const sleDest = crCache->getLedger()->read( - keylet::account(*raDstAccount)); - - Json::Value& jvDestCur = - (jvStatus[jss::destination_currencies] = Json::arrayValue); - - if (!sleDest) - { - // no destination account - jvDestCur.append (Json::Value ("XRP")); - - if (!saDstAmount.native ()) - { - // only XRP can be send to a non-existent account - bValid = false; - jvStatus = rpcError (rpcACT_NOT_FOUND); - } - else if (saDstAmount < STAmount (lrLedger->fees().accountReserve (0))) - { - // payment must meet reserve - bValid = false; - jvStatus = rpcError (rpcDST_AMT_MALFORMED); - } - } - else - { - bool const disallowXRP ( - sleDest->getFlags() & lsfDisallowXRP); - - auto usDestCurrID = accountDestCurrencies ( - *raDstAccount, crCache, !disallowXRP); - - for (auto const& currency : usDestCurrID) - jvDestCur.append (to_string (currency)); - - jvStatus["destination_tag"] = - (sleDest->getFlags () & lsfRequireDestTag) - != 0; - } + // Source account does not exist. + jvStatus = rpcError (rpcSRC_ACT_NOT_FOUND); + return false; } - if (bValid) + auto const& lrLedger = crCache->getLedger(); + auto const sleDest = crCache->getLedger()->read( + keylet::account(*raDstAccount)); + + Json::Value& jvDestCur = + (jvStatus[jss::destination_currencies] = Json::arrayValue); + + if (! sleDest) { - jvStatus[jss::ledger_hash] = to_string (lrLedger->info().hash); - jvStatus[jss::ledger_index] = lrLedger->seq(); + jvDestCur.append (Json::Value (systemCurrencyCode())); + if (! saDstAmount.native ()) + { + // Only XRP can be send to a non-existent account. + jvStatus = rpcError (rpcACT_NOT_FOUND); + return false; + } + + if (! convert_all_ && saDstAmount < + STAmount (lrLedger->fees().accountReserve (0))) + { + // Payment must meet reserve. + jvStatus = rpcError (rpcDST_AMT_MALFORMED); + return false; + } } - return bValid; + else + { + bool const disallowXRP ( + sleDest->getFlags() & lsfDisallowXRP); + + auto usDestCurrID = accountDestCurrencies ( + *raDstAccount, crCache, ! disallowXRP); + + for (auto const& currency : usDestCurrID) + jvDestCur.append (to_string (currency)); + jvStatus[jss::destination_tag] = + (sleDest->getFlags() & lsfRequireDestTag); + } + + jvStatus[jss::ledger_hash] = to_string (lrLedger->info().hash); + jvStatus[jss::ledger_index] = lrLedger->seq(); + return true; } Json::Value PathRequest::doCreate ( @@ -246,27 +233,22 @@ Json::Value PathRequest::doCreate ( Json::Value const& value, bool& valid) { - Json::Value status; - if (parseJson (value, true) != PFR_PJ_INVALID) + if (parseJson (value) != PFR_PJ_INVALID) { - bValid = isValid (cache); - - if (bValid) - status = doUpdate (cache, true); - else - status = jvStatus; + valid = isValid (cache); + status = valid ? doUpdate(cache, true) : jvStatus; } else { - bValid = false; + valid = false; status = jvStatus; } if (m_journal.debug) { - if (bValid) + if (valid) { m_journal.debug << iIdentifier << " valid: " << toBase58(*raSrcAccount); @@ -279,63 +261,85 @@ Json::Value PathRequest::doCreate ( } } - valid = bValid; return status; } -int PathRequest::parseJson (Json::Value const& jvParams, bool complete) +int PathRequest::parseJson (Json::Value const& jvParams) { - int ret = PFR_PJ_NOCHANGE; - - if (jvParams.isMember (jss::source_account)) + if (! jvParams.isMember(jss::source_account)) { - raSrcAccount = parseBase58( - jvParams[jss::source_account].asString()); - if (! raSrcAccount) - { - jvStatus = rpcError (rpcSRC_ACT_MALFORMED); - return PFR_PJ_INVALID; - } - } - else if (complete) - { - jvStatus = rpcError (rpcSRC_ACT_MISSING); + jvStatus = rpcError(rpcSRC_ACT_MISSING); return PFR_PJ_INVALID; } - if (jvParams.isMember (jss::destination_account)) + if (! jvParams.isMember(jss::destination_account)) { - raDstAccount = parseBase58( - jvParams[jss::destination_account].asString()); - if (! raDstAccount) - { - jvStatus = rpcError (rpcDST_ACT_MALFORMED); - return PFR_PJ_INVALID; - } - } - else if (complete) - { - jvStatus = rpcError (rpcDST_ACT_MISSING); + jvStatus = rpcError(rpcDST_ACT_MISSING); return PFR_PJ_INVALID; } - if (jvParams.isMember (jss::destination_amount)) + if (! jvParams.isMember(jss::destination_amount)) { - if (! amountFromJsonNoThrow ( - saDstAmount, jvParams[jss::destination_amount]) || - (saDstAmount.getCurrency ().isZero () && - saDstAmount.getIssuer ().isNonZero ()) || - (saDstAmount.getCurrency () == badCurrency ()) || - saDstAmount <= zero) + jvStatus = rpcError(rpcDST_AMT_MISSING); + return PFR_PJ_INVALID; + } + + raSrcAccount = parseBase58( + jvParams[jss::source_account].asString()); + if (! raSrcAccount) + { + jvStatus = rpcError (rpcSRC_ACT_MALFORMED); + return PFR_PJ_INVALID; + } + + raDstAccount = parseBase58( + jvParams[jss::destination_account].asString()); + if (! raDstAccount) + { + jvStatus = rpcError (rpcDST_ACT_MALFORMED); + return PFR_PJ_INVALID; + } + + if (! amountFromJsonNoThrow ( + saDstAmount, jvParams[jss::destination_amount])) + { + jvStatus = rpcError (rpcDST_AMT_MALFORMED); + return PFR_PJ_INVALID; + } + + convert_all_ = + saDstAmount == STAmount(saDstAmount.issue(), 1u, 0, true); + + if ((saDstAmount.getCurrency ().isZero () && + saDstAmount.getIssuer ().isNonZero ()) || + (saDstAmount.getCurrency () == badCurrency ()) || + (! convert_all_ && saDstAmount <= zero)) + { + jvStatus = rpcError (rpcDST_AMT_MALFORMED); + return PFR_PJ_INVALID; + } + + if (jvParams.isMember(jss::send_max)) + { + // Send_max requires destination amount to be -1. + if (! convert_all_) { - jvStatus = rpcError (rpcDST_AMT_MALFORMED); + jvStatus = rpcError(rpcDST_AMT_MALFORMED); + return PFR_PJ_INVALID; + } + + saSendMax.emplace(); + if (! amountFromJsonNoThrow( + *saSendMax, jvParams[jss::send_max]) || + (saSendMax->getCurrency().isZero() && + saSendMax->getIssuer().isNonZero()) || + (saSendMax->getCurrency() == badCurrency()) || + (*saSendMax <= zero && + *saSendMax != STAmount(saSendMax->issue(), 1u, 0, true))) + { + jvStatus = rpcError(rpcSENDMAX_MALFORMED); return PFR_PJ_INVALID; } - } - else if (complete) - { - jvStatus = rpcError (rpcDST_ACT_MISSING); - return PFR_PJ_INVALID; } if (jvParams.isMember (jss::source_currencies)) @@ -356,8 +360,9 @@ int PathRequest::parseJson (Json::Value const& jvParams, bool complete) Currency uCur; AccountID uIss; - if (!jvCur.isObject() || !jvCur.isMember (jss::currency) || - !to_currency (uCur, jvCur[jss::currency].asString ())) + if (! jvCur.isObject() || + ! jvCur.isMember (jss::currency) || + ! to_currency (uCur, jvCur[jss::currency].asString ())) { jvStatus = rpcError (rpcSRC_CUR_MALFORMED); return PFR_PJ_INVALID; @@ -376,19 +381,46 @@ int PathRequest::parseJson (Json::Value const& jvParams, bool complete) } if (uCur.isNonZero() && uIss.isZero()) - { uIss = *raSrcAccount; - } - sciSourceCurrencies.insert ({uCur, uIss}); + if (saSendMax) + { + // If the currencies don't match, ignore the source currency. + if (uCur == saSendMax->getCurrency()) + { + // If neither is the source and they are not equal, then the + // source issuer is illegal. + if (uIss != *raSrcAccount && + saSendMax->getIssuer() != *raSrcAccount && + uIss != saSendMax->getIssuer()) + { + jvStatus = rpcError (rpcSRC_ISR_MALFORMED); + return PFR_PJ_INVALID; + } + + // If both are the source, use the source. + // Otherwise, use the one that's not the source. + if (uIss != *raSrcAccount) + sciSourceCurrencies.insert({uCur, uIss}); + else if (saSendMax->getIssuer() != *raSrcAccount) + sciSourceCurrencies.insert({uCur, saSendMax->getIssuer()}); + else + sciSourceCurrencies.insert({uCur, *raSrcAccount}); + } + } + else + { + sciSourceCurrencies.insert({uCur, uIss}); + } } } if (jvParams.isMember ("id")) jvId = jvParams["id"]; - return ret; + return PFR_PJ_NOCHANGE; } + Json::Value PathRequest::doClose (Json::Value const&) { m_journal.debug << iIdentifier << " closed"; @@ -408,6 +440,94 @@ void PathRequest::resetLevel (int l) iLastLevel = l; } +bool +PathRequest::findPaths ( + RippleLineCache::ref cache, + FindPaths& fp, Issue const& issue, + Json::Value& jvArray) +{ + STPath fullLiquidityPath; + auto result = fp.findPathsForIssue(issue, + mContext[issue], fullLiquidityPath); + if (! result) + { + m_journal.debug << iIdentifier << " No paths found"; + return false; + } + mContext[issue] = *result; + + boost::optional sandbox; + sandbox.emplace(&*cache->getLedger(), tapNONE); + + auto& sourceAccount = ! isXRP(issue.account) + ? issue.account + : isXRP(issue.currency) + ? xrpAccount() + : *raSrcAccount; + + STAmount saMaxAmount = saSendMax.value_or( + STAmount({issue.currency, sourceAccount}, 1u, 0, true)); + + m_journal.debug << iIdentifier + << " Paths found, calling rippleCalc"; + + auto rc = path::RippleCalc::rippleCalculate( + *sandbox, saMaxAmount, + convert_all_ ? STAmount(saDstAmount.issue(), STAmount::cMaxValue, + STAmount::cMaxOffset) : saDstAmount, + *raDstAccount, *raSrcAccount, *result); + + if (! convert_all_ && + ! fullLiquidityPath.empty() && + (rc.result() == terNO_LINE || rc.result() == tecPATH_PARTIAL)) + { + m_journal.debug << iIdentifier + << " Trying with an extra path element"; + result->push_back(fullLiquidityPath); + sandbox.emplace(&*cache->getLedger(), tapNONE); + + rc = path::RippleCalc::rippleCalculate( + *sandbox, saMaxAmount, saDstAmount, + *raDstAccount, *raSrcAccount, *result); + if (rc.result() != tesSUCCESS) + { + m_journal.warning << iIdentifier + << " Failed with covering path " + << transHuman(rc.result()); + } + else + { + m_journal.debug << iIdentifier + << " Extra path element gives " + << transHuman(rc.result()); + } + } + + if (rc.result () == tesSUCCESS) + { + Json::Value jvEntry (Json::objectValue); + rc.actualAmountIn.setIssuer (sourceAccount); + jvEntry[jss::source_amount] = rc.actualAmountIn.getJson (0); + jvEntry[jss::paths_computed] = result->getJson(0); + + if (convert_all_) + jvEntry[jss::destination_amount] = rc.actualAmountOut.getJson(0); + + if (hasCompletion ()) + { + // Old ripple_path_find API requires this + jvEntry[jss::paths_canonical] = Json::arrayValue; + } + + jvArray.append (jvEntry); + return true; + } + + m_journal.debug << iIdentifier << " rippleCalc returns " + << transHuman (rc.result ()); + return false; +} + Json::Value PathRequest::doUpdate (RippleLineCache::ref cache, bool fast) { m_journal.debug << iIdentifier << " update " << (fast ? "fast" : "normal"); @@ -454,8 +574,6 @@ Json::Value PathRequest::doUpdate (RippleLineCache::ref cache, bool fast) if (jvId) jvStatus["id"] = jvId; - Json::Value jvArray = Json::arrayValue; - int iLevel = iLastLevel; bool loaded = getApp().getFeeTrack().isLoadedLocal(); @@ -493,108 +611,24 @@ Json::Value PathRequest::doUpdate (RippleLineCache::ref cache, bool fast) m_journal.debug << iIdentifier << " processing at level " << iLevel; bool found = false; - + Json::Value jvArray = Json::arrayValue; FindPaths fp ( cache, *raSrcAccount, *raDstAccount, - saDstAmount, + convert_all_ ? STAmount(saDstAmount.issue(), STAmount::cMaxValue, + STAmount::cMaxOffset) : saDstAmount, + saSendMax, iLevel, 4); // iMaxPaths - for (auto const& currIssuer: sourceCurrencies) + for (auto const& i: sourceCurrencies) { - { - STAmount test (currIssuer, 1); - if (m_journal.debug) - { - m_journal.debug - << iIdentifier - << " Trying to find paths: " << test.getFullText (); - } - } - STPathSet& spsPaths = mContext[currIssuer]; - STPath fullLiquidityPath; - auto valid = fp.findPathsForIssue ( - currIssuer, - spsPaths, - fullLiquidityPath); - CondLog (!valid, lsDEBUG, PathRequest) - << iIdentifier << " PF request not valid"; - - if (valid) - { - boost::optional sandbox; - sandbox.emplace(&*cache->getLedger(), tapNONE); - - auto& sourceAccount = !isXRP (currIssuer.account) - ? currIssuer.account - : isXRP (currIssuer.currency) - ? xrpAccount() - : *raSrcAccount; - STAmount saMaxAmount ({currIssuer.currency, sourceAccount}, 1); - - saMaxAmount.negate (); - m_journal.debug << iIdentifier - << " Paths found, calling rippleCalc"; - auto rc = path::RippleCalc::rippleCalculate ( - *sandbox, - saMaxAmount, - saDstAmount, - *raDstAccount, - *raSrcAccount, - spsPaths); - - if (!fullLiquidityPath.empty() && - (rc.result () == terNO_LINE || rc.result () == tecPATH_PARTIAL)) - { - m_journal.debug - << iIdentifier << " Trying with an extra path element"; - spsPaths.push_back (fullLiquidityPath); - sandbox.emplace(&*cache->getLedger(), tapNONE); - rc = path::RippleCalc::rippleCalculate ( - *sandbox, - saMaxAmount, - saDstAmount, - *raDstAccount, - *raSrcAccount, - spsPaths); - if (rc.result () != tesSUCCESS) - m_journal.warning - << iIdentifier << " Failed with covering path " - << transHuman (rc.result ()); - else - m_journal.debug - << iIdentifier << " Extra path element gives " - << transHuman (rc.result ()); - } - - if (rc.result () == tesSUCCESS) - { - Json::Value jvEntry (Json::objectValue); - rc.actualAmountIn.setIssuer (sourceAccount); - - jvEntry[jss::source_amount] = rc.actualAmountIn.getJson (0); - jvEntry[jss::paths_computed] = spsPaths.getJson (0); - - if (hasCompletion ()) - { - // Old ripple_path_find API requires this - jvEntry[jss::paths_canonical] = Json::arrayValue; - } - - found = true; - jvArray.append (jvEntry); - } - else - { - m_journal.debug << iIdentifier << " rippleCalc returns " - << transHuman (rc.result ()); - } - } - else - { - m_journal.debug << iIdentifier << " No paths found"; - } + JLOG(m_journal.debug) + << iIdentifier + << " Trying to find paths: " + << STAmount(i, 1).getFullText(); + if (findPaths(cache, fp, i, jvArray)) + found = true; } iLastLevel = iLevel; diff --git a/src/ripple/app/paths/PathRequest.h b/src/ripple/app/paths/PathRequest.h index 6342d37983..20c77a8f07 100644 --- a/src/ripple/app/paths/PathRequest.h +++ b/src/ripple/app/paths/PathRequest.h @@ -21,6 +21,7 @@ #define RIPPLE_APP_PATHS_PATHREQUEST_H_INCLUDED #include +#include #include #include #include @@ -70,7 +71,6 @@ public: ~PathRequest (); - bool isValid (); bool isNew (); bool needsUpdate (bool newOnly, LedgerIndex index); void updateComplete (); @@ -92,7 +92,13 @@ private: bool isValid (RippleLineCache::ref crCache); void setValid (); void resetLevel (int level); - int parseJson (Json::Value const&, bool complete); + + bool + findPaths (RippleLineCache::ref, + FindPaths&, Issue const&, + Json::Value& jvArray); + + int parseJson (Json::Value const&); beast::Journal m_journal; @@ -112,11 +118,12 @@ private: boost::optional raSrcAccount; boost::optional raDstAccount; STAmount saDstAmount; + boost::optional saSendMax; std::set sciSourceCurrencies; std::map mContext; - bool bValid; + bool convert_all_; LockType mIndexLock; LedgerIndex mLastIndex; diff --git a/src/ripple/app/paths/PathState.cpp b/src/ripple/app/paths/PathState.cpp index cc58d00e07..5792bcd144 100644 --- a/src/ripple/app/paths/PathState.cpp +++ b/src/ripple/app/paths/PathState.cpp @@ -36,7 +36,6 @@ class RippleCalc; // for logging void PathState::clear() { - allLiquidityConsumed_ = false; saInPass = saInReq.zeroed(); saOutPass = saOutReq.zeroed(); unfundedOffers_.clear (); @@ -68,7 +67,7 @@ void PathState::reset(STAmount const& in, STAmount const& out) << " saOutAct=" << outAct() << " saOutReq=" << outReq(); - assert (outAct() < outReq()); + assert(outAct() < outReq()); assert (nodes().size () >= 2); } @@ -323,7 +322,8 @@ TER PathState::pushNode ( node.issue_.currency); STAmount saLimit; - if (saOwed <= zero) { + if (saOwed <= zero) + { saLimit = creditLimit (view(), node.account_, backNode.account_, @@ -361,7 +361,7 @@ TER PathState::pushNode ( else node.issue_.account = backNode.issue_.account; - node.saRateMax = saZero; + node.saRateMax = STAmount::saZero; node.saRevDeliver = STAmount (node.issue_); node.saFwdDeliver = node.saRevDeliver; diff --git a/src/ripple/app/paths/PathState.h b/src/ripple/app/paths/PathState.h index 839bb91b48..952f07aa8d 100644 --- a/src/ripple/app/paths/PathState.h +++ b/src/ripple/app/paths/PathState.h @@ -91,9 +91,6 @@ class PathState : public CountedObject std::uint64_t quality() const { return uQuality; } void setQuality (std::uint64_t q) { uQuality = q; } - bool allLiquidityConsumed() const { return allLiquidityConsumed_; } - void consumeAllLiquidity () { allLiquidityConsumed_ = true; } - void setIndex (int i) { mIndex = i; } int index() const { return mIndex; } @@ -153,9 +150,6 @@ private: STAmount saOutAct; // --> Amount actually sent so far. STAmount saOutPass; // <-- Amount actually sent. - // If true, all liquidity on this path has been consumed. - bool allLiquidityConsumed_ = false; - TER terStatus; path::Node::List nodes_; diff --git a/src/ripple/app/paths/Pathfinder.cpp b/src/ripple/app/paths/Pathfinder.cpp index 5462cdb2b1..9985da3245 100644 --- a/src/ripple/app/paths/Pathfinder.cpp +++ b/src/ripple/app/paths/Pathfinder.cpp @@ -66,32 +66,6 @@ namespace ripple { namespace { -// We sort possible paths by: -// cost of path -// length of path -// width of path - -// Compare two PathRanks. A better PathRank is lower, so the best are sorted to -// the beginning. -bool comparePathRank ( - Pathfinder::PathRank const& a, Pathfinder::PathRank const& b) -{ - // 1) Higher quality (lower cost) is better - if (a.quality != b.quality) - return a.quality < b.quality; - - // 2) More liquidity (higher volume) is better - if (a.liquidity != b.liquidity) - return a.liquidity > b.liquidity; - - // 3) Shorter paths are better - if (a.length != b.length) - return a.length < b.length; - - // 4) Tie breaker - return a.index > b.index; -} - struct AccountCandidate { int priority; @@ -172,8 +146,9 @@ Pathfinder::Pathfinder ( AccountID const& uSrcAccount, AccountID const& uDstAccount, Currency const& uSrcCurrency, - AccountID const& uSrcIssuer, - STAmount const& saDstAmount) + boost::optional const& uSrcIssuer, + STAmount const& saDstAmount, + boost::optional const& srcAmount) : mSrcAccount (uSrcAccount), mDstAccount (uDstAccount), mEffectiveDst (isXRP(saDstAmount.getIssuer ()) ? @@ -181,33 +156,15 @@ Pathfinder::Pathfinder ( mDstAmount (saDstAmount), mSrcCurrency (uSrcCurrency), mSrcIssuer (uSrcIssuer), - mSrcAmount ({uSrcCurrency, uSrcIssuer}, 1u, 0, true), - mLedger (cache->getLedger ()), - mRLCache (cache) -{ - assert (isXRP(uSrcCurrency) == isXRP(uSrcIssuer)); -} - -Pathfinder::Pathfinder ( - RippleLineCache::ref cache, - AccountID const& uSrcAccount, - AccountID const& uDstAccount, - Currency const& uSrcCurrency, - STAmount const& saDstAmount) - : mSrcAccount (uSrcAccount), - mDstAccount (uDstAccount), - mEffectiveDst (isXRP(saDstAmount.getIssuer ()) ? - uDstAccount : saDstAmount.getIssuer ()), - mDstAmount (saDstAmount), - mSrcCurrency (uSrcCurrency), - mSrcAmount ( - { - uSrcCurrency, - isXRP (uSrcCurrency) ? xrpAccount () : uSrcAccount - }, 1u, 0, true), + mSrcAmount (srcAmount.value_or(STAmount({uSrcCurrency, + uSrcIssuer.value_or(isXRP(uSrcCurrency) ? + xrpAccount() : uSrcAccount)}, 1u, 0, true))), + convert_all_ (mDstAmount == + STAmount(mDstAmount.issue(), STAmount::cMaxValue, STAmount::cMaxOffset)), mLedger (cache->getLedger ()), mRLCache (cache) { + assert (! uSrcIssuer || isXRP(uSrcCurrency) == isXRP(uSrcIssuer.get())); } Pathfinder::~Pathfinder() @@ -383,6 +340,9 @@ TER Pathfinder::getPathLiquidity ( try { // Compute a path that provides at least the minimum liquidity. + if (convert_all_) + rcInput.partialPaymentAllowed = true; + auto rc = path::RippleCalc::rippleCalculate ( sandbox, mSrcAmount, @@ -391,7 +351,6 @@ TER Pathfinder::getPathLiquidity ( mSrcAccount, pathSet, &rcInput); - // If we can't get even the minimum liquidity requested, we're done. if (rc.result () != tesSUCCESS) return rc.result (); @@ -399,20 +358,23 @@ TER Pathfinder::getPathLiquidity ( qualityOut = getRate (rc.actualAmountOut, rc.actualAmountIn); amountOut = rc.actualAmountOut; - // Now try to compute the remaining liquidity. - rcInput.partialPaymentAllowed = true; - rc = path::RippleCalc::rippleCalculate ( - sandbox, - mSrcAmount, - mDstAmount - amountOut, - mDstAccount, - mSrcAccount, - pathSet, - &rcInput); + if (! convert_all_) + { + // Now try to compute the remaining liquidity. + rcInput.partialPaymentAllowed = true; + rc = path::RippleCalc::rippleCalculate ( + sandbox, + mSrcAmount, + mDstAmount - amountOut, + mDstAccount, + mSrcAccount, + pathSet, + &rcInput); - // If we found further liquidity, add it into the result. - if (rc.result () == tesSUCCESS) - amountOut += rc.actualAmountOut; + // If we found further liquidity, add it into the result. + if (rc.result () == tesSUCCESS) + amountOut += rc.actualAmountOut; + } return tesSUCCESS; } @@ -438,7 +400,10 @@ STAmount smallestUsefulAmount (STAmount const& amount, int maxPaths) void Pathfinder::computePathRanks (int maxPaths) { - mRemainingAmount = mDstAmount; + mRemainingAmount = convert_all_ ? + STAmount(mDstAmount.issue(), STAmount::cMaxValue, + STAmount::cMaxOffset) + : mDstAmount; // Must subtract liquidity in default path from remaining amount. try @@ -450,7 +415,7 @@ void Pathfinder::computePathRanks (int maxPaths) auto rc = path::RippleCalc::rippleCalculate ( sandbox, mSrcAmount, - mDstAmount, + mRemainingAmount, mDstAccount, mSrcAccount, STPathSet(), @@ -514,47 +479,81 @@ void Pathfinder::rankPaths ( STPathSet const& paths, std::vector & rankedPaths) { - rankedPaths.clear (); rankedPaths.reserve (paths.size()); - // Ignore paths that move only very small amounts. - auto saMinDstAmount = smallestUsefulAmount (mDstAmount, maxPaths); + STAmount saMinDstAmount; + if (convert_all_) + { + // On convert_all_ partialPaymentAllowed will be set to true + // and requiring a huge amount will find the highest liquidity. + saMinDstAmount = STAmount(mDstAmount.issue(), + STAmount::cMaxValue, STAmount::cMaxOffset); + } + else + { + // Ignore paths that move only very small amounts. + saMinDstAmount = smallestUsefulAmount(mDstAmount, maxPaths); + } for (int i = 0; i < paths.size (); ++i) { auto const& currentPath = paths[i]; - STAmount liquidity; - uint64_t uQuality; - - if (currentPath.empty ()) - continue; - - auto const resultCode = getPathLiquidity ( - currentPath, saMinDstAmount, liquidity, uQuality); - - if (resultCode != tesSUCCESS) + if (! currentPath.empty()) { - WriteLog (lsDEBUG, Pathfinder) << - "findPaths: dropping : " << transToken (resultCode) << - ": " << currentPath.getJson (0); - } - else - { - WriteLog (lsDEBUG, Pathfinder) << - "findPaths: quality: " << uQuality << - ": " << currentPath.getJson (0); + STAmount liquidity; + uint64_t uQuality; + auto const resultCode = getPathLiquidity ( + currentPath, saMinDstAmount, liquidity, uQuality); + if (resultCode != tesSUCCESS) + { + WriteLog (lsDEBUG, Pathfinder) << + "findPaths: dropping : " << + transToken (resultCode) << + ": " << currentPath.getJson (0); + } + else + { + WriteLog (lsDEBUG, Pathfinder) << + "findPaths: quality: " << uQuality << + ": " << currentPath.getJson (0); - rankedPaths.push_back ({uQuality, currentPath.size (), liquidity, i}); + rankedPaths.push_back ({uQuality, + currentPath.size (), liquidity, i}); + } } } - std::sort (rankedPaths.begin (), rankedPaths.end (), comparePathRank); + + // Sort paths by: + // cost of path (when considering quality) + // width of path + // length of path + // A better PathRank is lower, best are sorted to the beginning. + std::sort(rankedPaths.begin(), rankedPaths.end(), + [&](Pathfinder::PathRank const& a, Pathfinder::PathRank const& b) + { + // 1) Higher quality (lower cost) is better + if (! convert_all_ && a.quality != b.quality) + return a.quality < b.quality; + + // 2) More liquidity (higher volume) is better + if (a.liquidity != b.liquidity) + return a.liquidity > b.liquidity; + + // 3) Shorter paths are better + if (a.length != b.length) + return a.length < b.length; + + // 4) Tie breaker + return a.index > b.index; + }); } -STPathSet Pathfinder::getBestPaths ( +STPathSet +Pathfinder::getBestPaths ( int maxPaths, STPath& fullLiquidityPath, - STPathSet& extraPaths, + STPathSet const& extraPaths, AccountID const& srcIssuer) { WriteLog (lsDEBUG, Pathfinder) << "findPaths: " << @@ -607,7 +606,7 @@ STPathSet Pathfinder::getBestPaths ( auto& pathRank = usePath ? *pathsIterator : *extraPathsIterator; - auto& path = usePath ? mCompletePaths[pathRank.index] : + auto const& path = usePath ? mCompletePaths[pathRank.index] : extraPaths[pathRank.index]; if (useExtraPath) @@ -631,13 +630,12 @@ STPathSet Pathfinder::getBestPaths ( if (! issuerIsSender && usePath) { // Need to make sure path matches issuer constraints - - if (path.front ().getAccountID() != srcIssuer) - continue; - if (isDefaultPath (path)) + if (isDefaultPath(path) || + path.front().getAccountID() != srcIssuer) { continue; } + startsWithIssuer = true; } diff --git a/src/ripple/app/paths/Pathfinder.h b/src/ripple/app/paths/Pathfinder.h index fd36466039..bb600a99e7 100644 --- a/src/ripple/app/paths/Pathfinder.h +++ b/src/ripple/app/paths/Pathfinder.h @@ -37,23 +37,15 @@ namespace ripple { class Pathfinder { public: - /** Construct a pathfinder with an issuer.*/ - Pathfinder ( - RippleLineCache::ref cache, - AccountID const& srcAccount, - AccountID const& dstAccount, - Currency const& uSrcCurrency, - AccountID const& uSrcIssuer, - STAmount const& dstAmount); - /** Construct a pathfinder without an issuer.*/ Pathfinder ( RippleLineCache::ref cache, AccountID const& srcAccount, AccountID const& dstAccount, Currency const& uSrcCurrency, - STAmount const& dstAmount); - + boost::optional const& uSrcIssuer, + STAmount const& dstAmount, + boost::optional const& srcAmount); ~Pathfinder(); static void initPathTable (); @@ -68,10 +60,11 @@ public: On return, if fullLiquidityPath is not empty, then it contains the best additional single path which can consume all the liquidity. */ - STPathSet getBestPaths ( + STPathSet + getBestPaths ( int maxPaths, STPath& fullLiquidityPath, - STPathSet& extraPaths, + STPathSet const& extraPaths, AccountID const& srcIssuer); enum NodeType @@ -184,6 +177,7 @@ private: /** The amount remaining from mSrcAccount after the default liquidity has been removed. */ STAmount mRemainingAmount; + bool convert_all_; std::shared_ptr mLedger; LoadEvent::pointer m_loadEvent; diff --git a/src/ripple/app/paths/RippleCalc.cpp b/src/ripple/app/paths/RippleCalc.cpp index e8fe972aef..4d81027c2a 100644 --- a/src/ripple/app/paths/RippleCalc.cpp +++ b/src/ripple/app/paths/RippleCalc.cpp @@ -335,7 +335,7 @@ TER RippleCalc::rippleCalculate () actualAmountIn_ += pathState->inPass(); actualAmountOut_ += pathState->outPass(); - if (pathState->allLiquidityConsumed() || multiQuality) + if (multiQuality) { ++iDry; pathState->setQuality(0); diff --git a/src/ripple/app/paths/cursor/DeliverNodeForward.cpp b/src/ripple/app/paths/cursor/DeliverNodeForward.cpp index bf86fca3b8..a752bd3f68 100644 --- a/src/ripple/app/paths/cursor/DeliverNodeForward.cpp +++ b/src/ripple/app/paths/cursor/DeliverNodeForward.cpp @@ -83,7 +83,7 @@ TER PathCursor::deliverNodeForward ( bool noFee = isXRP (previousNode().issue_) || uInAccountID == previousNode().issue_.account || node().offerOwnerAccount_ == previousNode().issue_.account; - const STAmount saInFeeRate = noFee ? saOne + const STAmount saInFeeRate = noFee ? STAmount::saOne : previousNode().transferRate_; // Transfer rate of issuer. // First calculate assuming no output fees: saInPassAct, diff --git a/src/ripple/app/paths/cursor/DeliverNodeReverse.cpp b/src/ripple/app/paths/cursor/DeliverNodeReverse.cpp index bc7e566a18..bdca1c6e0f 100644 --- a/src/ripple/app/paths/cursor/DeliverNodeReverse.cpp +++ b/src/ripple/app/paths/cursor/DeliverNodeReverse.cpp @@ -95,7 +95,7 @@ TER PathCursor::deliverNodeReverseImpl ( // Issuer sending or receiving. const STAmount saOutFeeRate = hasFee - ? saOne // No fee. + ? STAmount::saOne // No fee. : node().transferRate_; // Transfer rate of issuer. WriteLog (lsTRACE, RippleCalc) @@ -342,7 +342,7 @@ TER PathCursor::deliverNodeReverseImpl ( << " saOutAct=" << saOutAct << " saOutReq=" << saOutReq; - assert (saOutAct <= saOutReq); + assert(saOutAct <= saOutReq); if (resultCode == tesSUCCESS && !saOutAct) resultCode = tecPATH_DRY; diff --git a/src/ripple/app/paths/cursor/ReverseLiquidityForAccount.cpp b/src/ripple/app/paths/cursor/ReverseLiquidityForAccount.cpp index 8f4fc79206..cdf46f4e64 100644 --- a/src/ripple/app/paths/cursor/ReverseLiquidityForAccount.cpp +++ b/src/ripple/app/paths/cursor/ReverseLiquidityForAccount.cpp @@ -462,17 +462,11 @@ TER PathCursor::reverseLiquidityForAccount () const if (saCurWantedReq <= zero) { - // TEMPORARY emergency fix - // - // TODO(tom): why can't saCurWantedReq be -1 if you want to - // compute maximum liquidity? This might be unimplemented - // functionality. TODO(tom): should the same check appear in - // other paths or even be pulled up? + assert(false); WriteLog (lsFATAL, RippleCalc) << "CurWantReq was not positive"; return tefEXCEPTION; } - assert (saCurWantedReq > zero); // FIXME: We got one of these // The previous node is an offer; we are receiving our own currency. // The previous order book's entries might hold our issuances; might diff --git a/src/ripple/app/tests/Path_test.cpp b/src/ripple/app/tests/Path_test.cpp index e79521da2c..3b0086015e 100644 --- a/src/ripple/app/tests/Path_test.cpp +++ b/src/ripple/app/tests/Path_test.cpp @@ -1,103 +1,222 @@ //------------------------------------------------------------------------------ /* - This file is part of rippled: https://github.com/ripple/rippled - Copyright (c) 2012-2015 Ripple Labs Inc. + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2012-2015 Ripple Labs Inc. - Permission to use, copy, modify, and/or distribute this software for any - purpose with or without fee is hereby granted, provided that the above - copyright notice and this permission notice appear in all copies. + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. - THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ //============================================================================== #include -#include +#include #include #include +#include +#include #include #include -#include +#include #include +#include namespace ripple { namespace test { -using namespace jtx; +//------------------------------------------------------------------------------ + +namespace detail { + +void +stpath_append_one (STPath& st, + jtx::Account const& account) +{ + st.push_back(STPathElement({ + account.id(), boost::none, boost::none })); +} + +template +std::enable_if_t< + std::is_constructible::value> +stpath_append_one (STPath& st, + T const& t) +{ + stpath_append_one(st, jtx::Account{ t }); +} + +void +stpath_append_one (STPath& st, + jtx::IOU const& iou) +{ + st.push_back(STPathElement({ + iou.account.id(), iou.currency, boost::none })); +} + +void +stpath_append_one (STPath& st, + jtx::BookSpec const& book) +{ + st.push_back(STPathElement({ + boost::none, book.currency, book.account })); +} + +inline +void +stpath_append (STPath& st) +{ +} + +template +void +stpath_append (STPath& st, + T const& t, Args const&... args) +{ + stpath_append_one(st, t); + stpath_append(st, args...); +} + +inline +void +stpathset_append (STPathSet& st) +{ +} + +template +void +stpathset_append(STPathSet& st, + STPath const& p, Args const&... args) +{ + st.push_back(p); + stpathset_append(st, args...); +} + +} // detail + +template +STPath +stpath (Args const&... args) +{ + STPath st; + detail::stpath_append(st, args...); + return st; +} + +template +bool +same (STPathSet const& st1, Args const&... args) +{ + STPathSet st2; + detail::stpathset_append(st2, args...); + if (st1.size() != st2.size()) + return false; + + for (auto const& p : st2) + { + if (std::find(st1.begin(), st1.end(), p) == st1.end()) + return false; + } + return true; +} + +bool +equal(STAmount const& sa1, STAmount const& sa2) +{ + return sa1 == sa2 && + sa1.issue().account == sa2.issue().account; +} + +std::tuple +find_paths(std::shared_ptr const& view, + jtx::Account const& src, jtx::Account const& dst, + STAmount const& saDstAmount, + boost::optional const& saSendMax = boost::none) +{ + static int const level = 8; + auto cache = std::make_shared(view); + auto currencies = accountSourceCurrencies(src, cache, true); + auto jvSrcCurrencies = Json::Value(Json::arrayValue); + for (auto const& c : currencies) + { + Json::Value jvCurrency = Json::objectValue; + jvCurrency[jss::currency] = to_string(c); + jvSrcCurrencies.append(jvCurrency); + } + + auto result = ripplePathFind(cache, src.id(), dst.id(), + saDstAmount, jvSrcCurrencies, boost::none, level, saSendMax, + saDstAmount == STAmount(saDstAmount.issue(), 1u, 0, true)); + if (! result.first) + { + throw std::runtime_error( + "Path_test::findPath: ripplePathFind find failed"); + } + + auto const& jv = result.second[0u]; + Json::Value paths; + paths["Paths"] = jv["paths_computed"]; + STParsedJSONObject stp("generic", paths); + + STAmount sa; + if (jv.isMember(jss::source_amount)) + sa = amountFromJson(sfGeneric, jv[jss::source_amount]); + + STAmount da; + if (jv.isMember(jss::destination_amount)) + da = amountFromJson(sfGeneric, jv[jss::destination_amount]); + + return std::make_tuple( + std::move(stp.object->getFieldPathSet(sfPaths)), + std::move(sa), std::move(da)); +} + +//------------------------------------------------------------------------------ class Path_test : public beast::unit_test::suite { public: - Json::Value - findPath (std::shared_ptr const& view, - Account const& src, Account const& dest, - std::vector const& srcIssues, - STAmount const& saDstAmount) - { - auto jvSrcCurrencies = Json::Value(Json::arrayValue); - for (auto const& i : srcIssues) - { - STAmount const a = STAmount(i, 0); - jvSrcCurrencies.append(a.getJson(0)); - } - - int const level = 8; - auto result = ripplePathFind( - std::make_shared(view), - src.id(), dest.id(), saDstAmount, - jvSrcCurrencies, boost::none, level); - if(!result.first) - throw std::runtime_error( - "Path_test::findPath: ripplePathFind find failed"); - - return result.second; - } - void - test_no_direct_path_no_intermediary_no_alternatives() + no_direct_path_no_intermediary_no_alternatives() { + using namespace jtx; testcase("no direct path no intermediary no alternatives"); Env env(*this); env.fund(XRP(10000), "alice", "bob"); - auto const alternatives = findPath(env.open(), "alice", "bob", - {Account("alice")["USD"]}, Account("bob")["USD"](5)); - expect(alternatives.size() == 0); + auto const result = find_paths(env.open(), + "alice", "bob", Account("bob")["USD"](5)); + expect(std::get<0>(result).empty()); } void - test_direct_path_no_intermediary() + direct_path_no_intermediary() { + using namespace jtx; testcase("direct path no intermediary"); Env env(*this); env.fund(XRP(10000), "alice", "bob"); env.trust(Account("alice")["USD"](700), "bob"); - auto const alternatives = findPath(env.open(), "alice", "bob", - {Account("alice")["USD"]}, Account("bob")["USD"](5)); - Json::Value jv; - Json::Reader().parse(R"([{ - "paths_canonical" : [], - "paths_computed" : [], - "source_amount" : - { - "currency" : "USD", - "issuer" : "rG1QQv2nh2gr7RCZ1P8YYcBUKCCN633jCn", - "value" : "5" - } - }])", jv); - expect(jv == alternatives); + STPathSet st; + STAmount sa; + std::tie(st, sa, std::ignore) = find_paths(env.open(), + "alice", "bob", Account("bob")["USD"](5)); + expect(st.empty()); + expect(equal(sa, Account("alice")["USD"](5))); } void - test_payment_auto_path_find() + payment_auto_path_find() { + using namespace jtx; testcase("payment auto path find"); Env env(*this); auto const gw = Account("gateway"); @@ -114,8 +233,9 @@ public: } void - test_path_find() + path_find() { + using namespace jtx; testcase("path find"); Env env(*this); auto const gw = Account("gateway"); @@ -126,52 +246,68 @@ public: env(pay(gw, "alice", USD(70))); env(pay(gw, "bob", USD(50))); - auto const alternatives = findPath(env.open(), "alice", "bob", - {USD}, Account("bob")["USD"](5)); - Json::Value jv; - Json::Reader().parse(R"([{ - "paths_canonical" : [], - "paths_computed" : [], - "source_amount" : - { - "currency" : "USD", - "issuer" : "r9QxhA9RghPZBbUchA9HkrmLKaWvkLXU29", - "value" : "5" - } - }])", jv); - expect(jv == alternatives); + STPathSet st; + STAmount sa; + std::tie(st, sa, std::ignore) = find_paths(env.open(), + "alice", "bob", Account("bob")["USD"](5)); + expect(same(st, stpath("gateway"))); + expect(equal(sa, Account("alice")["USD"](5))); } void - test_path_find_consume_all() + path_find_consume_all() { + using namespace jtx; testcase("path find consume all"); - Env env(*this); - auto const gw = Account("gateway"); - auto const USD = gw["USD"]; - env.fund(XRP(10000), "alice", "bob", gw); - env.trust(Account("alice")["USD"](600), gw); - env.trust(USD(700), "bob"); - auto const alternatives = findPath(env.open(), "alice", "bob", - {USD}, Account("bob")["USD"](1)); - Json::Value jv; - Json::Reader().parse(R"([{ - "paths_canonical" : [], - "paths_computed" : [], - "source_amount" : - { - "currency" : "USD", - "issuer" : "r9QxhA9RghPZBbUchA9HkrmLKaWvkLXU29", - "value" : "1" - } - }])", jv); - expect(jv == alternatives); + { + Env env(*this); + env.fund(XRP(10000), "alice", "bob", "carol", + "dan", "edward"); + env.trust(Account("alice")["USD"](10), "bob"); + env.trust(Account("bob")["USD"](10), "carol"); + env.trust(Account("carol")["USD"](10), "edward"); + env.trust(Account("alice")["USD"](100), "dan"); + env.trust(Account("dan")["USD"](100), "edward"); + + STPathSet st; + STAmount sa; + STAmount da; + std::tie(st, sa, da) = find_paths(env.open(), + "alice", "edward", Account("edward")["USD"](-1)); + expect(same(st, stpath("dan"), stpath("bob", "carol"))); + expect(equal(sa, Account("alice")["USD"](110))); + expect(equal(da, Account("edward")["USD"](110))); + } + + { + Env env(*this); + auto const gw = Account("gateway"); + auto const USD = gw["USD"]; + env.fund(XRP(10000), "alice", "bob", "carol", gw); + env.trust(USD(100), "bob", "carol"); + env(pay(gw, "carol", USD(100))); + env(offer("carol", XRP(100), USD(100))); + + STPathSet st; + STAmount sa; + STAmount da; + std::tie(st, sa, da) = find_paths(env.open(), + "alice", "bob", Account("bob")["AUD"](-1), + boost::optional(XRP(100000000))); + expect(st.empty()); + std::tie(st, sa, da) = find_paths(env.open(), + "alice", "bob", Account("bob")["USD"](-1), + boost::optional(XRP(100000000))); + expect(sa == XRP(100)); + expect(equal(da, Account("bob")["USD"](100))); + } } void - test_alternative_path_consume_both() + alternative_path_consume_both() { + using namespace jtx; testcase("alternative path consume both"); Env env(*this); auto const gw = Account("gateway"); @@ -198,8 +334,9 @@ public: } void - test_alternative_paths_consume_best_transfer() + alternative_paths_consume_best_transfer() { + using namespace jtx; testcase("alternative paths consume best transfer"); Env env(*this); auto const gw = Account("gateway"); @@ -226,8 +363,9 @@ public: } void - test_alternative_paths_consume_best_transfer_first() + alternative_paths_consume_best_transfer_first() { + using namespace jtx; testcase("alternative paths - consume best transfer first"); Env env(*this); auto const gw = Account("gateway"); @@ -256,8 +394,9 @@ public: } void - test_alternative_paths_limit_returned_paths_to_best_quality() + alternative_paths_limit_returned_paths_to_best_quality() { + using namespace jtx; testcase("alternative paths - limit returned paths to best quality"); Env env(*this); auto const gw = Account("gateway"); @@ -276,25 +415,19 @@ public: env(pay("carol", "alice", Account("carol")["USD"](100))); env(pay(gw, "alice", USD(100))); - auto const alternatives = findPath(env.open(), "alice", "bob", - {USD}, Account("bob")["USD"](5)); - Json::Value jv; - Json::Reader().parse(R"([{ - "paths_canonical" : [], - "paths_computed" : [], - "source_amount" : - { - "currency" : "USD", - "issuer" : "r9QxhA9RghPZBbUchA9HkrmLKaWvkLXU29", - "value" : "5" - } - }])", jv); - expect(jv == alternatives); + STPathSet st; + STAmount sa; + std::tie(st, sa, std::ignore) = find_paths(env.open(), + "alice", "bob", Account("bob")["USD"](5)); + expect(same(st, stpath("gateway"), stpath("gateway2"), + stpath("dan"), stpath("carol"))); + expect(equal(sa, Account("alice")["USD"](5))); } void - test_issues_path_negative_issue() + issues_path_negative_issue() { + using namespace jtx; testcase("path negative: Issue #5"); Env env(*this); env.fund(XRP(10000), "alice", "bob", "carol", "dan"); @@ -305,16 +438,16 @@ public: env.require(balance("bob", Account("carol")["USD"](-75))); env.require(balance("carol", Account("bob")["USD"](75))); - auto alternatives = findPath(env.open(), "alice", "bob", - {Account("alice")["USD"]}, Account("bob")["USD"](25)); - expect(alternatives.size() == 0); + auto result = find_paths(env.open(), + "alice", "bob", Account("bob")["USD"](25)); + expect(std::get<0>(result).empty()); env(pay("alice", "bob", Account("alice")["USD"](25)), ter(tecPATH_DRY)); - alternatives = findPath(env.open(), "alice", "bob", - {Account("alice")["USD"]}, Account("alice")["USD"](25)); - expect(alternatives.size() == 0); + result = find_paths(env.open(), + "alice", "bob", Account("alice")["USD"](25)); + expect(std::get<0>(result).empty()); env.require(balance("alice", Account("bob")["USD"](0))); env.require(balance("alice", Account("dan")["USD"](0))); @@ -332,8 +465,9 @@ public: // alice --> carol --> dan --> bob // Balance of 100 USD Bob - Balance of 37 USD -> Rod void - test_issues_path_negative_ripple_client_issue_23_smaller() + issues_path_negative_ripple_client_issue_23_smaller() { + using namespace jtx; testcase("path negative: ripple-client issue #23: smaller"); Env env(*this); env.fund(XRP(10000), "alice", "bob", "carol", "dan"); @@ -350,8 +484,9 @@ public: // alice -120 USD-> edward -25 USD-> bob // alice -25 USD-> carol -75 USD -> dan -100 USD-> bob void - test_issues_path_negative_ripple_client_issue_23_larger() + issues_path_negative_ripple_client_issue_23_larger() { + using namespace jtx; testcase("path negative: ripple-client issue #23: larger"); Env env(*this); env.fund(XRP(10000), "alice", "bob", "carol", "dan", "edward"); @@ -376,64 +511,49 @@ public: // bob will hold gateway AUD // alice pays bob gateway AUD using XRP void - test_via_offers_via_gateway() + via_offers_via_gateway() { + using namespace jtx; testcase("via gateway"); Env env(*this); auto const gw = Account("gateway"); auto const AUD = gw["AUD"]; env.fund(XRP(10000), "alice", "bob", "carol", gw); env(rate(gw, 1.1)); - env.trust(AUD(100), "bob"); - env.trust(AUD(100), "carol"); + env.trust(AUD(100), "bob", "carol"); env(pay(gw, "carol", AUD(50))); env(offer("carol", XRP(50), AUD(50))); env(pay("alice", "bob", AUD(10)), sendmax(XRP(100)), paths(XRP)); env.require(balance("bob", AUD(10))); env.require(balance("carol", AUD(39))); - auto const alternatives = findPath(env.open(), "alice", "bob", - {Account("alice")["USD"]}, Account("bob")["USD"](25)); - expect(alternatives.size() == 0); + auto const result = find_paths(env.open(), + "alice", "bob", Account("bob")["USD"](25)); + expect(std::get<0>(result).empty()); } void - test_indirect_paths_path_find() + indirect_paths_path_find() { + using namespace jtx; testcase("path find"); Env env(*this); env.fund(XRP(10000), "alice", "bob", "carol"); env.trust(Account("alice")["USD"](1000), "bob"); env.trust(Account("bob")["USD"](1000), "carol"); - auto const alternatives = findPath(env.open(), "alice", "carol", - {Account("alice")["USD"]}, Account("carol")["USD"](5)); - Json::Value jv; - Json::Reader().parse(R"([{ - "paths_canonical" : [], - "paths_computed" : - [ - [ - { - "account" : "rPMh7Pi9ct699iZUTWaytJUoHcJ7cgyziK", - "type" : 1, - "type_hex" : "0000000000000001" - } - ] - ], - "source_amount" : - { - "currency" : "USD", - "issuer" : "rG1QQv2nh2gr7RCZ1P8YYcBUKCCN633jCn", - "value" : "5" - } - }])", jv); - expect(jv == alternatives); + STPathSet st; + STAmount sa; + std::tie(st, sa, std::ignore) = find_paths(env.open(), + "alice", "carol", Account("carol")["USD"](5)); + expect(same(st, stpath("bob"))); + expect(equal(sa, Account("alice")["USD"](5))); } void - test_quality_paths_quality_set_and_test() + quality_paths_quality_set_and_test() { + using namespace jtx; testcase("quality set and test"); Env env(*this); env.fund(XRP(10000), "alice", "bob"); @@ -473,8 +593,9 @@ public: } void - test_trust_auto_clear_trust_normal_clear() + trust_auto_clear_trust_normal_clear() { + using namespace jtx; testcase("trust normal clear"); Env env(*this); env.fund(XRP(10000), "alice", "bob"); @@ -516,8 +637,9 @@ public: } void - test_trust_auto_clear_trust_auto_clear() + trust_auto_clear_trust_auto_clear() { + using namespace jtx; testcase("trust auto clear"); Env env(*this); env.fund(XRP(10000), "alice", "bob"); @@ -564,23 +686,23 @@ public: void run() { - test_no_direct_path_no_intermediary_no_alternatives(); - test_direct_path_no_intermediary(); - test_payment_auto_path_find(); - test_path_find(); - test_path_find_consume_all(); - test_alternative_path_consume_both(); - test_alternative_paths_consume_best_transfer(); - test_alternative_paths_consume_best_transfer_first(); - test_alternative_paths_limit_returned_paths_to_best_quality(); - test_issues_path_negative_issue(); - test_issues_path_negative_ripple_client_issue_23_smaller(); - test_issues_path_negative_ripple_client_issue_23_larger(); - test_via_offers_via_gateway(); - test_indirect_paths_path_find(); - test_quality_paths_quality_set_and_test(); - test_trust_auto_clear_trust_normal_clear(); - test_trust_auto_clear_trust_auto_clear(); + no_direct_path_no_intermediary_no_alternatives(); + direct_path_no_intermediary(); + payment_auto_path_find(); + path_find(); + path_find_consume_all(); + alternative_path_consume_both(); + alternative_paths_consume_best_transfer(); + alternative_paths_consume_best_transfer_first(); + alternative_paths_limit_returned_paths_to_best_quality(); + issues_path_negative_issue(); + issues_path_negative_ripple_client_issue_23_smaller(); + issues_path_negative_ripple_client_issue_23_larger(); + via_offers_via_gateway(); + indirect_paths_path_find(); + quality_paths_quality_set_and_test(); + trust_auto_clear_trust_normal_clear(); + trust_auto_clear_trust_auto_clear(); } }; diff --git a/src/ripple/ledger/impl/PaymentSandbox.cpp b/src/ripple/ledger/impl/PaymentSandbox.cpp index 7a0cc11dd6..7123b87e16 100644 --- a/src/ripple/ledger/impl/PaymentSandbox.cpp +++ b/src/ripple/ledger/impl/PaymentSandbox.cpp @@ -43,7 +43,7 @@ void DeferredCredits::credit (AccountID const& sender, using std::get; assert (sender != receiver); - assert (!amount.negative ()); + assert (!amount.negative()); auto const k = makeKey (sender, receiver, amount.getCurrency ()); auto i = map_.find (k); diff --git a/src/ripple/protocol/ErrorCodes.h b/src/ripple/protocol/ErrorCodes.h index e071e907fd..1761119cec 100644 --- a/src/ripple/protocol/ErrorCodes.h +++ b/src/ripple/protocol/ErrorCodes.h @@ -90,6 +90,7 @@ enum error_code_i rpcDST_ACT_MALFORMED, rpcDST_ACT_MISSING, rpcDST_AMT_MALFORMED, + rpcDST_AMT_MISSING, rpcDST_ISR_MALFORMED, rpcGETS_ACT_MALFORMED, rpcGETS_AMT_MALFORMED, @@ -101,6 +102,7 @@ enum error_code_i rpcPORT_MALFORMED, rpcPUBLIC_MALFORMED, rpcSIGN_FOR_MALFORMED, + rpcSENDMAX_MALFORMED, rpcSRC_ACT_MALFORMED, rpcSRC_ACT_MISSING, rpcSRC_ACT_NOT_FOUND, diff --git a/src/ripple/protocol/JsonFields.h b/src/ripple/protocol/JsonFields.h index 26ed7908d9..d77f0b7909 100644 --- a/src/ripple/protocol/JsonFields.h +++ b/src/ripple/protocol/JsonFields.h @@ -131,6 +131,7 @@ JSS ( descending ); // in: AccountTx* JSS ( destination_account ); // in: PathRequest, RipplePathFind JSS ( destination_amount ); // in: PathRequest, RipplePathFind JSS ( destination_currencies ); // in: PathRequest, RipplePathFind +JSS ( destination_tag ); // in: PathRequest JSS ( dir_entry ); // out: DirectoryEntryIterator JSS ( dir_index ); // out: DirectoryEntryIterator JSS ( dir_root ); // out: DirectoryEntryIterator @@ -311,6 +312,7 @@ JSS ( secret ); // in: TransactionSign, WalletSeed, JSS ( seed ); // in: WalletAccounts, out: WalletSeed JSS ( seed_hex ); // in: WalletPropose, TransactionSign JSS ( send_currencies ); // out: AccountCurrencies +JSS ( send_max ); // in: PathRequest, RipplePathFind JSS ( seq ); // in: LedgerEntry; // out: NetworkOPs, RPCSub, AccountOffers JSS ( seqNum ); // out: LedgerToJson diff --git a/src/ripple/protocol/STAmount.h b/src/ripple/protocol/STAmount.h index 601b8a0c24..2f22bfcf9c 100644 --- a/src/ripple/protocol/STAmount.h +++ b/src/ripple/protocol/STAmount.h @@ -71,6 +71,9 @@ public: static std::uint64_t const uRateOne; + static STAmount const saZero; + static STAmount const saOne; + //-------------------------------------------------------------------------- STAmount(SerialIter& sit, SField const& name); @@ -382,10 +385,6 @@ inline bool isXRP(STAmount const& amount) return isXRP (amount.issue().currency); } -// VFALCO TODO Make static member accessors for these in STAmount -extern const STAmount saZero; -extern const STAmount saOne; - } // ripple #endif diff --git a/src/ripple/protocol/STPathSet.h b/src/ripple/protocol/STPathSet.h index 82d4f31974..0ae6abff5e 100644 --- a/src/ripple/protocol/STPathSet.h +++ b/src/ripple/protocol/STPathSet.h @@ -318,6 +318,12 @@ public: return value[n]; } + std::vector::reference + operator[] (std::vector::size_type n) + { + return value[n]; + } + std::vector::const_iterator begin () const { diff --git a/src/ripple/protocol/impl/STAmount.cpp b/src/ripple/protocol/impl/STAmount.cpp index 438a999214..3a8ee807d3 100644 --- a/src/ripple/protocol/impl/STAmount.cpp +++ b/src/ripple/protocol/impl/STAmount.cpp @@ -39,9 +39,6 @@ static const std::uint64_t tenTo14 = 100000000000000ull; static const std::uint64_t tenTo14m1 = tenTo14 - 1; static const std::uint64_t tenTo17 = tenTo14 * 1000; -STAmount const saZero (noIssue(), 0); -STAmount const saOne (noIssue(), 1); - //------------------------------------------------------------------------------ static std::int64_t @@ -340,6 +337,9 @@ STAmount operator- (STAmount const& v1, STAmount const& v2) std::uint64_t const STAmount::uRateOne = getRate (STAmount (1), STAmount (1)); +STAmount const STAmount::saZero (noIssue (), 0u); +STAmount const STAmount::saOne (noIssue (), 1u); + void STAmount::setIssue (Issue const& issue) { diff --git a/src/ripple/rpc/RipplePathFind.h b/src/ripple/rpc/RipplePathFind.h index 2bbabbea52..9f230f2b41 100644 --- a/src/ripple/rpc/RipplePathFind.h +++ b/src/ripple/rpc/RipplePathFind.h @@ -31,7 +31,8 @@ ripplePathFind (RippleLineCache::pointer const& cache, STAmount const& saDstAmount, Json::Value const& jvSrcCurrencies, boost::optional const& contextPaths, - int const& level); + int const& level, boost::optional saSendMax, + bool convert_all); } diff --git a/src/ripple/rpc/handlers/RipplePathFind.cpp b/src/ripple/rpc/handlers/RipplePathFind.cpp index e2fe84c274..4cda192ebe 100644 --- a/src/ripple/rpc/handlers/RipplePathFind.cpp +++ b/src/ripple/rpc/handlers/RipplePathFind.cpp @@ -144,7 +144,8 @@ Json::Value doRipplePathFind (RPC::Context& context) // Parse saDstAmount. !context.params.isMember (jss::destination_amount) || ! amountFromJsonNoThrow(saDstAmount, context.params[jss::destination_amount]) - || saDstAmount <= zero + || (saDstAmount <= zero && saDstAmount != + STAmount(saDstAmount.issue(), 1u, 0, true)) || (!isXRP(saDstAmount.getCurrency ()) && (!saDstAmount.getIssuer () || noAccount() == saDstAmount.getIssuer ()))) @@ -219,11 +220,34 @@ Json::Value doRipplePathFind (RPC::Context& context) level = rLev; } + auto const convert_all = + saDstAmount == STAmount(saDstAmount.issue(), 1u, 0, true); + + boost::optional saSendMax; + if (context.params.isMember(jss::send_max)) + { + // Send_max requires destination amount to be -1. + if (! convert_all) + return rpcError(rpcDST_AMT_MALFORMED); + + saSendMax.emplace(); + if (!amountFromJsonNoThrow( + *saSendMax, context.params[jss::send_max]) || + (saSendMax->getCurrency().isZero() && + saSendMax->getIssuer().isNonZero()) || + (saSendMax->getCurrency() == badCurrency()) || + (*saSendMax <= zero && + *saSendMax != STAmount(saSendMax->issue(), 1u, 0, true))) + { + return rpcError(rpcSENDMAX_MALFORMED); + } + } + auto contextPaths = context.params.isMember(jss::paths) ? boost::optional(context.params[jss::paths]) : boost::optional(boost::none); auto pathFindResult = ripplePathFind(cache, raSrc, raDst, saDstAmount, - jvSrcCurrencies, contextPaths, level); + jvSrcCurrencies, contextPaths, level, saSendMax, convert_all); if (!pathFindResult.first) return pathFindResult.second; @@ -241,13 +265,18 @@ std::pair ripplePathFind (RippleLineCache::pointer const& cache, AccountID const& raSrc, AccountID const& raDst, STAmount const& saDstAmount, Json::Value const& jvSrcCurrencies, - boost::optional const& contextPaths, int const& level) + boost::optional const& contextPaths, + int const& level, boost::optional saSendMax, + bool convert_all) { + auto const sa = STAmount(saDstAmount.issue(), + STAmount::cMaxValue, STAmount::cMaxOffset); FindPaths fp( cache, raSrc, raDst, - saDstAmount, + convert_all ? sa : saDstAmount, + saSendMax, level, 4); // max paths @@ -269,7 +298,6 @@ ripplePathFind (RippleLineCache::pointer const& cache, uSrcCurrencyID, jvSource[jss::currency].asString())) { WriteLog(lsINFO, RPCHandler) << "Bad currency."; - return std::make_pair(false, rpcError(rpcSRC_CUR_MALFORMED)); } @@ -287,6 +315,30 @@ ripplePathFind (RippleLineCache::pointer const& cache, return std::make_pair(false, rpcError(rpcSRC_ISR_MALFORMED)); } + auto issue = Issue(uSrcCurrencyID, uSrcIssuerID); + if (saSendMax) + { + // If the currencies don't match, ignore the source currency. + if (uSrcCurrencyID != saSendMax->getCurrency()) + continue; + + // If neither is the source and they are not equal, then the + // source issuer is illegal. + if (uSrcIssuerID != raSrc && saSendMax->getIssuer() != raSrc && + uSrcIssuerID != saSendMax->getIssuer()) + { + return std::make_pair(false, rpcError(rpcSRC_ISR_MALFORMED)); + } + + // If both are the source, use the source. + // Otherwise, use the one that's not the source. + if (uSrcIssuerID == raSrc) + { + issue.account = saSendMax->getIssuer() != raSrc ? + saSendMax->getIssuer() : raSrc; + } + } + STPathSet spsComputed; if (contextPaths) { @@ -304,11 +356,11 @@ ripplePathFind (RippleLineCache::pointer const& cache, } STPath fullLiquidityPath; - auto valid = fp.findPathsForIssue( - { uSrcCurrencyID, uSrcIssuerID }, + auto result = fp.findPathsForIssue( + issue, spsComputed, fullLiquidityPath); - if (!valid) + if (! result) { WriteLog(lsWARNING, RPCHandler) << "ripple_path_find: No paths found."; @@ -321,22 +373,26 @@ ripplePathFind (RippleLineCache::pointer const& cache, xrpAccount() : (raSrc) : uSrcIssuerID; // Use specifed issuer. - - STAmount saMaxAmount({ uSrcCurrencyID, issuer }, 1); - saMaxAmount.negate(); + STAmount saMaxAmount = saSendMax.value_or( + STAmount({uSrcCurrencyID, issuer}, 1u, 0, true)); boost::optional sandbox; sandbox.emplace(&*cache->getLedger(), tapNONE); assert(sandbox->open()); + path::RippleCalc::Input rcInput; + if (convert_all) + rcInput.partialPaymentAllowed = true; + auto rc = path::RippleCalc::rippleCalculate( *sandbox, - saMaxAmount, // --> Amount to send is unlimited - // to get an estimate. - saDstAmount, // --> Amount to deliver. - raDst, // --> Account to deliver to. - raSrc, // --> Account sending from. - spsComputed); // --> Path set. + saMaxAmount, // --> Amount to send is unlimited + // to get an estimate. + convert_all ? sa : saDstAmount, // --> Amount to deliver. + raDst, // --> Account to deliver to. + raSrc, // --> Account sending from. + *result, // --> Path set. + &rcInput); WriteLog(lsWARNING, RPCHandler) << "ripple_path_find:" @@ -345,23 +401,23 @@ ripplePathFind (RippleLineCache::pointer const& cache, << " saMaxAmountAct=" << rc.actualAmountIn << " saDstAmountAct=" << rc.actualAmountOut; - if (fullLiquidityPath.size() > 0 && + if (! convert_all && + ! fullLiquidityPath.empty() && (rc.result() == terNO_LINE || rc.result() == tecPATH_PARTIAL)) { WriteLog(lsDEBUG, PathRequest) << "Trying with an extra path element"; - - spsComputed.push_back(fullLiquidityPath); + result->push_back(fullLiquidityPath); sandbox.emplace(&*cache->getLedger(), tapNONE); assert(sandbox->open()); rc = path::RippleCalc::rippleCalculate( *sandbox, saMaxAmount, // --> Amount to send is unlimited - // to get an estimate. + // to get an estimate. saDstAmount, // --> Amount to deliver. raDst, // --> Account to deliver to. raSrc, // --> Account sending from. - spsComputed); // --> Path set. + *result); // --> Path set. WriteLog(lsDEBUG, PathRequest) << "Extra path element gives " << transHuman(rc.result()); @@ -379,7 +435,10 @@ ripplePathFind (RippleLineCache::pointer const& cache, jvEntry[jss::source_amount] = rc.actualAmountIn.getJson(0); jvEntry[jss::paths_canonical] = Json::arrayValue; - jvEntry[jss::paths_computed] = spsComputed.getJson(0); + jvEntry[jss::paths_computed] = result->getJson(0); + + if (convert_all) + jvEntry[jss::destination_amount] = rc.actualAmountOut.getJson(0); jvArray.append(jvEntry); } @@ -399,7 +458,7 @@ ripplePathFind (RippleLineCache::pointer const& cache, } } - return std::make_pair(true, jvArray); + return std::make_pair(true, std::move(jvArray)); } } // ripple diff --git a/src/ripple/rpc/impl/TransactionSign.cpp b/src/ripple/rpc/impl/TransactionSign.cpp index 8b56dd2d46..d5fbd0a4b1 100644 --- a/src/ripple/rpc/impl/TransactionSign.cpp +++ b/src/ripple/rpc/impl/TransactionSign.cpp @@ -140,21 +140,26 @@ void TxnSignApiFacade::processTransaction ( netOPs_->processTransaction(transaction, bAdmin, bLocal, failType); } -bool TxnSignApiFacade::findPathsForOneIssuer ( +boost::optional +TxnSignApiFacade::findPathsForOneIssuer ( AccountID const& dstAccountID, Issue const& srcIssue, STAmount const& dstAmount, int searchLevel, unsigned int const maxPaths, - STPathSet& pathsOut, + STPathSet const& paths, STPath& fullLiquidityPath) const { - if (!ledger_) // Unit testing. - // Note that unit tests don't (yet) need pathsOut or fullLiquidityPath. - return true; + if (! ledger_) // Unit testing. + { + // Note that unit tests don't (yet) need paths or fullLiquidityPath. + boost::optional result; + result.emplace(); + return result; + } auto cache = std::make_shared (ledger_); - return ripple::findPathsForOneIssuer ( + return ripple::findPathsForOneIssuer( cache, accountID_, dstAccountID, @@ -162,7 +167,7 @@ bool TxnSignApiFacade::findPathsForOneIssuer ( dstAmount, searchLevel, maxPaths, - pathsOut, + paths, fullLiquidityPath); } @@ -413,18 +418,17 @@ static Json::Value checkPayment( if (!lpf.isOk ()) return rpcError (rpcTOO_BUSY); - STPathSet spsPaths; STPath fullLiquidityPath; - bool valid = apiFacade.findPathsForOneIssuer ( + auto result = apiFacade.findPathsForOneIssuer ( *dstAccountID, sendMax.issue (), amount, getConfig ().PATH_SEARCH_OLD, 4, // iMaxPaths - spsPaths, + {}, fullLiquidityPath); - if (!valid) + if (! result) { WriteLog (lsDEBUG, RPCHandler) << "transactionSign: build_path: No paths found."; @@ -432,10 +436,10 @@ static Json::Value checkPayment( } WriteLog (lsDEBUG, RPCHandler) << "transactionSign: build_path: " - << spsPaths.getJson (0); + << result->getJson (0); - if (!spsPaths.empty ()) - tx_json[jss::Paths] = spsPaths.getJson (0); + if (! result->empty ()) + tx_json[jss::Paths] = result->getJson (0); } } return Json::Value(); diff --git a/src/ripple/rpc/impl/TransactionSign.h b/src/ripple/rpc/impl/TransactionSign.h index 5b90af4c51..dcbe093fbe 100644 --- a/src/ripple/rpc/impl/TransactionSign.h +++ b/src/ripple/rpc/impl/TransactionSign.h @@ -70,13 +70,14 @@ public: std::uint32_t getSeq () const; - bool findPathsForOneIssuer ( + boost::optional + findPathsForOneIssuer ( AccountID const& dstAccountID, Issue const& srcIssue, STAmount const& dstAmount, int searchLevel, unsigned int const maxPaths, - STPathSet& pathsOut, + STPathSet const& paths, STPath& fullLiquidityPath) const; void processTransaction ( diff --git a/src/ripple/test/jtx/impl/paths.cpp b/src/ripple/test/jtx/impl/paths.cpp index bad2a95e5c..469e08fbce 100644 --- a/src/ripple/test/jtx/impl/paths.cpp +++ b/src/ripple/test/jtx/impl/paths.cpp @@ -37,16 +37,15 @@ paths::operator()(Env const& env, JTx& jt) const auto const amount = amountFromJson( sfAmount, jv[jss::Amount]); STPath fp; - STPathSet ps; auto const found = findPathsForOneIssuer( std::make_shared( env.open()), from, to, in_, amount, - depth_, limit_, ps, fp); + depth_, limit_, {}, fp); // VFALCO TODO API to allow caller to examine the STPathSet // VFALCO isDefault should be renamed to empty() - if (found && ! ps.isDefault()) - jv[jss::Paths] = ps.getJson(0); + if (found && ! found->isDefault()) + jv[jss::Paths] = found->getJson(0); } //------------------------------------------------------------------------------