Skip to content

Commit

Permalink
[FOLD] Convert the "outgoing" bool to an enum
Browse files Browse the repository at this point in the history
* (Naming things is Hard)
  • Loading branch information
ximinez committed Mar 24, 2022
1 parent 48b184b commit e9c9bf3
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 32 deletions.
6 changes: 4 additions & 2 deletions src/ripple/app/paths/AccountCurrencies.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ accountSourceCurrencies(
if (includeXRP)
currencies.insert(xrpCurrency());

if (auto const lines = lrCache->getRippleLines(account, true))
if (auto const lines =
lrCache->getRippleLines(account, LineDirection::outgoing))
{
for (auto const& rspEntry : *lines)
{
Expand Down Expand Up @@ -68,7 +69,8 @@ accountDestCurrencies(
currencies.insert(xrpCurrency());
// Even if account doesn't exist

if (auto const lines = lrCache->getRippleLines(account, true))
if (auto const lines =
lrCache->getRippleLines(account, LineDirection::outgoing))
{
for (auto const& rspEntry : *lines)
{
Expand Down
14 changes: 8 additions & 6 deletions src/ripple/app/paths/Pathfinder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -736,7 +736,7 @@ Pathfinder::getPathsOut(
{
count = app_.getOrderBookDB().getBookSize(issue);

if (auto const lines = mRLCache->getRippleLines(account, outgoing))
if (auto const lines = mRLCache->getRippleLines(account, direction))
{
for (auto const& rspEntry : *lines)
{
Expand Down Expand Up @@ -981,8 +981,10 @@ Pathfinder::addLink(
bool const bIsNoRippleOut(isNoRippleOut(currentPath));
bool const bDestOnly(addFlags & afAC_LAST);

if (auto const lines =
mRLCache->getRippleLines(uEndAccount, !bIsNoRippleOut))
if (auto const lines = mRLCache->getRippleLines(
uEndAccount,
bIsNoRippleOut ? LineDirection::incoming
: LineDirection::outgoing))
{
auto& rippleLines = *lines;

Expand All @@ -994,7 +996,7 @@ Pathfinder::addLink(
if (continueCallback && !continueCallback())
return;
auto const& acct = rs.getAccountIDPeer();
bool const outgoing = !rs.getNoRipplePeer();
LineDirection const direction = rs.getDirectionPeer();

if (hasEffectiveDestination && (acct == mDstAccount))
{
Expand Down Expand Up @@ -1058,7 +1060,7 @@ Pathfinder::addLink(
int out = getPathsOut(
uEndCurrency,
acct,
outgoing,
direction,
bIsEndCurrency,
mEffectiveDst,
continueCallback);
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/paths/Pathfinder.h
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ class Pathfinder : public CountedObject<Pathfinder>
getPathsOut(
Currency const& currency,
AccountID const& account,
bool outgoing,
LineDirection direction,
bool isDestCurrency,
AccountID const& dest,
std::function<bool(void)> const& continueCallback);
Expand Down
36 changes: 25 additions & 11 deletions src/ripple/app/paths/RippleLineCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,29 +40,40 @@ RippleLineCache::~RippleLineCache()
}

std::shared_ptr<std::vector<PathFindTrustLine>>
RippleLineCache::getRippleLines(AccountID const& accountID, bool outgoing)
RippleLineCache::getRippleLines(
AccountID const& accountID,
LineDirection direction)
{
auto const hash = hasher_(accountID);
AccountKey key(accountID, outgoing, hash);
AccountKey otherkey(accountID, !outgoing, hash);
AccountKey key(accountID, direction, hash);
AccountKey otherkey(
accountID,
direction == LineDirection::outgoing ? LineDirection::incoming
: LineDirection::outgoing,
hash);

std::lock_guard sl(mLock);

auto [it, inserted] = [&]() {
if (auto otheriter = lines_.find(otherkey); otheriter != lines_.end())
{
// The whole point of using the outgoing flag is to reduce the
// 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 " << (outgoing ? "outgoing" : "incoming")
<< "Request for "
<< (direction == LineDirection::outgoing ? "outgoing"
: "incoming")
<< " trust lines for account " << accountID << " found " << size
<< (outgoing ? " incoming" : " outgoing") << " trust lines. "
<< (outgoing ? "Deleting the subset of incoming"
: "Returning the superset of outgoing")
<< (direction == LineDirection::outgoing ? " incoming"
: " outgoing")
<< " trust lines. "
<< (direction == LineDirection::outgoing
? "Deleting the subset of incoming"
: "Returning the superset of outgoing")
<< " trust lines. ";
if (outgoing)
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
Expand Down Expand Up @@ -90,7 +101,8 @@ RippleLineCache::getRippleLines(AccountID const& accountID, bool outgoing)
if (inserted)
{
assert(it->second == nullptr);
auto lines = PathFindTrustLine::getItems(accountID, *ledger_, outgoing);
auto lines =
PathFindTrustLine::getItems(accountID, *ledger_, direction);
if (lines.size())
{
it->second = std::make_shared<std::vector<PathFindTrustLine>>(
Expand All @@ -103,7 +115,9 @@ RippleLineCache::getRippleLines(AccountID const& accountID, bool outgoing)
auto const size = it->second ? it->second->size() : 0;
JLOG(journal_.trace()) << "getRippleLines for ledger "
<< ledger_->info().seq << " found " << size
<< (key.outgoing_ ? " outgoing" : " incoming")
<< (key.direction_ == LineDirection::outgoing
? " outgoing"
: " incoming")
<< " lines for " << (inserted ? "new " : "existing ")
<< accountID << " out of a total of "
<< lines_.size() << " accounts and "
Expand Down
15 changes: 9 additions & 6 deletions src/ripple/app/paths/RippleLineCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class RippleLineCache final : public CountedObject<RippleLineCache>
/** 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
Expand All @@ -60,7 +60,7 @@ class RippleLineCache final : public CountedObject<RippleLineCache>
@return Returns a vector of the usable trust lines.
*/
std::shared_ptr<std::vector<PathFindTrustLine>>
getRippleLines(AccountID const& accountID, bool outgoing);
getRippleLines(AccountID const& accountID, LineDirection direction);

private:
std::mutex mLock;
Expand All @@ -73,11 +73,14 @@ class RippleLineCache final : public CountedObject<RippleLineCache>
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)
{
}

Expand All @@ -90,7 +93,7 @@ class RippleLineCache final : public CountedObject<RippleLineCache>
operator==(AccountKey const& lhs) const
{
return hash_value_ == lhs.hash_value_ && account_ == lhs.account_ &&
outgoing_ == lhs.outgoing_;
direction_ == lhs.direction_;
}

std::size_t
Expand Down
11 changes: 6 additions & 5 deletions src/ripple/app/paths/TrustLine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,17 @@ 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
Expand All @@ -88,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(
Expand Down
28 changes: 27 additions & 1 deletion src/ripple/app/paths/TrustLine.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -109,6 +118,20 @@ class TrustLineBase
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
Expand Down Expand Up @@ -170,7 +193,10 @@ class PathFindTrustLine final : public TrustLineBase,
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
Expand Down

0 comments on commit e9c9bf3

Please sign in to comment.