refactor: Add ServiceRegistry to help modularization (#6222)

Currently we're passing the `Application` object around, whereby the `Application` class acts more like a service registry that gives other classes access to other services. In order to allow modularization, we should replace `Application` with a service registry class so that modules depending on `Application` for other services can be moved easily. This change adds the `ServiceRegistry` class.

.github/scripts/levelization/results/ordering.txt

@@ -153,6 +153,7 @@ tests.libxrpl > xrpl.json
 tests.libxrpl > xrpl.net
 xrpl.core > xrpl.basics
 xrpl.core > xrpl.json
+xrpl.core > xrpl.ledger
 xrpl.json > xrpl.basics
 xrpl.ledger > xrpl.basics
 xrpl.ledger > xrpl.protocol

include/xrpl/core/ServiceRegistry.h

@@ -0,0 +1,202 @@
+#ifndef XRPL_CORE_SERVICEREGISTRY_H_INCLUDED
+#define XRPL_CORE_SERVICEREGISTRY_H_INCLUDED
+
+#include <xrpl/basics/Blob.h>
+#include <xrpl/basics/SHAMapHash.h>
+#include <xrpl/basics/TaggedCache.h>
+#include <xrpl/ledger/CachedSLEs.h>
+
+namespace xrpl {
+
+// Forward declarations
+namespace NodeStore {
+class Database;
+}
+namespace Resource {
+class Manager;
+}
+namespace perf {
+class PerfLog;
+}
+
+class AcceptedLedger;
+class AmendmentTable;
+class Cluster;
+class CollectorManager;
+class DatabaseCon;
+class Family;
+class HashRouter;
+class InboundLedgers;
+class InboundTransactions;
+class JobQueue;
+class LedgerCleaner;
+class LedgerMaster;
+class LedgerReplayer;
+class LoadFeeTrack;
+class LoadManager;
+class ManifestCache;
+class NetworkOPs;
+class OpenLedger;
+class OrderBookDB;
+class Overlay;
+class PathRequests;
+class PeerReservationTable;
+class PendingSaves;
+class RelationalDatabase;
+class ServerHandler;
+class SHAMapStore;
+class TimeKeeper;
+class TransactionMaster;
+class TxQ;
+class ValidatorList;
+class ValidatorSite;
+
+template <class Adaptor>
+class Validations;
+class RCLValidationsAdaptor;
+using RCLValidations = Validations<RCLValidationsAdaptor>;
+
+using NodeCache = TaggedCache<SHAMapHash, Blob>;
+
+/** Service registry for dependency injection.
+
+    This abstract interface provides access to various services and components
+    used throughout the application. It separates the service locator pattern
+    from the Application lifecycle management.
+
+    Components that need access to services can hold a reference to
+    ServiceRegistry rather than Application when they only need service
+    access and not lifecycle management.
+
+*/
+class ServiceRegistry
+{
+public:
+    ServiceRegistry() = default;
+    virtual ~ServiceRegistry() = default;
+
+    // Core infrastructure services
+    virtual CollectorManager&
+    getCollectorManager() = 0;
+
+    virtual Family&
+    getNodeFamily() = 0;
+
+    virtual TimeKeeper&
+    timeKeeper() = 0;
+
+    virtual JobQueue&
+    getJobQueue() = 0;
+
+    virtual NodeCache&
+    getTempNodeCache() = 0;
+
+    virtual CachedSLEs&
+    cachedSLEs() = 0;
+
+    // Protocol and validation services
+    virtual AmendmentTable&
+    getAmendmentTable() = 0;
+
+    virtual HashRouter&
+    getHashRouter() = 0;
+
+    virtual LoadFeeTrack&
+    getFeeTrack() = 0;
+
+    virtual LoadManager&
+    getLoadManager() = 0;
+
+    virtual RCLValidations&
+    getValidations() = 0;
+
+    virtual ValidatorList&
+    validators() = 0;
+
+    virtual ValidatorSite&
+    validatorSites() = 0;
+
+    virtual ManifestCache&
+    validatorManifests() = 0;
+
+    virtual ManifestCache&
+    publisherManifests() = 0;
+
+    // Network services
+    virtual Overlay&
+    overlay() = 0;
+
+    virtual Cluster&
+    cluster() = 0;
+
+    virtual PeerReservationTable&
+    peerReservations() = 0;
+
+    virtual Resource::Manager&
+    getResourceManager() = 0;
+
+    // Storage services
+    virtual NodeStore::Database&
+    getNodeStore() = 0;
+
+    virtual SHAMapStore&
+    getSHAMapStore() = 0;
+
+    virtual RelationalDatabase&
+    getRelationalDatabase() = 0;
+
+    // Ledger services
+    virtual InboundLedgers&
+    getInboundLedgers() = 0;
+
+    virtual InboundTransactions&
+    getInboundTransactions() = 0;
+
+    virtual TaggedCache<uint256, AcceptedLedger>&
+    getAcceptedLedgerCache() = 0;
+
+    virtual LedgerMaster&
+    getLedgerMaster() = 0;
+
+    virtual LedgerCleaner&
+    getLedgerCleaner() = 0;
+
+    virtual LedgerReplayer&
+    getLedgerReplayer() = 0;
+
+    virtual PendingSaves&
+    pendingSaves() = 0;
+
+    virtual OpenLedger&
+    openLedger() = 0;
+
+    virtual OpenLedger const&
+    openLedger() const = 0;
+
+    // Transaction and operation services
+    virtual NetworkOPs&
+    getOPs() = 0;
+
+    virtual OrderBookDB&
+    getOrderBookDB() = 0;
+
+    virtual TransactionMaster&
+    getMasterTransaction() = 0;
+
+    virtual TxQ&
+    getTxQ() = 0;
+
+    virtual PathRequests&
+    getPathRequests() = 0;
+
+    // Server services
+    virtual ServerHandler&
+    getServerHandler() = 0;
+
+    virtual perf::PerfLog&
+    getPerfLog() = 0;
+};
+
+}  // namespace xrpl
+
+#endif

include/xrpl/protocol/detail/features.macro

@@ -16,6 +16,7 @@
 // Add new amendments to the top of this list.
 // Keep it sorted in reverse chronological order.
 
+XRPL_FIX    (ExpiredNFTokenOfferRemoval, Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FIX    (BatchInnerSigs,             Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(LendingProtocol,            Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(PermissionDelegationV1_1,   Supported::no,  VoteBehavior::DefaultNo)

src/test/app/NFToken_test.cpp

@@ -876,42 +876,48 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
         env.fund(XRP(1000), alice, buyer, gw);
         env.close();
         BEAST_EXPECT(ownerCount(env, alice) == 0);
+        BEAST_EXPECT(ownerCount(env, buyer) == 0);
 
         uint256 const nftAlice0ID = token::getNextID(env, alice, 0, tfTransferable);
         env(token::mint(alice, 0u), txflags(tfTransferable));
         env.close();
-        BEAST_EXPECT(ownerCount(env, alice) == 1);
+        uint8_t aliceCount = 1;
+        BEAST_EXPECT(ownerCount(env, alice) == aliceCount);
 
         uint256 const nftXrpOnlyID = token::getNextID(env, alice, 0, tfOnlyXRP | tfTransferable);
         env(token::mint(alice, 0), txflags(tfOnlyXRP | tfTransferable));
         env.close();
-        BEAST_EXPECT(ownerCount(env, alice) == 1);
+        BEAST_EXPECT(ownerCount(env, alice) == aliceCount);
 
         uint256 nftNoXferID = token::getNextID(env, alice, 0);
         env(token::mint(alice, 0));
         env.close();
-        BEAST_EXPECT(ownerCount(env, alice) == 1);
+        BEAST_EXPECT(ownerCount(env, alice) == aliceCount);
 
         // alice creates sell offers for her nfts.
         uint256 const plainOfferIndex = keylet::nftoffer(alice, env.seq(alice)).key;
         env(token::createOffer(alice, nftAlice0ID, XRP(10)), txflags(tfSellNFToken));
         env.close();
-        BEAST_EXPECT(ownerCount(env, alice) == 2);
+        aliceCount++;
+        BEAST_EXPECT(ownerCount(env, alice) == aliceCount);
 
         uint256 const audOfferIndex = keylet::nftoffer(alice, env.seq(alice)).key;
         env(token::createOffer(alice, nftAlice0ID, gwAUD(30)), txflags(tfSellNFToken));
         env.close();
-        BEAST_EXPECT(ownerCount(env, alice) == 3);
+        aliceCount++;
+        BEAST_EXPECT(ownerCount(env, alice) == aliceCount);
 
         uint256 const xrpOnlyOfferIndex = keylet::nftoffer(alice, env.seq(alice)).key;
         env(token::createOffer(alice, nftXrpOnlyID, XRP(20)), txflags(tfSellNFToken));
         env.close();
-        BEAST_EXPECT(ownerCount(env, alice) == 4);
+        aliceCount++;
+        BEAST_EXPECT(ownerCount(env, alice) == aliceCount);
 
         uint256 const noXferOfferIndex = keylet::nftoffer(alice, env.seq(alice)).key;
         env(token::createOffer(alice, nftNoXferID, XRP(30)), txflags(tfSellNFToken));
         env.close();
-        BEAST_EXPECT(ownerCount(env, alice) == 5);
+        aliceCount++;
+        BEAST_EXPECT(ownerCount(env, alice) == aliceCount);
 
         // alice creates a sell offer that will expire soon.
         uint256 const aliceExpOfferIndex = keylet::nftoffer(alice, env.seq(alice)).key;
@@ -919,7 +925,17 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             txflags(tfSellNFToken),
             token::expiration(lastClose(env) + 5));
         env.close();
-        BEAST_EXPECT(ownerCount(env, alice) == 6);
+        aliceCount++;
+        BEAST_EXPECT(ownerCount(env, alice) == aliceCount);
+
+        // buyer creates a Buy offer that will expire soon.
+        uint256 const buyerExpOfferIndex = keylet::nftoffer(buyer, env.seq(buyer)).key;
+        env(token::createOffer(buyer, nftAlice0ID, XRP(40)),
+            token::owner(alice),
+            token::expiration(lastClose(env) + 5));
+        env.close();
+        uint8_t buyerCount = 1;
+        BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
         //----------------------------------------------------------------------
         // preflight
@@ -927,12 +943,12 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
         // Set a negative fee.
         env(token::acceptSellOffer(buyer, noXferOfferIndex), fee(STAmount(10ull, true)), ter(temBAD_FEE));
         env.close();
-        BEAST_EXPECT(ownerCount(env, buyer) == 0);
+        BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
         // Set an invalid flag.
         env(token::acceptSellOffer(buyer, noXferOfferIndex), txflags(0x00008000), ter(temINVALID_FLAG));
         env.close();
-        BEAST_EXPECT(ownerCount(env, buyer) == 0);
+        BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
         // Supply nether an sfNFTokenBuyOffer nor an sfNFTokenSellOffer field.
         {
@@ -940,7 +956,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             jv.removeMember(sfNFTokenSellOffer.jsonName);
             env(jv, ter(temMALFORMED));
             env.close();
-            BEAST_EXPECT(ownerCount(env, buyer) == 0);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
         }
 
         // A buy offer may not contain a sfNFTokenBrokerFee field.
@@ -949,7 +965,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             jv[sfNFTokenBrokerFee.jsonName] = STAmount(500000).getJson(JsonOptions::none);
             env(jv, ter(temMALFORMED));
             env.close();
-            BEAST_EXPECT(ownerCount(env, buyer) == 0);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
         }
 
         // A sell offer may not contain a sfNFTokenBrokerFee field.
@@ -958,7 +974,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             jv[sfNFTokenBrokerFee.jsonName] = STAmount(500000).getJson(JsonOptions::none);
             env(jv, ter(temMALFORMED));
             env.close();
-            BEAST_EXPECT(ownerCount(env, buyer) == 0);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
         }
 
         // A brokered offer may not contain a negative or zero brokerFee.
@@ -966,7 +982,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             token::brokerFee(gwAUD(0)),
             ter(temMALFORMED));
         env.close();
