fix: Ensures canonical order for PriceDataSeries upon PriceOracle creation (#5485)

This change fixes an issue where the order of `PriceDataSeries` was out of sync between when `PriceOracle` was created and when it was updated. Although they are registered in the canonical order when updated, they are created using the order specified in the transaction; this change ensures that they are also registered in the canonical order when created.

include/xrpl/protocol/detail/features.macro

@@ -32,6 +32,7 @@
 // If you add an amendment here, then do not forget to increment `numFeatures`
 // in include/xrpl/protocol/Feature.h.
 
+XRPL_FIX    (PriceOracleOrder,           Supported::no,  VoteBehavior::DefaultNo)
 XRPL_FIX    (MPTDeliveredAmount,         Supported::no,  VoteBehavior::DefaultNo)
 XRPL_FIX    (AMMClawbackRounding,        Supported::no,  VoteBehavior::DefaultNo)
 XRPL_FEATURE(TokenEscrow,                Supported::yes, VoteBehavior::DefaultNo)
@@ -39,7 +40,7 @@ XRPL_FIX    (EnforceNFTokenTrustlineV2,  Supported::yes, VoteBehavior::DefaultNo
 XRPL_FIX    (AMMv1_3,                    Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(PermissionedDEX,            Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(Batch,                      Supported::yes, VoteBehavior::DefaultNo)
-XRPL_FEATURE(SingleAssetVault,           Supported::no, VoteBehavior::DefaultNo)
+XRPL_FEATURE(SingleAssetVault,           Supported::no,  VoteBehavior::DefaultNo)
 XRPL_FEATURE(PermissionDelegation,       Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FIX    (PayChanCancelAfter,         Supported::yes, VoteBehavior::DefaultNo)
 // Check flags in Credential transactions

src/test/app/Oracle_test.cpp

@@ -678,6 +678,61 @@ private:
             oracle.set(
                 UpdateArg{.series = {{"XRP", "USD", 742, 2}}, .fee = baseFee});
         }
+
+        for (bool const withFixOrder : {false, true})
+        {
+            // Should be same order as creation
+            Env env(
+                *this,
+                withFixOrder ? testable_amendments()
+                             : testable_amendments() - fixPriceOracleOrder);
+            auto const baseFee =
+                static_cast<int>(env.current()->fees().base.drops());
+
+            auto test = [&](Env& env, DataSeries const& series) {
+                env.fund(XRP(1'000), owner);
+                Oracle oracle(
+                    env, {.owner = owner, .series = series, .fee = baseFee});
+                BEAST_EXPECT(oracle.exists());
+                auto sle = env.le(keylet::oracle(owner, oracle.documentID()));
+                BEAST_EXPECT(
+                    sle->getFieldArray(sfPriceDataSeries).size() ==
+                    series.size());
+
+                auto const beforeQuoteAssetName1 =
+                    sle->getFieldArray(sfPriceDataSeries)[0]
+                        .getFieldCurrency(sfQuoteAsset)
+                        .getText();
+                auto const beforeQuoteAssetName2 =
+                    sle->getFieldArray(sfPriceDataSeries)[1]
+                        .getFieldCurrency(sfQuoteAsset)
+                        .getText();
+
+                oracle.set(UpdateArg{.series = series, .fee = baseFee});
+                sle = env.le(keylet::oracle(owner, oracle.documentID()));
+
+                auto const afterQuoteAssetName1 =
+                    sle->getFieldArray(sfPriceDataSeries)[0]
+                        .getFieldCurrency(sfQuoteAsset)
+                        .getText();
+                auto const afterQuoteAssetName2 =
+                    sle->getFieldArray(sfPriceDataSeries)[1]
+                        .getFieldCurrency(sfQuoteAsset)
+                        .getText();
+
+                if (env.current()->rules().enabled(fixPriceOracleOrder))
+                {
+                    BEAST_EXPECT(afterQuoteAssetName1 == beforeQuoteAssetName1);
+                    BEAST_EXPECT(afterQuoteAssetName2 == beforeQuoteAssetName2);
+                }
+                else
+                {
+                    BEAST_EXPECT(afterQuoteAssetName1 != beforeQuoteAssetName1);
+                    BEAST_EXPECT(afterQuoteAssetName2 != beforeQuoteAssetName2);
+                }
+            };
+            test(env, {{"XRP", "USD", 742, 2}, {"XRP", "EUR", 711, 2}});
+        }
     }
 
     void

src/xrpld/app/tx/detail/SetOracle.cpp

@@ -209,6 +209,17 @@ SetOracle::doApply()
 {
     auto const oracleID = keylet::oracle(account_, ctx_.tx[sfOracleDocumentID]);
 
+    auto populatePriceData = [](STObject& priceData, STObject const& entry) {
+        setPriceDataInnerObjTemplate(priceData);
+        priceData.setFieldCurrency(
+            sfBaseAsset, entry.getFieldCurrency(sfBaseAsset));
+        priceData.setFieldCurrency(
+            sfQuoteAsset, entry.getFieldCurrency(sfQuoteAsset));
+        priceData.setFieldU64(sfAssetPrice, entry.getFieldU64(sfAssetPrice));
+        if (entry.isFieldPresent(sfScale))
+            priceData.setFieldU8(sfScale, entry.getFieldU8(sfScale));
+    };
+
     if (auto sle = ctx_.view().peek(oracleID))
     {
         // update
@@ -249,15 +260,7 @@ SetOracle::doApply()
             {
                 // add a token pair with the price
                 STObject priceData{sfPriceData};
-                setPriceDataInnerObjTemplate(priceData);
+                populatePriceData(priceData, entry);
-                priceData.setFieldCurrency(
-                    sfBaseAsset, entry.getFieldCurrency(sfBaseAsset));
-                priceData.setFieldCurrency(
-                    sfQuoteAsset, entry.getFieldCurrency(sfQuoteAsset));
-                priceData.setFieldU64(
-                    sfAssetPrice, entry.getFieldU64(sfAssetPrice));
-                if (entry.isFieldPresent(sfScale))
-                    priceData.setFieldU8(sfScale, entry.getFieldU8(sfScale));
                 pairs.emplace(key, std::move(priceData));
             }
         }
@@ -285,7 +288,26 @@ SetOracle::doApply()
         sle->setFieldVL(sfProvider, ctx_.tx[sfProvider]);
         if (ctx_.tx.isFieldPresent(sfURI))
             sle->setFieldVL(sfURI, ctx_.tx[sfURI]);
-        auto const& series = ctx_.tx.getFieldArray(sfPriceDataSeries);
+
+        STArray series;
+        if (!ctx_.view().rules().enabled(fixPriceOracleOrder))
+        {
+            series = ctx_.tx.getFieldArray(sfPriceDataSeries);
+        }
+        else
+        {
+            std::map<std::pair<Currency, Currency>, STObject> pairs;
+            for (auto const& entry : ctx_.tx.getFieldArray(sfPriceDataSeries))
+            {
+                auto const key = tokenPairKey(entry);
+                STObject priceData{sfPriceData};
+                populatePriceData(priceData, entry);
+                pairs.emplace(key, std::move(priceData));
+            }
+            for (auto const& iter : pairs)
+                series.push_back(std::move(iter.second));
+        }
+
         sle->setFieldArray(sfPriceDataSeries, series);
         sle->setFieldVL(sfAssetClass, ctx_.tx[sfAssetClass]);
         sle->setFieldU32(sfLastUpdateTime, ctx_.tx[sfLastUpdateTime]);