diff --git a/include/xrpl/ledger/helpers/AMMHelpers.h b/include/xrpl/ledger/helpers/AMMHelpers.h index 173b4924ae1..34597b3cb5c 100644 --- a/include/xrpl/ledger/helpers/AMMHelpers.h +++ b/include/xrpl/ledger/helpers/AMMHelpers.h @@ -1,8 +1,13 @@ #pragma once +#include #include #include #include +#include +#include +#include +#include #include #include #include @@ -11,6 +16,7 @@ #include #include #include +#include namespace xrpl { @@ -713,4 +719,94 @@ adjustFracByTokens( STAmount const& tokens, Number const& frac); +/** Get AMM pool balances. + */ +std::pair +ammPoolHolds( + ReadView const& view, + AccountID const& ammAccountID, + Asset const& asset1, + Asset const& asset2, + FreezeHandling freezeHandling, + AuthHandling authHandling, + beast::Journal const j); + +/** Get AMM pool and LP token balances. If both optIssue are + * provided then they are used as the AMM token pair issues. + * Otherwise the missing issues are fetched from ammSle. + */ +Expected, TER> +ammHolds( + ReadView const& view, + SLE const& ammSle, + std::optional const& optAsset1, + std::optional const& optAsset2, + FreezeHandling freezeHandling, + AuthHandling authHandling, + beast::Journal const j); + +/** Get the balance of LP tokens. + */ +STAmount +ammLPHolds( + ReadView const& view, + Asset const& asset1, + Asset const& asset2, + AccountID const& ammAccount, + AccountID const& lpAccount, + beast::Journal const j); + +STAmount +ammLPHolds( + ReadView const& view, + SLE const& ammSle, + AccountID const& lpAccount, + beast::Journal const j); + +/** Get AMM trading fee for the given account. The fee is discounted + * if the account is the auction slot owner or one of the slot's authorized + * accounts. + */ +std::uint16_t +getTradingFee(ReadView const& view, SLE const& ammSle, AccountID const& account); + +/** Returns total amount held by AMM for the given token. + */ +STAmount +ammAccountHolds(ReadView const& view, AccountID const& ammAccountID, Asset const& asset); + +/** Delete trustlines to AMM. If all trustlines are deleted then + * AMM object and account are deleted. Otherwise tecINCOMPLETE is returned. + */ +TER +deleteAMMAccount(Sandbox& view, Asset const& asset, Asset 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, + Asset const& lptAsset, + std::uint16_t tfee); + +/** Return true if the Liquidity Provider is the only AMM provider, false + * otherwise. Return tecINTERNAL if encountered an unexpected condition, + * for instance Liquidity Provider has more than one LPToken trustline. + */ +Expected +isOnlyLiquidityProvider(ReadView const& view, Issue const& ammIssue, AccountID const& lpAccount); + +/** Due to rounding, the LPTokenBalance of the last LP might + * not match the LP's trustline balance. If it's within the tolerance, + * update LPTokenBalance to match the LP's trustline balance. + */ +Expected +verifyAndAdjustLPTokenBalance( + Sandbox& sb, + STAmount const& lpTokens, + std::shared_ptr& ammSle, + AccountID const& account); + } // namespace xrpl diff --git a/include/xrpl/ledger/helpers/AMMUtils.h b/include/xrpl/ledger/helpers/AMMUtils.h index a9e9c9344a5..e69de29bb2d 100644 --- a/include/xrpl/ledger/helpers/AMMUtils.h +++ b/include/xrpl/ledger/helpers/AMMUtils.h @@ -1,108 +0,0 @@ -#pragma once - -#include -#include -#include -#include -#include -#include -#include - -namespace xrpl { - -class ReadView; -class ApplyView; -class Sandbox; -class NetClock; - -/** Get AMM pool balances. - */ -std::pair -ammPoolHolds( - ReadView const& view, - AccountID const& ammAccountID, - Asset const& asset1, - Asset const& asset2, - FreezeHandling freezeHandling, - AuthHandling authHandling, - beast::Journal const j); - -/** Get AMM pool and LP token balances. If both optIssue are - * provided then they are used as the AMM token pair issues. - * Otherwise the missing issues are fetched from ammSle. - */ -Expected, TER> -ammHolds( - ReadView const& view, - SLE const& ammSle, - std::optional const& optAsset1, - std::optional const& optAsset2, - FreezeHandling freezeHandling, - AuthHandling authHandling, - beast::Journal const j); - -/** Get the balance of LP tokens. - */ -STAmount -ammLPHolds( - ReadView const& view, - Asset const& asset1, - Asset const& asset2, - AccountID const& ammAccount, - AccountID const& lpAccount, - beast::Journal const j); - -STAmount -ammLPHolds( - ReadView const& view, - SLE const& ammSle, - AccountID const& lpAccount, - beast::Journal const j); - -/** Get AMM trading fee for the given account. The fee is discounted - * if the account is the auction slot owner or one of the slot's authorized - * accounts. - */ -std::uint16_t -getTradingFee(ReadView const& view, SLE const& ammSle, AccountID const& account); - -/** Returns total amount held by AMM for the given token. - */ -STAmount -ammAccountHolds(ReadView const& view, AccountID const& ammAccountID, Asset const& asset); - -/** Delete trustlines to AMM. If all trustlines are deleted then - * AMM object and account are deleted. Otherwise tecIMPCOMPLETE is returned. - */ -TER -deleteAMMAccount(Sandbox& view, Asset const& asset, Asset 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, - Asset const& lptAsset, - std::uint16_t tfee); - -/** Return true if the Liquidity Provider is the only AMM provider, false - * otherwise. Return tecINTERNAL if encountered an unexpected condition, - * for instance Liquidity Provider has more than one LPToken trustline. - */ -Expected -isOnlyLiquidityProvider(ReadView const& view, Issue const& ammIssue, AccountID const& lpAccount); - -/** Due to rounding, the LPTokenBalance of the last LP might - * not match the LP's trustline balance. If it's within the tolerance, - * update LPTokenBalance to match the LP's trustline balance. - */ -Expected -verifyAndAdjustLPTokenBalance( - Sandbox& sb, - STAmount const& lpTokens, - std::shared_ptr& ammSle, - AccountID const& account); - -} // namespace xrpl diff --git a/include/xrpl/ledger/helpers/MPTokenHelpers.h b/include/xrpl/ledger/helpers/MPTokenHelpers.h index 6544b18dd15..adfb9e5cf2c 100644 --- a/include/xrpl/ledger/helpers/MPTokenHelpers.h +++ b/include/xrpl/ledger/helpers/MPTokenHelpers.h @@ -121,6 +121,21 @@ canTransfer( [[nodiscard]] TER canTrade(ReadView const& view, Asset const& asset); +/** Convenience to combine canTrade/Transfer. Returns tesSUCCESS if Asset is Issue. + */ +[[nodiscard]] TER +canMPTTradeAndTransfer( + ReadView const& v, + Asset const& asset, + AccountID const& from, + AccountID const& to); + +/** Check if Asset can be traded on DEX. return tecNO_PERMISSION + * if it doesn't and tesSUCCESS otherwise. + */ +[[nodiscard]] TER +canTrade(ReadView const& view, Asset const& asset); + //------------------------------------------------------------------------------ // // Empty holding operations (MPT-specific) @@ -227,17 +242,4 @@ issuerFundsToSelfIssue(ReadView const& view, MPTIssue const& issue); void issuerSelfDebitHookMPT(ApplyView& view, MPTIssue const& issue, std::uint64_t amount); -//------------------------------------------------------------------------------ -// -// MPT DEX -// -//------------------------------------------------------------------------------ - -/* Return true if a transaction is allowed for the specified MPT/account. The - * function checks MPTokenIssuance and MPToken objects flags to determine if the - * transaction is allowed. - */ -TER -checkMPTTxAllowed(ReadView const& v, TxType tx, Asset const& asset, AccountID const& accountID); - } // namespace xrpl diff --git a/include/xrpl/ledger/helpers/TokenHelpers.h b/include/xrpl/ledger/helpers/TokenHelpers.h index c05308295aa..b79113dad05 100644 --- a/include/xrpl/ledger/helpers/TokenHelpers.h +++ b/include/xrpl/ledger/helpers/TokenHelpers.h @@ -54,9 +54,15 @@ enum class AuthType { StrongAuth, WeakAuth, Legacy }; [[nodiscard]] bool isGlobalFrozen(ReadView const& view, Asset const& asset); +[[nodiscard]] TER +checkGlobalFrozen(ReadView const& view, Asset const& asset); + [[nodiscard]] bool isIndividualFrozen(ReadView const& view, AccountID const& account, Asset const& asset); +[[nodiscard]] TER +checkIndividualFrozen(ReadView const& view, AccountID const& account, Asset const& asset); + /** * isFrozen check is recursive for MPT shares in a vault, descending to * assets in the vault, up to maxAssetCheckDepth recursion depth. This is diff --git a/include/xrpl/protocol/TER.h b/include/xrpl/protocol/TER.h index 0d8bb94b061..b0dafecf1c6 100644 --- a/include/xrpl/protocol/TER.h +++ b/include/xrpl/protocol/TER.h @@ -344,10 +344,6 @@ enum TECcodes : TERUnderlyingType { tecLIMIT_EXCEEDED = 195, tecPSEUDO_ACCOUNT = 196, tecPRECISION_LOSS = 197, - // DEPRECATED: This error code tecNO_DELEGATE_PERMISSION is reserved for - // backward compatibility with historical data on non-prod networks, can be - // reclaimed after those networks reset. - tecNO_DELEGATE_PERMISSION = 198, }; //------------------------------------------------------------------------------ diff --git a/include/xrpl/tx/invariants/InvariantCheck.h b/include/xrpl/tx/invariants/InvariantCheck.h index ad4c5e16c43..2d7c05e505e 100644 --- a/include/xrpl/tx/invariants/InvariantCheck.h +++ b/include/xrpl/tx/invariants/InvariantCheck.h @@ -399,7 +399,8 @@ using InvariantChecks = std::tuple< ValidLoanBroker, ValidLoan, ValidVault, - ValidMPTPayment>; + ValidMPTPayment, + ValidMPTTransfer>; /** * @brief get a tuple of all invariant checks diff --git a/include/xrpl/tx/invariants/MPTInvariant.h b/include/xrpl/tx/invariants/MPTInvariant.h index dd064af3964..bb3752eaa12 100644 --- a/include/xrpl/tx/invariants/MPTInvariant.h +++ b/include/xrpl/tx/invariants/MPTInvariant.h @@ -56,4 +56,22 @@ class ValidMPTPayment finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&); }; +class ValidMPTTransfer +{ + struct Value + { + std::optional amtBefore; + std::optional amtAfter; + }; + // MPTID: {holder: Value} + hash_map> amount_; + +public: + void + visitEntry(bool, std::shared_ptr const&, std::shared_ptr const&); + + bool + finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&); +}; + } // namespace xrpl diff --git a/include/xrpl/tx/paths/AMMLiquidity.h b/include/xrpl/tx/paths/AMMLiquidity.h index 5716ea531d5..128052f8517 100644 --- a/include/xrpl/tx/paths/AMMLiquidity.h +++ b/include/xrpl/tx/paths/AMMLiquidity.h @@ -4,7 +4,6 @@ #include #include #include -#include #include #include #include diff --git a/src/libxrpl/ledger/helpers/AMMHelpers.cpp b/src/libxrpl/ledger/helpers/AMMHelpers.cpp index 4c161bd1350..631e4d0774a 100644 --- a/src/libxrpl/ledger/helpers/AMMHelpers.cpp +++ b/src/libxrpl/ledger/helpers/AMMHelpers.cpp @@ -1,4 +1,6 @@ #include +// +#include namespace xrpl { @@ -376,4 +378,535 @@ adjustFracByTokens( return tokens / lptAMMBalance; } +std::pair +ammPoolHolds( + ReadView const& view, + AccountID const& ammAccountID, + Asset const& asset1, + Asset const& asset2, + FreezeHandling freezeHandling, + AuthHandling authHandling, + beast::Journal const j) +{ + auto const assetInBalance = + accountHolds(view, ammAccountID, asset1, freezeHandling, authHandling, j); + auto const assetOutBalance = + accountHolds(view, ammAccountID, asset2, freezeHandling, authHandling, j); + return std::make_pair(assetInBalance, assetOutBalance); +} + +Expected, TER> +ammHolds( + ReadView const& view, + SLE const& ammSle, + std::optional const& optAsset1, + std::optional const& optAsset2, + FreezeHandling freezeHandling, + AuthHandling authHandling, + beast::Journal const j) +{ + auto const assets = [&]() -> std::optional> { + auto const asset1 = ammSle[sfAsset]; + auto const asset2 = ammSle[sfAsset2]; + if (optAsset1 && optAsset2) + { + if (invalidAMMAssetPair( + *optAsset1, *optAsset2, std::make_optional(std::make_pair(asset1, asset2)))) + { + // This error can only be hit if the AMM is corrupted + // LCOV_EXCL_START + JLOG(j.debug()) << "ammHolds: Invalid optAsset1 or optAsset2 " << *optAsset1 << " " + << *optAsset2; + return std::nullopt; + // LCOV_EXCL_STOP + } + return std::make_optional(std::make_pair(*optAsset1, *optAsset2)); + } + auto const singleAsset = [&asset1, &asset2, &j]( + Asset checkIssue, + char const* label) -> std::optional> { + if (checkIssue == asset1) + { + return std::make_optional(std::make_pair(asset1, asset2)); + } + if (checkIssue == asset2) + { + return std::make_optional(std::make_pair(asset2, asset1)); + } + // Unreachable unless AMM corrupted. + // LCOV_EXCL_START + JLOG(j.debug()) << "ammHolds: Invalid " << label << " " << checkIssue; + return std::nullopt; + // LCOV_EXCL_STOP + }; + if (optAsset1) + { + return singleAsset(*optAsset1, "optAsset1"); + } + if (optAsset2) + { + // Cannot have Amount2 without Amount. + return singleAsset(*optAsset2, "optAsset2"); // LCOV_EXCL_LINE + } + return std::make_optional(std::make_pair(asset1, asset2)); + }(); + if (!assets) + return Unexpected(tecAMM_INVALID_TOKENS); + auto const [amount1, amount2] = ammPoolHolds( + view, + ammSle.getAccountID(sfAccount), + assets->first, + assets->second, + freezeHandling, + authHandling, + j); + return std::make_tuple(amount1, amount2, ammSle[sfLPTokenBalance]); +} + +STAmount +ammLPHolds( + ReadView const& view, + Asset const& asset1, + Asset const& asset2, + AccountID const& ammAccount, + AccountID const& lpAccount, + beast::Journal const j) +{ + // This function looks similar to `accountHolds`. However, it only checks if + // a LPToken holder has enough balance. On the other hand, `accountHolds` + // checks if the underlying assets of LPToken are frozen with the + // fixFrozenLPTokenTransfer amendment + + auto const currency = ammLPTCurrency(asset1, asset2); + STAmount amount; + + auto const sle = view.read(keylet::line(lpAccount, ammAccount, currency)); + if (!sle) + { + amount.clear(Issue{currency, ammAccount}); + JLOG(j.trace()) << "ammLPHolds: no SLE " + << " lpAccount=" << to_string(lpAccount) + << " amount=" << amount.getFullText(); + } + else if (isFrozen(view, lpAccount, currency, ammAccount)) + { + amount.clear(Issue{currency, ammAccount}); + JLOG(j.trace()) << "ammLPHolds: frozen currency " + << " lpAccount=" << to_string(lpAccount) + << " amount=" << amount.getFullText(); + } + else + { + amount = sle->getFieldAmount(sfBalance); + if (lpAccount > ammAccount) + { + // Put balance in account terms. + amount.negate(); + } + amount.get().account = ammAccount; + + JLOG(j.trace()) << "ammLPHolds:" + << " lpAccount=" << to_string(lpAccount) + << " amount=" << amount.getFullText(); + } + + return view.balanceHookIOU(lpAccount, ammAccount, amount); +} + +STAmount +ammLPHolds( + ReadView const& view, + SLE const& ammSle, + AccountID const& lpAccount, + beast::Journal const j) +{ + return ammLPHolds(view, ammSle[sfAsset], ammSle[sfAsset2], ammSle[sfAccount], lpAccount, j); +} + +std::uint16_t +getTradingFee(ReadView const& view, SLE const& ammSle, AccountID const& account) +{ + using namespace std::chrono; + XRPL_ASSERT( + !view.rules().enabled(fixInnerObjTemplate) || ammSle.isFieldPresent(sfAuctionSlot), + "xrpl::getTradingFee : auction present"); + if (ammSle.isFieldPresent(sfAuctionSlot)) + { + auto const& auctionSlot = safe_downcast(ammSle.peekAtField(sfAuctionSlot)); + // Not expired + if (auto const expiration = auctionSlot[~sfExpiration]; + duration_cast(view.header().parentCloseTime.time_since_epoch()).count() < + expiration) + { + if (auctionSlot[~sfAccount] == account) + return auctionSlot[sfDiscountedFee]; + if (auctionSlot.isFieldPresent(sfAuthAccounts)) + { + for (auto const& acct : auctionSlot.getFieldArray(sfAuthAccounts)) + { + if (acct[~sfAccount] == account) + return auctionSlot[sfDiscountedFee]; + } + } + } + } + return ammSle[sfTradingFee]; +} + +STAmount +ammAccountHolds(ReadView const& view, AccountID const& ammAccountID, Asset const& asset) +{ + // Get the actual AMM balance without factoring in the balance hook + return asset.visit( + [&](MPTIssue const& issue) { + if (auto const sle = view.read(keylet::mptoken(issue, ammAccountID)); + sle && !isFrozen(view, ammAccountID, issue)) + return STAmount{issue, (*sle)[sfMPTAmount]}; + return STAmount{asset}; + }, + [&](Issue const& issue) { + if (isXRP(issue)) + { + if (auto const sle = view.read(keylet::account(ammAccountID))) + return (*sle)[sfBalance]; + } + else if ( + auto const sle = + view.read(keylet::line(ammAccountID, issue.account, issue.currency)); + sle && !isFrozen(view, ammAccountID, issue.currency, issue.account)) + { + STAmount amount = (*sle)[sfBalance]; + if (ammAccountID > issue.account) + amount.negate(); + amount.get().account = issue.account; + return amount; + } + return STAmount{asset}; + }); +} + +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) -> std::pair { + // Skip AMM and MPToken + if (nodeType == ltAMM || nodeType == ltMPTOKEN) + return {tesSUCCESS, SkipEntry::Yes}; + + if (nodeType == ltRIPPLE_STATE) + { + // Trustlines must have zero balance + if (sleItem->getFieldAmount(sfBalance) != beast::zero) + { + // LCOV_EXCL_START + JLOG(j.error()) << "deleteAMMObjects: deleting trustline with " + "non-zero balance."; + return {tecINTERNAL, SkipEntry::No}; + // LCOV_EXCL_STOP + } + + return {deleteAMMTrustLine(sb, sleItem, ammAccountID, j), SkipEntry::No}; + } + // LCOV_EXCL_START + JLOG(j.error()) << "deleteAMMObjects: deleting non-trustline or non-MPT " << nodeType; + return {tecINTERNAL, SkipEntry::No}; + // LCOV_EXCL_STOP + }, + j, + maxTrustlinesToDelete); +} + +static TER +deleteAMMMPTokens(Sandbox& sb, AccountID const& ammAccountID, beast::Journal j) +{ + return cleanupOnAccountDelete( + sb, + keylet::ownerDir(ammAccountID), + [&](LedgerEntryType nodeType, + uint256 const&, + std::shared_ptr& sleItem) -> std::pair { + // Skip AMM + if (nodeType == ltAMM) + return {tesSUCCESS, SkipEntry::Yes}; + + if (nodeType == ltMPTOKEN) + { + // MPT must have zero balance + if (sleItem->getFieldU64(sfMPTAmount) != 0 || + (*sleItem)[~sfLockedAmount].value_or(0) != 0) + { + // LCOV_EXCL_START + JLOG(j.error()) << "deleteAMMObjects: deleting MPT with " + "non-zero balance."; + return {tecINTERNAL, SkipEntry::No}; + // LCOV_EXCL_STOP + } + + return {deleteAMMMPToken(sb, sleItem, ammAccountID, j), SkipEntry::No}; + } + if (nodeType == ltRIPPLE_STATE) + { + // Trustlines should have been deleted + // LCOV_EXCL_START + JLOG(j.error()) << "deleteAMMObjects: trustlines should have been deleted"; + return {tecINTERNAL, SkipEntry::No}; + // LCOV_EXCL_STOP + } + // LCOV_EXCL_START + JLOG(j.error()) << "deleteAMMObjects: deleting non-trustline or non-MPT " << nodeType; + return {tecINTERNAL, SkipEntry::No}; + // LCOV_EXCL_STOP + }, + j, + 3); // At most two MPToken plus AMM object +} + +TER +deleteAMMAccount(Sandbox& sb, Asset const& asset, Asset const& asset2, beast::Journal j) +{ + auto ammSle = sb.peek(keylet::amm(asset, asset2)); + if (!ammSle) + { + // LCOV_EXCL_START + JLOG(j.error()) << "deleteAMMAccount: AMM object does not exist " << asset << " " << asset2; + return tecINTERNAL; + // LCOV_EXCL_STOP + } + + auto const ammAccountID = (*ammSle)[sfAccount]; + auto sleAMMRoot = sb.peek(keylet::account(ammAccountID)); + if (!sleAMMRoot) + { + // LCOV_EXCL_START + JLOG(j.error()) << "deleteAMMAccount: AMM account does not exist " + << to_string(ammAccountID); + return tecINTERNAL; + // LCOV_EXCL_STOP + } + + if (auto const ter = deleteAMMTrustLines(sb, ammAccountID, maxDeletableAMMTrustLines, j); + !isTesSuccess(ter)) + return ter; + + // Delete AMM's MPTokens only if all trustlines are deleted. If trustlines + // are not deleted then AMM can be re-created with Deposit and + // AMM's MPToken(s) must exist. + if (auto const ter = deleteAMMMPTokens(sb, ammAccountID, j); !isTesSuccess(ter)) + return ter; + + auto const ownerDirKeylet = keylet::ownerDir(ammAccountID); + if (!sb.dirRemove(ownerDirKeylet, (*ammSle)[sfOwnerNode], ammSle->key(), false)) + { + // LCOV_EXCL_START + JLOG(j.error()) << "deleteAMMAccount: failed to remove dir link"; + return tecINTERNAL; + // LCOV_EXCL_STOP + } + if (sb.exists(ownerDirKeylet) && !sb.emptyDirDelete(ownerDirKeylet)) + { + // LCOV_EXCL_START + JLOG(j.error()) << "deleteAMMAccount: cannot delete root dir node of " + << toBase58(ammAccountID); + return tecINTERNAL; + // LCOV_EXCL_STOP + } + + sb.erase(ammSle); + sb.erase(sleAMMRoot); + + return tesSUCCESS; +} + +void +initializeFeeAuctionVote( + ApplyView& view, + std::shared_ptr& ammSle, + AccountID const& account, + Asset const& lptAsset, + std::uint16_t tfee) +{ + auto const& rules = view.rules(); + // AMM creator gets the voting slot. + STArray voteSlots; + STObject voteEntry = STObject::makeInnerObject(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. + // AuctionSlot is created on AMMCreate and updated on AMMDeposit + // when AMM is in an empty state + if (rules.enabled(fixInnerObjTemplate) && !ammSle->isFieldPresent(sfAuctionSlot)) + { + STObject auctionSlot = STObject::makeInnerObject(sfAuctionSlot); + ammSle->set(std::move(auctionSlot)); + } + STObject& auctionSlot = ammSle->peekFieldObject(sfAuctionSlot); + auctionSlot.setAccountID(sfAccount, account); + // current + sec in 24h + auto const expiration = std::chrono::duration_cast( + view.header().parentCloseTime.time_since_epoch()) + .count() + + TOTAL_TIME_SLOT_SECS; + auctionSlot.setFieldU32(sfExpiration, expiration); + auctionSlot.setFieldAmount(sfPrice, STAmount{lptAsset, 0}); + // Set the fee + if (tfee != 0) + { + ammSle->setFieldU16(sfTradingFee, tfee); + } + else if (ammSle->isFieldPresent(sfTradingFee)) + { + ammSle->makeFieldAbsent(sfTradingFee); // LCOV_EXCL_LINE + } + if (auto const dfee = tfee / AUCTION_SLOT_DISCOUNTED_FEE_FRACTION) + { + auctionSlot.setFieldU16(sfDiscountedFee, dfee); + } + else if (auctionSlot.isFieldPresent(sfDiscountedFee)) + { + auctionSlot.makeFieldAbsent(sfDiscountedFee); // LCOV_EXCL_LINE + } +} + +Expected +isOnlyLiquidityProvider(ReadView const& view, Issue const& ammIssue, AccountID const& lpAccount) +{ + // Liquidity Provider (LP) must have one LPToken trustline + std::uint8_t nLPTokenTrustLines = 0; + // AMM account has at most two IOU (pool tokens, not LPToken) trustlines. + // One or both trustlines could be to the LP if LP is the issuer, + // or a different account if LP is not an issuer. For instance, + // if AMM has two tokens USD and EUR and LP is not the issuer of the tokens + // then the trustlines are between AMM account and the issuer. + // There is one LPToken trustline for each LP. Only remaining LP has + // exactly one LPToken trustlines and at most two IOU trustline for each + // pool token. One or both tokens could be MPT. + std::uint8_t nIOUTrustLines = 0; + // There are at most two MPT objects, one for each side of the pool. + std::uint8_t nMPT = 0; + // There is only one AMM object + bool hasAMM = false; + // AMM LP has at most three trustlines, at most two MPTs, and only one + // AMM object must exist. If there are more than four objects then + // it's either an error or there are more than one LP. Ten pages should + // be sufficient to include four objects. + std::uint8_t limit = 10; + auto const root = keylet::ownerDir(ammIssue.account); + auto currentIndex = root; + + // Iterate over AMM owner directory objects. + while (limit-- >= 1) + { + auto const ownerDir = view.read(currentIndex); + if (!ownerDir) + return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE + for (auto const& key : ownerDir->getFieldV256(sfIndexes)) + { + auto const sle = view.read(keylet::child(key)); + if (!sle) + return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE + auto const entryType = sle->getFieldU16(sfLedgerEntryType); + // Only one AMM object + if (entryType == ltAMM) + { + if (hasAMM) + return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE + hasAMM = true; + continue; + } + if (entryType == ltMPTOKEN) + { + ++nMPT; + continue; + } + if (entryType != ltRIPPLE_STATE) + return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE + auto const lowLimit = sle->getFieldAmount(sfLowLimit); + auto const highLimit = sle->getFieldAmount(sfHighLimit); + auto const isLPTrustline = + lowLimit.getIssuer() == lpAccount || highLimit.getIssuer() == lpAccount; + auto const isLPTokenTrustline = + lowLimit.asset() == ammIssue || highLimit.asset() == ammIssue; + + // Liquidity Provider trustline + if (isLPTrustline) + { + // LPToken trustline + if (isLPTokenTrustline) + { + // LP has exactly one LPToken trustline + if (++nLPTokenTrustLines > 1) + return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE + } + // AMM account has at most two IOU trustlines + else if (++nIOUTrustLines > 2) + { + return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE + } + } + // Another Liquidity Provider LPToken trustline + else if (isLPTokenTrustline) + { + return false; + } + // AMM account has at most two IOU trustlines + else if (++nIOUTrustLines > 2) + { + return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE + } + } + auto const uNodeNext = ownerDir->getFieldU64(sfIndexNext); + if (uNodeNext == 0) + { + if (nLPTokenTrustLines != 1 || (nIOUTrustLines == 0 && nMPT == 0) || + (nIOUTrustLines > 2 || nMPT > 2) || (nIOUTrustLines + nMPT) > 2) + return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE + return true; + } + currentIndex = keylet::page(root, uNodeNext); + } + return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE +} + +Expected +verifyAndAdjustLPTokenBalance( + Sandbox& sb, + STAmount const& lpTokens, + std::shared_ptr& ammSle, + AccountID const& account) +{ + auto const res = isOnlyLiquidityProvider(sb, lpTokens.get(), account); + if (!res.has_value()) + { + return Unexpected(res.error()); + } + + if (res.value()) + { + if (withinRelativeDistance( + lpTokens, ammSle->getFieldAmount(sfLPTokenBalance), Number{1, -3})) + { + ammSle->setFieldAmount(sfLPTokenBalance, lpTokens); + sb.update(ammSle); + } + else + { + return Unexpected(tecAMM_INVALID_TOKENS); + } + } + return true; +} + } // namespace xrpl diff --git a/src/libxrpl/ledger/helpers/AMMUtils.cpp b/src/libxrpl/ledger/helpers/AMMUtils.cpp index 166e9e94941..e69de29bb2d 100644 --- a/src/libxrpl/ledger/helpers/AMMUtils.cpp +++ b/src/libxrpl/ledger/helpers/AMMUtils.cpp @@ -1,542 +0,0 @@ -#include -#include -#include -#include -#include -#include -#include - -namespace xrpl { - -std::pair -ammPoolHolds( - ReadView const& view, - AccountID const& ammAccountID, - Asset const& asset1, - Asset const& asset2, - FreezeHandling freezeHandling, - AuthHandling authHandling, - beast::Journal const j) -{ - auto const assetInBalance = - accountHolds(view, ammAccountID, asset1, freezeHandling, authHandling, j); - auto const assetOutBalance = - accountHolds(view, ammAccountID, asset2, freezeHandling, authHandling, j); - return std::make_pair(assetInBalance, assetOutBalance); -} - -Expected, TER> -ammHolds( - ReadView const& view, - SLE const& ammSle, - std::optional const& optAsset1, - std::optional const& optAsset2, - FreezeHandling freezeHandling, - AuthHandling authHandling, - beast::Journal const j) -{ - auto const assets = [&]() -> std::optional> { - auto const asset1 = ammSle[sfAsset]; - auto const asset2 = ammSle[sfAsset2]; - if (optAsset1 && optAsset2) - { - if (invalidAMMAssetPair( - *optAsset1, *optAsset2, std::make_optional(std::make_pair(asset1, asset2)))) - { - // This error can only be hit if the AMM is corrupted - // LCOV_EXCL_START - JLOG(j.debug()) << "ammHolds: Invalid optAsset1 or optAsset2 " << *optAsset1 << " " - << *optAsset2; - return std::nullopt; - // LCOV_EXCL_STOP - } - return std::make_optional(std::make_pair(*optAsset1, *optAsset2)); - } - auto const singleAsset = [&asset1, &asset2, &j]( - Asset checkIssue, - char const* label) -> std::optional> { - if (checkIssue == asset1) - { - return std::make_optional(std::make_pair(asset1, asset2)); - } - if (checkIssue == asset2) - { - return std::make_optional(std::make_pair(asset2, asset1)); - } - // Unreachable unless AMM corrupted. - // LCOV_EXCL_START - JLOG(j.debug()) << "ammHolds: Invalid " << label << " " << checkIssue; - return std::nullopt; - // LCOV_EXCL_STOP - }; - if (optAsset1) - { - return singleAsset(*optAsset1, "optAsset1"); - } - if (optAsset2) - { - // Cannot have Amount2 without Amount. - return singleAsset(*optAsset2, "optAsset2"); // LCOV_EXCL_LINE - } - return std::make_optional(std::make_pair(asset1, asset2)); - }(); - if (!assets) - return Unexpected(tecAMM_INVALID_TOKENS); - auto const [amount1, amount2] = ammPoolHolds( - view, - ammSle.getAccountID(sfAccount), - assets->first, - assets->second, - freezeHandling, - authHandling, - j); - return std::make_tuple(amount1, amount2, ammSle[sfLPTokenBalance]); -} - -STAmount -ammLPHolds( - ReadView const& view, - Asset const& asset1, - Asset const& asset2, - AccountID const& ammAccount, - AccountID const& lpAccount, - beast::Journal const j) -{ - // This function looks similar to `accountHolds`. However, it only checks if - // a LPToken holder has enough balance. On the other hand, `accountHolds` - // checks if the underlying assets of LPToken are frozen with the - // fixFrozenLPTokenTransfer amendment - - auto const currency = ammLPTCurrency(asset1, asset2); - STAmount amount; - - auto const sle = view.read(keylet::line(lpAccount, ammAccount, currency)); - if (!sle) - { - amount.clear(Issue{currency, ammAccount}); - JLOG(j.trace()) << "ammLPHolds: no SLE " - << " lpAccount=" << to_string(lpAccount) - << " amount=" << amount.getFullText(); - } - else if (isFrozen(view, lpAccount, currency, ammAccount)) - { - amount.clear(Issue{currency, ammAccount}); - JLOG(j.trace()) << "ammLPHolds: frozen currency " - << " lpAccount=" << to_string(lpAccount) - << " amount=" << amount.getFullText(); - } - else - { - amount = sle->getFieldAmount(sfBalance); - if (lpAccount > ammAccount) - { - // Put balance in account terms. - amount.negate(); - } - amount.get().account = ammAccount; - - JLOG(j.trace()) << "ammLPHolds:" - << " lpAccount=" << to_string(lpAccount) - << " amount=" << amount.getFullText(); - } - - return view.balanceHookIOU(lpAccount, ammAccount, amount); -} - -STAmount -ammLPHolds( - ReadView const& view, - SLE const& ammSle, - AccountID const& lpAccount, - beast::Journal const j) -{ - return ammLPHolds(view, ammSle[sfAsset], ammSle[sfAsset2], ammSle[sfAccount], lpAccount, j); -} - -std::uint16_t -getTradingFee(ReadView const& view, SLE const& ammSle, AccountID const& account) -{ - using namespace std::chrono; - XRPL_ASSERT( - !view.rules().enabled(fixInnerObjTemplate) || ammSle.isFieldPresent(sfAuctionSlot), - "xrpl::getTradingFee : auction present"); - if (ammSle.isFieldPresent(sfAuctionSlot)) - { - auto const& auctionSlot = safe_downcast(ammSle.peekAtField(sfAuctionSlot)); - // Not expired - if (auto const expiration = auctionSlot[~sfExpiration]; - duration_cast(view.header().parentCloseTime.time_since_epoch()).count() < - expiration) - { - if (auctionSlot[~sfAccount] == account) - return auctionSlot[sfDiscountedFee]; - if (auctionSlot.isFieldPresent(sfAuthAccounts)) - { - for (auto const& acct : auctionSlot.getFieldArray(sfAuthAccounts)) - { - if (acct[~sfAccount] == account) - return auctionSlot[sfDiscountedFee]; - } - } - } - } - return ammSle[sfTradingFee]; -} - -STAmount -ammAccountHolds(ReadView const& view, AccountID const& ammAccountID, Asset const& asset) -{ - // Get the actual AMM balance without factoring in the balance hook - return asset.visit( - [&](MPTIssue const& issue) { - if (auto const sle = view.read(keylet::mptoken(issue, ammAccountID)); - sle && !isFrozen(view, ammAccountID, issue)) - return STAmount{issue, (*sle)[sfMPTAmount]}; - return STAmount{asset}; - }, - [&](Issue const& issue) { - if (isXRP(issue)) - { - if (auto const sle = view.read(keylet::account(ammAccountID))) - return (*sle)[sfBalance]; - } - else if ( - auto const sle = - view.read(keylet::line(ammAccountID, issue.account, issue.currency)); - sle && !isFrozen(view, ammAccountID, issue.currency, issue.account)) - { - STAmount amount = (*sle)[sfBalance]; - if (ammAccountID > issue.account) - amount.negate(); - amount.get().account = issue.account; - return amount; - } - return STAmount{asset}; - }); -} - -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) -> std::pair { - // Skip AMM and MPToken - if (nodeType == ltAMM || nodeType == ltMPTOKEN) - return {tesSUCCESS, SkipEntry::Yes}; - - if (nodeType == ltRIPPLE_STATE) - { - // Trustlines must have zero balance - if (sleItem->getFieldAmount(sfBalance) != beast::zero) - { - // LCOV_EXCL_START - JLOG(j.error()) << "deleteAMMObjects: deleting trustline with " - "non-zero balance."; - return {tecINTERNAL, SkipEntry::No}; - // LCOV_EXCL_STOP - } - - return {deleteAMMTrustLine(sb, sleItem, ammAccountID, j), SkipEntry::No}; - } - // LCOV_EXCL_START - JLOG(j.error()) << "deleteAMMObjects: deleting non-trustline or non-MPT " << nodeType; - return {tecINTERNAL, SkipEntry::No}; - // LCOV_EXCL_STOP - }, - j, - maxTrustlinesToDelete); -} - -static TER -deleteAMMMPTokens(Sandbox& sb, AccountID const& ammAccountID, beast::Journal j) -{ - return cleanupOnAccountDelete( - sb, - keylet::ownerDir(ammAccountID), - [&](LedgerEntryType nodeType, - uint256 const&, - std::shared_ptr& sleItem) -> std::pair { - // Skip AMM - if (nodeType == ltAMM) - return {tesSUCCESS, SkipEntry::Yes}; - - if (nodeType == ltMPTOKEN) - { - // MPT must have zero balance - if (sleItem->getFieldU64(sfMPTAmount) != 0 || - (*sleItem)[~sfLockedAmount].value_or(0) != 0) - { - // LCOV_EXCL_START - JLOG(j.error()) << "deleteAMMObjects: deleting MPT with " - "non-zero balance."; - return {tecINTERNAL, SkipEntry::No}; - // LCOV_EXCL_STOP - } - - return {deleteAMMMPToken(sb, sleItem, ammAccountID, j), SkipEntry::No}; - } - if (nodeType == ltRIPPLE_STATE) - { - // Trustlines should have been deleted - // LCOV_EXCL_START - JLOG(j.error()) << "deleteAMMObjects: trustlines should have been deleted"; - return {tecINTERNAL, SkipEntry::No}; - // LCOV_EXCL_STOP - } - // LCOV_EXCL_START - JLOG(j.error()) << "deleteAMMObjects: deleting non-trustline or non-MPT " << nodeType; - return {tecINTERNAL, SkipEntry::No}; - // LCOV_EXCL_STOP - }, - j, - 3); // At most two MPToken plus AMM object -} - -TER -deleteAMMAccount(Sandbox& sb, Asset const& asset, Asset const& asset2, beast::Journal j) -{ - auto ammSle = sb.peek(keylet::amm(asset, asset2)); - if (!ammSle) - { - // LCOV_EXCL_START - JLOG(j.error()) << "deleteAMMAccount: AMM object does not exist " << asset << " " << asset2; - return tecINTERNAL; - // LCOV_EXCL_STOP - } - - auto const ammAccountID = (*ammSle)[sfAccount]; - auto sleAMMRoot = sb.peek(keylet::account(ammAccountID)); - if (!sleAMMRoot) - { - // LCOV_EXCL_START - JLOG(j.error()) << "deleteAMMAccount: AMM account does not exist " - << to_string(ammAccountID); - return tecINTERNAL; - // LCOV_EXCL_STOP - } - - if (auto const ter = deleteAMMTrustLines(sb, ammAccountID, maxDeletableAMMTrustLines, j); - !isTesSuccess(ter)) - return ter; - - // Delete AMM's MPTokens only if all trustlines are deleted. If trustlines - // are not deleted then AMM can be re-created with Deposit and - // AMM's MPToken(s) must exist. - if (auto const ter = deleteAMMMPTokens(sb, ammAccountID, j); !isTesSuccess(ter)) - return ter; - - auto const ownerDirKeylet = keylet::ownerDir(ammAccountID); - if (!sb.dirRemove(ownerDirKeylet, (*ammSle)[sfOwnerNode], ammSle->key(), false)) - { - // LCOV_EXCL_START - JLOG(j.error()) << "deleteAMMAccount: failed to remove dir link"; - return tecINTERNAL; - // LCOV_EXCL_STOP - } - if (sb.exists(ownerDirKeylet) && !sb.emptyDirDelete(ownerDirKeylet)) - { - // LCOV_EXCL_START - JLOG(j.error()) << "deleteAMMAccount: cannot delete root dir node of " - << toBase58(ammAccountID); - return tecINTERNAL; - // LCOV_EXCL_STOP - } - - sb.erase(ammSle); - sb.erase(sleAMMRoot); - - return tesSUCCESS; -} - -void -initializeFeeAuctionVote( - ApplyView& view, - std::shared_ptr& ammSle, - AccountID const& account, - Asset const& lptAsset, - std::uint16_t tfee) -{ - auto const& rules = view.rules(); - // AMM creator gets the voting slot. - STArray voteSlots; - STObject voteEntry = STObject::makeInnerObject(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. - // AuctionSlot is created on AMMCreate and updated on AMMDeposit - // when AMM is in an empty state - if (rules.enabled(fixInnerObjTemplate) && !ammSle->isFieldPresent(sfAuctionSlot)) - { - STObject auctionSlot = STObject::makeInnerObject(sfAuctionSlot); - ammSle->set(std::move(auctionSlot)); - } - STObject& auctionSlot = ammSle->peekFieldObject(sfAuctionSlot); - auctionSlot.setAccountID(sfAccount, account); - // current + sec in 24h - auto const expiration = std::chrono::duration_cast( - view.header().parentCloseTime.time_since_epoch()) - .count() + - TOTAL_TIME_SLOT_SECS; - auctionSlot.setFieldU32(sfExpiration, expiration); - auctionSlot.setFieldAmount(sfPrice, STAmount{lptAsset, 0}); - // Set the fee - if (tfee != 0) - { - ammSle->setFieldU16(sfTradingFee, tfee); - } - else if (ammSle->isFieldPresent(sfTradingFee)) - { - ammSle->makeFieldAbsent(sfTradingFee); // LCOV_EXCL_LINE - } - if (auto const dfee = tfee / AUCTION_SLOT_DISCOUNTED_FEE_FRACTION) - { - auctionSlot.setFieldU16(sfDiscountedFee, dfee); - } - else if (auctionSlot.isFieldPresent(sfDiscountedFee)) - { - auctionSlot.makeFieldAbsent(sfDiscountedFee); // LCOV_EXCL_LINE - } -} - -Expected -isOnlyLiquidityProvider(ReadView const& view, Issue const& ammIssue, AccountID const& lpAccount) -{ - // Liquidity Provider (LP) must have one LPToken trustline - std::uint8_t nLPTokenTrustLines = 0; - // AMM account has at most two IOU (pool tokens, not LPToken) trustlines. - // One or both trustlines could be to the LP if LP is the issuer, - // or a different account if LP is not an issuer. For instance, - // if AMM has two tokens USD and EUR and LP is not the issuer of the tokens - // then the trustlines are between AMM account and the issuer. - // There is one LPToken trustline for each LP. Only remaining LP has - // exactly one LPToken trustlines and at most two IOU trustline for each - // pool token. One or both tokens could be MPT. - std::uint8_t nIOUTrustLines = 0; - // There are at most two MPT objects, one for each side of the pool. - std::uint8_t nMPT = 0; - // There is only one AMM object - bool hasAMM = false; - // AMM LP has at most three trustlines, at most two MPTs, and only one - // AMM object must exist. If there are more than four objects then - // it's either an error or there are more than one LP. Ten pages should - // be sufficient to include four objects. - std::uint8_t limit = 10; - auto const root = keylet::ownerDir(ammIssue.account); - auto currentIndex = root; - - // Iterate over AMM owner directory objects. - while (limit-- >= 1) - { - auto const ownerDir = view.read(currentIndex); - if (!ownerDir) - return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE - for (auto const& key : ownerDir->getFieldV256(sfIndexes)) - { - auto const sle = view.read(keylet::child(key)); - if (!sle) - return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE - auto const entryType = sle->getFieldU16(sfLedgerEntryType); - // Only one AMM object - if (entryType == ltAMM) - { - if (hasAMM) - return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE - hasAMM = true; - continue; - } - if (entryType == ltMPTOKEN) - { - ++nMPT; - continue; - } - if (entryType != ltRIPPLE_STATE) - return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE - auto const lowLimit = sle->getFieldAmount(sfLowLimit); - auto const highLimit = sle->getFieldAmount(sfHighLimit); - auto const isLPTrustline = - lowLimit.getIssuer() == lpAccount || highLimit.getIssuer() == lpAccount; - auto const isLPTokenTrustline = - lowLimit.asset() == ammIssue || highLimit.asset() == ammIssue; - - // Liquidity Provider trustline - if (isLPTrustline) - { - // LPToken trustline - if (isLPTokenTrustline) - { - // LP has exactly one LPToken trustline - if (++nLPTokenTrustLines > 1) - return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE - } - // AMM account has at most two IOU trustlines - else if (++nIOUTrustLines > 2) - { - return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE - } - } - // Another Liquidity Provider LPToken trustline - else if (isLPTokenTrustline) - { - return false; - } - // AMM account has at most two IOU trustlines - else if (++nIOUTrustLines > 2) - { - return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE - } - } - auto const uNodeNext = ownerDir->getFieldU64(sfIndexNext); - if (uNodeNext == 0) - { - if (nLPTokenTrustLines != 1 || (nIOUTrustLines == 0 && nMPT == 0) || - (nIOUTrustLines > 2 || nMPT > 2) || (nIOUTrustLines + nMPT) > 2) - return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE - return true; - } - currentIndex = keylet::page(root, uNodeNext); - } - return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE -} - -Expected -verifyAndAdjustLPTokenBalance( - Sandbox& sb, - STAmount const& lpTokens, - std::shared_ptr& ammSle, - AccountID const& account) -{ - auto const res = isOnlyLiquidityProvider(sb, lpTokens.get(), account); - if (!res.has_value()) - { - return Unexpected(res.error()); - } - - if (res.value()) - { - if (withinRelativeDistance( - lpTokens, ammSle->getFieldAmount(sfLPTokenBalance), Number{1, -3})) - { - ammSle->setFieldAmount(sfLPTokenBalance, lpTokens); - sb.update(ammSle); - } - else - { - return Unexpected(tecAMM_INVALID_TOKENS); - } - } - return true; -} - -} // namespace xrpl diff --git a/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp b/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp index 3cc359408ab..624d4d0e810 100644 --- a/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp +++ b/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp @@ -495,6 +495,22 @@ canTrade(ReadView const& view, Asset const& asset) }); } +TER +canMPTTradeAndTransfer( + ReadView const& view, + Asset const& asset, + AccountID const& from, + AccountID const& to) +{ + if (!asset.holds()) + return tesSUCCESS; + + if (auto const ter = canTrade(view, asset); !isTesSuccess(ter)) + return ter; + + return canTransfer(view, asset, from, to); +} + TER lockEscrowMPT(ApplyView& view, AccountID const& sender, STAmount const& amount, beast::Journal j) { @@ -862,65 +878,4 @@ issuerSelfDebitHookMPT(ApplyView& view, MPTIssue const& issue, std::uint64_t amo view.issuerSelfDebitHookMPT(issue, amount, available); } -static TER -checkMPTAllowed(ReadView const& view, TxType txType, Asset const& asset, AccountID const& accountID) -{ - if (!asset.holds()) - return tesSUCCESS; - - auto const& issuanceID = asset.get().getMptID(); - auto const validTx = txType == ttAMM_CREATE || txType == ttAMM_DEPOSIT || - txType == ttAMM_WITHDRAW || txType == ttOFFER_CREATE || txType == ttCHECK_CREATE || - txType == ttCHECK_CASH || txType == ttPAYMENT; - XRPL_ASSERT(validTx, "xrpl::checkMPTAllowed : all MPT tx or DEX"); - if (!validTx) - return tefINTERNAL; // LCOV_EXCL_LINE - - auto const& issuer = asset.getIssuer(); - if (!view.exists(keylet::account(issuer))) - return tecNO_ISSUER; // LCOV_EXCL_LINE - - auto const issuanceKey = keylet::mptIssuance(issuanceID); - auto const issuanceSle = view.read(issuanceKey); - if (!issuanceSle) - return tecOBJECT_NOT_FOUND; // LCOV_EXCL_LINE - - auto const flags = issuanceSle->getFlags(); - - if ((flags & lsfMPTLocked) != 0u) - return tecLOCKED; // LCOV_EXCL_LINE - // Offer crossing and Payment - if ((flags & lsfMPTCanTrade) == 0) - return tecNO_PERMISSION; - - if (accountID != issuer) - { - if ((flags & lsfMPTCanTransfer) == 0) - return tecNO_PERMISSION; - - auto const mptSle = view.read(keylet::mptoken(issuanceKey.key, accountID)); - // Allow to succeed since some tx create MPToken if it doesn't exist. - // Tx's have their own check for missing MPToken. - if (!mptSle) - return tesSUCCESS; - - if (mptSle->isFlag(lsfMPTLocked)) - return tecLOCKED; - } - - return tesSUCCESS; -} - -TER -checkMPTTxAllowed( - ReadView const& view, - TxType txType, - Asset const& asset, - AccountID const& accountID) -{ - // use isDEXAllowed for payment/offer crossing - XRPL_ASSERT(txType != ttPAYMENT, "xrpl::checkMPTTxAllowed : not payment"); - return checkMPTAllowed(view, txType, asset, accountID); -} - } // namespace xrpl diff --git a/src/libxrpl/ledger/helpers/TokenHelpers.cpp b/src/libxrpl/ledger/helpers/TokenHelpers.cpp index ec9ccaa7ae0..7e723640588 100644 --- a/src/libxrpl/ledger/helpers/TokenHelpers.cpp +++ b/src/libxrpl/ledger/helpers/TokenHelpers.cpp @@ -40,6 +40,14 @@ isGlobalFrozen(ReadView const& view, Asset const& asset) [&](MPTIssue const& issue) { return isGlobalFrozen(view, issue); }); } +TER +checkGlobalFrozen(ReadView const& view, Asset const& asset) +{ + if (isGlobalFrozen(view, asset)) + return asset.holds() ? tecLOCKED : tecFROZEN; + return tesSUCCESS; +} + bool isIndividualFrozen(ReadView const& view, AccountID const& account, Asset const& asset) { @@ -47,6 +55,14 @@ isIndividualFrozen(ReadView const& view, AccountID const& account, Asset const& [&](auto const& issue) { return isIndividualFrozen(view, account, issue); }, asset.value()); } +TER +checkIndividualFrozen(ReadView const& view, AccountID const& account, Asset const& asset) +{ + if (isIndividualFrozen(view, account, asset)) + return asset.holds() ? tecLOCKED : tecFROZEN; + return tesSUCCESS; +} + bool isFrozen(ReadView const& view, AccountID const& account, Asset const& asset, int depth) { diff --git a/src/libxrpl/protocol/Permissions.cpp b/src/libxrpl/protocol/Permissions.cpp index 47fc0d28b6c..686b6376c1c 100644 --- a/src/libxrpl/protocol/Permissions.cpp +++ b/src/libxrpl/protocol/Permissions.cpp @@ -67,6 +67,11 @@ Permission::Permission() #pragma pop_macro("PERMISSION") }; + XRPL_ASSERT( + txFeatureMap_.size() == delegableTx_.size(), + "xrpl::Permission : txFeatureMap_ and delegableTx_ must have same " + "size"); + for ([[maybe_unused]] auto const& permission : granularPermissionMap_) { XRPL_ASSERT( diff --git a/src/libxrpl/protocol/TER.cpp b/src/libxrpl/protocol/TER.cpp index 30e7662f560..1e3f64cc227 100644 --- a/src/libxrpl/protocol/TER.cpp +++ b/src/libxrpl/protocol/TER.cpp @@ -215,6 +215,7 @@ transResults() MAKE_ERROR(terNO_AMM, "AMM doesn't exist for the asset pair."), MAKE_ERROR(terADDRESS_COLLISION, "Failed to allocate an unique account address."), MAKE_ERROR(terNO_DELEGATE_PERMISSION, "Delegated account lacks permission to perform this transaction."), + MAKE_ERROR(terLOCKED, "Fund is locked."), MAKE_ERROR(tesSUCCESS, "The transaction was applied. Only final in a validated ledger."), }; diff --git a/src/libxrpl/tx/invariants/AMMInvariant.cpp b/src/libxrpl/tx/invariants/AMMInvariant.cpp index 03eb4c1d4a5..35fd356918b 100644 --- a/src/libxrpl/tx/invariants/AMMInvariant.cpp +++ b/src/libxrpl/tx/invariants/AMMInvariant.cpp @@ -3,7 +3,6 @@ #include #include #include -#include #include namespace xrpl { diff --git a/src/libxrpl/tx/invariants/MPTInvariant.cpp b/src/libxrpl/tx/invariants/MPTInvariant.cpp index de7bcb790ad..c3c9b1ebb22 100644 --- a/src/libxrpl/tx/invariants/MPTInvariant.cpp +++ b/src/libxrpl/tx/invariants/MPTInvariant.cpp @@ -369,4 +369,116 @@ ValidMPTPayment::finalize( return true; } +void +ValidMPTTransfer::visitEntry( + bool, + std::shared_ptr const& before, + std::shared_ptr const& after) +{ + // Record the before/after MPTAmount for each (issuanceID, account) pair + // so finalize() can determine whether a transfer actually occurred. + auto update = [&](SLE const& sle, bool isBefore) { + if (sle.getType() == ltMPTOKEN) + { + auto const issuanceID = sle[sfMPTokenIssuanceID]; + auto const account = sle[sfAccount]; + auto const amount = sle[sfMPTAmount]; + if (isBefore) + { + amount_[issuanceID][account].amtBefore = amount; + } + else + { + amount_[issuanceID][account].amtAfter = amount; + } + } + }; + + if (before) + update(*before, true); + + if (after) + update(*after, false); +} + +bool +ValidMPTTransfer::finalize( + STTx const& tx, + TER const, + XRPAmount const, + ReadView const& view, + beast::Journal const& j) +{ + // AMMClawback is called by the issuer, so freeze restrictions do not apply. + auto const txnType = tx.getTxnType(); + if (txnType == ttAMM_CLAWBACK) + return true; + + // DEX transactions (payments, check cash, offer creates) are subject to + // the MPTCanTrade flag in addition to the standard transfer rules. + // A payment is only DEX if it is a cross-currency payment. + auto const isDEX = [&tx, &txnType] { + if (txnType == ttPAYMENT) + { + // A payment is cross-currency (and thus DEX) only if SendMax is present + // and its asset differs from the destination asset. + auto const amount = tx[sfAmount]; + return tx[~sfSendMax].value_or(amount).asset() != amount.asset(); + } + return txnType == ttOFFER_CREATE; + }(); + + // Only enforce once MPTokensV2 is enabled to preserve consensus with non-V2 nodes. + auto const enforce = !view.rules().enabled(featureMPTokensV2); + + for (auto const& [mptID, values] : amount_) + { + std::uint16_t senders = 0; + std::uint16_t receivers = 0; + bool frozen = false; + auto const sleIssuance = view.read(keylet::mptIssuance(mptID)); + if (!sleIssuance) + { + continue; + } + + auto const canTransfer = sleIssuance->isFlag(lsfMPTCanTransfer); + auto const canTrade = sleIssuance->isFlag(lsfMPTCanTrade); + + for (auto const& [account, value] : values) + { + // Check once: if any involved account is frozen, the whole + // issuance transfer is considered frozen. + if (!frozen && isFrozen(view, account, MPTIssue{mptID})) + { + frozen = true; + } + // Classify each account as a sender or receiver based on whether + // their MPTAmount decreased or increased. + if (value.amtBefore.has_value() && value.amtAfter.has_value() && + *value.amtBefore != *value.amtAfter) + { + if (*value.amtAfter > *value.amtBefore) + { + ++receivers; + } + else + { + ++senders; + } + } + } + // A transfer between holders has occurred (senders > 0 && receivers > 0). + // Fail if the issuance is frozen, does not permit transfers, or — for + // DEX transactions — does not permit trading. + if ((frozen || !canTransfer || (isDEX && !canTrade)) && senders > 0 && receivers > 0) + { + JLOG(j.fatal()) << "Invariant failed: invalid MPToken transfer between holders"; + return enforce; + } + } + + return true; +} + } // namespace xrpl diff --git a/src/libxrpl/tx/paths/BookStep.cpp b/src/libxrpl/tx/paths/BookStep.cpp index 253f7021149..ab2efcf39a8 100644 --- a/src/libxrpl/tx/paths/BookStep.cpp +++ b/src/libxrpl/tx/paths/BookStep.cpp @@ -2,7 +2,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/src/libxrpl/tx/transactors/check/CheckCreate.cpp b/src/libxrpl/tx/transactors/check/CheckCreate.cpp index 70dddd7338d..da63da8c079 100644 --- a/src/libxrpl/tx/transactors/check/CheckCreate.cpp +++ b/src/libxrpl/tx/transactors/check/CheckCreate.cpp @@ -94,10 +94,10 @@ CheckCreate::preclaim(PreclaimContext const& ctx) { // The currency may not be globally frozen AccountID const& issuerId{sendMax.getIssuer()}; - if (isGlobalFrozen(ctx.view, sendMax.asset())) + if (auto const ter = checkGlobalFrozen(ctx.view, sendMax.asset()); !isTesSuccess(ter)) { - JLOG(ctx.j.warn()) << "Creating a check for frozen asset"; - return sendMax.asset().holds() ? tecLOCKED : tecFROZEN; + JLOG(ctx.j.warn()) << "Creating a check for frozen or locked asset"; + return ter; } auto const err = sendMax.asset().visit( [&](Issue const& issue) -> std::optional { @@ -136,9 +136,21 @@ CheckCreate::preclaim(PreclaimContext const& ctx) }, [&](MPTIssue const& issue) -> std::optional { if (srcId != issuerId && isFrozen(ctx.view, srcId, issue)) + { + JLOG(ctx.j.warn()) << "Creating a check for locked MPT."; return tecLOCKED; + } if (dstId != issuerId && isFrozen(ctx.view, dstId, issue)) + { + JLOG(ctx.j.warn()) << "Creating a check for locked MPT."; return tecLOCKED; + } + if (auto const ter = canTransfer(ctx.view, issue, srcId, dstId); + !isTesSuccess(ter)) + { + JLOG(ctx.j.warn()) << "MPT transfer is disabled."; + return ter; + } return std::nullopt; }); @@ -152,7 +164,7 @@ CheckCreate::preclaim(PreclaimContext const& ctx) return tecEXPIRED; } - return canTrade(ctx.view, ctx.tx[sfSendMax].asset()); + return tesSUCCESS; } TER diff --git a/src/libxrpl/tx/transactors/delegate/DelegateUtils.cpp b/src/libxrpl/tx/transactors/delegate/DelegateUtils.cpp index 862fcf280c2..f5e92bf06dd 100644 --- a/src/libxrpl/tx/transactors/delegate/DelegateUtils.cpp +++ b/src/libxrpl/tx/transactors/delegate/DelegateUtils.cpp @@ -6,7 +6,7 @@ NotTEC checkTxPermission(std::shared_ptr const& delegate, STTx const& tx) { if (!delegate) - return terNO_DELEGATE_PERMISSION; // LCOV_EXCL_LINE + return terNO_DELEGATE_PERMISSION; auto const permissionArray = delegate->getFieldArray(sfPermissions); auto const txPermission = tx.getTxnType() + 1; @@ -28,7 +28,7 @@ loadGranularPermission( std::unordered_set& granularPermissions) { if (!delegate) - return; // LCOV_EXCL_LINE + return; auto const permissionArray = delegate->getFieldArray(sfPermissions); for (auto const& permission : permissionArray) diff --git a/src/libxrpl/tx/transactors/dex/AMMBid.cpp b/src/libxrpl/tx/transactors/dex/AMMBid.cpp index 8efc36537a7..3fed28e3c20 100644 --- a/src/libxrpl/tx/transactors/dex/AMMBid.cpp +++ b/src/libxrpl/tx/transactors/dex/AMMBid.cpp @@ -1,7 +1,6 @@ #include #include #include -#include #include #include #include diff --git a/src/libxrpl/tx/transactors/dex/AMMClawback.cpp b/src/libxrpl/tx/transactors/dex/AMMClawback.cpp index 4b98711c19f..e437da3e70d 100644 --- a/src/libxrpl/tx/transactors/dex/AMMClawback.cpp +++ b/src/libxrpl/tx/transactors/dex/AMMClawback.cpp @@ -1,7 +1,6 @@ #include #include #include -#include #include #include #include diff --git a/src/libxrpl/tx/transactors/dex/AMMCreate.cpp b/src/libxrpl/tx/transactors/dex/AMMCreate.cpp index 53680f360ab..7e3805adce6 100644 --- a/src/libxrpl/tx/transactors/dex/AMMCreate.cpp +++ b/src/libxrpl/tx/transactors/dex/AMMCreate.cpp @@ -2,7 +2,6 @@ #include #include #include -#include #include #include #include @@ -95,11 +94,16 @@ AMMCreate::preclaim(PreclaimContext const& ctx) } // Globally or individually frozen - if (isFrozen(ctx.view, accountID, amount.asset()) || - isFrozen(ctx.view, accountID, amount2.asset())) + if (auto const ter = checkFrozen(ctx.view, accountID, amount.asset()); !isTesSuccess(ter)) + { - JLOG(ctx.j.debug()) << "AMM Instance: involves frozen asset."; - return tecFROZEN; + JLOG(ctx.j.debug()) << "AMM Instance: involves frozen or locked asset."; + return ter; + } + if (auto const ter = checkFrozen(ctx.view, accountID, amount2.asset()); !isTesSuccess(ter)) + { + JLOG(ctx.j.debug()) << "AMM Instance: involves frozen or locked asset."; + return ter; } auto noDefaultRipple = [](ReadView const& view, Asset const& asset) { @@ -166,10 +170,10 @@ AMMCreate::preclaim(PreclaimContext const& ctx) return terADDRESS_COLLISION; } - if (auto const ter = checkMPTTxAllowed(ctx.view, ttAMM_CREATE, amount.asset(), accountID); + if (auto const ter = canMPTTradeAndTransfer(ctx.view, amount.asset(), accountID, accountID); !isTesSuccess(ter)) return ter; - if (auto const ter = checkMPTTxAllowed(ctx.view, ttAMM_CREATE, amount2.asset(), accountID); + if (auto const ter = canMPTTradeAndTransfer(ctx.view, amount2.asset(), accountID, accountID); !isTesSuccess(ter)) return ter; diff --git a/src/libxrpl/tx/transactors/dex/AMMDelete.cpp b/src/libxrpl/tx/transactors/dex/AMMDelete.cpp index 1033f227222..f601a360d4e 100644 --- a/src/libxrpl/tx/transactors/dex/AMMDelete.cpp +++ b/src/libxrpl/tx/transactors/dex/AMMDelete.cpp @@ -1,5 +1,5 @@ #include -#include +#include #include #include #include diff --git a/src/libxrpl/tx/transactors/dex/AMMDeposit.cpp b/src/libxrpl/tx/transactors/dex/AMMDeposit.cpp index 4fc2c14de8c..b4da5aa1fc6 100644 --- a/src/libxrpl/tx/transactors/dex/AMMDeposit.cpp +++ b/src/libxrpl/tx/transactors/dex/AMMDeposit.cpp @@ -1,7 +1,6 @@ #include #include #include -#include #include #include #include @@ -244,12 +243,12 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) return ter; } - if (isFrozen(ctx.view, accountID, asset)) + if (auto const ter = checkFrozen(ctx.view, accountID, asset); !isTesSuccess(ter)) { - JLOG(ctx.j.debug()) << "AMM Deposit: account or currency is frozen, " + JLOG(ctx.j.debug()) << "AMM Deposit: account or currency is frozen or locked, " << to_string(accountID) << " " << to_string(asset); - return tecFROZEN; + return ter; } return tesSUCCESS; @@ -281,18 +280,20 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) // LCOV_EXCL_STOP } // AMM account or currency frozen - if (isFrozen(ctx.view, ammAccountID, amount->asset())) + if (auto const ter = checkFrozen(ctx.view, ammAccountID, amount->asset()); + !isTesSuccess(ter)) { - JLOG(ctx.j.debug()) - << "AMM Deposit: AMM account or currency is frozen, " << to_string(accountID); - return tecFROZEN; + JLOG(ctx.j.debug()) << "AMM Deposit: AMM account or currency is frozen or locked, " + << to_string(accountID); + return ter; } // Account frozen - if (isIndividualFrozen(ctx.view, accountID, amount->asset())) + if (auto const ter = checkIndividualFrozen(ctx.view, accountID, amount->asset()); + !isTesSuccess(ter)) { - JLOG(ctx.j.debug()) << "AMM Deposit: account is frozen, " << to_string(accountID) - << " " << to_string(amount->asset()); - return tecFROZEN; + JLOG(ctx.j.debug()) << "AMM Deposit: account is frozen or locked, " + << to_string(accountID) << " " << to_string(amount->asset()); + return ter; } if (checkBalance) { @@ -345,10 +346,10 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) } } - if (auto const ter = checkMPTTxAllowed(ctx.view, ttAMM_DEPOSIT, ctx.tx[sfAsset], accountID); + if (auto const ter = canMPTTradeAndTransfer(ctx.view, ctx.tx[sfAsset], accountID, accountID); !isTesSuccess(ter)) return ter; - if (auto const ter = checkMPTTxAllowed(ctx.view, ttAMM_DEPOSIT, ctx.tx[sfAsset2], accountID); + if (auto const ter = canMPTTradeAndTransfer(ctx.view, ctx.tx[sfAsset2], accountID, accountID); !isTesSuccess(ter)) return ter; diff --git a/src/libxrpl/tx/transactors/dex/AMMVote.cpp b/src/libxrpl/tx/transactors/dex/AMMVote.cpp index ed75cf5d901..42b339a65e0 100644 --- a/src/libxrpl/tx/transactors/dex/AMMVote.cpp +++ b/src/libxrpl/tx/transactors/dex/AMMVote.cpp @@ -1,5 +1,5 @@ #include -#include +#include #include #include #include diff --git a/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp b/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp index f4f27309b55..7749a3861af 100644 --- a/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp +++ b/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp @@ -1,7 +1,6 @@ #include #include #include -#include #include #include #include @@ -212,22 +211,24 @@ AMMWithdraw::preclaim(PreclaimContext const& ctx) return ter; } // AMM account or currency frozen - if (isFrozen(ctx.view, ammAccountID, amount->asset())) + if (auto const ter = checkFrozen(ctx.view, ammAccountID, amount->asset()); + !isTesSuccess(ter)) { - JLOG(ctx.j.debug()) - << "AMM Withdraw: AMM account or currency is frozen, " << to_string(accountID); - return tecFROZEN; + JLOG(ctx.j.debug()) << "AMM Withdraw: AMM account or currency is frozen or locked, " + << to_string(accountID); + return ter; } // Account frozen - if (isIndividualFrozen(ctx.view, accountID, amount->asset())) + if (auto const ter = checkIndividualFrozen(ctx.view, accountID, amount->asset()); + !isTesSuccess(ter)) { - JLOG(ctx.j.debug()) << "AMM Withdraw: account is frozen, " << to_string(accountID) - << " " << to_string(amount->asset()); - return tecFROZEN; + JLOG(ctx.j.debug()) << "AMM Withdraw: account is frozen or locked, " + << to_string(accountID) << " " << to_string(amount->asset()); + return ter; } if (auto const ter = - checkMPTTxAllowed(ctx.view, ttAMM_WITHDRAW, amount->asset(), accountID); + canMPTTradeAndTransfer(ctx.view, amount->asset(), accountID, accountID); !isTesSuccess(ter)) return ter; } diff --git a/src/libxrpl/tx/transactors/dex/OfferCreate.cpp b/src/libxrpl/tx/transactors/dex/OfferCreate.cpp index 2568806f51d..73c1e50e4b8 100644 --- a/src/libxrpl/tx/transactors/dex/OfferCreate.cpp +++ b/src/libxrpl/tx/transactors/dex/OfferCreate.cpp @@ -146,11 +146,15 @@ OfferCreate::preclaim(PreclaimContext const& ctx) auto viewJ = ctx.registry.get().getJournal("View"); - if (isGlobalFrozen(ctx.view, saTakerPays.asset()) || - isGlobalFrozen(ctx.view, saTakerGets.asset())) + if (auto const ter = checkGlobalFrozen(ctx.view, saTakerPays.asset()); !isTesSuccess(ter)) { JLOG(ctx.j.debug()) << "Offer involves frozen asset"; - return tecFROZEN; + return ter; + } + if (auto const ter = checkGlobalFrozen(ctx.view, saTakerGets.asset()); !isTesSuccess(ter)) + { + JLOG(ctx.j.debug()) << "Offer involves frozen asset"; + return ter; } // Allow unfunded MPT for issuer (OutstandingAmount >= MaximumAmount) @@ -281,7 +285,13 @@ OfferCreate::checkAcceptAsset( [&](MPTIssue const& issue) -> TER { // WeakAuth - don't check if MPToken exists since it's created // if needed. - return requireAuth(view, issue, id, AuthType::WeakAuth); + if (auto const ter = requireAuth(view, issue, id, AuthType::WeakAuth); + !isTesSuccess(ter)) + { + return ter; + } + + return checkFrozen(view, id, issue); }); } diff --git a/src/libxrpl/tx/transactors/payment/Payment.cpp b/src/libxrpl/tx/transactors/payment/Payment.cpp index 8b804912784..3a64b5c7390 100644 --- a/src/libxrpl/tx/transactors/payment/Payment.cpp +++ b/src/libxrpl/tx/transactors/payment/Payment.cpp @@ -265,6 +265,7 @@ Payment::checkPermission(ReadView const& view, STTx const& tx) tx.isFieldPresent(sfPaths)) return terNO_DELEGATE_PERMISSION; + // PaymentMint and PaymentBurn apply to both IOU and MPT direct payments. if (granularPermissions.contains(PaymentMint) && !isXRP(amountAsset) && amountAsset.getIssuer() == tx[sfAccount]) return tesSUCCESS; diff --git a/src/test/app/AMMClawbackMPT_test.cpp b/src/test/app/AMMClawbackMPT_test.cpp index 2708218df95..c1da7aab9c1 100644 --- a/src/test/app/AMMClawbackMPT_test.cpp +++ b/src/test/app/AMMClawbackMPT_test.cpp @@ -2,7 +2,7 @@ #include #include -#include +#include #include namespace xrpl { diff --git a/src/test/app/AMMClawback_test.cpp b/src/test/app/AMMClawback_test.cpp index 3160354445f..5803d1a0d39 100644 --- a/src/test/app/AMMClawback_test.cpp +++ b/src/test/app/AMMClawback_test.cpp @@ -2,7 +2,7 @@ #include #include -#include +#include #include namespace xrpl { diff --git a/src/test/app/AMMExtendedMPT_test.cpp b/src/test/app/AMMExtendedMPT_test.cpp index 9ef71dec8c6..852e704d0da 100644 --- a/src/test/app/AMMExtendedMPT_test.cpp +++ b/src/test/app/AMMExtendedMPT_test.cpp @@ -3063,8 +3063,7 @@ struct AMMExtendedMPT_test : public jtx::AMMTest BTC.set({.holder = bob, .flags = tfMPTLock}); { - // different from IOU. The offer is created but not crossed. - env(offer(bob, BTC(5), XRP(25))); + env(offer(bob, BTC(5), XRP(25)), ter(tecLOCKED)); env.close(); BEAST_EXPECT(expectOffers(env, bob, 1, {{{BTC(5), XRP(25)}}})); BEAST_EXPECT(ammAlice.expectBalances(XRP(500), BTC(105), ammAlice.tokens())); @@ -3169,7 +3168,7 @@ struct AMMExtendedMPT_test : public jtx::AMMTest BTC.set({.flags = tfMPTLock}); // assets can't be bought on the market - AMM const ammA3(env, A3, BTC(1), XRP(1), ter(tecFROZEN)); + AMM const ammA3(env, A3, BTC(1), XRP(1), ter(tecLOCKED)); // direct issues can be sent env(pay(G1, A2, BTC(1))); diff --git a/src/test/app/AMMExtended_test.cpp b/src/test/app/AMMExtended_test.cpp index e98822266dd..4b9f00bdd1c 100644 --- a/src/test/app/AMMExtended_test.cpp +++ b/src/test/app/AMMExtended_test.cpp @@ -6,7 +6,7 @@ #include #include -#include +#include #include #include #include diff --git a/src/test/app/AMMMPT_test.cpp b/src/test/app/AMMMPT_test.cpp index 0a2f1de2340..7621dfa9192 100644 --- a/src/test/app/AMMMPT_test.cpp +++ b/src/test/app/AMMMPT_test.cpp @@ -100,7 +100,7 @@ struct AMMMPT_test : public jtx::AMMTest .pay = 30'000, .flags = tfMPTCanLock | MPTDEXFlags}); USD.set({.flags = tfMPTLock}); - AMM const ammAliceFail(env, alice, XRP(10'000), USD(10'000), ter(tecFROZEN)); + AMM const ammAliceFail(env, alice, XRP(10'000), USD(10'000), ter(tecLOCKED)); USD.set({.flags = tfMPTUnlock}); AMM const ammAlice(env, alice, XRP(10'000), USD(10'000)); } @@ -304,7 +304,7 @@ struct AMMMPT_test : public jtx::AMMTest .pay = 30'000, .flags = tfMPTCanLock | MPTDEXFlags}); BTC.set({.flags = tfMPTLock}); - AMM const ammAlice(env, alice, USD(10'000), BTC(10'000), ter(tecFROZEN)); + AMM const ammAlice(env, alice, USD(10'000), BTC(10'000), ter(tecLOCKED)); BEAST_EXPECT(!ammAlice.ammExists()); } @@ -321,7 +321,7 @@ struct AMMMPT_test : public jtx::AMMTest BTC.set({.holder = alice, .flags = tfMPTLock}); // alice's token is locked - AMM const ammAlice(env, alice, USD(10'000), BTC(10'000), ter(tecFROZEN)); + AMM const ammAlice(env, alice, USD(10'000), BTC(10'000), ter(tecLOCKED)); BEAST_EXPECT(!ammAlice.ammExists()); // bob can create @@ -646,14 +646,14 @@ struct AMMMPT_test : public jtx::AMMTest BTC.set({.flags = tfMPTLock}); ammAlice.deposit( - carol, BTC(100), std::nullopt, std::nullopt, std::nullopt, ter(tecFROZEN)); + carol, BTC(100), std::nullopt, std::nullopt, std::nullopt, ter(tecLOCKED)); ammAlice.deposit( - carol, USD(100), std::nullopt, std::nullopt, std::nullopt, ter(tecFROZEN)); + carol, USD(100), std::nullopt, std::nullopt, std::nullopt, ter(tecLOCKED)); - ammAlice.deposit(carol, 1'000, std::nullopt, std::nullopt, ter(tecFROZEN)); + ammAlice.deposit(carol, 1'000, std::nullopt, std::nullopt, ter(tecLOCKED)); - ammAlice.deposit(carol, USD(100), BTC(100), std::nullopt, std::nullopt, ter(tecFROZEN)); + ammAlice.deposit(carol, USD(100), BTC(100), std::nullopt, std::nullopt, ter(tecLOCKED)); } // Individually lock MPT or freeze IOU (AMM) with IOU/MPT AMM @@ -674,12 +674,17 @@ struct AMMMPT_test : public jtx::AMMTest // Carol can not deposit locked mpt ammAlice.deposit( - carol, BTC(100), std::nullopt, std::nullopt, std::nullopt, ter(tecFROZEN)); + carol, BTC(100), std::nullopt, std::nullopt, std::nullopt, ter(tecLOCKED)); - ammAlice.deposit(carol, 1'000, std::nullopt, std::nullopt, ter(tecFROZEN)); + ammAlice.deposit(carol, 1'000, std::nullopt, std::nullopt, ter(tecLOCKED)); if (!features[featureAMMClawback]) { + ammAlice.deposit(carol, USD(100), std::nullopt, std::nullopt, std::nullopt); + } + else + { + // Carol can not deposit non-frozen token either ammAlice.deposit( carol, USD(100), std::nullopt, std::nullopt, std::nullopt, ter(tecLOCKED)); } @@ -728,9 +733,9 @@ struct AMMMPT_test : public jtx::AMMTest ammAlice.deposit(carol, USD(100), std::nullopt, std::nullopt, std::nullopt); // Can not deposit locked token - ammAlice.deposit(carol, 1'000, std::nullopt, std::nullopt, ter(tecFROZEN)); + ammAlice.deposit(carol, 1'000, std::nullopt, std::nullopt, ter(tecLOCKED)); ammAlice.deposit( - carol, BTC(100), std::nullopt, std::nullopt, std::nullopt, ter(tecFROZEN)); + carol, BTC(100), std::nullopt, std::nullopt, std::nullopt, ter(tecLOCKED)); // Unlock AMM MPT BTC.set({.holder = ammAlice.ammAccount(), .flags = tfMPTUnlock}); @@ -762,10 +767,10 @@ struct AMMMPT_test : public jtx::AMMTest // Carol's BTC is locked BTC.set({.holder = carol, .flags = tfMPTLock}); ammAlice.deposit( - carol, USD(100), std::nullopt, std::nullopt, std::nullopt, ter(tecFROZEN)); + carol, USD(100), std::nullopt, std::nullopt, std::nullopt, ter(tecLOCKED)); ammAlice.deposit( - carol, BTC(100), std::nullopt, std::nullopt, std::nullopt, ter(tecFROZEN)); + carol, BTC(100), std::nullopt, std::nullopt, std::nullopt, ter(tecLOCKED)); // Unlock carol's BTC BTC.set({.holder = carol, .flags = tfMPTUnlock}); @@ -781,9 +786,9 @@ struct AMMMPT_test : public jtx::AMMTest ammAlice.deposit(carol, USD(100), std::nullopt, std::nullopt, std::nullopt); // Can not deposit locked token BTC - ammAlice.deposit(carol, 1'000, std::nullopt, std::nullopt, ter(tecFROZEN)); + ammAlice.deposit(carol, 1'000, std::nullopt, std::nullopt, ter(tecLOCKED)); ammAlice.deposit( - carol, BTC(100), std::nullopt, std::nullopt, std::nullopt, ter(tecFROZEN)); + carol, BTC(100), std::nullopt, std::nullopt, std::nullopt, ter(tecLOCKED)); // Unlock AMM MPT BTC BTC.set({.holder = ammAlice.ammAccount(), .flags = tfMPTUnlock}); @@ -798,9 +803,9 @@ struct AMMMPT_test : public jtx::AMMTest ammAlice.deposit(carol, BTC(100), std::nullopt, std::nullopt, std::nullopt); // Can not deposit locked token USD - ammAlice.deposit(carol, 1'000, std::nullopt, std::nullopt, ter(tecFROZEN)); + ammAlice.deposit(carol, 1'000, std::nullopt, std::nullopt, ter(tecLOCKED)); ammAlice.deposit( - carol, USD(100), std::nullopt, std::nullopt, std::nullopt, ter(tecFROZEN)); + carol, USD(100), std::nullopt, std::nullopt, std::nullopt, ter(tecLOCKED)); // Unlock AMM MPT USD USD.set({.holder = ammAlice.ammAccount(), .flags = tfMPTUnlock}); @@ -854,7 +859,7 @@ struct AMMMPT_test : public jtx::AMMTest AMM amm(env, gw, XRP(10'000), BTC(10'000)); - amm.deposit({.account = alice, .asset1In = BTC(10), .err = ter(tecNO_PERMISSION)}); + amm.deposit({.account = alice, .asset1In = BTC(10), .err = ter(tecNO_AUTH)}); } // Insufficient XRP balance @@ -2212,7 +2217,7 @@ struct AMMMPT_test : public jtx::AMMTest .account = alice, .asset1Out = BTC(100), .assets = {{XRP, BTC}}, - .err = ter(tecNO_PERMISSION)}); + .err = ter(tecNO_AUTH)}); } // Globally locked MPT @@ -2233,8 +2238,8 @@ struct AMMMPT_test : public jtx::AMMTest BTC.set({.flags = tfMPTLock}); ammAlice.withdraw( - alice, MPT(ammAlice[1])(100), std::nullopt, std::nullopt, ter(tecFROZEN)); - ammAlice.withdraw(alice, 1'000, std::nullopt, std::nullopt, ter(tecFROZEN)); + alice, MPT(ammAlice[1])(100), std::nullopt, std::nullopt, ter(tecLOCKED)); + ammAlice.withdraw(alice, 1'000, std::nullopt, std::nullopt, ter(tecLOCKED)); // can single withdraw the other asset ammAlice.withdraw({.account = alice, .asset1Out = XRP(100)}); @@ -2262,8 +2267,8 @@ struct AMMMPT_test : public jtx::AMMTest // Alice's BTC is locked BTC.set({.holder = alice, .flags = tfMPTLock}); - ammAlice.withdraw(alice, 1000, std::nullopt, std::nullopt, ter(tecFROZEN)); - ammAlice.withdraw(alice, BTC(100), std::nullopt, std::nullopt, ter(tecFROZEN)); + ammAlice.withdraw(alice, 1000, std::nullopt, std::nullopt, ter(tecLOCKED)); + ammAlice.withdraw(alice, BTC(100), std::nullopt, std::nullopt, ter(tecLOCKED)); // can withdraw the other asset ammAlice.withdraw(alice, USD(100), std::nullopt, std::nullopt); @@ -2291,8 +2296,8 @@ struct AMMMPT_test : public jtx::AMMTest // Alice's BTC is locked BTC.set({.holder = alice, .flags = tfMPTLock}); - ammAlice.withdraw(alice, 1'000, std::nullopt, std::nullopt, ter(tecFROZEN)); - ammAlice.withdraw(alice, BTC(100), std::nullopt, std::nullopt, ter(tecFROZEN)); + ammAlice.withdraw(alice, 1'000, std::nullopt, std::nullopt, ter(tecLOCKED)); + ammAlice.withdraw(alice, BTC(100), std::nullopt, std::nullopt, ter(tecLOCKED)); // can still single withdraw the unlocked other asset ammAlice.withdraw(alice, USD(100), std::nullopt, std::nullopt); @@ -2311,8 +2316,8 @@ struct AMMMPT_test : public jtx::AMMTest ammAlice.withdraw(alice, USD(100), std::nullopt, std::nullopt); // Can not withdraw locked token BTC - ammAlice.withdraw(alice, 1'000, std::nullopt, std::nullopt, ter(tecFROZEN)); - ammAlice.withdraw(alice, BTC(100), std::nullopt, std::nullopt, ter(tecFROZEN)); + ammAlice.withdraw(alice, 1'000, std::nullopt, std::nullopt, ter(tecLOCKED)); + ammAlice.withdraw(alice, BTC(100), std::nullopt, std::nullopt, ter(tecLOCKED)); // Unlock AMM MPT BTC.set({.holder = ammAlice.ammAccount(), .flags = tfMPTUnlock}); @@ -6808,33 +6813,33 @@ struct AMMMPT_test : public jtx::AMMTest cb(amm, BTC); }; - // Deposit two assets, one of which is frozen, - // then we should get tecFROZEN error. + // Deposit two assets, one of which is locked, + // then we should get tecLOCKED error. { Env env(*this); testAMMDeposit(env, [&](AMM& amm, MPTTester& BTC) { - amm.deposit(alice, BTC(100), XRP(100), std::nullopt, tfTwoAsset, ter(tecFROZEN)); + amm.deposit(alice, BTC(100), XRP(100), std::nullopt, tfTwoAsset, ter(tecLOCKED)); }); } - // Deposit one asset, which is the frozen token, - // then we should get tecFROZEN error. + // Deposit one asset, which is the frozen locked, + // then we should get tecLOCKED error. { Env env(*this); testAMMDeposit(env, [&](AMM& amm, MPTTester& BTC) { amm.deposit( - alice, BTC(100), std::nullopt, std::nullopt, tfSingleAsset, ter(tecFROZEN)); + alice, BTC(100), std::nullopt, std::nullopt, tfSingleAsset, ter(tecLOCKED)); }); } // Deposit one asset which is not the frozen token, - // but the other asset is frozen. We should get tecFROZEN error + // but the other asset is frozen. We should get tecLOCKED error // when feature AMMClawback is enabled. { Env env(*this); testAMMDeposit(env, [&](AMM& amm, MPTTester& BTC) { amm.deposit( - alice, XRP(100), std::nullopt, std::nullopt, tfSingleAsset, ter(tecFROZEN)); + alice, XRP(100), std::nullopt, std::nullopt, tfSingleAsset, ter(tecLOCKED)); }); } } diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index 8a56e841d6c..a434eb96c79 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -8,7 +8,6 @@ #include #include -#include #include #include #include diff --git a/src/test/app/Batch_test.cpp b/src/test/app/Batch_test.cpp index f301b6d60f9..0bc2157b9d2 100644 --- a/src/test/app/Batch_test.cpp +++ b/src/test/app/Batch_test.cpp @@ -4155,8 +4155,12 @@ class Batch_test : public beast::unit_test::suite std::vector const testCases = { {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, {1, "TrustSet", "tesSUCCESS", txIDs[0], batchID}, + // jv2 fails with terNO_DELEGATE_PERMISSION. }; validateClosedLedger(env, testCases); + + // verify jv2 is not present in the closed ledger. + BEAST_EXPECT(env.rpc("tx", txIDs[1])[jss::result][jss::error] == "txnNotFound"); } } diff --git a/src/test/app/CheckMPT_test.cpp b/src/test/app/CheckMPT_test.cpp index a1f35d0507f..310a64a4fe3 100644 --- a/src/test/app/CheckMPT_test.cpp +++ b/src/test/app/CheckMPT_test.cpp @@ -1813,10 +1813,10 @@ class CheckMPT_test : public beast::unit_test::suite // Use offers to automatically create MPT. MPT const OF4 = gw1["OF4"]; gw1.set(OF4, tfMPTLock); - env(offer(gw1, XRP(92), OF4(92)), ter(tecFROZEN)); + env(offer(gw1, XRP(92), OF4(92)), ter(tecLOCKED)); env.close(); BEAST_EXPECT(env.le(keylet::mptoken(OF4, alice)) == nullptr); - env(offer(alice, OF4(92), XRP(92)), ter(tecFROZEN)); + env(offer(alice, OF4(92), XRP(92)), ter(tecLOCKED)); env.close(); // No one's owner count should have changed. @@ -1904,10 +1904,10 @@ class CheckMPT_test : public beast::unit_test::suite // Use offers to automatically create MPT. MPT const OF4 = gw1["OF4"]; gw1.set(OF4, tfMPTLock); - env(offer(alice, XRP(91), OF4(91)), ter(tecFROZEN)); + env(offer(alice, XRP(91), OF4(91)), ter(tecLOCKED)); env.close(); BEAST_EXPECT(env.le(keylet::mptoken(OF4, alice)) == nullptr); - env(offer(bob, OF4(91), XRP(91)), ter(tecFROZEN)); + env(offer(bob, OF4(91), XRP(91)), ter(tecLOCKED)); env.close(); // No one's owner count should have changed. diff --git a/src/test/app/Delegate_test.cpp b/src/test/app/Delegate_test.cpp index d6e970190ef..1d036266ad1 100644 --- a/src/test/app/Delegate_test.cpp +++ b/src/test/app/Delegate_test.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -1856,6 +1857,21 @@ class Delegate_test : public beast::unit_test::suite "\n Action: Verify security requirements to interact with Delegation feature"); } + void + testDelegateUtilsNullptrCheck() + { + testcase("DelegateUtils nullptr check"); + + // checkTxPermission nullptr check + STTx const tx{ttPAYMENT, [](STObject&) {}}; + BEAST_EXPECT(checkTxPermission(nullptr, tx) == terNO_DELEGATE_PERMISSION); + + // loadGranularPermission nullptr check + std::unordered_set granularPermissions; + loadGranularPermission(nullptr, ttPAYMENT, granularPermissions); + BEAST_EXPECT(granularPermissions.empty()); + } + void run() override { @@ -1881,6 +1897,7 @@ class Delegate_test : public beast::unit_test::suite testPermissionValue(all); testTxRequireFeatures(all); testTxDelegableCount(); + testDelegateUtilsNullptrCheck(); } }; BEAST_DEFINE_TESTSUITE(Delegate, app, xrpl); diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index 69dd3972107..6b4cac1f331 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -3395,10 +3395,10 @@ class MPToken_test : public beast::unit_test::suite auto const [errBuy, errSell] = [&]() -> std::pair { // Global lock if (lockMPTIssue) - return std::make_pair(tecFROZEN, tecFROZEN); + return std::make_pair(tecLOCKED, tecLOCKED); // Local lock if (lockMPToken) - return std::make_pair(tesSUCCESS, error(tecUNFUNDED_OFFER)); + return std::make_pair(error(tecLOCKED), error(tecUNFUNDED_OFFER)); // MPToken doesn't exist if (requireAuth) return std::make_pair(error(tecNO_AUTH), error(tecUNFUNDED_OFFER)); @@ -3496,10 +3496,9 @@ class MPToken_test : public beast::unit_test::suite else { auto const err = flag == tfMPTLock ? ter(tecUNFUNDED_OFFER) : ter(tesSUCCESS); + auto const err1 = flag == tfMPTLock ? ter(tecLOCKED) : ter(tesSUCCESS); env(offer(alice, ETH(1), BTC(1)), err); - // Offer created by not crossed - env(offer(carol, BTC(1), ETH(1))); - BEAST_EXPECT(expectOffers(env, carol, 1, {{BTC(1), ETH(1)}})); + env(offer(carol, BTC(1), ETH(1)), err1); } }; @@ -5606,7 +5605,7 @@ class MPToken_test : public beast::unit_test::suite env(offer(bob, USD1(100), GBP(100))); // dan has USD/gw and USD1/gw. Had USD been IOU, it would have - // been able to ripple through dan's account. + // rippled through dan's account. auto const [pathSet, srcAmt, dstAmt] = find_paths(env, john, sean, GBP(-1), XRP(-1)); BEAST_EXPECT(pathSet.empty()); @@ -5833,7 +5832,7 @@ class MPToken_test : public beast::unit_test::suite env.close(); } - // MPTCanTransfer disabled + // MPTCanTransfer is disabled { Env env{*this, features}; env.fund(XRP(1'000), gw, alice, carol); @@ -5874,49 +5873,44 @@ class MPToken_test : public beast::unit_test::suite BEAST_EXPECT(env.balance(alice, mpt) == mpt(0)); BEAST_EXPECT(env.balance(gw, mpt) == mpt(0)); - // neither src nor dst is issuer, can still create + // neither src nor dst is issuer, can't create checkId = keylet::check(alice, env.seq(alice)).key; - env(check::create(alice, carol, mpt(100))); - env.close(); - - // can't cash - env(check::cash(carol, checkId, mpt(10)), ter(tecPATH_PARTIAL)); + env(check::create(alice, carol, mpt(100)), ter(tecNO_AUTH)); env.close(); - // can cash now + // can create now mpt.set({.account = gw, .mutableFlags = tmfMPTSetCanTransfer}); + checkId = keylet::check(alice, env.seq(alice)).key; + env(check::create(alice, carol, mpt(100))); + env.close(); + // can cash env(pay(gw, alice, mpt(10))); env.close(); env(check::cash(carol, checkId, mpt(10))); env.close(); } - // MPTCanTrade disabled + // MPTCanTrade is disabled { Env env{*this, features}; env.fund(XRP(1'000), gw, alice, carol); env.close(); - MPTTester mpt( + MPT const mpt = MPTTester( {.env = env, .issuer = gw, .holders = {alice, carol}, - .flags = tfMPTCanTransfer, - .mutableFlags = tmfMPTCanMutateCanTrade}); + .pay = 10, + .flags = tfMPTCanTransfer}); - uint256 checkId{keylet::check(gw, env.seq(gw)).key}; + uint256 const checkId{keylet::check(alice, env.seq(alice)).key}; - // can't create - env(check::create(gw, alice, mpt(100)), ter(tecNO_PERMISSION)); + // can create + env(check::create(alice, carol, mpt(100))); env.close(); - mpt.set({.account = gw, .mutableFlags = tmfMPTSetCanTrade}); - // can't cash - checkId = keylet::check(gw, env.seq(gw)).key; - env(check::create(gw, carol, mpt(100))); - env.close(); - mpt.set({.account = gw, .mutableFlags = tmfMPTClearCanTrade}); - env(check::cash(carol, checkId, mpt(10)), ter(tecNO_PERMISSION)); + // can cash + env(check::cash(carol, checkId, mpt(10))); env.close(); } @@ -5971,33 +5965,6 @@ class MPToken_test : public beast::unit_test::suite env.close(); } - // MPTCanTransfer is not set and the account is not the issuer of MPT - { - Env env{*this, features}; - env.fund(XRP(1'000), gw, alice, carol); - - auto EUR = MPTTester( - {.env = env, .issuer = gw, .holders = {alice, carol}, .flags = tfMPTCanTrade}); - uint256 const chkId{getCheckIndex(alice, env.seq(alice))}; - // alice can create - env(check::create(alice, carol, EUR(1))); - env.close(); - - // carol can't cash - env(check::cash(carol, chkId, EUR(1)), ter(tecPATH_PARTIAL)); - env.close(); - - // if issuer creates a check then carol can cash since - // it's a transfer from the issuer - uint256 const chkId1{getCheckIndex(gw, env.seq(gw))}; - // alice can't create since CanTransfer is not set - env(check::create(gw, carol, EUR(1))); - env.close(); - - env(check::cash(carol, chkId1, EUR(1))); - env.close(); - } - // Can create check if src/dst don't own MPT { Env env{*this, features}; @@ -6158,8 +6125,7 @@ class MPToken_test : public beast::unit_test::suite AMM amm(env, gw, BTC(100), USD(100)); env.close(); // alice can't deposit since MPTCanTransfer is not set - amm.deposit( - DepositArg{.account = alice, .tokens = 1'000, .err = ter(tecNO_PERMISSION)}); + amm.deposit(DepositArg{.account = alice, .tokens = 1'000, .err = ter(tecNO_AUTH)}); env.close(); // can't clawback since alice is not an LP @@ -6392,8 +6358,8 @@ class MPToken_test : public beast::unit_test::suite // alice and issuer can't create USD.set({.flags = tfMPTLock}); - createFail(alice, tecFROZEN); - createFail(gw, tecFROZEN); + createFail(alice, tecLOCKED); + createFail(gw, tecLOCKED); // MPTRequireAuth is set @@ -6413,7 +6379,7 @@ class MPToken_test : public beast::unit_test::suite USD.set({.mutableFlags = tmfMPTClearRequireAuth}); USD.set({.mutableFlags = tmfMPTClearCanTransfer}); // alice can't create - createFail(alice, tecNO_PERMISSION); + createFail(alice, tecNO_AUTH); // issuer can create createDeleteAMM(gw); USD.set({.mutableFlags = tmfMPTSetCanTransfer}); @@ -6459,12 +6425,12 @@ class MPToken_test : public beast::unit_test::suite {.account = account, .asset1In = USD(1), .asset2In = EUR(1), - .err = ter(tecFROZEN)}); + .err = ter(tecLOCKED)}); amm.deposit( {.account = account, .asset1In = EUR(1), .assets = std::make_pair(EUR, USD), - .err = ter(tecFROZEN)}); + .err = ter(tecLOCKED)}); } USD.set({.flags = tfMPTUnlock}); @@ -6499,15 +6465,12 @@ class MPToken_test : public beast::unit_test::suite USD.set({.mutableFlags = tmfMPTClearCanTransfer}); // carol can't deposit amm.deposit( - {.account = carol, - .asset1In = USD(1), - .asset2In = EUR(1), - .err = ter(tecNO_PERMISSION)}); + {.account = carol, .asset1In = USD(1), .asset2In = EUR(1), .err = ter(tecNO_AUTH)}); amm.deposit( {.account = carol, .asset1In = EUR(1), .assets = std::make_pair(EUR, USD), - .err = ter(tecNO_PERMISSION)}); + .err = ter(tecNO_AUTH)}); // issuer can deposit amm.deposit({.account = gw, .tokens = 1'000}); // carol can deposit @@ -6549,8 +6512,8 @@ class MPToken_test : public beast::unit_test::suite {.account = account, .asset1Out = USD(1), .asset2Out = EUR(1), - .err = ter(tecFROZEN)}); - amm.withdraw({.account = account, .tokens = 1'000, .err = ter(tecFROZEN)}); + .err = ter(tecLOCKED)}); + amm.withdraw({.account = account, .tokens = 1'000, .err = ter(tecLOCKED)}); // can single withdraw another asset amm.withdraw( {.account = account, .asset1Out = EUR(1), .assets = std::make_pair(EUR, USD)}); @@ -6576,7 +6539,7 @@ class MPToken_test : public beast::unit_test::suite USD.authorize({.account = gw, .holder = carol}); amm.withdraw({.account = carol, .asset1Out = USD(1), .asset2Out = EUR(1)}); - // MPTCanTransfer is set + // MPTCanTransfer is not set USD.set({.mutableFlags = tmfMPTClearRequireAuth}); USD.set({.mutableFlags = tmfMPTClearCanTransfer}); @@ -6585,7 +6548,7 @@ class MPToken_test : public beast::unit_test::suite {.account = carol, .asset1Out = USD(1), .asset2Out = EUR(1), - .err = ter(tecNO_PERMISSION)}); + .err = ter(tecNO_AUTH)}); // can withdraw another asset amm.withdraw( {.account = carol, .asset1Out = EUR(1), .assets = std::make_pair(EUR, USD)}); @@ -6596,6 +6559,9 @@ class MPToken_test : public beast::unit_test::suite amm.withdraw({.account = carol, .asset1Out = USD(1), .asset2Out = EUR(1)}); USD.set({.mutableFlags = tmfMPTSetCanTransfer}); + + // MPTCanTrade is not set + USD.set({.mutableFlags = tmfMPTClearCanTrade}); amm.withdraw({.account = gw, .tokens = 1'000, .err = ter(tecNO_PERMISSION)}); amm.withdraw({.account = carol, .tokens = 1'000, .err = ter(tecNO_PERMISSION)}); @@ -6617,6 +6583,48 @@ class MPToken_test : public beast::unit_test::suite } } + void + testFixDoubleOwnerCount(FeatureBitset all) + { + testcase("Fix Double adjustOwnerCount in AMMWithdraw"); + + using namespace jtx; + + // Carol deposits XRP into an XRP/MPT pool, then withdraws MPT. + // Carol has no MPToken before the withdrawal. If the bug exists, + // her ownerCount will be inflated by +1 extra. + Account const gw{"gw"}; + Account const alice{"alice"}; + Account const carol{"carol"}; + Env env(*this, all); + env.fund(XRP(30'000), gw, alice, carol); + env.close(); + + // Create MPT with DEX flags. Only alice is a holder initially. + MPT const BTC = MPTTester( + {.env = env, .issuer = gw, .holders = {alice}, .pay = 20'000, .flags = MPTDEXFlags}); + + // Alice creates XRP/MPT AMM pool + AMM amm(env, alice, XRP(10'000), BTC(10'000)); + + // Carol deposits XRP (single asset) into the pool. + // Carol gets LP tokens but does NOT have an MPToken yet. + auto const carolOwnersBefore = ownerCount(env, carol); + amm.deposit(carol, XRP(1'000), std::nullopt, std::nullopt, tfSingleAsset); + auto const carolOwnersAfterDeposit = ownerCount(env, carol); + // Carol should have +1 for LP token trustline + BEAST_EXPECT(carolOwnersAfterDeposit == carolOwnersBefore + 1); + + auto const carolOwnersBeforeWithdraw = ownerCount(env, carol); + // Carol withdraws single MPT asset. She doesn't have an MPToken, + // so one must be created. Bug: ownerCount incremented twice. + amm.withdraw({.account = carol, .asset1Out = BTC(100), .flags = tfSingleAsset}); + auto const carolOwnersAfterWithdraw = ownerCount(env, carol); + + // Expected: +1 for the new MPToken (so total increase = 1) + BEAST_EXPECT(carolOwnersAfterWithdraw == carolOwnersBeforeWithdraw + 1); + } + public: void run() override @@ -6723,6 +6731,9 @@ class MPToken_test : public beast::unit_test::suite // Test AMM testBasicAMM(all); + + // Fixes + testFixDoubleOwnerCount(all); } }; diff --git a/src/test/jtx/impl/AMM.cpp b/src/test/jtx/impl/AMM.cpp index dfc08497932..79f4fa222e3 100644 --- a/src/test/jtx/impl/AMM.cpp +++ b/src/test/jtx/impl/AMM.cpp @@ -3,7 +3,6 @@ #include #include -#include #include #include #include diff --git a/src/test/jtx/impl/WSClient.cpp b/src/test/jtx/impl/WSClient.cpp index 2c3389c1310..0c9b72c4d05 100644 --- a/src/test/jtx/impl/WSClient.cpp +++ b/src/test/jtx/impl/WSClient.cpp @@ -184,14 +184,7 @@ class WSClientImpl : public WSClient jp[jss::command] = cmd; } auto const s = to_string(jp); - - // Use the error_code overload to avoid an unhandled exception - // when the server closes the WebSocket connection (e.g. after - // booting a client that exceeded resource thresholds). - error_code ec; - ws_.write_some(true, buffer(s), ec); - if (ec) - return {}; + ws_.write_some(true, buffer(s)); } auto jv = diff --git a/src/xrpld/app/ledger/OrderBookDBImpl.cpp b/src/xrpld/app/ledger/OrderBookDBImpl.cpp index 67597531abd..1a764d952f5 100644 --- a/src/xrpld/app/ledger/OrderBookDBImpl.cpp +++ b/src/xrpld/app/ledger/OrderBookDBImpl.cpp @@ -2,7 +2,7 @@ #include #include -#include +#include #include #include diff --git a/src/xrpld/rpc/handlers/orderbook/AMMInfo.cpp b/src/xrpld/rpc/handlers/orderbook/AMMInfo.cpp index 425642b471b..272e66018c2 100644 --- a/src/xrpld/rpc/handlers/orderbook/AMMInfo.cpp +++ b/src/xrpld/rpc/handlers/orderbook/AMMInfo.cpp @@ -5,7 +5,7 @@ #include #include #include -#include +#include #include #include