-        BEAST_EXPECT(ownerCount(env, buyer) == 0);
+        BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
         //----------------------------------------------------------------------
         // preclaim
@@ -974,33 +990,48 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
         // The buy offer must be non-zero.
         env(token::acceptBuyOffer(buyer, beast::zero), ter(tecOBJECT_NOT_FOUND));
         env.close();
-        BEAST_EXPECT(ownerCount(env, buyer) == 0);
+        BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
         // The buy offer must be present in the ledger.
         uint256 const missingOfferIndex = keylet::nftoffer(alice, 1).key;
         env(token::acceptBuyOffer(buyer, missingOfferIndex), ter(tecOBJECT_NOT_FOUND));
         env.close();
-        BEAST_EXPECT(ownerCount(env, buyer) == 0);
+        BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
         // The buy offer must not have expired.
-        env(token::acceptBuyOffer(buyer, aliceExpOfferIndex), ter(tecEXPIRED));
+        // NOTE: this is only a preclaim check with the
+        // fixExpiredNFTokenOfferRemoval amendment disabled.
+        env(token::acceptBuyOffer(alice, buyerExpOfferIndex), ter(tecEXPIRED));
         env.close();
-        BEAST_EXPECT(ownerCount(env, buyer) == 0);
+        if (features[fixExpiredNFTokenOfferRemoval])
+        {
+            buyerCount--;
+        }
+        BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
         // The sell offer must be non-zero.
         env(token::acceptSellOffer(buyer, beast::zero), ter(tecOBJECT_NOT_FOUND));
         env.close();
