diff --git a/API-CHANGELOG.md b/API-CHANGELOG.md index a04f265328..79fb8ff522 100644 --- a/API-CHANGELOG.md +++ b/API-CHANGELOG.md @@ -42,6 +42,7 @@ This section contains changes targeting a future version. ### Bugfixes +- `get_aggregate_price`: Duplicate entries in the `oracles` request array are now ignored. [#6586](https://github.com/XRPLF/rippled/pull/6586) - Peer Crawler: The `port` field in `overlay.active[]` now consistently returns an integer instead of a string for outbound peers. [#6318](https://github.com/XRPLF/rippled/pull/6318) - `ping`: The `ip` field is no longer returned as an empty string for proxied connections without a forwarded-for header. It is now omitted, consistent with the behavior for identified connections. [#6730](https://github.com/XRPLF/rippled/pull/6730) - gRPC `GetLedgerDiff`: Fixed error message that incorrectly said "base ledger not validated" when the desired ledger was not validated. [#6730](https://github.com/XRPLF/rippled/pull/6730) diff --git a/src/libxrpl/tx/invariants/VaultInvariant.cpp b/src/libxrpl/tx/invariants/VaultInvariant.cpp index eca50eb809..c577fdf356 100644 --- a/src/libxrpl/tx/invariants/VaultInvariant.cpp +++ b/src/libxrpl/tx/invariants/VaultInvariant.cpp @@ -465,7 +465,7 @@ ValidVault::finalize( if (afterVault.assetsAvailable < kZero) { - JLOG(j.fatal()) << "Invariant failed: assets available must be positive"; + JLOG(j.fatal()) << "Invariant failed: assets available must not be negative"; result = false; } @@ -491,13 +491,13 @@ ValidVault::finalize( if (afterVault.assetsTotal < kZero) { - JLOG(j.fatal()) << "Invariant failed: assets outstanding must be positive"; + JLOG(j.fatal()) << "Invariant failed: assets outstanding must not be negative"; result = false; } if (afterVault.assetsMaximum < kZero) { - JLOG(j.fatal()) << "Invariant failed: assets maximum must be positive"; + JLOG(j.fatal()) << "Invariant failed: assets maximum must not be negative"; result = false; } diff --git a/src/libxrpl/tx/transactors/lending/LoanPay.cpp b/src/libxrpl/tx/transactors/lending/LoanPay.cpp index 74e8efeda2..4619540295 100644 --- a/src/libxrpl/tx/transactors/lending/LoanPay.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanPay.cpp @@ -813,7 +813,7 @@ LoanPay::doApply() XRPL_ASSERT_PARTS( vaultBalanceAfter >= beast::kZero && brokerBalanceAfter >= beast::kZero, "xrpl::LoanPay::doApply", - "positive vault and broker balances"); + "non-negative vault and broker balances"); XRPL_ASSERT_PARTS( vaultBalanceAfter >= vaultBalanceBefore, "xrpl::LoanPay::doApply", diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 9bc9524f80..6234d81762 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -3260,9 +3260,9 @@ class Invariants_test : public beast::unit_test::Suite "set must not change assets available", "set must not change shares outstanding", "set must not change vault balance", - "assets available must be positive", + "assets available must not be negative", "assets available must not be greater than assets outstanding", - "assets outstanding must be positive"}, + "assets outstanding must not be negative"}, [&](Account const& a1, Account const& a2, ApplyContext& ac) { auto const keylet = keylet::vault(a1.id(), ac.view().seq()); auto sleVault = ac.view().peek(keylet); @@ -3424,7 +3424,7 @@ class Invariants_test : public beast::unit_test::Suite TxAccount::A2); doInvariantCheck( - {"assets maximum must be positive"}, + {"assets maximum must not be negative"}, [&](Account const& a1, Account const& a2, ApplyContext& ac) { auto const keylet = keylet::vault(a1.id(), ac.view().seq()); return kAdjust(ac.view(), keylet, kArgs(a2.id(), 0, [&](Adjustments& sample) { @@ -3612,7 +3612,7 @@ class Invariants_test : public beast::unit_test::Suite doInvariantCheck( { - "assets maximum must be positive", + "assets maximum must not be negative", "create operation must not have updated a vault", }, [&](Account const& a1, Account const& a2, ApplyContext& ac) { diff --git a/src/test/rpc/GetAggregatePrice_test.cpp b/src/test/rpc/GetAggregatePrice_test.cpp index 3e0bfa1fd3..58c2e8b996 100644 --- a/src/test/rpc/GetAggregatePrice_test.cpp +++ b/src/test/rpc/GetAggregatePrice_test.cpp @@ -320,6 +320,48 @@ public: BEAST_EXPECT(ret[jss::median] == "74"); BEAST_EXPECT(ret[jss::time] == 946695000); } + + // Duplicate oracle entries should be deduplicated. + // Two separate oracles with different prices give size=2. + // Listing the first oracle twice in the query must not + // inflate the size to 3. + { + Env env(*this); + auto const baseFee = static_cast(env.current()->fees().base.drops()); + + Account const owner1{"owner1"}; + Account const owner2{"owner2"}; + env.fund(XRP(1'000), owner1); + env.fund(XRP(1'000), owner2); + Oracle const oracle1( + env, {.owner = owner1, .series = {{"XRP", "USD", 740, 1}}, .fee = baseFee}); + Oracle const oracle2( + env, {.owner = owner2, .series = {{"XRP", "USD", 840, 1}}, .fee = baseFee}); + + // Query with both oracles listed once + OraclesData const single = { + {owner1, oracle1.documentID()}, {owner2, oracle2.documentID()}}; + auto const retSingle = Oracle::aggregatePrice(env, "XRP", "USD", single); + + // Query with oracle1 listed twice + OraclesData const duplicated = { + {owner1, oracle1.documentID()}, + {owner1, oracle1.documentID()}, + {owner2, oracle2.documentID()}}; + auto const retDup = Oracle::aggregatePrice(env, "XRP", "USD", duplicated); + + // Results should be identical - duplicates must not be + // double-counted + BEAST_EXPECT( + retSingle[jss::entire_set][jss::size] == retDup[jss::entire_set][jss::size]); + BEAST_EXPECT(retDup[jss::entire_set][jss::size].asUInt() == 2); + BEAST_EXPECT( + retSingle[jss::entire_set][jss::mean] == retDup[jss::entire_set][jss::mean]); + BEAST_EXPECT( + retSingle[jss::entire_set][jss::standard_deviation] == + retDup[jss::entire_set][jss::standard_deviation]); + BEAST_EXPECT(retSingle[jss::median] == retDup[jss::median]); + } } void diff --git a/src/xrpld/rpc/handlers/orderbook/GetAggregatePrice.cpp b/src/xrpld/rpc/handlers/orderbook/GetAggregatePrice.cpp index f493000d0b..33eaa9ce1e 100644 --- a/src/xrpld/rpc/handlers/orderbook/GetAggregatePrice.cpp +++ b/src/xrpld/rpc/handlers/orderbook/GetAggregatePrice.cpp @@ -33,7 +33,9 @@ #include #include #include +#include #include +#include #include namespace xrpl { @@ -251,6 +253,8 @@ doGetAggregatePrice(rpc::JsonContext& context) // Collect the dataset into bimap keyed by lastUpdateTime and // STAmount (Number is int64 and price is uint64) Prices prices; + // Track seen {account, documentID} pairs to skip duplicates + std::set> seen; for (auto const& oracle : params[jss::oracles]) { if (!oracle.isMember(jss::oracle_document_id) || !oracle.isMember(jss::account)) @@ -268,6 +272,10 @@ doGetAggregatePrice(rpc::JsonContext& context) return result; } + // Skip duplicate oracle entries + if (!seen.emplace(*account, *documentID).second) + continue; + auto const sle = ledger->read(keylet::oracle(*account, *documentID)); iteratePriceData(context, sle, [&](STObject const& node) { auto const& series = node.getFieldArray(sfPriceDataSeries);