mirror of
https://github.com/XRPLF/rippled.git
synced 2025-12-06 17:27:55 +00:00
Don't load trust lines that can't participate in path finding
* "A path is considered invalid if and only if it enters and exits an address node through trust lines where No Ripple has been enabled for that address." (https://xrpl.org/rippling.html#specifics) * When loading trust lines for an account "Alice" which was reached via a trust line that has the No Ripple flag set on Alice's side, do not use or cache any of Alice's trust lines which have the No Ripple flag set on Alice's side. For typical "end-user" accounts, this will return no trust lines.
This commit is contained in:
@@ -33,7 +33,7 @@ accountSourceCurrencies(
|
||||
if (includeXRP)
|
||||
currencies.insert(xrpCurrency());
|
||||
|
||||
for (auto const& rspEntry : lrCache->getRippleLines(account))
|
||||
for (auto const& rspEntry : lrCache->getRippleLines(account, true))
|
||||
{
|
||||
auto& saBalance = rspEntry.getBalance();
|
||||
|
||||
@@ -64,7 +64,7 @@ accountDestCurrencies(
|
||||
currencies.insert(xrpCurrency());
|
||||
// Even if account doesn't exist
|
||||
|
||||
for (auto const& rspEntry : lrCache->getRippleLines(account))
|
||||
for (auto const& rspEntry : lrCache->getRippleLines(account, true))
|
||||
{
|
||||
auto& saBalance = rspEntry.getBalance();
|
||||
|
||||
|
||||
@@ -748,6 +748,8 @@ PathRequest::doUpdate(
|
||||
jvStatus = newStatus;
|
||||
}
|
||||
|
||||
JLOG(m_journal.debug())
|
||||
<< iIdentifier << " update finished " << (fast ? "fast" : "normal");
|
||||
return newStatus;
|
||||
}
|
||||
|
||||
|
||||
@@ -708,6 +708,7 @@ int
|
||||
Pathfinder::getPathsOut(
|
||||
Currency const& currency,
|
||||
AccountID const& account,
|
||||
bool outgoing,
|
||||
bool isDstCurrency,
|
||||
AccountID const& dstAccount,
|
||||
std::function<bool(void)> const& continueCallback)
|
||||
@@ -735,7 +736,7 @@ Pathfinder::getPathsOut(
|
||||
{
|
||||
count = app_.getOrderBookDB().getBookSize(issue);
|
||||
|
||||
for (auto const& rspEntry : mRLCache->getRippleLines(account))
|
||||
for (auto const& rspEntry : mRLCache->getRippleLines(account, outgoing))
|
||||
{
|
||||
if (currency != rspEntry.getLimit().getCurrency())
|
||||
{
|
||||
@@ -976,7 +977,8 @@ Pathfinder::addLink(
|
||||
bool const bIsNoRippleOut(isNoRippleOut(currentPath));
|
||||
bool const bDestOnly(addFlags & afAC_LAST);
|
||||
|
||||
auto& rippleLines(mRLCache->getRippleLines(uEndAccount));
|
||||
auto& rippleLines(
|
||||
mRLCache->getRippleLines(uEndAccount, !bIsNoRippleOut));
|
||||
|
||||
AccountCandidates candidates;
|
||||
candidates.reserve(rippleLines.size());
|
||||
@@ -986,6 +988,7 @@ Pathfinder::addLink(
|
||||
if (continueCallback && !continueCallback())
|
||||
return;
|
||||
auto const& acct = rs.getAccountIDPeer();
|
||||
bool const outgoing = !rs.getNoRipplePeer();
|
||||
|
||||
if (hasEffectiveDestination && (acct == mDstAccount))
|
||||
{
|
||||
@@ -1047,6 +1050,7 @@ Pathfinder::addLink(
|
||||
int out = getPathsOut(
|
||||
uEndCurrency,
|
||||
acct,
|
||||
outgoing,
|
||||
bIsEndCurrency,
|
||||
mEffectiveDst,
|
||||
continueCallback);
|
||||
|
||||
@@ -144,6 +144,7 @@ private:
|
||||
getPathsOut(
|
||||
Currency const& currency,
|
||||
AccountID const& account,
|
||||
bool outgoing,
|
||||
bool isDestCurrency,
|
||||
AccountID const& dest,
|
||||
std::function<bool(void)> const& continueCallback);
|
||||
|
||||
@@ -42,16 +42,17 @@ RippleLineCache::~RippleLineCache()
|
||||
}
|
||||
|
||||
std::vector<PathFindTrustLine> const&
|
||||
RippleLineCache::getRippleLines(AccountID const& accountID)
|
||||
RippleLineCache::getRippleLines(AccountID const& accountID, bool outgoing)
|
||||
{
|
||||
AccountKey key(accountID, hasher_(accountID));
|
||||
AccountKey key(
|
||||
accountID, outgoing, hasher_(std::pair{accountID, outgoing}));
|
||||
|
||||
std::lock_guard sl(mLock);
|
||||
|
||||
auto [it, inserted] = lines_.emplace(key, std::vector<PathFindTrustLine>());
|
||||
|
||||
if (inserted)
|
||||
it->second = PathFindTrustLine::getItems(accountID, *mLedger);
|
||||
it->second = PathFindTrustLine::getItems(accountID, *mLedger, outgoing);
|
||||
|
||||
JLOG(journal_.debug()) << "RippleLineCache getRippleLines for ledger "
|
||||
<< mLedger->info().seq << " found "
|
||||
|
||||
@@ -47,8 +47,20 @@ public:
|
||||
return mLedger;
|
||||
}
|
||||
|
||||
/** Find the trust lines associated with an account.
|
||||
|
||||
@param accountID The account
|
||||
@param outgoing 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
|
||||
not "outgoing", then any trust lines that don't have rippling enabled are
|
||||
not usable, so only return trust lines that have rippling enabled on
|
||||
@accountID's side.
|
||||
@return Returns a vector of the usable trust lines.
|
||||
*/
|
||||
std::vector<PathFindTrustLine> const&
|
||||
getRippleLines(AccountID const& accountID);
|
||||
getRippleLines(AccountID const& accountID, bool outgoing);
|
||||
|
||||
private:
|
||||
std::mutex mLock;
|
||||
@@ -61,10 +73,11 @@ private:
|
||||
struct AccountKey final : public CountedObject<AccountKey>
|
||||
{
|
||||
AccountID account_;
|
||||
bool outgoing_;
|
||||
std::size_t hash_value_;
|
||||
|
||||
AccountKey(AccountID const& account, std::size_t hash)
|
||||
: account_(account), hash_value_(hash)
|
||||
AccountKey(AccountID const& account, bool outgoing, std::size_t hash)
|
||||
: account_(account), outgoing_(outgoing), hash_value_(hash)
|
||||
{
|
||||
}
|
||||
|
||||
@@ -76,7 +89,8 @@ private:
|
||||
bool
|
||||
operator==(AccountKey const& lhs) const
|
||||
{
|
||||
return hash_value_ == lhs.hash_value_ && account_ == lhs.account_;
|
||||
return hash_value_ == lhs.hash_value_ && account_ == lhs.account_ &&
|
||||
outgoing_ == lhs.outgoing_;
|
||||
}
|
||||
|
||||
std::size_t
|
||||
|
||||
@@ -61,15 +61,19 @@ PathFindTrustLine::makeItem(
|
||||
namespace detail {
|
||||
template <class T>
|
||||
std::vector<T>
|
||||
getTrustLineItems(AccountID const& accountID, ReadView const& view)
|
||||
getTrustLineItems(
|
||||
AccountID const& accountID,
|
||||
ReadView const& view,
|
||||
bool outgoing = true)
|
||||
{
|
||||
std::vector<T> items;
|
||||
forEachItem(
|
||||
view,
|
||||
accountID,
|
||||
[&items, &accountID](std::shared_ptr<SLE const> const& sleCur) {
|
||||
[&items, &accountID, &outgoing](
|
||||
std::shared_ptr<SLE const> const& sleCur) {
|
||||
auto ret = T::makeItem(accountID, sleCur);
|
||||
if (ret)
|
||||
if (ret && (outgoing || !ret->getNoRipple()))
|
||||
items.push_back(std::move(*ret));
|
||||
});
|
||||
|
||||
@@ -78,9 +82,13 @@ getTrustLineItems(AccountID const& accountID, ReadView const& view)
|
||||
} // namespace detail
|
||||
|
||||
std::vector<PathFindTrustLine>
|
||||
PathFindTrustLine::getItems(AccountID const& accountID, ReadView const& view)
|
||||
PathFindTrustLine::getItems(
|
||||
AccountID const& accountID,
|
||||
ReadView const& view,
|
||||
bool outgoing)
|
||||
{
|
||||
return detail::getTrustLineItems<PathFindTrustLine>(accountID, view);
|
||||
return detail::getTrustLineItems<PathFindTrustLine>(
|
||||
accountID, view, outgoing);
|
||||
}
|
||||
|
||||
RPCTrustLine::RPCTrustLine(
|
||||
|
||||
@@ -170,7 +170,7 @@ public:
|
||||
makeItem(AccountID const& accountID, std::shared_ptr<SLE const> const& sle);
|
||||
|
||||
static std::vector<PathFindTrustLine>
|
||||
getItems(AccountID const& accountID, ReadView const& view);
|
||||
getItems(AccountID const& accountID, ReadView const& view, bool outgoing);
|
||||
};
|
||||
|
||||
// This wrapper is used for the `AccountLines` command and includes the quality
|
||||
|
||||
@@ -43,6 +43,12 @@ class AmendmentBlocked_test : public beast::unit_test::suite
|
||||
Account const ali{"ali", KeyType::secp256k1};
|
||||
env.fund(XRP(10000), alice, bob, gw);
|
||||
env.memoize(ali);
|
||||
// This close() ensures that all the accounts get created and their
|
||||
// default ripple flag gets set before the trust lines are created.
|
||||
// Without it, the ordering manages to create alice's trust line with
|
||||
// noRipple set on gw's end. The existing tests pass either way, but
|
||||
// better to do it right.
|
||||
env.close();
|
||||
env.trust(USD(600), alice);
|
||||
env.trust(USD(700), bob);
|
||||
env(pay(gw, alice, USD(70)));
|
||||
|
||||
Reference in New Issue
Block a user