-        BEAST_EXPECT(ownerCount(env, buyer) == 0);
+        BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
         // The sell offer must be present in the ledger.
         env(token::acceptSellOffer(buyer, missingOfferIndex), ter(tecOBJECT_NOT_FOUND));
         env.close();
-        BEAST_EXPECT(ownerCount(env, buyer) == 0);
+        BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
         // The sell offer must not have expired.
+        // NOTE: this is only a preclaim check with the
+        // fixExpiredNFTokenOfferRemoval amendment disabled.
         env(token::acceptSellOffer(buyer, aliceExpOfferIndex), ter(tecEXPIRED));
         env.close();
-        BEAST_EXPECT(ownerCount(env, buyer) == 0);
+        // Alice's count is decremented by one when the expired offer is
+        // removed.
+        if (features[fixExpiredNFTokenOfferRemoval])
+        {
+            aliceCount--;
+        }
+        BEAST_EXPECT(ownerCount(env, alice) == aliceCount);
+        BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
         //----------------------------------------------------------------------
         // preclaim brokered
@@ -1012,8 +1043,13 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
         env.close();
         env(pay(gw, buyer, gwAUD(30)));
         env.close();
-        BEAST_EXPECT(ownerCount(env, alice) == 7);
-        BEAST_EXPECT(ownerCount(env, buyer) == 1);
+        aliceCount++;
+        buyerCount++;
+        BEAST_EXPECT(ownerCount(env, alice) == aliceCount);
+        BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
+
+        BEAST_EXPECT(ownerCount(env, alice) == aliceCount);
+        BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
         // We're about to exercise offer brokering, so we need
         // corresponding buy and sell offers.
@@ -1022,35 +1058,38 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             uint256 const buyerOfferIndex = keylet::nftoffer(buyer, env.seq(buyer)).key;
             env(token::createOffer(buyer, nftAlice0ID, gwAUD(29)), token::owner(alice));
             env.close();
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
+            buyerCount++;
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // gw attempts to broker offers that are not for the same token.
             env(token::brokerOffers(gw, buyerOfferIndex, xrpOnlyOfferIndex), ter(tecNFTOKEN_BUY_SELL_MISMATCH));
             env.close();
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // gw attempts to broker offers that are not for the same currency.
             env(token::brokerOffers(gw, buyerOfferIndex, plainOfferIndex), ter(tecNFTOKEN_BUY_SELL_MISMATCH));
             env.close();
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // In a brokered offer, the buyer must offer greater than or
             // equal to the selling price.
             env(token::brokerOffers(gw, buyerOfferIndex, audOfferIndex), ter(tecINSUFFICIENT_PAYMENT));
             env.close();
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // Remove buyer's offer.
             env(token::cancelOffer(buyer, {buyerOfferIndex}));
             env.close();
-            BEAST_EXPECT(ownerCount(env, buyer) == 1);
+            buyerCount--;
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
         }
         {
             // buyer creates a buy offer for one of alice's nfts.
             uint256 const buyerOfferIndex = keylet::nftoffer(buyer, env.seq(buyer)).key;
             env(token::createOffer(buyer, nftAlice0ID, gwAUD(31)), token::owner(alice));
             env.close();
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
+            buyerCount++;
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // Broker sets their fee in a denomination other than the one
             // used by the offers
@@ -1058,14 +1097,14 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
                 token::brokerFee(XRP(40)),
                 ter(tecNFTOKEN_BUY_SELL_MISMATCH));
             env.close();
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // Broker fee way too big.
             env(token::brokerOffers(gw, buyerOfferIndex, audOfferIndex),
                 token::brokerFee(gwAUD(31)),
                 ter(tecINSUFFICIENT_PAYMENT));
             env.close();
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // Broker fee is smaller, but still too big once the offer
             // seller's minimum is taken into account.
@@ -1073,12 +1112,13 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
                 token::brokerFee(gwAUD(1.5)),
                 ter(tecINSUFFICIENT_PAYMENT));
             env.close();
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // Remove buyer's offer.
             env(token::cancelOffer(buyer, {buyerOfferIndex}));
             env.close();
-            BEAST_EXPECT(ownerCount(env, buyer) == 1);
+            buyerCount--;
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
         }
         //----------------------------------------------------------------------
         // preclaim buy
@@ -1087,17 +1127,18 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             uint256 const buyerOfferIndex = keylet::nftoffer(buyer, env.seq(buyer)).key;
             env(token::createOffer(buyer, nftAlice0ID, gwAUD(30)), token::owner(alice));
             env.close();
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
+            buyerCount++;
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // Don't accept a buy offer if the sell flag is set.
             env(token::acceptBuyOffer(buyer, plainOfferIndex), ter(tecNFTOKEN_OFFER_TYPE_MISMATCH));
             env.close();
