[FOLD] Address reviewer's feedback

* Replace CompareDescending with std::greater * Replace temBAD_ARRAY_SIZE with temBAD_ARRAY_SIZE_[ZERO,TOO_LARGE]
gregtatcam · Dec 12, 2023 · e73b445 · e73b445
1 parent 7cf3455
commit e73b445
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 22 deletions.
diff --git a/src/ripple/app/tx/impl/SetOracle.cpp b/src/ripple/app/tx/impl/SetOracle.cpp
@@ -48,8 +48,10 @@ SetOracle::preflight(PreflightContext const& ctx)
         return temINVALID_FLAG;
 
     auto const& dataSeries = ctx.tx.getFieldArray(sfPriceDataSeries);
-    if (dataSeries.size() == 0 || dataSeries.size() > maxOracleDataSeries)
-        return temBAD_ARRAY_SIZE;
+    if (dataSeries.empty())
+        return temBAD_ARRAY_SIZE_ZERO;
+    if (dataSeries.size() > maxOracleDataSeries)
+        return temBAD_ARRAY_SIZE_TOO_LARGE;
 
     auto isInvalidLength = [&](auto const& sField, std::size_t length) {
         return ctx.tx.isFieldPresent(sField) &&
@@ -145,8 +147,10 @@ SetOracle::preclaim(PreclaimContext const& ctx)
             return temMALFORMED;
     }
 
-    if (pairs.empty() || pairs.size() > maxOracleDataSeries)
-        return temBAD_ARRAY_SIZE;
+    if (pairs.empty())
+        return temBAD_ARRAY_SIZE_ZERO;
+    if (pairs.size() > maxOracleDataSeries)
+        return temBAD_ARRAY_SIZE_TOO_LARGE;
 
     auto const add = pairs.size() > 5 ? 2 : 1;
     auto const reserve = ctx.view.fees().accountReserve(

diff --git a/src/ripple/protocol/TER.h b/src/ripple/protocol/TER.h
@@ -136,7 +136,8 @@ enum TEMcodes : TERUnderlyingType {
 
     temEMPTY_DID,
 
-    temBAD_ARRAY_SIZE,
+    temBAD_ARRAY_SIZE_ZERO,
+    temBAD_ARRAY_SIZE_TOO_LARGE,
 };
 
 //------------------------------------------------------------------------------

diff --git a/src/ripple/protocol/impl/TER.cpp b/src/ripple/protocol/impl/TER.cpp
@@ -198,7 +198,8 @@ transResults()
         MAKE_ERROR(temXCHAIN_BRIDGE_NONDOOR_OWNER,   "Malformed: Bridge owner must be one of the door accounts."),
         MAKE_ERROR(temXCHAIN_BRIDGE_BAD_MIN_ACCOUNT_CREATE_AMOUNT,   "Malformed: Bad min account create amount."),
         MAKE_ERROR(temXCHAIN_BRIDGE_BAD_REWARD_AMOUNT, "Malformed: Bad reward amount."),
-        MAKE_ERROR(temBAD_ARRAY_SIZE,            "Malformed: Invalid array size."),
+        MAKE_ERROR(temBAD_ARRAY_SIZE_ZERO,       "Malformed: Array size is zero."),
+        MAKE_ERROR(temBAD_ARRAY_SIZE_TOO_LARGE,  "Malformed: Array size is tool large."),
 
         MAKE_ERROR(terRETRY,                  "Retry transaction."),
         MAKE_ERROR(terFUNDS_SPENT,            "DEPRECATED."),

diff --git a/src/ripple/rpc/handlers/GetAggregatePrice.cpp b/src/ripple/rpc/handlers/GetAggregatePrice.cpp
@@ -31,22 +31,14 @@
 
 namespace ripple {
 
-struct CompareDescending
-{
-    bool
-    operator()(std::uint32_t const& a, std::uint32_t const& b) const
-    {
-        return a > b;  // Sort lastUpdateTime in descending order
-    }
-};
-
 using namespace boost::bimaps;
 // sorted descending by lastUpdateTime, ascending by AssetPrice
-using Prices =
-    bimap<multiset_of<std::uint32_t, CompareDescending>, multiset_of<STAmount>>;
+using Prices = bimap<
+    multiset_of<std::uint32_t, std::greater<std::uint32_t>>,
+    multiset_of<STAmount>>;
 
 /** Calls callback "f" on the ledger-object sle and up to three previous
- * metadata objects. Returns if the callback returns true.
+ * metadata objects. Stops early if the callback returns true.
  */
 static void
 iteratePriceData(

diff --git a/src/test/app/Oracle_test.cpp b/src/test/app/Oracle_test.cpp
@@ -115,8 +115,9 @@ struct Oracle_test : public beast::unit_test::suite
                      {"XRP", "US9", 740, 1},
                      {"XRP", "U10", 750, 1},
                      {"XRP", "U11", 740, 1}},
-                .ter = ter(temBAD_ARRAY_SIZE)});
-            oracle.set(CreateArg{.series = {}, .ter = ter(temBAD_ARRAY_SIZE)});
+                .ter = ter(temBAD_ARRAY_SIZE_TOO_LARGE)});
+            oracle.set(
+                CreateArg{.series = {}, .ter = ter(temBAD_ARRAY_SIZE_ZERO)});
         }
 
         // Array of token pair exceeds 10 after update
@@ -142,7 +143,7 @@ struct Oracle_test : public beast::unit_test::suite
                         {"XRP", "US9", 740, 1},
                         {"XRP", "U10", 750, 1},
                     },
-                .ter = ter(temBAD_ARRAY_SIZE)});
+                .ter = ter(temBAD_ARRAY_SIZE_TOO_LARGE)});
         }
 
         {
@@ -245,7 +246,7 @@ struct Oracle_test : public beast::unit_test::suite
             // delete all token pairs
             oracle.set(UpdateArg{
                 .series = {{"XRP", "USD", std::nullopt, std::nullopt}},
-                .ter = ter(temBAD_ARRAY_SIZE)});
+                .ter = ter(temBAD_ARRAY_SIZE_ZERO)});
         }
     }