Proposed 1.12.0-rc4

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

@@ -200,14 +200,17 @@ deleteAMMTrustLines(
         keylet::ownerDir(ammAccountID),
         [&](LedgerEntryType nodeType,
             uint256 const&,
-            std::shared_ptr<SLE>& sleItem) -> TER {
+            std::shared_ptr<SLE>& sleItem) -> std::pair<TER, SkipEntry> {
+            // Skip AMM
+            if (nodeType == LedgerEntryType::ltAMM)
+                return {tesSUCCESS, SkipEntry::Yes};
             // Should only have the trustlines
             if (nodeType != LedgerEntryType::ltRIPPLE_STATE)
             {
                 JLOG(j.error())
                     << "deleteAMMTrustLines: deleting non-trustline "
                     << nodeType;
-                return tecINTERNAL;
+                return {tecINTERNAL, SkipEntry::No};
             }
 
             // Trustlines must have zero balance
@@ -216,10 +219,12 @@ deleteAMMTrustLines(
                 JLOG(j.error())
                     << "deleteAMMTrustLines: deleting trustline with "
                        "non-zero balance.";
-                return tecINTERNAL;
+                return {tecINTERNAL, SkipEntry::No};
             }
 
-            return deleteAMMTrustLine(sb, sleItem, ammAccountID, j);
+            return {
+                deleteAMMTrustLine(sb, sleItem, ammAccountID, j),
+                SkipEntry::No};
         },
         j,
         maxTrustlinesToDelete);