-            BEAST_EXPECT(ownerCount(env, alice) == 7);
+            BEAST_EXPECT(ownerCount(env, alice) == aliceCount);
 
             // An account can't accept its own offer.
             env(token::acceptBuyOffer(buyer, buyerOfferIndex), ter(tecCANT_ACCEPT_OWN_NFTOKEN_OFFER));
             env.close();
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // An offer acceptor must have enough funds to pay for the offer.
             env(pay(buyer, gw, gwAUD(30)));
@@ -1105,7 +1146,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             BEAST_EXPECT(env.balance(buyer, gwAUD) == gwAUD(0));
             env(token::acceptBuyOffer(alice, buyerOfferIndex), ter(tecINSUFFICIENT_FUNDS));
             env.close();
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // alice gives her NFT to gw, so alice no longer owns nftAlice0.
             {
@@ -1114,7 +1155,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
                 env.close();
                 env(token::acceptSellOffer(gw, offerIndex));
                 env.close();
-                BEAST_EXPECT(ownerCount(env, alice) == 7);
+                BEAST_EXPECT(ownerCount(env, alice) == aliceCount);
             }
             env(pay(gw, buyer, gwAUD(30)));
             env.close();
@@ -1122,12 +1163,13 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             // alice can't accept a buy offer for an NFT she no longer owns.
             env(token::acceptBuyOffer(alice, buyerOfferIndex), ter(tecNO_PERMISSION));
             env.close();
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // Remove buyer's offer.
             env(token::cancelOffer(buyer, {buyerOfferIndex}));
             env.close();
-            BEAST_EXPECT(ownerCount(env, buyer) == 1);
+            buyerCount--;
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
         }
         //----------------------------------------------------------------------
         // preclaim sell
@@ -1136,23 +1178,24 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             uint256 const buyerOfferIndex = keylet::nftoffer(buyer, env.seq(buyer)).key;
             env(token::createOffer(buyer, nftXrpOnlyID, XRP(30)), token::owner(alice));
             env.close();
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
+            buyerCount++;
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // Don't accept a sell offer without the sell flag set.
             env(token::acceptSellOffer(alice, buyerOfferIndex), ter(tecNFTOKEN_OFFER_TYPE_MISMATCH));
             env.close();
-            BEAST_EXPECT(ownerCount(env, alice) == 7);
+            BEAST_EXPECT(ownerCount(env, alice) == aliceCount);
 
             // An account can't accept its own offer.
             env(token::acceptSellOffer(alice, plainOfferIndex), ter(tecCANT_ACCEPT_OWN_NFTOKEN_OFFER));
             env.close();
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // The seller must currently be in possession of the token they
             // are selling.  alice gave nftAlice0ID to gw.
             env(token::acceptSellOffer(buyer, plainOfferIndex), ter(tecNO_PERMISSION));
             env.close();
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // gw gives nftAlice0ID back to alice.  That allows us to check
             // buyer attempting to accept one of alice's offers with
@@ -1163,14 +1206,14 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
                 env.close();
                 env(token::acceptSellOffer(alice, offerIndex));
                 env.close();
-                BEAST_EXPECT(ownerCount(env, alice) == 7);
+                BEAST_EXPECT(ownerCount(env, alice) == aliceCount);
             }
             env(pay(buyer, gw, gwAUD(30)));
             env.close();
             BEAST_EXPECT(env.balance(buyer, gwAUD) == gwAUD(0));
             env(token::acceptSellOffer(buyer, audOfferIndex), ter(tecINSUFFICIENT_FUNDS));
             env.close();
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
         }
 
         //----------------------------------------------------------------------
@@ -2769,6 +2812,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
         uint256 const nftokenID1 = token::getNextID(env, issuer, 0, tfTransferable);
         env(token::mint(minter, 0), token::issuer(issuer), txflags(tfTransferable));
         env.close();
+        uint8_t issuerCount, minterCount, buyerCount;
 
         // Test how adding an Expiration field to an offer affects permissions
         // for cancelling offers.
@@ -2792,9 +2836,12 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             uint256 const offerBuyerToMinter = keylet::nftoffer(buyer, env.seq(buyer)).key;
             env(token::createOffer(buyer, nftokenID0, drops(1)), token::owner(minter), token::expiration(expiration));
             env.close();
-            BEAST_EXPECT(ownerCount(env, issuer) == 1);
-            BEAST_EXPECT(ownerCount(env, minter) == 3);
-            BEAST_EXPECT(ownerCount(env, buyer) == 1);
+            issuerCount = 1;
+            minterCount = 3;
+            buyerCount = 1;
+            BEAST_EXPECT(ownerCount(env, issuer) == issuerCount);
+            BEAST_EXPECT(ownerCount(env, minter) == minterCount);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // Test who gets to cancel the offers.  Anyone outside of the
             // offer-owner/destination pair should not be able to cancel
@@ -2806,32 +2853,36 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             env(token::cancelOffer(buyer, {offerIssuerToMinter}), ter(tecNO_PERMISSION));
             env.close();
             BEAST_EXPECT(lastClose(env) < expiration);
-            BEAST_EXPECT(ownerCount(env, issuer) == 1);
-            BEAST_EXPECT(ownerCount(env, minter) == 3);
-            BEAST_EXPECT(ownerCount(env, buyer) == 1);
+            BEAST_EXPECT(ownerCount(env, issuer) == issuerCount);
+            BEAST_EXPECT(ownerCount(env, minter) == minterCount);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // The offer creator can cancel their own unexpired offer.
             env(token::cancelOffer(minter, {offerMinterToAnyone}));
+            minterCount--;
 
             // The destination of a sell offer can cancel the NFT owner's
             // unexpired offer.
             env(token::cancelOffer(issuer, {offerMinterToIssuer}));
+            minterCount--;
 
             // Close enough ledgers to get past the expiration.
             while (lastClose(env) < expiration)
                 env.close();
 
-            BEAST_EXPECT(ownerCount(env, issuer) == 1);
-            BEAST_EXPECT(ownerCount(env, minter) == 1);
-            BEAST_EXPECT(ownerCount(env, buyer) == 1);
+            BEAST_EXPECT(ownerCount(env, issuer) == issuerCount);
+            BEAST_EXPECT(ownerCount(env, minter) == minterCount);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // Anyone can cancel expired offers.
             env(token::cancelOffer(issuer, {offerBuyerToMinter}));
+            buyerCount--;
             env(token::cancelOffer(buyer, {offerIssuerToMinter}));
