From 041aba9a0b0d03d7341cd352ed128f03332cb819 Mon Sep 17 00:00:00 2001 From: Alex Kremer Date: Fri, 18 Nov 2022 18:51:18 +0100 Subject: [PATCH] Implement account ownership check and fix paging (#383) Fixes #222 --- src/rpc/Errors.h | 28 ++++++++++++ src/rpc/RPC.cpp | 2 +- src/rpc/RPC.h | 4 +- src/rpc/RPCHelpers.cpp | 70 +++++++---------------------- src/rpc/RPCHelpers.h | 1 - src/rpc/handlers/AccountObjects.cpp | 3 +- src/rpc/handlers/Subscribe.cpp | 3 +- 7 files changed, 48 insertions(+), 63 deletions(-) diff --git a/src/rpc/Errors.h b/src/rpc/Errors.h index 6854cc45..3b143c6b 100644 --- a/src/rpc/Errors.h +++ b/src/rpc/Errors.h @@ -81,6 +81,34 @@ struct Status return *err != RippledError::rpcSUCCESS; return true; } + + /** + * @brief Returns true if the Status contains the desired @ref RippledError + * + * @param other The RippledError to match + * @return bool true if status matches given error; false otherwise + */ + bool + operator==(RippledError other) const + { + if (auto err = std::get_if(&code)) + return *err == other; + return false; + } + + /** + * @brief Returns true if the Status contains the desired @ref ClioError + * + * @param other The RippledError to match + * @return bool true if status matches given error; false otherwise + */ + bool + operator==(ClioError other) const + { + if (auto err = std::get_if(&code)) + return *err == other; + return false; + } }; /** diff --git a/src/rpc/RPC.cpp b/src/rpc/RPC.cpp index 86ebd39f..1082da27 100644 --- a/src/rpc/RPC.cpp +++ b/src/rpc/RPC.cpp @@ -352,7 +352,7 @@ buildResponse(Context const& ctx) if (auto object = get_if(&v); object && not shouldSuppressValidatedFlag(ctx)) { - (*object)["validated"] = true; + (*object)[JS(validated)] = true; } return v; diff --git a/src/rpc/RPC.h b/src/rpc/RPC.h index 636bf6f5..d595f835 100644 --- a/src/rpc/RPC.h +++ b/src/rpc/RPC.h @@ -73,13 +73,13 @@ struct AccountCursor std::uint32_t hint; std::string - toString() + toString() const { return ripple::strHex(index) + "," + std::to_string(hint); } bool - isNonZero() + isNonZero() const { return index.isNonZero() || hint != 0; } diff --git a/src/rpc/RPCHelpers.cpp b/src/rpc/RPCHelpers.cpp index 10bc8c24..f05edfe6 100644 --- a/src/rpc/RPCHelpers.cpp +++ b/src/rpc/RPCHelpers.cpp @@ -81,35 +81,11 @@ getRequiredUInt(boost::json::object const& request, std::string const& field) throw InvalidParamsError("Missing field " + field); } -bool -isOwnedByAccount(ripple::SLE const& sle, ripple::AccountID const& accountID) -{ - if (sle.getType() == ripple::ltRIPPLE_STATE) - { - return (sle.getFieldAmount(ripple::sfLowLimit).getIssuer() == - accountID) || - (sle.getFieldAmount(ripple::sfHighLimit).getIssuer() == accountID); - } - else if (sle.isFieldPresent(ripple::sfAccount)) - { - return sle.getAccountID(ripple::sfAccount) == accountID; - } - else if (sle.getType() == ripple::ltSIGNER_LIST) - { - ripple::Keylet const accountSignerList = - ripple::keylet::signers(accountID); - return sle.key() == accountSignerList.key; - } - - return false; -} - std::optional parseAccountCursor( BackendInterface const& backend, std::uint32_t seq, std::optional jsonCursor, - ripple::AccountID const& accountID, boost::asio::yield_context& yield) { ripple::uint256 cursorIndex = beast::zero; @@ -140,19 +116,6 @@ parseAccountCursor( return {}; } - // We then must check if the object pointed to by the marker is actually - // owned by the account in the request. - auto const ownedNode = backend.fetchLedgerObject(cursorIndex, seq, yield); - - if (!ownedNode) - return {}; - - ripple::SerialIter it{ownedNode->data(), ownedNode->size()}; - ripple::SLE sle{it, cursorIndex}; - - if (!isOwnedByAccount(sle, accountID)) - return {}; - return AccountCursor({cursorIndex, startHint}); } @@ -663,13 +626,11 @@ traverseOwnedNodes( ripple::keylet::account(accountID).key, sequence, yield)) return Status{RippledError::rpcACT_NOT_FOUND}; - auto parsedCursor = - parseAccountCursor(backend, sequence, jsonCursor, accountID, yield); - - if (!parsedCursor) + auto maybeCursor = parseAccountCursor(backend, sequence, jsonCursor, yield); + if (!maybeCursor) return Status(ripple::rpcINVALID_PARAMS, "Malformed cursor"); - auto [hexCursor, startHint] = *parsedCursor; + auto [hexCursor, startHint] = *maybeCursor; return traverseOwnedNodes( backend, @@ -715,22 +676,21 @@ traverseOwnedNodes( auto hintDir = backend.fetchLedgerObject(hintIndex.key, sequence, yield); - if (hintDir) - { - ripple::SerialIter it{hintDir->data(), hintDir->size()}; - ripple::SLE sle{it, hintIndex.key}; + if (!hintDir) + return Status(ripple::rpcINVALID_PARAMS, "Invalid marker"); - for (auto const& key : sle.getFieldV256(ripple::sfIndexes)) - { - if (key == hexMarker) - { - // We found the hint, we can start here - currentIndex = hintIndex; - break; - } - } + ripple::SerialIter it{hintDir->data(), hintDir->size()}; + ripple::SLE sle{it, hintIndex.key}; + + if (auto const& indexes = sle.getFieldV256(ripple::sfIndexes); + std::find(std::begin(indexes), std::end(indexes), hexMarker) == + std::end(indexes)) + { + // result in empty dataset + return AccountCursor({beast::zero, 0}); } + currentIndex = hintIndex; bool found = false; for (;;) { diff --git a/src/rpc/RPCHelpers.h b/src/rpc/RPCHelpers.h index 24815879..5ebf864e 100644 --- a/src/rpc/RPCHelpers.h +++ b/src/rpc/RPCHelpers.h @@ -36,7 +36,6 @@ parseAccountCursor( BackendInterface const& backend, std::uint32_t seq, std::optional jsonCursor, - ripple::AccountID const& accountID, boost::asio::yield_context& yield); // TODO this function should probably be in a different file and namespace diff --git a/src/rpc/handlers/AccountObjects.cpp b/src/rpc/handlers/AccountObjects.cpp index b059d94c..c3107790 100644 --- a/src/rpc/handlers/AccountObjects.cpp +++ b/src/rpc/handlers/AccountObjects.cpp @@ -202,8 +202,7 @@ doAccountObjects(Context const& context) if (auto status = std::get_if(&next)) return *status; - auto nextMarker = std::get(next); - + auto const& nextMarker = std::get(next); if (nextMarker.isNonZero()) response[JS(marker)] = nextMarker.toString(); diff --git a/src/rpc/handlers/Subscribe.cpp b/src/rpc/handlers/Subscribe.cpp index 1ce2fd76..450ce717 100644 --- a/src/rpc/handlers/Subscribe.cpp +++ b/src/rpc/handlers/Subscribe.cpp @@ -442,8 +442,7 @@ doUnsubscribe(Context const& context) if (request.contains("books")) unsubscribeToBooks(books, context.session, *context.subscriptions); - boost::json::object response = {{"status", "success"}}; - return response; + return boost::json::object{}; } } // namespace RPC