[FOLD] Revert AuthAccounts change and other fixes

* Create AuctionSlot on AMMCreate and update on AMMDeposit
* Add assert to check for AuctionSlot

Author: gregtatcam

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

@@ -138,6 +138,10 @@ std::uint16_t
 getTradingFee(ReadView const& view, SLE const& ammSle, AccountID const& account)
 {
     using namespace std::chrono;
+    // should not happen
+    assert(
+        !view.rules().enabled(fixInnerObjTemplate) ||
+        ammSle.isFieldPresent(sfAuctionSlot));
     if (ammSle.isFieldPresent(sfAuctionSlot))
     {
         auto const& auctionSlot =
@@ -298,18 +302,14 @@ initializeFeeAuctionVote(
     voteSlots.push_back(voteEntry);
     ammSle->setFieldArray(sfVoteSlots, voteSlots);
     // AMM creator gets the auction slot for free.
-    STObject auctionSlot = STObject::makeInnerObject(sfAuctionSlot, rules);
-    if (!rules.enabled(fixInnerObjTemplate) &&
-        ammSle->isFieldPresent(sfAuctionSlot))
+    // AuctionSlot is created on AMMCreate and updated on AMMDeposit
+    // when AMM is in an empty state
+    if (!ammSle->isFieldPresent(sfAuctionSlot))
     {
-        auto const& origAuctionSlot = ammSle->peekFieldObject(sfAuctionSlot);
-        // this is executed on deposit from empty state.
-        // pre-amendment code re-uses AuthAccounts
-        // post-amendment code resets AuthAccounts
-        if (origAuctionSlot.isFieldPresent(sfAuthAccounts))
-            auctionSlot.setFieldArray(
-                sfAuthAccounts, origAuctionSlot.getFieldArray(sfAuthAccounts));
+        STObject auctionSlot = STObject::makeInnerObject(sfAuctionSlot, rules);
+        ammSle->set(&auctionSlot);
     }
+    STObject& auctionSlot = ammSle->peekFieldObject(sfAuctionSlot);
     auctionSlot.setAccountID(sfAccount, account);
     // current + sec in 24h
     auto const expiration = std::chrono::duration_cast<std::chrono::seconds>(
@@ -327,7 +327,6 @@ initializeFeeAuctionVote(
         auctionSlot.setFieldU16(sfDiscountedFee, dfee);
     else if (auctionSlot.isFieldPresent(sfDiscountedFee))
         auctionSlot.makeFieldAbsent(sfDiscountedFee);
-    ammSle->set(&auctionSlot);
 }
 
 }  // namespace ripple

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

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

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

@@ -200,6 +200,11 @@ applyVote(
         }
     }
 
+    // should not happen
+    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/rpc/handlers/AMMInfo.cpp

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

src/test/app/AMM_test.cpp

@@ -4887,61 +4887,6 @@ struct AMM_test : public jtx::AMMTest
             tesSUCCESS,
             9,
             false);
-
-        auto testDepositOnEmpty = [&](FeatureBitset features, TER const& err) {
-            Env env(
-                *this,
-                envconfig([](std::unique_ptr<Config> cfg) {
-                    cfg->FEES.reference_fee = XRPAmount(1);
-                    return cfg;
-                }),
-                features);
-            fund(env, gw, {alice, bob}, XRP(20'000), {USD(10'000)});
-            AMM amm(env, gw, XRP(10'000), USD(10'000));
-            for (auto i = 0; i < maxDeletableAMMTrustLines + 10; ++i)
-            {
-                Account const a{std::to_string(i)};
-                env.fund(XRP(1'000), a);
-                env(trust(a, STAmount{amm.lptIssue(), 10'000}));
-                env.close();
-            }
-            amm.bid(BidArg{.authAccounts = {bob, alice}});
-            // The trustlines are partially deleted,
-            // AMM is set to an empty state.
-            amm.withdrawAll(gw);
-            amm.setClose(false);
-            amm.deposit(DepositArg{
-                .account = alice,
-                .asset1In = XRP(10'000),
-                .asset2In = USD(10'000),
-                .flags = tfTwoAssetIfEmpty,
-                .tfee = 0});
-            // AuthAccounts is reset post-amendment
-            if (features[fixInnerObjTemplate])
-                BEAST_EXPECT(amm.expectAuctionSlot({}));
-            // AuthAccounts is re-used pre-amendment
-            else
-                BEAST_EXPECT(amm.expectAuctionSlot({bob, alice}));
-            // repeat some fail/success scenarios
-            amm.vote(VoteArg{.account = alice, .tfee = 0, .err = ter(err)});
-            amm.withdraw(WithdrawArg{
-                .account = alice, .asset1Out = USD(1), .err = ter(err)});
-            amm.vote(VoteArg{.account = alice, .tfee = 20, .err = ter(err)});
-            amm.withdraw(WithdrawArg{
-                .account = alice, .asset1Out = USD(2), .err = ter(err)});
-            env.close();
-            amm.vote(VoteArg{.account = alice, .tfee = 0});
-            amm.vote(VoteArg{.account = alice, .tfee = 0, .err = ter(err)});
-            amm.withdraw(WithdrawArg{.account = alice, .asset1Out = USD(3)});
-            amm.deposit(DepositArg{
-                .account = bob, .asset1In = XRP(100), .asset2In = USD(100)});
-            amm.bid(BidArg{.account = bob});
-            amm.withdraw(WithdrawArg{.account = bob, .asset1Out = USD(4)});
-            amm.vote(VoteArg{.account = bob, .tfee = 10, .err = ter(err)});
-        };
-
-        testDepositOnEmpty(all, tesSUCCESS);
-        testDepositOnEmpty(all - fixInnerObjTemplate, tefEXCEPTION);
     }
 
     void

src/test/jtx/impl/AMM.cpp

@@ -670,6 +670,9 @@ AMM::bid(
     if (auto const amm =
             env_.current()->read(keylet::amm(asset1_.issue(), asset2_.issue())))
     {
+        assert(
+            !env_.current()->rules().enabled(fixInnerObjTemplate) ||
+            amm->isFieldPresent(sfAuctionSlot));
         if (amm->isFieldPresent(sfAuctionSlot))
         {
             auto const& auctionSlot =
@@ -773,22 +776,28 @@ bool
 AMM::expectAuctionSlot(auto&& cb) const
 {
     if (auto const amm =
-            env_.current()->read(keylet::amm(asset1_.issue(), asset2_.issue()));
-        amm && amm->isFieldPresent(sfAuctionSlot))
+            env_.current()->read(keylet::amm(asset1_.issue(), asset2_.issue())))
     {
-        auto const& auctionSlot =
-            static_cast<STObject const&>(amm->peekAtField(sfAuctionSlot));
-        if (auctionSlot.isFieldPresent(sfAccount))
+        assert(
+            !env_.current()->rules().enabled(fixInnerObjTemplate) ||
+            amm->isFieldPresent(sfAuctionSlot));
+        if (amm->isFieldPresent(sfAuctionSlot))
         {
-            // this could fail in pre-fixInnerObjTemplate test if one
-            // the fail-cases. access as optional to avoid the failure
-            auto const slotFee = auctionSlot[~sfDiscountedFee].value_or(0);
-            auto const slotInterval = ammAuctionTimeSlot(
-                env_.app().timeKeeper().now().time_since_epoch().count(),
-                auctionSlot);
-            auto const slotPrice = auctionSlot[sfPrice].iou();
-            auto const authAccounts = auctionSlot.getFieldArray(sfAuthAccounts);
-            return cb(slotFee, slotInterval, slotPrice, authAccounts);
+            auto const& auctionSlot =
+                static_cast<STObject const&>(amm->peekAtField(sfAuctionSlot));
+            if (auctionSlot.isFieldPresent(sfAccount))
+            {
+                // this could fail in pre-fixInnerObjTemplate test if one
+                // the fail-cases. access as optional to avoid the failure
+                auto const slotFee = auctionSlot[~sfDiscountedFee].value_or(0);
+                auto const slotInterval = ammAuctionTimeSlot(
+                    env_.app().timeKeeper().now().time_since_epoch().count(),
+                    auctionSlot);
+                auto const slotPrice = auctionSlot[sfPrice].iou();
+                auto const authAccounts =
+                    auctionSlot.getFieldArray(sfAuthAccounts);
+                return cb(slotFee, slotInterval, slotPrice, authAccounts);
+            }
         }
     }
     return false;