+            issuerCount--;
             env.close();
-            BEAST_EXPECT(ownerCount(env, issuer) == 0);
-            BEAST_EXPECT(ownerCount(env, minter) == 1);
-            BEAST_EXPECT(ownerCount(env, buyer) == 0);
+            BEAST_EXPECT(ownerCount(env, issuer) == issuerCount);
+            BEAST_EXPECT(ownerCount(env, minter) == minterCount);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
         }
         // Show that:
         //  1. An unexpired sell offer with an expiration can be accepted.
@@ -2844,44 +2895,70 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             env(token::createOffer(minter, nftokenID0, drops(1)),
                 token::expiration(expiration),
                 txflags(tfSellNFToken));
+            minterCount++;
 
             uint256 const offer1 = keylet::nftoffer(minter, env.seq(minter)).key;
             env(token::createOffer(minter, nftokenID1, drops(1)),
                 token::expiration(expiration),
                 txflags(tfSellNFToken));
+            minterCount++;
             env.close();
             BEAST_EXPECT(lastClose(env) < expiration);
-            BEAST_EXPECT(ownerCount(env, issuer) == 0);
-            BEAST_EXPECT(ownerCount(env, minter) == 3);
-            BEAST_EXPECT(ownerCount(env, buyer) == 0);
+            BEAST_EXPECT(ownerCount(env, issuer) == issuerCount);
+            BEAST_EXPECT(ownerCount(env, minter) == minterCount);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // Anyone can accept an unexpired sell offer.
             env(token::acceptSellOffer(buyer, offer0));
+            minterCount--;
+            buyerCount++;
 
             // Close enough ledgers to get past the expiration.
             while (lastClose(env) < expiration)
                 env.close();
 
-            BEAST_EXPECT(ownerCount(env, issuer) == 0);
-            BEAST_EXPECT(ownerCount(env, minter) == 2);
-            BEAST_EXPECT(ownerCount(env, buyer) == 1);
+            BEAST_EXPECT(ownerCount(env, issuer) == issuerCount);
+            BEAST_EXPECT(ownerCount(env, minter) == minterCount);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // No one can accept an expired sell offer.
             env(token::acceptSellOffer(buyer, offer1), ter(tecEXPIRED));
-            env(token::acceptSellOffer(issuer, offer1), ter(tecEXPIRED));
+
+            // With fixExpiredNFTokenOfferRemoval amendment, the first accept
+            // attempt deletes the expired offer. Without the amendment,
+            // the offer remains and we can try to accept it again.
+            if (features[fixExpiredNFTokenOfferRemoval])
+            {
+                // After amendment: offer was deleted by first accept attempt
+                minterCount--;
+                env(token::acceptSellOffer(issuer, offer1), ter(tecOBJECT_NOT_FOUND));
+            }
+            else
+            {
+                // Before amendment: offer still exists, second accept also
+                // fails
+                env(token::acceptSellOffer(issuer, offer1), ter(tecEXPIRED));
+            }
             env.close();
 
-            // The expired sell offer is still in the ledger.
-            BEAST_EXPECT(ownerCount(env, issuer) == 0);
-            BEAST_EXPECT(ownerCount(env, minter) == 2);
-            BEAST_EXPECT(ownerCount(env, buyer) == 1);
+            // Check if the expired sell offer behavior matches amendment status
+            BEAST_EXPECT(ownerCount(env, issuer) == issuerCount);
+            BEAST_EXPECT(ownerCount(env, minter) == minterCount);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
-            // Anyone can cancel the expired sell offer.
-            env(token::cancelOffer(issuer, {offer1}));
-            env.close();
-            BEAST_EXPECT(ownerCount(env, issuer) == 0);
-            BEAST_EXPECT(ownerCount(env, minter) == 1);
-            BEAST_EXPECT(ownerCount(env, buyer) == 1);
+            if (!features[fixExpiredNFTokenOfferRemoval])
+            {
+                // Before amendment: expired offer still exists and needs to be
+                // cancelled
+                env(token::cancelOffer(issuer, {offer1}));
+                env.close();
+                minterCount--;
+            }
+            // Ensure that owner counts are correct with and without the
+            // amendment
+            BEAST_EXPECT(ownerCount(env, issuer) == 0 && issuerCount == 0);
+            BEAST_EXPECT(ownerCount(env, minter) == 1 && minterCount == 1);
+            BEAST_EXPECT(ownerCount(env, buyer) == 1 && buyerCount == 1);
 
             // Transfer nftokenID0 back to minter so we start the next test in
             // a simple place.
@@ -2889,10 +2966,11 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             env(token::createOffer(buyer, nftokenID0, XRP(0)), txflags(tfSellNFToken), token::destination(minter));
             env.close();
             env(token::acceptSellOffer(minter, offerSellBack));
+            buyerCount--;
             env.close();
-            BEAST_EXPECT(ownerCount(env, issuer) == 0);
-            BEAST_EXPECT(ownerCount(env, minter) == 1);
-            BEAST_EXPECT(ownerCount(env, buyer) == 0);
+            BEAST_EXPECT(ownerCount(env, issuer) == issuerCount);
+            BEAST_EXPECT(ownerCount(env, minter) == minterCount);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
         }
         // Show that:
         //  1. An unexpired buy offer with an expiration can be accepted.
@@ -2903,14 +2981,16 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
 
             uint256 const offer0 = keylet::nftoffer(buyer, env.seq(buyer)).key;
             env(token::createOffer(buyer, nftokenID0, drops(1)), token::owner(minter), token::expiration(expiration));
+            buyerCount++;
 
             uint256 const offer1 = keylet::nftoffer(buyer, env.seq(buyer)).key;
             env(token::createOffer(buyer, nftokenID1, drops(1)), token::owner(minter), token::expiration(expiration));
+            buyerCount++;
             env.close();
             BEAST_EXPECT(lastClose(env) < expiration);
-            BEAST_EXPECT(ownerCount(env, issuer) == 0);
-            BEAST_EXPECT(ownerCount(env, minter) == 1);
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
+            BEAST_EXPECT(ownerCount(env, issuer) == issuerCount);
+            BEAST_EXPECT(ownerCount(env, minter) == minterCount);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // An unexpired buy offer can be accepted.
             env(token::acceptBuyOffer(minter, offer0));
