From f52896b3308fb744d9989c2be224454ccdff1445 Mon Sep 17 00:00:00 2001
From: Gregory Tsipenyuk <gtsipenyuk@ripple.com>
Date: Tue, 24 Oct 2023 18:14:09 -0400
Subject: [PATCH] [FOLD] Check if price included in set, other minor
 refactoring

---
 src/ripple/app/tx/impl/DeleteOracle.cpp |  3 ++-
 src/ripple/app/tx/impl/SetOracle.cpp    | 12 ++++++++----
 src/test/app/Oracle_test.cpp            | 14 ++++++++++++++
 src/test/jtx/Oracle.h                   |  7 +++++--
 src/test/jtx/impl/Oracle.cpp            |  8 ++++----
 5 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/src/ripple/app/tx/impl/DeleteOracle.cpp b/src/ripple/app/tx/impl/DeleteOracle.cpp
index 3464ec274b2..5784c3418b1 100644
--- 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
index 81456136675..51ae90f999e 100644
--- 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,6 +176,7 @@ 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))
@@ -181,6 +184,7 @@ applySet(
             }
             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
index 725f1d19eca..f36dd3ac53f 100644
--- 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
index 62a7548634f..f3e5fd21487 100644
--- 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
index 5e313f99366..c0c2b70e79e 100644
--- 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);
     }