From 63d2cfd6ba3bfbe57e8fda849f974dd63c20f9ae Mon Sep 17 00:00:00 2001 From: Miguel Portilla Date: Thu, 6 Nov 2014 16:28:14 -0500 Subject: [PATCH] Fix account_lines, account_offers and book_offers result (RIPD-682): The RPC account_lines and account_offers commands respond with the correct ledger info. account_offers, account_lines and book_offers allow admins unlimited size on the limit param. Specifying a negative value on limit clamps to the minimum value allowed. Incorrect types for limit are correctly reported in the result. --- src/ripple/app/misc/NetworkOPs.cpp | 33 ++++++++----------- src/ripple/app/misc/NetworkOPs.h | 1 + src/ripple/rpc/handlers/AccountLines.cpp | 39 +++++++++++++++-------- src/ripple/rpc/handlers/AccountOffers.cpp | 34 +++++++++++++------- src/ripple/rpc/handlers/BookOffers.cpp | 21 ++++++++---- src/ripple/rpc/handlers/Subscribe.cpp | 3 ++ 6 files changed, 79 insertions(+), 52 deletions(-) diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 66a4a37c8..fde92f5e9 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -253,13 +253,9 @@ public: // Book functions // - void getBookPage (Ledger::pointer lpLedger, - Book const&, - Account const& uTakerID, - const bool bProof, - const unsigned int iLimit, - Json::Value const& jvMarker, - Json::Value& jvResult); + void getBookPage (bool bAdmin, Ledger::pointer lpLedger, Book const&, + Account const& uTakerID, const bool bProof, const unsigned int iLimit, + Json::Value const& jvMarker, Json::Value& jvResult); // ledger proposal/close functions void processTrustedProposal ( @@ -3090,6 +3086,7 @@ InfoSub::pointer NetworkOPsImp::addRpcSub ( // // FIXME : support iLimit. void NetworkOPsImp::getBookPage ( + bool bAdmin, Ledger::pointer lpLedger, Book const& book, Account const& uTakerID, @@ -3127,14 +3124,13 @@ void NetworkOPsImp::getBookPage ( unsigned int uBookEntry; STAmount saDirRate; - unsigned int iLeft = iLimit; - - if (iLeft == 0 || iLeft > 300) - iLeft = 300; - auto uTransferRate = rippleTransferRate (lesActive, book.out.account); - while (! bDone && iLeft-- > 0) + unsigned int left (iLimit == 0 ? 300 : iLimit); + if (! bAdmin && left > 300) + left = 300; + + while (!bDone && left-- > 0) { if (bDirectAdvance) { @@ -3308,6 +3304,7 @@ void NetworkOPsImp::getBookPage ( // FIXME : support iLimit. void NetworkOPsImp::getBookPage ( + bool bAdmin, Ledger::pointer lpLedger, Book const& book, Account const& uTakerID, @@ -3323,18 +3320,16 @@ void NetworkOPsImp::getBookPage ( LedgerEntrySet lesActive (lpLedger, tapNONE, true); OrderBookIterator obIterator (lesActive, book); - unsigned int iLeft = iLimit; - - if (iLeft == 0 || iLeft > 300) - iLeft = 300; - auto uTransferRate = rippleTransferRate (lesActive, book.out.account); const bool bGlobalFreeze = lesActive.isGlobalFrozen (book.out.account) || lesActive.isGlobalFrozen (book.in.account); + unsigned int left (iLimit == 0 ? 300 : iLimit); + if (! bAdmin && left > 300) + left = 300; - while (iLeft-- > 0 && obIterator.nextOffer ()) + while (left-- > 0 && obIterator.nextOffer ()) { SLE::pointer sleOffer = obIterator.getCurrentOffer(); diff --git a/src/ripple/app/misc/NetworkOPs.h b/src/ripple/app/misc/NetworkOPs.h index 4ae697aad..bbaecbf6a 100644 --- a/src/ripple/app/misc/NetworkOPs.h +++ b/src/ripple/app/misc/NetworkOPs.h @@ -188,6 +188,7 @@ public: // virtual void getBookPage ( + bool bAdmin, Ledger::pointer lpLedger, Book const& book, Account const& uTakerID, diff --git a/src/ripple/rpc/handlers/AccountLines.cpp b/src/ripple/rpc/handlers/AccountLines.cpp index 825cfe923..fae3d92d9 100644 --- a/src/ripple/rpc/handlers/AccountLines.cpp +++ b/src/ripple/rpc/handlers/AccountLines.cpp @@ -74,27 +74,29 @@ void addLine (Json::Value& jsonLines, RippleState const& line) // } Json::Value doAccountLines (RPC::Context& context) { - auto& params = context.params; + auto const& params (context.params); + if (! params.isMember (jss::account)) + return RPC::missing_field_error ("account"); Ledger::pointer ledger; Json::Value result (RPC::lookupLedger (params, ledger, context.netOps)); - if (! ledger) return result; - if (! params.isMember (jss::account)) - return RPC::missing_field_error ("account"); - std::string strIdent (params[jss::account].asString ()); bool bIndex (params.isMember (jss::account_index)); int iIndex (bIndex ? params[jss::account_index].asUInt () : 0); RippleAddress rippleAddress; - result = RPC::accountFromString ( - ledger, rippleAddress, bIndex, strIdent, iIndex, false, context.netOps); + Json::Value const jv (RPC::accountFromString (ledger, rippleAddress, bIndex, + strIdent, iIndex, false, context.netOps)); + if (! jv.empty ()) + { + for (Json::Value::const_iterator it (jv.begin ()); it != jv.end (); ++it) + result[it.memberName ()] = it.key (); - if (! result.empty ()) return result; + } if (! ledger->hasAccount (rippleAddress)) return rpcError (rpcACT_NOT_FOUND); @@ -113,8 +115,8 @@ Json::Value doAccountLines (RPC::Context& context) if (bPeerIndex) result[jss::peer_index] = iPeerIndex; - result = RPC::accountFromString (ledger, rippleAddressPeer, bPeerIndex, strPeer, - iPeerIndex, false, context.netOps); + result = RPC::accountFromString (ledger, rippleAddressPeer, bPeerIndex, + strPeer, iPeerIndex, false, context.netOps); if (! result.empty ()) return result; @@ -127,9 +129,18 @@ Json::Value doAccountLines (RPC::Context& context) unsigned int limit; if (params.isMember (jss::limit)) { - limit = std::max (RPC::Tuning::minLinesPerRequest, - std::min (params[jss::limit].asUInt (), - RPC::Tuning::maxLinesPerRequest)); + auto const& jvLimit (params[jss::limit]); + if (! jvLimit.isIntegral ()) + return RPC::expected_field_error ("limit", "unsigned integer"); + + limit = jvLimit.isUInt () ? jvLimit.asUInt () : + std::max (0, jvLimit.asInt ()); + + if (context.role != Role::ADMIN) + { + limit = std::max (RPC::Tuning::minLinesPerRequest, + std::min (limit, RPC::Tuning::maxLinesPerRequest)); + } } else { @@ -150,7 +161,7 @@ Json::Value doAccountLines (RPC::Context& context) Json::Value const& marker (params[jss::marker]); if (! marker.isString ()) - return rpcError (rpcACT_MALFORMED); + return RPC::expected_field_error ("marker", "string"); startAfter.SetHex (marker.asString ()); SLE::pointer sleLine (ledger->getSLEi (startAfter)); diff --git a/src/ripple/rpc/handlers/AccountOffers.cpp b/src/ripple/rpc/handlers/AccountOffers.cpp index a5f66b554..66dcae80a 100644 --- a/src/ripple/rpc/handlers/AccountOffers.cpp +++ b/src/ripple/rpc/handlers/AccountOffers.cpp @@ -32,27 +32,28 @@ namespace ripple { Json::Value doAccountOffers (RPC::Context& context) { auto const& params (context.params); + if (! params.isMember (jss::account)) + return RPC::missing_field_error ("account"); Ledger::pointer ledger; Json::Value result (RPC::lookupLedger (params, ledger, context.netOps)); - if (! ledger) return result; - if (! params.isMember (jss::account)) - return RPC::missing_field_error ("account"); - std::string strIdent (params[jss::account].asString ()); bool bIndex (params.isMember (jss::account_index)); int const iIndex (bIndex ? params[jss::account_index].asUInt () : 0); - RippleAddress rippleAddress; - result = RPC::accountFromString (ledger, rippleAddress, bIndex, strIdent, - iIndex, false, context.netOps); + Json::Value const jv (RPC::accountFromString (ledger, rippleAddress, bIndex, + strIdent, iIndex, false, context.netOps)); + if (! jv.empty ()) + { + for (Json::Value::const_iterator it (jv.begin ()); it != jv.end (); ++it) + result[it.memberName ()] = it.key (); - if (! result.empty ()) return result; + } // Get info on account. result[jss::account] = rippleAddress.humanAccountID (); @@ -66,9 +67,18 @@ Json::Value doAccountOffers (RPC::Context& context) unsigned int limit; if (params.isMember (jss::limit)) { - limit = std::max (RPC::Tuning::minOffersPerRequest, - std::min (params[jss::limit].asUInt (), - RPC::Tuning::maxOffersPerRequest)); + auto const& jvLimit (params[jss::limit]); + if (! jvLimit.isIntegral ()) + return RPC::expected_field_error ("limit", "unsigned integer"); + + limit = jvLimit.isUInt () ? jvLimit.asUInt () : + std::max (0, jvLimit.asInt ()); + + if (context.role != Role::ADMIN) + { + limit = std::max (RPC::Tuning::minOffersPerRequest, + std::min (limit, RPC::Tuning::maxOffersPerRequest)); + } } else { @@ -89,7 +99,7 @@ Json::Value doAccountOffers (RPC::Context& context) Json::Value const& marker (params[jss::marker]); if (! marker.isString ()) - return rpcError (rpcACT_MALFORMED); + return RPC::expected_field_error ("marker", "string"); startAfter.SetHex (marker.asString ()); SLE::pointer sleOffer (ledger->getSLEi (startAfter)); diff --git a/src/ripple/rpc/handlers/BookOffers.cpp b/src/ripple/rpc/handlers/BookOffers.cpp index d29bb96b5..851c6a0ac 100644 --- a/src/ripple/rpc/handlers/BookOffers.cpp +++ b/src/ripple/rpc/handlers/BookOffers.cpp @@ -162,15 +162,21 @@ Json::Value doBookOffers (RPC::Context& context) return RPC::make_error (rpcBAD_MARKET); } - if (context.params.isMember ("limit") && - !context.params ["limit"].isIntegral()) + unsigned int iLimit; + if (context.params.isMember (jss::limit)) { - return RPC::expected_field_error ("limit", "integer"); - } + auto const& jvLimit (context.params[jss::limit]); - unsigned int const iLimit (context.params.isMember ("limit") - ? context.params ["limit"].asUInt () - : 0); + if (! jvLimit.isIntegral ()) + return RPC::expected_field_error ("limit", "unsigned integer"); + + iLimit = jvLimit.isUInt () ? jvLimit.asUInt () : + std::max (0, jvLimit.asInt ()); + } + else + { + iLimit = 0; + } bool const bProof (context.params.isMember ("proof")); @@ -179,6 +185,7 @@ Json::Value doBookOffers (RPC::Context& context) : Json::Value (Json::nullValue)); context.netOps.getBookPage ( + context.role == Role::ADMIN, lpLedger, {{pay_currency, pay_issuer}, {get_currency, get_issuer}}, raTakerID.getAccountID (), bProof, iLimit, jvMarker, jvResult); diff --git a/src/ripple/rpc/handlers/Subscribe.cpp b/src/ripple/rpc/handlers/Subscribe.cpp index 5b26e142f..8abed0224 100644 --- a/src/ripple/rpc/handlers/Subscribe.cpp +++ b/src/ripple/rpc/handlers/Subscribe.cpp @@ -312,6 +312,7 @@ Json::Value doSubscribe (RPC::Context& context) Json::Value jvAsks (Json::objectValue); context.netOps.getBookPage ( + context.role == Role::ADMIN, lpLedger, book, raTakerID.getAccountID (), false, 0, jvMarker, jvBids); @@ -319,6 +320,7 @@ Json::Value doSubscribe (RPC::Context& context) jvResult[jss::bids] = jvBids[jss::offers]; context.netOps.getBookPage ( + context.role == Role::ADMIN, lpLedger, book, raTakerID.getAccountID (), false, 0, jvMarker, jvAsks); @@ -328,6 +330,7 @@ Json::Value doSubscribe (RPC::Context& context) else { context.netOps.getBookPage ( + context.role == Role::ADMIN, lpLedger, book, raTakerID.getAccountID (), false, 0, jvMarker, jvResult); }