@@ -2919,26 +2999,48 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             while (lastClose(env) < expiration)
                 env.close();
 
-            BEAST_EXPECT(ownerCount(env, issuer) == 0);
-            BEAST_EXPECT(ownerCount(env, minter) == 1);
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
+            BEAST_EXPECT(ownerCount(env, issuer) == issuerCount);
+            BEAST_EXPECT(ownerCount(env, minter) == minterCount);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // An expired buy offer cannot be accepted.
             env(token::acceptBuyOffer(minter, offer1), ter(tecEXPIRED));
-            env(token::acceptBuyOffer(issuer, offer1), ter(tecEXPIRED));
+
+            // With fixExpiredNFTokenOfferRemoval amendment, the first accept
+            // attempt deletes the expired offer. Without the amendment,
+            // the offer remains and we can try to accept it again.
+            if (features[fixExpiredNFTokenOfferRemoval])
+            {
+                // After amendment: offer was deleted by first accept attempt
+                buyerCount--;
+                env(token::acceptBuyOffer(issuer, offer1), ter(tecOBJECT_NOT_FOUND));
+            }
+            else
+            {
+                // Before amendment: offer still exists, second accept also
+                // fails
+                env(token::acceptBuyOffer(issuer, offer1), ter(tecEXPIRED));
+            }
             env.close();
 
-            // The expired buy offer is still in the ledger.
-            BEAST_EXPECT(ownerCount(env, issuer) == 0);
-            BEAST_EXPECT(ownerCount(env, minter) == 1);
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
+            // Check if the expired buy offer behavior matches amendment status
+            BEAST_EXPECT(ownerCount(env, issuer) == issuerCount);
+            BEAST_EXPECT(ownerCount(env, minter) == minterCount);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
-            // Anyone can cancel the expired buy offer.
-            env(token::cancelOffer(issuer, {offer1}));
-            env.close();
-            BEAST_EXPECT(ownerCount(env, issuer) == 0);
-            BEAST_EXPECT(ownerCount(env, minter) == 1);
-            BEAST_EXPECT(ownerCount(env, buyer) == 1);
+            if (!features[fixExpiredNFTokenOfferRemoval])
+            {
+                // Before amendment: expired offer still exists and can be
+                // cancelled
+                env(token::cancelOffer(issuer, {offer1}));
+                env.close();
+                buyerCount--;
+            }
+            // Ensure that owner counts are the same with and without the
+            // amendment
+            BEAST_EXPECT(ownerCount(env, issuer) == 0 && issuerCount == 0);
+            BEAST_EXPECT(ownerCount(env, minter) == 1 && minterCount == 1);
+            BEAST_EXPECT(ownerCount(env, buyer) == 1 && buyerCount == 1);
 
             // Transfer nftokenID0 back to minter so we start the next test in
             // a simple place.
@@ -2947,9 +3049,10 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             env.close();
             env(token::acceptSellOffer(minter, offerSellBack));
             env.close();
-            BEAST_EXPECT(ownerCount(env, issuer) == 0);
-            BEAST_EXPECT(ownerCount(env, minter) == 1);
-            BEAST_EXPECT(ownerCount(env, buyer) == 0);
+            buyerCount--;
+            BEAST_EXPECT(ownerCount(env, issuer) == issuerCount);
+            BEAST_EXPECT(ownerCount(env, minter) == minterCount);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
         }
         // Show that in brokered mode:
         //  1. An unexpired sell offer with an expiration can be accepted.
@@ -2962,50 +3065,74 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             env(token::createOffer(minter, nftokenID0, drops(1)),
                 token::expiration(expiration),
                 txflags(tfSellNFToken));
+            minterCount++;
 
             uint256 const sellOffer1 = keylet::nftoffer(minter, env.seq(minter)).key;
             env(token::createOffer(minter, nftokenID1, drops(1)),
                 token::expiration(expiration),
                 txflags(tfSellNFToken));
+            minterCount++;
 
             uint256 const buyOffer0 = keylet::nftoffer(buyer, env.seq(buyer)).key;
             env(token::createOffer(buyer, nftokenID0, drops(1)), token::owner(minter));
+            buyerCount++;
 
             uint256 const buyOffer1 = keylet::nftoffer(buyer, env.seq(buyer)).key;
             env(token::createOffer(buyer, nftokenID1, drops(1)), token::owner(minter));
+            buyerCount++;
 
             env.close();
             BEAST_EXPECT(lastClose(env) < expiration);
-            BEAST_EXPECT(ownerCount(env, issuer) == 0);
-            BEAST_EXPECT(ownerCount(env, minter) == 3);
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
+            BEAST_EXPECT(ownerCount(env, issuer) == issuerCount);
+            BEAST_EXPECT(ownerCount(env, minter) == minterCount);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // An unexpired offer can be brokered.
             env(token::brokerOffers(issuer, buyOffer0, sellOffer0));
+            minterCount--;
 
             // Close enough ledgers to get past the expiration.
             while (lastClose(env) < expiration)
                 env.close();
 
-            BEAST_EXPECT(ownerCount(env, issuer) == 0);
-            BEAST_EXPECT(ownerCount(env, minter) == 2);
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
+            BEAST_EXPECT(ownerCount(env, issuer) == issuerCount);
+            BEAST_EXPECT(ownerCount(env, minter) == minterCount);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
             // If the sell offer is expired it cannot be brokered.
             env(token::brokerOffers(issuer, buyOffer1, sellOffer1), ter(tecEXPIRED));
             env.close();
 
-            // The expired sell offer is still in the ledger.
-            BEAST_EXPECT(ownerCount(env, issuer) == 0);
-            BEAST_EXPECT(ownerCount(env, minter) == 2);
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
+            if (features[fixExpiredNFTokenOfferRemoval])
+            {
+                // With amendment: expired offers are deleted
+                minterCount--;
+            }
 
