Merge remote-tracking branch 'XRPLF/develop' into ximinez/directory

* XRPLF/develop:
  feat: Create new transaction testing framework `TxTest` (6537)
  feat: Add cleanup amendment for 3.2.0 (7037)
  fix: Fix ubsan flagged issues (6151)

include/xrpl/ledger/helpers/NFTokenHelpers.h

@@ -20,6 +20,10 @@ removeTokenOffersWithLimit(
     Keylet const& directory,
     std::size_t maxDeletableOffers);
 
+/** Returns tesSUCCESS if NFToken has few enough offers that it can be burned */
+TER
+notTooManyOffers(ReadView const& view, uint256 const& nftokenID);
+
 /** Finds the specified token in the owner's token directory. */
 std::optional<STObject>
 findToken(ReadView const& view, AccountID const& owner, uint256 const& nftokenID);

include/xrpl/protocol/detail/features.macro

@@ -15,10 +15,12 @@
 // Add new amendments to the top of this list.
 // Keep it sorted in reverse chronological order.
 
+XRPL_FEATURE(DefragDirectories,          Supported::no,  VoteBehavior::DefaultNo)
 XRPL_FIX    (Cleanup3_2_0,                Supported::no, VoteBehavior::DefaultNo)
 XRPL_FEATURE(MPTokensV2,                  Supported::no, VoteBehavior::DefaultNo)
 XRPL_FIX    (Security3_1_3,               Supported::no, VoteBehavior::DefaultNo)
 XRPL_FIX    (PermissionedDomainInvariant, Supported::yes, VoteBehavior::DefaultNo)
