diff --git a/src/ripple/app/tx/impl/DeleteOracle.cpp b/src/ripple/app/tx/impl/DeleteOracle.cpp index dfaecc384d4..b13954b52b7 100644 --- a/src/ripple/app/tx/impl/DeleteOracle.cpp +++ b/src/ripple/app/tx/impl/DeleteOracle.cpp @@ -48,7 +48,11 @@ TER DeleteOracle::preclaim(PreclaimContext const& ctx) { if (!ctx.view.exists(keylet::account(ctx.tx.getAccountID(sfAccount)))) + { + // LCOV_EXCL_START return terNO_ACCOUNT; + // LCOV_EXCL_STOP + } if (auto const sle = ctx.view.read(keylet::oracle( ctx.tx.getAccountID(sfAccount), ctx.tx[sfOracleDocumentID])); @@ -60,8 +64,10 @@ DeleteOracle::preclaim(PreclaimContext const& ctx) else if (ctx.tx.getAccountID(sfAccount) != sle->getAccountID(sfOwner)) { // this can't happen because of the above check + // LCOV_EXCL_START JLOG(ctx.j.debug()) << "Oracle Delete: invalid account."; return tecINTERNAL; + // LCOV_EXCL_STOP } return tesSUCCESS; } @@ -74,18 +80,28 @@ DeleteOracle::deleteOracle( beast::Journal j) { if (!sle) - return tesSUCCESS; + { + // LCOV_EXCL_START + return tecINTERNAL; + // LCOV_EXCL_STOP + } if (!view.dirRemove( keylet::ownerDir(account), (*sle)[sfOwnerNode], sle->key(), true)) { + // LCOV_EXCL_START JLOG(j.fatal()) << "Unable to delete Oracle from owner."; return tefBAD_LEDGER; + // LCOV_EXCL_STOP } auto const sleOwner = view.peek(keylet::account(account)); if (!sleOwner) + { + // LCOV_EXCL_START return tecINTERNAL; + // LCOV_EXCL_STOP + } auto const count = sle->getFieldArray(sfPriceDataSeries).size() > 5 ? -2 : -1; @@ -104,7 +120,9 @@ DeleteOracle::doApply() keylet::oracle(account_, ctx_.tx[sfOracleDocumentID]))) return deleteOracle(ctx_.view(), sle, account_, j_); + // LCOV_EXCL_START return tecINTERNAL; + // LCOV_EXCL_STOP } } // namespace ripple diff --git a/src/ripple/app/tx/impl/SetOracle.cpp b/src/ripple/app/tx/impl/SetOracle.cpp index 37dc6fcd212..3d60f3dc02d 100644 --- a/src/ripple/app/tx/impl/SetOracle.cpp +++ b/src/ripple/app/tx/impl/SetOracle.cpp @@ -74,7 +74,11 @@ SetOracle::preclaim(PreclaimContext const& ctx) auto const sleSetter = ctx.view.read(keylet::account(ctx.tx.getAccountID(sfAccount))); if (!sleSetter) + { + // LCOV_EXCL_START return terNO_ACCOUNT; + // LCOV_EXCL_STOP + } // lastUpdateTime must be within maxLastUpdateTimeDelta seconds // of the last closed ledger @@ -88,8 +92,11 @@ SetOracle::preclaim(PreclaimContext const& ctx) std::size_t const lastUpdateTimeEpoch = lastUpdateTime - epoch_offset.count(); if (closeTime < maxLastUpdateTimeDelta) - Throw( - "Oracle: close time is less than maxLastUpdateTimeDelta"); + { + // LCOV_EXCL_START + return tecINTERNAL; + // LCOV_EXCL_STOP + } if (lastUpdateTimeEpoch < (closeTime - maxLastUpdateTimeDelta) || lastUpdateTimeEpoch > (closeTime + maxLastUpdateTimeDelta)) return tecINVALID_UPDATE_TIME; @@ -194,7 +201,9 @@ adjustOwnerCount(ApplyContext& ctx, int count) return true; } + // LCOV_EXCL_START return false; + // LCOV_EXCL_STOP } static void @@ -274,7 +283,11 @@ SetOracle::doApply() auto const newCount = pairs.size() > 5 ? 2 : 1; auto const adjust = newCount - oldCount; if (adjust != 0 && !adjustOwnerCount(ctx_, adjust)) + { + // LCOV_EXCL_START return tefINTERNAL; + // LCOV_EXCL_STOP + } ctx_.view().update(sle); } @@ -295,13 +308,21 @@ SetOracle::doApply() auto page = ctx_.view().dirInsert( keylet::ownerDir(account_), sle->key(), describeOwnerDir(account_)); if (!page) + { + // LCOV_EXCL_START return tecDIR_FULL; + // LCOV_EXCL_STOP + } (*sle)[sfOwnerNode] = *page; auto const count = series.size() > 5 ? 2 : 1; if (!adjustOwnerCount(ctx_, count)) + { + // LCOV_EXCL_START return tefINTERNAL; + // LCOV_EXCL_STOP + } ctx_.view().insert(sle); } diff --git a/src/ripple/rpc/handlers/GetAggregatePrice.cpp b/src/ripple/rpc/handlers/GetAggregatePrice.cpp index 5490cc4fcff..aa4019c0a74 100644 --- a/src/ripple/rpc/handlers/GetAggregatePrice.cpp +++ b/src/ripple/rpc/handlers/GetAggregatePrice.cpp @@ -85,10 +85,15 @@ iteratePriceData( auto const ledger = context.ledgerMaster.getLedgerBySeq(prevSeq); if (!ledger) + { + // LCOV_EXCL_START return; + // LCOV_EXCL_STOP + } meta = ledger->txRead(prevTx).second; + prevChain = chain; for (STObject const& node : meta->getFieldArray(sfAffectedNodes)) { if (node.getFieldU16(sfLedgerEntryType) != ltORACLE) @@ -96,7 +101,6 @@ iteratePriceData( continue; } - prevChain = chain; chain = &node; isNew = node.isFieldPresent(sfNewFields); // if a meta is for the new and this is the first @@ -170,21 +174,49 @@ doGetAggregatePrice(RPC::JsonContext& context) if (!params.isMember(jss::quote_asset)) return RPC::missing_field_error(jss::quote_asset); + // Lambda to validate uint type + // support positive int, uint, and a number represented as a string + auto validUInt = [](Json::Value const& params, + Json::StaticString const& field) { + auto const& jv = params[field]; + std::uint32_t v; + return jv.isUInt() || (jv.isInt() && jv.asInt() >= 0) || + (jv.isString() && beast::lexicalCastChecked(v, jv.asString())); + }; + // Lambda to get `trim` and `time_threshold` fields. If the field // is not included in the input then a default value is returned. - auto getField = [¶ms]( + auto getField = [¶ms, &validUInt]( Json::StaticString const& field, unsigned int def = 0) -> std::variant { if (params.isMember(field)) { - if (!params[field].isConvertibleTo(Json::ValueType::uintValue)) - return rpcORACLE_MALFORMED; + if (!validUInt(params, field)) + return rpcINVALID_PARAMS; return params[field].asUInt(); } return def; }; + // Lambda to get `base_asset` and `quote_asset`. The values have + // to conform to the Currency type. + auto getCurrency = + [¶ms](SField const& sField, Json::StaticString const& field) + -> std::variant { + try + { + if (params[field].asString().empty()) + return rpcINVALID_PARAMS; + currencyFromJson(sField, params[field]); + return params[field]; + } + catch (...) + { + return rpcINVALID_PARAMS; + } + }; + auto const trim = getField(jss::trim); if (std::holds_alternative(trim)) { @@ -206,8 +238,18 @@ doGetAggregatePrice(RPC::JsonContext& context) return result; } - auto const& baseAsset = params[jss::base_asset]; - auto const& quoteAsset = params[jss::quote_asset]; + auto const baseAsset = getCurrency(sfBaseAsset, jss::base_asset); + if (std::holds_alternative(baseAsset)) + { + RPC::inject_error(std::get(baseAsset), result); + return result; + } + auto const quoteAsset = getCurrency(sfQuoteAsset, jss::quote_asset); + if (std::holds_alternative(quoteAsset)) + { + RPC::inject_error(std::get(quoteAsset), result); + return result; + } // Collect the dataset into bimap keyed by lastUpdateTime and // STAmount (Number is int64 and price is uint64) @@ -220,8 +262,7 @@ doGetAggregatePrice(RPC::JsonContext& context) RPC::inject_error(rpcORACLE_MALFORMED, result); return result; } - auto const documentID = oracle[jss::oracle_document_id].isConvertibleTo( - Json::ValueType::uintValue) + auto const documentID = validUInt(oracle, jss::oracle_document_id) ? std::make_optional(oracle[jss::oracle_document_id].asUInt()) : std::nullopt; auto const account = @@ -235,7 +276,11 @@ doGetAggregatePrice(RPC::JsonContext& context) std::shared_ptr ledger; result = RPC::lookupLedger(ledger, context); if (!ledger) + { + // LCOV_EXCL_START return result; + // LCOV_EXCL_STOP + } auto const sle = ledger->read(keylet::oracle(*account, *documentID)); iteratePriceData(context, sle, [&](STObject const& node) { @@ -246,9 +291,9 @@ doGetAggregatePrice(RPC::JsonContext& context) series.end(), [&](STObject const& o) -> bool { return o.getFieldCurrency(sfBaseAsset).getText() == - baseAsset && + std::get(baseAsset) && o.getFieldCurrency(sfQuoteAsset).getText() == - quoteAsset && + std::get(quoteAsset) && o.isFieldPresent(sfAssetPrice); }); iter != series.end()) @@ -287,10 +332,15 @@ doGetAggregatePrice(RPC::JsonContext& context) prices.left.erase( prices.left.upper_bound(upperBound), prices.left.end()); + // At least one element should remain since upperBound is either + // equal to oldestTime or is less than latestTime, in which case + // the data is deleted between the oldestTime and upperBound. if (prices.empty()) { - RPC::inject_error(rpcOBJECT_NOT_FOUND, result); + // LCOV_EXCL_START + RPC::inject_error(rpcINTERNAL, result); return result; + // LCOV_EXCL_STOP } } result[jss::time] = latestTime; diff --git a/src/ripple/rpc/handlers/LedgerEntry.cpp b/src/ripple/rpc/handlers/LedgerEntry.cpp index dfbc32e606a..8985a880824 100644 --- a/src/ripple/rpc/handlers/LedgerEntry.cpp +++ b/src/ripple/rpc/handlers/LedgerEntry.cpp @@ -624,7 +624,7 @@ doLedgerEntry(RPC::JsonContext& context) auto const& oracle = context.params[jss::oracle]; auto const documentID = [&]() -> std::optional { auto const& id = oracle[jss::oracle_document_id]; - if (id.isConvertibleTo(Json::ValueType::uintValue)) + if (id.isUInt() || (id.isInt() && id.asInt() >= 0)) return std::make_optional(id.asUInt()); else if (id.isString()) { diff --git a/src/test/app/Oracle_test.cpp b/src/test/app/Oracle_test.cpp index f5488c793a1..4dd446230e7 100644 --- a/src/test/app/Oracle_test.cpp +++ b/src/test/app/Oracle_test.cpp @@ -163,7 +163,7 @@ struct Oracle_test : public beast::unit_test::suite env.fund(XRP(1'000), owner); Oracle oracle(env, {.owner = owner}, false); - // Symbol class or provider not included on create + // Asset class or provider not included on create oracle.set(CreateArg{ .assetClass = std::nullopt, .provider = "provider", @@ -174,7 +174,7 @@ struct Oracle_test : public beast::unit_test::suite .uri = "URI", .err = ter(temMALFORMED)}); - // Symbol class or provider are included on update + // Asset class or provider are included on update // and don't match the current values oracle.set(CreateArg{}); BEAST_EXPECT(oracle.exists()); @@ -194,7 +194,7 @@ struct Oracle_test : public beast::unit_test::suite Oracle oracle(env, {.owner = owner}, false); // Fields too long - // Symbol class + // Asset class std::string assetClass(17, '0'); oracle.set( CreateArg{.assetClass = assetClass, .err = ter(temMALFORMED)}); @@ -203,6 +203,13 @@ struct Oracle_test : public beast::unit_test::suite oracle.set(CreateArg{.provider = large, .err = ter(temMALFORMED)}); // URI oracle.set(CreateArg{.uri = large, .err = ter(temMALFORMED)}); + // Empty field + // Asset class + oracle.set(CreateArg{.assetClass = "", .err = ter(temMALFORMED)}); + // provider + oracle.set(CreateArg{.provider = "", .err = ter(temMALFORMED)}); + // URI + oracle.set(CreateArg{.uri = "", .err = ter(temMALFORMED)}); } { @@ -224,6 +231,12 @@ struct Oracle_test : public beast::unit_test::suite // Invalid update time using namespace std::chrono; Env env(*this); + auto closeTime = [&]() { + return duration_cast( + env.current()->info().closeTime.time_since_epoch() - + 10'000s) + .count(); + }; env.fund(XRP(1'000), owner); Oracle oracle(env, {.owner = owner}); BEAST_EXPECT(oracle.exists()); @@ -231,20 +244,25 @@ struct Oracle_test : public beast::unit_test::suite // Less than the last close time - 300s oracle.set(UpdateArg{ .series = {{"XRP", "USD", 740, 1}}, - .lastUpdateTime = testStartTime.count() + 400 - 301, + .lastUpdateTime = static_cast(closeTime() - 301), .err = ter(tecINVALID_UPDATE_TIME)}); // Greater than last close time + 300s oracle.set(UpdateArg{ .series = {{"XRP", "USD", 740, 1}}, - .lastUpdateTime = testStartTime.count() + 400 + 301, + .lastUpdateTime = static_cast(closeTime() + 311), .err = ter(tecINVALID_UPDATE_TIME)}); oracle.set(UpdateArg{.series = {{"XRP", "USD", 740, 1}}}); - BEAST_EXPECT( - oracle.expectLastUpdateTime(testStartTime.count() + 450)); + BEAST_EXPECT(oracle.expectLastUpdateTime( + static_cast(testStartTime.count() + 450))); // Less than the previous lastUpdateTime oracle.set(UpdateArg{ .series = {{"XRP", "USD", 740, 1}}, - .lastUpdateTime = testStartTime.count() + 449, + .lastUpdateTime = static_cast(449), + .err = ter(tecINVALID_UPDATE_TIME)}); + // Less than the epoch time + oracle.set(UpdateArg{ + .series = {{"XRP", "USD", 740, 1}}, + .lastUpdateTime = static_cast(epoch_offset.count() - 1), .err = ter(tecINVALID_UPDATE_TIME)}); } @@ -284,6 +302,38 @@ struct Oracle_test : public beast::unit_test::suite .series = {{"USD", "BTC", 740, maxPriceScale + 1}}, .err = ter(temMALFORMED)}); } + + { + // Updating token pair to add and delete + Env env(*this); + env.fund(XRP(1'000), owner); + Oracle oracle(env, {.owner = owner}); + oracle.set(UpdateArg{ + .series = + {{"XRP", "EUR", std::nullopt, std::nullopt}, + {"XRP", "EUR", 740, 1}}, + .err = ter(temMALFORMED)}); + // Delete token pair that doesn't exist in this oracle + oracle.set(UpdateArg{ + .series = {{"XRP", "EUR", std::nullopt, std::nullopt}}, + .err = ter(tecTOKEN_PAIR_NOT_FOUND)}); + // Delete token pair in oracle, which is not in the ledger + oracle.set(UpdateArg{ + .documentID = 10, + .series = {{"XRP", "EUR", std::nullopt, std::nullopt}}, + .err = ter(temMALFORMED)}); + } + + { + // Bad fee + Env env(*this); + env.fund(XRP(1'000), owner); + Oracle oracle( + env, {.owner = owner, .fee = -1, .err = ter(temBAD_FEE)}); + Oracle oracle1(env, {.owner = owner}); + oracle.set( + UpdateArg{.owner = owner, .fee = -1, .err = ter(temBAD_FEE)}); + } } void @@ -356,13 +406,19 @@ struct Oracle_test : public beast::unit_test::suite {.owner = bad, .seq = seq(1), .err = ter(terNO_ACCOUNT)}); } - // Invalid Sequence + // Invalid DocumentID oracle.remove({.documentID = 2, .err = ter(tecNO_ENTRY)}); // Invalid owner Account const invalid("invalid"); env.fund(XRP(1'000), invalid); oracle.remove({.owner = invalid, .err = ter(tecNO_ENTRY)}); + + // Invalid flags + oracle.remove({.flags = tfSellNFToken, .err = ter(temINVALID_FLAG)}); + + // Bad fee + oracle.remove({.fee = -1, .err = ter(temBAD_FEE)}); } void @@ -622,6 +678,39 @@ struct Oracle_test : public beast::unit_test::suite } } + void + testInvalidLedgerEntry() + { + testcase("Invalid Ledger Entry"); + using namespace jtx; + + Env env(*this); + Account const owner("owner"); + env.fund(XRP(1'000), owner); + Oracle oracle(env, {.owner = owner}); + + // Malformed document id + std::vector invalid = {"%None%", -1, 1.2, "", "Invalid"}; + for (auto const& v : invalid) + { + auto const res = Oracle::ledgerEntry(env, owner, v); + BEAST_EXPECT(res[jss::error].asString() == "malformedDocumentID"); + } + // Missing document id + auto res = Oracle::ledgerEntry(env, owner, std::nullopt); + BEAST_EXPECT(res[jss::error].asString() == "malformedRequest"); + + // Missing account + res = Oracle::ledgerEntry(env, std::nullopt, 1); + BEAST_EXPECT(res[jss::error].asString() == "malformedRequest"); + + // Malformed account + std::string malfAccount = to_string(owner.id()); + malfAccount.replace(10, 1, 1, '!'); + res = Oracle::ledgerEntry(env, malfAccount, 1); + BEAST_EXPECT(res[jss::error].asString() == "malformedAddress"); + } + void testLedgerEntry() { @@ -683,6 +772,7 @@ struct Oracle_test : public beast::unit_test::suite all - featureMultiSignReserve - featureExpandedSignerList, all - featureExpandedSignerList}) testMultisig(features); + testInvalidLedgerEntry(); testLedgerEntry(); } }; diff --git a/src/test/jtx/Oracle.h b/src/test/jtx/Oracle.h index f6fdbbff34a..474a003ff87 100644 --- a/src/test/jtx/Oracle.h +++ b/src/test/jtx/Oracle.h @@ -28,6 +28,22 @@ namespace test { namespace jtx { namespace oracle { +using AnyValue = std::variant; +using OraclesData = + std::vector, std::optional>>; + +std::uint32_t +asUInt(AnyValue const& v); + +bool +validDocumentID(AnyValue const& v); + +void +toJson(Json::Value& jv, AnyValue const& v); + +void +toJsonHex(Json::Value& jv, AnyValue const& v); + // base asset, quote asset, price, scale using DataSeries = std::vector owner = std::nullopt; - std::optional documentID = 1; + std::optional documentID = 1; DataSeries series = {{"XRP", "USD", 740, 1}}; - std::optional assetClass = "currency"; - std::optional provider = "provider"; - std::optional uri = "URI"; - std::optional lastUpdateTime = std::nullopt; + std::optional assetClass = "currency"; + std::optional provider = "provider"; + std::optional uri = "URI"; + std::optional lastUpdateTime = std::nullopt; std::uint32_t flags = 0; std::optional msig = std::nullopt; std::optional seq = std::nullopt; - std::uint32_t fee = 10; + int fee = 10; std::optional err = std::nullopt; bool close = false; }; @@ -57,26 +73,27 @@ struct CreateArg struct UpdateArg { std::optional owner = std::nullopt; - std::optional documentID = std::nullopt; + std::optional documentID = std::nullopt; DataSeries series = {}; - std::optional assetClass = std::nullopt; - std::optional provider = std::nullopt; - std::optional uri = "URI"; - std::optional lastUpdateTime = std::nullopt; + std::optional assetClass = std::nullopt; + std::optional provider = std::nullopt; + std::optional uri = "URI"; + std::optional lastUpdateTime = std::nullopt; std::uint32_t flags = 0; std::optional msig = std::nullopt; std::optional seq = std::nullopt; - std::uint32_t fee = 10; + int fee = 10; std::optional err = std::nullopt; }; struct RemoveArg { std::optional const& owner = std::nullopt; - std::optional const& documentID = std::nullopt; + std::optional const& documentID = std::nullopt; + std::uint32_t flags = 0; std::optional const& msig = std::nullopt; std::optional seq = std::nullopt; - std::uint32_t fee = 10; + int fee = 10; std::optional const& err = std::nullopt; }; @@ -123,12 +140,11 @@ class Oracle static Json::Value aggregatePrice( Env& env, - std::optional const& baseAsset, - std::optional const& quoteAsset, - std::optional>> const& - oracles = std::nullopt, - std::optional const& trim = std::nullopt, - std::optional const& timeTreshold = std::nullopt); + std::optional const& baseAsset, + std::optional const& quoteAsset, + std::optional const& oracles = std::nullopt, + std::optional const& trim = std::nullopt, + std::optional const& timeTreshold = std::nullopt); std::uint32_t documentID() const @@ -154,8 +170,8 @@ class Oracle static Json::Value ledgerEntry( Env& env, - AccountID const& account, - std::variant const& documentID, + std::optional> const& account, + std::optional const& documentID, std::optional const& index = std::nullopt); Json::Value diff --git a/src/test/jtx/impl/Oracle.cpp b/src/test/jtx/impl/Oracle.cpp index 95da59952a0..a24540f7326 100644 --- a/src/test/jtx/impl/Oracle.cpp +++ b/src/test/jtx/impl/Oracle.cpp @@ -22,6 +22,7 @@ #include #include +#include #include @@ -43,8 +44,8 @@ Oracle::Oracle(Env& env, CreateArg const& arg, bool submit) env_.close(now + testStartTime - epoch_offset); if (arg.owner) owner_ = *arg.owner; - if (arg.documentID) - documentID_ = *arg.documentID; + if (arg.documentID && validDocumentID(*arg.documentID)) + documentID_ = asUInt(*arg.documentID); if (submit) set(arg); } @@ -55,13 +56,15 @@ Oracle::remove(RemoveArg const& arg) Json::Value jv; jv[jss::TransactionType] = jss::OracleDelete; jv[jss::Account] = to_string(arg.owner.value_or(owner_)); - jv[jss::OracleDocumentID] = arg.documentID.value_or(documentID_); + toJson(jv[jss::OracleDocumentID], arg.documentID.value_or(documentID_)); if (Oracle::fee != 0) jv[jss::Fee] = std::to_string(Oracle::fee); else if (arg.fee != 0) jv[jss::Fee] = std::to_string(arg.fee); else jv[jss::Fee] = std::to_string(env_.current()->fees().increment.drops()); + if (arg.flags != 0) + jv[jss::Flags] = arg.flags; submit(jv, arg.msig, arg.seq, arg.err); } @@ -142,12 +145,11 @@ Oracle::expectLastUpdateTime(std::uint32_t lastUpdateTime) const Json::Value Oracle::aggregatePrice( Env& env, - std::optional const& baseAsset, - std::optional const& quoteAsset, - std::optional>> const& - oracles, - std::optional const& trim, - std::optional const& timeThreshold) + std::optional const& baseAsset, + std::optional const& quoteAsset, + std::optional const& oracles, + std::optional const& trim, + std::optional const& timeThreshold) { Json::Value jv; Json::Value jvOracles(Json::arrayValue); @@ -156,26 +158,34 @@ Oracle::aggregatePrice( for (auto const& id : *oracles) { Json::Value oracle; - oracle[jss::account] = to_string(id.first.id()); - oracle[jss::oracle_document_id] = id.second; + if (id.first) + oracle[jss::account] = to_string((*id.first).id()); + if (id.second) + toJson(oracle[jss::oracle_document_id], *id.second); jvOracles.append(oracle); } jv[jss::oracles] = jvOracles; } if (trim) - jv[jss::trim] = *trim; + toJson(jv[jss::trim], *trim); if (baseAsset) - jv[jss::base_asset] = *baseAsset; + toJson(jv[jss::base_asset], *baseAsset); if (quoteAsset) - jv[jss::quote_asset] = *quoteAsset; + toJson(jv[jss::quote_asset], *quoteAsset); if (timeThreshold) - jv[jss::time_threshold] = *timeThreshold; - - auto jr = env.rpc("json", "get_aggregate_price", to_string(jv)); + toJson(jv[jss::time_threshold], *timeThreshold); + // Convert "%None%" to None + auto str = to_string(jv); + str = boost::regex_replace(str, boost::regex("\"%None%\""), "None"); + auto jr = env.rpc("json", "get_aggregate_price", str); - if (jr.isObject() && jr.isMember(jss::result) && - jr[jss::result].isMember(jss::status)) - return jr[jss::result]; + if (jr.isObject()) + { + if (jr.isMember(jss::result) && jr[jss::result].isMember(jss::status)) + return jr[jss::result]; + else if (jr.isMember(jss::error)) + return jr; + } return Json::nullValue; } @@ -186,17 +196,24 @@ Oracle::set(UpdateArg const& arg) Json::Value jv; if (arg.owner) owner_ = *arg.owner; - if (arg.documentID) - documentID_ = *arg.documentID; + if (arg.documentID && + std::holds_alternative(*arg.documentID)) + { + documentID_ = std::get(*arg.documentID); + jv[jss::OracleDocumentID] = documentID_; + } + else if (arg.documentID) + toJson(jv[jss::OracleDocumentID], *arg.documentID); + else + jv[jss::OracleDocumentID] = documentID_; jv[jss::TransactionType] = jss::OracleSet; jv[jss::Account] = to_string(owner_); - jv[jss::OracleDocumentID] = documentID_; if (arg.assetClass) - jv[jss::AssetClass] = strHex(*arg.assetClass); + toJsonHex(jv[jss::AssetClass], *arg.assetClass); if (arg.provider) - jv[jss::Provider] = strHex(*arg.provider); + toJsonHex(jv[jss::Provider], *arg.provider); if (arg.uri) - jv[jss::URI] = strHex(*arg.uri); + toJsonHex(jv[jss::URI], *arg.uri); if (arg.flags != 0) jv[jss::Flags] = arg.flags; if (Oracle::fee != 0) @@ -207,8 +224,14 @@ Oracle::set(UpdateArg const& arg) jv[jss::Fee] = std::to_string(env_.current()->fees().increment.drops()); // lastUpdateTime if provided is offset from testStartTime if (arg.lastUpdateTime) - jv[jss::LastUpdateTime] = - to_string(testStartTime.count() + *arg.lastUpdateTime); + { + if (std::holds_alternative(*arg.lastUpdateTime)) + jv[jss::LastUpdateTime] = to_string( + testStartTime.count() + + std::get(*arg.lastUpdateTime)); + else + toJson(jv[jss::LastUpdateTime], *arg.lastUpdateTime); + } else jv[jss::LastUpdateTime] = to_string( duration_cast( @@ -263,18 +286,22 @@ Oracle::set(CreateArg const& arg) Json::Value Oracle::ledgerEntry( Env& env, - AccountID const& account, - std::variant const& documentID, + std::optional> const& account, + std::optional const& documentID, std::optional const& index) { Json::Value jvParams; - jvParams[jss::oracle][jss::account] = to_string(account); - if (std::holds_alternative(documentID)) - jvParams[jss::oracle][jss::oracle_document_id] = - std::get(documentID); - else - jvParams[jss::oracle][jss::oracle_document_id] = - std::get(documentID); + if (account) + { + if (std::holds_alternative(*account)) + jvParams[jss::oracle][jss::account] = + to_string(std::get(*account)); + else + jvParams[jss::oracle][jss::account] = + std::get(*account); + } + if (documentID) + toJson(jvParams[jss::oracle][jss::oracle_document_id], *documentID); if (index) { std::uint32_t i; @@ -283,9 +310,61 @@ Oracle::ledgerEntry( else jvParams[jss::oracle][jss::ledger_index] = *index; } + // Convert "%None%" to None + auto str = to_string(jvParams); + str = boost::regex_replace(str, boost::regex("\"%None%\""), "None"); return env.rpc("json", "ledger_entry", to_string(jvParams))[jss::result]; } +void +toJson(Json::Value& jv, AnyValue const& v) +{ + std::visit([&](auto&& arg) { jv = arg; }, v); +} + +void +toJsonHex(Json::Value& jv, AnyValue const& v) +{ + std::visit( + [&](T&& arg) { + if constexpr (std::is_same_v) + { + if (arg.starts_with("##")) + jv = arg.substr(2); + else + jv = strHex(arg); + } + else + jv = arg; + }, + v); +} + +std::uint32_t +asUInt(AnyValue const& v) +{ + Json::Value jv; + toJson(jv, v); + return jv.asUInt(); +} + +bool +validDocumentID(AnyValue const& v) +{ + try + { + Json::Value jv; + toJson(jv, v); + jv.asUInt(); + jv.isNumeric(); + return true; + } + catch (...) + { + } + return false; +} + } // namespace oracle } // namespace jtx } // namespace test diff --git a/src/test/rpc/GetAggregatePrice_test.cpp b/src/test/rpc/GetAggregatePrice_test.cpp index 4c45b7b9d2b..2ec65dac192 100644 --- a/src/test/rpc/GetAggregatePrice_test.cpp +++ b/src/test/rpc/GetAggregatePrice_test.cpp @@ -35,10 +35,9 @@ class GetAggregatePrice_test : public beast::unit_test::suite { testcase("Errors"); using namespace jtx; - using Oracles = std::vector>; Account const owner{"owner"}; Account const some{"some"}; - static Oracles oracles = {{owner, 1}}; + static OraclesData oracles = {{owner, 1}}; { Env env(*this); @@ -55,6 +54,32 @@ class GetAggregatePrice_test : public beast::unit_test::suite ret[jss::error_message].asString() == "Missing field 'quote_asset'."); + // invalid base_asset, quote_asset + std::vector invalidAsset = { + "%None%", + 1, + -1, + 1.2, + "", + "invalid", + "a", + "ab", + "A", + "AB", + "ABCD", + "010101", + "012345678901234567890123456789012345678", + "012345678901234567890123456789012345678G"}; + for (auto const& v : invalidAsset) + { + ret = Oracle::aggregatePrice(env, "USD", v, oracles); + BEAST_EXPECT(ret[jss::error].asString() == "invalidParams"); + ret = Oracle::aggregatePrice(env, v, "USD", oracles); + BEAST_EXPECT(ret[jss::error].asString() == "invalidParams"); + ret = Oracle::aggregatePrice(env, v, v, oracles); + BEAST_EXPECT(ret[jss::error].asString() == "invalidParams"); + } + // missing oracles array ret = Oracle::aggregatePrice(env, "XRP", "USD"); BEAST_EXPECT( @@ -62,16 +87,39 @@ class GetAggregatePrice_test : public beast::unit_test::suite "Missing field 'oracles'."); // empty oracles array - ret = Oracle::aggregatePrice(env, "XRP", "USD", Oracles{}); + ret = Oracle::aggregatePrice(env, "XRP", "USD", OraclesData{}); BEAST_EXPECT(ret[jss::error].asString() == "oracleMalformed"); + // no token pairs found + ret = Oracle::aggregatePrice(env, "YAN", "USD", oracles); + BEAST_EXPECT(ret[jss::error].asString() == "objectNotFound"); + // invalid oracle document id + // id doesn't exist ret = Oracle::aggregatePrice(env, "XRP", "USD", {{{owner, 2}}}); BEAST_EXPECT(ret[jss::error].asString() == "objectNotFound"); + // invalid values + std::vector invalidDocument = { + "%None%", 1.2, -1, "", "none", "1.2"}; + for (auto const& v : invalidDocument) + { + ret = Oracle::aggregatePrice(env, "XRP", "USD", {{{owner, v}}}); + Json::Value jv; + toJson(jv, v); + BEAST_EXPECT(ret[jss::error].asString() == "invalidParams"); + } + // missing document id + ret = Oracle::aggregatePrice( + env, "XRP", "USD", {{{owner, std::nullopt}}}); + BEAST_EXPECT(ret[jss::error].asString() == "oracleMalformed"); // invalid owner ret = Oracle::aggregatePrice(env, "XRP", "USD", {{{some, 1}}}); BEAST_EXPECT(ret[jss::error].asString() == "objectNotFound"); + // missing account + ret = Oracle::aggregatePrice( + env, "XRP", "USD", {{{std::nullopt, 1}}}); + BEAST_EXPECT(ret[jss::error].asString() == "oracleMalformed"); // oracles have wrong asset pair env.fund(XRP(1'000), owner); @@ -82,18 +130,35 @@ class GetAggregatePrice_test : public beast::unit_test::suite BEAST_EXPECT(ret[jss::error].asString() == "objectNotFound"); // invalid trim value - ret = Oracle::aggregatePrice( - env, "XRP", "USD", {{{owner, oracle.documentID()}}}, 0); - BEAST_EXPECT(ret[jss::error].asString() == "invalidParams"); - ret = Oracle::aggregatePrice( - env, "XRP", "USD", {{{owner, oracle.documentID()}}}, 26); - BEAST_EXPECT(ret[jss::error].asString() == "invalidParams"); + std::vector invalidTrim = { + "%None%", 0, 26, -1, 1.2, "", "none", "1.2"}; + for (auto const& v : invalidTrim) + { + ret = Oracle::aggregatePrice( + env, "XRP", "USD", {{{owner, oracle.documentID()}}}, v); + BEAST_EXPECT(ret[jss::error].asString() == "invalidParams"); + } + + // invalid time threshold value + std::vector invalidTime = { + "%None%", -1, 1.2, "", "none", "1.2"}; + for (auto const& v : invalidTime) + { + ret = Oracle::aggregatePrice( + env, + "XRP", + "USD", + {{{owner, oracle.documentID()}}}, + std::nullopt, + v); + BEAST_EXPECT(ret[jss::error].asString() == "invalidParams"); + } } // too many oracles { Env env(*this); - std::vector> oracles; + OraclesData oracles; for (int i = 0; i < 201; ++i) { Account const owner(std::to_string(i)); @@ -132,7 +197,7 @@ class GetAggregatePrice_test : public beast::unit_test::suite // or time threshold { Env env(*this); - std::vector> oracles; + OraclesData oracles; prep(env, oracles); // entire and trimmed stats auto ret = Oracle::aggregatePrice(env, "XRP", "USD", oracles); @@ -148,7 +213,7 @@ class GetAggregatePrice_test : public beast::unit_test::suite // Aggregate data set includes all price oracle instances { Env env(*this); - std::vector> oracles; + OraclesData oracles; prep(env, oracles); // entire and trimmed stats auto ret = @@ -171,14 +236,14 @@ class GetAggregatePrice_test : public beast::unit_test::suite // updated ledgers { Env env(*this); - std::vector> oracles; + OraclesData oracles; prep(env, oracles); for (int i = 0; i < 3; ++i) { Oracle oracle( env, {.owner = oracles[i].first, - .documentID = oracles[i].second}, + .documentID = asUInt(*oracles[i].second)}, false); // push XRP/USD by more than three ledgers, so this price // oracle is not included in the dataset @@ -191,7 +256,7 @@ class GetAggregatePrice_test : public beast::unit_test::suite Oracle oracle( env, {.owner = oracles[i].first, - .documentID = oracles[i].second}, + .documentID = asUInt(*oracles[i].second)}, false); // push XRP/USD by two ledgers, so this price // is included in the dataset @@ -201,7 +266,7 @@ class GetAggregatePrice_test : public beast::unit_test::suite // entire and trimmed stats auto ret = - Oracle::aggregatePrice(env, "XRP", "USD", oracles, 20, 200); + Oracle::aggregatePrice(env, "XRP", "USD", oracles, 20, "200"); BEAST_EXPECT(ret[jss::entire_set][jss::mean] == "74.6"); BEAST_EXPECT(ret[jss::entire_set][jss::size].asUInt() == 7); BEAST_EXPECT( @@ -219,14 +284,14 @@ class GetAggregatePrice_test : public beast::unit_test::suite // Reduced data set because of the time threshold { Env env(*this); - std::vector> oracles; + OraclesData oracles; prep(env, oracles); for (int i = 0; i < oracles.size(); ++i) { Oracle oracle( env, {.owner = oracles[i].first, - .documentID = oracles[i].second}, + .documentID = asUInt(*oracles[i].second)}, false); // push XRP/USD by two ledgers, so this price // is included in the dataset