-            // Anyone can cancel the expired sell offer.
-            env(token::cancelOffer(buyer, {buyOffer1, sellOffer1}));
+            BEAST_EXPECT(ownerCount(env, issuer) == issuerCount);
+            BEAST_EXPECT(ownerCount(env, minter) == minterCount);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
+
+            if (features[fixExpiredNFTokenOfferRemoval])
+            {
+                // The buy offer was deleted, so no need to cancel it
+                // The sell offer still exists, so we can cancel it
+                env(token::cancelOffer(buyer, {buyOffer1}));
+                buyerCount--;
+            }
+            else
+            {
+                // Anyone can cancel the expired offers
+                env(token::cancelOffer(buyer, {buyOffer1, sellOffer1}));
+                minterCount--;
+                buyerCount--;
+            }
             env.close();
-            BEAST_EXPECT(ownerCount(env, issuer) == 0);
-            BEAST_EXPECT(ownerCount(env, minter) == 1);
-            BEAST_EXPECT(ownerCount(env, buyer) == 1);
+            // Ensure that owner counts are the same with and without the
+            // amendment
+            BEAST_EXPECT(ownerCount(env, issuer) == 0 && issuerCount == 0);
+            BEAST_EXPECT(ownerCount(env, minter) == 1 && minterCount == 1);
+            BEAST_EXPECT(ownerCount(env, buyer) == 1 && buyerCount == 1);
 
             // Transfer nftokenID0 back to minter so we start the next test in
             // a simple place.
@@ -3014,9 +3141,10 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             env.close();
             env(token::acceptSellOffer(minter, offerSellBack));
             env.close();
-            BEAST_EXPECT(ownerCount(env, issuer) == 0);
-            BEAST_EXPECT(ownerCount(env, minter) == 1);
-            BEAST_EXPECT(ownerCount(env, buyer) == 0);
+            buyerCount--;
+            BEAST_EXPECT(ownerCount(env, issuer) == issuerCount);
+            BEAST_EXPECT(ownerCount(env, minter) == minterCount);
+            BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
         }
         // Show that in brokered mode:
         //  1. An unexpired buy offer with an expiration can be accepted.
@@ -3054,17 +3182,28 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             BEAST_EXPECT(ownerCount(env, minter) == 2);
             BEAST_EXPECT(ownerCount(env, buyer) == 2);
 
-            // If the buy offer is expired it cannot be brokered.
             env(token::brokerOffers(issuer, buyOffer1, sellOffer1), ter(tecEXPIRED));
             env.close();
 
-            // The expired buy offer is still in the ledger.
             BEAST_EXPECT(ownerCount(env, issuer) == 0);
-            BEAST_EXPECT(ownerCount(env, minter) == 2);
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
-
-            // Anyone can cancel the expired buy offer.
-            env(token::cancelOffer(minter, {buyOffer1, sellOffer1}));
+            if (features[fixExpiredNFTokenOfferRemoval])
+            {
+                // After amendment: expired offers were deleted during broker
+                // attempt
+                BEAST_EXPECT(ownerCount(env, minter) == 2);
+                BEAST_EXPECT(ownerCount(env, buyer) == 1);
+                // The buy offer was deleted, so no need to cancel it
+                // The sell offer still exists, so we can cancel it
+                env(token::cancelOffer(minter, {sellOffer1}));
+            }
+            else
+            {
+                // Before amendment: expired offers still exist in ledger
+                BEAST_EXPECT(ownerCount(env, minter) == 2);
+                BEAST_EXPECT(ownerCount(env, buyer) == 2);
+                // Anyone can cancel the expired offers
+                env(token::cancelOffer(minter, {buyOffer1, sellOffer1}));
+            }
             env.close();
             BEAST_EXPECT(ownerCount(env, issuer) == 0);
             BEAST_EXPECT(ownerCount(env, minter) == 1);
@@ -3122,17 +3261,19 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             BEAST_EXPECT(ownerCount(env, minter) == 2);
             BEAST_EXPECT(ownerCount(env, buyer) == 2);
 
-            // If the offers are expired they cannot be brokered.
             env(token::brokerOffers(issuer, buyOffer1, sellOffer1), ter(tecEXPIRED));
             env.close();
 
             // The expired offers are still in the ledger.
             BEAST_EXPECT(ownerCount(env, issuer) == 0);
-            BEAST_EXPECT(ownerCount(env, minter) == 2);
-            BEAST_EXPECT(ownerCount(env, buyer) == 2);
-
-            // Anyone can cancel the expired offers.
-            env(token::cancelOffer(issuer, {buyOffer1, sellOffer1}));
+            if (!features[fixExpiredNFTokenOfferRemoval])
+            {
+                // Before amendment: expired offers still exist in ledger
+                BEAST_EXPECT(ownerCount(env, minter) == 2);
+                BEAST_EXPECT(ownerCount(env, buyer) == 2);
+                // Anyone can cancel the expired offers
+                env(token::cancelOffer(issuer, {buyOffer1, sellOffer1}));
+            }
             env.close();
             BEAST_EXPECT(ownerCount(env, issuer) == 0);
             BEAST_EXPECT(ownerCount(env, minter) == 1);
@@ -6736,7 +6877,9 @@ public:
     void
     run() override
     {
-        testWithFeats(allFeatures - fixNFTokenReserve - featureNFTokenMintOffer - featureDynamicNFT);
+        testWithFeats(
+            allFeatures - fixNFTokenReserve - featureNFTokenMintOffer - featureDynamicNFT -
+            fixExpiredNFTokenOfferRemoval);
     }
 };
 
@@ -6767,6 +6910,15 @@ class NFTokenWOModify_test : public NFTokenBaseUtil_test
     }
 };
 
