Document reserveCount parameters and return value

reserveCount() had no params to document but was missing an @return
tag describing what the reserve count means.

include/xrpl/ledger/entries/OracleEntry.h

@@ -4,9 +4,13 @@
 #include <xrpl/ledger/ApplyView.h>
 #include <xrpl/ledger/ReadView.h>
 #include <xrpl/ledger/entries/SLEBase.h>
+#include <xrpl/ledger/helpers/OracleHelpers.h>
 #include <xrpl/protocol/AccountID.h>
 #include <xrpl/protocol/Indexes.h>
 #include <xrpl/protocol/LedgerFormats.h>
+#include <xrpl/protocol/SField.h>
+#include <xrpl/protocol/STArray.h>  // IWYU pragma: keep
+#include <xrpl/protocol/TER.h>
 
 #include <cstdint>
 
@@ -30,6 +34,33 @@ public:
         : Base(keylet::oracle(account, documentID), view, j)
     {
     }
+
+    /**
+     * Returns the number of owner reserves this Oracle holds, based on the
+     * size of its PriceDataSeries.
+     *
+     * @throws std::logic_error if exists() is false.
+     *
+     * @return The number of owner reserves: kMinOracleReserveCount, or
+     *         kMaxOracleReserveCount if the PriceDataSeries size exceeds
+     *         kOracleReserveCountThreshold.
+     */
+    [[nodiscard]] std::uint32_t
+    reserveCount() const
+    {
+        return calculateOracleReserve((*this)->getFieldArray(sfPriceDataSeries));
+    }
+
+    /**
+     * Removes this Oracle from its owner's directory, releases its owner
+     * reserves, and erases it from the view.
+     *
+     * @param owner The Oracle's owner.
+     * @return tesSUCCESS, or tecINTERNAL / tefBAD_LEDGER on ledger corruption.
+     */
+    [[nodiscard]] TER
+    removeFromLedger(AccountID const& owner)
+        requires Base::kIsWritable;
 };
 
 using OracleEntryR = OracleEntry<ReadView>;

include/xrpl/ledger/helpers/OracleHelpers.h

@@ -1,9 +1,5 @@
 #pragma once
 
-#include <xrpl/protocol/SField.h>
-#include <xrpl/protocol/STArray.h>  // IWYU pragma: keep
-#include <xrpl/protocol/STLedgerEntry.h>
-
 #include <cstddef>
 #include <cstdint>
 
@@ -22,10 +18,4 @@ calculateOracleReserve(T const& priceDataSeries)
                                                                  : kMinOracleReserveCount;
 }
 
-inline std::uint32_t
-calculateOracleReserve(SLE::ConstRef oracleSle)
-{
-    return calculateOracleReserve(oracleSle->getFieldArray(sfPriceDataSeries));
-}
-
 }  // namespace xrpl

include/xrpl/tx/transactors/oracle/OracleDelete.h

@@ -2,9 +2,7 @@
 
 #include <xrpl/beast/utility/Journal.h>
 #include <xrpl/core/ServiceRegistry.h>
-#include <xrpl/ledger/ApplyView.h>
 #include <xrpl/ledger/ReadView.h>
-#include <xrpl/protocol/AccountID.h>
 #include <xrpl/protocol/STTx.h>
 #include <xrpl/protocol/TER.h>
 #include <xrpl/protocol/XRPAmount.h>
@@ -50,9 +48,6 @@ public:
         XRPAmount fee,
         ReadView const& view,
         beast::Journal const& j) override;
-
-    static TER
-    deleteOracle(ApplyView& view, SLE::Ref sle, AccountID const& account, beast::Journal j);
 };
 
 }  // namespace xrpl

src/libxrpl/ledger/entries/OracleEntry.cpp

