Track total trustlines and avoid duplications

This commit is contained in:
Ed Hennis
2022-02-18 18:28:49 -05:00
committed by manojsdoshi
parent e836375d99
commit 04bd5878f1
8 changed files with 373 additions and 185 deletions

View File

@@ -33,18 +33,23 @@ accountSourceCurrencies(
if (includeXRP)
currencies.insert(xrpCurrency());
for (auto const& rspEntry : lrCache->getRippleLines(account, true))
if (auto const lines =
lrCache->getRippleLines(account, LineDirection::outgoing))
{
auto& saBalance = rspEntry.getBalance();
// Filter out non
if (saBalance > beast::zero
// Have IOUs to send.
|| (rspEntry.getLimitPeer()
// Peer extends credit.
&& ((-saBalance) < rspEntry.getLimitPeer()))) // Credit left.
for (auto const& rspEntry : *lines)
{
currencies.insert(saBalance.getCurrency());
auto& saBalance = rspEntry.getBalance();
// Filter out non
if (saBalance > beast::zero
// Have IOUs to send.
||
(rspEntry.getLimitPeer()
// Peer extends credit.
&& ((-saBalance) < rspEntry.getLimitPeer()))) // Credit left.
{
currencies.insert(saBalance.getCurrency());
}
}
}
@@ -64,12 +69,16 @@ accountDestCurrencies(
currencies.insert(xrpCurrency());
// Even if account doesn't exist
for (auto const& rspEntry : lrCache->getRippleLines(account, true))
if (auto const lines =
lrCache->getRippleLines(account, LineDirection::outgoing))
{
auto& saBalance = rspEntry.getBalance();
for (auto const& rspEntry : *lines)
{
auto& saBalance = rspEntry.getBalance();
if (saBalance < rspEntry.getLimit()) // Can take more
currencies.insert(saBalance.getCurrency());
if (saBalance < rspEntry.getLimit()) // Can take more
currencies.insert(saBalance.getCurrency());
}
}
currencies.erase(badCurrency());

View File

@@ -708,7 +708,7 @@ int
Pathfinder::getPathsOut(
Currency const& currency,
AccountID const& account,
bool outgoing,
LineDirection direction,
bool isDstCurrency,
AccountID const& dstAccount,
std::function<bool(void)> const& continueCallback)
@@ -736,33 +736,37 @@ Pathfinder::getPathsOut(
{
count = app_.getOrderBookDB().getBookSize(issue);
for (auto const& rspEntry : mRLCache->getRippleLines(account, outgoing))
if (auto const lines = mRLCache->getRippleLines(account, direction))
{
if (currency != rspEntry.getLimit().getCurrency())
for (auto const& rspEntry : *lines)
{
}
else if (
rspEntry.getBalance() <= beast::zero &&
(!rspEntry.getLimitPeer() ||
-rspEntry.getBalance() >= rspEntry.getLimitPeer() ||
(bAuthRequired && !rspEntry.getAuth())))
{
}
else if (isDstCurrency && dstAccount == rspEntry.getAccountIDPeer())
{
count += 10000; // count a path to the destination extra
}
else if (rspEntry.getNoRipplePeer())
{
// This probably isn't a useful path out
}
else if (rspEntry.getFreezePeer())
{
// Not a useful path out
}
else
{
++count;
if (currency != rspEntry.getLimit().getCurrency())
{
}
else if (
rspEntry.getBalance() <= beast::zero &&
(!rspEntry.getLimitPeer() ||
-rspEntry.getBalance() >= rspEntry.getLimitPeer() ||
(bAuthRequired && !rspEntry.getAuth())))
{
}
else if (
isDstCurrency && dstAccount == rspEntry.getAccountIDPeer())
{
count += 10000; // count a path to the destination extra
}
else if (rspEntry.getNoRipplePeer())
{
// This probably isn't a useful path out
}
else if (rspEntry.getFreezePeer())
{
// Not a useful path out
}
else
{
++count;
}
}
}
}
@@ -977,120 +981,128 @@ Pathfinder::addLink(
bool const bIsNoRippleOut(isNoRippleOut(currentPath));
bool const bDestOnly(addFlags & afAC_LAST);
auto& rippleLines(
mRLCache->getRippleLines(uEndAccount, !bIsNoRippleOut));
AccountCandidates candidates;
candidates.reserve(rippleLines.size());
for (auto const& rs : rippleLines)
if (auto const lines = mRLCache->getRippleLines(
uEndAccount,
bIsNoRippleOut ? LineDirection::incoming
: LineDirection::outgoing))
{
if (continueCallback && !continueCallback())
return;
auto const& acct = rs.getAccountIDPeer();
bool const outgoing = !rs.getNoRipplePeer();
auto& rippleLines = *lines;
if (hasEffectiveDestination && (acct == mDstAccount))
{
// We skipped the gateway
continue;
}
AccountCandidates candidates;
candidates.reserve(rippleLines.size());
bool bToDestination = acct == mEffectiveDst;
if (bDestOnly && !bToDestination)
{
continue;
}
if ((uEndCurrency == rs.getLimit().getCurrency()) &&
!currentPath.hasSeen(acct, uEndCurrency, 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())
{
// Can't leave on this path
}
else if (bToDestination)
{
// destination is always worth trying
if (uEndCurrency == mDstAmount.getCurrency())
{
// this is a complete path
if (!currentPath.empty())
{
JLOG(j_.trace())
<< "complete path found ae: "
<< currentPath.getJson(
JsonOptions::none);
addUniquePath(mCompletePaths, currentPath);
}
}
else if (!bDestOnly)
{
// this is a high-priority candidate
candidates.push_back(
{AccountCandidate::highPriority, acct});
}
}
else if (acct == mSrcAccount)
{
// going back to the source is bad
}
else
{
// save this candidate
int out = getPathsOut(
uEndCurrency,
acct,
outgoing,
bIsEndCurrency,
mEffectiveDst,
continueCallback);
if (out)
candidates.push_back({out, acct});
}
}
}
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)
for (auto const& rs : rippleLines)
{
if (continueCallback && !continueCallback())
return;
// Add accounts to incompletePaths
STPathElement pathElement(
STPathElement::typeAccount,
it->account,
uEndCurrency,
it->account);
incompletePaths.assembleAdd(currentPath, pathElement);
++it;
auto const& acct = rs.getAccountIDPeer();
LineDirection const direction = rs.getDirectionPeer();
if (hasEffectiveDestination && (acct == mDstAccount))
{
// We skipped the gateway
continue;
}
bool bToDestination = acct == mEffectiveDst;
if (bDestOnly && !bToDestination)
{
continue;
}
if ((uEndCurrency == rs.getLimit().getCurrency()) &&
!currentPath.hasSeen(acct, uEndCurrency, 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())
{
// Can't leave on this path
}
else if (bToDestination)
{
// destination is always worth trying
if (uEndCurrency == mDstAmount.getCurrency())
{
// this is a complete path
if (!currentPath.empty())
{
JLOG(j_.trace())
<< "complete path found ae: "
<< currentPath.getJson(
JsonOptions::none);
addUniquePath(
mCompletePaths, currentPath);
}
}
else if (!bDestOnly)
{
// this is a high-priority candidate
candidates.push_back(
{AccountCandidate::highPriority, acct});
}
}
else if (acct == mSrcAccount)
{
// going back to the source is bad
}
else
{
// save this candidate
int out = getPathsOut(
uEndCurrency,
acct,
direction,
bIsEndCurrency,
mEffectiveDst,
continueCallback);
if (out)
candidates.push_back({out, acct});
}
}
}
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,
uEndCurrency,
it->account);
incompletePaths.assembleAdd(
currentPath, pathElement);
++it;
}
}
}
}