+class NFTokenWOExpiredOfferRemoval_test : public NFTokenBaseUtil_test
+{
+    void
+    run() override
+    {
+        testWithFeats(allFeatures - fixExpiredNFTokenOfferRemoval);
+    }
+};
+
 class NFTokenAllFeatures_test : public NFTokenBaseUtil_test
 {
     void

src/xrpld/app/main/Application.h

@@ -6,6 +6,7 @@
 
 #include <xrpl/basics/TaggedCache.h>
 #include <xrpl/beast/utility/PropertyStream.h>
+#include <xrpl/core/ServiceRegistry.h>
 #include <xrpl/protocol/Protocol.h>
 #include <xrpl/shamap/TreeNodeCache.h>
 
@@ -91,7 +92,7 @@ class Validations;
 class RCLValidationsAdaptor;
 using RCLValidations = Validations<RCLValidationsAdaptor>;
 
-class Application : public beast::PropertyStream::Source
+class Application : public ServiceRegistry, public beast::PropertyStream::Source
 {
 public:
     /* VFALCO NOTE
@@ -146,92 +147,12 @@ public:
     virtual boost::asio::io_context&
     getIOContext() = 0;
 
-    virtual CollectorManager&
-    getCollectorManager() = 0;
-    virtual Family&
-    getNodeFamily() = 0;
-    virtual TimeKeeper&
-    timeKeeper() = 0;
-    virtual JobQueue&
-    getJobQueue() = 0;
-    virtual NodeCache&
-    getTempNodeCache() = 0;
-    virtual CachedSLEs&
-    cachedSLEs() = 0;
-    virtual AmendmentTable&
-    getAmendmentTable() = 0;
-    virtual HashRouter&
-    getHashRouter() = 0;
-    virtual LoadFeeTrack&
-    getFeeTrack() = 0;
-    virtual LoadManager&
-    getLoadManager() = 0;
-    virtual Overlay&
-    overlay() = 0;
-    virtual TxQ&
-    getTxQ() = 0;
-    virtual ValidatorList&
-    validators() = 0;
-    virtual ValidatorSite&
-    validatorSites() = 0;
-    virtual ManifestCache&
-    validatorManifests() = 0;
-    virtual ManifestCache&
-    publisherManifests() = 0;
-    virtual Cluster&
-    cluster() = 0;
-    virtual PeerReservationTable&
-    peerReservations() = 0;
-    virtual RCLValidations&
-    getValidations() = 0;
-    virtual NodeStore::Database&
-    getNodeStore() = 0;
-    virtual InboundLedgers&
-    getInboundLedgers() = 0;
-    virtual InboundTransactions&
-    getInboundTransactions() = 0;
-
-    virtual TaggedCache<uint256, AcceptedLedger>&
-    getAcceptedLedgerCache() = 0;
-
-    virtual LedgerMaster&
-    getLedgerMaster() = 0;
-    virtual LedgerCleaner&
-    getLedgerCleaner() = 0;
-    virtual LedgerReplayer&
-    getLedgerReplayer() = 0;
-    virtual NetworkOPs&
-    getOPs() = 0;
-    virtual OrderBookDB&
-    getOrderBookDB() = 0;
-    virtual ServerHandler&
-    getServerHandler() = 0;
-    virtual TransactionMaster&
-    getMasterTransaction() = 0;
-    virtual perf::PerfLog&
-    getPerfLog() = 0;
-
     virtual std::pair<PublicKey, SecretKey> const&
     nodeIdentity() = 0;
 
     virtual std::optional<PublicKey const>
     getValidationPublicKey() const = 0;
 
-    virtual Resource::Manager&
-    getResourceManager() = 0;
-    virtual PathRequests&
-    getPathRequests() = 0;
-    virtual SHAMapStore&
-    getSHAMapStore() = 0;
-    virtual PendingSaves&
-    pendingSaves() = 0;
-    virtual OpenLedger&
-    openLedger() = 0;
-    virtual OpenLedger const&
-    openLedger() const = 0;
-    virtual RelationalDatabase&
-    getRelationalDatabase() = 0;
-
     virtual std::chrono::milliseconds
     getIOLatency() = 0;
 

src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp

@@ -53,7 +53,17 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)
                 return {nullptr, tecOBJECT_NOT_FOUND};
 
             if (hasExpired(ctx.view, (*offerSLE)[~sfExpiration]))
-                return {nullptr, tecEXPIRED};
+            {
+                // Before fixExpiredNFTokenOfferRemoval amendment, expired
+                // offers caused tecEXPIRED in preclaim, leaving them on ledger
+                // forever. After the amendment, we allow expired offers to
+                // reach doApply() where they get deleted and tecEXPIRED is
+                // returned.
+                if (!ctx.view.rules().enabled(fixExpiredNFTokenOfferRemoval))
+                    return {nullptr, tecEXPIRED};
+                // Amendment enabled: return the expired offer to be handled in
+                // doApply
+            }
 
             if ((*offerSLE)[sfAmount].negative())
                 return {nullptr, temBAD_OFFER};
@@ -299,7 +309,7 @@ NFTokenAcceptOffer::pay(AccountID const& from, AccountID const& to, STAmount con
 {
     // This should never happen, but it's easy and quick to check.
     if (amount < beast::zero)
-        return tecINTERNAL;
+        return tecINTERNAL;  // LCOV_EXCL_LINE
 
     auto const result = accountSend(view(), from, to, amount, j_);
 
@@ -410,6 +420,39 @@ NFTokenAcceptOffer::doApply()
     auto bo = loadToken(ctx_.tx[~sfNFTokenBuyOffer]);
     auto so = loadToken(ctx_.tx[~sfNFTokenSellOffer]);
 
+    // With fixExpiredNFTokenOfferRemoval amendment, check for expired offers
+    // and delete them, returning tecEXPIRED. This ensures expired offers
+    // are properly cleaned up from the ledger.
+    if (view().rules().enabled(fixExpiredNFTokenOfferRemoval))
+    {
+        bool foundExpired = false;
+
+        auto const deleteOfferIfExpired = [this, &foundExpired](std::shared_ptr<SLE> const& offer) -> TER {
+            if (offer && hasExpired(view(), (*offer)[~sfExpiration]))
+            {
+                JLOG(j_.trace()) << "Offer is expired, deleting: " << offer->key();
+                if (!nft::deleteTokenOffer(view(), offer))
+                {
+                    // LCOV_EXCL_START
+                    JLOG(j_.fatal()) << "Unable to delete expired offer '" << offer->key() << "': ignoring";
+                    return tecINTERNAL;
+                    // LCOV_EXCL_STOP
+                }
+                JLOG(j_.trace()) << "Deleted offer " << offer->key();
+                foundExpired = true;
+            }
+            return tesSUCCESS;
+        };
+
+        if (auto const r = deleteOfferIfExpired(bo); !isTesSuccess(r))
+            return r;
+        if (auto const r = deleteOfferIfExpired(so); !isTesSuccess(r))
+            return r;
+
+        if (foundExpired)
+            return tecEXPIRED;
+    }
+
     if (bo && !nft::deleteTokenOffer(view(), bo))
     {
         // LCOV_EXCL_START