Pathfinding bugfixes (RIPD-735):

* Fix specifying paths and search level for ripple_path_find
* Don't modify the pathfinder, it has issuer-neutral paths.
* Handle previous paths correctly
* Compare paths correctly
This commit is contained in:
JoelKatz
2014-12-23 14:39:30 -08:00
committed by Nik Bougalis
parent 4f2d93bb65
commit 98d5eefc86
5 changed files with 163 additions and 135 deletions

View File

@@ -43,15 +43,16 @@ public:
bool findPathsForIssue (
Issue const& issue,
STPathSet& pathsOut,
STPathSet& pathsInOut,
STPath& fullLiquidityPath)
{
if (auto& pathfinder = getPathFinder (issue.currency))
{
pathsOut = pathfinder->getBestPaths (
maxPaths_, fullLiquidityPath, issue.account);
pathsInOut = pathfinder->getBestPaths (
maxPaths_, fullLiquidityPath, pathsInOut, issue.account);
return true;
}
assert (false);
return false;
}
@@ -99,10 +100,10 @@ FindPaths::~FindPaths() = default;
bool FindPaths::findPathsForIssue (
Issue const& issue,
STPathSet& pathsOut,
STPathSet& pathsInOut,
STPath& fullLiquidityPath)
{
return impl_->findPathsForIssue (issue, pathsOut, fullLiquidityPath);
return impl_->findPathsForIssue (issue, pathsInOut, fullLiquidityPath);
}
bool findPathsForOneIssuer (
@@ -113,7 +114,7 @@ bool findPathsForOneIssuer (
STAmount const& dstAmount,
int searchLevel,
unsigned int const maxPaths,
STPathSet& pathsOut,
STPathSet& pathsInOut,
STPath& fullLiquidityPath)
{
Pathfinder pf (
@@ -127,9 +128,8 @@ bool findPathsForOneIssuer (
if (!pf.findPaths (searchLevel))
return false;
pf.addPathsFromPreviousPathfinding (pathsOut);
pf.computePathRanks (maxPaths);
pathsOut = pf.getBestPaths(maxPaths, fullLiquidityPath, srcIssue.account);
pathsInOut = pf.getBestPaths(maxPaths, fullLiquidityPath, pathsInOut, srcIssue.account);
return true;
}

View File

@@ -41,12 +41,12 @@ public:
bool findPathsForIssue (
Issue const& issue,
/** On input, pathsOut contains any paths you want to ensure are
/** On input, pathsInOut contains any paths you want to ensure are
included if still good.
On output, pathsOut will have any additional paths found. Only
On output, pathsInOut will have any additional paths found. Only
non-default paths without source or destination will be added. */
STPathSet& pathsOut,
STPathSet& pathsInOut,
/** On input, fullLiquidityPath must be an empty STPath.
@@ -73,12 +73,12 @@ bool findPathsForOneIssuer (
pathsOut. */
unsigned int const maxPaths,
/** On input, pathsOut contains any paths you want to ensure are included if
/** On input, pathsInOut contains any paths you want to ensure are included if
still good.
On output, pathsOut will have any additional paths found. Only
On output, pathsInOut will have any additional paths found. Only
non-default paths without source or destination will be added. */
STPathSet& pathsOut,
STPathSet& pathsInOut,
/** On input, fullLiquidityPath must be an empty STPath.

View File

@@ -23,47 +23,46 @@
#include <ripple/core/JobQueue.h>
#include <tuple>
/*
Core Pathfinding Engine
The pathfinding request is identified by category, XRP to XRP, XRP to
non-XRP, non-XRP to XRP, same currency non-XRP to non-XRP, cross-currency
non-XRP to non-XRP. For each category, there is a table of paths that the
pathfinder searches for. Complete paths are collected.
Each complete path is then rated and sorted. Paths with no or trivial
liquidity are dropped. Otherwise, paths are sorted based on quality,
liquidity, and path length.
Path slots are filled in quality (ratio of out to in) order, with the
exception that the last path must have enough liquidity to complete the
payment (assuming no liquidity overlap). In addition, if no selected path
is capable of providing enough liquidity to complete the payment by itself,
an extra "covering" path is returned.
The selected paths are then tested to determine if they can complete the
payment and, if so, at what cost. If they fail and a covering path was
found, the test is repeated with the covering path. If this succeeds, the
final paths and the estimated cost are returned.
The engine permits the search depth to be selected and the paths table
includes the depth at which each path type is found. A search depth of zero
causes no searching to be done. Extra paths can also be injected, and this
should be used to preserve previously-found paths across invokations for the
same path request (particularly if the search depth may change).
*/
namespace ripple {
/*
we just need to find a succession of the highest quality paths there until we
find enough width
Don't do branching within each path
We have a list of paths we are working on but how do we compare the ones that
are terminating in a different currency?
Loops
TODO: what is a good way to come up with multiple paths?
Maybe just change the sort criteria?
first a low cost one and then a fat short one?
OrderDB:
getXRPOffers();
// return list of all orderbooks that want XRP
// return list of all orderbooks that want Issuer
// return list of all orderbooks that want this issuer and currency
*/
/*
Test sending to XRP
Test XRP to XRP
Test offer in middle
Test XRP to USD
Test USD to EUR
*/
namespace {
// We sort possible paths by:
// cost of path
// length of path
// width of path
// correct currency at the end.
// Compare two PathRanks. A better PathRank is lower, so the best are sorted to
// the beginning.
@@ -339,35 +338,6 @@ bool Pathfinder::findPaths (int searchLevel)
return true;
}
void Pathfinder::addPathsFromPreviousPathfinding (STPathSet& pathsOut)
{
// Add any result paths that aren't in mCompletePaths.
// TODO(tom): this is also quadratic in the size of the paths.
for (auto const& path : pathsOut)
{
// make sure no paths were lost
bool found = false;
if (!path.empty ())
{
for (auto const& ePath : mCompletePaths)
{
if (ePath == path)
{
found = true;
break;
}
}
// TODO(tom): write a test that exercises this code path.
if (!found)
mCompletePaths.push_back (path);
}
}
WriteLog (lsDEBUG, Pathfinder)
<< mCompletePaths.size () << " paths to filter";
}
TER Pathfinder::getPathLiquidity (
STPath const& path, // IN: The path to check.
STAmount const& minDstAmount, // IN: The minimum output this path must
@@ -441,9 +411,6 @@ STAmount smallestUsefulAmount (STAmount const& amount, int maxPaths)
void Pathfinder::computePathRanks (int maxPaths)
{
if (mCompletePaths.size () <= maxPaths)
return;
mRemainingAmount = mDstAmount;
// Must subtract liquidity in default path from remaining amount.
@@ -479,34 +446,7 @@ void Pathfinder::computePathRanks (int maxPaths)
WriteLog (lsDEBUG, Pathfinder) << "Default path causes exception";
}
// Ignore paths that move only very small amounts.
auto saMinDstAmount = smallestUsefulAmount (mDstAmount, maxPaths);
// Get the PathRank for each path.
for (int i = 0; i < mCompletePaths.size (); ++i)
{
auto const& currentPath = mCompletePaths[i];
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);
mPathRanks.push_back ({uQuality, currentPath.size (), liquidity, i});
}
}
std::sort (mPathRanks.begin (), mPathRanks.end (), comparePathRank);
rankPaths (maxPaths, mCompletePaths, mPathRanks);
}
static bool isDefaultPath (STPath const& path)
@@ -540,37 +480,133 @@ static STPath removeIssuer (STPath const& path)
return ret;
}
// For each useful path in the input path set,
// create a ranking entry in the output vector of path ranks
void Pathfinder::rankPaths (
int maxPaths,
STPathSet const& paths,
std::vector <PathRank>& rankedPaths)
{
rankedPaths.clear ();
rankedPaths.reserve (paths.size());
// Ignore paths that move only very small amounts.
auto 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)
{
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});
}
}
std::sort (rankedPaths.begin (), rankedPaths.end (), comparePathRank);
}
STPathSet Pathfinder::getBestPaths (
int maxPaths,
STPath& fullLiquidityPath,
STPathSet& extraPaths,
Account const& srcIssuer)
{
WriteLog (lsDEBUG, Pathfinder) << "findPaths: " <<
mCompletePaths.size() << " paths and " <<
extraPaths.size () << " extras";
assert (fullLiquidityPath.empty ());
const bool issuerIsSender = isXRP (mSrcCurrency) || (srcIssuer == mSrcAccount);
if (issuerIsSender && (mCompletePaths.size () <= maxPaths))
if (issuerIsSender &&
(mCompletePaths.size () <= maxPaths) &&
(extraPaths.size() == 0))
return mCompletePaths;
std::vector <PathRank> extraPathRanks;
rankPaths (maxPaths, extraPaths, extraPathRanks);
STPathSet bestPaths;
// The best PathRanks are now at the start. Pull off enough of them to
// fill bestPaths, then look through the rest for the best individual
// path that can satisfy the entire liquidity - if one exists.
STAmount remaining = mRemainingAmount;
for (auto& pathRank: mPathRanks)
auto pathsIterator = mPathRanks.begin();
auto extraPathsIterator = extraPathRanks.begin();
while (pathsIterator != mPathRanks.end() ||
extraPathsIterator != extraPathRanks.end())
{
bool usePath = false;
bool useExtraPath = false;
if (pathsIterator == mPathRanks.end())
useExtraPath = true;
else if (extraPathsIterator == extraPathRanks.end())
usePath = true;
else if (extraPathsIterator->quality < pathsIterator->quality)
useExtraPath = true;
else if (extraPathsIterator->quality > pathsIterator->quality)
usePath = true;
else if (extraPathsIterator->liquidity > pathsIterator->liquidity)
useExtraPath = true;
else if (extraPathsIterator->liquidity < pathsIterator->liquidity)
usePath = true;
else
{
// Risk is high they have identical liquidity
useExtraPath = true;
usePath = true;
}
auto& pathRank = usePath ? *pathsIterator : *extraPathsIterator;
auto& path = usePath ? mCompletePaths[pathRank.index] :
extraPaths[pathRank.index];
if (useExtraPath)
++extraPathsIterator;
if (usePath)
++pathsIterator;
auto iPathsLeft = maxPaths - bestPaths.size ();
if (!(iPathsLeft > 0 || fullLiquidityPath.empty ()))
break;
auto& path = mCompletePaths[pathRank.index];
assert (!path.empty ());
if (path.empty ())
{
assert (false);
continue;
}
bool startsWithIssuer = false;
if (! issuerIsSender)
if (! issuerIsSender && usePath)
{
// Need to make sure path matches issuer constraints
if (path.front ().getAccountID() != srcIssuer)
continue;
if (isDefaultPath (path))

View File

@@ -54,9 +54,6 @@ public:
bool findPaths (int searchLevel);
/** Make sure that all the input paths are included in mCompletePaths. */
void addPathsFromPreviousPathfinding (STPathSet&);
/** Compute the rankings of the paths. */
void computePathRanks (int maxPaths);
@@ -68,6 +65,7 @@ public:
STPathSet getBestPaths (
int maxPaths,
STPath& fullLiquidityPath,
STPathSet& extraPaths,
Account const& srcIssuer);
enum NodeType
@@ -115,8 +113,6 @@ private:
isNoRippleOut:
isNoRipple
addPathsFromPreviousPathfinding
computePathRanks:
rippleCalculate
getPathLiquidity:
@@ -167,6 +163,11 @@ private:
Account const& toAccount,
Currency const& currency);
void rankPaths (
int maxPaths,
STPathSet const& paths,
std::vector <PathRank>& rankedPaths);
Account mSrcAccount;
Account mDstAccount;
STAmount mDstAmount;

View File

@@ -156,8 +156,8 @@ Json::Value doRipplePathFind (RPC::Context& context)
++level;
}
if (context.params.isMember("depth")
&& context.params["depth"].isIntegral())
if (context.params.isMember("search_depth")
&& context.params["search_depth"].isIntegral())
{
int rLev = context.params["search_depth"].asInt ();
if ((rLev < level) || (context.role == Role::ADMIN))
@@ -206,28 +206,19 @@ Json::Value doRipplePathFind (RPC::Context& context)
return rpcError (rpcSRC_ISR_MALFORMED);
}
int level = getConfig().PATH_SEARCH_OLD;
if ((getConfig().PATH_SEARCH_MAX > level)
&& !getApp().getFeeTrack().isLoadedLocal())
{
++level;
}
if (context.params.isMember("depth")
&& context.params["depth"].isIntegral())
{
int rLev = context.params["search_depth"].asInt ();
if ((rLev < level) || (context.role == Role::ADMIN))
level = rLev;
}
STPathSet spsComputed;
if (context.params.isMember("paths"))
{
STParsedJSONObject paths ("paths", context.params["paths"]);
Json::Value pathSet = Json::objectValue;
pathSet["Paths"] = context.params["paths"];
STParsedJSONObject paths ("pathSet", pathSet);
if (paths.object.get() == nullptr)
return paths.error;
else
spsComputed = paths.object.get()->downcast<STPathSet> ();
{
spsComputed = paths.object.get()->getFieldPathSet (sfPaths);
WriteLog (lsTRACE, RPCHandler) << "ripple_path_find: Paths: " << spsComputed.getJson (0);
}
}
STPath fullLiquidityPath;