Merge remote-tracking branch 'origin/develop' into dangell7/batch-v1

src/test/app/AMM_test.cpp

@@ -2229,6 +2229,31 @@ private:
             ammAlice.withdraw(alice_, XRPAmount{9'999'999'999});
             BEAST_EXPECT(ammAlice.expectBalances(XRPAmount{1}, USD(10'000), IOUAmount{100}));
         });
+
+        // singleWithdrawEPrice: crafted ePrice = lptAMMBalance*f/amountBalance
+        // makes the denominator (T*f - A*E) exactly zero.
+        // Pre-fixCleanup3_3_0: std::overflow_error escapes to the
+        // transactor backstop and is returned as tefEXCEPTION.
+        // Post-fixCleanup3_3_0: denominator check returns tecAMM_FAILED.
+        //
+        // Pool: USD(100)/EUR(100), baseFee=1000 (1%).
+        // Alice is the creator so her discounted fee is 100 (0.1%), f=0.001.
+        // ePrice = lptAMMBalance(100) * f(0.001) / amountBalance(100) = 0.001
+        testAMM(
+            [&](AMM& ammAlice, Env& env) {
+                auto const err =
+                    env.enabled(fixCleanup3_3_0) ? Ter(tecAMM_FAILED) : Ter(tefEXCEPTION);
+                ammAlice.withdraw(
+                    WithdrawArg{
+                        .account = alice_,
+                        .asset1Out = USD(0),
+                        .maxEP = IOUAmount{1, -3},  // ePrice=0.001 → denom=0
+                        .err = err});
+            },
+            {{USD(100), EUR(100)}},
+            1000,
+            std::nullopt,
+            {all - fixCleanup3_3_0, all});
     }
 
     void

src/test/rpc/GetAggregatePrice_test.cpp

@@ -3,12 +3,21 @@
 #include <test/jtx/Oracle.h>
 #include <test/jtx/amount.h>
 
+#include <xrpld/app/ledger/OpenLedger.h>
+
 #include <xrpl/basics/Number.h>
+#include <xrpl/basics/base_uint.h>
 #include <xrpl/beast/unit_test/suite.h>
+#include <xrpl/beast/utility/Journal.h>
+#include <xrpl/core/ServiceRegistry.h>
+#include <xrpl/ledger/OpenView.h>
 #include <xrpl/protocol/Feature.h>
+#include <xrpl/protocol/Indexes.h>
+#include <xrpl/protocol/SField.h>
 #include <xrpl/protocol/jss.h>
 
 #include <cstdlib>
+#include <memory>
 #include <optional>
 #include <string>
 #include <vector>
@@ -312,11 +321,91 @@ public:
         }
     }
 
+    void
+    testNullTxReadMeta()
+    {
+        testcase("Null txRead metadata");
+        using namespace jtx;
+
+        // Verify that iteratePriceData handles a null txRead result
+        // gracefully (returns early) rather than crashing with a
+        // nullptr dereference. This simulates local data corruption
+        // where a transaction referenced by sfPreviousTxnID is missing
+        // from the ledger's transaction map.
+        Env env(*this);
+        auto const baseFee = static_cast<int>(env.current()->fees().base.drops());
+
+        Account const owner{"owner"};
+        env.fund(XRP(1'000), owner);
+
+        // Create oracle with XRP/USD and XRP/EUR
+        Oracle oracle(
+            env,
+            {.owner = owner,
+             .series = {{"XRP", "USD", 740, 1}, {"XRP", "EUR", 840, 1}},
+             .fee = baseFee});
+
+        // Update oracle to only have XRP/EUR, pushing XRP/USD into
+        // history. iteratePriceData will need to read historical tx
+        // metadata to find the XRP/USD price.
+        oracle.set(UpdateArg{.series = {{"XRP", "EUR", 850, 1}}, .fee = baseFee});
+
+        OraclesData const oracles{{owner, oracle.documentID()}};
+
+        // Precondition: with an uncorrupted oracle, the historical
+        // traversal must succeed and produce a price for XRP/USD.
+        // This proves the test reaches iteratePriceData's history
+        // path; without it, a future change that breaks the setup
+        // could turn the post-corruption assertion into a vacuous
+        // pass (objectNotFound is reachable from many unrelated
+        // code paths).
+        {
+            auto const ret = Oracle::aggregatePrice(env, "XRP", "USD", oracles);
+            BEAST_EXPECT(!ret.isMember(jss::error));
+            BEAST_EXPECT(ret.isMember(jss::median));
+        }
+
+        // Simulate data corruption: modify the oracle SLE in the open
+        // ledger to have a bogus sfPreviousTxnID that doesn't exist in
+        // any ledger. sfPreviousTxnLgrSeq still points to a valid closed
+        // ledger, so getLedgerBySeq succeeds but txRead returns null.
+        auto const oracleKeylet = keylet::oracle(owner, oracle.documentID());
+        uint256 const bogusTxnID{0xABCABCAB};
+        bool const modified = env.app().getOpenLedger().modify(
+            [&oracleKeylet, &bogusTxnID](OpenView& view, beast::Journal) -> bool {
+                auto const sle = view.read(oracleKeylet);
+                if (!sle)
+                    return false;
+                auto replacement = std::make_shared<SLE>(*sle, sle->key());
+                replacement->setFieldH256(sfPreviousTxnID, bogusTxnID);
+                view.rawReplace(replacement);
+                return true;
+            });
+
+        // Confirm the injection actually took effect: modify must
+        // report success, and re-reading the SLE must show the
+        // bogus hash. Otherwise the failure-mode assertion below
+        // would not be exercising the null-txRead path at all.
+        BEAST_EXPECT(modified);
+        if (auto const sle = env.current()->read(oracleKeylet); BEAST_EXPECT(sle))
+            BEAST_EXPECT(sle->getFieldH256(sfPreviousTxnID) == bogusTxnID);
+
+        // Query for XRP/USD using the "current" (open) ledger.
+        // The oracle SLE now has a bogus sfPreviousTxnID. The current
+        // oracle only has EUR, so iteratePriceData will try to read
+        // history. txRead returns null for the bogus hash, and the
+        // null check should cause a graceful early return instead of
+        // a nullptr dereference.
+        auto const ret = Oracle::aggregatePrice(env, "XRP", "USD", oracles);
+        BEAST_EXPECT(ret[jss::error].asString() == "objectNotFound");
+    }
+
     void
     run() override
     {
         testErrors();
         testRpc();
+        testNullTxReadMeta();
     }
 };