From bf540e917b1725e5665aea91c95cfbbb97a2151c Mon Sep 17 00:00:00 2001 From: natenichols Date: Wed, 15 Dec 2021 18:56:49 -0600 Subject: [PATCH] Enforce account RPC limits by account objects traversed --- src/ripple/rpc/handlers/AccountChannels.cpp | 83 ++++++++----- src/ripple/rpc/handlers/AccountLines.cpp | 127 +++++++++----------- src/ripple/rpc/handlers/AccountOffers.cpp | 88 +++++++++----- src/ripple/rpc/impl/RPCHelpers.cpp | 62 ++++++++-- src/ripple/rpc/impl/RPCHelpers.h | 19 +++ src/ripple/rpc/impl/Tuning.h | 2 +- src/test/app/PayChan_test.cpp | 3 +- src/test/rpc/AccountLinesRPC_test.cpp | 98 ++++++++++++++- src/test/rpc/AccountOffers_test.cpp | 18 ++- 9 files changed, 352 insertions(+), 148 deletions(-) diff --git a/src/ripple/rpc/handlers/AccountChannels.cpp b/src/ripple/rpc/handlers/AccountChannels.cpp index d222e4c72cd..cc79173e55f 100644 --- a/src/ripple/rpc/handlers/AccountChannels.cpp +++ b/src/ripple/rpc/handlers/AccountChannels.cpp @@ -101,6 +101,9 @@ doAccountChannels(RPC::JsonContext& context) if (auto err = readLimitField(limit, RPC::Tuning::accountChannels, context)) return *err; + if (limit == 0u) + return rpcError(rpcINVALID_PARAMS); + Json::Value jsonChannels{Json::arrayValue}; struct VisitData { @@ -110,71 +113,93 @@ doAccountChannels(RPC::JsonContext& context) AccountID const& raDstAccount; }; VisitData visitData = {{}, accountID, hasDst, raDstAccount}; - visitData.items.reserve(limit + 1); - uint256 startAfter; - std::uint64_t startHint; + visitData.items.reserve(limit); + uint256 startAfter = beast::zero; + std::uint64_t startHint = 0; if (params.isMember(jss::marker)) { - Json::Value const& marker(params[jss::marker]); - - if (!marker.isString()) + if (!params[jss::marker].isString()) return RPC::expected_field_error(jss::marker, "string"); - if (!startAfter.parseHex(marker.asString())) - { + // Marker is composed of a comma separated index and start hint. The + // former will be read as hex, and the latter using boost lexical cast. + std::stringstream marker(params[jss::marker].asString()); + std::string value; + if (!std::getline(marker, value, ',')) return rpcError(rpcINVALID_PARAMS); - } - auto const sleChannel = ledger->read({ltPAYCHAN, startAfter}); + if (!startAfter.parseHex(value)) + return rpcError(rpcINVALID_PARAMS); - if (!sleChannel) + if (!std::getline(marker, value, ',')) return rpcError(rpcINVALID_PARAMS); - if (!visitData.hasDst || - visitData.raDstAccount == (*sleChannel)[sfDestination]) + try { - visitData.items.emplace_back(sleChannel); - startHint = sleChannel->getFieldU64(sfOwnerNode); + startHint = boost::lexical_cast(value); } - else + catch (boost::bad_lexical_cast&) { return rpcError(rpcINVALID_PARAMS); } - } - else - { - startHint = 0; + + // We then must check if the object pointed to by the marker is actually + // owned by the account in the request. + auto const sle = ledger->read({ltANY, startAfter}); + + if (!sle) + return rpcError(rpcINVALID_PARAMS); + + if (!RPC::isOwnedByAccount(*ledger, sle, accountID)) + return rpcError(rpcINVALID_PARAMS); } + auto count = 0; + std::optional marker = {}; + std::uint64_t nextHint = 0; if (!forEachItemAfter( *ledger, accountID, startAfter, startHint, - limit - visitData.items.size() + 1, - [&visitData, &accountID](std::shared_ptr const& sleCur) { - if (sleCur && sleCur->getType() == ltPAYCHAN && + limit + 1, + [&visitData, &accountID, &count, &limit, &marker, &nextHint]( + std::shared_ptr const& sleCur) { + if (!sleCur) + { + assert(false); + return false; + } + + if (++count == limit) + { + marker = sleCur->key(); + nextHint = RPC::getStartHint(sleCur, visitData.accountID); + } + + if (count <= limit && sleCur->getType() == ltPAYCHAN && (*sleCur)[sfAccount] == accountID && (!visitData.hasDst || visitData.raDstAccount == (*sleCur)[sfDestination])) { visitData.items.emplace_back(sleCur); - return true; } - return false; + return true; })) { return rpcError(rpcINVALID_PARAMS); } - if (visitData.items.size() == limit + 1) + // Both conditions need to be checked because marker is set on the limit-th + // item, but if there is no item on the limit + 1 iteration, then there is + // no need to return a marker. + if (count == limit + 1 && marker) { result[jss::limit] = limit; - - result[jss::marker] = to_string(visitData.items.back()->key()); - visitData.items.pop_back(); + result[jss::marker] = + to_string(*marker) + "," + std::to_string(nextHint); } result[jss::account] = context.app.accountIDCache().toBase58(accountID); diff --git a/src/ripple/rpc/handlers/AccountLines.cpp b/src/ripple/rpc/handlers/AccountLines.cpp index ad3cd9f84d2..1044dcc7239 100644 --- a/src/ripple/rpc/handlers/AccountLines.cpp +++ b/src/ripple/rpc/handlers/AccountLines.cpp @@ -130,6 +130,9 @@ doAccountLines(RPC::JsonContext& context) if (auto err = readLimitField(limit, RPC::Tuning::accountLines, context)) return *err; + if (limit == 0) + return rpcError(rpcINVALID_PARAMS); + // this flag allows the requester to ask incoming trustlines in default // state be omitted bool ignoreDefault = params.isMember(jss::ignore_default) && @@ -138,75 +141,59 @@ doAccountLines(RPC::JsonContext& context) Json::Value& jsonLines(result[jss::lines] = Json::arrayValue); VisitData visitData = { {}, accountID, hasPeer, raPeerAccount, ignoreDefault, 0, nullptr}; - unsigned int reserve(limit); - uint256 startAfter; - std::uint64_t startHint; + uint256 startAfter = beast::zero; + std::uint64_t startHint = 0; if (params.isMember(jss::marker)) { - // We have a start point. Use limit - 1 from the result and use the - // very last one for the resume. - Json::Value const& marker(params[jss::marker]); - - if (!marker.isString()) + if (!params[jss::marker].isString()) return RPC::expected_field_error(jss::marker, "string"); - if (!startAfter.parseHex(marker.asString())) + // Marker is composed of a comma separated index and start hint. The + // former will be read as hex, and the latter using boost lexical cast. + std::stringstream marker(params[jss::marker].asString()); + std::string value; + if (!std::getline(marker, value, ',')) return rpcError(rpcINVALID_PARAMS); - auto const sleLine = ledger->read({ltRIPPLE_STATE, startAfter}); + if (!startAfter.parseHex(value)) + return rpcError(rpcINVALID_PARAMS); - if (!sleLine) + if (!std::getline(marker, value, ',')) return rpcError(rpcINVALID_PARAMS); - bool isDefault = false; - if (sleLine->getFieldAmount(sfLowLimit).getIssuer() == accountID) + try { - startHint = sleLine->getFieldU64(sfLowNode); - isDefault = !(sleLine->getFieldU32(sfFlags) & lsfLowReserve); + startHint = boost::lexical_cast(value); } - else if (sleLine->getFieldAmount(sfHighLimit).getIssuer() == accountID) + catch (boost::bad_lexical_cast&) { - startHint = sleLine->getFieldU64(sfHighNode); - isDefault = !(sleLine->getFieldU32(sfFlags) & lsfHighReserve); - } - else return rpcError(rpcINVALID_PARAMS); + } - // Caller provided the first line (startAfter), add it as first result - // (but only if it meets inclusion criteria) + // We then must check if the object pointed to by the marker is actually + // owned by the account in the request. + auto const sle = ledger->read({ltANY, startAfter}); - if (isDefault && ignoreDefault) - { - // even though we're starting our search here we don't include the - // first entry in this edge case - visitData.items.reserve(++reserve); - } - else - { - auto const line = RippleState::makeItem(accountID, sleLine); - if (line == nullptr) - return rpcError(rpcINVALID_PARAMS); + if (!sle) + return rpcError(rpcINVALID_PARAMS); - addLine(jsonLines, *line); - visitData.items.reserve(reserve); - } - } - else - { - startHint = 0; - // We have no start point, limit should be one higher than requested. - visitData.items.reserve(++reserve); + if (!RPC::isOwnedByAccount(*ledger, sle, accountID)) + return rpcError(rpcINVALID_PARAMS); } + auto count = 0; + std::optional marker = {}; + std::uint64_t nextHint = 0; { if (!forEachItemAfter( *ledger, accountID, startAfter, startHint, - reserve, - [&visitData](std::shared_ptr const& sleCur) { + limit + 1, + [&visitData, &count, &marker, &limit, &nextHint]( + std::shared_ptr const& sleCur) { bool ignore = false; if (visitData.ignoreDefault) { @@ -219,42 +206,48 @@ doAccountLines(RPC::JsonContext& context) sleCur->getFieldU32(sfFlags) & lsfHighReserve); } - auto const line = - RippleState::makeItem(visitData.accountID, sleCur); - if (line != nullptr && - (!visitData.hasPeer || - visitData.raPeerAccount == line->getAccountIDPeer())) + if (!sleCur) { - if (!ignore) - visitData.items.emplace_back(line); + assert(false); + return false; + } - visitData.lastFound = line; - visitData.foundCount++; + if (++count == limit) + { + marker = sleCur->key(); + nextHint = + RPC::getStartHint(sleCur, visitData.accountID); + } - return true; + if (!ignore && count <= limit) + { + auto const line = + RippleState::makeItem(visitData.accountID, sleCur); + + if (line != nullptr && + (!visitData.hasPeer || + visitData.raPeerAccount == + line->getAccountIDPeer())) + { + visitData.items.emplace_back(line); + } } - return false; + return true; })) { return rpcError(rpcINVALID_PARAMS); } } - // RH Note: - // If ignore_default flag is present all lines must still be iterated, the - // flag only suppresses output. It does not change how iteration works. This - // means the RPC call may return an empty set AND a marker. In this case - // another query must be made until iteration is complete if a complete set - // of non-default state lines are required. - if (visitData.items.size() == reserve || visitData.foundCount >= reserve) + // Both conditions need to be checked because marker is set on the limit-th + // item, but if there is no item on the limit + 1 iteration, then there is + // no need to return a marker. + if (count == limit + 1 && marker) { result[jss::limit] = limit; - - RippleState::pointer line(visitData.lastFound); - result[jss::marker] = to_string(line->key()); - if (visitData.items.back() == visitData.lastFound) - visitData.items.pop_back(); + result[jss::marker] = + to_string(*marker) + "," + std::to_string(nextHint); } result[jss::account] = context.app.accountIDCache().toBase58(accountID); diff --git a/src/ripple/rpc/handlers/AccountOffers.cpp b/src/ripple/rpc/handlers/AccountOffers.cpp index 063d0874378..d3178756300 100644 --- a/src/ripple/rpc/handlers/AccountOffers.cpp +++ b/src/ripple/rpc/handlers/AccountOffers.cpp @@ -86,68 +86,94 @@ doAccountOffers(RPC::JsonContext& context) if (auto err = readLimitField(limit, RPC::Tuning::accountOffers, context)) return *err; + if (limit == 0) + return rpcError(rpcINVALID_PARAMS); + Json::Value& jsonOffers(result[jss::offers] = Json::arrayValue); std::vector> offers; - unsigned int reserve(limit); - uint256 startAfter; - std::uint64_t startHint; + uint256 startAfter = beast::zero; + std::uint64_t startHint = 0; if (params.isMember(jss::marker)) { - // We have a start point. Use limit - 1 from the result and use the - // very last one for the resume. - Json::Value const& marker(params[jss::marker]); - - if (!marker.isString()) + if (!params[jss::marker].isString()) return RPC::expected_field_error(jss::marker, "string"); - if (!startAfter.parseHex(marker.asString())) + // Marker is composed of a comma separated index and start hint. The + // former will be read as hex, and the latter using boost lexical cast. + std::stringstream marker(params[jss::marker].asString()); + std::string value; + if (!std::getline(marker, value, ',')) return rpcError(rpcINVALID_PARAMS); - auto const sleOffer = ledger->read({ltOFFER, startAfter}); + if (!startAfter.parseHex(value)) + return rpcError(rpcINVALID_PARAMS); + + if (!std::getline(marker, value, ',')) + return rpcError(rpcINVALID_PARAMS); - if (!sleOffer || accountID != sleOffer->getAccountID(sfAccount)) + try + { + startHint = boost::lexical_cast(value); + } + catch (boost::bad_lexical_cast&) { return rpcError(rpcINVALID_PARAMS); } - startHint = sleOffer->getFieldU64(sfOwnerNode); - // Caller provided the first offer (startAfter), add it as first result - appendOfferJson(sleOffer, jsonOffers); - offers.reserve(reserve); - } - else - { - startHint = 0; - // We have no start point, limit should be one higher than requested. - offers.reserve(++reserve); + // We then must check if the object pointed to by the marker is actually + // owned by the account in the request. + auto const sle = ledger->read({ltANY, startAfter}); + + if (!sle) + return rpcError(rpcINVALID_PARAMS); + + if (!RPC::isOwnedByAccount(*ledger, sle, accountID)) + return rpcError(rpcINVALID_PARAMS); } + auto count = 0; + std::optional marker = {}; + std::uint64_t nextHint = 0; if (!forEachItemAfter( *ledger, accountID, startAfter, startHint, - reserve, - [&offers](std::shared_ptr const& offer) { - if (offer->getType() == ltOFFER) + limit + 1, + [&offers, &count, &marker, &limit, &nextHint, &accountID]( + std::shared_ptr const& sle) { + if (!sle) + { + assert(false); + return false; + } + + if (++count == limit) { - offers.emplace_back(offer); - return true; + marker = sle->key(); + nextHint = RPC::getStartHint(sle, accountID); } - return false; + if (count <= limit && sle->getType() == ltOFFER) + { + offers.emplace_back(sle); + } + + return true; })) { return rpcError(rpcINVALID_PARAMS); } - if (offers.size() == reserve) + // Both conditions need to be checked because marker is set on the limit-th + // item, but if there is no item on the limit + 1 iteration, then there is + // no need to return a marker. + if (count == limit + 1 && marker) { result[jss::limit] = limit; - - result[jss::marker] = to_string(offers.back()->key()); - offers.pop_back(); + result[jss::marker] = + to_string(*marker) + "," + std::to_string(nextHint); } for (auto const& offer : offers) diff --git a/src/ripple/rpc/impl/RPCHelpers.cpp b/src/ripple/rpc/impl/RPCHelpers.cpp index be1d005a38a..5c42aae969b 100644 --- a/src/ripple/rpc/impl/RPCHelpers.cpp +++ b/src/ripple/rpc/impl/RPCHelpers.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -89,6 +90,47 @@ accountFromString(AccountID& result, std::string const& strIdent, bool bStrict) return Json::objectValue; } +std::uint64_t +getStartHint(std::shared_ptr const& sle, AccountID const& accountID) +{ + if (sle->getType() == ltRIPPLE_STATE) + { + if (sle->getFieldAmount(sfLowLimit).getIssuer() == accountID) + return sle->getFieldU64(sfLowNode); + else if (sle->getFieldAmount(sfHighLimit).getIssuer() == accountID) + return sle->getFieldU64(sfHighNode); + } + + if (!sle->isFieldPresent(sfOwnerNode)) + return 0; + + return sle->getFieldU64(sfOwnerNode); +} + +bool +isOwnedByAccount( + ReadView const& ledger, + std::shared_ptr const& sle, + AccountID const& accountID) +{ + if (sle->getType() == ltRIPPLE_STATE) + { + return (sle->getFieldAmount(sfLowLimit).getIssuer() == accountID) || + (sle->getFieldAmount(sfHighLimit).getIssuer() == accountID); + } + else if (sle->isFieldPresent(sfAccount)) + { + return sle->getAccountID(sfAccount) == accountID; + } + else if (sle->getType() == ltSIGNER_LIST) + { + Keylet const accountSignerList = keylet::signers(accountID); + return sle->key() == accountSignerList.key; + } + + return false; +} + bool getAccountObjects( ReadView const& ledger, @@ -144,19 +186,19 @@ getAccountObjects( typeMatchesFilter(typeFilter.value(), sleNode->getType())) { jvObjects.append(sleNode->getJson(JsonOptions::none)); + } - if (++i == limit) + if (++i == limit) + { + if (++iter != entries.end()) { - if (++iter != entries.end()) - { - jvResult[jss::limit] = limit; - jvResult[jss::marker] = - to_string(dirIndex) + ',' + to_string(*iter); - return true; - } - - break; + jvResult[jss::limit] = limit; + jvResult[jss::marker] = + to_string(dirIndex) + ',' + to_string(*iter); + return true; } + + break; } } diff --git a/src/ripple/rpc/impl/RPCHelpers.h b/src/ripple/rpc/impl/RPCHelpers.h index 0ecdebe24a8..e3d44c2e730 100644 --- a/src/ripple/rpc/impl/RPCHelpers.h +++ b/src/ripple/rpc/impl/RPCHelpers.h @@ -71,6 +71,25 @@ accountFromStringWithCode( std::string const& strIdent, bool bStrict = false); +/** Gets the start hint for traversing account objects + * @param sle - Ledger entry defined by the marker passed into the RPC. + * @param accountID - The ID of the account whose objects you are traversing. + */ +std::uint64_t +getStartHint(std::shared_ptr const& sle, AccountID const& accountID); + +/** + * Tests if a SLE is owned by accountID. + * @param ledger - The ledger used to search for the sle. + * @param sle - The SLE to test for ownership. + * @param account - The account being tested for SLE ownership. + */ +bool +isOwnedByAccount( + ReadView const& ledger, + std::shared_ptr const& sle, + AccountID const& accountID); + /** Gathers all objects for an account in a ledger. @param ledger Ledger to search account objects. @param account AccountID to find objects for. diff --git a/src/ripple/rpc/impl/Tuning.h b/src/ripple/rpc/impl/Tuning.h index f52c60f096d..233e7379489 100644 --- a/src/ripple/rpc/impl/Tuning.h +++ b/src/ripple/rpc/impl/Tuning.h @@ -46,7 +46,7 @@ static LimitRange constexpr accountObjects = {10, 200, 400}; static LimitRange constexpr accountOffers = {10, 200, 400}; /** Limits for the book_offers command. */ -static LimitRange constexpr bookOffers = {0, 300, 400}; +static LimitRange constexpr bookOffers = {0, 60, 100}; /** Limits for the no_ripple_check command. */ static LimitRange constexpr noRippleCheck = {10, 300, 400}; diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp index 0e38de517de..cf600a9fc87 100644 --- a/src/test/app/PayChan_test.cpp +++ b/src/test/app/PayChan_test.cpp @@ -1033,8 +1033,7 @@ struct PayChan_test : public beast::unit_test::suite { // degenerate case auto const r = testLimit(env, alice, 0); - BEAST_EXPECT(r.isMember(jss::marker)); - BEAST_EXPECT(r[jss::channels].size() == 0); + BEAST_EXPECT(r.isMember(jss::error_message)); } } diff --git a/src/test/rpc/AccountLinesRPC_test.cpp b/src/test/rpc/AccountLinesRPC_test.cpp index e7dbe55933d..bdd376b3aae 100644 --- a/src/test/rpc/AccountLinesRPC_test.cpp +++ b/src/test/rpc/AccountLinesRPC_test.cpp @@ -320,7 +320,7 @@ class AccountLinesRPC_test : public beast::unit_test::suite "account_lines", R"({"account": ")" + alice.human() + R"(", )" - R"("limit": 1, )" + R"("limit": 10, )" R"("peer": ")" + gw2.human() + R"("})"); auto const& line = lines[jss::result][jss::lines][0u]; @@ -363,6 +363,91 @@ class AccountLinesRPC_test : public beast::unit_test::suite } } + void + testAccountLinesMarker() + { + testcase("Entry pointed to by marker is not owned by account"); + using namespace test::jtx; + Env env(*this); + + // The goal of this test is observe account_lines RPC calls return an + // error message when the SLE pointed to by the marker is not owned by + // the Account being traversed. + // + // To start, we'll create an environment with some trust lines, offers + // and a signers list. + Account const alice{"alice"}; + Account const becky{"becky"}; + Account const gw1{"gw1"}; + env.fund(XRP(10000), alice, becky, gw1); + env.close(); + + // Give alice a SignerList. + Account const bogie{"bogie"}; + env(signers(alice, 2, {{bogie, 3}})); + env.close(); + + auto const EUR = gw1["EUR"]; + env(trust(alice, EUR(200))); + env(trust(becky, EUR(200))); + env.close(); + + // Get all account objects for alice and verify that her + // signerlist is first. This is only a (reliable) coincidence of + // object naming. So if any of alice's objects are renamed this + // may fail. + Json::Value const aliceObjects = env.rpc( + "json", + "account_objects", + R"({"account": ")" + alice.human() + + R"(", )" + R"("limit": 10})"); + Json::Value const& aliceSignerList = + aliceObjects[jss::result][jss::account_objects][0u]; + if (!(aliceSignerList[sfLedgerEntryType.jsonName] == jss::SignerList)) + { + fail( + "alice's account objects are misordered. " + "Please reorder the objects so the SignerList is first.", + __FILE__, + __LINE__); + return; + } + + // Get account_lines for alice. Limit at 1, so we get a marker + // pointing to her SignerList. + auto const aliceLines1 = env.rpc( + "json", + "account_lines", + R"({"account": ")" + alice.human() + R"(", "limit": 1})"); + BEAST_EXPECT(aliceLines1[jss::result].isMember(jss::marker)); + + // Verify that the marker points at the signer list. + std::string const aliceMarker = + aliceLines1[jss::result][jss::marker].asString(); + std::string const markerIndex = + aliceMarker.substr(0, aliceMarker.find(',')); + BEAST_EXPECT(markerIndex == aliceSignerList[jss::index].asString()); + + // When we fetch Alice's remaining lines we should find one and no more. + auto const aliceLines2 = env.rpc( + "json", + "account_lines", + R"({"account": ")" + alice.human() + R"(", "marker": ")" + + aliceMarker + R"("})"); + BEAST_EXPECT(aliceLines2[jss::result][jss::lines].size() == 1); + BEAST_EXPECT(!aliceLines2[jss::result].isMember(jss::marker)); + + // Get account lines for beckys account, using alices SignerList as a + // marker. This should cause an error. + auto const beckyLines = env.rpc( + "json", + "account_lines", + R"({"account": ")" + becky.human() + R"(", "marker": ")" + + aliceMarker + R"("})"); + BEAST_EXPECT(beckyLines[jss::result].isMember(jss::error_message)); + } + void testAccountLineDelete() { @@ -390,8 +475,10 @@ class AccountLinesRPC_test : public beast::unit_test::suite env.close(); auto const USD = gw1["USD"]; + auto const AUD = gw1["AUD"]; auto const EUR = gw2["EUR"]; env(trust(alice, USD(200))); + env(trust(alice, AUD(200))); env(trust(becky, EUR(200))); env(trust(cheri, EUR(200))); env.close(); @@ -414,7 +501,7 @@ class AccountLinesRPC_test : public beast::unit_test::suite "account_lines", R"({"account": ")" + alice.human() + R"(", )" - R"("limit": 1})"); + R"("limit": 2})"); BEAST_EXPECT( linesBeg[jss::result][jss::lines][0u][jss::currency] == "USD"); BEAST_EXPECT(linesBeg[jss::result].isMember(jss::marker)); @@ -966,7 +1053,7 @@ class AccountLinesRPC_test : public beast::unit_test::suite R"({"account": ")" + alice.human() + R"(", )" - R"("limit": 1, )" + R"("limit": 10, )" R"("peer": ")" + gw2.human() + R"("}})"); auto const& line = lines[jss::result][jss::lines][0u]; @@ -1068,8 +1155,10 @@ class AccountLinesRPC_test : public beast::unit_test::suite env.close(); auto const USD = gw1["USD"]; + auto const AUD = gw1["AUD"]; auto const EUR = gw2["EUR"]; env(trust(alice, USD(200))); + env(trust(alice, AUD(200))); env(trust(becky, EUR(200))); env(trust(cheri, EUR(200))); env.close(); @@ -1098,7 +1187,7 @@ class AccountLinesRPC_test : public beast::unit_test::suite R"({"account": ")" + alice.human() + R"(", )" - R"("limit": 1}})"); + R"("limit": 2}})"); BEAST_EXPECT( linesBeg[jss::result][jss::lines][0u][jss::currency] == "USD"); BEAST_EXPECT(linesBeg[jss::result].isMember(jss::marker)); @@ -1143,6 +1232,7 @@ class AccountLinesRPC_test : public beast::unit_test::suite run() override { testAccountLines(); + testAccountLinesMarker(); testAccountLineDelete(); testAccountLines2(); testAccountLineDelete2(); diff --git a/src/test/rpc/AccountOffers_test.cpp b/src/test/rpc/AccountOffers_test.cpp index 8871315aa79..f4ad0a72595 100644 --- a/src/test/rpc/AccountOffers_test.cpp +++ b/src/test/rpc/AccountOffers_test.cpp @@ -81,7 +81,8 @@ class AccountOffers_test : public beast::unit_test::suite "json", "account_offers", jvParams.toStyledString())[jss::result]; auto const& jro_l = jrr_l[jss::offers]; BEAST_EXPECT(checkMarker(jrr_l)); - BEAST_EXPECT(checkArraySize(jro_l, 10u)); + // 9u is the expected size, since one account object is a trustline + BEAST_EXPECT(checkArraySize(jro_l, 9u)); } void @@ -173,6 +174,7 @@ class AccountOffers_test : public beast::unit_test::suite // last item...with previous marker passed jvParams[jss::marker] = jrr_l_2[jss::marker]; + jvParams[jss::limit] = 10u; auto const jrr_l_3 = env.rpc( "json", "account_offers", @@ -203,9 +205,17 @@ class AccountOffers_test : public beast::unit_test::suite "account_offers", jvParams.toStyledString())[jss::result]; auto const& jro = jrr[jss::offers]; - BEAST_EXPECT(checkArraySize(jro, asAdmin ? 0u : 3u)); - BEAST_EXPECT( - asAdmin ? checkMarker(jrr) : (!jrr.isMember(jss::marker))); + if (asAdmin) + { + // limit == 0 is invalid + BEAST_EXPECT(jrr.isMember(jss::error_message)); + } + else + { + // Call should enforce min limit of 10 + BEAST_EXPECT(checkArraySize(jro, 3u)); + BEAST_EXPECT(!jrr.isMember(jss::marker)); + } } }