diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake index 493d2625081..ad82b1786a7 100644 --- a/Builds/CMake/RippledCore.cmake +++ b/Builds/CMake/RippledCore.cmake @@ -517,13 +517,13 @@ target_sources (rippled PRIVATE src/ripple/app/misc/impl/ValidatorList.cpp src/ripple/app/misc/impl/ValidatorSite.cpp src/ripple/app/paths/AccountCurrencies.cpp + src/ripple/app/paths/AssetCache.cpp src/ripple/app/paths/Credit.cpp src/ripple/app/paths/Flow.cpp src/ripple/app/paths/PathRequest.cpp src/ripple/app/paths/PathRequests.cpp src/ripple/app/paths/Pathfinder.cpp src/ripple/app/paths/RippleCalc.cpp - src/ripple/app/paths/RippleLineCache.cpp src/ripple/app/paths/TrustLine.cpp src/ripple/app/paths/impl/AMMLiquidity.cpp src/ripple/app/paths/impl/AMMOffer.cpp diff --git a/src/ripple/app/paths/AccountCurrencies.cpp b/src/ripple/app/paths/AccountCurrencies.cpp index 18452725b67..a92393b3f28 100644 --- a/src/ripple/app/paths/AccountCurrencies.cpp +++ b/src/ripple/app/paths/AccountCurrencies.cpp @@ -24,7 +24,7 @@ namespace ripple { hash_set accountSourceCurrencies( AccountID const& account, - std::shared_ptr const& lrCache, + std::shared_ptr const& lrCache, bool includeXRP) { hash_set currencies; @@ -60,7 +60,7 @@ accountSourceCurrencies( hash_set accountDestCurrencies( AccountID const& account, - std::shared_ptr const& lrCache, + std::shared_ptr const& lrCache, bool includeXRP) { hash_set currencies; diff --git a/src/ripple/app/paths/AccountCurrencies.h b/src/ripple/app/paths/AccountCurrencies.h index fa70ec2a081..d5321fbd32c 100644 --- a/src/ripple/app/paths/AccountCurrencies.h +++ b/src/ripple/app/paths/AccountCurrencies.h @@ -20,7 +20,7 @@ #ifndef RIPPLE_APP_PATHS_ACCOUNTCURRENCIES_H_INCLUDED #define RIPPLE_APP_PATHS_ACCOUNTCURRENCIES_H_INCLUDED -#include +#include #include namespace ripple { @@ -28,13 +28,13 @@ namespace ripple { hash_set accountDestCurrencies( AccountID const& account, - std::shared_ptr const& cache, + std::shared_ptr const& cache, bool includeXRP); hash_set accountSourceCurrencies( AccountID const& account, - std::shared_ptr const& lrLedger, + std::shared_ptr const& lrLedger, bool includeXRP); } // namespace ripple diff --git a/src/ripple/app/paths/RippleLineCache.cpp b/src/ripple/app/paths/AssetCache.cpp similarity index 83% rename from src/ripple/app/paths/RippleLineCache.cpp rename to src/ripple/app/paths/AssetCache.cpp index 2487924ff0e..93adb84c3d9 100644 --- a/src/ripple/app/paths/RippleLineCache.cpp +++ b/src/ripple/app/paths/AssetCache.cpp @@ -17,13 +17,13 @@ */ //============================================================================== -#include +#include #include #include namespace ripple { -RippleLineCache::RippleLineCache( +AssetCache::AssetCache( std::shared_ptr const& ledger, beast::Journal j) : ledger_(ledger), journal_(j) @@ -31,7 +31,7 @@ RippleLineCache::RippleLineCache( JLOG(journal_.debug()) << "created for ledger " << ledger_->info().seq; } -RippleLineCache::~RippleLineCache() +AssetCache::~AssetCache() { JLOG(journal_.debug()) << "destroyed for ledger " << ledger_->info().seq << " with " << lines_.size() << " accounts and " @@ -39,9 +39,7 @@ RippleLineCache::~RippleLineCache() } std::shared_ptr> -RippleLineCache::getRippleLines( - AccountID const& accountID, - LineDirection direction) +AssetCache::getRippleLines(AccountID const& accountID, LineDirection direction) { auto const hash = hasher_(accountID); AccountKey key(accountID, direction, hash); @@ -125,4 +123,33 @@ RippleLineCache::getRippleLines( return it->second; } +std::shared_ptr> const& +AssetCache::getMPTs(const ripple::AccountID& account) +{ + std::lock_guard sl(mLock); + + if (auto it = mpts_.find(account); it != mpts_.end()) + return it->second; + + std::vector mpts; + // Get issued/authorized tokens + forEachItem(*ledger_, account, [&](std::shared_ptr const& sle) { + if (sle->getType() == ltMPTOKEN_ISSUANCE) + mpts.push_back( + std::make_pair(sle->getFieldU32(sfSequence), account)); + else if (sle->getType() == ltMPTOKEN) + mpts.push_back(getMPT(sle->getFieldH192(sfMPTokenIssuanceID))); + }); + + totalMPTCount_ += mpts.size(); + + if (mpts.empty()) + mpts_.emplace(account, nullptr); + else + mpts_.emplace( + account, std::make_shared>(std::move(mpts))); + + return mpts_[account]; +} + } // namespace ripple diff --git a/src/ripple/app/paths/RippleLineCache.h b/src/ripple/app/paths/AssetCache.h similarity index 93% rename from src/ripple/app/paths/RippleLineCache.h rename to src/ripple/app/paths/AssetCache.h index 590c50082f7..f44e0bed496 100644 --- a/src/ripple/app/paths/RippleLineCache.h +++ b/src/ripple/app/paths/AssetCache.h @@ -33,13 +33,13 @@ namespace ripple { // Used by Pathfinder -class RippleLineCache final : public CountedObject +class AssetCache final : public CountedObject { public: - explicit RippleLineCache( + explicit AssetCache( std::shared_ptr const& l, beast::Journal j); - ~RippleLineCache(); + ~AssetCache(); std::shared_ptr const& getLedger() const @@ -62,6 +62,9 @@ class RippleLineCache final : public CountedObject std::shared_ptr> getRippleLines(AccountID const& accountID, LineDirection direction); + std::shared_ptr> const& + getMPTs(AccountID const& account); + private: std::mutex mLock; @@ -125,6 +128,8 @@ class RippleLineCache final : public CountedObject AccountKey::Hash> lines_; std::size_t totalLineCount_ = 0; + hash_map>> mpts_; + std::size_t totalMPTCount_ = 0; }; } // namespace ripple diff --git a/src/ripple/app/paths/PathRequest.cpp b/src/ripple/app/paths/PathRequest.cpp index b6b2b1dbdd9..f9baaed94f6 100644 --- a/src/ripple/app/paths/PathRequest.cpp +++ b/src/ripple/app/paths/PathRequest.cpp @@ -169,7 +169,7 @@ PathRequest::updateComplete() } bool -PathRequest::isValid(std::shared_ptr const& crCache) +PathRequest::isValid(std::shared_ptr const& crCache) { if (!raSrcAccount || !raDstAccount) return false; @@ -222,6 +222,13 @@ PathRequest::isValid(std::shared_ptr const& crCache) for (auto const& currency : usDestCurrID) jvDestCur.append(to_string(currency)); + + if (auto mpts = crCache->getMPTs(*raDstAccount)) + { + for (auto const& mpt : *mpts) + jvDestCur.append(to_string(mpt)); + } + jvStatus[jss::destination_tag] = (sleDest->getFlags() & lsfRequireDestTag); } @@ -242,7 +249,7 @@ PathRequest::isValid(std::shared_ptr const& crCache) */ std::pair PathRequest::doCreate( - std::shared_ptr const& cache, + std::shared_ptr const& cache, Json::Value const& value) { bool valid = false; @@ -497,7 +504,7 @@ PathRequest::doAborting() const std::unique_ptr const& PathRequest::getPathFinder( - std::shared_ptr const& cache, + std::shared_ptr const& cache, hash_map>& pathasset_map, PathAsset const& asset, STAmount const& dst_amount, @@ -525,7 +532,7 @@ PathRequest::getPathFinder( bool PathRequest::findPaths( - std::shared_ptr const& cache, + std::shared_ptr const& cache, int const level, Json::Value& jvArray, std::function const& continueCallback) @@ -537,7 +544,6 @@ PathRequest::findPaths( } if (sourceAssets.empty()) { - // TODO MPT auto currencies = accountSourceCurrencies(*raSrcAccount, cache, true); bool const sameAccount = *raSrcAccount == *raDstAccount; for (auto const& c : currencies) @@ -551,6 +557,13 @@ PathRequest::findPaths( {c, c.isZero() ? xrpAccount() : *raSrcAccount}); } } + if (auto mpts = cache->getMPTs(*raSrcAccount)) + { + if (sourceAssets.size() >= RPC::Tuning::max_auto_src_cur) + return false; + for (auto const& mpt : *mpts) + sourceAssets.insert(mpt); + } } auto const dst_amount = convertAmount(saDstAmount, convert_all_); @@ -659,7 +672,9 @@ PathRequest::findPaths( if (rc.result() == tesSUCCESS) { Json::Value jvEntry(Json::objectValue); - rc.actualAmountIn.setIssuer(sourceAccount); + // TODO MPT + if (rc.actualAmountIn.isIssue()) + rc.actualAmountIn.setIssuer(sourceAccount); jvEntry[jss::source_amount] = rc.actualAmountIn.getJson(JsonOptions::none); jvEntry[jss::paths_computed] = ps.getJson(JsonOptions::none); @@ -694,7 +709,7 @@ PathRequest::findPaths( Json::Value PathRequest::doUpdate( - std::shared_ptr const& cache, + std::shared_ptr const& cache, bool fast, std::function const& continueCallback) { @@ -714,11 +729,16 @@ PathRequest::doUpdate( if (hasCompletion()) { // Old ripple_path_find API gives destination_currencies - auto& destCurrencies = + auto& destAssets = (newStatus[jss::destination_currencies] = Json::arrayValue); - auto usCurrencies = accountDestCurrencies(*raDstAccount, cache, true); - for (auto const& c : usCurrencies) - destCurrencies.append(to_string(c)); + auto usAssets = accountDestCurrencies(*raDstAccount, cache, true); + for (auto const& c : usAssets) + destAssets.append(to_string(c)); + if (auto mpts = cache->getMPTs(*raDstAccount)) + { + for (auto const& mpt : *mpts) + destAssets.append(to_string(mpt)); + } } newStatus[jss::source_account] = toBase58(*raSrcAccount); diff --git a/src/ripple/app/paths/PathRequest.h b/src/ripple/app/paths/PathRequest.h index 264e071a8d1..e7d570f5c2c 100644 --- a/src/ripple/app/paths/PathRequest.h +++ b/src/ripple/app/paths/PathRequest.h @@ -21,8 +21,8 @@ #define RIPPLE_APP_PATHS_PATHREQUEST_H_INCLUDED #include +#include #include -#include #include #include #include @@ -38,7 +38,7 @@ namespace ripple { // A pathfinding request submitted by a client // The request issuer must maintain a strong pointer -class RippleLineCache; +class AssetCache; class PathRequests; // Return values from parseJson <0 = invalid, >0 = valid @@ -87,7 +87,7 @@ class PathRequest final : public InfoSubRequest, updateComplete(); std::pair - doCreate(std::shared_ptr const&, Json::Value const&); + doCreate(std::shared_ptr const&, Json::Value const&); Json::Value doClose() override; @@ -99,7 +99,7 @@ class PathRequest final : public InfoSubRequest, // update jvStatus Json::Value doUpdate( - std::shared_ptr const&, + std::shared_ptr const&, bool fast, std::function const& continueCallback = {}); InfoSub::pointer @@ -109,11 +109,11 @@ class PathRequest final : public InfoSubRequest, private: bool - isValid(std::shared_ptr const& crCache); + isValid(std::shared_ptr const& crCache); std::unique_ptr const& getPathFinder( - std::shared_ptr const&, + std::shared_ptr const&, hash_map>&, PathAsset const&, STAmount const&, @@ -125,7 +125,7 @@ class PathRequest final : public InfoSubRequest, */ bool findPaths( - std::shared_ptr const&, + std::shared_ptr const&, int const, Json::Value&, std::function const&); diff --git a/src/ripple/app/paths/PathRequests.cpp b/src/ripple/app/paths/PathRequests.cpp index ac051e5f0c5..1c12526492f 100644 --- a/src/ripple/app/paths/PathRequests.cpp +++ b/src/ripple/app/paths/PathRequests.cpp @@ -30,21 +30,22 @@ namespace ripple { -/** Get the current RippleLineCache, updating it if necessary. +/** Get the current AssetCache, updating it if necessary. Get the correct ledger to use. */ -std::shared_ptr -PathRequests::getLineCache( +std::shared_ptr +PathRequests::getAssetCache( std::shared_ptr const& ledger, bool authoritative) { std::lock_guard sl(mLock); - auto lineCache = lineCache_.lock(); + auto assetCache = assetCache_.lock(); - std::uint32_t const lineSeq = lineCache ? lineCache->getLedger()->seq() : 0; + std::uint32_t const lineSeq = + assetCache ? assetCache->getLedger()->seq() : 0; std::uint32_t const lgrSeq = ledger->seq(); - JLOG(mJournal.debug()) << "getLineCache has cache for " << lineSeq + JLOG(mJournal.debug()) << "getAssetCache has cache for " << lineSeq << ", considering " << lgrSeq; if ((lineSeq == 0) || // no ledger @@ -54,14 +55,14 @@ PathRequests::getLineCache( (lgrSeq > (lineSeq + 8))) // we jumped way forward for some reason { JLOG(mJournal.debug()) - << "getLineCache creating new cache for " << lgrSeq; + << "getAssetCache creating new cache for " << lgrSeq; // Assign to the local before the member, because the member is a // weak_ptr, and will immediately discard it if there are no other // references. - lineCache_ = lineCache = std::make_shared( - ledger, app_.journal("RippleLineCache")); + assetCache_ = assetCache = + std::make_shared(ledger, app_.journal("AssetCache")); } - return lineCache; + return assetCache; } void @@ -71,13 +72,13 @@ PathRequests::updateAll(std::shared_ptr const& inLedger) app_.getJobQueue().makeLoadEvent(jtPATH_FIND, "PathRequest::updateAll"); std::vector requests; - std::shared_ptr cache; + std::shared_ptr cache; // Get the ledger and cache we should be using { std::lock_guard sl(mLock); requests = requests_; - cache = getLineCache(inLedger, true); + cache = getAssetCache(inLedger, true); } bool newRequests = app_.getLedgerMaster().isNewPathRequest(); @@ -202,7 +203,7 @@ PathRequests::updateAll(std::shared_ptr const& inLedger) // Hold on to the line cache until after the lock is released, so it can // be destroyed outside of the lock - std::shared_ptr lastCache; + std::shared_ptr lastCache; { // Get the latest requests, cache, and ledger for next pass std::lock_guard sl(mLock); @@ -211,7 +212,7 @@ PathRequests::updateAll(std::shared_ptr const& inLedger) break; requests = requests_; lastCache = cache; - cache = getLineCache(cache->getLedger(), false); + cache = getAssetCache(cache->getLedger(), false); } } while (!app_.getJobQueue().isStopping()); @@ -255,7 +256,7 @@ PathRequests::makePathRequest( app_, subscriber, ++mLastIdentifier, *this, mJournal); auto [valid, jvRes] = - req->doCreate(getLineCache(inLedger, false), requestJson); + req->doCreate(getAssetCache(inLedger, false), requestJson); if (valid) { @@ -280,7 +281,8 @@ PathRequests::makeLegacyPathRequest( req = std::make_shared( app_, completion, consumer, ++mLastIdentifier, *this, mJournal); - auto [valid, jvRes] = req->doCreate(getLineCache(inLedger, false), request); + auto [valid, jvRes] = + req->doCreate(getAssetCache(inLedger, false), request); if (!valid) { @@ -306,8 +308,8 @@ PathRequests::doLegacyPathRequest( std::shared_ptr const& inLedger, Json::Value const& request) { - auto cache = std::make_shared( - inLedger, app_.journal("RippleLineCache")); + auto cache = + std::make_shared(inLedger, app_.journal("AssetCache")); auto req = std::make_shared( app_, [] {}, consumer, ++mLastIdentifier, *this, mJournal); diff --git a/src/ripple/app/paths/PathRequests.h b/src/ripple/app/paths/PathRequests.h index db683ee4c13..1f977c62039 100644 --- a/src/ripple/app/paths/PathRequests.h +++ b/src/ripple/app/paths/PathRequests.h @@ -21,8 +21,8 @@ #define RIPPLE_APP_PATHS_PATHREQUESTS_H_INCLUDED #include +#include #include -#include #include #include #include @@ -54,8 +54,8 @@ class PathRequests bool requestsPending() const; - std::shared_ptr - getLineCache( + std::shared_ptr + getAssetCache( std::shared_ptr const& ledger, bool authoritative); @@ -111,8 +111,8 @@ class PathRequests // Track all requests std::vector requests_; - // Use a RippleLineCache - std::weak_ptr lineCache_; + // Use a AssetCache + std::weak_ptr assetCache_; std::atomic mLastIdentifier; diff --git a/src/ripple/app/paths/Pathfinder.cpp b/src/ripple/app/paths/Pathfinder.cpp index 0b49036f694..6086b434a8a 100644 --- a/src/ripple/app/paths/Pathfinder.cpp +++ b/src/ripple/app/paths/Pathfinder.cpp @@ -19,9 +19,9 @@ #include #include +#include #include #include -#include #include #include #include @@ -183,7 +183,7 @@ assetFromPathAsset(PathAsset const& pathAsset, AccountID const& account) } // namespace Pathfinder::Pathfinder( - std::shared_ptr const& cache, + std::shared_ptr const& cache, AccountID const& uSrcAccount, AccountID const& uDstAccount, PathAsset const& uSrcPathAsset, @@ -202,7 +202,7 @@ Pathfinder::Pathfinder( , mSrcAmount(amountFromPathAsset(uSrcPathAsset, uSrcIssuer, uSrcAccount)) , convert_all_(convertAllCheck(mDstAmount)) , mLedger(cache->getLedger()) - , mRLCache(cache) + , mAssetCache(cache) , app_(app) , j_(app.journal("Pathfinder")) { @@ -725,8 +725,8 @@ int Pathfinder::getPathsOut( PathAsset const& pathAsset, AccountID const& account, - LineDirection direction, - bool isDstCurrency, + std::optional direction, + bool isDstAsset, AccountID const& dstAccount, std::function const& continueCallback) { @@ -748,8 +748,16 @@ Pathfinder::getPathsOut( return 0; int aFlags = sleAccount->getFieldU32(sfFlags); - bool const bAuthRequired = (aFlags & lsfRequireAuth) != 0; - bool const bFrozen = ((aFlags & lsfGlobalFreeze) != 0); + bool const bAuthRequired = [&]() { + if (pathAsset.isCurrency()) + return (aFlags & lsfRequireAuth) != 0; + return requireAuth(*mLedger, asset.mptIssue(), account) != tesSUCCESS; + }(); + bool const bFrozen = [&]() { + if (pathAsset.isCurrency()) + return (aFlags & lsfGlobalFreeze) != 0; + return isGlobalFrozen(*mLedger, asset.mptIssue()); + }(); int count = 0; @@ -757,10 +765,11 @@ Pathfinder::getPathsOut( { count = app_.getOrderBookDB().getBookSize(asset); - // TODO MPT - if (pathAsset.isCurrency()) + if (asset.isIssue()) { - if (auto const lines = mRLCache->getRippleLines(account, direction)) + assert(direction); + if (auto const lines = + mAssetCache->getRippleLines(account, *direction)) { for (auto const& rspEntry : *lines) { @@ -776,8 +785,7 @@ Pathfinder::getPathsOut( { } else if ( - isDstCurrency && - dstAccount == rspEntry.getAccountIDPeer()) + isDstAsset && dstAccount == rspEntry.getAccountIDPeer()) { count += 10000; // count a path to the destination extra @@ -797,6 +805,29 @@ Pathfinder::getPathsOut( } } } + else if (auto const mpts = mAssetCache->getMPTs(account)) + { + for (auto const& mpt : *mpts) + { + if (pathAsset.mpt() != mpt) + { + } + // TODO MPT is this correct + else if ( + bAuthRequired && + requireAuth(*mLedger, MPTIssue{mpt}, account) != tesSUCCESS) + { + } + else if (isDstAsset && dstAccount == mpt.second) + { + count += 10000; + } + else + { + ++count; + } + } + } } it->second = count; return count; @@ -1010,23 +1041,37 @@ Pathfinder::addLink( bool const bIsNoRippleOut(isNoRippleOut(currentPath)); bool const bDestOnly(addFlags & afAC_LAST); - // TODO MPT - if (auto const lines = mRLCache->getRippleLines( - uEndAccount, - bIsNoRippleOut ? LineDirection::incoming - : LineDirection::outgoing)) - { - auto& rippleLines = *lines; + AccountCandidates candidates; + + auto forAssets = [&]( + AssetType const& assets) { + candidates.reserve(assets.size()); - AccountCandidates candidates; - candidates.reserve(rippleLines.size()); + static bool constexpr isLine = std:: + is_same_v>; + static bool constexpr isMPT = + std::is_same_v>; - for (auto const& rs : rippleLines) + for (auto const& asset : assets) { if (continueCallback && !continueCallback()) return; - auto const& acct = rs.getAccountIDPeer(); - LineDirection const direction = rs.getDirectionPeer(); + auto const& acct = [&]() constexpr + { + if constexpr (isLine) + return asset.getAccountIDPeer(); + if constexpr (isMPT) + return asset.second; + } + (); + auto const direction = [&]() constexpr->std::optional< + LineDirection> + { + if constexpr (isLine) + return asset.getDirectionPeer(); + return std::nullopt; + } + (); if (hasEffectiveDestination && (acct == mDstAccount)) { @@ -1041,29 +1086,42 @@ Pathfinder::addLink( continue; } - if ((uEndPathAsset.isCurrency() && - uEndPathAsset.currency() == - rs.getLimit().getCurrency()) && + auto const correctAsset = [&]() { + if constexpr (isLine) + return uEndPathAsset.currency() == + asset.getLimit().getCurrency(); + if constexpr (isMPT) + return uEndPathAsset.mpt() == asset; + }(); + auto checkLine = [&]() { + if constexpr (isLine) + { + return ( + (asset.getBalance() <= beast::zero && + (!asset.getLimitPeer() || + -asset.getBalance() >= + asset.getLimitPeer() || + (bRequireAuth && !asset.getAuth()))) || + (bIsNoRippleOut && asset.getNoRipple())); + } + if constexpr (isMPT) + return false; + }; + + if (correctAsset && !currentPath.hasSeen(acct, uEndPathAsset, acct)) { // path is for correct currency and has not been // seen - if (rs.getBalance() <= beast::zero && - (!rs.getLimitPeer() || - -rs.getBalance() >= rs.getLimitPeer() || - (bRequireAuth && !rs.getAuth()))) - { - // path has no credit - } - else if (bIsNoRippleOut && rs.getNoRipple()) + if (checkLine()) { + // path has no credit or // Can't leave on this path } else if (bToDestination) { // destination is always worth trying - if (uEndPathAsset.currency() == - mDstAmount.getCurrency()) + if (uEndPathAsset == mDstAmount.asset()) { // this is a complete path if (!currentPath.empty()) @@ -1102,40 +1160,54 @@ Pathfinder::addLink( } } } + }; - if (!candidates.empty()) + if (uEndPathAsset.isCurrency()) + { + if (auto const lines = mAssetCache->getRippleLines( + uEndAccount, + bIsNoRippleOut ? LineDirection::incoming + : LineDirection::outgoing)) { - std::sort( - candidates.begin(), - candidates.end(), - std::bind( - compareAccountCandidate, - mLedger->seq(), - std::placeholders::_1, - std::placeholders::_2)); - - int count = candidates.size(); - // allow more paths from source - if ((count > 10) && (uEndAccount != mSrcAccount)) - count = 10; - else if (count > 50) - count = 50; - - auto it = candidates.begin(); - while (count-- != 0) - { - if (continueCallback && !continueCallback()) - return; - // Add accounts to incompletePaths - STPathElement pathElement( - STPathElement::typeAccount, - it->account, - uEndPathAsset, - it->account); - incompletePaths.assembleAdd( - currentPath, pathElement); - ++it; - } + forAssets(*lines); + } + } + else if (auto const mpts = mAssetCache->getMPTs(uEndAccount)) + { + forAssets(*mpts); + } + + if (!candidates.empty()) + { + std::sort( + candidates.begin(), + candidates.end(), + std::bind( + compareAccountCandidate, + mLedger->seq(), + std::placeholders::_1, + std::placeholders::_2)); + + int count = candidates.size(); + // allow more paths from source + if ((count > 10) && (uEndAccount != mSrcAccount)) + count = 10; + else if (count > 50) + count = 50; + + auto it = candidates.begin(); + while (count-- != 0) + { + if (continueCallback && !continueCallback()) + return; + // Add accounts to incompletePaths + STPathElement pathElement( + STPathElement::typeAccount, + it->account, + uEndPathAsset, + it->account); + incompletePaths.assembleAdd(currentPath, pathElement); + ++it; } } } @@ -1193,7 +1265,7 @@ Pathfinder::addLink( xrpCurrency(), xrpAccount()); - if (mDstAmount.getCurrency().isZero()) + if (isXRP(mDstAmount.asset())) { // destination is XRP, add account and path is // complete @@ -1254,6 +1326,7 @@ Pathfinder::addLink( } else { + // TODO MPT why asset and issuer are also included? // add issuer's account, path still incomplete incompletePaths.assembleAdd( newPath, diff --git a/src/ripple/app/paths/Pathfinder.h b/src/ripple/app/paths/Pathfinder.h index 074c188a61d..3512a4dd3b0 100644 --- a/src/ripple/app/paths/Pathfinder.h +++ b/src/ripple/app/paths/Pathfinder.h @@ -21,7 +21,7 @@ #define RIPPLE_APP_PATHS_PATHFINDER_H_INCLUDED #include -#include +#include #include #include #include @@ -40,7 +40,7 @@ class Pathfinder : public CountedObject public: /** Construct a pathfinder without an issuer.*/ Pathfinder( - std::shared_ptr const& cache, + std::shared_ptr const& cache, AccountID const& srcAccount, AccountID const& dstAccount, PathAsset const& uSrcPathAsset, @@ -144,7 +144,7 @@ class Pathfinder : public CountedObject getPathsOut( PathAsset const& pathAsset, AccountID const& account, - LineDirection direction, + std::optional direction, bool isDestPathAsset, AccountID const& dest, std::function const& continueCallback); @@ -207,7 +207,7 @@ class Pathfinder : public CountedObject std::shared_ptr mLedger; std::unique_ptr m_loadEvent; - std::shared_ptr mRLCache; + std::shared_ptr mAssetCache; STPathElement mSource; STPathSet mCompletePaths; diff --git a/src/ripple/app/paths/impl/PathfinderUtils.h b/src/ripple/app/paths/impl/PathfinderUtils.h index e00f44878bb..2ec8011842f 100644 --- a/src/ripple/app/paths/impl/PathfinderUtils.h +++ b/src/ripple/app/paths/impl/PathfinderUtils.h @@ -30,7 +30,10 @@ largestAmount(STAmount const& amt) if (amt.native()) return INITIAL_XRP; - return STAmount(amt.asset(), STAmount::cMaxValue, STAmount::cMaxOffset); + if (amt.isIssue()) + return STAmount(amt.asset(), STAmount::cMaxValue, STAmount::cMaxOffset); + // TODO MPT + return STAmount(amt.asset(), STAmount::cMaxNativeN, 0); } inline STAmount diff --git a/src/ripple/app/paths/impl/PaySteps.cpp b/src/ripple/app/paths/impl/PaySteps.cpp index eb96e189251..5a1dcf3540f 100644 --- a/src/ripple/app/paths/impl/PaySteps.cpp +++ b/src/ripple/app/paths/impl/PaySteps.cpp @@ -222,7 +222,8 @@ toStrand( if (hasAccount && (pe.getAccountID() == noAccount())) return {temBAD_PATH, Strand{}}; - if (hasMPT && (hasIssuer || hasCurrency || hasAccount)) + // TODO MPT validation + if (hasMPT && hasCurrency) return {temBAD_PATH, Strand{}}; } @@ -353,6 +354,10 @@ toStrand( auto cur = &normPath[i]; auto const next = &normPath[i + 1]; + // TODO MPT is this valid? + if (curAsset.isMPT() && cur->hasCurrency()) + curAsset = Issue{}; + if (curAsset.isIssue()) { if (cur->isAccount()) diff --git a/src/ripple/protocol/impl/STPathSet.cpp b/src/ripple/protocol/impl/STPathSet.cpp index 23b5fc0ca5b..4113182ed5e 100644 --- a/src/ripple/protocol/impl/STPathSet.cpp +++ b/src/ripple/protocol/impl/STPathSet.cpp @@ -41,7 +41,10 @@ STPathElement::get_hash(STPathElement const& element) for (auto const x : element.getAccountID()) hash_account += (hash_account * 257) ^ x; - if (element.hasMPT()) + // Check pathAsset type instead of element's mType + // In some cases mType might be account but the asset + // is still set to either MPT or currency (see Pathfinder::addLink()) + if (element.getPathAsset().isMPT()) { hash_currency += beast::uhash<>{}(element.getPathAsset().mpt()); } diff --git a/src/ripple/rpc/impl/TransactionSign.cpp b/src/ripple/rpc/impl/TransactionSign.cpp index 2dda1169d64..c7f444be389 100644 --- a/src/ripple/rpc/impl/TransactionSign.cpp +++ b/src/ripple/rpc/impl/TransactionSign.cpp @@ -242,8 +242,8 @@ checkPayment( if (auto ledger = app.openLedger().current()) { Pathfinder pf( - std::make_shared( - ledger, app.journal("RippleLineCache")), + std::make_shared( + ledger, app.journal("AssetCache")), srcAddressID, *dstAccountID, sendMax.issue().currency, diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index ad80f1eeeca..689ef3a2c5c 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -1542,6 +1542,198 @@ class MPToken_test : public beast::unit_test::suite } } + void + testPath(FeatureBitset features) + { + using namespace test::jtx; + Account const gw{"gw"}; + Account const gw1{"gw1"}; + Account const alice{"alice"}; + Account const carol{"carol"}; + Account const bob{"bob"}; + Account const dan{"dan"}; + auto const USD = gw["USD"]; + auto const EUR = gw1["EUR"]; + + // MPT can be a mpt end point step or a book-step + + // Direct MPT payment + { + Env env = pathTestEnv(*this); + + MPTTester mpt(env, gw, {.holders = {&dan, &carol}}); + mpt.create({.ownerCount = 1, .holderCount = 0}); + mpt.authorize({.account = &dan}); + mpt.authorize({.account = &carol}); + mpt.pay(gw, carol, 200); + + auto const [pathSet, srcAmt, dstAmt] = + find_paths(env, carol, dan, mpt.mpt(-1)); + BEAST_EXPECT(srcAmt == mpt.mpt(200)); + BEAST_EXPECT(dstAmt == mpt.mpt(200)); + // Direct payment, no path + BEAST_EXPECT(pathSet.empty()); + } + + // Cross-asset payment via XRP/MPT offer (one step) + { + Env env = pathTestEnv(*this); + + env.fund(XRP(1'000), carol); + + MPTTester mpt(env, gw, {.holders = {&alice, &dan}}); + + mpt.create({.ownerCount = 1, .holderCount = 0}); + + mpt.authorize({.account = &alice}); + mpt.authorize({.account = &dan}); + mpt.pay(gw, alice, 200); + + env(offer(alice, XRP(100), mpt.mpt(100))); + env.close(); + + auto const [pathSet, srcAmt, dstAmt] = + find_paths(env, carol, dan, mpt.mpt(-1)); + BEAST_EXPECT(srcAmt == XRP(100)); + BEAST_EXPECT(dstAmt == mpt.mpt(100)); + // This path is consistent with XRP/IOU. + BEAST_EXPECT(same(pathSet, stpath(IPE(mpt.MPT().mpt())))); + } + + // Cross-asset payment via IOU/MPT offer (one step) + { + Env env = pathTestEnv(*this); + + env.fund(XRP(1'000), carol); + env.fund(XRP(1'000), gw); + + MPTTester mpt(env, gw1, {.holders = {&alice, &dan}}); + + mpt.create({.ownerCount = 1, .holderCount = 0}); + + mpt.authorize({.account = &alice}); + mpt.authorize({.account = &dan}); + mpt.pay(gw1, alice, 200); + + env(trust(alice, USD(400))); + env(trust(carol, USD(400))); + env(pay(gw, carol, USD(200))); + + env(offer(alice, USD(100), mpt.mpt(100))); + env.close(); + + auto const [pathSet, srcAmt, dstAmt] = + find_paths(env, carol, dan, mpt.mpt(-1)); + BEAST_EXPECT(srcAmt == USD(100)); + BEAST_EXPECT(dstAmt == mpt.mpt(100)); + // This path is consistent with IOU1/gw1 / IOU/gw + BEAST_EXPECT(same(pathSet, stpath(gw, IPE(mpt.MPT().mpt())))); + } + + // Cross-asset payment via MPT1/MPT offer (one step) + { + Env env = pathTestEnv(*this); + + MPTTester mpt(env, gw, {.holders = {&alice, &dan}}); + MPTTester mpt1(env, gw1, {.holders = {&carol}}); + + mpt.create({.ownerCount = 1, .holderCount = 0}); + mpt1.create({.ownerCount = 1, .holderCount = 0}); + + mpt.authorize({.account = &alice}); + mpt.authorize({.account = &dan}); + mpt.pay(gw, alice, 200); + + mpt1.authorize({.account = &carol}); + mpt1.authorize({.account = &alice}); + mpt1.pay(gw1, carol, 200); + + env(offer(alice, mpt1.mpt(100), mpt.mpt(100))); + env.close(); + + auto const [pathSet, srcAmt, dstAmt] = + find_paths(env, carol, dan, mpt.mpt(-1)); + BEAST_EXPECT(srcAmt == mpt1.mpt(100)); + BEAST_EXPECT(dstAmt == mpt.mpt(100)); + // This path is consistent with IOU1/gw / IOU/gw path - + // [gw1, IOU/gw], except for gw1. This is due to no MPT rippling + BEAST_EXPECT(same(pathSet, stpath(IPE(mpt.MPT().mpt())))); + } + + // Cross-asset payment via offers (two steps) + { + Env env = pathTestEnv(*this); + + env.fund(XRP(1'000), carol); + env.fund(XRP(1'000), dan); + + MPTTester mpt(env, gw, {.holders = {&alice, &bob}}); + + mpt.create({.ownerCount = 1, .holderCount = 0}); + + mpt.authorize({.account = &alice}); + mpt.authorize({.account = &bob}); + mpt.pay(gw, alice, 200); + mpt.pay(gw, bob, 200); + + env(trust(bob, USD(200))); + env(pay(gw, bob, USD(100))); + env(trust(dan, USD(200))); + env(trust(alice, USD(200))); + + env(offer(alice, XRP(100), mpt.mpt(100))); + env(offer(bob, mpt.mpt(100), USD(100))); + env.close(); + + auto const [pathSet, srcAmt, dstAmt] = + find_paths(env, carol, dan, USD(-1)); + BEAST_EXPECT(srcAmt == XRP(100)); + BEAST_EXPECT(dstAmt == USD(100)); + // This path is consistent with XRP/ IOU1/gw - IOU1/gw1 / IOU/gw + BEAST_EXPECT(same(pathSet, stpath(IPE(mpt.MPT().mpt()), IPE(USD)))); + } + + // Cross-asset payment via offers (two steps) + // Start/End with mpt/mp1 and book steps in the middle + { + Env env = pathTestEnv(*this); + Account const gw2{"gw2"}; + env.fund(XRP(1'000), gw2); + auto const USD2 = gw2["USD"]; + + MPTTester mpt(env, gw, {.holders = {&alice, &carol}}); + mpt.create({.ownerCount = 1, .holderCount = 0}); + mpt.authorize({.account = &alice}); + mpt.authorize({.account = &carol}); + mpt.pay(gw, carol, 200); + + MPTTester mpt1(env, gw1, {.holders = {&bob, &dan}}); + mpt1.create({.ownerCount = 1, .holderCount = 0}); + mpt1.authorize({.account = &bob}); + mpt1.pay(gw1, bob, 200); + mpt1.authorize({.account = &dan}); + + env(trust(alice, USD2(400))); + env(pay(gw2, alice, USD2(200))); + env(trust(bob, USD2(400))); + + env(offer(alice, mpt.mpt(100), USD2(100))); + env(offer(bob, USD2(100), mpt1.mpt(100))); + env.close(); + + auto const [pathSet, srcAmt, dstAmt] = + find_paths(env, carol, dan, mpt1.mpt(-1)); + BEAST_EXPECT(srcAmt == mpt.mpt(100)); + BEAST_EXPECT(dstAmt == mpt1.mpt(100)); + // This path is consistent with IOU/gw / IOU/gw2 - + // IOU/gw2 / IOU1/gw1 path - + // [gw, IOU2/gw2, IOU1/gw1], except for gw. + // This is due to no MPT rippling + BEAST_EXPECT( + same(pathSet, stpath(IPE(USD2), IPE(mpt1.MPT().mpt())))); + } + } + public: void run() override @@ -1576,6 +1768,9 @@ class MPToken_test : public beast::unit_test::suite testOfferCrossing(all); testCrossAssetPayment(all); + // Path-finding + testPath(all); + // Test MPT Amount is invalid in non-Payment Tx testMPTInvalidInTx(all); diff --git a/src/test/jtx/TestHelpers.h b/src/test/jtx/TestHelpers.h index b065cec470a..093f4614e0d 100644 --- a/src/test/jtx/TestHelpers.h +++ b/src/test/jtx/TestHelpers.h @@ -20,11 +20,15 @@ #define RIPPLE_TEST_JTX_TESTHELPERS_H_INCLUDED #include +#include #include #include #include +#include #include #include +#include +#include #include #include @@ -136,6 +140,9 @@ equal(STAmount const& sa1, STAmount const& sa2); STPathElement IPE(Issue const& iss); +STPathElement +IPE(MPTIssue const& iss); + template STPath stpath(Args const&... args) @@ -162,6 +169,63 @@ same(STPathSet const& st1, Args const&... args) return true; } +Json::Value +rpf(jtx::Account const& src, + jtx::Account const& dst, + STAmount const& dstAmount, + std::optional const& sendMax = std::nullopt, + std::optional const& srcCurrency = std::nullopt); + +jtx::Env +pathTestEnv(beast::unit_test::suite& suite); + +class gate +{ +private: + std::condition_variable cv_; + std::mutex mutex_; + bool signaled_ = false; + +public: + // Thread safe, blocks until signaled or period expires. + // Returns `true` if signaled. + template + bool + wait_for(std::chrono::duration const& rel_time) + { + std::unique_lock lk(mutex_); + auto b = cv_.wait_for(lk, rel_time, [this] { return signaled_; }); + signaled_ = false; + return b; + } + + void + signal() + { + std::lock_guard lk(mutex_); + signaled_ = true; + cv_.notify_all(); + } +}; + +Json::Value +find_paths_request( + jtx::Env& env, + jtx::Account const& src, + jtx::Account const& dst, + STAmount const& saDstAmount, + std::optional const& saSendMax = std::nullopt, + std::optional const& saSrcCurrency = std::nullopt); + +std::tuple +find_paths( + jtx::Env& env, + jtx::Account const& src, + jtx::Account const& dst, + STAmount const& saDstAmount, + std::optional const& saSendMax = std::nullopt, + std::optional const& saSrcCurrency = std::nullopt); + /******************************************************************************/ XRPAmount diff --git a/src/test/jtx/impl/TestHelpers.cpp b/src/test/jtx/impl/TestHelpers.cpp index 9d196fdb913..c39053733ae 100644 --- a/src/test/jtx/impl/TestHelpers.cpp +++ b/src/test/jtx/impl/TestHelpers.cpp @@ -84,6 +84,160 @@ IPE(Issue const& iss) PathAsset{iss.currency}, iss.account); } +STPathElement +IPE(MPTIssue const& iss) +{ + return STPathElement( + STPathElement::typeMPT | STPathElement::typeIssuer, + xrpAccount(), + PathAsset{iss.mpt()}, + iss.account()); +} + +Json::Value +rpf(jtx::Account const& src, + jtx::Account const& dst, + STAmount const& dstAmount, + std::optional const& sendMax, + std::optional const& srcCurrency) +{ + Json::Value jv = Json::objectValue; + jv[jss::command] = "ripple_path_find"; + jv[jss::source_account] = toBase58(src); + jv[jss::destination_account] = toBase58(dst); + jv[jss::destination_amount] = dstAmount.getJson(JsonOptions::none); + if (sendMax) + jv[jss::send_max] = sendMax->getJson(JsonOptions::none); + if (srcCurrency) + { + auto& sc = jv[jss::source_currencies] = Json::arrayValue; + Json::Value j = Json::objectValue; + j[jss::currency] = to_string(srcCurrency.value()); + sc.append(j); + } + + return jv; +} + +jtx::Env +pathTestEnv(beast::unit_test::suite& suite) +{ + // These tests were originally written with search parameters that are + // different from the current defaults. This function creates an env + // with the search parameters that the tests were written for. + using namespace jtx; + return Env(suite, envconfig([](std::unique_ptr cfg) { + cfg->PATH_SEARCH_OLD = 7; + cfg->PATH_SEARCH = 7; + cfg->PATH_SEARCH_MAX = 10; + return cfg; + })); +} + +Json::Value +find_paths_request( + jtx::Env& env, + jtx::Account const& src, + jtx::Account const& dst, + STAmount const& saDstAmount, + std::optional const& saSendMax, + std::optional const& saSrcCurrency) +{ + using namespace jtx; + + auto& app = env.app(); + Resource::Charge loadType = Resource::feeReferenceRPC; + Resource::Consumer c; + + RPC::JsonContext context{ + {env.journal, + app, + loadType, + app.getOPs(), + app.getLedgerMaster(), + c, + Role::USER, + {}, + {}, + RPC::apiVersionIfUnspecified}, + {}, + {}}; + + Json::Value params = Json::objectValue; + params[jss::command] = "ripple_path_find"; + params[jss::source_account] = toBase58(src); + params[jss::destination_account] = toBase58(dst); + params[jss::destination_amount] = saDstAmount.getJson(JsonOptions::none); + if (saSendMax) + params[jss::send_max] = saSendMax->getJson(JsonOptions::none); + if (saSrcCurrency) + { + auto& sc = params[jss::source_currencies] = Json::arrayValue; + Json::Value j = Json::objectValue; + j[jss::currency] = to_string(saSrcCurrency.value()); + sc.append(j); + } + + Json::Value result; + gate g; + app.getJobQueue().postCoro(jtCLIENT, "RPC-Client", [&](auto const& coro) { + context.params = std::move(params); + context.coro = coro; + RPC::doCommand(context, result); + g.signal(); + }); + + using namespace std::chrono_literals; + using namespace beast::unit_test; + g.wait_for(5s); + return result; +} + +std::tuple +find_paths( + jtx::Env& env, + jtx::Account const& src, + jtx::Account const& dst, + STAmount const& saDstAmount, + std::optional const& saSendMax, + std::optional const& saSrcCurrency) +{ + Json::Value result = find_paths_request( + env, src, dst, saDstAmount, saSendMax, saSrcCurrency); + if (result.isMember(jss::error)) + return std::make_tuple(STPathSet{}, STAmount{}, STAmount{}); + + STAmount da; + if (result.isMember(jss::destination_amount)) + da = amountFromJson(sfGeneric, result[jss::destination_amount]); + + STAmount sa; + STPathSet paths; + if (result.isMember(jss::alternatives)) + { + auto const& alts = result[jss::alternatives]; + if (alts.size() > 0) + { + auto const& path = alts[0u]; + + if (path.isMember(jss::source_amount)) + sa = amountFromJson(sfGeneric, path[jss::source_amount]); + + if (path.isMember(jss::destination_amount)) + da = amountFromJson(sfGeneric, path[jss::destination_amount]); + + if (path.isMember(jss::paths_computed)) + { + Json::Value p; + p["Paths"] = path[jss::paths_computed]; + STParsedJSONObject po("generic", p); + paths = po.object->getFieldPathSet(sfPaths); + } + } + } + + return std::make_tuple(std::move(paths), std::move(sa), std::move(da)); +} /******************************************************************************/ diff --git a/src/test/jtx/impl/paths.cpp b/src/test/jtx/impl/paths.cpp index 80ce6c05d9c..5a0ed44a48e 100644 --- a/src/test/jtx/impl/paths.cpp +++ b/src/test/jtx/impl/paths.cpp @@ -33,8 +33,8 @@ paths::operator()(Env& env, JTx& jt) const auto const to = env.lookup(jv[jss::Destination].asString()); auto const amount = amountFromJson(sfAmount, jv[jss::Amount]); Pathfinder pf( - std::make_shared( - env.current(), env.app().journal("RippleLineCache")), + std::make_shared( + env.current(), env.app().journal("AssetCache")), from, to, in_.currency,