@@ -0,0 +1,49 @@
+#include <xrpl/ledger/entries/OracleEntry.h>
+
+#include <xrpl/basics/Log.h>
+#include <xrpl/ledger/ApplyView.h>
+#include <xrpl/ledger/ReadView.h>
+#include <xrpl/ledger/helpers/AccountRootHelpers.h>
+#include <xrpl/protocol/AccountID.h>
+#include <xrpl/protocol/Indexes.h>
+#include <xrpl/protocol/SField.h>
+#include <xrpl/protocol/TER.h>
+
+#include <cstdint>
+
+namespace xrpl {
+
+template <typename ViewT>
+TER
+OracleEntry<ViewT>::removeFromLedger(AccountID const& owner)
+    requires Base::kIsWritable
+{
+    if (!this->exists())
+        return tecINTERNAL;  // LCOV_EXCL_LINE
+
+    auto& view = this->applyView();
+    auto const& sle = this->mutableRawSle();
+
+    if (!view.dirRemove(keylet::ownerDir(owner), (*sle)[sfOwnerNode], sle->key(), true))
+    {
+        // LCOV_EXCL_START
+        JLOG(this->journal().fatal()) << "Unable to delete Oracle from owner.";
+        return tefBAD_LEDGER;
+        // LCOV_EXCL_STOP
+    }
+
+    auto const sleOwner = view.peek(keylet::account(owner));
+    if (!sleOwner)
+        return tecINTERNAL;  // LCOV_EXCL_LINE
+
+    std::uint32_t const count = reserveCount();
+    decreaseOwnerCountForObject(view, sleOwner, sle, count, this->journal());
+    this->erase();
+
+    return tesSUCCESS;
+}
+
+template class OracleEntry<ReadView>;
+template class OracleEntry<ApplyView>;
+
+}  // namespace xrpl

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

@@ -9,6 +9,7 @@
 #include <xrpl/ledger/ApplyView.h>
 #include <xrpl/ledger/ReadView.h>
 #include <xrpl/ledger/View.h>
+#include <xrpl/ledger/entries/OracleEntry.h>
 #include <xrpl/ledger/helpers/CredentialHelpers.h>
 #include <xrpl/ledger/helpers/DirectoryHelpers.h>
 #include <xrpl/ledger/helpers/NFTokenHelpers.h>
@@ -28,7 +29,6 @@
 #include <xrpl/tx/transactors/account/SignerListSet.h>
 #include <xrpl/tx/transactors/delegate/DelegateSet.h>
 #include <xrpl/tx/transactors/did/DIDDelete.h>
-#include <xrpl/tx/transactors/oracle/OracleDelete.h>
 #include <xrpl/tx/transactors/payment/DepositPreauth.h>
 
 #include <cstdint>
@@ -154,11 +154,11 @@ removeOracleFromLedger(
     ServiceRegistry&,
     ApplyView& view,
     AccountID const& account,
-    UInt256 const&,
-    SLE::Ref sleDel,
+    UInt256 const& delIndex,
+    SLE::Ref,
     beast::Journal j)
 {
-    return OracleDelete::deleteOracle(view, sleDel, account, j);
+    return OracleEntryW(Keylet(ltORACLE, delIndex), view, j).removeFromLedger(account);
 }
 
 TER

src/libxrpl/tx/transactors/oracle/OracleDelete.cpp

@@ -2,10 +2,7 @@
 
 #include <xrpl/basics/Log.h>
 #include <xrpl/core/ServiceRegistry.h>
-#include <xrpl/ledger/ApplyView.h>
-#include <xrpl/ledger/helpers/AccountRootHelpers.h>
-#include <xrpl/ledger/helpers/OracleHelpers.h>
-#include <xrpl/protocol/AccountID.h>
+#include <xrpl/ledger/entries/OracleEntry.h>
 #include <xrpl/protocol/Indexes.h>
 #include <xrpl/protocol/SField.h>
 #include <xrpl/protocol/STLedgerEntry.h>
@@ -14,8 +11,6 @@
 #include <xrpl/protocol/XRPAmount.h>
 #include <xrpl/tx/Transactor.h>
 
