fixInnerObjTemplate: set inner object template (XRPLF#4906)

Add `STObject` constructor to explicitly set the inner object template.
This allows certain AMM transactions to apply in the same ledger:

There is no issue if the trading fee is greater than or equal to 0.01%.
If the trading fee is less than 0.01%, then:
- After AMM create, AMM transactions must wait for one ledger to close
  (3-5 seconds).
- After one ledger is validated, all AMM transactions succeed, as
  appropriate, except for AMMVote.
- The first AMMVote which votes for a 0 trading fee in a ledger will
  succeed. Subsequent AMMVote transactions which vote for a 0 trading
  fee will wait for the next ledger (3-5 seconds). This behavior repeats
  for each ledger.

This has no effect on the ultimate correctness of AMM. This amendment
will allow the transactions described above to succeed as expected, even
if the trading fee is 0 and the transactions are applied within one
ledger (block).

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,26 @@ 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
+    if (rules.enabled(fixInnerObjTemplate) &&
+        !ammSle->isFieldPresent(sfAuctionSlot))
+    {
+        STObject auctionSlot = STObject::makeInnerObject(sfAuctionSlot, rules);
+        ammSle->set(std::move(auctionSlot));
+    }
+    STObject& auctionSlot = 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