Merge branch 'develop' into ximinez/parse-guard

- Rename oldExpected to oldExpected_.
- Prevent copying, and moving.
- Make getParseFailureGuard [[nodiscard]].

include/xrpl/basics/TaggedCache.ipp

@@ -2,9 +2,6 @@
 
 #include <xrpl/basics/IntrusivePointer.ipp>
 #include <xrpl/basics/TaggedCache.h>
-#include <xrpl/basics/scope.h>
-
-#include <algorithm>
 
 namespace xrpl {
 
@@ -598,39 +595,8 @@ TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash,
     std::vector<key_type> v;
 
     {
-        // Keep track of how many iterations are needed. Exit the loop if the number of retries gets
-        // absurd. (Note that if this somehow ever happens, one more allocation will be done under
-        // lock, which is undesirable, but really should be almost impossible.)
-        std::size_t allocationIterations = 0;
-        std::unique_lock lock(mutex_);
-        for (auto size = cache_.size(); v.capacity() < size && allocationIterations < 20;
-             size = cache_.size())
-        {
-            ScopeUnlock const unlock(lock);
-            // Allocate the current size plus a little extra, in case the cache grows while
-            // allocating. Each time another allocation is needed, the extra also gets bigger until
-            // it ultimately doubles the size + 1.
-            size += (size >> (4 - std::min(allocationIterations, std::size_t{4}))) + 1;
-            v.reserve(size);
-            ++allocationIterations;
-        }
-        // In a normal operating environment, because of the padding added to size before
-        // allocating, even 2 iterations is going to be very rare. If 3 or more are ever needed,
-        // that's unusual enough that I want to know about it. Don't ask me to change it without
-        // empirical data. - Ed H.
-        XRPL_ASSERT(
-            allocationIterations < 3,
-            "xrpl::TaggedCache::getKeys(): limited allocation iterations");
-        if (v.capacity() < cache_.size())
-        {
-            // LCOV_EXCL_START
-            UNREACHABLE("xrpl::TaggedCache::getKeys(): failed to allocate sufficient capacity");
-            v.reserve(cache_.size());
-            // LCOV_EXCL_STOP
-        }
-        XRPL_ASSERT(lock.owns_lock(), "xrpl::TaggedCache::getKeys(): owns lock");
-        XRPL_ASSERT(
-            v.capacity() >= cache_.size(), "xrpl::TaggedCache::getKeys(): sufficient capacity");
+        std::scoped_lock const lock(mutex_);
+        v.reserve(cache_.size());
         for (auto const& _ : cache_)
             v.push_back(_.first);
     }

src/test/app/Batch_test.cpp

@@ -498,7 +498,7 @@ class Batch_test : public beast::unit_test::Suite
             auto const batchFee = batch::calcBatchFee(env, 0, 2);
             auto tx1 = batch::Inner(pay(alice, bob, XRP(1)), seq + 1);
             tx1[jss::Fee] = "1.5";
-            env.setParseFailureExpected(true);
+            auto const g = env.getParseFailureGuard(true);
             try
             {
                 env(batch::outer(alice, seq, batchFee, tfAllOrNothing),
@@ -510,7 +510,6 @@ class Batch_test : public beast::unit_test::Suite
             {
                 BEAST_EXPECT(true);
             }
-            env.setParseFailureExpected(false);
         }
 
         // temSEQ_AND_TICKET: Batch: inner txn cannot have both Sequence

src/test/app/Vault_test.cpp

@@ -5451,9 +5451,9 @@ class Vault_test : public beast::unit_test::Suite
             env.close();
 
             // 2. Mantissa larger than uint64 max
-            env.setParseFailureExpected(true);
             try
             {
+                auto const g = env.getParseFailureGuard(true);
                 tx[sfAssetsMaximum] = "18446744073709551617e5";  // uint64 max + 1
                 env(tx);
                 BEAST_EXPECTS(false, "Expected parse_error for mantissa larger than uint64 max");
@@ -5464,7 +5464,6 @@ class Vault_test : public beast::unit_test::Suite
                 BEAST_EXPECT(
                     e.what() == "invalidParamsField 'tx_json.AssetsMaximum' has invalid data."s);
             }
-            env.setParseFailureExpected(false);
         }
     }
 

src/test/jtx/Env.h

@@ -482,6 +482,46 @@ public:
         parseFailureExpected_ = b;
     }
 
+    /** RAII class to set and restore the parse failure flag (setParseFailureExpected).
+     *
+     * Can be created directly, or through the `getParseFailureGuard(bool)` function.
+     */
+    class ParseFailureGuard final
+    {
+        Env& self_;
+        bool oldExpected_ = self_.parseFailureExpected_;
+
+    public:
+        ParseFailureGuard(Env& self, bool b) : self_(self)
+        {
+            self_.setParseFailureExpected(b);
+        }
+
+        ~ParseFailureGuard()
+        {
+            self_.setParseFailureExpected(oldExpected_);
+        }
+
+        // No copy, no move
+        ParseFailureGuard(ParseFailureGuard const&) = delete;
+        ParseFailureGuard&
+        operator=(ParseFailureGuard const&) = delete;
+        ParseFailureGuard(ParseFailureGuard&& other) = delete;
+        ParseFailureGuard&
+        operator=(ParseFailureGuard&&) = delete;
+    };
+
+    /** Gets an RAII guard to set and restore the parse failure flag
+     *
+     * Usage:
+     * auto const guard = env.getParseFailureGuard(true/false);
+     */
+    [[nodiscard]] ParseFailureGuard
+    getParseFailureGuard(bool b)
+    {
+        return ParseFailureGuard{*this, b};
+    }
+
     /** Turn off signature checks. */
     void
     disableSigs()

src/test/jtx/impl/Env.cpp

@@ -602,7 +602,7 @@ Env::autofill(JTx& jt)
     catch (ParseError const&)
     {
         if (!parseFailureExpected_)
-            test.log << "parse failed:\n" << pretty(jv) << std::endl;
+            test.log << "parse failure:\n" << pretty(jv) << std::endl;
         rethrow();
     }
 }