Add fixExpiredNFTokenOfferRemoval amendment and implementation

Co-authored-by: mvadari <8029314+mvadari@users.noreply.github.com>

include/xrpl/protocol/detail/features.macro

@@ -32,6 +32,7 @@
 // If you add an amendment here, then do not forget to increment `numFeatures`
 // in include/xrpl/protocol/Feature.h.
 
+XRPL_FIX    (ExpiredNFTokenOfferRemoval, Supported::no,  VoteBehavior::DefaultNo)
 XRPL_FIX    (IncludeKeyletFields,        Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(DynamicMPT,                 Supported::no,  VoteBehavior::DefaultNo)
 XRPL_FIX    (TokenEscrowV1,              Supported::yes, VoteBehavior::DefaultNo)

src/test/app/NFToken_test.cpp

@@ -5975,6 +5975,113 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
         }
     }
 
+    void
+    testFixExpiredNFTokenOfferRemoval(FeatureBitset features)
+    {
+        // Test that fixExpiredNFTokenOfferRemoval amendment properly deletes
+        // expired offers when NFTokenAcceptOffer is attempted.
+        testcase("fixExpiredNFTokenOfferRemoval");
+
+        using namespace test::jtx;
+
+        Account const issuer{"issuer"};
+        Account const buyer{"buyer"};
+
+        // Test with and without the amendment
+        for (auto const& tweakedFeatures : {
+                 features - fixExpiredNFTokenOfferRemoval,
+                 features | fixExpiredNFTokenOfferRemoval})
+        {
+            bool const amendmentEnabled = 
+                tweakedFeatures.count(fixExpiredNFTokenOfferRemoval) > 0;
+
+            Env env{*this, tweakedFeatures};
+
+            env.fund(XRP(1000), issuer, buyer);
+            env.close();
+
+            // Create an NFToken
+            uint256 const nftID = token::getNextID(env, issuer, 0, tfTransferable);
+            env(token::mint(issuer, 0), txflags(tfTransferable));
+            env.close();
+
+            // Create an offer with a short expiration time (already expired)
+            std::uint32_t const expiration = lastClose(env) - 1;  // 1 second ago
+            
+            uint256 const sellOfferIndex = keylet::nftoffer(issuer, env.seq(issuer)).key;
+            env(token::createOffer(issuer, nftID, XRP(1)),
+                token::expiration(expiration),
+                txflags(tfSellNFToken));
+            env.close();
+
+            // Before: offer exists, owner count = 2 (NFT + offer)
+            BEAST_EXPECT(ownerCount(env, issuer) == 2);
+
+            // Try to accept the expired offer
+            env(token::acceptSellOffer(buyer, sellOfferIndex), ter(tecEXPIRED));
+            env.close();
+
+            if (amendmentEnabled)
+            {
+                // After amendment: expired offer should be deleted
+                // Owner count should be 1 (only NFT remains)
+                BEAST_EXPECT(ownerCount(env, issuer) == 1);
+                
+                // Verify the offer is actually gone from ledger
+                BEAST_EXPECT(!env.le(keylet::nftoffer(sellOfferIndex)));
+            }
+            else
+            {
+                // Before amendment: expired offer remains on ledger
+                // Owner count should still be 2 (NFT + expired offer)
+                BEAST_EXPECT(ownerCount(env, issuer) == 2);
+                
+                // Verify the offer still exists on ledger
+                BEAST_EXPECT(env.le(keylet::nftoffer(sellOfferIndex)));
+            }
+
+            // Test with buy offer as well
+            uint256 const buyOfferIndex = keylet::nftoffer(buyer, env.seq(buyer)).key;
+            env(token::createOffer(buyer, nftID, XRP(1)),
+                token::owner(issuer),
+                token::expiration(expiration));  // Also expired
+            env.close();
+
+            std::size_t expectedBuyerCount = amendmentEnabled ? 0 : 1;
+            BEAST_EXPECT(ownerCount(env, buyer) == expectedBuyerCount);
+
+            // Try to accept the expired buy offer
+            env(token::acceptBuyOffer(issuer, buyOfferIndex), ter(tecEXPIRED));
+            env.close();
+
+            if (amendmentEnabled)
+            {
+                // After amendment: expired buy offer should be deleted
+                BEAST_EXPECT(ownerCount(env, buyer) == 0);
+                BEAST_EXPECT(!env.le(keylet::nftoffer(buyOfferIndex)));
+            }
+            else
+            {
+                // Before amendment: expired buy offer remains
+                BEAST_EXPECT(ownerCount(env, buyer) == 1);
+                BEAST_EXPECT(env.le(keylet::nftoffer(buyOfferIndex)));
+            }
+
+            // Test brokered case with both offers expired
+            if (!amendmentEnabled)
+            {
+                // Only test if offers still exist (before amendment)
+                env(token::brokerOffers(buyer, buyOfferIndex, sellOfferIndex), 
+                    ter(tecEXPIRED));
+                env.close();
+                
+                // Offers should still exist
+                BEAST_EXPECT(ownerCount(env, issuer) == 2);
+                BEAST_EXPECT(ownerCount(env, buyer) == 1);
+            }
+        }
+    }
+
     void
     testBrokeredSaleToSelf(FeatureBitset features)
     {
@@ -8059,6 +8166,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
         testNFTokenDeleteAccount(features);
         testNftXxxOffers(features);
         testFixNFTokenNegOffer(features);
+        testFixExpiredNFTokenOfferRemoval(features);
         testIOUWithTransferFee(features);
         testBrokeredSaleToSelf(features);
         testFixNFTokenRemint(features);

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

@@ -73,7 +73,15 @@ 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
+            }
 
             // The initial implementation had a bug that allowed a negative
             // amount.  The fixNFTokenNegOffer amendment fixes that.
@@ -521,6 +529,43 @@ 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;
+        
+        if (bo && hasExpired(view(), (*bo)[~sfExpiration]))
+        {
+            JLOG(j_.trace()) << "Buy offer is expired, deleting: " << bo->getIndex();
+            if (!nft::deleteTokenOffer(view(), bo))
+            {
+                JLOG(j_.fatal()) << "Unable to delete expired buy offer '"
+                                 << to_string(bo->key()) << "': ignoring";
+                return tecINTERNAL;
+            }
+            foundExpired = true;
+            bo.reset(); // Clear the pointer since offer is deleted
+        }
+        
+        if (so && hasExpired(view(), (*so)[~sfExpiration]))
+        {
+            JLOG(j_.trace()) << "Sell offer is expired, deleting: " << so->getIndex();
+            if (!nft::deleteTokenOffer(view(), so))
+            {
+                JLOG(j_.fatal()) << "Unable to delete expired sell offer '"
+                                 << to_string(so->key()) << "': ignoring";
+                return tecINTERNAL;
+            }
+            foundExpired = true;
+            so.reset(); // Clear the pointer since offer is deleted
+        }
+        
+        if (foundExpired)
+            return tecEXPIRED;
+    }
+
     if (bo && !nft::deleteTokenOffer(view(), bo))
     {
         JLOG(j_.fatal()) << "Unable to delete buy offer '"