View File

@@ -144,7 +144,7 @@ private:
getPathsOut(
Currency const& currency,
AccountID const& account,
bool outgoing,
LineDirection direction,
bool isDestCurrency,
AccountID const& dest,
std::function<bool(void)> const& continueCallback);

View File

@@ -26,40 +26,101 @@ namespace ripple {
RippleLineCache::RippleLineCache(
std::shared_ptr<ReadView const> const& ledger,
beast::Journal j)
: journal_(j)
: ledger_(ledger), journal_(j)
{
mLedger = ledger;
JLOG(journal_.debug()) << "RippleLineCache created for ledger "
<< mLedger->info().seq;
JLOG(journal_.debug()) << "created for ledger " << ledger_->info().seq;
}
RippleLineCache::~RippleLineCache()
{
JLOG(journal_.debug()) << "~RippleLineCache destroyed for ledger "
<< mLedger->info().seq << " with " << lines_.size()
<< " accounts";
JLOG(journal_.debug()) << "destroyed for ledger " << ledger_->info().seq
<< " with " << lines_.size() << " accounts and "
<< totalLineCount_ << " distinct trust lines.";
}
std::vector<PathFindTrustLine> const&
RippleLineCache::getRippleLines(AccountID const& accountID, bool outgoing)
std::shared_ptr<std::vector<PathFindTrustLine>>
RippleLineCache::getRippleLines(
AccountID const& accountID,
LineDirection direction)
{
AccountKey key(
accountID, outgoing, hasher_(std::pair{accountID, outgoing}));
auto const hash = hasher_(accountID);
AccountKey key(accountID, direction, hash);
AccountKey otherkey(
accountID,
direction == LineDirection::outgoing ? LineDirection::incoming
: LineDirection::outgoing,
hash);
std::lock_guard sl(mLock);
auto [it, inserted] = lines_.emplace(key, std::vector<PathFindTrustLine>());
auto [it, inserted] = [&]() {
if (auto otheriter = lines_.find(otherkey); otheriter != lines_.end())
{
// The whole point of using the direction flag is to reduce the
// number of trust line objects held in memory. Ensure that there is
// only a single set of trustlines in the cache per account.
auto const size = otheriter->second ? otheriter->second->size() : 0;
JLOG(journal_.info())
<< "Request for "
<< (direction == LineDirection::outgoing ? "outgoing"
: "incoming")
<< " trust lines for account " << accountID << " found " << size
<< (direction == LineDirection::outgoing ? " incoming"
: " outgoing")
<< " trust lines. "
<< (direction == LineDirection::outgoing
? "Deleting the subset of incoming"
: "Returning the superset of outgoing")
<< " trust lines. ";
if (direction == LineDirection::outgoing)
{
// This request is for the outgoing set, but there is already a
// subset of incoming lines in the cache. Erase that subset
// to be replaced by the full set. The full set will be built
// below, and will be returned, if needed, on subsequent calls
// for either value of outgoing.
assert(size <= totalLineCount_);
totalLineCount_ -= size;
lines_.erase(otheriter);
}
else
{
// This request is for the incoming set, but there is
// already a superset of the outgoing trust lines in the cache.
// The path finding engine will disregard the non-rippling trust
// lines, so to prevent them from being stored twice, return the
// outgoing set.
key = otherkey;
return std::pair{otheriter, false};
}
}
return lines_.emplace(key, nullptr);
}();
if (inserted)
it->second = PathFindTrustLine::getItems(accountID, *mLedger, outgoing);
{
assert(it->second == nullptr);
auto lines =
PathFindTrustLine::getItems(accountID, *ledger_, direction);
if (lines.size())
{
it->second = std::make_shared<std::vector<PathFindTrustLine>>(
std::move(lines));
totalLineCount_ += it->second->size();
}
}
JLOG(journal_.debug()) << "RippleLineCache getRippleLines for ledger "
<< mLedger->info().seq << " found "
<< it->second.size() << " lines for "
<< (inserted ? "new " : "existing ") << accountID
<< " out of a total of " << lines_.size()
<< " accounts";
assert(!it->second || (it->second->size() > 0));
auto const size = it->second ? it->second->size() : 0;
JLOG(journal_.trace()) << "getRippleLines for ledger "
<< ledger_->info().seq << " found " << size
<< (key.direction_ == LineDirection::outgoing
? " outgoing"
: " incoming")
<< " lines for " << (inserted ? "new " : "existing ")
<< accountID << " out of a total of "
<< lines_.size() << " accounts and "
<< totalLineCount_ << " trust lines";
return it->second;
}

View File

@@ -44,13 +44,13 @@ public:
std::shared_ptr<ReadView const> const&
getLedger() const
{
return mLedger;
return ledger_;
}
/** Find the trust lines associated with an account.
@param accountID The account
@param outgoing Whether the account is an "outgoing" link on the path.
@param direction Whether the account is an "outgoing" link on the path.
"Outgoing" is defined as the source account, or an account found via a
trustline that has rippling enabled on the @accountID's side. If an
account is "outgoing", all trust lines will be returned. If an account is
@@ -59,25 +59,28 @@ public:
@accountID's side.
@return Returns a vector of the usable trust lines.
*/
std::vector<PathFindTrustLine> const&
getRippleLines(AccountID const& accountID, bool outgoing);
std::shared_ptr<std::vector<PathFindTrustLine>>
getRippleLines(AccountID const& accountID, LineDirection direction);
private:
std::mutex mLock;
ripple::hardened_hash<> hasher_;
std::shared_ptr<ReadView const> mLedger;
std::shared_ptr<ReadView const> ledger_;
beast::Journal journal_;
struct AccountKey final : public CountedObject<AccountKey>
{
AccountID account_;
bool outgoing_;
LineDirection direction_;
std::size_t hash_value_;
AccountKey(AccountID const& account, bool outgoing, std::size_t hash)
: account_(account), outgoing_(outgoing), hash_value_(hash)
AccountKey(
AccountID const& account,
LineDirection direction,
std::size_t hash)
: account_(account), direction_(direction), hash_value_(hash)
{
}
@@ -90,7 +93,7 @@ private:
operator==(AccountKey const& lhs) const
{
return hash_value_ == lhs.hash_value_ && account_ == lhs.account_ &&
outgoing_ == lhs.outgoing_;
direction_ == lhs.direction_;
}
std::size_t
@@ -111,8 +114,17 @@ private:
};
};
hash_map<AccountKey, std::vector<PathFindTrustLine>, AccountKey::Hash>
// Use a shared_ptr so entries can be removed from the map safely.
// Even though a shared_ptr to a vector will take more memory just a vector,
// most accounts are not going to have any entries (estimated over 90%), so
// vectors will not need to be created for them. This should lead to far
// less memory usage overall.
hash_map<
AccountKey,
std::shared_ptr<std::vector<PathFindTrustLine>>,
AccountKey::Hash>
lines_;
std::size_t totalLineCount_ = 0;
};
} // namespace ripple