+XRPL_FIX    (ExpiredNFTokenOfferRemoval,  Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FIX    (BatchInnerSigs,              Supported::no, VoteBehavior::DefaultNo)
 XRPL_FEATURE(LendingProtocol,             Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(PermissionDelegationV1_1,    Supported::no,  VoteBehavior::DefaultNo)

src/libxrpl/beast/core/CurrentThreadName.cpp

@@ -81,9 +81,12 @@ setCurrentThreadNameImpl(std::string_view name)
 {
     // truncate and set the thread name.
     char boundedName[maxThreadNameLength + 1];
-    auto const boundedSize = name.size() < maxThreadNameLength ? name.size() : maxThreadNameLength;
-    name.copy(boundedName, boundedSize);
-    boundedName[boundedSize] = '\0';
+    std::snprintf(
+        boundedName,
+        sizeof(boundedName),
+        "%.*s",
+        static_cast<int>(maxThreadNameLength),
+        name.data());  // NOLINT(bugprone-suspicious-stringview-data-usage)
 
     pthread_setname_np(pthread_self(), boundedName);
 

src/libxrpl/ledger/ApplyView.cpp

@@ -26,6 +26,14 @@ namespace xrpl {
 
 namespace directory {
 
+struct Gap
+{
+    uint64_t const page;
+    SLE::pointer node;
+    uint64_t const nextPage;
+    SLE::pointer next;
+};
+
 std::uint64_t
 createRoot(
     ApplyView& view,
@@ -126,7 +134,9 @@ insertPage(
     if (page == 0)
         return std::nullopt;
     if (!view.rules().enabled(fixDirectoryLimit) && page >= dirNodeMaxPages)  // Old pages limit
+    {
         return std::nullopt;
+    }
 
     // We are about to create a new node; we'll link it to
     // the chain first:
@@ -147,12 +157,8 @@ insertPage(
     // Save some space by not specifying the value 0 since it's the default.
     if (page != 1)
         node->setFieldU64(sfIndexPrevious, page - 1);
-    XRPL_ASSERT_PARTS(!nextPage, "xrpl::directory::insertPage", "nextPage has default value");
-    /* Reserved for future use when directory pages may be inserted in
-     * between two other pages instead of only at the end of the chain.
     if (nextPage)
         node->setFieldU64(sfIndexNext, nextPage);
-    */
     describe(node);
     view.insert(node);
 
@@ -168,7 +174,7 @@ ApplyView::dirAdd(
     uint256 const& key,
     std::function<void(std::shared_ptr<SLE> const&)> const& describe)
 {
-    auto root = peek(directory);
+    auto const root = peek(directory);
 
     if (!root)
     {
@@ -178,6 +184,43 @@ ApplyView::dirAdd(
 
     auto [page, node, indexes] = directory::findPreviousPage(*this, directory, root);
 
+    if (rules().enabled(featureDefragDirectories))
+    {
+        // If there are more nodes than just the root, and there's no space in
+        // the last one, walk backwards to find one with space, or to find one
+        // missing.
+        std::optional<directory::Gap> gapPages;
+        while (page && indexes.size() >= dirNodeMaxEntries)
+        {
+            // Find a page with space, or a gap in pages.
+            auto [prevPage, prevNode, prevIndexes] =
+                directory::findPreviousPage(*this, directory, node);
+            if (!gapPages && prevPage != page - 1)
+                gapPages.emplace(prevPage, prevNode, page, node);
+            page = prevPage;
+            node = prevNode;
+            indexes = prevIndexes;
+        }
+        // We looped through all the pages back to the root.
+        if (!page)
+        {
+            // If we found a gap, use it.
+            if (gapPages)
+            {
+                return directory::insertPage(
+                    *this,
+                    gapPages->page,
+                    gapPages->node,
+                    gapPages->nextPage,
+                    gapPages->next,
+                    key,
+                    directory,
+                    describe);
+            }
+            std::tie(page, node, indexes) = directory::findPreviousPage(*this, directory, root);
+        }
+    }
+
     // If there's space, we use it:
     if (indexes.size() < dirNodeMaxEntries)
     {

src/libxrpl/ledger/helpers/NFTokenHelpers.cpp

@@ -621,6 +621,33 @@ removeTokenOffersWithLimit(ApplyView& view, Keylet const& directory, std::size_t
     return deletedOffersCount;
 }
 
+TER
+notTooManyOffers(ReadView const& view, uint256 const& nftokenID)
+{
+    std::size_t totalOffers = 0;
+
+    {
+        Dir const buys(view, keylet::nft_buys(nftokenID));
+        for (auto iter = buys.begin(); iter != buys.end(); iter.next_page())
+        {
+            totalOffers += iter.page_size();
+            if (totalOffers > maxDeletableTokenOfferEntries)
+                return tefTOO_BIG;
+        }
+    }
+
+    {
+        Dir const sells(view, keylet::nft_sells(nftokenID));
+        for (auto iter = sells.begin(); iter != sells.end(); iter.next_page())
+        {
+            totalOffers += iter.page_size();
+            if (totalOffers > maxDeletableTokenOfferEntries)
+                return tefTOO_BIG;
+        }
+    }
+    return tesSUCCESS;
+}
+
 bool
 deleteTokenOffer(ApplyView& view, std::shared_ptr<SLE> const& offer)
 {

src/libxrpl/tx/transactors/nft/NFTokenAcceptOffer.cpp

@@ -68,12 +68,15 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)
 
             if (hasExpired(ctx.view, (*offerSLE)[~sfExpiration]))
             {
-                // Before fixSecurity3_1_3 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(fixSecurity3_1_3))
+                // 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.
+                // Amendment enabled: return the expired offer to be handled in
+                // doApply
             }
 
             if ((*offerSLE)[sfAmount].negative())
@@ -447,9 +450,10 @@ NFTokenAcceptOffer::doApply()
     auto bo = loadToken(ctx_.tx[~sfNFTokenBuyOffer]);
     auto so = loadToken(ctx_.tx[~sfNFTokenSellOffer]);
 
-    // With fixSecurity3_1_3 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(fixSecurity3_1_3))
+    // 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;
 

src/test/app/NFToken_test.cpp

@@ -1096,10 +1096,10 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
 
         // The buy offer must not have expired.
         // NOTE: this is only a preclaim check with the
-        // fixSecurity3_1_3 amendment disabled.
+        // fixExpiredNFTokenOfferRemoval amendment disabled.
         env(token::acceptBuyOffer(alice, buyerExpOfferIndex), ter(tecEXPIRED));
         env.close();
-        if (features[fixSecurity3_1_3])
+        if (features[fixExpiredNFTokenOfferRemoval])
         {
             buyerCount--;
         }
@@ -1117,12 +1117,12 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
 
         // The sell offer must not have expired.
         // NOTE: this is only a preclaim check with the
-        // fixSecurity3_1_3 amendment disabled.
+        // fixExpiredNFTokenOfferRemoval amendment disabled.
         env(token::acceptSellOffer(buyer, aliceExpOfferIndex), ter(tecEXPIRED));
         env.close();
         // Alice's count is decremented by one when the expired offer is
         // removed.
-        if (features[fixSecurity3_1_3])
+        if (features[fixExpiredNFTokenOfferRemoval])
         {
             aliceCount--;
         }
@@ -3101,10 +3101,10 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             // No one can accept an expired sell offer.
             env(token::acceptSellOffer(buyer, offer1), ter(tecEXPIRED));
 
-            // With fixSecurity3_1_3 amendment, the first accept
+            // 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[fixSecurity3_1_3])
+            if (features[fixExpiredNFTokenOfferRemoval])
             {
                 // After amendment: offer was deleted by first accept attempt
                 minterCount--;
@@ -3123,7 +3123,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             BEAST_EXPECT(ownerCount(env, minter) == minterCount);
             BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
-            if (!features[fixSecurity3_1_3])
+            if (!features[fixExpiredNFTokenOfferRemoval])
             {
                 // Before amendment: expired offer still exists and needs to be
                 // cancelled
@@ -3189,10 +3189,10 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             // An expired buy offer cannot be accepted.
             env(token::acceptBuyOffer(minter, offer1), ter(tecEXPIRED));
 
-            // With fixSecurity3_1_3 amendment, the first accept
+            // 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[fixSecurity3_1_3])
+            if (features[fixExpiredNFTokenOfferRemoval])
             {
                 // After amendment: offer was deleted by first accept attempt
                 buyerCount--;
@@ -3211,7 +3211,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             BEAST_EXPECT(ownerCount(env, minter) == minterCount);
             BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
-            if (!features[fixSecurity3_1_3])
+            if (!features[fixExpiredNFTokenOfferRemoval])
             {
                 // Before amendment: expired offer still exists and can be
                 // cancelled
@@ -3288,7 +3288,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             env(token::brokerOffers(issuer, buyOffer1, sellOffer1), ter(tecEXPIRED));
             env.close();
 
-            if (features[fixSecurity3_1_3])
+            if (features[fixExpiredNFTokenOfferRemoval])
             {
                 // With amendment: expired offers are deleted
                 minterCount--;
@@ -3298,7 +3298,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             BEAST_EXPECT(ownerCount(env, minter) == minterCount);
             BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
-            if (features[fixSecurity3_1_3])
+            if (features[fixExpiredNFTokenOfferRemoval])
             {
                 // The buy offer was deleted, so no need to cancel it
                 // The sell offer still exists, so we can cancel it
@@ -3377,7 +3377,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             env.close();
 
             BEAST_EXPECT(ownerCount(env, issuer) == 0);
-            if (features[fixSecurity3_1_3])
+            if (features[fixExpiredNFTokenOfferRemoval])
             {
                 // After amendment: expired offers were deleted during broker
                 // attempt
@@ -3463,7 +3463,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
 
             // The expired offers are still in the ledger.
             BEAST_EXPECT(ownerCount(env, issuer) == 0);
-            if (!features[fixSecurity3_1_3])
+            if (!features[fixExpiredNFTokenOfferRemoval])
             {
                 // Before amendment: expired offers still exist in ledger
                 BEAST_EXPECT(ownerCount(env, minter) == 2);
@@ -7190,7 +7190,7 @@ public:
     {
         testWithFeats(
             allFeatures - fixNFTokenReserve - featureNFTokenMintOffer - featureDynamicNFT -
-            fixSecurity3_1_3);
+            fixExpiredNFTokenOfferRemoval);
     }
 };
 
@@ -7227,7 +7227,7 @@ class NFTokenWOExpiredOfferRemoval_test : public NFTokenBaseUtil_test
     void
     run() override
     {
-        testWithFeats(allFeatures - fixSecurity3_1_3);
+        testWithFeats(allFeatures - fixExpiredNFTokenOfferRemoval);
     }
 };
 

src/test/rpc/LedgerEntry_test.cpp

@@ -1201,7 +1201,7 @@ class LedgerEntry_test : public beast::unit_test::suite
             checkErrorValue(
                 jrr[jss::result],
                 "malformedAuthorizedCredentials",
-                "Invalid field 'authorized_credentials', not array of objects.");
+                "Invalid field 'authorized_credentials', not array.");
         }
 
         {
@@ -1219,7 +1219,7 @@ class LedgerEntry_test : public beast::unit_test::suite
             checkErrorValue(
                 jrr[jss::result],
                 "malformedAuthorizedCredentials",
-                "Invalid field 'authorized_credentials', not array of objects.");
+                "Invalid field 'authorized_credentials', not array.");
         }
 
         {

src/xrpld/overlay/detail/PeerImp.cpp

@@ -2639,7 +2639,7 @@ PeerImp::onMessage(std::shared_ptr<protocol::TMGetObjectByHash> const& m)
                     {
                         fee_.update(
                             Resource::feeModerateBurdenPeer,
-                            "Reply limit reached. Truncating reply.");
+                            " Reply limit reached. Truncating reply.");
                         break;
                     }
                 }

src/xrpld/rpc/handlers/ledger/LedgerEntry.cpp

@@ -267,7 +267,7 @@ parseAuthorizeCredentials(Json::Value const& jv)
         if (!jo.isObject())
         {
             return LedgerEntryHelpers::invalidFieldError(
-                "malformedAuthorizedCredentials", jss::authorized_credentials, "array of objects");
+                "malformedAuthorizedCredentials", jss::authorized_credentials, "array");
         }
 
         if (auto const value = LedgerEntryHelpers::hasRequired(