From 3138ea795da2eac331be885e6a5d8b79bff9cdeb Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Mon, 17 Jul 2023 08:05:11 -0400 Subject: [PATCH] fix(AMM): prevent orphaned objects, inconsistent ledger state: (#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 #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: bugs@xrpl.org Signed-off-by: Manoj Doshi --- API-CHANGELOG.md | 22 + Builds/CMake/RippledCore.cmake | 1 + src/ripple/app/misc/AMMUtils.h | 20 + src/ripple/app/misc/impl/AMMUtils.cpp | 118 +++ src/ripple/app/paths/impl/BookStep.cpp | 3 +- src/ripple/app/tx/impl/AMMBid.cpp | 11 +- src/ripple/app/tx/impl/AMMCreate.cpp | 58 +- src/ripple/app/tx/impl/AMMDelete.cpp | 83 +++ src/ripple/app/tx/impl/AMMDelete.h | 54 ++ src/ripple/app/tx/impl/AMMDeposit.cpp | 91 ++- src/ripple/app/tx/impl/AMMDeposit.h | 17 + src/ripple/app/tx/impl/AMMVote.cpp | 10 +- src/ripple/app/tx/impl/AMMWithdraw.cpp | 57 +- src/ripple/app/tx/impl/AMMWithdraw.h | 8 - src/ripple/app/tx/impl/Clawback.cpp | 3 + src/ripple/app/tx/impl/CreateCheck.cpp | 4 + src/ripple/app/tx/impl/DeleteAccount.cpp | 77 +- src/ripple/app/tx/impl/Escrow.cpp | 2 +- src/ripple/app/tx/impl/InvariantCheck.cpp | 15 +- src/ripple/app/tx/impl/PayChan.cpp | 2 +- src/ripple/app/tx/impl/Payment.cpp | 2 +- src/ripple/app/tx/impl/SetTrust.cpp | 35 +- src/ripple/app/tx/impl/Transactor.cpp | 73 +- src/ripple/app/tx/impl/applySteps.cpp | 11 + src/ripple/ledger/View.h | 29 +- src/ripple/ledger/impl/View.cpp | 127 +++- src/ripple/protocol/LedgerFormats.h | 2 +- src/ripple/protocol/Protocol.h | 5 + src/ripple/protocol/TER.h | 6 +- src/ripple/protocol/TxFlags.h | 3 +- src/ripple/protocol/TxFormats.h | 3 + .../protocol/impl/InnerObjectFormats.cpp | 6 + src/ripple/protocol/impl/LedgerFormats.cpp | 3 +- src/ripple/protocol/impl/TER.cpp | 4 + src/ripple/protocol/impl/TxFormats.cpp | 10 + src/ripple/protocol/jss.h | 1 + src/test/app/AMMExtended_test.cpp | 1 - src/test/app/AMM_test.cpp | 695 +++++++++++++++++- src/test/app/CrossingLimits_test.cpp | 1 - src/test/app/Escrow_test.cpp | 1 - src/test/app/Flow_test.cpp | 1 - src/test/app/Freeze_test.cpp | 1 - src/test/app/Offer_test.cpp | 1 - src/test/app/Path_test.cpp | 1 - src/test/app/PayChan_test.cpp | 1 - src/test/app/PayStrand_test.cpp | 1 - src/test/app/TrustAndBalance_test.cpp | 1 - src/test/jtx.h | 1 + src/test/jtx/AMM.h | 20 + src/test/jtx/TestHelpers.h | 33 + src/test/jtx/check.h | 7 - src/test/jtx/impl/AMM.cpp | 22 +- src/test/jtx/impl/check.cpp | 16 - src/test/rpc/AccountOffers_test.cpp | 1 - src/test/rpc/LedgerData_test.cpp | 1 - src/test/rpc/NoRipple_test.cpp | 1 - 56 files changed, 1536 insertions(+), 247 deletions(-) create mode 100644 src/ripple/app/tx/impl/AMMDelete.cpp create mode 100644 src/ripple/app/tx/impl/AMMDelete.h diff --git a/API-CHANGELOG.md b/API-CHANGELOG.md index bbbe71d5545..ae988beeb33 100644 --- a/API-CHANGELOG.md +++ b/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://github.com/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://github.com/XRPLF/XRPL-Standards/discussions/78) ([#4294](https://github.com/XRPLF/rippled/pull/4294), [#4626](https://github.com/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 diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake index 3240f79f896..b050c646d33 100644 --- a/Builds/CMake/RippledCore.cmake +++ b/Builds/CMake/RippledCore.cmake @@ -476,6 +476,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 diff --git a/src/ripple/app/misc/AMMUtils.h b/src/ripple/app/misc/AMMUtils.h index 1718df496b8..c25503ceb9c 100644 --- a/src/ripple/app/misc/AMMUtils.h +++ b/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& ammSle, + AccountID const& account, + Issue const& lptIssue, + std::uint16_t tfee); + } // namespace ripple #endif // RIPPLE_APP_MISC_AMMUTILS_H_INLCUDED diff --git a/src/ripple/app/misc/impl/AMMUtils.cpp b/src/ripple/app/misc/impl/AMMUtils.cpp index 7156c77f21a..dcb403296c1 100644 --- a/src/ripple/app/misc/impl/AMMUtils.cpp +++ b/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& 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& 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( + 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 diff --git a/src/ripple/app/paths/impl/BookStep.cpp b/src/ripple/app/paths/impl/BookStep.cpp index e82acbde817..dd6abe577f5 100644 --- a/src/ripple/app/paths/impl/BookStep.cpp +++ b/src/ripple/app/paths/impl/BookStep.cpp @@ -99,7 +99,8 @@ class BookStep : public StepImp> , 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], diff --git a/src/ripple/app/tx/impl/AMMBid.cpp b/src/ripple/app/tx/impl/AMMBid.cpp index 7654e842df1..d059f88c76c 100644 --- a/src/ripple/app/tx/impl/AMMBid.cpp +++ b/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)) diff --git a/src/ripple/app/tx/impl/AMMCreate.cpp b/src/ripple/app/tx/impl/AMMCreate.cpp index f3596a6f9f4..0c6874953a3 100644 --- a/src/ripple/app/tx/impl/AMMCreate.cpp +++ b/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 @@ -247,7 +261,9 @@ 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. @@ -255,46 +271,16 @@ applyCreate( // Create ltAMM auto ammSle = std::make_shared(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( - 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) diff --git a/src/ripple/app/tx/impl/AMMDelete.cpp b/src/ripple/app/tx/impl/AMMDelete.cpp new file mode 100644 index 00000000000..25502be4f44 --- /dev/null +++ b/src/ripple/app/tx/impl/AMMDelete.cpp @@ -0,0 +1,83 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2022 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include + +#include +#include +#include +#include +#include +#include +#include + +namespace ripple { + +NotTEC +AMMDelete::preflight(PreflightContext const& ctx) +{ + if (!ammEnabled(ctx.rules)) + return temDISABLED; + + if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + return ret; + + if (ctx.tx.getFlags() & tfUniversalMask) + { + JLOG(ctx.j.debug()) << "AMM Delete: invalid flags."; + return temINVALID_FLAG; + } + + return preflight2(ctx); +} + +TER +AMMDelete::preclaim(PreclaimContext const& ctx) +{ + auto const ammSle = + ctx.view.read(keylet::amm(ctx.tx[sfAsset], ctx.tx[sfAsset2])); + if (!ammSle) + { + JLOG(ctx.j.debug()) << "AMM Delete: Invalid asset pair."; + return terNO_AMM; + } + + auto const lpTokensBalance = (*ammSle)[sfLPTokenBalance]; + if (lpTokensBalance != beast::zero) + return tecAMM_NOT_EMPTY; + + return tesSUCCESS; +} + +TER +AMMDelete::doApply() +{ + // This is the ledger view that we work against. Transactions are applied + // as we go on processing transactions. + Sandbox sb(&ctx_.view()); + + auto const ter = + deleteAMMAccount(sb, ctx_.tx[sfAsset], ctx_.tx[sfAsset2], j_); + if (ter == tesSUCCESS || ter == tecINCOMPLETE) + sb.apply(ctx_.rawView()); + + return ter; +} + +} // namespace ripple diff --git a/src/ripple/app/tx/impl/AMMDelete.h b/src/ripple/app/tx/impl/AMMDelete.h new file mode 100644 index 00000000000..cf7f55cb715 --- /dev/null +++ b/src/ripple/app/tx/impl/AMMDelete.h @@ -0,0 +1,54 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2023 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#ifndef RIPPLE_TX_AMMDELETE_H_INCLUDED +#define RIPPLE_TX_AMMDELETE_H_INCLUDED + +#include + +namespace ripple { + +/** AMMDelete implements AMM delete transactor. This is a mechanism to + * delete AMM in an empty state when the number of LP tokens is 0. + * AMMDelete deletes the trustlines up to configured maximum. If all + * trustlines are deleted then AMM ltAMM and root account are deleted. + * Otherwise AMMDelete should be called again. + */ +class AMMDelete : public Transactor +{ +public: + static constexpr ConsequencesFactoryType ConsequencesFactory{Normal}; + + explicit AMMDelete(ApplyContext& ctx) : Transactor(ctx) + { + } + + static NotTEC + preflight(PreflightContext const& ctx); + + static TER + preclaim(PreclaimContext const& ctx); + + TER + doApply() override; +}; + +} // namespace ripple + +#endif // RIPPLE_TX_AMMDELETE_H_INCLUDED diff --git a/src/ripple/app/tx/impl/AMMDeposit.cpp b/src/ripple/app/tx/impl/AMMDeposit.cpp index 2fb9bd1eaab..5ea49f5445c 100644 --- a/src/ripple/app/tx/impl/AMMDeposit.cpp +++ b/src/ripple/app/tx/impl/AMMDeposit.cpp @@ -52,10 +52,12 @@ AMMDeposit::preflight(PreflightContext const& ctx) auto const amount2 = ctx.tx[~sfAmount2]; auto const ePrice = ctx.tx[~sfEPrice]; auto const lpTokens = ctx.tx[~sfLPTokenOut]; + auto const tradingFee = ctx.tx[~sfTradingFee]; // Valid options for the flags are: // tfLPTokens: LPTokenOut, [Amount, Amount2] // tfSingleAsset: Amount, [LPTokenOut] // tfTwoAsset: Amount, Amount2, [LPTokenOut] + // tfTwoAssetIfEmpty: Amount, Amount2, [sfTradingFee] // tfOnAssetLPToken: Amount and LPTokenOut // tfLimitLPToken: Amount and EPrice if (std::popcount(flags & tfDepositSubTx) != 1) @@ -66,29 +68,35 @@ AMMDeposit::preflight(PreflightContext const& ctx) if (flags & tfLPToken) { // if included then both amount and amount2 are deposit min - if (!lpTokens || ePrice || (amount && !amount2) || (!amount && amount2)) + if (!lpTokens || ePrice || (amount && !amount2) || + (!amount && amount2) || tradingFee) return temMALFORMED; } else if (flags & tfSingleAsset) { // if included then lpTokens is deposit min - if (!amount || amount2 || ePrice) + if (!amount || amount2 || ePrice || tradingFee) return temMALFORMED; } else if (flags & tfTwoAsset) { // if included then lpTokens is deposit min - if (!amount || !amount2 || ePrice) + if (!amount || !amount2 || ePrice || tradingFee) return temMALFORMED; } else if (flags & tfOneAssetLPToken) { - if (!amount || !lpTokens || amount2 || ePrice) + if (!amount || !lpTokens || amount2 || ePrice || tradingFee) return temMALFORMED; } else if (flags & tfLimitLPToken) { - if (!amount || !ePrice || lpTokens || amount2) + if (!amount || !ePrice || lpTokens || amount2 || tradingFee) + return temMALFORMED; + } + else if (flags & tfTwoAssetIfEmpty) + { + if (!amount || !amount2 || ePrice || lpTokens) return temMALFORMED; } @@ -148,6 +156,12 @@ AMMDeposit::preflight(PreflightContext const& ctx) } } + if (tradingFee > TRADING_FEE_THRESHOLD) + { + JLOG(ctx.j.debug()) << "AMM Deposit: invalid trading fee."; + return temBAD_FEE; + } + return preflight2(ctx); } @@ -174,12 +188,27 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) if (!expected) return expected.error(); auto const [amountBalance, amount2Balance, lptAMMBalance] = *expected; - if (amountBalance <= beast::zero || amount2Balance <= beast::zero || - lptAMMBalance <= beast::zero) + if (ctx.tx.getFlags() & tfTwoAssetIfEmpty) { - JLOG(ctx.j.debug()) - << "AMM Deposit: reserves or tokens balance is zero."; - return tecINTERNAL; + if (lptAMMBalance != beast::zero) + return tecAMM_NOT_EMPTY; + if (amountBalance != beast::zero || amount2Balance != beast::zero) + { + JLOG(ctx.j.debug()) << "AMM Deposit: tokens balance is not zero."; + return tecINTERNAL; + } + } + else + { + if (lptAMMBalance == beast::zero) + return tecAMM_EMPTY; + if (amountBalance <= beast::zero || amount2Balance <= beast::zero || + lptAMMBalance < beast::zero) + { + JLOG(ctx.j.debug()) + << "AMM Deposit: reserves or tokens balance is zero."; + return tecINTERNAL; + } } // Check account has sufficient funds. @@ -313,8 +342,6 @@ AMMDeposit::applyGuts(Sandbox& sb) return {tecINTERNAL, false}; auto const ammAccountID = (*ammSle)[sfAccount]; - auto const tfee = getTradingFee(ctx_.view(), *ammSle, account_); - auto const expected = ammHolds( sb, *ammSle, @@ -325,6 +352,9 @@ AMMDeposit::applyGuts(Sandbox& sb) if (!expected) return {expected.error(), false}; auto const [amountBalance, amount2Balance, lptAMMBalance] = *expected; + auto const tfee = (lptAMMBalance == beast::zero) + ? ctx_.tx[~sfTradingFee].value_or(0) + : getTradingFee(ctx_.view(), *ammSle, account_); auto const subTxType = ctx_.tx.getFlags() & tfDepositSubTx; @@ -382,6 +412,14 @@ AMMDeposit::applyGuts(Sandbox& sb) amount, amount2, tfee); + if (subTxType & tfTwoAssetIfEmpty) + return equalDepositInEmptyState( + sb, + ammAccountID, + *amount, + *amount2, + lptAMMBalance.issue(), + tfee); // should not happen. JLOG(j_.error()) << "AMM Deposit: invalid options."; return std::make_pair(tecINTERNAL, STAmount{}); @@ -391,6 +429,12 @@ AMMDeposit::applyGuts(Sandbox& sb) { assert(newLPTokenBalance > beast::zero); ammSle->setFieldAmount(sfLPTokenBalance, newLPTokenBalance); + // LP depositing into AMM empty state gets the auction slot + // and the voting + if (lptAMMBalance == beast::zero) + initializeFeeAuctionVote( + sb, ammSle, account_, lptAMMBalance.issue(), tfee); + sb.update(ammSle); } @@ -834,4 +878,27 @@ AMMDeposit::singleDepositEPrice( tfee); } +std::pair +AMMDeposit::equalDepositInEmptyState( + Sandbox& view, + AccountID const& ammAccount, + STAmount const& amount, + STAmount const& amount2, + Issue const& lptIssue, + std::uint16_t tfee) +{ + return deposit( + view, + ammAccount, + amount, + amount, + amount2, + STAmount{lptIssue, 0}, + ammLPTokens(amount, amount2, lptIssue), + std::nullopt, + std::nullopt, + std::nullopt, + tfee); +} + } // namespace ripple diff --git a/src/ripple/app/tx/impl/AMMDeposit.h b/src/ripple/app/tx/impl/AMMDeposit.h index 10da43594d3..385ce7c24e5 100644 --- a/src/ripple/app/tx/impl/AMMDeposit.h +++ b/src/ripple/app/tx/impl/AMMDeposit.h @@ -223,6 +223,23 @@ class AMMDeposit : public Transactor STAmount const& lptAMMBalance, STAmount const& ePrice, std::uint16_t tfee); + + /** Equal deposit in empty AMM state (LP tokens balance is 0) + * @param view + * @param ammAccount + * @param amount requested asset1 deposit amount + * @param amount2 requested asset2 deposit amount + * @param tfee + * @return + */ + std::pair + equalDepositInEmptyState( + Sandbox& view, + AccountID const& ammAccount, + STAmount const& amount, + STAmount const& amount2, + Issue const& lptIssue, + std::uint16_t tfee); }; } // namespace ripple diff --git a/src/ripple/app/tx/impl/AMMVote.cpp b/src/ripple/app/tx/impl/AMMVote.cpp index 09361356c3a..ff0598aaa40 100644 --- a/src/ripple/app/tx/impl/AMMVote.cpp +++ b/src/ripple/app/tx/impl/AMMVote.cpp @@ -69,6 +69,8 @@ AMMVote::preclaim(PreclaimContext const& ctx) JLOG(ctx.j.debug()) << "AMM Vote: Invalid asset pair."; return terNO_AMM; } + else if (ammSle->getFieldAmount(sfLPTokenBalance) == beast::zero) + return tecAMM_EMPTY; else if (auto const lpTokensNew = ammLPHolds(ctx.view, *ammSle, ctx.tx[sfAccount], ctx.j); lpTokensNew == beast::zero) @@ -208,17 +210,19 @@ applyVote( if (auto const discountedFee = fee / AUCTION_SLOT_DISCOUNTED_FEE_FRACTION) auctionSlot.setFieldU16(sfDiscountedFee, discountedFee); - else + else if (auctionSlot.isFieldPresent(sfDiscountedFee)) auctionSlot.makeFieldAbsent(sfDiscountedFee); } } else { - ammSle->makeFieldAbsent(sfTradingFee); + if (ammSle->isFieldPresent(sfTradingFee)) + ammSle->makeFieldAbsent(sfTradingFee); if (ammSle->isFieldPresent(sfAuctionSlot)) { auto& auctionSlot = ammSle->peekFieldObject(sfAuctionSlot); - auctionSlot.makeFieldAbsent(sfDiscountedFee); + if (auctionSlot.isFieldPresent(sfDiscountedFee)) + auctionSlot.makeFieldAbsent(sfDiscountedFee); } } sb.update(ammSle); diff --git a/src/ripple/app/tx/impl/AMMWithdraw.cpp b/src/ripple/app/tx/impl/AMMWithdraw.cpp index 8c583bc6170..58fc6b549ce 100644 --- a/src/ripple/app/tx/impl/AMMWithdraw.cpp +++ b/src/ripple/app/tx/impl/AMMWithdraw.cpp @@ -25,7 +25,6 @@ #include #include #include -#include #include #include @@ -195,8 +194,10 @@ AMMWithdraw::preclaim(PreclaimContext const& ctx) if (!expected) return expected.error(); auto const [amountBalance, amount2Balance, lptAMMBalance] = *expected; + if (lptAMMBalance == beast::zero) + return tecAMM_EMPTY; if (amountBalance <= beast::zero || amount2Balance <= beast::zero || - lptAMMBalance <= beast::zero) + lptAMMBalance < beast::zero) { JLOG(ctx.j.debug()) << "AMM Withdraw: reserves or tokens balance is zero."; @@ -301,6 +302,9 @@ AMMWithdraw::applyGuts(Sandbox& sb) if (!ammSle) return {tecINTERNAL, false}; auto const ammAccountID = (*ammSle)[sfAccount]; + auto const accountSle = sb.read(keylet::account(ammAccountID)); + if (!accountSle) + return {tecINTERNAL, false}; auto const lpTokens = ammLPHolds(ctx_.view(), *ammSle, ctx_.tx[sfAccount], ctx_.journal); auto const lpTokensWithdraw = @@ -372,19 +376,31 @@ AMMWithdraw::applyGuts(Sandbox& sb) return std::make_pair(tecINTERNAL, STAmount{}); }(); - // AMM is deleted if zero tokens balance - if (result == tesSUCCESS && newLPTokenBalance != beast::zero) + if (result != tesSUCCESS) + return {result, false}; + + bool updateBalance = true; + if (newLPTokenBalance == beast::zero) + { + if (auto const ter = + deleteAMMAccount(sb, ctx_.tx[sfAsset], ctx_.tx[sfAsset2], j_); + ter != tesSUCCESS && ter != tecINCOMPLETE) + return {ter, false}; + else + updateBalance = (ter == tecINCOMPLETE); + } + + if (updateBalance) { ammSle->setFieldAmount(sfLPTokenBalance, newLPTokenBalance); sb.update(ammSle); - - JLOG(ctx_.journal.trace()) - << "AMM Withdraw: tokens " << to_string(newLPTokenBalance.iou()) - << " " << to_string(lpTokens.iou()) << " " - << to_string(lptAMMBalance.iou()); } - return {result, result == tesSUCCESS}; + JLOG(ctx_.journal.trace()) + << "AMM Withdraw: tokens " << to_string(newLPTokenBalance.iou()) << " " + << to_string(lpTokens.iou()) << " " << to_string(lptAMMBalance.iou()); + + return {tesSUCCESS, true}; } TER @@ -401,24 +417,6 @@ AMMWithdraw::doApply() return result.first; } -TER -AMMWithdraw::deleteAccount(Sandbox& sb, AccountID const& ammAccountID) -{ - auto sleAMMRoot = sb.peek(keylet::account(ammAccountID)); - auto ammSle = sb.peek(keylet::amm(ctx_.tx[sfAsset], ctx_.tx[sfAsset2])); - - if (!sleAMMRoot || !ammSle) - return tecINTERNAL; - - // Note, the AMM trust lines are deleted since the balance - // goes to 0. It also means there are no linked - // ledger objects. - sb.erase(ammSle); - sb.erase(sleAMMRoot); - - return tesSUCCESS; -} - std::pair AMMWithdraw::withdraw( Sandbox& view, @@ -562,9 +560,6 @@ AMMWithdraw::withdraw( return {res, STAmount{}}; } - if (lpTokensWithdrawActual == lpTokensAMMBalance) - return {deleteAccount(view, ammAccount), STAmount{}}; - return {tesSUCCESS, lpTokensAMMBalance - lpTokensWithdrawActual}; } diff --git a/src/ripple/app/tx/impl/AMMWithdraw.h b/src/ripple/app/tx/impl/AMMWithdraw.h index 1e5b1a99e4f..40686266315 100644 --- a/src/ripple/app/tx/impl/AMMWithdraw.h +++ b/src/ripple/app/tx/impl/AMMWithdraw.h @@ -215,14 +215,6 @@ class AMMWithdraw : public Transactor STAmount const& amount, STAmount const& ePrice, std::uint16_t tfee); - - /** Delete AMM account. - * @param view - * @param ammAccountID - * @return - */ - TER - deleteAccount(Sandbox& view, AccountID const& ammAccountID); }; } // namespace ripple diff --git a/src/ripple/app/tx/impl/Clawback.cpp b/src/ripple/app/tx/impl/Clawback.cpp index 4fb4d4bc8f8..58546db5ca7 100644 --- a/src/ripple/app/tx/impl/Clawback.cpp +++ b/src/ripple/app/tx/impl/Clawback.cpp @@ -63,6 +63,9 @@ Clawback::preclaim(PreclaimContext const& ctx) if (!sleIssuer || !sleHolder) return terNO_ACCOUNT; + if (sleHolder->isFieldPresent(sfAMMID)) + return tecAMM_ACCOUNT; + std::uint32_t const issuerFlagsIn = sleIssuer->getFieldU32(sfFlags); // If AllowTrustLineClawback is not set or NoFreeze is set, return no diff --git a/src/ripple/app/tx/impl/CreateCheck.cpp b/src/ripple/app/tx/impl/CreateCheck.cpp index f5c2cbfbfd9..77ce4d017a1 100644 --- a/src/ripple/app/tx/impl/CreateCheck.cpp +++ b/src/ripple/app/tx/impl/CreateCheck.cpp @@ -97,6 +97,10 @@ CreateCheck::preclaim(PreclaimContext const& ctx) (flags & lsfDisallowIncomingCheck)) return tecNO_PERMISSION; + // AMM can not cash the check + if (sleDst->isFieldPresent(sfAMMID)) + return tecNO_PERMISSION; + if ((flags & lsfRequireDestTag) && !ctx.tx.isFieldPresent(sfDestinationTag)) { // The tag is basically account-specific information we don't diff --git a/src/ripple/app/tx/impl/DeleteAccount.cpp b/src/ripple/app/tx/impl/DeleteAccount.cpp index 62cc9e1fbbf..eeffc66d382 100644 --- a/src/ripple/app/tx/impl/DeleteAccount.cpp +++ b/src/ripple/app/tx/impl/DeleteAccount.cpp @@ -292,76 +292,29 @@ DeleteAccount::doApply() if (!src || !dst) return tefBAD_LEDGER; - // Delete all of the entries in the account directory. Keylet const ownerDirKeylet{keylet::ownerDir(account_)}; - std::shared_ptr sleDirNode{}; - unsigned int uDirEntry{0}; - uint256 dirEntry{beast::zero}; - - if (view().exists(ownerDirKeylet) && - dirFirst(view(), ownerDirKeylet.key, sleDirNode, uDirEntry, dirEntry)) - { - do - { - // Choose the right way to delete each directory node. - auto sleItem = view().peek(keylet::child(dirEntry)); - if (!sleItem) - { - // Directory node has an invalid index. Bail out. - JLOG(j_.fatal()) - << "DeleteAccount: Directory node in ledger " - << view().seq() << " has index to object that is missing: " - << to_string(dirEntry); - return tefBAD_LEDGER; - } - - LedgerEntryType const nodeType{safe_cast( - sleItem->getFieldU16(sfLedgerEntryType))}; - + auto const ter = cleanupOnAccountDelete( + view(), + ownerDirKeylet, + [&](LedgerEntryType nodeType, + uint256 const& dirEntry, + std::shared_ptr& sleItem) -> TER { if (auto deleter = nonObligationDeleter(nodeType)) { TER const result{ deleter(ctx_.app, view(), account_, dirEntry, sleItem, j_)}; - if (!isTesSuccess(result)) - return result; - } - else - { - assert(!"Undeletable entry should be found in preclaim."); - JLOG(j_.error()) - << "DeleteAccount undeletable item not found in preclaim."; - return tecHAS_OBLIGATIONS; - } - - // dirFirst() and dirNext() are like iterators with exposed - // internal state. We'll take advantage of that exposed state - // to solve a common C++ problem: iterator invalidation while - // deleting elements from a container. - // - // We have just deleted one directory entry, which means our - // "iterator state" is invalid. - // - // 1. During the process of getting an entry from the - // directory uDirEntry was incremented from 0 to 1. - // - // 2. We then deleted the entry at index 0, which means the - // entry that was at 1 has now moved to 0. - // - // 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) - { - JLOG(j_.error()) - << "DeleteAccount iterator re-validation failed."; - return tefBAD_LEDGER; + return result; } - uDirEntry = 0; - } while (dirNext( - view(), ownerDirKeylet.key, sleDirNode, uDirEntry, dirEntry)); - } + assert(!"Undeletable entry should be found in preclaim."); + JLOG(j_.error()) << "DeleteAccount undeletable item not " + "found in preclaim."; + return tecHAS_OBLIGATIONS; + }, + ctx_.journal); + if (ter != tesSUCCESS) + return ter; // Transfer any XRP remaining after the fee is paid to the destination: (*dst)[sfBalance] = (*dst)[sfBalance] + mSourceBalance; diff --git a/src/ripple/app/tx/impl/Escrow.cpp b/src/ripple/app/tx/impl/Escrow.cpp index 981cc57e8fe..9d8aa6a2289 100644 --- a/src/ripple/app/tx/impl/Escrow.cpp +++ b/src/ripple/app/tx/impl/Escrow.cpp @@ -162,7 +162,7 @@ EscrowCreate::preclaim(PreclaimContext const& ctx) auto const sled = ctx.view.read(keylet::account(ctx.tx[sfDestination])); if (!sled) return tecNO_DST; - if (((*sled)[sfFlags] & lsfAMM)) + if (sled->isFieldPresent(sfAMMID)) return tecNO_PERMISSION; return tesSUCCESS; diff --git a/src/ripple/app/tx/impl/InvariantCheck.cpp b/src/ripple/app/tx/impl/InvariantCheck.cpp index 23fa7b17115..907611f1c9a 100644 --- a/src/ripple/app/tx/impl/InvariantCheck.cpp +++ b/src/ripple/app/tx/impl/InvariantCheck.cpp @@ -321,12 +321,12 @@ AccountRootsNotDeleted::finalize( ReadView const&, beast::Journal const& j) { - // AMM account root can be deleted as the result of AMM withdraw + // AMM account root can be deleted as the result of AMM withdraw/delete // transaction when the total AMM LP Tokens balance goes to 0. - // Not every AMM withdraw deletes the AMM account, accountsDeleted_ - // is set if it is deleted. + // A successful AccountDelete or AMMDelete MUST delete exactly + // one account root. if ((tx.getTxnType() == ttACCOUNT_DELETE || - (tx.getTxnType() == ttAMM_WITHDRAW && accountsDeleted_ == 1)) && + tx.getTxnType() == ttAMM_DELETE) && result == tesSUCCESS) { if (accountsDeleted_ == 1) @@ -341,6 +341,13 @@ AccountRootsNotDeleted::finalize( return false; } + // A successful AMMWithdraw MAY delete one account root + // when the total AMM LP Tokens balance goes to 0. Not every AMM withdraw + // deletes the AMM account, accountsDeleted_ is set if it is deleted. + if (tx.getTxnType() == ttAMM_WITHDRAW && result == tesSUCCESS && + accountsDeleted_ == 1) + return true; + if (accountsDeleted_ == 0) return true; diff --git a/src/ripple/app/tx/impl/PayChan.cpp b/src/ripple/app/tx/impl/PayChan.cpp index ac85603957f..3fe2a28a7cf 100644 --- a/src/ripple/app/tx/impl/PayChan.cpp +++ b/src/ripple/app/tx/impl/PayChan.cpp @@ -234,7 +234,7 @@ PayChanCreate::preclaim(PreclaimContext const& ctx) (flags & lsfDisallowXRP)) return tecNO_TARGET; - if (flags & lsfAMM) + if (sled->isFieldPresent(sfAMMID)) return tecNO_PERMISSION; } diff --git a/src/ripple/app/tx/impl/Payment.cpp b/src/ripple/app/tx/impl/Payment.cpp index a23e41bac2a..d3c26a37023 100644 --- a/src/ripple/app/tx/impl/Payment.cpp +++ b/src/ripple/app/tx/impl/Payment.cpp @@ -468,7 +468,7 @@ Payment::doApply() // AMMs can never receive an XRP payment. // Must use AMMDeposit transaction instead. - if (sleDst->getFlags() & lsfAMM) + if (sleDst->isFieldPresent(sfAMMID)) return tecNO_PERMISSION; // The source account does have enough money. Make sure the diff --git a/src/ripple/app/tx/impl/SetTrust.cpp b/src/ripple/app/tx/impl/SetTrust.cpp index acbbedabf10..7869cc7027d 100644 --- a/src/ripple/app/tx/impl/SetTrust.cpp +++ b/src/ripple/app/tx/impl/SetTrust.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -128,20 +129,46 @@ SetTrust::preclaim(PreclaimContext const& ctx) } } + // This might be nullptr + auto const sleDst = ctx.view.read(keylet::account(uDstAccountID)); + // If the destination has opted to disallow incoming trustlines // then honour that flag if (ctx.view.rules().enabled(featureDisallowIncoming)) { - auto const sleDst = ctx.view.read(keylet::account(uDstAccountID)); - if (!sleDst) return tecNO_DST; - auto const dstFlags = sleDst->getFlags(); - if (dstFlags & lsfDisallowIncomingTrustline) + if (sleDst->getFlags() & lsfDisallowIncomingTrustline) return tecNO_PERMISSION; } + // If destination is AMM and the trustline doesn't exist then only + // allow SetTrust if the asset is AMM LP token and AMM is not + // in empty state. + if (ammEnabled(ctx.view.rules())) + { + if (!sleDst) + return tecNO_DST; + + if (sleDst->isFieldPresent(sfAMMID) && + !ctx.view.read(keylet::line(id, uDstAccountID, currency))) + { + if (auto const ammSle = + ctx.view.read({ltAMM, sleDst->getFieldH256(sfAMMID)})) + { + if (auto const lpTokens = + ammSle->getFieldAmount(sfLPTokenBalance); + lpTokens == beast::zero) + return tecAMM_EMPTY; + else if (lpTokens.getCurrency() != saLimitAmount.getCurrency()) + return tecNO_PERMISSION; + } + else + return tecINTERNAL; + } + } + return tesSUCCESS; } diff --git a/src/ripple/app/tx/impl/Transactor.cpp b/src/ripple/app/tx/impl/Transactor.cpp index 66aa10227d4..449392531b8 100644 --- a/src/ripple/app/tx/impl/Transactor.cpp +++ b/src/ripple/app/tx/impl/Transactor.cpp @@ -757,6 +757,32 @@ removeExpiredNFTokenOffers( } } +static void +removeDeletedTrustLines( + ApplyView& view, + std::vector const& trustLines, + beast::Journal viewJ) +{ + if (trustLines.size() > maxDeletableAMMTrustLines) + { + JLOG(viewJ.error()) + << "removeDeletedTrustLines: deleted trustlines exceed max " + << trustLines.size(); + return; + } + + for (auto const& index : trustLines) + { + if (auto const sleState = view.peek({ltRIPPLE_STATE, index}); + deleteAMMTrustLine(view, sleState, std::nullopt, viewJ) != + tesSUCCESS) + { + JLOG(viewJ.error()) + << "removeDeletedTrustLines: failed to delete AMM trustline"; + } + } +} + /** Reset the context, discarding any changes made and adjust the fee */ std::pair Transactor::reset(XRPAmount fee) @@ -849,7 +875,8 @@ Transactor::operator()() } else if ( (result == tecOVERSIZE) || (result == tecKILLED) || - (result == tecEXPIRED) || (isTecClaimHardFail(result, view().flags()))) + (result == tecINCOMPLETE) || (result == tecEXPIRED) || + (isTecClaimHardFail(result, view().flags()))) { JLOG(j_.trace()) << "reapplying because of " << transToken(result); @@ -858,10 +885,21 @@ Transactor::operator()() // should be used, making it possible to do more useful work // when transactions fail with a `tec` code. std::vector removedOffers; + std::vector removedTrustLines; + std::vector expiredNFTokenOffers; - if ((result == tecOVERSIZE) || (result == tecKILLED)) + bool const doOffers = + ((result == tecOVERSIZE) || (result == tecKILLED)); + bool const doLines = (result == tecINCOMPLETE); + bool const doNFTokenOffers = (result == tecEXPIRED); + if (doOffers || doLines || doNFTokenOffers) { - ctx_.visit([&removedOffers]( + ctx_.visit([&doOffers, + &removedOffers, + &doLines, + &removedTrustLines, + &doNFTokenOffers, + &expiredNFTokenOffers]( uint256 const& index, bool isDelete, std::shared_ptr const& before, @@ -869,30 +907,23 @@ Transactor::operator()() if (isDelete) { assert(before && after); - if (before && after && (before->getType() == ltOFFER) && + if (doOffers && before && after && + (before->getType() == ltOFFER) && (before->getFieldAmount(sfTakerPays) == after->getFieldAmount(sfTakerPays))) { // Removal of offer found or made unfunded removedOffers.push_back(index); } - } - }); - } - std::vector expiredNFTokenOffers; + if (doLines && before && after && + (before->getType() == ltRIPPLE_STATE)) + { + // Removal of obsolete AMM trust line + removedTrustLines.push_back(index); + } - if (result == tecEXPIRED) - { - ctx_.visit([&expiredNFTokenOffers]( - uint256 const& index, - bool isDelete, - std::shared_ptr const& before, - std::shared_ptr const& after) { - if (isDelete) - { - assert(before && after); - if (before && after && + if (doNFTokenOffers && before && after && (before->getType() == ltNFTOKEN_OFFER)) expiredNFTokenOffers.push_back(index); } @@ -917,6 +948,10 @@ Transactor::operator()() removeExpiredNFTokenOffers( view(), expiredNFTokenOffers, ctx_.app.journal("View")); + if (result == tecINCOMPLETE) + removeDeletedTrustLines( + view(), removedTrustLines, ctx_.app.journal("View")); + applied = isTecClaim(result); } diff --git a/src/ripple/app/tx/impl/applySteps.cpp b/src/ripple/app/tx/impl/applySteps.cpp index f0d092d793d..fdb84c271a0 100644 --- a/src/ripple/app/tx/impl/applySteps.cpp +++ b/src/ripple/app/tx/impl/applySteps.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -165,6 +166,8 @@ invoke_preflight(PreflightContext const& ctx) return invoke_preflight_helper(ctx); case ttAMM_BID: return invoke_preflight_helper(ctx); + case ttAMM_DELETE: + return invoke_preflight_helper(ctx); default: assert(false); return {temUNKNOWN, TxConsequences{temUNKNOWN}}; @@ -278,6 +281,8 @@ invoke_preclaim(PreclaimContext const& ctx) return invoke_preclaim(ctx); case ttAMM_BID: return invoke_preclaim(ctx); + case ttAMM_DELETE: + return invoke_preclaim(ctx); default: assert(false); return temUNKNOWN; @@ -353,6 +358,8 @@ invoke_calculateBaseFee(ReadView const& view, STTx const& tx) return AMMVote::calculateBaseFee(view, tx); case ttAMM_BID: return AMMBid::calculateBaseFee(view, tx); + case ttAMM_DELETE: + return AMMDelete::calculateBaseFee(view, tx); default: assert(false); return XRPAmount{0}; @@ -529,6 +536,10 @@ invoke_apply(ApplyContext& ctx) AMMBid p(ctx); return p(); } + case ttAMM_DELETE: { + AMMDelete p(ctx); + return p(); + } default: assert(false); return {temUNKNOWN, false}; diff --git a/src/ripple/ledger/View.h b/src/ripple/ledger/View.h index 939624ae24e..2c8d2354e0c 100644 --- a/src/ripple/ledger/View.h +++ b/src/ripple/ledger/View.h @@ -42,7 +42,7 @@ namespace ripple { -enum class WaiveTransferFee { Yes, No }; +enum class WaiveTransferFee : bool { No = false, Yes }; //------------------------------------------------------------------------------ // @@ -458,6 +458,33 @@ transferXRP( [[nodiscard]] TER requireAuth(ReadView const& view, Issue const& issue, AccountID const& account); +/** 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 + * specific account-owned object deletion. + * @return tecINCOMPLETE indicates maxNodesToDelete + * are deleted and there remains more nodes to delete. + */ +[[nodiscard]] TER +cleanupOnAccountDelete( + ApplyView& view, + Keylet const& ownerDirKeylet, + std::function&)> + deleter, + beast::Journal j, + std::optional maxNodesToDelete = std::nullopt); + +/** Delete trustline to AMM. The passed `sle` must be obtained from a prior + * call to view.peek(). Fail if neither side of the trustline is AMM or + * if ammAccountID is seated and is not one of the trustline's side. + */ +[[nodiscard]] TER +deleteAMMTrustLine( + ApplyView& view, + std::shared_ptr sleState, + std::optional const& ammAccountID, + beast::Journal j); + } // namespace ripple #endif diff --git a/src/ripple/ledger/impl/View.cpp b/src/ripple/ledger/impl/View.cpp index 2de92bfc20f..8d059aaf4b7 100644 --- a/src/ripple/ledger/impl/View.cpp +++ b/src/ripple/ledger/impl/View.cpp @@ -375,7 +375,7 @@ xrpLiquid( view.ownerCountHook(id, sle->getFieldU32(sfOwnerCount)), ownerCountAdj); // AMMs have no reserve requirement - auto const reserve = (sle->getFlags() & lsfAMM) + auto const reserve = sle->isFieldPresent(sfAMMID) ? XRPAmount{0} : view.fees().accountReserve(ownerCount); @@ -1544,4 +1544,129 @@ requireAuth(ReadView const& view, Issue const& issue, AccountID const& account) return tesSUCCESS; } +TER +cleanupOnAccountDelete( + ApplyView& view, + Keylet const& ownerDirKeylet, + std::function&)> + deleter, + beast::Journal j, + std::optional maxNodesToDelete) +{ + // Delete all the entries in the account directory. + std::shared_ptr sleDirNode{}; + unsigned int uDirEntry{0}; + uint256 dirEntry{beast::zero}; + std::uint32_t deleted = 0; + + if (view.exists(ownerDirKeylet) && + dirFirst(view, ownerDirKeylet.key, sleDirNode, uDirEntry, dirEntry)) + { + do + { + if (maxNodesToDelete && ++deleted > *maxNodesToDelete) + return tecINCOMPLETE; + + // Choose the right way to delete each directory node. + auto sleItem = view.peek(keylet::child(dirEntry)); + if (!sleItem) + { + // Directory node has an invalid index. Bail out. + JLOG(j.fatal()) + << "DeleteAccount: Directory node in ledger " << view.seq() + << " has index to object that is missing: " + << to_string(dirEntry); + return tefBAD_LEDGER; + } + + LedgerEntryType const nodeType{safe_cast( + sleItem->getFieldU16(sfLedgerEntryType))}; + + // Deleter handles the details of specific account-owned object + // deletion + if (auto const ter = deleter(nodeType, dirEntry, sleItem); + ter != tesSUCCESS) + return ter; + + // dirFirst() and dirNext() are like iterators with exposed + // internal state. We'll take advantage of that exposed state + // to solve a common C++ problem: iterator invalidation while + // deleting elements from a container. + // + // We have just deleted one directory entry, which means our + // "iterator state" is invalid. + // + // 1. During the process of getting an entry from the + // directory uDirEntry was incremented from 0 to 1. + // + // 2. We then deleted the entry at index 0, which means the + // entry that was at 1 has now moved to 0. + // + // 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) + { + JLOG(j.error()) + << "DeleteAccount iterator re-validation failed."; + return tefBAD_LEDGER; + } + uDirEntry = 0; + + } while ( + dirNext(view, ownerDirKeylet.key, sleDirNode, uDirEntry, dirEntry)); + } + + return tesSUCCESS; +} + +TER +deleteAMMTrustLine( + ApplyView& view, + std::shared_ptr sleState, + std::optional const& ammAccountID, + beast::Journal j) +{ + if (!sleState || sleState->getType() != ltRIPPLE_STATE) + return tecINTERNAL; + + auto const& [low, high] = std::minmax( + sleState->getFieldAmount(sfLowLimit).getIssuer(), + sleState->getFieldAmount(sfHighLimit).getIssuer()); + auto sleLow = view.peek(keylet::account(low)); + auto sleHigh = view.peek(keylet::account(high)); + if (!sleLow || !sleHigh) + return tecINTERNAL; + bool const ammLow = sleLow->isFieldPresent(sfAMMID); + bool const ammHigh = sleHigh->isFieldPresent(sfAMMID); + + // can't both be AMM + if (ammLow && ammHigh) + return tecINTERNAL; + + // at least one must be + if (!ammLow && !ammHigh) + return terNO_AMM; + + // one must be the target amm + if (ammAccountID && (low != *ammAccountID && high != *ammAccountID)) + return terNO_AMM; + + if (auto const ter = trustDelete(view, sleState, low, high, j); + ter != tesSUCCESS) + { + JLOG(j.error()) + << "deleteAMMTrustLine: failed to delete the trustline."; + return ter; + } + + auto const uFlags = !ammLow ? lsfLowReserve : lsfHighReserve; + if (!(sleState->getFlags() & uFlags)) + return tecINTERNAL; + + adjustOwnerCount(view, !ammLow ? sleLow : sleHigh, -1, j); + + return tesSUCCESS; +} + } // namespace ripple diff --git a/src/ripple/protocol/LedgerFormats.h b/src/ripple/protocol/LedgerFormats.h index a613c3a470d..b9205e7888a 100644 --- a/src/ripple/protocol/LedgerFormats.h +++ b/src/ripple/protocol/LedgerFormats.h @@ -249,7 +249,7 @@ enum LedgerSpecificFlags { 0x10000000, // True, reject new paychans lsfDisallowIncomingTrustline = 0x20000000, // True, reject new trustlines (only if no issued assets) - lsfAMM [[maybe_unused]] = 0x40000000, // True, AMM account + // 0x40000000 is available lsfAllowTrustLineClawback = 0x80000000, // True, enable clawback diff --git a/src/ripple/protocol/Protocol.h b/src/ripple/protocol/Protocol.h index 5df24271f68..6e4879cd746 100644 --- a/src/ripple/protocol/Protocol.h +++ b/src/ripple/protocol/Protocol.h @@ -95,6 +95,11 @@ using LedgerIndex = std::uint32_t; */ using TxID = uint256; +/** The maximum number of trustlines to delete as part of AMM account + * deletion cleanup. + */ +std::uint16_t constexpr maxDeletableAMMTrustLines = 512; + } // namespace ripple #endif diff --git a/src/ripple/protocol/TER.h b/src/ripple/protocol/TER.h index 955181de7e2..a2743bace8d 100644 --- a/src/ripple/protocol/TER.h +++ b/src/ripple/protocol/TER.h @@ -298,7 +298,11 @@ enum TECcodes : TERUnderlyingType { tecUNFUNDED_AMM = 162, tecAMM_BALANCE = 163, tecAMM_FAILED = 164, - tecAMM_INVALID_TOKENS = 165 + tecAMM_INVALID_TOKENS = 165, + tecAMM_EMPTY = 166, + tecAMM_NOT_EMPTY = 167, + tecAMM_ACCOUNT = 168, + tecINCOMPLETE = 169 }; //------------------------------------------------------------------------------ diff --git a/src/ripple/protocol/TxFlags.h b/src/ripple/protocol/TxFlags.h index 75d405cb827..39680e41d95 100644 --- a/src/ripple/protocol/TxFlags.h +++ b/src/ripple/protocol/TxFlags.h @@ -171,12 +171,13 @@ constexpr std::uint32_t tfSingleAsset = 0x00080000; constexpr std::uint32_t tfTwoAsset = 0x00100000; constexpr std::uint32_t tfOneAssetLPToken = 0x00200000; constexpr std::uint32_t tfLimitLPToken = 0x00400000; +constexpr std::uint32_t tfTwoAssetIfEmpty = 0x00800000; constexpr std::uint32_t tfWithdrawSubTx = tfLPToken | tfSingleAsset | tfTwoAsset | tfOneAssetLPToken | tfLimitLPToken | tfWithdrawAll | tfOneAssetWithdrawAll; constexpr std::uint32_t tfDepositSubTx = tfLPToken | tfSingleAsset | tfTwoAsset | tfOneAssetLPToken | - tfLimitLPToken; + tfLimitLPToken | tfTwoAssetIfEmpty; constexpr std::uint32_t tfWithdrawMask = ~(tfUniversal | tfWithdrawSubTx); constexpr std::uint32_t tfDepositMask = ~(tfUniversal | tfDepositSubTx); diff --git a/src/ripple/protocol/TxFormats.h b/src/ripple/protocol/TxFormats.h index a79f7dd79cb..2d7ba40c44c 100644 --- a/src/ripple/protocol/TxFormats.h +++ b/src/ripple/protocol/TxFormats.h @@ -157,6 +157,9 @@ enum TxType : std::uint16_t /** This transaction type bids for the auction slot */ ttAMM_BID = 39, + /** This transaction type deletes AMM in the empty state */ + ttAMM_DELETE = 40, + /** This system-generated transaction type is used to update the status of the various amendments. For details, see: https://xrpl.org/amendments.html diff --git a/src/ripple/protocol/impl/InnerObjectFormats.cpp b/src/ripple/protocol/impl/InnerObjectFormats.cpp index e6de3cc04e4..ba1a40a87ee 100644 --- a/src/ripple/protocol/impl/InnerObjectFormats.cpp +++ b/src/ripple/protocol/impl/InnerObjectFormats.cpp @@ -77,6 +77,12 @@ InnerObjectFormats::InnerObjectFormats() {sfPrice, soeREQUIRED}, {sfAuthAccounts, soeOPTIONAL}, }); + + add(sfAuthAccount.jsonName.c_str(), + sfAuthAccount.getCode(), + { + {sfAccount, soeREQUIRED}, + }); } InnerObjectFormats const& diff --git a/src/ripple/protocol/impl/LedgerFormats.cpp b/src/ripple/protocol/impl/LedgerFormats.cpp index 5228b625bb3..9192513457a 100644 --- a/src/ripple/protocol/impl/LedgerFormats.cpp +++ b/src/ripple/protocol/impl/LedgerFormats.cpp @@ -56,6 +56,7 @@ LedgerFormats::LedgerFormats() {sfMintedNFTokens, soeDEFAULT}, {sfBurnedNFTokens, soeDEFAULT}, {sfFirstNFTokenSequence, soeOPTIONAL}, + {sfAMMID, soeOPTIONAL}, }, commonFields); @@ -277,7 +278,7 @@ LedgerFormats::LedgerFormats() {sfAuctionSlot, soeOPTIONAL}, {sfLPTokenBalance, soeREQUIRED}, {sfAsset, soeREQUIRED}, - {sfAsset2, soeREQUIRED} + {sfAsset2, soeREQUIRED}, }, commonFields); diff --git a/src/ripple/protocol/impl/TER.cpp b/src/ripple/protocol/impl/TER.cpp index c16e9541fbf..29b87351204 100644 --- a/src/ripple/protocol/impl/TER.cpp +++ b/src/ripple/protocol/impl/TER.cpp @@ -45,6 +45,9 @@ transResults() MAKE_ERROR(tecAMM_BALANCE, "AMM has invalid balance."), MAKE_ERROR(tecAMM_INVALID_TOKENS, "AMM invalid LP tokens."), MAKE_ERROR(tecAMM_FAILED, "AMM transaction failed."), + MAKE_ERROR(tecAMM_EMPTY, "AMM is in empty state."), + MAKE_ERROR(tecAMM_NOT_EMPTY, "AMM is not in empty state."), + MAKE_ERROR(tecAMM_ACCOUNT, "This operation is not allowed on an AMM Account."), MAKE_ERROR(tecCLAIM, "Fee claimed. Sequence used. No action."), MAKE_ERROR(tecDIR_FULL, "Can not add entry to full directory."), MAKE_ERROR(tecFAILED_PROCESSING, "Failed to correctly process transaction."), @@ -92,6 +95,7 @@ transResults() MAKE_ERROR(tecINSUFFICIENT_FUNDS, "Not enough funds available to complete requested transaction."), MAKE_ERROR(tecOBJECT_NOT_FOUND, "A requested object could not be located."), MAKE_ERROR(tecINSUFFICIENT_PAYMENT, "The payment is not sufficient."), + MAKE_ERROR(tecINCOMPLETE, "Some work was completed, but more submissions required to finish."), MAKE_ERROR(tefALREADY, "The exact transaction was already in this ledger."), MAKE_ERROR(tefBAD_ADD_AUTH, "Not authorized to add account."), diff --git a/src/ripple/protocol/impl/TxFormats.cpp b/src/ripple/protocol/impl/TxFormats.cpp index 062174815c9..549e6619c1a 100644 --- a/src/ripple/protocol/impl/TxFormats.cpp +++ b/src/ripple/protocol/impl/TxFormats.cpp @@ -101,6 +101,7 @@ TxFormats::TxFormats() {sfEPrice, soeOPTIONAL}, {sfLPTokenOut, soeOPTIONAL}, {sfTicketSequence, soeOPTIONAL}, + {sfTradingFee, soeOPTIONAL}, }, commonFields); @@ -139,6 +140,15 @@ TxFormats::TxFormats() }, commonFields); + add(jss::AMMDelete, + ttAMM_DELETE, + { + {sfAsset, soeREQUIRED}, + {sfAsset2, soeREQUIRED}, + {sfTicketSequence, soeOPTIONAL}, + }, + commonFields); + add(jss::OfferCancel, ttOFFER_CANCEL, { diff --git a/src/ripple/protocol/jss.h b/src/ripple/protocol/jss.h index 2a7e2b14e82..a27a564e112 100644 --- a/src/ripple/protocol/jss.h +++ b/src/ripple/protocol/jss.h @@ -52,6 +52,7 @@ JSS(AMMBid); // transaction type JSS(AMMID); // field JSS(AMMCreate); // transaction type JSS(AMMDeposit); // transaction type +JSS(AMMDelete); // transaction type JSS(AMMVote); // transaction type JSS(AMMWithdraw); // transaction type JSS(Amendments); // ledger type. diff --git a/src/test/app/AMMExtended_test.cpp b/src/test/app/AMMExtended_test.cpp index 6b888ce8a2c..ad2c1f67805 100644 --- a/src/test/app/AMMExtended_test.cpp +++ b/src/test/app/AMMExtended_test.cpp @@ -32,7 +32,6 @@ #include #include #include -#include #include #include diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index 98804e77b74..b0e5106f9eb 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -27,7 +27,6 @@ #include #include #include -#include #include #include @@ -109,6 +108,15 @@ struct AMM_test : public jtx::AMMTest env.close(); AMM ammAlice(env, alice, XRP(10'000), USD(10'000)); } + + // Trading fee + testAMM( + [&](AMM& amm, Env&) { + BEAST_EXPECT(amm.expectTradingFee(1'000)); + BEAST_EXPECT(amm.expectAuctionSlot(100, 0, IOUAmount{0})); + }, + std::nullopt, + 1'000); } void @@ -392,6 +400,21 @@ struct AMM_test : public jtx::AMMTest AMM ammAlice1( env, alice, USD(10'000), USD1(10'000), ter(terNO_RIPPLE)); } + + // Issuer has clawback enabled + { + Env env(*this); + env.fund(XRP(1'000), gw); + env(fset(gw, asfAllowTrustLineClawback)); + fund(env, gw, {alice}, XRP(1'000), {USD(1'000)}, Fund::Acct); + env.close(); + AMM amm(env, gw, XRP(100), USD(100), ter(tecNO_PERMISSION)); + AMM amm1(env, alice, USD(100), XRP(100), ter(tecNO_PERMISSION)); + env(fclear(gw, asfAllowTrustLineClawback)); + env.close(); + // Can't be cleared + AMM amm2(env, gw, XRP(100), USD(100), ter(tecNO_PERMISSION)); + } } void @@ -416,96 +439,167 @@ struct AMM_test : public jtx::AMMTest std::optional, std::optional, std::optional, - std::optional>> + std::optional, + std::optional>> invalidOptions = { - // flags, tokens, asset1In, asset2in, EPrice - {tfLPToken, 1'000, std::nullopt, USD(100), std::nullopt}, - {tfLPToken, 1'000, XRP(100), std::nullopt, std::nullopt}, + // flags, tokens, asset1In, asset2in, EPrice, tfee + {tfLPToken, + 1'000, + std::nullopt, + USD(100), + std::nullopt, + std::nullopt}, {tfLPToken, 1'000, + XRP(100), std::nullopt, std::nullopt, - STAmount{USD, 1, -1}}, + std::nullopt}, + {tfLPToken, + 1'000, + std::nullopt, + std::nullopt, + STAmount{USD, 1, -1}, + std::nullopt}, {tfLPToken, std::nullopt, USD(100), std::nullopt, - STAmount{USD, 1, -1}}, + STAmount{USD, 1, -1}, + std::nullopt}, {tfLPToken, 1'000, XRP(100), std::nullopt, - STAmount{USD, 1, -1}}, + STAmount{USD, 1, -1}, + std::nullopt}, + {tfLPToken, + 1'000, + std::nullopt, + std::nullopt, + std::nullopt, + 1'000}, {tfSingleAsset, 1'000, std::nullopt, std::nullopt, + std::nullopt, std::nullopt}, {tfSingleAsset, std::nullopt, std::nullopt, USD(100), + std::nullopt, std::nullopt}, {tfSingleAsset, std::nullopt, std::nullopt, std::nullopt, - STAmount{USD, 1, -1}}, + STAmount{USD, 1, -1}, + std::nullopt}, + {tfSingleAsset, + std::nullopt, + USD(100), + std::nullopt, + std::nullopt, + 1'000}, {tfTwoAsset, 1'000, std::nullopt, std::nullopt, + std::nullopt, std::nullopt}, {tfTwoAsset, std::nullopt, XRP(100), USD(100), - STAmount{USD, 1, -1}}, + STAmount{USD, 1, -1}, + std::nullopt}, {tfTwoAsset, std::nullopt, XRP(100), std::nullopt, + std::nullopt, std::nullopt}, + {tfTwoAsset, + std::nullopt, + XRP(100), + USD(100), + std::nullopt, + 1'000}, {tfTwoAsset, std::nullopt, std::nullopt, USD(100), - STAmount{USD, 1, -1}}, + STAmount{USD, 1, -1}, + std::nullopt}, {tfOneAssetLPToken, 1'000, std::nullopt, std::nullopt, + std::nullopt, std::nullopt}, {tfOneAssetLPToken, std::nullopt, XRP(100), USD(100), + std::nullopt, std::nullopt}, {tfOneAssetLPToken, std::nullopt, XRP(100), std::nullopt, - STAmount{USD, 1, -1}}, + STAmount{USD, 1, -1}, + std::nullopt}, + {tfOneAssetLPToken, + 1'000, + XRP(100), + std::nullopt, + std::nullopt, + 1'000}, {tfLimitLPToken, 1'000, std::nullopt, std::nullopt, + std::nullopt, std::nullopt}, {tfLimitLPToken, 1'000, USD(100), std::nullopt, + std::nullopt, std::nullopt}, {tfLimitLPToken, std::nullopt, USD(100), XRP(100), + std::nullopt, std::nullopt}, {tfLimitLPToken, std::nullopt, + XRP(100), + std::nullopt, + STAmount{USD, 1, -1}, + 1'000}, + {tfTwoAssetIfEmpty, + std::nullopt, + std::nullopt, + std::nullopt, + std::nullopt, + 1'000}, + {tfTwoAssetIfEmpty, + 1'000, std::nullopt, std::nullopt, - STAmount{USD, 1, -1}}}; + std::nullopt, + std::nullopt}, + {tfTwoAssetIfEmpty, + std::nullopt, + XRP(100), + USD(100), + STAmount{USD, 1, -1}, + std::nullopt}, + }; for (auto const& it : invalidOptions) { ammAlice.deposit( @@ -517,6 +611,7 @@ struct AMM_test : public jtx::AMMTest std::get<0>(it), std::nullopt, std::nullopt, + std::get<5>(it), ter(temMALFORMED)); } @@ -543,6 +638,7 @@ struct AMM_test : public jtx::AMMTest std::nullopt, {{iss1, iss2}}, std::nullopt, + std::nullopt, ter(terNO_AMM)); } @@ -617,6 +713,7 @@ struct AMM_test : public jtx::AMMTest std::nullopt, std::nullopt, seq(1), + std::nullopt, ter(terNO_ACCOUNT)); // Invalid AMM @@ -629,6 +726,7 @@ struct AMM_test : public jtx::AMMTest std::nullopt, {{USD, GBP}}, std::nullopt, + std::nullopt, ter(terNO_AMM)); // Single deposit: 100000 tokens worth of USD @@ -642,6 +740,7 @@ struct AMM_test : public jtx::AMMTest std::nullopt, std::nullopt, std::nullopt, + std::nullopt, ter(tecAMM_FAILED)); // Single deposit: 100000 tokens worth of XRP @@ -655,6 +754,7 @@ struct AMM_test : public jtx::AMMTest std::nullopt, std::nullopt, std::nullopt, + std::nullopt, ter(tecAMM_FAILED)); // Deposit amount is invalid @@ -689,6 +789,15 @@ struct AMM_test : public jtx::AMMTest std::nullopt, std::nullopt, ter(tecAMM_INVALID_TOKENS)); + + // Deposit non-empty AMM + ammAlice.deposit( + carol, + XRP(100), + USD(100), + std::nullopt, + tfTwoAssetIfEmpty, + ter(tecAMM_NOT_EMPTY)); }); // Invalid AMM @@ -790,6 +899,7 @@ struct AMM_test : public jtx::AMMTest std::nullopt, std::nullopt, std::nullopt, + std::nullopt, ter(tecUNFUNDED_AMM)); }); @@ -809,6 +919,7 @@ struct AMM_test : public jtx::AMMTest std::nullopt, std::nullopt, std::nullopt, + std::nullopt, ter(tecUNFUNDED_AMM)); }); @@ -884,6 +995,7 @@ struct AMM_test : public jtx::AMMTest tfTwoAsset, std::nullopt, std::nullopt, + std::nullopt, ter(temBAD_AMM_TOKENS)); // min amounts can't be <= zero ammAlice.deposit( @@ -895,6 +1007,7 @@ struct AMM_test : public jtx::AMMTest tfTwoAsset, std::nullopt, std::nullopt, + std::nullopt, ter(temBAD_AMOUNT)); ammAlice.deposit( carol, @@ -905,6 +1018,7 @@ struct AMM_test : public jtx::AMMTest tfTwoAsset, std::nullopt, std::nullopt, + std::nullopt, ter(temBAD_AMOUNT)); // min amount bad currency ammAlice.deposit( @@ -916,6 +1030,7 @@ struct AMM_test : public jtx::AMMTest tfTwoAsset, std::nullopt, std::nullopt, + std::nullopt, ter(temBAD_CURRENCY)); // min amount bad token pair ammAlice.deposit( @@ -927,6 +1042,7 @@ struct AMM_test : public jtx::AMMTest tfTwoAsset, std::nullopt, std::nullopt, + std::nullopt, ter(temBAD_AMM_TOKENS)); ammAlice.deposit( carol, @@ -937,6 +1053,7 @@ struct AMM_test : public jtx::AMMTest tfTwoAsset, std::nullopt, std::nullopt, + std::nullopt, ter(temBAD_AMM_TOKENS)); }); @@ -952,6 +1069,7 @@ struct AMM_test : public jtx::AMMTest tfLPToken, std::nullopt, std::nullopt, + std::nullopt, ter(tecAMM_FAILED)); ammAlice.deposit( carol, @@ -962,6 +1080,7 @@ struct AMM_test : public jtx::AMMTest tfLPToken, std::nullopt, std::nullopt, + std::nullopt, ter(tecAMM_FAILED)); // Equal deposit by asset ammAlice.deposit( @@ -973,6 +1092,7 @@ struct AMM_test : public jtx::AMMTest tfTwoAsset, std::nullopt, std::nullopt, + std::nullopt, ter(tecAMM_FAILED)); // Single deposit by asset ammAlice.deposit( @@ -984,6 +1104,7 @@ struct AMM_test : public jtx::AMMTest tfSingleAsset, std::nullopt, std::nullopt, + std::nullopt, ter(tecAMM_FAILED)); }); } @@ -1250,6 +1371,16 @@ struct AMM_test : public jtx::AMMTest std::nullopt, std::nullopt, ter(temINVALID_FLAG)); + ammAlice.withdraw( + alice, + 1'000'000, + std::nullopt, + std::nullopt, + std::nullopt, + tfTwoAssetIfEmpty, + std::nullopt, + std::nullopt, + ter(temINVALID_FLAG)); // Invalid options std::vector cfg) { + cfg->FEES.reference_fee = XRPAmount(1); + return cfg; + }), + all); + fund(env, gw, {alice}, 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(); + } + // The trustlines are partially deleted, + // AMM is set to an empty state. + amm.withdrawAll(gw); + BEAST_EXPECT(amm.ammExists()); + + // Bid,Vote,Deposit,Withdraw,SetTrust failing with + // tecAMM_EMPTY. Deposit succeeds with tfTwoAssetIfEmpty option. + amm.bid( + alice, + 1000, + std::nullopt, + {}, + std::nullopt, + std::nullopt, + std::nullopt, + ter(tecAMM_EMPTY)); + amm.vote( + std::nullopt, + 100, + std::nullopt, + std::nullopt, + std::nullopt, + ter(tecAMM_EMPTY)); + amm.withdraw( + alice, 100, std::nullopt, std::nullopt, ter(tecAMM_EMPTY)); + amm.deposit( + alice, + USD(100), + std::nullopt, + std::nullopt, + std::nullopt, + ter(tecAMM_EMPTY)); + env(trust(alice, STAmount{amm.lptIssue(), 10'000}), + ter(tecAMM_EMPTY)); + + // Can deposit with tfTwoAssetIfEmpty option + amm.deposit( + alice, + std::nullopt, + XRP(10'000), + USD(10'000), + std::nullopt, + tfTwoAssetIfEmpty, + std::nullopt, + std::nullopt, + 1'000); + BEAST_EXPECT( + amm.expectBalances(XRP(10'000), USD(10'000), amm.tokens())); + BEAST_EXPECT(amm.expectTradingFee(1'000)); + BEAST_EXPECT(amm.expectAuctionSlot(100, 0, IOUAmount{0})); + + // Withdrawing all tokens deletes AMM since the number + // of remaining trustlines is less than max + amm.withdrawAll(alice); + BEAST_EXPECT(!amm.ammExists()); + BEAST_EXPECT(!env.le(keylet::ownerDir(amm.ammAccount()))); + } + + { + Env env( + *this, + envconfig([](std::unique_ptr cfg) { + cfg->FEES.reference_fee = XRPAmount(1); + return cfg; + }), + all); + fund(env, gw, {alice}, XRP(20'000), {USD(10'000)}); + AMM amm(env, gw, XRP(10'000), USD(10'000)); + for (auto i = 0; i < maxDeletableAMMTrustLines * 2 + 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(); + } + // The trustlines are partially deleted. + amm.withdrawAll(gw); + BEAST_EXPECT(amm.ammExists()); + + // AMMDelete has to be called twice to delete AMM. + amm.ammDelete(alice, ter(tecINCOMPLETE)); + BEAST_EXPECT(amm.ammExists()); + // Deletes remaining trustlines and deletes AMM. + amm.ammDelete(alice); + BEAST_EXPECT(!amm.ammExists()); + BEAST_EXPECT(!env.le(keylet::ownerDir(amm.ammAccount()))); + } + } + + void + testClawback() + { + testcase("Clawback"); + using namespace jtx; + Env env(*this); + env.fund(XRP(2'000), gw); + env.fund(XRP(2'000), alice); + AMM amm(env, gw, XRP(1'000), USD(1'000)); + env(fset(gw, asfAllowTrustLineClawback), ter(tecOWNERS)); + } + + void + testAMMID() + { + testcase("AMMID"); + using namespace jtx; + testAMM([&](AMM& amm, Env& env) { + amm.setClose(false); + auto const info = env.rpc( + "json", + "account_info", + std::string( + "{\"account\": \"" + to_string(amm.ammAccount()) + "\"}")); + try + { + BEAST_EXPECT( + info[jss::result][jss::account_data][jss::AMMID] + .asString() == to_string(amm.ammID())); + } + catch (...) + { + fail(); + } + amm.deposit(carol, 1'000); + auto affected = env.meta()->getJson( + JsonOptions::none)[sfAffectedNodes.fieldName]; + try + { + bool found = false; + for (auto const& node : affected) + { + if (node.isMember(sfModifiedNode.fieldName) && + node[sfModifiedNode.fieldName] + [sfLedgerEntryType.fieldName] + .asString() == "AccountRoot" && + node[sfModifiedNode.fieldName][sfFinalFields.fieldName] + [jss::Account] + .asString() == to_string(amm.ammAccount())) + { + found = node[sfModifiedNode.fieldName] + [sfFinalFields.fieldName][jss::AMMID] + .asString() == to_string(amm.ammID()); + break; + } + } + BEAST_EXPECT(found); + } + catch (...) + { + fail(); + } + }); + } + + void + testSelection() + { + testcase("Offer/Strand Selection"); + using namespace jtx; + Account const ed("ed"); + Account const gw1("gw1"); + auto const ETH = gw1["ETH"]; + auto const CAN = gw1["CAN"]; + + // These tests are expected to fail if the OwnerPaysFee feature + // is ever supported. Updates will need to be made to AMM handling + // in the payment engine, and these tests will need to be updated. + + auto prep = [&](Env& env, auto gwRate, auto gw1Rate) { + fund(env, gw, {alice, carol, bob, ed}, XRP(2'000), {USD(2'000)}); + env.fund(XRP(2'000), gw1); + fund( + env, + gw1, + {alice, carol, bob, ed}, + {ETH(2'000), CAN(2'000)}, + Fund::IOUOnly); + env(rate(gw, gwRate)); + env(rate(gw1, gw1Rate)); + env.close(); + }; + + for (auto const& rates : + {std::make_pair(1.5, 1.9), std::make_pair(1.9, 1.5)}) + { + // Offer Selection + + // Cross-currency payment: AMM has the same spot price quality + // as CLOB's offer and can't generate a better quality offer. + // The transfer fee in this case doesn't change the CLOB quality + // because trIn is ignored on adjustment and trOut on payment is + // also ignored because ownerPaysTransferFee is false in this case. + // Run test for 0) offer, 1) AMM, 2) offer and AMM + // to verify that the quality is better in the first case, + // and CLOB is selected in the second case. + { + std::array q; + for (auto i = 0; i < 3; ++i) + { + Env env(*this); + prep(env, rates.first, rates.second); + std::optional amm; + if (i == 0 || i == 2) + { + env(offer(ed, ETH(400), USD(400)), txflags(tfPassive)); + env.close(); + } + if (i > 0) + amm.emplace(env, ed, USD(1'000), ETH(1'000)); + env(pay(carol, bob, USD(100)), + path(~USD), + sendmax(ETH(500))); + env.close(); + // CLOB and AMM, AMM is not selected + if (i == 2) + { + BEAST_EXPECT(amm->expectBalances( + USD(1'000), ETH(1'000), amm->tokens())); + } + BEAST_EXPECT(expectLine(env, bob, USD(2'100))); + q[i] = Quality(Amounts{ + ETH(2'000) - env.balance(carol, ETH), + env.balance(bob, USD) - USD(2'000)}); + } + // CLOB is better quality than AMM + BEAST_EXPECT(q[0] > q[1]); + // AMM is not selected with CLOB + BEAST_EXPECT(q[0] == q[2]); + } + // Offer crossing: AMM has the same spot price quality + // as CLOB's offer and can't generate a better quality offer. + // The transfer fee in this case doesn't change the CLOB quality + // because the quality adjustment is ignored for the offer crossing. + for (auto i = 0; i < 3; ++i) + { + Env env(*this); + prep(env, rates.first, rates.second); + std::optional amm; + if (i == 0 || i == 2) + { + env(offer(ed, ETH(400), USD(400)), txflags(tfPassive)); + env.close(); + } + if (i > 0) + amm.emplace(env, ed, USD(1'000), ETH(1'000)); + env(offer(alice, USD(400), ETH(400))); + env.close(); + // AMM is not selected + if (i > 0) + { + BEAST_EXPECT(amm->expectBalances( + USD(1'000), ETH(1'000), amm->tokens())); + } + if (i == 0 || i == 2) + { + // Fully crosses + BEAST_EXPECT(expectOffers(env, alice, 0)); + } + // Fails to cross because AMM is not selected + else + { + BEAST_EXPECT(expectOffers( + env, alice, 1, {Amounts{USD(400), ETH(400)}})); + } + BEAST_EXPECT(expectOffers(env, ed, 0)); + } + + // Show that the CLOB quality reduction + // results in AMM offer selection. + + // Same as the payment but reduced offer quality + { + std::array q; + for (auto i = 0; i < 3; ++i) + { + Env env(*this); + prep(env, rates.first, rates.second); + std::optional amm; + if (i == 0 || i == 2) + { + env(offer(ed, ETH(400), USD(300)), txflags(tfPassive)); + env.close(); + } + if (i > 0) + amm.emplace(env, ed, USD(1'000), ETH(1'000)); + env(pay(carol, bob, USD(100)), + path(~USD), + sendmax(ETH(500))); + env.close(); + // AMM and CLOB are selected + if (i > 0) + { + BEAST_EXPECT(!amm->expectBalances( + USD(1'000), ETH(1'000), amm->tokens())); + } + if (i == 2) + { + if (rates.first == 1.5) + { + BEAST_EXPECT(expectOffers( + env, + ed, + 1, + {{Amounts{ + STAmount{ + ETH, UINT64_C(378'6327949540823), -13}, + STAmount{ + USD, + UINT64_C(283'9745962155617), + -13}}}})); + } + else + { + BEAST_EXPECT(expectOffers( + env, + ed, + 1, + {{Amounts{ + STAmount{ + ETH, UINT64_C(325'299461620749), -12}, + STAmount{ + USD, + UINT64_C(243'9745962155617), + -13}}}})); + } + } + BEAST_EXPECT(expectLine(env, bob, USD(2'100))); + q[i] = Quality(Amounts{ + ETH(2'000) - env.balance(carol, ETH), + env.balance(bob, USD) - USD(2'000)}); + } + // AMM is better quality + BEAST_EXPECT(q[1] > q[0]); + // AMM and CLOB produce better quality + BEAST_EXPECT(q[2] > q[1]); + } + + // Same as the offer-crossing but reduced offer quality + for (auto i = 0; i < 3; ++i) + { + Env env(*this); + prep(env, rates.first, rates.second); + std::optional amm; + if (i == 0 || i == 2) + { + env(offer(ed, ETH(400), USD(250)), txflags(tfPassive)); + env.close(); + } + if (i > 0) + amm.emplace(env, ed, USD(1'000), ETH(1'000)); + env(offer(alice, USD(250), ETH(400))); + env.close(); + // AMM is selected in both cases + if (i > 0) + { + BEAST_EXPECT(!amm->expectBalances( + USD(1'000), ETH(1'000), amm->tokens())); + } + // Partially crosses, AMM is selected, CLOB fails limitQuality + if (i == 2) + { + if (rates.first == 1.5) + { + BEAST_EXPECT(expectOffers( + env, ed, 1, {{Amounts{ETH(400), USD(250)}}})); + BEAST_EXPECT(expectOffers( + env, + alice, + 1, + {{Amounts{ + STAmount{USD, UINT64_C(40'5694150420947), -13}, + STAmount{ETH, UINT64_C(64'91106406735152), -14}, + }}})); + } + else + { + // Ed offer is partially crossed. + BEAST_EXPECT(expectOffers( + env, + ed, + 1, + {{Amounts{ + STAmount{ETH, UINT64_C(335'0889359326485), -13}, + STAmount{USD, UINT64_C(209'4305849579053), -13}, + }}})); + BEAST_EXPECT(expectOffers(env, alice, 0)); + } + } + } + + // Strand selection + + // Two book steps strand quality is 1. + // AMM strand's best quality is equal to AMM's spot price + // quality, which is 1. Both strands (steps) are adjusted + // for the transfer fee in qualityUpperBound. In case + // of two strands, AMM offers have better quality and are consumed + // first, remaining liquidity is generated by CLOB offers. + // Liquidity from two strands is better in this case than in case + // of one strand with two book steps. Liquidity from one strand + // with AMM has better quality than either one strand with two book + // steps or two strands. It may appear unintuitive, but one strand + // with AMM is optimized and generates one AMM offer, while in case + // of two strands, multiple AMM offers are generated, which results + // in slightly worse overall quality. + { + std::array q; + for (auto i = 0; i < 3; ++i) + { + Env env(*this); + prep(env, rates.first, rates.second); + std::optional amm; + + if (i == 0 || i == 2) + { + env(offer(ed, ETH(400), CAN(400)), txflags(tfPassive)); + env(offer(ed, CAN(400), USD(400))), txflags(tfPassive); + env.close(); + } + + if (i > 0) + amm.emplace(env, ed, ETH(1'000), USD(1'000)); + + env(pay(carol, bob, USD(100)), + path(~USD), + path(~CAN, ~USD), + sendmax(ETH(600))); + env.close(); + + BEAST_EXPECT(expectLine(env, bob, USD(2'100))); + + if (i == 2) + { + if (rates.first == 1.5) + { + // Liquidity is consumed from AMM strand only + BEAST_EXPECT(amm->expectBalances( + STAmount{ETH, UINT64_C(1'176'66038955758), -11}, + USD(850), + amm->tokens())); + } + else + { + BEAST_EXPECT(amm->expectBalances( + STAmount{ + ETH, UINT64_C(1'179'540094339627), -12}, + STAmount{USD, UINT64_C(847'7880529867501), -13}, + amm->tokens())); + BEAST_EXPECT(expectOffers( + env, + ed, + 2, + {{Amounts{ + STAmount{ + ETH, + UINT64_C(343'3179205198749), + -13}, + STAmount{ + CAN, + UINT64_C(343'3179205198749), + -13}, + }, + Amounts{ + STAmount{ + CAN, + UINT64_C(362'2119470132499), + -13}, + STAmount{ + USD, + UINT64_C(362'2119470132499), + -13}, + }}})); + } + } + q[i] = Quality(Amounts{ + ETH(2'000) - env.balance(carol, ETH), + env.balance(bob, USD) - USD(2'000)}); + } + BEAST_EXPECT(q[1] > q[0]); + BEAST_EXPECT(q[2] > q[0] && q[2] < q[1]); + } + } + } + void testCore() { @@ -4154,6 +4811,10 @@ struct AMM_test : public jtx::AMMTest testAMMAndCLOB(); testTradingFee(); testAdjustedTokens(); + testAutoDelete(); + testClawback(); + testAMMID(); + testSelection(); } void @@ -4166,4 +4827,4 @@ struct AMM_test : public jtx::AMMTest BEAST_DEFINE_TESTSUITE_PRIO(AMM, app, ripple, 1); } // namespace test -} // namespace ripple \ No newline at end of file +} // namespace ripple diff --git a/src/test/app/CrossingLimits_test.cpp b/src/test/app/CrossingLimits_test.cpp index d2ae9799ce8..09cca1e82af 100644 --- a/src/test/app/CrossingLimits_test.cpp +++ b/src/test/app/CrossingLimits_test.cpp @@ -18,7 +18,6 @@ #include #include #include -#include namespace ripple { namespace test { diff --git a/src/test/app/Escrow_test.cpp b/src/test/app/Escrow_test.cpp index 255325ca8ac..083764fd371 100644 --- a/src/test/app/Escrow_test.cpp +++ b/src/test/app/Escrow_test.cpp @@ -24,7 +24,6 @@ #include #include #include -#include #include #include diff --git a/src/test/app/Flow_test.cpp b/src/test/app/Flow_test.cpp index b9d2b715aa5..131cad6f042 100644 --- a/src/test/app/Flow_test.cpp +++ b/src/test/app/Flow_test.cpp @@ -28,7 +28,6 @@ #include #include #include -#include namespace ripple { namespace test { diff --git a/src/test/app/Freeze_test.cpp b/src/test/app/Freeze_test.cpp index fc51a6869a6..cb4653c086d 100644 --- a/src/test/app/Freeze_test.cpp +++ b/src/test/app/Freeze_test.cpp @@ -22,7 +22,6 @@ #include #include #include -#include namespace ripple { diff --git a/src/test/app/Offer_test.cpp b/src/test/app/Offer_test.cpp index e54f9706b36..1c8e544af30 100644 --- a/src/test/app/Offer_test.cpp +++ b/src/test/app/Offer_test.cpp @@ -22,7 +22,6 @@ #include #include #include -#include #include namespace ripple { diff --git a/src/test/app/Path_test.cpp b/src/test/app/Path_test.cpp index ea2dc51132a..0f9916cbcff 100644 --- a/src/test/app/Path_test.cpp +++ b/src/test/app/Path_test.cpp @@ -35,7 +35,6 @@ #include #include #include -#include #include #include diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp index dd82a622cc9..f8162dd107d 100644 --- a/src/test/app/PayChan_test.cpp +++ b/src/test/app/PayChan_test.cpp @@ -25,7 +25,6 @@ #include #include #include -#include #include diff --git a/src/test/app/PayStrand_test.cpp b/src/test/app/PayStrand_test.cpp index 4eb6c9fecb6..a70ab099745 100644 --- a/src/test/app/PayStrand_test.cpp +++ b/src/test/app/PayStrand_test.cpp @@ -29,7 +29,6 @@ #include "ripple/app/paths/AMMContext.h" #include #include -#include namespace ripple { namespace test { diff --git a/src/test/app/TrustAndBalance_test.cpp b/src/test/app/TrustAndBalance_test.cpp index f83eeac27ec..5b0c1d6480e 100644 --- a/src/test/app/TrustAndBalance_test.cpp +++ b/src/test/app/TrustAndBalance_test.cpp @@ -22,7 +22,6 @@ #include #include #include -#include #include namespace ripple { diff --git a/src/test/jtx.h b/src/test/jtx.h index bcf51398d5c..dd987472cc8 100644 --- a/src/test/jtx.h +++ b/src/test/jtx.h @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include diff --git a/src/test/jtx/AMM.h b/src/test/jtx/AMM.h index dd7e91a3ef8..3cf06bfe401 100644 --- a/src/test/jtx/AMM.h +++ b/src/test/jtx/AMM.h @@ -65,8 +65,10 @@ class AMM Account const creatorAccount_; STAmount const asset1_; STAmount const asset2_; + uint256 const ammID_; IOUAmount const initialLPTokens_; bool log_; + bool doClose_; // Predict next purchase price IOUAmount lastPurchasePrice_; std::optional bidMin_; @@ -180,6 +182,7 @@ class AMM std::optional const& flags, std::optional> const& assets, std::optional const& seq, + std::optional const& tfee = std::nullopt, std::optional const& ter = std::nullopt); IOUAmount @@ -286,6 +289,23 @@ class AMM return ammRpcInfo(lp); } + void + ammDelete( + AccountID const& deleter, + std::optional const& ter = std::nullopt); + + void + setClose(bool close) + { + doClose_ = close; + } + + uint256 + ammID() const + { + return ammID_; + } + private: void setTokens( diff --git a/src/test/jtx/TestHelpers.h b/src/test/jtx/TestHelpers.h index a29a894452b..ff681ffa50b 100644 --- a/src/test/jtx/TestHelpers.h +++ b/src/test/jtx/TestHelpers.h @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -405,6 +406,38 @@ STPathElement allpe(AccountID const& a, Issue const& iss); /***************************************************************/ +/* Check */ +/***************************************************************/ +namespace check { + +/** Create a check. */ +// clang-format off +template + requires std::is_same_v +Json::Value +create(A const& account, A const& dest, STAmount const& sendMax) +{ + Json::Value jv; + jv[sfAccount.jsonName] = to_string(account); + jv[sfSendMax.jsonName] = sendMax.getJson(JsonOptions::none); + jv[sfDestination.jsonName] = to_string(dest); + jv[sfTransactionType.jsonName] = jss::CheckCreate; + jv[sfFlags.jsonName] = tfUniversal; + return jv; +} +// clang-format on + +inline Json::Value +create( + jtx::Account const& account, + jtx::Account const& dest, + STAmount const& sendMax) +{ + return create(account.id(), dest.id(), sendMax); +} + +} // namespace check + } // namespace jtx } // namespace test } // namespace ripple diff --git a/src/test/jtx/check.h b/src/test/jtx/check.h index 325e5897258..6bad6b9db5e 100644 --- a/src/test/jtx/check.h +++ b/src/test/jtx/check.h @@ -31,13 +31,6 @@ namespace jtx { /** Check operations. */ namespace check { -/** Create a check. */ -Json::Value -create( - jtx::Account const& account, - jtx::Account const& dest, - STAmount const& sendMax); - /** Cash a check requiring that a specific amount be delivered. */ Json::Value cash(jtx::Account const& dest, uint256 const& checkId, STAmount const& amount); diff --git a/src/test/jtx/impl/AMM.cpp b/src/test/jtx/impl/AMM.cpp index 37efdc112a0..c09d496f439 100644 --- a/src/test/jtx/impl/AMM.cpp +++ b/src/test/jtx/impl/AMM.cpp @@ -62,8 +62,10 @@ AMM::AMM( , creatorAccount_(account) , asset1_(asset1) , asset2_(asset2) + , ammID_(keylet::amm(asset1_.issue(), asset2_.issue()).key) , initialLPTokens_(initialTokens(asset1, asset2)) , log_(log) + , doClose_(true) , lastPurchasePrice_(0) , bidMin_() , bidMax_() @@ -382,6 +384,7 @@ AMM::deposit( flags, std::nullopt, std::nullopt, + std::nullopt, ter); } @@ -404,6 +407,7 @@ AMM::deposit( flags, std::nullopt, std::nullopt, + std::nullopt, ter); } @@ -417,6 +421,7 @@ AMM::deposit( std::optional const& flags, std::optional> const& assets, std::optional const& seq, + std::optional const& tfee, std::optional const& ter) { Json::Value jv; @@ -428,6 +433,8 @@ AMM::deposit( asset2In->setJson(jv[jss::Amount2]); if (maxEP) maxEP->setJson(jv[jss::EPrice]); + if (tfee) + jv[jss::TradingFee] = *tfee; std::uint32_t jvflags = 0; if (flags) jvflags = *flags; @@ -671,7 +678,8 @@ AMM::submit( env_(jv, *ter); else env_(jv); - env_.close(); + if (doClose_) + env_.close(); } bool @@ -697,6 +705,18 @@ AMM::expectAuctionSlot(auto&& cb) const return false; } +void +AMM::ammDelete(AccountID const& deleter, std::optional const& ter) +{ + Json::Value jv; + jv[jss::Account] = to_string(deleter); + setTokens(jv); + jv[jss::TransactionType] = jss::AMMDelete; + if (fee_ != 0) + jv[jss::Fee] = std::to_string(fee_); + submit(jv, std::nullopt, ter); +} + namespace amm { Json::Value trust(AccountID const& account, STAmount const& amount, std::uint32_t flags) diff --git a/src/test/jtx/impl/check.cpp b/src/test/jtx/impl/check.cpp index 8abcaea1b34..d862130bc70 100644 --- a/src/test/jtx/impl/check.cpp +++ b/src/test/jtx/impl/check.cpp @@ -27,22 +27,6 @@ namespace jtx { namespace check { -// Create a check. -Json::Value -create( - jtx::Account const& account, - jtx::Account const& dest, - STAmount const& sendMax) -{ - Json::Value jv; - jv[sfAccount.jsonName] = account.human(); - jv[sfSendMax.jsonName] = sendMax.getJson(JsonOptions::none); - jv[sfDestination.jsonName] = dest.human(); - jv[sfTransactionType.jsonName] = jss::CheckCreate; - jv[sfFlags.jsonName] = tfUniversal; - return jv; -} - // Cash a check requiring that a specific amount be delivered. Json::Value cash(jtx::Account const& dest, uint256 const& checkId, STAmount const& amount) diff --git a/src/test/rpc/AccountOffers_test.cpp b/src/test/rpc/AccountOffers_test.cpp index 4979ac07216..d94442ea8e5 100644 --- a/src/test/rpc/AccountOffers_test.cpp +++ b/src/test/rpc/AccountOffers_test.cpp @@ -19,7 +19,6 @@ #include #include -#include namespace ripple { namespace test { diff --git a/src/test/rpc/LedgerData_test.cpp b/src/test/rpc/LedgerData_test.cpp index ebc1b9fb553..f0811ba34c4 100644 --- a/src/test/rpc/LedgerData_test.cpp +++ b/src/test/rpc/LedgerData_test.cpp @@ -20,7 +20,6 @@ #include #include #include -#include namespace ripple { diff --git a/src/test/rpc/NoRipple_test.cpp b/src/test/rpc/NoRipple_test.cpp index 268cd1f14a3..472706f0b73 100644 --- a/src/test/rpc/NoRipple_test.cpp +++ b/src/test/rpc/NoRipple_test.cpp @@ -20,7 +20,6 @@ #include #include #include -#include namespace ripple {