test: Unit tests to recreate invalid index logic error (#5242)

* One hits the global cache, one does not. * Also some extra checking. Co-authored-by: Bronek Kozicki <[email protected]>
XRPLF · Jan 10, 2025 · aef823e · aef823e
1 parent ccc0889
commit aef823e
Show file tree

Hide file tree

Showing 2 changed files with 104 additions and 0 deletions.
diff --git a/src/test/app/Regression_test.cpp b/src/test/app/Regression_test.cpp
@@ -16,8 +16,11 @@
 //==============================================================================
 
 #include <test/jtx.h>
+#include <test/jtx/check.h>
 #include <test/jtx/envconfig.h>
+#include <xrpld/app/ledger/LedgerMaster.h>
 #include <xrpld/app/tx/apply.h>
+#include <xrpl/basics/CountedObject.h>
 #include <xrpl/basics/StringUtilities.h>
 #include <xrpl/json/json_reader.h>
 #include <xrpl/protocol/Feature.h>
@@ -247,6 +250,100 @@ struct Regression_test : public beast::unit_test::suite
             jrReader.parse(jvRequest, buffers) && jvRequest.isObject());
     }
 
+    void
+    testInvalidTxObjectIDType()
+    {
+        testcase("Invalid Transaction Object ID Type");
+        // Crasher bug introduced in 2.0.1. Fixed in 2.3.0.
+
+        using namespace jtx;
+        Env env(*this);
+
+        Account alice("alice");
+        Account bob("bob");
+        env.fund(XRP(10'000), alice, bob);
+        env.close();
+
+        auto index = [&](auto account) {
+            Json::Value req(Json::objectValue);
+            req[jss::account] = account;
+            auto const resp = env.rpc("json", "account_info", to_string(req));
+            BEAST_EXPECT(
+                resp.isMember(jss::result) && resp[jss::result].isObject());
+            auto const& result = resp[jss::result];
+            BEAST_EXPECT(
+                result.isMember(jss::account_data) &&
+                result[jss::account_data].isObject());
+            auto const& ad = result[jss::account_data];
+            BEAST_EXPECT(ad.isMember(jss::index) && ad[jss::index].isString());
+            uint256 index;
+            BEAST_EXPECT(index.parseHex(ad[jss::index].asString()));
+            return index;
+        };
+
+        {
+            auto const alice_index = index(alice.human());
+            if (BEAST_EXPECT(alice_index.isNonZero()))
+            {
+                env(check::cash(
+                        alice, alice_index, check::DeliverMin(XRP(100))),
+                    ter(tecNO_ENTRY));
+            }
+        }
+
+        {
+            auto const bob_index = index(bob.human());
+
+            auto const digest = [&]() -> std::optional<uint256> {
+                auto const& state =
+                    env.app().getLedgerMaster().getClosedLedger()->stateMap();
+                SHAMapHash digest;
+                if (!state.peekItem(bob_index, digest))
+                    return std::nullopt;
+                return digest.as_uint256();
+            }();
+
+            auto const mapCounts = [&](CountedObjects::List const& list) {
+                std::map<std::string, int> result;
+                for (auto const& e : list)
+                {
+                    result[e.first] = e.second;
+                }
+
+                return result;
+            };
+
+            if (BEAST_EXPECT(bob_index.isNonZero()))
+            {
+                if (BEAST_EXPECT(digest.has_value()))
+                {
+                    auto& cache = env.app().cachedSLEs();
+                    cache.del(*digest, false);
+                    auto const beforeCounts =
+                        mapCounts(CountedObjects::getInstance().getCounts(0));
+
+                    env(check::cash(
+                            alice, bob_index, check::DeliverMin(XRP(100))),
+                        ter(tecNO_ENTRY));
+
+                    auto const afterCounts =
+                        mapCounts(CountedObjects::getInstance().getCounts(0));
+
+                    using namespace std::string_literals;
+                    BEAST_EXPECT(
+                        beforeCounts.at("CachedView::hit"s) ==
+                        afterCounts.at("CachedView::hit"s));
+                    BEAST_EXPECT(
+                        beforeCounts.at("CachedView::hitExpired"s) + 1 ==
+                        afterCounts.at("CachedView::hitExpired"s));
+                    BEAST_EXPECT(
+                        beforeCounts.at("CachedView::miss"s) ==
+                        afterCounts.at("CachedView::miss"s));
+                }
+            }
+        }
+    }
+
     void
     run() override
     {
@@ -256,6 +353,7 @@ struct Regression_test : public beast::unit_test::suite
         testFeeEscalationAutofill();
         testFeeEscalationExtremeConfig();
         testJsonInvalid();
+        testInvalidTxObjectIDType();
     }
 };
 

diff --git a/src/xrpld/ledger/detail/CachedView.cpp b/src/xrpld/ledger/detail/CachedView.cpp
@@ -57,6 +57,12 @@ CachedViewImpl::read(Keylet const& k) const
         baseRead = true;
         return base_.read(k);
     });
+    {
+        // If the sle is null, then a failure must have occurred in base_.read()
+        XRPL_ASSERT(
+            sle || baseRead,
+            "ripple::CachedView::read : null SLE result from base");
+    }
     if (cacheHit && baseRead)
         hitsexpired.increment();
     else if (cacheHit)