diff --git a/src/ripple/app/paths/AccountCurrencies.cpp b/src/ripple/app/paths/AccountCurrencies.cpp index 2892ff869c..1323917a5a 100644 --- a/src/ripple/app/paths/AccountCurrencies.cpp +++ b/src/ripple/app/paths/AccountCurrencies.cpp @@ -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(); diff --git a/src/ripple/app/paths/PathRequest.cpp b/src/ripple/app/paths/PathRequest.cpp index e5b15fd9d0..d1acb3ac1f 100644 --- a/src/ripple/app/paths/PathRequest.cpp +++ b/src/ripple/app/paths/PathRequest.cpp @@ -748,6 +748,8 @@ PathRequest::doUpdate( jvStatus = newStatus; } + JLOG(m_journal.debug()) + << iIdentifier << " update finished " << (fast ? "fast" : "normal"); return newStatus; } diff --git a/src/ripple/app/paths/Pathfinder.cpp b/src/ripple/app/paths/Pathfinder.cpp index 4e81bebd3c..91f4290c43 100644 --- a/src/ripple/app/paths/Pathfinder.cpp +++ b/src/ripple/app/paths/Pathfinder.cpp @@ -708,6 +708,7 @@ int Pathfinder::getPathsOut( Currency const& currency, AccountID const& account, + bool outgoing, bool isDstCurrency, AccountID const& dstAccount, std::function 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); diff --git a/src/ripple/app/paths/Pathfinder.h b/src/ripple/app/paths/Pathfinder.h index 45da9ec112..b176388a6f 100644 --- a/src/ripple/app/paths/Pathfinder.h +++ b/src/ripple/app/paths/Pathfinder.h @@ -144,6 +144,7 @@ private: getPathsOut( Currency const& currency, AccountID const& account, + bool outgoing, bool isDestCurrency, AccountID const& dest, std::function const& continueCallback); diff --git a/src/ripple/app/paths/RippleLineCache.cpp b/src/ripple/app/paths/RippleLineCache.cpp index a0b26ba284..c504a474ac 100644 --- a/src/ripple/app/paths/RippleLineCache.cpp +++ b/src/ripple/app/paths/RippleLineCache.cpp @@ -42,16 +42,17 @@ RippleLineCache::~RippleLineCache() } std::vector 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()); 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 " diff --git a/src/ripple/app/paths/RippleLineCache.h b/src/ripple/app/paths/RippleLineCache.h index e7a7e0f74a..d4b777aeae 100644 --- a/src/ripple/app/paths/RippleLineCache.h +++ b/src/ripple/app/paths/RippleLineCache.h @@ -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 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 { 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 diff --git a/src/ripple/app/paths/TrustLine.cpp b/src/ripple/app/paths/TrustLine.cpp index 12020acf71..dbd9c57c1e 100644 --- a/src/ripple/app/paths/TrustLine.cpp +++ b/src/ripple/app/paths/TrustLine.cpp @@ -61,15 +61,19 @@ PathFindTrustLine::makeItem( namespace detail { template std::vector -getTrustLineItems(AccountID const& accountID, ReadView const& view) +getTrustLineItems( + AccountID const& accountID, + ReadView const& view, + bool outgoing = true) { std::vector items; forEachItem( view, accountID, - [&items, &accountID](std::shared_ptr const& sleCur) { + [&items, &accountID, &outgoing]( + std::shared_ptr 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::getItems(AccountID const& accountID, ReadView const& view) +PathFindTrustLine::getItems( + AccountID const& accountID, + ReadView const& view, + bool outgoing) { - return detail::getTrustLineItems(accountID, view); + return detail::getTrustLineItems( + accountID, view, outgoing); } RPCTrustLine::RPCTrustLine( diff --git a/src/ripple/app/paths/TrustLine.h b/src/ripple/app/paths/TrustLine.h index 0217f0e750..29948feea2 100644 --- a/src/ripple/app/paths/TrustLine.h +++ b/src/ripple/app/paths/TrustLine.h @@ -170,7 +170,7 @@ public: makeItem(AccountID const& accountID, std::shared_ptr const& sle); static std::vector - 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 diff --git a/src/test/rpc/AmendmentBlocked_test.cpp b/src/test/rpc/AmendmentBlocked_test.cpp index 54c8729222..a90bcdcd0c 100644 --- a/src/test/rpc/AmendmentBlocked_test.cpp +++ b/src/test/rpc/AmendmentBlocked_test.cpp @@ -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)));