@@ -255,6 +260,12 @@ deleteAMMAccount(
         return ter;
 
     auto const ownerDirKeylet = keylet::ownerDir(ammAccountID);
+    if (!sb.dirRemove(
+            ownerDirKeylet, (*ammSle)[sfOwnerNode], ammSle->key(), false))
+    {
+        JLOG(j.error()) << "deleteAMMAccount: failed to remove dir link";
+        return tecINTERNAL;
+    }
     if (sb.exists(ownerDirKeylet) && !sb.emptyDirDelete(ownerDirKeylet))
     {
         JLOG(j.error()) << "deleteAMMAccount: cannot delete root dir node of "

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

@@ -223,9 +223,11 @@ applyBid(
             lptAMMBalance, toSTAmount(lptAMMBalance.issue(), burn), false);
         if (saBurn >= lptAMMBalance)
         {
-            JLOG(ctx_.journal.debug())
-                << "AMM Bid: invalid burn " << burn << " " << lptAMMBalance;
-            return tecAMM_BALANCE;
+            // This error case should never occur.
+            JLOG(ctx_.journal.fatal())
+                << "AMM Bid: LP Token burn exceeds AMM balance " << burn << " "
+                << lptAMMBalance;
+            return tecINTERNAL;
         }
         auto res =
             redeemIOU(sb, account_, saBurn, lpTokens.issue(), ctx_.journal);
@@ -316,9 +318,10 @@ applyBid(
         auto const refund = fractionRemaining * pricePurchased;
         if (refund > *payPrice)
         {
-            JLOG(ctx_.journal.debug())
-                << "AMM Bid: invalid refund " << refund << " " << *payPrice;
-            return {tecINSUFFICIENT_PAYMENT, false};
+            // This error case should never occur.
+            JLOG(ctx_.journal.fatal()) << "AMM Bid: refund exceeds payPrice "
+                                       << refund << " " << *payPrice;
+            return {tecINTERNAL, false};
         }
         res = accountSend(
             sb,

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

@@ -279,6 +279,20 @@ applyCreate(
     // AMM creator gets the auction slot and the voting slot.
     initializeFeeAuctionVote(
         ctx_.view(), ammSle, account_, lptIss, ctx_.tx[sfTradingFee]);
+
+    // Add owner directory to link the root account and AMM object.
+    if (auto const page = sb.dirInsert(
+            keylet::ownerDir(*ammAccount),
+            ammSle->key(),
+            describeOwnerDir(*ammAccount)))
+    {
+        ammSle->setFieldU64(sfOwnerNode, *page);
+    }
+    else
+    {
+        JLOG(j_.debug()) << "AMM Instance: failed to insert owner dir";
+        return {tecDIR_FULL, false};
+    }
     sb.insert(ammSle);
 
     // Send LPT to LP.

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

@@ -298,19 +298,19 @@ DeleteAccount::doApply()
         ownerDirKeylet,
         [&](LedgerEntryType nodeType,
             uint256 const& dirEntry,
-            std::shared_ptr<SLE>& sleItem) -> TER {
+            std::shared_ptr<SLE>& sleItem) -> std::pair<TER, SkipEntry> {
             if (auto deleter = nonObligationDeleter(nodeType))
             {
                 TER const result{
                     deleter(ctx_.app, view(), account_, dirEntry, sleItem, j_)};
 
-                return result;
+                return {result, SkipEntry::No};
             }
 
             assert(!"Undeletable entry should be found in preclaim.");
             JLOG(j_.error()) << "DeleteAccount undeletable item not "
                                 "found in preclaim.";
-            return tecHAS_OBLIGATIONS;
+            return {tecHAS_OBLIGATIONS, SkipEntry::No};
         },
         ctx_.journal);
     if (ter != tesSUCCESS)

src/ripple/ledger/View.h

@@ -43,6 +43,7 @@
 namespace ripple {
 
 enum class WaiveTransferFee : bool { No = false, Yes };
+enum class SkipEntry : bool { No = false, Yes };
 
 //------------------------------------------------------------------------------
 //
@@ -458,6 +459,14 @@ transferXRP(
 [[nodiscard]] TER
 requireAuth(ReadView const& view, Issue const& issue, AccountID const& account);
 
+/** Deleter function prototype. Returns the status of the entry deletion
+ * (if should not be skipped) and if the entry should be skipped. The status
+ * is always tesSUCCESS if the entry should be skipped.
+ */
+using EntryDeleter = std::function<std::pair<TER, SkipEntry>(
+    LedgerEntryType,
+    uint256 const&,
+    std::shared_ptr<SLE>&)>;
 /** Cleanup owner directory entries on account delete.
  * Used for a regular and AMM accounts deletion. The caller
  * has to provide the deleter function, which handles details of
@@ -469,8 +478,7 @@ requireAuth(ReadView const& view, Issue const& issue, AccountID const& account);
 cleanupOnAccountDelete(
     ApplyView& view,
     Keylet const& ownerDirKeylet,
-    std::function<TER(LedgerEntryType, uint256 const&, std::shared_ptr<SLE>&)>
-        deleter,
+    EntryDeleter const& deleter,
     beast::Journal j,
     std::optional<std::uint16_t> maxNodesToDelete = std::nullopt);
 

src/ripple/ledger/impl/View.cpp

@@ -1531,8 +1531,7 @@ TER
 cleanupOnAccountDelete(
     ApplyView& view,
     Keylet const& ownerDirKeylet,
-    std::function<TER(LedgerEntryType, uint256 const&, std::shared_ptr<SLE>&)>
-        deleter,
+    EntryDeleter const& deleter,
     beast::Journal j,
     std::optional<uint16_t> maxNodesToDelete)
 {
@@ -1567,8 +1566,8 @@ cleanupOnAccountDelete(
 
             // Deleter handles the details of specific account-owned object
             // deletion
-            if (auto const ter = deleter(nodeType, dirEntry, sleItem);
-                ter != tesSUCCESS)
+            auto const [ter, skipEntry] = deleter(nodeType, dirEntry, sleItem);
+            if (ter != tesSUCCESS)
                 return ter;
 
             // dirFirst() and dirNext() are like iterators with exposed
@@ -1580,21 +1579,22 @@ cleanupOnAccountDelete(
             // "iterator state" is invalid.
             //
             //  1. During the process of getting an entry from the
-            //     directory uDirEntry was incremented from 0 to 1.
+            //     directory uDirEntry was incremented from 'it' to 'it'+1.
             //
-            //  2. We then deleted the entry at index 0, which means the
-            //     entry that was at 1 has now moved to 0.
+            //  2. We then deleted the entry at index 'it', which means the
+            //     entry that was at 'it'+1 has now moved to 'it'.
             //
-            //  3. So we verify that uDirEntry is indeed 1.  Then we jam it
-            //     back to zero to "un-invalidate" the iterator.
-            assert(uDirEntry == 1);
-            if (uDirEntry != 1)
+            //  3. So we verify that uDirEntry is indeed 'it'+1.  Then we jam it
+            //     back to 'it' to "un-invalidate" the iterator.
+            assert(uDirEntry >= 1);
+            if (uDirEntry == 0)
             {
                 JLOG(j.error())
                     << "DeleteAccount iterator re-validation failed.";
                 return tefBAD_LEDGER;
             }
-            uDirEntry = 0;
+            if (skipEntry == SkipEntry::No)
+                uDirEntry--;
 
         } while (
             dirNext(view, ownerDirKeylet.key, sleDirNode, uDirEntry, dirEntry));

src/ripple/protocol/impl/BuildInfo.cpp

@@ -33,7 +33,7 @@ namespace BuildInfo {
 //  and follow the format described at http://semver.org/
 //------------------------------------------------------------------------------
 // clang-format off
-char const* const versionString = "1.12.0-rc3"
+char const* const versionString = "1.12.0-rc4"
 // clang-format on
 
 #if defined(DEBUG) || defined(SANITIZER)

src/ripple/protocol/impl/LedgerFormats.cpp

@@ -279,6 +279,7 @@ LedgerFormats::LedgerFormats()
             {sfLPTokenBalance, soeREQUIRED},
             {sfAsset, soeREQUIRED},
             {sfAsset2, soeREQUIRED},
+            {sfOwnerNode, soeREQUIRED},
         },
         commonFields);
 

src/ripple/protocol/jss.h

@@ -160,6 +160,7 @@ JSS(alternatives);           // out: PathRequest, RipplePathFind
 JSS(amendment_blocked);      // out: NetworkOPs
 JSS(amendments);             // in: AccountObjects, out: NetworkOPs
 JSS(amm);                    // out: amm_info
+JSS(amm_account);            // in: amm_info
 JSS(amount);                 // out: AccountChannels, amm_info
 JSS(amount2);                // out: amm_info
 JSS(api_version);            // in: many, out: Version

src/ripple/rpc/handlers/AMMInfo.cpp

@@ -74,51 +74,96 @@ doAMMInfo(RPC::JsonContext& context)
 {
     auto const& params(context.params);
     Json::Value result;
-    std::optional<AccountID> accountID;
-
-    Issue issue1;
-    Issue issue2;
-
-    if (!params.isMember(jss::asset) || !params.isMember(jss::asset2))
-    {
-        RPC::inject_error(rpcINVALID_PARAMS, result);
-        return result;
-    }
-
-    if (auto const i = getIssue(params[jss::asset], context.j); !i)
-    {
-        RPC::inject_error(i.error(), result);
-        return result;
-    }
-    else
-        issue1 = *i;
-    if (auto const i = getIssue(params[jss::asset2], context.j); !i)
-    {
-        RPC::inject_error(i.error(), result);
-        return result;
-    }
-    else
-        issue2 = *i;
 
     std::shared_ptr<ReadView const> ledger;
     result = RPC::lookupLedger(ledger, context);
     if (!ledger)
         return result;
 
-    if (params.isMember(jss::account))
+    struct ValuesFromContextParams
     {
-        accountID = getAccount(params[jss::account], result);
-        if (!accountID || !ledger->read(keylet::account(*accountID)))
+        std::optional<AccountID> accountID;
+        Issue issue1;
+        Issue issue2;
+        std::shared_ptr<SLE const> amm;
+    };
+
+    auto getValuesFromContextParams =
+        [&]() -> Expected<ValuesFromContextParams, error_code_i> {
+        std::optional<AccountID> accountID;
+        std::optional<Issue> issue1;
+        std::optional<Issue> issue2;
+        std::optional<uint256> ammID;
+
+        if ((params.isMember(jss::asset) != params.isMember(jss::asset2)) ||
+            (params.isMember(jss::asset) == params.isMember(jss::amm_account)))
+            return Unexpected(rpcINVALID_PARAMS);
+
+        if (params.isMember(jss::asset))
+        {
+            if (auto const i = getIssue(params[jss::asset], context.j))
+                issue1 = *i;
+            else
+                return Unexpected(i.error());
+        }
+
+        if (params.isMember(jss::asset2))
         {
-            RPC::inject_error(rpcACT_MALFORMED, result);
-            return result;
+            if (auto const i = getIssue(params[jss::asset2], context.j))
+                issue2 = *i;
+            else
+                return Unexpected(i.error());
         }
+
+        if (params.isMember(jss::amm_account))
+        {
+            auto const id = getAccount(params[jss::amm_account], result);
+            if (!id)
+                return Unexpected(rpcACT_MALFORMED);
+            auto const sle = ledger->read(keylet::account(*id));
+            if (!sle)
+                return Unexpected(rpcACT_MALFORMED);
+            ammID = sle->getFieldH256(sfAMMID);
+        }
+
+        assert(
+            (issue1.has_value() == issue2.has_value()) &&
+            (issue1.has_value() != ammID.has_value()));
+
+        if (params.isMember(jss::account))
+        {
+            accountID = getAccount(params[jss::account], result);
+            if (!accountID || !ledger->read(keylet::account(*accountID)))
+                return Unexpected(rpcACT_MALFORMED);
+        }
+
+        auto const ammKeylet = [&]() {
+            if (issue1 && issue2)
+                return keylet::amm(*issue1, *issue2);
+            assert(ammID);
+            return keylet::amm(*ammID);
+        }();
+        auto const amm = ledger->read(ammKeylet);
+        if (!amm)
+            return Unexpected(rpcACT_NOT_FOUND);
+        if (!issue1 && !issue2)
+        {
+            issue1 = (*amm)[sfAsset];
+            issue2 = (*amm)[sfAsset2];
+        }
+
+        return ValuesFromContextParams{
+            accountID, *issue1, *issue2, std::move(amm)};
+    };
+
+    auto const r = getValuesFromContextParams();
+    if (!r)
+    {
+        RPC::inject_error(r.error(), result);
+        return result;
     }
 
-    auto const ammKeylet = keylet::amm(issue1, issue2);
-    auto const amm = ledger->read(ammKeylet);
-    if (!amm)
-        return rpcError(rpcACT_NOT_FOUND);
+    auto const& [accountID, issue1, issue2, amm] = *r;
 
     auto const ammAccountID = amm->getAccountID(sfAccount);
 

src/test/jtx/AMM.h

@@ -105,7 +105,10 @@ class AMM
     ammRpcInfo(
         std::optional<AccountID> const& account = std::nullopt,
         std::optional<std::string> const& ledgerIndex = std::nullopt,
-        std::optional<std::pair<Issue, Issue>> tokens = std::nullopt) const;
+        std::optional<Issue> issue1 = std::nullopt,
+        std::optional<Issue> issue2 = std::nullopt,
+        std::optional<AccountID> const& ammAccount = std::nullopt,
+        bool ignoreParams = false) const;
 
     /** Verify the AMM balances.
      */
@@ -150,7 +153,8 @@ class AMM
         STAmount const& asset2,
         IOUAmount const& balance,
         std::optional<AccountID> const& account = std::nullopt,
-        std::optional<std::string> const& ledger_index = std::nullopt) const;
+        std::optional<std::string> const& ledger_index = std::nullopt,
+        std::optional<AccountID> const& ammAccount = std::nullopt) const;
 
     [[nodiscard]] bool
     ammExists() const;