[FOLD] Check if price included in set, other minor refactoring

gregtatcam · Oct 24, 2023 · f52896b · f52896b
1 parent c46ae2e
commit f52896b
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 11 deletions.
diff --git a/src/ripple/app/tx/impl/DeleteOracle.cpp b/src/ripple/app/tx/impl/DeleteOracle.cpp
@@ -56,8 +56,9 @@ DeleteOracle::preclaim(PreclaimContext const& ctx)
     }
     else if (ctx.tx.getAccountID(sfAccount) != sle->getAccountID(sfOwner))
     {
+        // this can't happen because of the above check
         JLOG(ctx.j.debug()) << "Oracle Delete: invalid account.";
-        return tecNO_PERMISSION;
+        return tecINTERNAL;
     }
     return tesSUCCESS;
 }

diff --git a/src/ripple/app/tx/impl/SetOracle.cpp b/src/ripple/app/tx/impl/SetOracle.cpp
@@ -87,13 +87,15 @@ SetOracle::preclaim(PreclaimContext const& ctx)
             entry.getFieldCurrency(sfPriceUnit).currency());
         if (pairs.contains(hash))
             return tecDUPLICATE;
+        if (!entry.isFieldPresent(sfSymbolPrice))
+            return temMALFORMED;
         pairs.emplace(hash);
     }
 
-    // update
     if (auto const sle = ctx.view.read(keylet::oracle(
             ctx.tx.getAccountID(sfAccount), ctx.tx[sfOracleSequence])))
     {
+        // update
         if (ctx.tx[sfAccount] != sle->getAccountID(sfOwner))
             return tecNO_PERMISSION;
 
@@ -110,9 +112,9 @@ SetOracle::preclaim(PreclaimContext const& ctx)
                 pairs.emplace(hash);
         }
     }
-    // create
     else
     {
+        // create
         if (!ctx.tx.isFieldPresent(sfProvider) ||
             !ctx.tx.isFieldPresent(sfSymbolClass))
             return temMALFORMED;
@@ -148,9 +150,9 @@ applySet(
 {
     auto const oracleID = keylet::oracle(account_, ctx_.tx[sfOracleSequence]);
 
-    // update
     if (auto sle = sb.peek(oracleID))
     {
+        // update
         hash_map<uint256, STObject> pairs;
         // collect current pairs
         for (auto const& entry : sle->getFieldArray(sfPriceDataSeries))
@@ -174,13 +176,15 @@ applySet(
                 entry.getFieldCurrency(sfPriceUnit).currency());
             if (auto iter = pairs.find(hash); iter != pairs.end())
             {
+                // update the price
                 iter->second.setFieldU64(
                     sfSymbolPrice, entry.getFieldU64(sfSymbolPrice));
                 if (entry.isFieldPresent(sfScale))
                     iter->second.setFieldU8(sfScale, entry.getFieldU8(sfScale));
             }
             else
             {
+                // add a token pair with the price
                 STObject priceData{sfPriceData};
                 priceData.setFieldCurrency(
                     sfSymbol, entry.getFieldCurrency(sfSymbol));
@@ -207,9 +211,9 @@ applySet(
 
         sb.update(sle);
     }
-    // create
     else
     {
+        // create
         sle = std::make_shared<SLE>(oracleID);
         sle->setAccountID(sfOwner, ctx_.tx.getAccountID(sfAccount));
         sle->setFieldVL(sfProvider, ctx_.tx[sfProvider]);

diff --git a/src/test/app/Oracle_test.cpp b/src/test/app/Oracle_test.cpp
@@ -94,6 +94,20 @@ struct Oracle_test : public beast::unit_test::suite
                 0,
                 ter(tecDUPLICATE));
 
+            // Price is not included
+            oracle.create(
+                owner,
+                {{"XRP", "USD", 740, 1}, {"XRP", "USD", std::nullopt, 1}},
+                std::nullopt,
+                "currency",
+                "provider",
+                "URI",
+                std::nullopt,
+                0,
+                std::nullopt,
+                0,
+                ter(temMALFORMED));
+
             // Array of token pair is 0 or exceeds 10
             oracle.create(
                 owner,

diff --git a/src/test/jtx/Oracle.h b/src/test/jtx/Oracle.h
@@ -29,8 +29,11 @@ namespace jtx {
 class Oracle
 {
     // symbol, price unit, price, scale
-    using DataSeries = std::vector<
-        std::tuple<std::string, std::string, std::uint32_t, std::uint8_t>>;
+    using DataSeries = std::vector<std::tuple<
+        std::string,
+        std::string,
+        std::optional<std::uint32_t>,
+        std::optional<std::uint8_t>>>;
 
 private:
     static inline std::uint32_t id_ = 0;

diff --git a/src/test/jtx/impl/Oracle.cpp b/src/test/jtx/impl/Oracle.cpp
@@ -331,10 +331,10 @@ Oracle::set(
         Json::Value price;
         price[jss::Symbol] = std::get<0>(data);
         price[jss::PriceUnit] = std::get<1>(data);
-        // std::stringstream str;
-        // str << std::hex << std::get<3>(data);
-        price[jss::SymbolPrice] = std::get<2>(data);
-        price[jss::Scale] = std::get<3>(data);
+        if (std::get<2>(data))
+            price[jss::SymbolPrice] = *std::get<2>(data);
+        if (std::get<3>(data))
+            price[jss::Scale] = *std::get<3>(data);
         priceData[jss::PriceData] = price;
         dataSeries.append(priceData);
     }