fixInnerObjTemplate: Add STObject constructor to explicitly set inner object template

src/ripple/app/misc/impl/AMMUtils.cpp

@@ -138,6 +138,9 @@ std::uint16_t
 getTradingFee(ReadView const& view, SLE const& ammSle, AccountID const& account)
 {
     using namespace std::chrono;
+    assert(
+        !view.rules().enabled(fixInnerObjTemplate) ||
+        ammSle.isFieldPresent(sfAuctionSlot));
     if (ammSle.isFieldPresent(sfAuctionSlot))
     {
         auto const& auctionSlot =
@@ -287,17 +290,35 @@ initializeFeeAuctionVote(
     Issue const& lptIssue,
     std::uint16_t tfee)
 {
+    auto const& rules = view.rules();
     // AMM creator gets the voting slot.
     STArray voteSlots;
-    STObject voteEntry{sfVoteEntry};
+    STObject voteEntry = STObject::makeInnerObject(sfVoteEntry, rules);
     if (tfee != 0)
         voteEntry.setFieldU16(sfTradingFee, tfee);
     voteEntry.setFieldU32(sfVoteWeight, VOTE_WEIGHT_SCALE_FACTOR);
     voteEntry.setAccountID(sfAccount, account);
     voteSlots.push_back(voteEntry);
     ammSle->setFieldArray(sfVoteSlots, voteSlots);
     // AMM creator gets the auction slot for free.
-    auto& auctionSlot = ammSle->peekFieldObject(sfAuctionSlot);
+    // AuctionSlot is created on AMMCreate and updated on AMMDeposit
+    // when AMM is in an empty state
+    STObject& auctionSlot = [&]() -> STObject& {
+        if (!rules.enabled(fixInnerObjTemplate))
+        {
+            return ammSle->peekFieldObject(sfAuctionSlot);
+        }
+        else
+        {
+            if (!ammSle->isFieldPresent(sfAuctionSlot))
+            {
+                STObject auctionSlot =
+                    STObject::makeInnerObject(sfAuctionSlot, rules);
+                ammSle->set(std::move(auctionSlot));
+            }
+            return ammSle->peekFieldObject(sfAuctionSlot);
+        }
+    }();
     auctionSlot.setAccountID(sfAccount, account);
     // current + sec in 24h
     auto const expiration = std::chrono::duration_cast<std::chrono::seconds>(

src/ripple/app/tx/impl/AMMBid.cpp

@@ -173,8 +173,18 @@ applyBid(
         return {tecINTERNAL, false};
     STAmount const lptAMMBalance = (*ammSle)[sfLPTokenBalance];
     auto const lpTokens = ammLPHolds(sb, *ammSle, account_, ctx_.journal);
-    if (!ammSle->isFieldPresent(sfAuctionSlot))
-        ammSle->makeFieldPresent(sfAuctionSlot);
+    auto const& rules = ctx_.view().rules();
+    if (!rules.enabled(fixInnerObjTemplate))
+    {
+        if (!ammSle->isFieldPresent(sfAuctionSlot))
+            ammSle->makeFieldPresent(sfAuctionSlot);
+    }
+    else
+    {
+        assert(ammSle->isFieldPresent(sfAuctionSlot));
+        if (!ammSle->isFieldPresent(sfAuctionSlot))
+            return {tecINTERNAL, false};
+    }
     auto& auctionSlot = ammSle->peekFieldObject(sfAuctionSlot);
     auto const current =
         duration_cast<seconds>(

src/ripple/app/tx/impl/AMMVote.cpp

@@ -104,6 +104,7 @@ applyVote(
     Number den{0};
     // Account already has vote entry
     bool foundAccount = false;
+    auto const& rules = ctx_.view().rules();
     // Iterate over the current vote entries and update each entry
     // per current total tokens balance and each LP tokens balance.
     // Find the entry with the least tokens and whether the account
@@ -119,7 +120,7 @@ applyVote(
             continue;
         }
         auto feeVal = entry[sfTradingFee];
-        STObject newEntry{sfVoteEntry};
+        STObject newEntry = STObject::makeInnerObject(sfVoteEntry, rules);
         // The account already has the vote entry.
         if (account == account_)
         {
@@ -158,7 +159,7 @@ applyVote(
     {
         auto update = [&](std::optional<std::uint8_t> const& minPos =
                               std::nullopt) {
-            STObject newEntry{sfVoteEntry};
+            STObject newEntry = STObject::makeInnerObject(sfVoteEntry, rules);
             if (feeNew != 0)
                 newEntry.setFieldU16(sfTradingFee, feeNew);
             newEntry.setFieldU32(
@@ -199,6 +200,10 @@ applyVote(
         }
     }
 
+    assert(
+        !ctx_.view().rules().enabled(fixInnerObjTemplate) ||
+        ammSle->isFieldPresent(sfAuctionSlot));
+
     // Update the vote entries and the trading/discounted fee.
     ammSle->setFieldArray(sfVoteSlots, updatedVoteSlots);
     if (auto const fee = static_cast<std::int64_t>(num / den))

src/ripple/protocol/Feature.h

@@ -74,7 +74,7 @@ namespace detail {
 // Feature.cpp. Because it's only used to reserve storage, and determine how
 // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than
 // the actual number of amendments. A LogicError on startup will verify this.
-static constexpr std::size_t numFeatures = 66;
+static constexpr std::size_t numFeatures = 67;
 
 /** Amendments that this server supports and the default voting behavior.
    Whether they are enabled depends on the Rules defined in the validated
@@ -353,6 +353,7 @@ extern uint256 const fixDisallowIncomingV1;
 extern uint256 const featureDID;
 extern uint256 const fixFillOrKill;
 extern uint256 const fixNFTokenReserve;
+extern uint256 const fixInnerObjTemplate;
 
 }  // namespace ripple
 

src/ripple/protocol/SOTemplate.h

@@ -35,6 +35,8 @@ enum SOEStyle {
     soeREQUIRED = 0,  // required
     soeOPTIONAL = 1,  // optional, may be present with default value
     soeDEFAULT = 2,   // optional, if present, must not have default value
+                      // inner object with the default fields has to be
+                      // constructed with STObject::makeInnerObject()
 };
 
 //------------------------------------------------------------------------------

src/ripple/protocol/STObject.h

@@ -43,6 +43,7 @@
 namespace ripple {
 
 class STArray;
+class Rules;
 
 inline void
 throwFieldNotFound(SField const& field)
@@ -102,6 +103,9 @@ class STObject : public STBase, public CountedObject<STObject>
     STObject(SerialIter&& sit, SField const& name);
     explicit STObject(SField const& name);
 
+    static STObject
+    makeInnerObject(SField const& name, Rules const& rules);
+
     iterator
     begin() const;
 
@@ -339,7 +343,7 @@ class STObject : public STBase, public CountedObject<STObject>
     set(std::unique_ptr<STBase> v);
 
     void
-    set(STBase* v);
+    set(STBase&& v);
 
     void
     setFieldU8(SField const& field, unsigned char);

src/ripple/protocol/impl/Feature.cpp

@@ -460,6 +460,7 @@ REGISTER_FIX    (fixDisallowIncomingV1,         Supported::yes, VoteBehavior::De
 REGISTER_FEATURE(DID,                           Supported::yes, VoteBehavior::DefaultNo);
 REGISTER_FIX(fixFillOrKill,                     Supported::yes, VoteBehavior::DefaultNo);
 REGISTER_FIX    (fixNFTokenReserve,             Supported::yes, VoteBehavior::DefaultNo);
+REGISTER_FIX(fixInnerObjTemplate,               Supported::yes, VoteBehavior::DefaultNo);
 
 // The following amendments are obsolete, but must remain supported
 // because they could potentially get enabled.

src/ripple/protocol/impl/InnerObjectFormats.cpp

@@ -25,6 +25,9 @@ namespace ripple {
 
 InnerObjectFormats::InnerObjectFormats()
 {
+    // inner objects with the default fields have to be
+    // constructed with STObject::makeInnerObject()
+
     add(sfSignerEntry.jsonName.c_str(),
         sfSignerEntry.getCode(),
         {

src/ripple/protocol/impl/STObject.cpp

@@ -18,7 +18,9 @@
 //==============================================================================
 
 #include <ripple/basics/Log.h>
+#include <ripple/protocol/Feature.h>
 #include <ripple/protocol/InnerObjectFormats.h>
+#include <ripple/protocol/Rules.h>
 #include <ripple/protocol/STAccount.h>
 #include <ripple/protocol/STArray.h>
 #include <ripple/protocol/STBlob.h>
@@ -57,6 +59,19 @@ STObject::STObject(SerialIter& sit, SField const& name, int depth) noexcept(
     set(sit, depth);
 }
 
+STObject
+STObject::makeInnerObject(SField const& name, Rules const& rules)
+{
+    STObject obj{name};
+    if (rules.enabled(fixInnerObjTemplate))
+    {
+        if (SOTemplate const* elements =
+                InnerObjectFormats::getInstance().findSOTemplateBySField(name))
+            obj.set(*elements);
+    }
+    return obj;
+}
+
 STBase*
 STObject::copy(std::size_t n, void* buf) const
 {
@@ -630,16 +645,22 @@ STObject::getFieldArray(SField const& field) const
 void
 STObject::set(std::unique_ptr<STBase> v)
 {
-    auto const i = getFieldIndex(v->getFName());
+    set(std::move(*v.get()));
+}
+
+void
+STObject::set(STBase&& v)
+{
+    auto const i = getFieldIndex(v.getFName());
     if (i != -1)
     {
-        v_[i] = std::move(*v);
+        v_[i] = std::move(v);
     }
     else
     {
         if (!isFree())
             Throw<std::runtime_error>("missing field in templated STObject");
-        v_.emplace_back(std::move(*v));
+        v_.emplace_back(std::move(v));
     }
 }
 

src/ripple/rpc/handlers/AMMInfo.cpp

@@ -200,6 +200,9 @@ doAMMInfo(RPC::JsonContext& context)
     }
     if (voteSlots.size() > 0)
         ammResult[jss::vote_slots] = std::move(voteSlots);
+    assert(
+        !ledger->rules().enabled(fixInnerObjTemplate) ||
+        amm->isFieldPresent(sfAuctionSlot));
     if (amm->isFieldPresent(sfAuctionSlot))
     {
         auto const& auctionSlot =

src/test/app/AMM_test.cpp

@@ -4789,6 +4789,106 @@ struct AMM_test : public jtx::AMMTest
         }
     }
 
+    void
+    testFixDefaultInnerObj()
+    {
+        testcase("Fix Default Inner Object");
+        using namespace jtx;
+        FeatureBitset const all{supported_amendments()};
+
+        auto test = [&](FeatureBitset features,
+                        TER const& err1,
+                        TER const& err2,
+                        TER const& err3,
+                        TER const& err4,
+                        std::uint16_t tfee,
+                        bool closeLedger,
+                        std::optional<std::uint16_t> extra = std::nullopt) {
+            Env env(*this, features);
+            fund(env, gw, {alice}, XRP(1'000), {USD(10)});
+            AMM amm(
+                env,
+                gw,
+                XRP(10),
+                USD(10),
+                {.tfee = tfee, .close = closeLedger});
+            amm.deposit(alice, USD(10), XRP(10));
+            amm.vote(VoteArg{.account = alice, .tfee = tfee, .err = ter(err1)});
+            amm.withdraw(WithdrawArg{
+                .account = gw, .asset1Out = USD(1), .err = ter(err2)});
+            // with the amendment disabled and ledger not closed,
+            // second vote succeeds if the first vote sets the trading fee
+            // to non-zero; if the first vote sets the trading fee to >0 && <9
+            // then the second withdraw succeeds if the second vote sets
+            // the trading fee so that the discounted fee is non-zero
+            amm.vote(VoteArg{.account = alice, .tfee = 20, .err = ter(err3)});
+            amm.withdraw(WithdrawArg{
+                .account = gw, .asset1Out = USD(2), .err = ter(err4)});
+        };
+
+        // ledger is closed after each transaction, vote/withdraw don't fail
+        // regardless whether the amendment is enabled or not
+        test(all, tesSUCCESS, tesSUCCESS, tesSUCCESS, tesSUCCESS, 0, true);
+        test(
+            all - fixInnerObjTemplate,
+            tesSUCCESS,
+            tesSUCCESS,
+            tesSUCCESS,
+            tesSUCCESS,
+            0,
+            true);
+        // ledger is not closed after each transaction
+        // vote/withdraw don't fail if the amendment is enabled
+        test(all, tesSUCCESS, tesSUCCESS, tesSUCCESS, tesSUCCESS, 0, false);
+        // vote/withdraw fail if the amendment is not enabled
+        // second vote/withdraw still fail: second vote fails because
+        // the initial trading fee is 0, consequently second withdraw fails
+        // because the second vote fails
+        test(
+            all - fixInnerObjTemplate,
+            tefEXCEPTION,
+            tefEXCEPTION,
+            tefEXCEPTION,
+            tefEXCEPTION,
+            0,
+            false);
+        // if non-zero trading/discounted fee then vote/withdraw
+        // don't fail whether the ledger is closed or not and
+        // the amendment is enabled or not
+        test(all, tesSUCCESS, tesSUCCESS, tesSUCCESS, tesSUCCESS, 10, true);
+        test(
+            all - fixInnerObjTemplate,
+            tesSUCCESS,
+            tesSUCCESS,
+            tesSUCCESS,
+            tesSUCCESS,
+            10,
+            true);
+        test(all, tesSUCCESS, tesSUCCESS, tesSUCCESS, tesSUCCESS, 10, false);
+        test(
+            all - fixInnerObjTemplate,
+            tesSUCCESS,
+            tesSUCCESS,
+            tesSUCCESS,
+            tesSUCCESS,
+            10,
+            false);
+        // non-zero trading fee but discounted fee is 0, vote doesn't fail
+        // but withdraw fails
+        test(all, tesSUCCESS, tesSUCCESS, tesSUCCESS, tesSUCCESS, 9, false);
+        // second vote sets the trading fee to non-zero, consequently
+        // second withdraw doesn't fail even if the amendment is not
+        // enabled and the ledger is not closed
+        test(
+            all - fixInnerObjTemplate,
+            tesSUCCESS,
+            tefEXCEPTION,
+            tesSUCCESS,
+            tesSUCCESS,
+            9,
+            false);
+    }
+
     void
     testCore()
     {
@@ -4815,6 +4915,7 @@ struct AMM_test : public jtx::AMMTest
         testClawback();
         testAMMID();
         testSelection();
+        testFixDefaultInnerObj();
     }
 
     void