-#include <cstdint>
-
 namespace xrpl {
 
 NotTEC
@@ -31,7 +26,7 @@ OracleDelete::preclaim(PreclaimContext const& ctx)
         return terNO_ACCOUNT;  // LCOV_EXCL_LINE
 
     auto const sle =
-        ctx.view.read(keylet::oracle(ctx.tx.getAccountID(sfAccount), ctx.tx[sfOracleDocumentID]));
+        OracleEntryR(ctx.tx.getAccountID(sfAccount), ctx.tx[sfOracleDocumentID], ctx.view);
     if (!sle)
     {
         JLOG(ctx.j.debug()) << "Oracle Delete: Oracle does not exist.";
@@ -49,40 +44,11 @@ OracleDelete::preclaim(PreclaimContext const& ctx)
     return tesSUCCESS;
 }
 
-TER
-OracleDelete::deleteOracle(
-    ApplyView& view,
-    SLE::Ref sle,
-    AccountID const& account,
-    beast::Journal j)
-{
-    if (!sle)
-        return tecINTERNAL;  // LCOV_EXCL_LINE
-
-    if (!view.dirRemove(keylet::ownerDir(account), (*sle)[sfOwnerNode], sle->key(), true))
-    {
-        // LCOV_EXCL_START
-        JLOG(j.fatal()) << "Unable to delete Oracle from owner.";
-        return tefBAD_LEDGER;
-        // LCOV_EXCL_STOP
-    }
-
-    auto const sleOwner = view.peek(keylet::account(account));
-    if (!sleOwner)
-        return tecINTERNAL;  // LCOV_EXCL_LINE
-
-    std::uint32_t const count = calculateOracleReserve(sle);
-    decreaseOwnerCountForObject(view, sleOwner, sle, count, j);
-    view.erase(sle);
-
-    return tesSUCCESS;
-}
-
 TER
 OracleDelete::doApply()
 {
-    if (auto sle = ctx_.view().peek(keylet::oracle(accountID_, ctx_.tx[sfOracleDocumentID])))
-        return deleteOracle(ctx_.view(), sle, accountID_, j_);
+    if (auto oracle = OracleEntryW(accountID_, ctx_.tx[sfOracleDocumentID], ctx_.view(), j_))
+        return oracle.removeFromLedger(accountID_);
 
     return tecINTERNAL;  // LCOV_EXCL_LINE
 }

src/libxrpl/tx/transactors/oracle/OracleSet.cpp

@@ -3,6 +3,7 @@
 #include <xrpl/basics/chrono.h>
 #include <xrpl/beast/utility/instrumentation.h>
 #include <xrpl/core/ServiceRegistry.h>
+#include <xrpl/ledger/entries/OracleEntry.h>
 #include <xrpl/ledger/helpers/AccountRootHelpers.h>
 #include <xrpl/ledger/helpers/DirectoryHelpers.h>
 #include <xrpl/ledger/helpers/OracleHelpers.h>
@@ -26,7 +27,6 @@
 #include <cstdint>
 #include <cstdlib>
 #include <map>
-#include <memory>
 #include <set>
 #include <utility>
 
@@ -84,7 +84,7 @@ OracleSet::preclaim(PreclaimContext const& ctx)
         return tecINVALID_UPDATE_TIME;
 
     auto const sle =
-        ctx.view.read(keylet::oracle(ctx.tx.getAccountID(sfAccount), ctx.tx[sfOracleDocumentID]));
+        OracleEntryR(ctx.tx.getAccountID(sfAccount), ctx.tx[sfOracleDocumentID], ctx.view);
 
     // token pairs to add/update
     std::set<std::pair<Currency, Currency>> pairs;
@@ -153,7 +153,7 @@ OracleSet::preclaim(PreclaimContext const& ctx)
         if (!pairsDel.empty())
             return tecTOKEN_PAIR_NOT_FOUND;
 
-        auto const oldCount = calculateOracleReserve(sle);
+        auto const oldCount = sle.reserveCount();
         auto const newCount = calculateOracleReserve(pairs);
 
         adjustReserve = newCount - oldCount;
@@ -227,7 +227,7 @@ OracleSet::doApply()
             priceData.setFieldU8(sfScale, entry.getFieldU8(sfScale));
     };
 
-    if (auto sle = ctx_.view().peek(oracleID))
+    if (auto sle = OracleEntryW(oracleID, ctx_.view()))
     {
         // update
         // the token pair that doesn't have their price updated will not
@@ -287,13 +287,13 @@ OracleSet::doApply()
         if (adjust != 0 && !adjustOracleOwnerCount(ctx_, adjust))
             return tefINTERNAL;  // LCOV_EXCL_LINE
 
-        ctx_.view().update(sle);
+        sle.update();
     }
     else
     {
         // create
 
-        sle = std::make_shared<SLE>(oracleID);
+        sle.newSLE();
         sle->setAccountID(sfOwner, ctx_.tx.getAccountID(sfAccount));
         if (ctx_.view().rules().enabled(fixIncludeKeyletFields))
         {
@@ -337,7 +337,7 @@ OracleSet::doApply()
         if (!adjustOracleOwnerCount(ctx_, count))
             return tefINTERNAL;  // LCOV_EXCL_LINE
 
-        ctx_.view().insert(sle);
+        sle.insert();
     }
 
     return tesSUCCESS;

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>