amm_info: fetch by amm account id; add AMM object entry (XRPLF#4682)

- Update amm_info to fetch AMM by amm account id.
  - This is an additional way to retrieve an AMM object.
  - Alternatively, AMM can still be fetched by the asset pair as well.
- Add owner directory entry for AMM object.

Context:

- Add back the AMM object directory entry, which was deleted by XRPLF#4626.
  - This fixes `account_objects` for `amm` type.

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/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/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;