From 48d28826d084dae51e646a27a3c4a479eb7cd06f Mon Sep 17 00:00:00 2001 From: Miguel Portilla Date: Thu, 3 Mar 2016 17:24:42 -0500 Subject: [PATCH] Pathfinding cleanup --- src/ripple/app/paths/PathRequest.cpp | 81 ++++++++----- src/ripple/resource/README.md | 2 +- src/ripple/rpc/handlers/RipplePathFind.cpp | 125 ++++++++------------ src/ripple/server/impl/ServerHandlerImp.cpp | 25 ++-- src/ripple/websocket/Connection.h | 1 - 5 files changed, 115 insertions(+), 119 deletions(-) diff --git a/src/ripple/app/paths/PathRequest.cpp b/src/ripple/app/paths/PathRequest.cpp index 1c4b706d9..bf0b2703d 100644 --- a/src/ripple/app/paths/PathRequest.cpp +++ b/src/ripple/app/paths/PathRequest.cpp @@ -343,9 +343,9 @@ int PathRequest::parseJson (Json::Value const& jvParams) if (jvParams.isMember (jss::source_currencies)) { - Json::Value const& jvSrcCur = jvParams[jss::source_currencies]; - - if (! jvSrcCur.isArray() || jvSrcCur.size() > RPC::Tuning::max_src_cur) + Json::Value const& jvSrcCurrencies = jvParams[jss::source_currencies]; + if (! jvSrcCurrencies.isArray() || jvSrcCurrencies.size() == 0 || + jvSrcCurrencies.size() > RPC::Tuning::max_src_cur) { jvStatus = rpcError (rpcSRC_CUR_MALFORMED); return PFR_PJ_INVALID; @@ -353,44 +353,51 @@ int PathRequest::parseJson (Json::Value const& jvParams) sciSourceCurrencies.clear (); - for (unsigned i = 0; i < jvSrcCur.size (); ++i) + for (auto const& c : jvSrcCurrencies) { - Json::Value const& jvCur = jvSrcCur[i]; - Currency uCur; - if (! jvCur.isObject() || - ! jvCur.isMember (jss::currency) || - ! to_currency (uCur, jvCur[jss::currency].asString ())) + // Mandatory currency + Currency srcCurrencyID; + if (! c.isObject() || + ! c.isMember(jss::currency) || + ! to_currency(srcCurrencyID, c[jss::currency].asString())) { jvStatus = rpcError (rpcSRC_CUR_MALFORMED); return PFR_PJ_INVALID; } - AccountID uIss; - if (jvCur.isMember (jss::issuer) && - !to_issuer (uIss, jvCur[jss::issuer].asString ())) + // Optional issuer + AccountID srcIssuerID; + if (c.isMember (jss::issuer) && + (! c[jss::issuer].isString() || + ! to_issuer(srcIssuerID, c[jss::issuer].asString()))) { jvStatus = rpcError (rpcSRC_ISR_MALFORMED); - } - - if (uCur.isZero () && uIss.isNonZero ()) - { - jvStatus = rpcError (rpcSRC_CUR_MALFORMED); return PFR_PJ_INVALID; } - if (uCur.isNonZero() && uIss.isZero()) - uIss = *raSrcAccount; + if (srcCurrencyID.isZero()) + { + if (srcIssuerID.isNonZero()) + { + jvStatus = rpcError(rpcSRC_CUR_MALFORMED); + return PFR_PJ_INVALID; + } + } + else if (srcIssuerID.isZero()) + { + srcIssuerID = *raSrcAccount; + } if (saSendMax) { // If the currencies don't match, ignore the source currency. - if (uCur == saSendMax->getCurrency()) + if (srcCurrencyID == saSendMax->getCurrency()) { // If neither is the source and they are not equal, then the // source issuer is illegal. - if (uIss != *raSrcAccount && + if (srcIssuerID != *raSrcAccount && saSendMax->getIssuer() != *raSrcAccount && - uIss != saSendMax->getIssuer()) + srcIssuerID != saSendMax->getIssuer()) { jvStatus = rpcError (rpcSRC_ISR_MALFORMED); return PFR_PJ_INVALID; @@ -398,17 +405,26 @@ int PathRequest::parseJson (Json::Value const& jvParams) // If both are the source, use the source. // Otherwise, use the one that's not the source. - if (uIss != *raSrcAccount) - sciSourceCurrencies.insert({uCur, uIss}); + if (srcIssuerID != *raSrcAccount) + { + sciSourceCurrencies.insert( + {srcCurrencyID, srcIssuerID}); + } else if (saSendMax->getIssuer() != *raSrcAccount) - sciSourceCurrencies.insert({uCur, saSendMax->getIssuer()}); + { + sciSourceCurrencies.insert( + {srcCurrencyID, saSendMax->getIssuer()}); + } else - sciSourceCurrencies.insert({uCur, *raSrcAccount}); + { + sciSourceCurrencies.insert( + {srcCurrencyID, *raSrcAccount}); + } } } else { - sciSourceCurrencies.insert({uCur, uIss}); + sciSourceCurrencies.insert({srcCurrencyID, srcIssuerID}); } } } @@ -454,15 +470,15 @@ PathRequest::getPathFinder(std::shared_ptr const& cache, } bool -PathRequest::findPaths (std::shared_ptr const& cache, int const level, - Json::Value& jvArray) +PathRequest::findPaths (std::shared_ptr const& cache, + int const level, Json::Value& jvArray) { auto sourceCurrencies = sciSourceCurrencies; if (sourceCurrencies.empty ()) { - auto usCurrencies = accountSourceCurrencies(*raSrcAccount, cache, true); + auto currencies = accountSourceCurrencies(*raSrcAccount, cache, true); bool const sameAccount = *raSrcAccount == *raDstAccount; - for (auto const& c : usCurrencies) + for (auto const& c : currencies) { if (! sameAccount || c != saDstAmount.getCurrency()) { @@ -590,7 +606,8 @@ PathRequest::findPaths (std::shared_ptr const& cache, int const return true; } -Json::Value PathRequest::doUpdate (std::shared_ptr const& cache, bool fast) +Json::Value PathRequest::doUpdate( + std::shared_ptr const& cache, bool fast) { using namespace std::chrono; m_journal.debug << iIdentifier << " update " << (fast ? "fast" : "normal"); diff --git a/src/ripple/resource/README.md b/src/ripple/resource/README.md index fa8370b61..8fed985bf 100644 --- a/src/ripple/resource/README.md +++ b/src/ripple/resource/README.md @@ -17,7 +17,7 @@ performed, or simply disconnecting the endpoint. Currently, consumption endpoints include websocket connections used to service clients, and peer connections used to create the peer to peer -overlay network implementing the Ripple protcool. +overlay network implementing the Ripple protocol. The current "balance" of a Consumer represents resource consumption debt or credit. Debt is accrued when bad loads are imposed. Credit is diff --git a/src/ripple/rpc/handlers/RipplePathFind.cpp b/src/ripple/rpc/handlers/RipplePathFind.cpp index 55a245610..e627108bb 100644 --- a/src/ripple/rpc/handlers/RipplePathFind.cpp +++ b/src/ripple/rpc/handlers/RipplePathFind.cpp @@ -49,24 +49,6 @@ namespace ripple { static unsigned int const max_paths = 4; -static -Json::Value -buildSrcCurrencies(AccountID const& account, - std::shared_ptr const& cache) -{ - auto currencies = accountSourceCurrencies(account, cache, true); - auto jvSrcCurrencies = Json::Value(Json::arrayValue); - - for (auto const& uCurrency : currencies) - { - Json::Value jvCurrency(Json::objectValue); - jvCurrency[jss::currency] = to_string(uCurrency); - jvSrcCurrencies.append(jvCurrency); - } - - return jvSrcCurrencies; -} - // This interface is deprecated. Json::Value doRipplePathFind (RPC::Context& context) { @@ -75,24 +57,13 @@ Json::Value doRipplePathFind (RPC::Context& context) context.loadType = Resource::feeHighBurdenRPC; - AccountID raSrc; - AccountID raDst; - STAmount saDstAmount; std::shared_ptr lpLedger; - Json::Value jvResult; - if (context.app.config().RUN_STANDALONE || - context.params.isMember(jss::ledger) || - context.params.isMember(jss::ledger_index) || - context.params.isMember(jss::ledger_hash)) - { - // The caller specified a ledger - jvResult = RPC::lookupLedger (lpLedger, context); - if (!lpLedger) - return jvResult; - } - else + if (! context.app.config().RUN_STANDALONE && + ! context.params.isMember(jss::ledger) && + ! context.params.isMember(jss::ledger_index) && + ! context.params.isMember(jss::ledger_hash)) { if (context.app.getLedgerMaster().getValidatedLedgerAge() > RPC::Tuning::maxValidatedLedgerAge) @@ -115,10 +86,18 @@ Json::Value doRipplePathFind (RPC::Context& context) return jvResult; } + // The caller specified a ledger + jvResult = RPC::lookupLedger (lpLedger, context); + if (! lpLedger) + return jvResult; + RPC::LegacyPathFind lpf (isUnlimited (context.role), context.app); if (! lpf.isOk ()) return rpcError (rpcTOO_BUSY); + AccountID raSrc; + AccountID raDst; + STAmount saDstAmount; if (! context.params.isMember (jss::source_account)) { jvResult = rpcError (rpcSRC_ACT_MISSING); @@ -152,14 +131,14 @@ Json::Value doRipplePathFind (RPC::Context& context) } else if ( // Checks on source_currencies. - context.params.isMember (jss::source_currencies) - && (!context.params[jss::source_currencies].isArray () - || !context.params[jss::source_currencies].size ()) - // Don't allow empty currencies. - ) + context.params.isMember(jss::source_currencies) && + (! context.params[jss::source_currencies].isArray() || + context.params[jss::source_currencies].size() == 0 || + context.params[jss::source_currencies].size() > + RPC::Tuning::max_src_cur)) { JLOG (context.j.info) << "Bad source_currencies."; - jvResult = rpcError (rpcINVALID_PARAMS); + jvResult = rpcError(rpcINVALID_PARAMS); } else { @@ -182,15 +161,19 @@ Json::Value doRipplePathFind (RPC::Context& context) if (context.params.isMember (jss::source_currencies)) { jvSrcCurrencies = context.params[jss::source_currencies]; - if (! jvSrcCurrencies.isArray() || - jvSrcCurrencies.size() > RPC::Tuning::max_src_cur) - return rpcError(rpcSRC_CUR_MALFORMED); } else { - jvSrcCurrencies = buildSrcCurrencies(raSrc, cache); - if (jvSrcCurrencies.size() > RPC::Tuning::max_auto_src_cur) + auto currencies = accountSourceCurrencies(raSrc, cache, true); + if (currencies.size() > RPC::Tuning::max_auto_src_cur) return rpcError(rpcINTERNAL); + auto jvSrcCurrencies = Json::Value(Json::arrayValue); + for (auto const& c : currencies) + { + Json::Value jvCurrency(Json::objectValue); + jvCurrency[jss::currency] = to_string(c); + jvSrcCurrencies.append(jvCurrency); + } } // Fill in currencies destination will accept @@ -296,57 +279,53 @@ ripplePathFind (std::shared_ptr const& cache, auto j = app.journal ("RPCHandler"); - for (unsigned int i = 0; i != jvSrcCurrencies.size(); ++i) + for (auto const& c : jvSrcCurrencies) { - Json::Value jvSource = jvSrcCurrencies[i]; - - Currency uSrcCurrencyID; - AccountID uSrcIssuerID; - - if (!jvSource.isObject()) + if (! c.isObject()) return std::make_pair(false, rpcError(rpcINVALID_PARAMS)); - // Parse mandatory currency. - if (!jvSource.isMember(jss::currency) - || !to_currency( - uSrcCurrencyID, jvSource[jss::currency].asString())) + // Mandatory currency + Currency srcCurrencyID; + if (! c.isMember(jss::currency) || + ! to_currency(srcCurrencyID, c[jss::currency].asString())) { JLOG (j.info) << "Bad currency."; return std::make_pair(false, rpcError(rpcSRC_CUR_MALFORMED)); } - if (uSrcCurrencyID.isNonZero()) - uSrcIssuerID = raSrc; - - // Parse optional issuer. - if (jvSource.isMember(jss::issuer) && - ((!jvSource[jss::issuer].isString() || - !to_issuer(uSrcIssuerID, jvSource[jss::issuer].asString())) || - (uSrcIssuerID.isZero() != uSrcCurrencyID.isZero()) || - (noAccount() == uSrcIssuerID))) + // Optional issuer + AccountID srcIssuerID; + if (c.isMember (jss::issuer) && + (! c[jss::issuer].isString() || + ! to_issuer(srcIssuerID, c[jss::issuer].asString()) || + srcIssuerID.isZero() != srcCurrencyID.isZero() || + noAccount() == srcIssuerID)) { JLOG (j.info) << "Bad issuer."; return std::make_pair(false, rpcError(rpcSRC_ISR_MALFORMED)); } - auto issue = Issue(uSrcCurrencyID, uSrcIssuerID); + if (srcIssuerID.isZero()) + srcIssuerID = raSrc; + + auto issue = Issue(srcCurrencyID, srcIssuerID); if (saSendMax) { // If the currencies don't match, ignore the source currency. - if (uSrcCurrencyID != saSendMax->getCurrency()) + if (srcCurrencyID != saSendMax->getCurrency()) continue; // If neither is the source and they are not equal, then the // source issuer is illegal. - if (uSrcIssuerID != raSrc && saSendMax->getIssuer() != raSrc && - uSrcIssuerID != saSendMax->getIssuer()) + if (srcIssuerID != raSrc && saSendMax->getIssuer() != raSrc && + srcIssuerID != saSendMax->getIssuer()) { return std::make_pair(false, rpcError(rpcSRC_ISR_MALFORMED)); } // If both are the source, use the source. // Otherwise, use the one that's not the source. - if (uSrcIssuerID == raSrc) + if (srcIssuerID == raSrc) { issue.account = saSendMax->getIssuer() != raSrc ? saSendMax->getIssuer() : raSrc; @@ -379,13 +358,13 @@ ripplePathFind (std::shared_ptr const& cache, auto ps = pathfinder->getBestPaths(max_paths, fullLiquidityPath, spsComputed, issue.account); auto& issuer = - isXRP(uSrcIssuerID) ? - isXRP(uSrcCurrencyID) ? // Default to source account. + isXRP(srcIssuerID) ? + isXRP(srcCurrencyID) ? // Default to source account. xrpAccount() : (raSrc) - : uSrcIssuerID; // Use specifed issuer. + : srcIssuerID; // Use specifed issuer. STAmount saMaxAmount = saSendMax.value_or( - STAmount({uSrcCurrencyID, issuer}, 1u, 0, true)); + STAmount({ srcCurrencyID, issuer}, 1u, 0, true)); boost::optional sandbox; sandbox.emplace(&*cache->getLedger(), tapNONE); diff --git a/src/ripple/server/impl/ServerHandlerImp.cpp b/src/ripple/server/impl/ServerHandlerImp.cpp index df71d1ede..b474e2be7 100644 --- a/src/ripple/server/impl/ServerHandlerImp.cpp +++ b/src/ripple/server/impl/ServerHandlerImp.cpp @@ -296,17 +296,19 @@ ServerHandlerImp::processRequest (Port const& port, } Resource::Consumer usage; - - if (isUnlimited (role)) - usage = m_resourceManager.newUnlimitedEndpoint ( - remoteIPAddress.to_string()); - else - usage = m_resourceManager.newInboundEndpoint(remoteIPAddress); - - if (usage.disconnect ()) + if (isUnlimited(role)) { - HTTPReply (503, "Server is overloaded", output, rpcJ); - return; + usage = m_resourceManager.newUnlimitedEndpoint( + remoteIPAddress.to_string()); + } + else + { + usage = m_resourceManager.newInboundEndpoint(remoteIPAddress); + if (usage.disconnect()) + { + HTTPReply(503, "Server is overloaded", output, rpcJ); + return; + } } std::string strMethod = method.asString (); @@ -353,8 +355,6 @@ ServerHandlerImp::processRequest (Port const& port, return; } - Resource::Charge loadType = Resource::feeReferenceRPC; - JLOG(m_journal.debug) << "Query: " << strMethod << params; // Provide the JSON-RPC method as the field "command" in the request. @@ -362,6 +362,7 @@ ServerHandlerImp::processRequest (Port const& port, JLOG (m_journal.trace) << "doRpcCommand:" << strMethod << ":" << params; + Resource::Charge loadType = Resource::feeReferenceRPC; auto const start (std::chrono::high_resolution_clock::now ()); RPC::Context context {m_journal, params, app_, loadType, m_networkOPs, diff --git a/src/ripple/websocket/Connection.h b/src/ripple/websocket/Connection.h index 9147a447b..47d42b3b0 100644 --- a/src/ripple/websocket/Connection.h +++ b/src/ripple/websocket/Connection.h @@ -106,7 +106,6 @@ private: Application& app_; Port const& m_port; Resource::Manager& m_resourceManager; - Resource::Consumer m_usage; beast::IP::Endpoint const m_remoteAddress; std::string const m_forwardedFor; std::string const m_user;