Include Keylet.h in the SLEBase test

(cherry picked from commit 3cca9d3266)

include/xrpl/ledger/entries/NFTokenOfferEntry.h

@@ -38,6 +38,18 @@ public:
         : Base(keylet::nftokenOffer(offerID), view, j)
     {
     }
+
+    /**
+     * Deletes this offer from the ledger: unlinks it from the owner
+     * directory and from the token's buy/sell directory, and decreases the
+     * owner's reserve count.
+     *
+     * @return false if the entry does not exist or either directory unlink
+     *         fails; true on success.
+     */
+    bool
+    removeFromLedger()
+        requires Base::kIsWritable;
 };
 
 using NFTokenOfferEntryR = NFTokenOfferEntry<ReadView>;

include/xrpl/ledger/helpers/NFTokenHelpers.h

@@ -70,19 +70,6 @@ removeToken(ApplyView& view, AccountID const& owner, UInt256 const& nftokenID);
 TER
 removeToken(ApplyView& view, AccountID const& owner, UInt256 const& nftokenID, SLE::Ref page);
 
-/**
- * Deletes the given token offer.
- *
- * An offer is tracked in two separate places:
- *     - The token's 'buy' directory, if it's a buy offer; or
- *     - The token's 'sell' directory, if it's a sell offer; and
- *     - The owner directory of the account that placed the offer.
- *
- * The offer also consumes one incremental reserve.
- */
-bool
-deleteTokenOffer(ApplyView& view, SLE::Ref offer);
-
 /**
  * Repairs the links in an NFTokenPage directory.
  *

include/xrpl/tx/transactors/nft/NFTokenAcceptOffer.h

@@ -4,6 +4,7 @@
 #include <xrpl/beast/utility/Journal.h>
 #include <xrpl/core/ServiceRegistry.h>
 #include <xrpl/ledger/ReadView.h>
+#include <xrpl/ledger/entries/NFTokenOfferEntry.h>
 #include <xrpl/protocol/AccountID.h>
 #include <xrpl/protocol/STAmount.h>
 #include <xrpl/protocol/STTx.h>
@@ -21,10 +22,10 @@ private:
     pay(AccountID const& from, AccountID const& to, STAmount const& amount);
 
     TER
-    acceptOffer(SLE::Ref offer);
+    acceptOffer(NFTokenOfferEntryR const& offer);
 
     TER
-    bridgeOffers(SLE::Ref buy, SLE::Ref sell);
+    bridgeOffers(NFTokenOfferEntryR const& buy, NFTokenOfferEntryR const& sell);
 
     TER
     transferNFToken(AccountID const& buyer, AccountID const& seller, UInt256 const& nfTokenID);

src/libxrpl/ledger/entries/NFTokenOfferEntry.cpp

@@ -0,0 +1,45 @@
+#include <xrpl/ledger/entries/NFTokenOfferEntry.h>
+
+#include <xrpl/ledger/ApplyView.h>
+#include <xrpl/ledger/ReadView.h>
+#include <xrpl/ledger/helpers/AccountRootHelpers.h>
+#include <xrpl/protocol/Indexes.h>
+#include <xrpl/protocol/LedgerFormats.h>
+#include <xrpl/protocol/SField.h>
+
+namespace xrpl {
+
+template <typename ViewT>
+bool
+NFTokenOfferEntry<ViewT>::removeFromLedger()
+    requires Base::kIsWritable
+{
+    if (!this->exists())
+        return false;
+
+    auto const sle = this->rawSle();
+    auto const owner = (*sle)[sfOwner];
+
+    if (!this->applyView().dirRemove(
+            keylet::ownerDir(owner), (*sle)[sfOwnerNode], sle->key(), false))
+        return false;
+
+    auto const nftokenID = (*sle)[sfNFTokenID];
+
+    if (!this->applyView().dirRemove(
+            sle->isFlag(lsfSellNFToken) ? keylet::nftSells(nftokenID) : keylet::nftBuys(nftokenID),
+            (*sle)[sfNFTokenOfferNode],
+            sle->key(),
+            false))
+        return false;
+
+    decreaseOwnerCount(this->applyView(), owner, {}, 1, this->journal());
+
+    this->erase();
+    return true;
+}
+
+template class NFTokenOfferEntry<ReadView>;
+template class NFTokenOfferEntry<ApplyView>;
+
+}  // namespace xrpl

src/libxrpl/ledger/helpers/NFTokenHelpers.cpp

@@ -7,6 +7,7 @@
 #include <xrpl/beast/utility/instrumentation.h>
 #include <xrpl/ledger/ApplyView.h>
 #include <xrpl/ledger/ReadView.h>
+#include <xrpl/ledger/entries/NFTokenOfferEntry.h>
 #include <xrpl/ledger/helpers/AccountRootHelpers.h>
 #include <xrpl/ledger/helpers/DirectoryHelpers.h>
 #include <xrpl/ledger/helpers/RippleStateHelpers.h>
@@ -591,9 +592,9 @@ removeTokenOffersWithLimit(ApplyView& view, Keylet const& directory, std::size_t
         // deleting during iteration.
         for (int i = offerIndexes.size() - 1; i >= 0; --i)
         {
-            if (auto const offer = view.peek(keylet::nftokenOffer(offerIndexes[i])))
+            if (NFTokenOfferEntryW offer(offerIndexes[i], view); offer)
             {
-                if (deleteTokenOffer(view, offer))
+                if (offer.removeFromLedger())
                 {
                     ++deletedOffersCount;
                 }
@@ -612,33 +613,6 @@ removeTokenOffersWithLimit(ApplyView& view, Keylet const& directory, std::size_t
     return deletedOffersCount;
 }
 
-bool
-deleteTokenOffer(ApplyView& view, SLE::Ref offer)
-{
-    if (offer->getType() != ltNFTOKEN_OFFER)
-        return false;
-
-    auto const owner = (*offer)[sfOwner];
-
-    if (!view.dirRemove(keylet::ownerDir(owner), (*offer)[sfOwnerNode], offer->key(), false))
-        return false;
-
-    auto const nftokenID = (*offer)[sfNFTokenID];
-
-    if (!view.dirRemove(
-            offer->isFlag(lsfSellNFToken) ? keylet::nftSells(nftokenID)
-                                          : keylet::nftBuys(nftokenID),
-            (*offer)[sfNFTokenOfferNode],
-            offer->key(),
-            false))
-        return false;
-
-    decreaseOwnerCount(view, owner, {}, 1, beast::Journal{beast::Journal::getNullSink()});
-
-    view.erase(offer);
-    return true;
-}
-
 bool
 repairNFTokenDirectoryLinks(ApplyView& view, AccountID const& owner)
 {
@@ -942,7 +916,8 @@ tokenOfferCreateApply(
         priorBalance < accountReserve(view, acct, j, {.ownerCountDelta = 1}))
         return tecINSUFFICIENT_RESERVE;
 
-    auto const offerID = keylet::nftokenOffer(acctID, seqProxy);
+    NFTokenOfferEntryW offer(acctID, seqProxy, view, j);
+    Keylet const offerID = offer.keylet();
 
     // Create the offer:
     {
@@ -973,7 +948,7 @@ tokenOfferCreateApply(
         if (isSellOffer)
             sleFlags |= lsfSellNFToken;
 
-        auto offer = std::make_shared<SLE>(offerID);
+        offer.newSLE();
         (*offer)[sfOwner] = acctID;
         (*offer)[sfNFTokenID] = nftokenID;
         (*offer)[sfAmount] = amount;
@@ -987,7 +962,7 @@ tokenOfferCreateApply(
         if (dest)
             (*offer)[sfDestination] = *dest;
 
-        view.insert(offer);
+        offer.insert();
     }
 
     // Update owner count.

src/libxrpl/tx/Transactor.cpp

@@ -11,10 +11,10 @@
 #include <xrpl/json/to_string.h>  // IWYU pragma: keep
 #include <xrpl/ledger/ApplyView.h>
 #include <xrpl/ledger/ReadView.h>
+#include <xrpl/ledger/entries/NFTokenOfferEntry.h>
 #include <xrpl/ledger/helpers/AccountRootHelpers.h>
 #include <xrpl/ledger/helpers/CredentialHelpers.h>
 #include <xrpl/ledger/helpers/DelegateHelpers.h>
-#include <xrpl/ledger/helpers/NFTokenHelpers.h>
 #include <xrpl/ledger/helpers/OfferHelpers.h>
 #include <xrpl/ledger/helpers/RippleStateHelpers.h>
 #include <xrpl/ledger/helpers/SponsorHelpers.h>
@@ -1236,9 +1236,9 @@ removeExpiredNFTokenOffers(
 
     for (auto const& index : offers)
     {
-        if (auto const offer = view.peek(keylet::nftokenOffer(index)))
+        if (NFTokenOfferEntryW offer(index, view, viewJ); offer)
         {
-            nft::deleteTokenOffer(view, offer);
+            offer.removeFromLedger();
             if (++removed == kExpiredOfferRemoveLimit)
                 return;
         }

src/libxrpl/tx/transactors/account/AccountDelete.cpp

@@ -9,9 +9,9 @@
 #include <xrpl/ledger/ApplyView.h>
 #include <xrpl/ledger/ReadView.h>
 #include <xrpl/ledger/View.h>
+#include <xrpl/ledger/entries/NFTokenOfferEntry.h>
 #include <xrpl/ledger/helpers/CredentialHelpers.h>
 #include <xrpl/ledger/helpers/DirectoryHelpers.h>
-#include <xrpl/ledger/helpers/NFTokenHelpers.h>
 #include <xrpl/ledger/helpers/OfferHelpers.h>
 #include <xrpl/protocol/AccountID.h>
 #include <xrpl/protocol/Feature.h>
@@ -128,10 +128,11 @@ removeNFTokenOfferFromLedger(
     ApplyView& view,
     AccountID const& account,
     UInt256 const& delIndex,
-    SLE::Ref sleDel,
-    beast::Journal)
+    SLE::Ref,
+    beast::Journal j)
 {
-    if (!nft::deleteTokenOffer(view, sleDel))
+    NFTokenOfferEntryW offer(Keylet(ltNFTOKEN_OFFER, delIndex), view, j);
+    if (!offer.removeFromLedger())
         return tefBAD_LEDGER;  // LCOV_EXCL_LINE
 
     return tesSUCCESS;

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

@@ -5,6 +5,7 @@
 #include <xrpl/beast/utility/Zero.h>
 #include <xrpl/core/ServiceRegistry.h>
 #include <xrpl/ledger/View.h>
+#include <xrpl/ledger/entries/NFTokenOfferEntry.h>
 #include <xrpl/ledger/helpers/AccountRootHelpers.h>
 #include <xrpl/ledger/helpers/NFTokenHelpers.h>
 #include <xrpl/ledger/helpers/TokenHelpers.h>
@@ -64,16 +65,17 @@ TER
 NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)
 {
     auto const checkOffer =
-        [&ctx](std::optional<UInt256> id) -> std::pair<SLE::const_pointer, TER> {
+        [&ctx](std::optional<UInt256> id) -> std::pair<NFTokenOfferEntryR, TER> {
+        NFTokenOfferEntryR const none(SLE::const_pointer{}, ctx.view);
         if (id)
         {
             if (id->isZero())
-                return {nullptr, tecOBJECT_NOT_FOUND};
+                return {none, tecOBJECT_NOT_FOUND};
 
-            auto offerSLE = ctx.view.read(keylet::nftokenOffer(*id));
+            NFTokenOfferEntryR offerSLE(*id, ctx.view);
 
             if (!offerSLE)
-                return {nullptr, tecOBJECT_NOT_FOUND};
+                return {none, tecOBJECT_NOT_FOUND};
 
             if (hasExpired(ctx.view, (*offerSLE)[~sfExpiration]))
             {
@@ -81,16 +83,16 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)
                 // 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(fixCleanup3_1_3))
-                    return {nullptr, tecEXPIRED};
+                    return {none, tecEXPIRED};
                 // Amendment enabled: return the expired offer to be handled in doApply.
             }
 
             if ((*offerSLE)[sfAmount].negative())
-                return {nullptr, temBAD_OFFER};
+                return {none, temBAD_OFFER};
 
             return {std::move(offerSLE), tesSUCCESS};
         }
-        return {nullptr, tesSUCCESS};
+        return {none, tesSUCCESS};
     };
 
     auto const [bo, err1] = checkOffer(ctx.tx[~sfNFTokenBuyOffer]);
@@ -405,7 +407,7 @@ NFTokenAcceptOffer::transferNFToken(
 }
 
 TER
-NFTokenAcceptOffer::acceptOffer(SLE::Ref offer)
+NFTokenAcceptOffer::acceptOffer(NFTokenOfferEntryR const& offer)
 {
     bool const isSell = offer->isFlag(lsfSellNFToken);
     AccountID const owner = (*offer)[sfOwner];
@@ -443,10 +445,14 @@ TER
 NFTokenAcceptOffer::doApply()
 {
     auto const loadToken = [this](std::optional<UInt256> const& id) {
-        SLE::pointer sle;
+        std::optional<NFTokenOfferEntryW> offer;
         if (id)
-            sle = view().peek(keylet::nftokenOffer(*id));
-        return sle;
+        {
+            offer.emplace(*id, view(), j_);
+            if (!*offer)
+                offer.reset();
+        }
+        return offer;
     };
 
     auto bo = loadToken(ctx_.tx[~sfNFTokenBuyOffer]);
@@ -458,11 +464,12 @@ NFTokenAcceptOffer::doApply()
     {
         bool foundExpired = false;
 
-        auto const deleteOfferIfExpired = [this, &foundExpired](SLE::Ref offer) -> TER {
-            if (offer && hasExpired(view(), (*offer)[~sfExpiration]))
+        auto const deleteOfferIfExpired =
+            [this, &foundExpired](std::optional<NFTokenOfferEntryW>& offer) -> TER {
+            if (offer && hasExpired(view(), (*offer)->at(~sfExpiration)))
             {
                 JLOG(j_.trace()) << "Offer is expired, deleting: " << offer->key();
-                if (!nft::deleteTokenOffer(view(), offer))
+                if (!offer->removeFromLedger())
                 {
                     // LCOV_EXCL_START
                     JLOG(j_.fatal())
@@ -485,7 +492,12 @@ NFTokenAcceptOffer::doApply()
             return tecEXPIRED;
     }
 
-    if (bo && !nft::deleteTokenOffer(view(), bo))
+    // removeFromLedger() erases the writable entries, but the offers' fields
+    // are still read below: keep read-only handles to the same SLEs.
+    std::optional<NFTokenOfferEntryR> const buyOffer(bo);
+    std::optional<NFTokenOfferEntryR> const sellOffer(so);
+
+    if (bo && !bo->removeFromLedger())
     {
         // LCOV_EXCL_START
         JLOG(j_.fatal()) << "Unable to delete buy offer '" << to_string(bo->key()) << "': ignoring";
@@ -493,7 +505,7 @@ NFTokenAcceptOffer::doApply()
         // LCOV_EXCL_STOP
     }
 
-    if (so && !nft::deleteTokenOffer(view(), so))
+    if (so && !so->removeFromLedger())
     {
         // LCOV_EXCL_START
         JLOG(j_.fatal()) << "Unable to delete sell offer '" << to_string(so->key())
@@ -503,15 +515,15 @@ NFTokenAcceptOffer::doApply()
     }
 
     // Bridging two different offers
-    if (bo && so)
+    if (buyOffer && sellOffer)
     {
-        AccountID const buyer = (*bo)[sfOwner];
-        AccountID const seller = (*so)[sfOwner];
+        AccountID const buyer = (*buyOffer)->at(sfOwner);
+        AccountID const seller = (*sellOffer)->at(sfOwner);
 
-        auto const nftokenID = (*so)[sfNFTokenID];
+        auto const nftokenID = (*sellOffer)->at(sfNFTokenID);
 
         // The amount is what the buyer of the NFT pays:
-        STAmount amount = (*bo)[sfAmount];
+        STAmount amount = (*buyOffer)->at(sfAmount);
 
         // Three different folks may be paid.  The order of operations is
         // important.
@@ -560,11 +572,11 @@ NFTokenAcceptOffer::doApply()
         return transferNFToken(buyer, seller, nftokenID);
     }
 
-    if (bo)
-        return acceptOffer(bo);
+    if (buyOffer)
+        return acceptOffer(*buyOffer);
 
-    if (so)
-        return acceptOffer(so);
+    if (sellOffer)
+        return acceptOffer(*sellOffer);
 
     return tecINTERNAL;  // LCOV_EXCL_LINE
 }

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

@@ -3,7 +3,7 @@
 #include <xrpl/basics/Log.h>
 #include <xrpl/basics/base_uint.h>
 #include <xrpl/ledger/View.h>
-#include <xrpl/ledger/helpers/NFTokenHelpers.h>
+#include <xrpl/ledger/entries/NFTokenOfferEntry.h>
 #include <xrpl/protocol/Feature.h>
 #include <xrpl/protocol/Indexes.h>
 #include <xrpl/protocol/LedgerFormats.h>
@@ -88,8 +88,7 @@ NFTokenCancelOffer::doApply()
 {
     for (auto const& id : ctx_.tx[sfNFTokenOffers])
     {
-        if (auto offer = view().peek(keylet::nftokenOffer(id));
-            offer && !nft::deleteTokenOffer(view(), offer))
+        if (NFTokenOfferEntryW offer(id, view(), j_); offer && !offer.removeFromLedger())
         {
             // LCOV_EXCL_START
             JLOG(j_.fatal()) << "Unable to delete token offer " << id << " (ledger " << view().seq()

src/tests/libxrpl/ledger/SLEBase.cpp

@@ -38,6 +38,7 @@
 #include <xrpl/ledger/entries/XChainOwnedClaimIDEntry.h>               // IWYU pragma: keep
 #include <xrpl/ledger/entries/XChainOwnedCreateAccountClaimIDEntry.h>  // IWYU pragma: keep
 #include <xrpl/protocol/Indexes.h>
+#include <xrpl/protocol/Keylet.h>
 #include <xrpl/protocol/LedgerFormats.h>
 #include <xrpl/protocol/SField.h>
 #include <xrpl/protocol/STLedgerEntry.h>
@@ -76,12 +77,22 @@ template class SLEBase<ApplyView>;
 #pragma push_macro("LEDGER_ENTRY")
 #undef LEDGER_ENTRY
 
+// No explicit instantiation here, neither a definition nor a declaration.
+// A definition would instantiate an entry a second time once its member
+// bodies live in its own .cpp (MSVC rejects that with C4661 under /WX), and a
+// declaration (`extern template`) stops clang from emitting the inline
+// constructors of the header-only entries at -O0, which leaves them undefined
+// at link time in the debug builds. Naming the class in a static_assert is
+// enough to instantiate its definition, which is all this exhaustiveness
+// check needs; the per-type suites exercise the members.
 #define LEDGER_ENTRY(tag, value, name, ...)                                                    \
-    template class name##Entry<ReadView>;                                                      \
-    template class name##Entry<ApplyView>;                                                     \
     static_assert(                                                                             \
         name##Entry<ReadView>::kEntryType == tag && name##Entry<ApplyView>::kEntryType == tag, \
-        #name "Entry must be bound to " #tag);
+        #name "Entry must be bound to " #tag);                                                 \
+    static_assert(                                                                             \
+        std::is_constructible_v<name##Entry<ReadView>, Keylet const&, ReadView const&> &&      \
+            std::is_constructible_v<name##Entry<ApplyView>, Keylet const&, ApplyView&>,        \
+        #name "Entry must be constructible from a Keylet and a view");
 
 #include <xrpl/protocol/detail/ledger_entries.macro>