fix(AMM): prevent orphaned objects, inconsistent ledger state: (XRPLF…

…#4626)

When an AMM account is deleted, the owner directory entries must be
deleted in order to ensure consistent ledger state.

* When deleting AMM account:
  * Clean up AMM owner dir, linking AMM account and AMM object
  * Delete trust lines to AMM
* Disallow `CheckCreate` to AMM accounts
  * AMM cannot cash a check
* Constrain entries in AuthAccounts array to be accounts
  * AuthAccounts is an array of objects for the AMMBid transaction
* SetTrust (TrustSet): Allow on AMM only for LP tokens
  * If the destination is an AMM account and the trust line doesn't
    exist, then:
    * If the asset is not the AMM LP token, then fail the tx with
      `tecNO_PERMISSION`
    * If the AMM is in empty state, then fail the tx with `tecAMM_EMPTY`
      * This disallows trustlines to AMM in empty state
* Add AMMID to AMM root account
  * Remove lsfAMM flag and use sfAMMID instead
* Remove owner dir entry for ltAMM
* Add `AMMDelete` transaction type to handle amortized deletion
  * Limit number of trust lines to delete on final withdraw + AMMDelete
  * Put AMM in empty state when LPTokens is 0 upon final withdraw
  * Add `tfTwoAssetIfEmpty` deposit option in AMM empty state
  * Fail all AMM transactions in AMM empty state except special deposit
  * Add `tecINCOMPLETE` to indicate that not all AMM trust lines are
    deleted (i.e. partial deletion)
    * This is handled in Transactor similar to deleted offers
  * Fail AMMDelete with `tecINTERNAL` if AMM root account is nullptr
  * Don't validate for invalid asset pair in AMMDelete
* AMMWithdraw deletes AMM trust lines and AMM account/object only if the
  number of trust lines is less than max
  * Current `maxDeletableAMMTrustLines` = 512
  * Check no directory left after AMM trust lines are deleted
  * Enable partial trustline deletion in AMMWithdraw
* Add `tecAMM_NOT_EMPTY` to fail any transaction that expects an AMM in
  empty state
* Clawback considerations
  * Disallow clawback out of AMM account
  * Disallow AMM create if issuer can claw back

This patch applies to the AMM implementation in XRPLF#4294.

Acknowledgements:
Richard Holland and Nik Bougalis for responsibly disclosing this issue.

Bug Bounties and Responsible Disclosures:
We welcome reviews of the project code and urge researchers to
responsibly disclose any issues they may find.

To report a bug, please send a detailed report to:

    [email protected]

API-CHANGELOG.md

@@ -47,6 +47,28 @@ Additions are intended to be non-breaking (because they are purely additive).
  - Adds the [Clawback transaction type](https:/XRPLF/XRPL-Standards/blob/master/XLS-39d-clawback/README.md#331-clawback-transaction), containing these fields:
  - `Account`: The issuer of the asset being clawed back. Must also be the sender of the transaction.
  - `Amount`: The amount being clawed back, with the `Amount.issuer` being the token holder's address.
+- Adds [AMM](https:/XRPLF/XRPL-Standards/discussions/78) ([#4294](https:/XRPLF/rippled/pull/4294), [#4626](https:/XRPLF/rippled/pull/4626)) feature:
+ - Adds `amm_info` API to retrieve AMM information for a given tokens pair.
+ - Adds `AMMCreate` transaction type to create `AMM` instance.
+ - Adds `AMMDeposit` transaction type to deposit funds into `AMM` instance.
+ - Adds `AMMWithdraw` transaction type to withdraw funds from `AMM` instance.
+ - Adds `AMMVote` transaction type to vote for the trading fee of `AMM` instance.
+ - Adds `AMMBid` transaction type to bid for the Auction Slot of `AMM` instance.
+ - Adds `AMMDelete` transaction type to delete `AMM` instance.
+ - Adds `sfAMMID` to `AccountRoot` to indicate that the account is `AMM`'s account. `AMMID` is used to fetch `ltAMM`.
+ - Adds `lsfAMMNode` `TrustLine` flag to indicate that one side of the `TrustLine` is `AMM` account.
+ - Adds `tfLPToken`, `tfSingleAsset`, `tfTwoAsset`, `tfOneAssetLPToken`, `tfLimitLPToken`, `tfTwoAssetIfEmpty`,
+ `tfWithdrawAll`, `tfOneAssetWithdrawAll` which allow a trader to specify different fields combination 
+ for `AMMDeposit` and `AMMWithdraw` transactions.
+ - Adds new transaction result codes:
+ - tecUNFUNDED_AMM: insufficient balance to fund AMM. The account does not have funds for liquidity provision.
+ - tecAMM_BALANCE: AMM has invalid balance. Calculated balances greater than the current pool balances.
+ - tecAMM_FAILED: AMM transaction failed. Fails due to a processing failure.
+ - tecAMM_INVALID_TOKENS: AMM invalid LP tokens. Invalid input values, format, or calculated values.
+ - tecAMM_EMPTY: AMM is in empty state. Transaction expects AMM in non-empty state (LP tokens > 0).
+ - tecAMM_NOT_EMPTY: AMM is not in empty state. Transaction expects AMM in empty state (LP tokens == 0).
+ - tecAMM_ACCOUNT: AMM account. Clawback of AMM account.
+ - tecINCOMPLETE: Some work was completed, but more submissions required to finish. AMMDelete partially deletes the trustlines.
 
 ## XRP Ledger version 1.11.0
 

Builds/CMake/RippledCore.cmake

@@ -498,6 +498,7 @@ target_sources (rippled PRIVATE
  src/ripple/app/rdb/impl/Wallet.cpp
  src/ripple/app/tx/impl/AMMBid.cpp
  src/ripple/app/tx/impl/AMMCreate.cpp
+ src/ripple/app/tx/impl/AMMDelete.cpp
  src/ripple/app/tx/impl/AMMDeposit.cpp
  src/ripple/app/tx/impl/AMMVote.cpp
  src/ripple/app/tx/impl/AMMWithdraw.cpp

src/ripple/app/misc/AMMUtils.h

@@ -93,6 +93,26 @@ ammAccountHolds(
  AccountID const& ammAccountID,
  Issue const& issue);
 
+/** Delete trustlines to AMM. If all trustlines are deleted then
+ * AMM object and account are deleted. Otherwise tecIMPCOMPLETE is returned.
+ */
+TER
+deleteAMMAccount(
+ Sandbox& view,
+ Issue const& asset,
+ Issue const& asset2,
+ beast::Journal j);
+
+/** Initialize Auction and Voting slots and set the trading/discounted fee.
+ */
+void
+initializeFeeAuctionVote(
+ ApplyView& view,
+ std::shared_ptr<SLE>& ammSle,
+ AccountID const& account,
+ Issue const& lptIssue,
+ std::uint16_t tfee);
+
 } // namespace ripple
 
 #endif // RIPPLE_APP_MISC_AMMUTILS_H_INLCUDED

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

@@ -188,4 +188,122 @@ ammAccountHolds(
  return STAmount{issue};
 }
 
+static TER
+deleteAMMTrustLines(
+ Sandbox& sb,
+ AccountID const& ammAccountID,
+ std::uint16_t maxTrustlinesToDelete,
+ beast::Journal j)
+{
+ return cleanupOnAccountDelete(
+ sb,
+ keylet::ownerDir(ammAccountID),
+ [&](LedgerEntryType nodeType,
+ uint256 const&,
+ std::shared_ptr<SLE>& sleItem) -> TER {
+ // Should only have the trustlines
+ if (nodeType != LedgerEntryType::ltRIPPLE_STATE)
+ {
+ JLOG(j.error())
+ << "deleteAMMTrustLines: deleting non-trustline "
+ << nodeType;
+ return tecINTERNAL;
+ }
+
+ // Trustlines must have zero balance
+ if (sleItem->getFieldAmount(sfBalance) != beast::zero)
+ {
+ JLOG(j.error())
+ << "deleteAMMTrustLines: deleting trustline with "
+ "non-zero balance.";
+ return tecINTERNAL;
+ }
+
+ return deleteAMMTrustLine(sb, sleItem, ammAccountID, j);
+ },
+ j,
+ maxTrustlinesToDelete);
+}
+
+TER
+deleteAMMAccount(
+ Sandbox& sb,
+ Issue const& asset,
+ Issue const& asset2,
+ beast::Journal j)
+{
+ auto ammSle = sb.peek(keylet::amm(asset, asset2));
+ if (!ammSle)
+ {
+ JLOG(j.error()) << "deleteAMMAccount: AMM object does not exist "
+ << asset << " " << asset2;
+ return tecINTERNAL;
+ }
+
+ auto const ammAccountID = (*ammSle)[sfAccount];
+ auto sleAMMRoot = sb.peek(keylet::account(ammAccountID));
+ if (!sleAMMRoot)
+ {
+ JLOG(j.error()) << "deleteAMMAccount: AMM account does not exist "
+ << to_string(ammAccountID);
+ return tecINTERNAL;
+ }
+
+ if (auto const ter =
+ deleteAMMTrustLines(sb, ammAccountID, maxDeletableAMMTrustLines, j);
+ ter != tesSUCCESS)
+ return ter;
+
+ auto const ownerDirKeylet = keylet::ownerDir(ammAccountID);
+ if (sb.exists(ownerDirKeylet) && !sb.emptyDirDelete(ownerDirKeylet))
+ {
+ JLOG(j.error()) << "deleteAMMAccount: cannot delete root dir node of "
+ << toBase58(ammAccountID);
+ return tecINTERNAL;
+ }
+
+ sb.erase(ammSle);
+ sb.erase(sleAMMRoot);
+
+ return tesSUCCESS;
+}
+
+void
+initializeFeeAuctionVote(
+ ApplyView& view,
+ std::shared_ptr<SLE>& ammSle,
+ AccountID const& account,
+ Issue const& lptIssue,
+ std::uint16_t tfee)
+{
+ // AMM creator gets the voting slot.
+ STArray voteSlots;
+ STObject voteEntry{sfVoteEntry};
+ 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.setAccountID(sfAccount, account);
+ // current + sec in 24h
+ auto const expiration = std::chrono::duration_cast<std::chrono::seconds>(
+ view.info().parentCloseTime.time_since_epoch())
+ .count() +
+ TOTAL_TIME_SLOT_SECS;
+ auctionSlot.setFieldU32(sfExpiration, expiration);
+ auctionSlot.setFieldAmount(sfPrice, STAmount{lptIssue, 0});
+ // Set the fee
+ if (tfee != 0)
+ ammSle->setFieldU16(sfTradingFee, tfee);
+ else if (ammSle->isFieldPresent(sfTradingFee))
+ ammSle->makeFieldAbsent(sfTradingFee);
+ if (auto const dfee = tfee / AUCTION_SLOT_DISCOUNTED_FEE_FRACTION)
+ auctionSlot.setFieldU16(sfDiscountedFee, dfee);
+ else if (auctionSlot.isFieldPresent(sfDiscountedFee))
+ auctionSlot.makeFieldAbsent(sfDiscountedFee);
+}
+
 } // namespace ripple

src/ripple/app/paths/impl/BookStep.cpp

@@ -99,7 +99,8 @@ class BookStep : public StepImp<TIn, TOut, BookStep<TIn, TOut, TDerived>>
  , ownerPaysTransferFee_(ctx.ownerPaysTransferFee)
  , j_(ctx.j)
  {
- if (auto const ammSle = ctx.view.read(keylet::amm(in, out)))
+ if (auto const ammSle = ctx.view.read(keylet::amm(in, out));
+ ammSle && ammSle->getFieldAmount(sfLPTokenBalance) != beast::zero)
  ammLiquidity_.emplace(
  ctx.view,
  (*ammSle)[sfAccount],

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

@@ -94,6 +94,10 @@ AMMBid::preclaim(PreclaimContext const& ctx)
  return terNO_AMM;
  }
 
+ auto const lpTokensBalance = (*ammSle)[sfLPTokenBalance];
+ if (lpTokensBalance == beast::zero)
+ return tecAMM_EMPTY;
+
  if (ctx.tx.isFieldPresent(sfAuthAccounts))
  {
  for (auto const& account : ctx.tx.getFieldArray(sfAuthAccounts))
@@ -114,7 +118,6 @@ AMMBid::preclaim(PreclaimContext const& ctx)
  JLOG(ctx.j.debug()) << "AMM Bid: account is not LP.";
  return tecAMM_INVALID_TOKENS;
  }
- auto const lpTokensBalance = (*ammSle)[sfLPTokenBalance];
 
  auto const bidMin = ctx.tx[~sfBidMin];
 
@@ -204,10 +207,10 @@ applyBid(
  Number const& burn) -> TER {
  auctionSlot.setAccountID(sfAccount, account_);
  auctionSlot.setFieldU32(sfExpiration, current + TOTAL_TIME_SLOT_SECS);
- if (fee == 0)
- auctionSlot.makeFieldAbsent(sfDiscountedFee);
- else
+ if (fee != 0)
  auctionSlot.setFieldU16(sfDiscountedFee, fee);
+ else if (auctionSlot.isFieldPresent(sfDiscountedFee))
+ auctionSlot.makeFieldAbsent(sfDiscountedFee);
  auctionSlot.setFieldAmount(
  sfPrice, toSTAmount(lpTokens.issue(), minPrice));
  if (ctx_.tx.isFieldPresent(sfAuthAccounts))

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

@@ -173,7 +173,7 @@ AMMCreate::preclaim(PreclaimContext const& ctx)
  auto isLPToken = [&](STAmount const& amount) -> bool {
  if (auto const sle =
  ctx.view.read(keylet::account(amount.issue().account)))
- return (sle->getFlags() & lsfAMM);
+ return sle->isFieldPresent(sfAMMID);
  return false;
  };
 
@@ -184,7 +184,21 @@ AMMCreate::preclaim(PreclaimContext const& ctx)
  return tecAMM_INVALID_TOKENS;
  }
 
- return tesSUCCESS;
+ // Disallow AMM if the issuer has clawback enabled
+ auto clawbackDisabled = [&](Issue const& issue) -> TER {
+ if (isXRP(issue))
+ return tesSUCCESS;
+ if (auto const sle = ctx.view.read(keylet::account(issue.account));
+ !sle)
+ return tecINTERNAL;
+ else if (sle->getFlags() & lsfAllowTrustLineClawback)
+ return tecNO_PERMISSION;
+ return tesSUCCESS;
+ };
+
+ if (auto const ter = clawbackDisabled(amount.issue()); ter != tesSUCCESS)
+ return ter;
+ return clawbackDisabled(amount2.issue());
 }
 
 static std::pair<TER, bool>
@@ -247,54 +261,26 @@ applyCreate(
  // A user can only receive LPTokens through affirmative action -
  // either an AMMDeposit, TrustSet, crossing an offer, etc.
  sleAMMRoot->setFieldU32(
- sfFlags, lsfAMM | lsfDisableMaster | lsfDefaultRipple | lsfDepositAuth);
+ sfFlags, lsfDisableMaster | lsfDefaultRipple | lsfDepositAuth);
+ // Link the root account and AMM object
+ sleAMMRoot->setFieldH256(sfAMMID, ammKeylet.key);
  sb.insert(sleAMMRoot);
 
  // Calculate initial LPT balance.
  auto const lpTokens = ammLPTokens(amount, amount2, lptIss);
 
  // Create ltAMM
  auto ammSle = std::make_shared<SLE>(ammKeylet);
- if (ctx_.tx[sfTradingFee] != 0)
- ammSle->setFieldU16(sfTradingFee, ctx_.tx[sfTradingFee]);
  ammSle->setAccountID(sfAccount, *ammAccount);
  ammSle->setFieldAmount(sfLPTokenBalance, lpTokens);
  auto const& [issue1, issue2] = std::minmax(amount.issue(), amount2.issue());
  ammSle->setFieldIssue(sfAsset, STIssue{sfAsset, issue1});
  ammSle->setFieldIssue(sfAsset2, STIssue{sfAsset2, issue2});
- // AMM creator gets the voting slot.
- STArray voteSlots;
- STObject voteEntry{sfVoteEntry};
- if (ctx_.tx[sfTradingFee] != 0)
- voteEntry.setFieldU16(sfTradingFee, ctx_.tx[sfTradingFee]);
- voteEntry.setFieldU32(sfVoteWeight, 100000);
- 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.setAccountID(sfAccount, account_);
- // current + sec in 24h
- auto const expiration =
- std::chrono::duration_cast<std::chrono::seconds>(
- ctx_.view().info().parentCloseTime.time_since_epoch())
- .count() +
- TOTAL_TIME_SLOT_SECS;
- auctionSlot.setFieldU32(sfExpiration, expiration);
- auctionSlot.setFieldAmount(sfPrice, STAmount{lpTokens.issue(), 0});
+ // AMM creator gets the auction slot and the voting slot.
+ initializeFeeAuctionVote(
+ ctx_.view(), ammSle, account_, lptIss, ctx_.tx[sfTradingFee]);
  sb.insert(ammSle);
 
- // Add owner directory to link the root account and AMM object.
- if (auto const page = sb.dirInsert(
- keylet::ownerDir(*ammAccount),
- ammSle->key(),
- describeOwnerDir(*ammAccount));
- !page)
- {
- JLOG(j_.debug()) << "AMM Instance: failed to insert owner dir";
- return {tecDIR_FULL, false};
- }
-
  // Send LPT to LP.
  auto res = accountSend(sb, *ammAccount, account_, lpTokens, ctx_.journal);
  if (res != tesSUCCESS)