View File

@@ -64,18 +64,22 @@ std::vector<T>
getTrustLineItems(
AccountID const& accountID,
ReadView const& view,
bool outgoing = true)
LineDirection direction = LineDirection::outgoing)
{
std::vector<T> items;
forEachItem(
view,
accountID,
[&items, &accountID, &outgoing](
[&items, &accountID, &direction](
std::shared_ptr<SLE const> const& sleCur) {
auto ret = T::makeItem(accountID, sleCur);
if (ret && (outgoing || !ret->getNoRipple()))
if (ret &&
(direction == LineDirection::outgoing || !ret->getNoRipple()))
items.push_back(std::move(*ret));
});
// This list may be around for a while, so free up any unneeded
// capacity
items.shrink_to_fit();
return items;
}
@@ -85,10 +89,10 @@ std::vector<PathFindTrustLine>
PathFindTrustLine::getItems(
AccountID const& accountID,
ReadView const& view,
bool outgoing)
LineDirection direction)
{
return detail::getTrustLineItems<PathFindTrustLine>(
accountID, view, outgoing);
accountID, view, direction);
}
RPCTrustLine::RPCTrustLine(

View File

@@ -31,6 +31,15 @@
namespace ripple {
/** Describes how an account was found in a path, and how to find the next set
of paths. "Outgoing" is defined as the source account, or an account found via a
trustline that has rippling enabled on the account's side.
"Incoming" is defined as an account found via a trustline that has rippling
disabled on the account's side. Any trust lines for an incoming account that
have rippling disabled are unusable in paths.
*/
enum class LineDirection : bool { incoming = false, outgoing = true };
/** Wraps a trust line SLE for convenience.
The complication of trust lines is that there is a
"low" account and a "high" account. This wraps the
@@ -109,6 +118,20 @@ public:
return mFlags & (!mViewLowest ? lsfLowNoRipple : lsfHighNoRipple);
}
LineDirection
getDirection() const
{
return getNoRipple() ? LineDirection::incoming
: LineDirection::outgoing;
}
LineDirection
getDirectionPeer() const
{
return getNoRipplePeer() ? LineDirection::incoming
: LineDirection::outgoing;
}
/** Have we set the freeze flag on our peer */
bool
getFreeze() const
@@ -170,7 +193,10 @@ public:
makeItem(AccountID const& accountID, std::shared_ptr<SLE const> const& sle);
static std::vector<PathFindTrustLine>
getItems(AccountID const& accountID, ReadView const& view, bool outgoing);
getItems(
AccountID const& accountID,
ReadView const& view,
LineDirection direction);
};
// This wrapper is used for the `AccountLines` command and includes the quality

View File

@@ -1378,6 +1378,69 @@ public:
}
}
void
noripple_combinations()
{
using namespace jtx;
// This test will create trust lines with various values of the noRipple
// flag. alice <-> george <-> bob george will sort of act like a
// gateway, but use a different name to avoid the usual assumptions
// about gateways.
auto const alice = Account("alice");
auto const bob = Account("bob");
auto const george = Account("george");
auto const USD = george["USD"];
auto test = [&](std::string casename,
bool aliceRipple,
bool bobRipple,
bool expectPath) {
testcase(casename);
Env env = pathTestEnv();
env.fund(XRP(10000), noripple(alice, bob, george));
env.close();
// Set the same flags at both ends of the trustline, even though
// only george's matter.
env(trust(
alice,
USD(100),
aliceRipple ? tfClearNoRipple : tfSetNoRipple));
env(trust(
george,
alice["USD"](100),
aliceRipple ? tfClearNoRipple : tfSetNoRipple));
env(trust(
bob, USD(100), bobRipple ? tfClearNoRipple : tfSetNoRipple));
env(trust(
george,
bob["USD"](100),
bobRipple ? tfClearNoRipple : tfSetNoRipple));
env.close();
env(pay(george, alice, USD(70)));
env.close();
auto [st, sa, da] =
find_paths(env, "alice", "bob", Account("bob")["USD"](5));
BEAST_EXPECT(equal(da, bob["USD"](5)));
if (expectPath)
{
BEAST_EXPECT(st.size() == 1);
BEAST_EXPECT(same(st, stpath("george")));
BEAST_EXPECT(equal(sa, alice["USD"](5)));
}
else
{
BEAST_EXPECT(st.size() == 0);
BEAST_EXPECT(equal(sa, XRP(0)));
}
};
test("ripple -> ripple", true, true, true);
test("ripple -> no ripple", true, false, true);
test("no ripple -> ripple", false, true, true);
test("no ripple -> no ripple", false, false, false);
}
void
run() override
{
@@ -1401,6 +1464,7 @@ public:
trust_auto_clear_trust_auto_clear();
xrp_to_xrp();
receive_max();
noripple_combinations();
// The following path_find_NN tests are data driven tests
// that were originally implemented in js/coffee and migrated