diff --git a/include/xrpl/ledger/helpers/TokenHelpers.h b/include/xrpl/ledger/helpers/TokenHelpers.h index f736e51d285..661867c336f 100644 --- a/include/xrpl/ledger/helpers/TokenHelpers.h +++ b/include/xrpl/ledger/helpers/TokenHelpers.h @@ -72,6 +72,9 @@ isIndividualFrozen(ReadView const& view, AccountID const& account, Asset const& [[nodiscard]] TER checkIndividualFrozen(ReadView const& view, AccountID const& account, Asset const& asset); +[[nodiscard]] TER +checkIndividualDeepFrozen(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 @@ -131,6 +134,60 @@ checkDeepFrozen(ReadView const& view, AccountID const& account, MPTIssue const& [[nodiscard]] TER checkDeepFrozen(ReadView const& view, AccountID const& account, Asset const& asset); +/** + * Checks freeze compliance for withdrawing an asset from a pseudo-account + * (e.g. Vault, AMM, LoanBroker) to a destination account. + * + * Asserts that sourceAcct is a pseudo-account. + * + * Issuer exemption: returns tesSUCCESS immediately when dstAcct is the asset + * issuer — the issuer can always receive their own token, even when the pool + * is frozen. Callers that need to block withdrawals from a frozen pool even + * for the issuer (e.g. because the pool math cannot handle it) must check + * checkFrozen(sourceAcct, asset) separately before calling this function. + * + * Otherwise checks, in order: + * 1. checkFrozen(sourceAcct, asset) — the pseudo-account's trustline / + * global freeze must not block sending. + * 2. checkFrozen(submitterAcct, asset) — skipped when submitter == dst + * (self-withdrawal); a regular freeze should not prevent recovering + * one's own funds. + * 3. checkDeepFrozen(dstAcct, asset) — the destination must not be + * deep-frozen (cannot receive under any circumstance). + * + * For IOUs a regular individual freeze on the withdrawer does NOT block + * self-withdrawal; only deep freeze does. For MPTs "locked" is equivalent + * to deep-frozen, so locked MPT holders are always blocked. + */ +[[nodiscard]] TER +checkWithdrawFreeze( + ReadView const& view, + AccountID const& srcAcct, + AccountID const& submitterAcct, + AccountID const& dstAcct, + Asset const& asset); + +/** + * Checks freeze compliance for depositing an asset into a pseudo-account + * (e.g. Vault, AMM, LoanBroker). + * + * Asserts that dstAcct is a pseudo-account. + * + * Checks, in order: + * 1. checkFrozen(srcAcct, asset) — the depositor must not be frozen + * (global or individual) for the asset. + * 2. checkFrozen(dstAcct, asset) — the pseudo-account must not be + * frozen for the asset. Unlike regular accounts, pseudo-accounts + * cannot receive deposits under a regular freeze because the + * deposited funds could not later be withdrawn. + */ +[[nodiscard]] TER +checkDepositFreeze( + ReadView const& view, + AccountID const& srcAcct, + AccountID const& dstAcct, + Asset const& asset); + //------------------------------------------------------------------------------ // // Account balance functions (Asset-based dispatchers) diff --git a/include/xrpl/tx/transactors/dex/AMMWithdraw.h b/include/xrpl/tx/transactors/dex/AMMWithdraw.h index 6e88320eae7..8b6d39349ba 100644 --- a/include/xrpl/tx/transactors/dex/AMMWithdraw.h +++ b/include/xrpl/tx/transactors/dex/AMMWithdraw.h @@ -159,6 +159,11 @@ class AMMWithdraw : public Transactor beast::Journal const& journal); private: + /** Returns IgnoreFreeze when the withdrawer is the issuer of a pool + * asset (post-fixCleanup3_3_0), ZeroIfFrozen otherwise. */ + [[nodiscard]] FreezeHandling + issuerFreezeHandling() const; + std::pair applyGuts(Sandbox& view); diff --git a/src/libxrpl/ledger/helpers/TokenHelpers.cpp b/src/libxrpl/ledger/helpers/TokenHelpers.cpp index 71914568681..a9f4c0459cf 100644 --- a/src/libxrpl/ledger/helpers/TokenHelpers.cpp +++ b/src/libxrpl/ledger/helpers/TokenHelpers.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -79,6 +80,14 @@ checkIndividualFrozen(ReadView const& view, AccountID const& account, Asset cons return tesSUCCESS; } +TER +checkIndividualDeepFrozen(ReadView const& view, AccountID const& account, Asset const& asset) +{ + if (isDeepFrozen(view, account, asset)) + return asset.holds() ? tecLOCKED : tecFROZEN; + return tesSUCCESS; +} + bool isFrozen(ReadView const& view, AccountID const& account, Asset const& asset, std::uint8_t depth) { @@ -164,6 +173,86 @@ checkDeepFrozen(ReadView const& view, AccountID const& account, Asset const& ass [&](auto const& issue) { return checkDeepFrozen(view, account, issue); }, asset.value()); } +[[nodiscard]] TER +checkWithdrawFreeze( + ReadView const& view, + AccountID const& srcAcct, + AccountID const& submitterAcct, + AccountID const& dstAcct, + Asset const& asset) +{ + XRPL_ASSERT( + isPseudoAccount(view, srcAcct), + "xrpl::checkWithdrawFreeze : source must be a pseudo-account"); + + // Funds can always be sent to the issuer + if (dstAcct == asset.getIssuer()) + return tesSUCCESS; + + // If the asset is globally frozen, other checks are redundant + if (auto const ret = checkGlobalFrozen(view, asset)) + return ret; + + // Special case for shares - check if the shares (and the transitive asset) is not frozen + if (asset.holds() && + isVaultPseudoAccountFrozen(view, dstAcct, asset.get(), 0)) + { + return tecLOCKED; + } + + // The transfer is from Submitter to Destination via Source (pseudo-account) + // Both Source and Submitter must not be frozen to allow sending funds + if (auto const ret = checkIndividualFrozen(view, srcAcct, asset)) + return ret; + + // Check submitter's individual freeze only when Submitter != Destination (a regular freeze + // should not block self-withdrawal). + if (submitterAcct != dstAcct) + { + if (auto const ret = checkIndividualFrozen(view, submitterAcct, asset)) + return ret; + } + + // The destination account must not be deep frozen to receive the funds + return checkIndividualDeepFrozen(view, dstAcct, asset); +} + +[[nodiscard]] TER +checkDepositFreeze( + ReadView const& view, + AccountID const& srcAcct, + AccountID const& dstAcct, + Asset const& asset) +{ + XRPL_ASSERT( + isPseudoAccount(view, dstAcct), + "xrpl::checkDepositFreeze : destination must be a pseudo-account"); + + // An Issuer cannot deposit when: + // 1. Asset is globally frozen + // 2. The trustline of the pseudo-account is frozen + + if (auto const ret = checkGlobalFrozen(view, asset)) + return ret; + + // Special case for shares - check if the shares (and the transitive asset) is not frozen + if (asset.holds() && + isVaultPseudoAccountFrozen(view, dstAcct, asset.get(), 0)) + { + return tecLOCKED; + } + + if (srcAcct != asset.getIssuer()) + { + if (auto const ret = checkIndividualFrozen(view, srcAcct, asset)) + return ret; + } + + // Unlike regular accounts, pseudo-accounts cannot receive deposits under a regular freeze + // because those funds cannot be later withdrawn + return checkIndividualFrozen(view, dstAcct, asset); +} + //------------------------------------------------------------------------------ // // Account balance functions diff --git a/src/libxrpl/tx/transactors/dex/AMMDeposit.cpp b/src/libxrpl/tx/transactors/dex/AMMDeposit.cpp index 91858e3cd7e..0ee234fe974 100644 --- a/src/libxrpl/tx/transactors/dex/AMMDeposit.cpp +++ b/src/libxrpl/tx/transactors/dex/AMMDeposit.cpp @@ -251,7 +251,36 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) : tecUNFUNDED_AMM; }; - if (ctx.view.rules().enabled(featureAMMClawback)) + auto const amount = ctx.tx[~sfAmount]; + auto const amount2 = ctx.tx[~sfAmount2]; + auto const ammAccountID = ammSle->getAccountID(sfAccount); + + if (ctx.view.rules().enabled(fixCleanup3_3_0)) + { + // Unified deposit freeze check for both pool assets. + // AMMDeposit is not allowed if either asset is frozen. + auto checkAsset = [&](Asset const& asset) -> TER { + if (auto const ter = requireAuth(ctx.view, asset, accountID, AuthType::WeakAuth)) + { + JLOG(ctx.j.debug()) << "AMM Deposit: account is not authorized, " << asset; + return ter; + } + if (auto const ter = checkDepositFreeze(ctx.view, accountID, ammAccountID, asset)) + { + JLOG(ctx.j.debug()) + << "AMM Deposit: frozen, " << to_string(accountID) << " " << to_string(asset); + return ter; + } + return tesSUCCESS; + }; + + if (auto const ter = checkAsset(ctx.tx[sfAsset])) + return ter; + + if (auto const ter = checkAsset(ctx.tx[sfAsset2])) + return ter; + } + else if (ctx.view.rules().enabled(featureAMMClawback)) { // Check if either of the assets is frozen, AMMDeposit is not allowed // if either asset is frozen @@ -283,10 +312,6 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) return ter; } - auto const amount = ctx.tx[~sfAmount]; - auto const amount2 = ctx.tx[~sfAmount2]; - auto const ammAccountID = ammSle->getAccountID(sfAccount); - auto checkAmount = [&](std::optional const& amount, bool checkBalance) -> TER { if (amount) { @@ -301,21 +326,26 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) return ter; // LCOV_EXCL_STOP } - // AMM account or currency frozen - if (auto const ter = checkFrozen(ctx.view, ammAccountID, amount->asset()); - !isTesSuccess(ter)) - { - JLOG(ctx.j.debug()) << "AMM Deposit: AMM account or currency is frozen or locked, " - << to_string(accountID); - return ter; - } - // Account frozen - if (auto const ter = checkIndividualFrozen(ctx.view, accountID, amount->asset()); - !isTesSuccess(ter)) + if (!ctx.view.rules().enabled(fixCleanup3_3_0)) { - JLOG(ctx.j.debug()) << "AMM Deposit: account is frozen or locked, " - << to_string(accountID) << " " << to_string(amount->asset()); - return ter; + // AMM account or currency frozen + if (auto const ter = checkFrozen(ctx.view, ammAccountID, amount->asset()); + !isTesSuccess(ter)) + { + JLOG(ctx.j.debug()) + << "AMM Deposit: AMM account or currency is frozen or locked, " + << to_string(accountID); + return ter; + } + // Account frozen + if (auto const ter = checkIndividualFrozen(ctx.view, accountID, amount->asset()); + !isTesSuccess(ter)) + { + JLOG(ctx.j.debug()) + << "AMM Deposit: account is frozen or locked, " << to_string(accountID) + << " " << to_string(amount->asset()); + return ter; + } } if (checkBalance) { diff --git a/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp b/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp index d3a6c9c74cf..9418b400710 100644 --- a/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp +++ b/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp @@ -238,21 +238,36 @@ AMMWithdraw::preclaim(PreclaimContext const& ctx) << "AMM Withdraw: account is not authorized, " << amount->asset(); return ter; } - // AMM account or currency frozen - if (auto const ter = checkFrozen(ctx.view, ammAccountID, amount->asset()); - !isTesSuccess(ter)) + if (ctx.view.rules().enabled(fixCleanup3_3_0)) { - JLOG(ctx.j.debug()) << "AMM Withdraw: AMM account or currency is frozen or locked, " - << to_string(accountID); - return ter; + if (auto const ret = checkWithdrawFreeze( + ctx.view, ammAccountID, accountID, accountID, amount->asset())) + { + JLOG(ctx.j.debug()) << "AMM Withdraw: frozen, " << to_string(accountID) << " " + << to_string(amount->asset()); + return ret; + } } - // Account frozen - if (auto const ter = checkIndividualFrozen(ctx.view, accountID, amount->asset()); - !isTesSuccess(ter)) + else { - JLOG(ctx.j.debug()) << "AMM Withdraw: account is frozen or locked, " - << to_string(accountID) << " " << to_string(amount->asset()); - return ter; + // AMM account or currency frozen + if (auto const ter = checkFrozen(ctx.view, ammAccountID, amount->asset()); + !isTesSuccess(ter)) + { + JLOG(ctx.j.debug()) + << "AMM Withdraw: AMM account or currency is frozen or locked, " + << to_string(accountID); + return ter; + } + // Account frozen + if (auto const ter = checkIndividualFrozen(ctx.view, accountID, amount->asset()); + !isTesSuccess(ter)) + { + JLOG(ctx.j.debug()) + << "AMM Withdraw: account is frozen or locked, " << to_string(accountID) + << " " << to_string(amount->asset()); + return ter; + } } } return tesSUCCESS; @@ -302,6 +317,24 @@ AMMWithdraw::preclaim(PreclaimContext const& ctx) return tesSUCCESS; } +FreezeHandling +AMMWithdraw::issuerFreezeHandling() const +{ + // When the withdrawer is the issuer of a pool asset, the issuer can + // always receive their own token — even when the pool is frozen. + // Use IgnoreFreeze so ammHolds returns real balances instead of zero. + if (ctx_.view().rules().enabled(fixCleanup3_3_0)) + { + auto const asset1 = Asset{ctx_.tx[sfAsset]}; + auto const asset2 = Asset{ctx_.tx[sfAsset2]}; + if (!asset1.native() && accountID_ == asset1.getIssuer()) + return FreezeHandling::IgnoreFreeze; + if (!asset2.native() && accountID_ == asset2.getIssuer()) + return FreezeHandling::IgnoreFreeze; + } + return FreezeHandling::ZeroIfFrozen; +} + std::pair AMMWithdraw::applyGuts(Sandbox& sb) { @@ -329,12 +362,14 @@ AMMWithdraw::applyGuts(Sandbox& sb) auto const tfee = getTradingFee(ctx_.view(), *ammSle, accountID_); + auto const freezeHandling = issuerFreezeHandling(); + auto const expected = ammHolds( sb, *ammSle, amount ? amount->asset() : std::optional{}, amount2 ? amount2->asset() : std::optional{}, - FreezeHandling::ZeroIfFrozen, + freezeHandling, AuthHandling::ZeroIfUnauthorized, ctx_.journal); if (!expected) @@ -459,7 +494,7 @@ AMMWithdraw::withdraw( lpTokensAMMBalance, lpTokensWithdraw, tfee, - FreezeHandling::ZeroIfFrozen, + issuerFreezeHandling(), AuthHandling::ZeroIfUnauthorized, isWithdrawAll(ctx_.tx), preFeeBalance_, @@ -746,7 +781,7 @@ AMMWithdraw::equalWithdrawTokens( lpTokens, lpTokensWithdraw, tfee, - FreezeHandling::ZeroIfFrozen, + issuerFreezeHandling(), AuthHandling::ZeroIfUnauthorized, isWithdrawAll(ctx_.tx), preFeeBalance_, diff --git a/src/libxrpl/tx/transactors/lending/LoanBrokerCoverDeposit.cpp b/src/libxrpl/tx/transactors/lending/LoanBrokerCoverDeposit.cpp index 537996ba57f..9f9d524929a 100644 --- a/src/libxrpl/tx/transactors/lending/LoanBrokerCoverDeposit.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanBrokerCoverDeposit.cpp @@ -43,6 +43,8 @@ LoanBrokerCoverDeposit::preflight(PreflightContext const& ctx) TER LoanBrokerCoverDeposit::preclaim(PreclaimContext const& ctx) { + auto const fix320Enabled = ctx.view.rules().enabled(fixCleanup3_2_0); + auto const fix330Enabled = ctx.view.rules().enabled(fixCleanup3_3_0); auto const& tx = ctx.tx; auto const account = tx[sfAccount]; @@ -77,12 +79,21 @@ LoanBrokerCoverDeposit::preclaim(PreclaimContext const& ctx) // Cannot transfer a non-transferable Asset if (auto const ret = canTransfer(ctx.view, vaultAsset, account, pseudoAccountID)) return ret; - // Cannot transfer a frozen Asset - if (auto const ret = checkFrozen(ctx.view, account, vaultAsset)) - return ret; - // Pseudo-account cannot receive if asset is deep frozen - if (auto const ret = checkDeepFrozen(ctx.view, pseudoAccountID, vaultAsset)) - return ret; + + if (fix330Enabled) + { + if (auto const ret = checkDepositFreeze(ctx.view, account, pseudoAccountID, vaultAsset)) + return ret; + } + else + { + if (auto const ret = checkFrozen(ctx.view, account, vaultAsset)) + return ret; + + if (auto const ret = checkDeepFrozen(ctx.view, pseudoAccountID, vaultAsset)) + return ret; + } + // Cannot transfer unauthorized asset if (auto const ret = requireAuth(ctx.view, vaultAsset, account, AuthType::StrongAuth)) return ret; @@ -92,7 +103,6 @@ LoanBrokerCoverDeposit::preclaim(PreclaimContext const& ctx) // `sfCoverAvailable +=` could credit the broker more than the depositor paid Computing it // here in preclaim lets us reject sub-cover-scale dust early with tecPRECISION_LOSS instead of // failing only in doApply. - bool const fix320Enabled = ctx.view.rules().enabled(fixCleanup3_2_0); auto const roundedAmount = [&]() -> STAmount { if (!fix320Enabled) return tx[sfAmount]; diff --git a/src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp b/src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp index fea6f3b9cb7..ef40abd1db2 100644 --- a/src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp @@ -55,6 +55,8 @@ LoanBrokerCoverWithdraw::preflight(PreflightContext const& ctx) TER LoanBrokerCoverWithdraw::preclaim(PreclaimContext const& ctx) { + auto const fix320Enabled = ctx.view.rules().enabled(fixCleanup3_2_0); + auto const fix330Enabled = ctx.view.rules().enabled(fixCleanup3_3_0); auto const& tx = ctx.tx; auto const account = tx[sfAccount]; @@ -103,8 +105,7 @@ LoanBrokerCoverWithdraw::preclaim(PreclaimContext const& ctx) // the lsfMPTCanTransfer flag check, so an issuer cannot trap a broker's // first-loss capital. Other transferability checks (IOU NoRipple, freeze, // requireAuth) still apply. - auto const waive = ctx.view.rules().enabled(fixCleanup3_2_0) ? WaiveMPTCanTransfer::Yes - : WaiveMPTCanTransfer::No; + auto const waive = fix320Enabled ? WaiveMPTCanTransfer::Yes : WaiveMPTCanTransfer::No; if (auto const ret = canTransfer(ctx.view, vaultAsset, pseudoAccountID, dstAcct, waive)) return ret; @@ -125,22 +126,30 @@ LoanBrokerCoverWithdraw::preclaim(PreclaimContext const& ctx) if (auto const ter = requireAuth(ctx.view, vaultAsset, dstAcct, authType)) return ter; - // Check for freezes, unless sending directly to the issuer - if (dstAcct != vaultAsset.getIssuer()) + if (fix330Enabled) { - // Cannot send a frozen Asset - if (auto const ret = checkFrozen(ctx.view, pseudoAccountID, vaultAsset)) - return ret; - // Destination account cannot receive if asset is deep frozen - if (auto const ret = checkDeepFrozen(ctx.view, dstAcct, vaultAsset)) + if (auto const ret = + checkWithdrawFreeze(ctx.view, pseudoAccountID, account, dstAcct, vaultAsset)) return ret; } + else + { // Check for freezes, unless sending directly to the issuer + if (dstAcct != vaultAsset.getIssuer()) + { + // Cannot send a frozen Asset + if (auto const ret = checkFrozen(ctx.view, pseudoAccountID, vaultAsset)) + return ret; + // Destination account cannot receive if asset is deep frozen + if (auto const ret = checkDeepFrozen(ctx.view, dstAcct, vaultAsset)) + return ret; + } + } auto const coverAvail = sleBroker->at(sfCoverAvailable); // Cover Rate is in 1/10 bips units auto const currentDebtTotal = sleBroker->at(sfDebtTotal); auto const minimumCover = [&]() { - if (ctx.view.rules().enabled(fixCleanup3_2_0)) + if (fix320Enabled) { return minimumBrokerCover( currentDebtTotal, TenthBips32{sleBroker->at(sfCoverRateMinimum)}, vault); @@ -159,11 +168,15 @@ LoanBrokerCoverWithdraw::preclaim(PreclaimContext const& ctx) if ((coverAvail - amount) < minimumCover) return tecINSUFFICIENT_FUNDS; + auto const freezeHandling = fix330Enabled && dstAcct == vaultAsset.getIssuer() + ? FreezeHandling::IgnoreFreeze + : FreezeHandling::ZeroIfFrozen; + if (accountHolds( ctx.view, pseudoAccountID, vaultAsset, - FreezeHandling::ZeroIfFrozen, + freezeHandling, AuthHandling::ZeroIfUnauthorized, ctx.j) < amount) return tecINSUFFICIENT_FUNDS; diff --git a/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp b/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp index c08d1e957ca..6179a0d55d7 100644 --- a/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp @@ -63,6 +63,9 @@ VaultDeposit::preflight(PreflightContext const& ctx) TER VaultDeposit::preclaim(PreclaimContext const& ctx) { + auto const fix320Enabled = ctx.view.rules().enabled(fixCleanup3_2_0); + auto const fix330Enabled = ctx.view.rules().enabled(fixCleanup3_3_0); + auto const vault = ctx.view.read(keylet::vault(ctx.tx[sfVaultID])); if (!vault) return tecNO_ENTRY; @@ -107,13 +110,21 @@ VaultDeposit::preclaim(PreclaimContext const& ctx) // LCOV_EXCL_STOP } - // Cannot deposit inside Vault an Asset frozen for the depositor - if (isFrozen(ctx.view, account, vaultAsset)) - return vaultAsset.holds() ? tecFROZEN : tecLOCKED; + if (fix330Enabled) + { + if (auto const ret = checkDepositFreeze(ctx.view, account, vaultAccount, vaultAsset)) + return ret; + } + else + { + // Cannot deposit inside Vault an Asset frozen for the depositor + if (isFrozen(ctx.view, account, vaultAsset)) + return vaultAsset.holds() ? tecFROZEN : tecLOCKED; - // Cannot deposit if the shares of the vault are frozen - if (isFrozen(ctx.view, account, vaultShare)) - return tecLOCKED; + // Cannot deposit if the shares of the vault are frozen + if (isFrozen(ctx.view, account, vaultShare)) + return tecLOCKED; + } if (vault->isFlag(lsfVaultPrivate) && account != vault->at(sfOwner)) { @@ -141,7 +152,6 @@ VaultDeposit::preclaim(PreclaimContext const& ctx) if (auto const ter = requireAuth(ctx.view, vaultAsset, account); !isTesSuccess(ter)) return ter; - bool const fix320Enabled = ctx.view.rules().enabled(fixCleanup3_2_0); auto const roundedAmount = fix320Enabled ? roundToVaultScale(amount, vault) : amount; if (fix320Enabled && roundedAmount == beast::kZero) diff --git a/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp b/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp index 05dcfea506b..438706ffeae 100644 --- a/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp @@ -65,6 +65,10 @@ VaultWithdraw::preflight(PreflightContext const& ctx) TER VaultWithdraw::preclaim(PreclaimContext const& ctx) { + auto const fix313Enabled = ctx.view.rules().enabled(fixCleanup3_1_3); + auto const fix320Enabled = ctx.view.rules().enabled(fixCleanup3_2_0); + auto const fix330Enabled = ctx.view.rules().enabled(fixCleanup3_3_0); + auto const vault = ctx.view.read(keylet::vault(ctx.tx[sfVaultID])); if (!vault) return tecNO_ENTRY; @@ -82,8 +86,7 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx) // lsfMPTCanTransfer flag check, so an issuer cannot trap depositor funds. // Other transferability checks (IOU NoRipple, freeze, requireAuth) still // apply. - auto const waive = ctx.view.rules().enabled(fixCleanup3_2_0) ? WaiveMPTCanTransfer::Yes - : WaiveMPTCanTransfer::No; + auto const waive = fix320Enabled ? WaiveMPTCanTransfer::Yes : WaiveMPTCanTransfer::No; if (auto ter = canTransfer(ctx.view, vaultAsset, vaultAccount, dstAcct, waive); !isTesSuccess(ter)) { @@ -100,7 +103,7 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx) // LCOV_EXCL_STOP } - if (ctx.view.rules().enabled(fixCleanup3_1_3) && amount.asset() == vaultShare) + if (fix313Enabled && amount.asset() == vaultShare) { // Post-fixCleanup3_1_3: if the user specified shares, convert // to the equivalent asset amount before checking withdrawal @@ -160,15 +163,30 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx) if (auto const ter = requireAuth(ctx.view, vaultAsset, dstAcct, authType); !isTesSuccess(ter)) return ter; - // Cannot withdraw from a Vault an Asset frozen for the destination account - if (auto const ret = checkFrozen(ctx.view, dstAcct, vaultAsset)) - return ret; - - // Cannot return shares to the vault, if the underlying asset was frozen for - // the submitter - if (auto const ret = checkFrozen(ctx.view, account, Asset{vaultShare})) - return ret; + if (fix330Enabled) + { + // checkWithdrawFreeze checks the underlying asset on the source + // (vault pseudo-account), the submitter, and the destination. + // A separate share-level freeze check is unnecessary: vault shares + // are issued by the vault pseudo-account, which cannot submit + // MPTokenIssuanceSet to individually lock a holder's MPToken. + // The only way shares become locked is transitively via the + // underlying asset, which checkWithdrawFreeze covers. + if (auto const ret = + checkWithdrawFreeze(ctx.view, vaultAccount, account, dstAcct, vaultAsset)) + return ret; + } + else + { + // Cannot withdraw from a Vault an Asset frozen for the destination account + if (auto const ret = checkFrozen(ctx.view, dstAcct, vaultAsset)) + return ret; + // Cannot return shares to the vault, if the underlying asset was frozen for + // the submitter + if (auto const ret = checkFrozen(ctx.view, account, Asset{vaultShare})) + return ret; + } return tesSUCCESS; } @@ -254,8 +272,11 @@ VaultWithdraw::doApply() return tecPATH_DRY; } - if (accountHolds( - view(), accountID_, share, FreezeHandling::ZeroIfFrozen, AuthHandling::IgnoreAuth, j_) < + // Post-fixCleanup3_3_0: preclaim already handles freeze checks + auto const freezeHandling = view().rules().enabled(fixCleanup3_3_0) + ? FreezeHandling::IgnoreFreeze + : FreezeHandling::ZeroIfFrozen; + if (accountHolds(view(), accountID_, share, freezeHandling, AuthHandling::IgnoreAuth, j_) < sharesRedeemed) { JLOG(j_.debug()) << "VaultWithdraw: account doesn't hold enough shares"; @@ -358,10 +379,9 @@ VaultWithdraw::doApply() // else quietly ignore, account balance is not zero } - auto const dstAcct = ctx_.tx[~sfDestination].value_or(accountID_); - associateAsset(*vault, vaultAsset); + auto const dstAcct = ctx_.tx[~sfDestination].value_or(accountID_); return doWithdraw( view(), ctx_.tx, accountID_, dstAcct, vaultAccount, preFeeBalance_, assetsWithdrawn, j_); } diff --git a/src/test/app/AMMMPT_test.cpp b/src/test/app/AMMMPT_test.cpp index 5b576b41e4f..9d4c8851b28 100644 --- a/src/test/app/AMMMPT_test.cpp +++ b/src/test/app/AMMMPT_test.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -44,8 +45,10 @@ #include #include +#include #include #include +#include #include #include #include @@ -62,6 +65,21 @@ namespace xrpl::test { struct AMMMPT_test : public jtx::AMMTest { private: + // All 2^N permutations of testableAmendments() with each subset of the + // given features excluded. + static std::vector + amendmentCombinations(std::initializer_list features) + { + std::vector result{jtx::testableAmendments()}; + for (auto const& f : features) + { + auto const n = result.size(); + for (std::size_t i = 0; i < n; ++i) + result.push_back(result[i] - f); + } + return result; + } + void testInstanceCreate() { @@ -726,13 +744,14 @@ struct AMMMPT_test : public jtx::AMMTest ammAlice.deposit(carol_, 1'000, std::nullopt, std::nullopt, Ter(tecLOCKED)); - if (!features[featureAMMClawback]) + // Post-fixCleanup3_3_0 a locked holder cannot deposit the other + // (non-locked) token either, matching featureAMMClawback. + if (!features[featureAMMClawback] && !features[fixCleanup3_3_0]) { 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)); } @@ -753,8 +772,15 @@ struct AMMMPT_test : public jtx::AMMTest gw_, STAmount{Issue{gw_["USD"].currency, ammAlice.ammAccount()}, 0}, tfSetFreeze)); env.close(); - // Can deposit non-frozen token - ammAlice.deposit(carol_, btc(100), std::nullopt, std::nullopt, std::nullopt); + // Post-fixCleanup3_3_0 the deposit checks both pool assets, so the + // non-frozen token cannot be deposited while the AMM's USD is frozen. + ammAlice.deposit( + carol_, + btc(100), + std::nullopt, + std::nullopt, + std::nullopt, + features[fixCleanup3_3_0] ? Ter(tecFROZEN) : Ter(tesSUCCESS)); // Cannot deposit frozen token ammAlice.deposit(carol_, 1'000'000, std::nullopt, std::nullopt, Ter(tecFROZEN)); @@ -773,8 +799,15 @@ struct AMMMPT_test : public jtx::AMMTest // Individually lock AMM btc.set({.holder = ammAlice.ammAccount(), .flags = tfMPTLock}); - // Can deposit non-frozen token - ammAlice.deposit(carol_, USD(100), std::nullopt, std::nullopt, std::nullopt); + // Post-fixCleanup3_3_0 the non-locked token cannot be deposited + // while the AMM's BTC is locked. + ammAlice.deposit( + carol_, + USD(100), + std::nullopt, + std::nullopt, + std::nullopt, + features[fixCleanup3_3_0] ? Ter(tecLOCKED) : Ter(tesSUCCESS)); // Can not deposit locked token ammAlice.deposit(carol_, 1'000, std::nullopt, std::nullopt, Ter(tecLOCKED)); @@ -788,7 +821,9 @@ struct AMMMPT_test : public jtx::AMMTest ammAlice.deposit(carol_, btc(100), std::nullopt, std::nullopt, std::nullopt); } - // Individually lock MPT (AMM) account with MPT/MPT AMM + // Individually lock MPT (AMM) account with MPT/MPT AMM. + // This block always runs with all amendments (incl. fixCleanup3_3_0), + // so the deposit checks both pool assets unconditionally. { Env env{*this}; env.fund(XRP(10'000), gw_, alice_, carol_); @@ -826,8 +861,10 @@ struct AMMMPT_test : public jtx::AMMTest // Individually lock MPT BTC (AMM) account btc.set({.holder = ammAlice.ammAccount(), .flags = tfMPTLock}); - // Can deposit non-locked token USD - ammAlice.deposit(carol_, usd(100), std::nullopt, std::nullopt, std::nullopt); + // Post-fixCleanup3_3_0 the non-locked token USD cannot be deposited + // while the AMM's BTC is locked. + ammAlice.deposit( + carol_, usd(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecLOCKED)); // Can not deposit locked token BTC ammAlice.deposit(carol_, 1'000, std::nullopt, std::nullopt, Ter(tecLOCKED)); @@ -843,8 +880,10 @@ struct AMMMPT_test : public jtx::AMMTest // Individually Lock MPT USD (AMM) account usd.set({.holder = ammAlice.ammAccount(), .flags = tfMPTLock}); - // Can deposit non-locked token BTC - ammAlice.deposit(carol_, btc(100), std::nullopt, std::nullopt, std::nullopt); + // Post-fixCleanup3_3_0 the non-locked token BTC cannot be deposited + // while the AMM's USD is locked. + ammAlice.deposit( + carol_, btc(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecLOCKED)); // Can not deposit locked token USD ammAlice.deposit(carol_, 1'000, std::nullopt, std::nullopt, Ter(tecLOCKED)); @@ -876,7 +915,9 @@ struct AMMMPT_test : public jtx::AMMTest AMM amm(env, alice, XRP(10'000), btc(10'000)); env.close(); - if (!features[featureAMMClawback]) + // Post-fixCleanup3_3_0 the deposit requires authorization for both + // pool assets, so the unauthorized MPT blocks the XRP deposit too. + if (!features[featureAMMClawback] && !features[fixCleanup3_3_0]) { amm.deposit(carol, XRP(10), std::nullopt, std::nullopt, std::nullopt); } @@ -6863,6 +6904,112 @@ struct AMMMPT_test : public jtx::AMMTest BEAST_EXPECT(!amm.ammExists()); } + void + testAMMWithVaultShares() + { + testcase("AMM with vault shares — underlying freeze blocks share withdrawal"); + using namespace jtx; + FeatureBitset const all{testableAmendments()}; + + // Withdrawing vault shares from an AMM is blocked when the vault's + // underlying asset is frozen for the withdrawer (post-fixCleanup3_3_0). + // Pre-fix, the old checkFrozen path only checks the AMM account's lock + // state on the share MPT, which is unset, so withdrawal succeeds. + + auto runIOU = [&](FeatureBitset const& features) { + bool const fix330 = features[fixCleanup3_3_0]; + Env env{*this, features}; + + env.fund(XRP(100'000), gw_, alice_); + env.close(); + + PrettyAsset const iou = gw_["IOU"]; + env.trust(iou(1'000'000), alice_); + env(pay(gw_, alice_, iou(10'000))); + env.close(); + + Vault vault{env}; + auto [createTx, vaultKeylet] = vault.create({.owner = alice_, .asset = iou}); + env(createTx); + env.close(); + + // 100 IOU → 100,000,000 vault shares (IOU vault scale = 6) + env(vault.deposit({.depositor = alice_, .id = vaultKeylet.key, .amount = iou(100)})); + env.close(); + + auto const shareMPTID = env.le(vaultKeylet)->at(sfShareMPTID); + STAmount const shareAmt{MPTIssue{shareMPTID}, 100'000'000}; + AMM amm{env, alice_, XRP(100), shareAmt}; + env.close(); + + // Freeze alice's IOU trustline (individual freeze on underlying) + env(trust(gw_, iou(0), alice_, tfSetFreeze)); + env.close(); + + // post-fix330: isVaultPseudoAccountFrozen(alice, share) detects + // alice's frozen underlying → tecLOCKED + // pre-fix330: checkFrozen(ammAccount, share) → no lock → tesSUCCESS + amm.withdraw( + {.account = alice_, + .tokens = 1'000, + .err = Ter(fix330 ? TER(tecLOCKED) : TER(tesSUCCESS))}); + + env(trust(gw_, iou(0), alice_, tfClearFreeze)); + env.close(); + + amm.withdraw({.account = alice_, .tokens = 1'000}); + }; + + runIOU(all); + runIOU(all - fixCleanup3_3_0); + + auto runMPT = [&](FeatureBitset const& features) { + bool const fix330 = features[fixCleanup3_3_0]; + Env env{*this, features}; + + env.fund(XRP(100'000), gw_, alice_); + env.close(); + + MPTTester mptt{env, gw_, kMptInitNoFund}; + mptt.create({.flags = tfMPTCanTransfer | tfMPTCanLock}); + PrettyAsset const mpt = mptt.issuanceID(); + mptt.authorize({.account = alice_}); + env(pay(gw_, alice_, mpt(1'000))); + env.close(); + + Vault vault{env}; + auto [createTx, vaultKeylet] = vault.create({.owner = alice_, .asset = mpt}); + env(createTx); + env.close(); + + // 100 MPT → 100 vault shares (MPT vault scale = 0) + env(vault.deposit({.depositor = alice_, .id = vaultKeylet.key, .amount = mpt(100)})); + env.close(); + + auto const shareMPTID = env.le(vaultKeylet)->at(sfShareMPTID); + STAmount const shareAmt{MPTIssue{shareMPTID}, 100}; + AMM amm{env, alice_, XRP(100), shareAmt}; + env.close(); + + // Lock alice's underlying MPT + mptt.set({.holder = alice_, .flags = tfMPTLock}); + + // post-fix330: isVaultPseudoAccountFrozen finds alice locked → tecLOCKED + // pre-fix330: checkFrozen(ammAccount, share) → tesSUCCESS + amm.withdraw( + {.account = alice_, + .tokens = 10, + .err = Ter(fix330 ? TER(tecLOCKED) : TER(tesSUCCESS))}); + + mptt.set({.holder = alice_, .flags = tfMPTUnlock}); + + amm.withdraw({.account = alice_, .tokens = 10}); + }; + + runMPT(all); + runMPT(all - fixCleanup3_3_0); + } + void testAMMDepositWithFrozenAssets() { @@ -7085,8 +7232,8 @@ struct AMMMPT_test : public jtx::AMMTest FeatureBitset const all{jtx::testableAmendments()}; testInstanceCreate(); testInvalidInstance(); - testInvalidDeposit(all); - testInvalidDeposit(all - featureAMMClawback); + for (auto const& f : amendmentCombinations({fixCleanup3_3_0, featureAMMClawback})) + testInvalidDeposit(f); testDeposit(); testInvalidWithdraw(); testWithdraw(); @@ -7113,6 +7260,7 @@ struct AMMMPT_test : public jtx::AMMTest testLPTokenBalance(all); testLPTokenBalance(all - fixAMMv1_3); testAMMDepositWithFrozenAssets(); + testAMMWithVaultShares(); testAutoDelete(); } }; diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index 1b54c2aab95..0863e7cb837 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -57,6 +57,7 @@ #include #include #include +#include #include #include #include @@ -85,6 +86,21 @@ struct AMM_test : public jtx::AMMTest return jtx::testableAmendments() - featureSingleAssetVault - featureLendingProtocol; } + // All 2^N permutations of testableAmendments() with each subset of the + // given features excluded. + static std::vector + amendmentCombinations(std::initializer_list features) + { + std::vector result{testableAmendments()}; + for (auto const& f : features) + { + auto const n = result.size(); + for (std::size_t i = 0; i < n; ++i) + result.push_back(result[i] - f); + } + return result; + } + void testInstanceCreate() { @@ -746,18 +762,19 @@ struct AMM_test : public jtx::AMMTest testAMM( [&](AMM& ammAlice, Env& env) { env(fset(gw_, asfGlobalFreeze)); - if (!features[featureAMMClawback]) + auto const freezeBlocksAll = + features[featureAMMClawback] || features[fixCleanup3_3_0]; + if (!freezeBlocksAll) { // If the issuer set global freeze, the holder still can - // deposit the other non-frozen token when AMMClawback is - // not enabled. + // deposit the other non-frozen token when neither + // AMMClawback nor fixCleanup3_3_0 is enabled. ammAlice.deposit(carol_, XRP(100)); } else { // If the issuer set global freeze, the holder cannot - // deposit the other non-frozen token when AMMClawback is - // enabled. + // deposit the other non-frozen token. ammAlice.deposit( carol_, XRP(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecFROZEN)); } @@ -786,16 +803,18 @@ struct AMM_test : public jtx::AMMTest [&](AMM& ammAlice, Env& env) { env(trust(gw_, carol_["USD"](0), tfSetFreeze)); env.close(); - if (!features[featureAMMClawback]) + auto const freezeBlocksAll = + features[featureAMMClawback] || features[fixCleanup3_3_0]; + if (!freezeBlocksAll) { - // Can deposit non-frozen token if AMMClawback is not - // enabled + // Can deposit non-frozen token if neither AMMClawback + // nor fixCleanup3_3_0 is enabled ammAlice.deposit(carol_, XRP(100)); } else { // Cannot deposit non-frozen token if the other token is - // frozen when AMMClawback is enabled + // frozen ammAlice.deposit( carol_, XRP(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecFROZEN)); } @@ -810,8 +829,18 @@ struct AMM_test : public jtx::AMMTest STAmount{Issue{gw_["USD"].currency, ammAlice.ammAccount()}, 0}, tfSetFreeze)); env.close(); - // Can deposit non-frozen token - ammAlice.deposit(carol_, XRP(100)); + // Post-fixCleanup3_3_0: checkDepositFreeze checks both pool + // assets against the AMM account, so depositing the + // non-frozen token is also blocked. + if (!features[fixCleanup3_3_0]) + { + ammAlice.deposit(carol_, XRP(100)); + } + else + { + ammAlice.deposit( + carol_, XRP(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecFROZEN)); + } ammAlice.deposit(carol_, 1'000'000, std::nullopt, std::nullopt, Ter(tecFROZEN)); ammAlice.deposit( carol_, USD(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecFROZEN)); @@ -861,11 +890,10 @@ struct AMM_test : public jtx::AMMTest AMM amm(env, alice_, XRP(10), gw_["USD"](10), Ter(tesSUCCESS)); env.close(); - if (features[featureAMMClawback]) + if (features[featureAMMClawback] || features[fixCleanup3_3_0]) { - // if featureAMMClawback is enabled, bob_ can not deposit XRP - // because he's not authorized to hold the paired token - // gw_["USD"]. + // bob_ can not deposit XRP because he's not authorized to + // hold the paired token gw_["USD"]. amm.deposit( bob_, XRP(10), std::nullopt, std::nullopt, std::nullopt, Ter(tecNO_AUTH)); } @@ -1734,36 +1762,57 @@ struct AMM_test : public jtx::AMMTest }); // Globally frozen asset - testAMM([&](AMM& ammAlice, Env& env) { - ammAlice.deposit({.account = gw_, .asset1In = USD(1'000), .asset2In = XRP(1'000)}); - env(fset(gw_, asfGlobalFreeze)); - env.close(); - // Can withdraw non-frozen token - for (auto const& account : {alice_, gw_}) - { - ammAlice.withdraw(account, XRP(100)); - ammAlice.withdraw(account, USD(100), std::nullopt, std::nullopt, Ter(tecFROZEN)); - ammAlice.withdraw(account, 1'000, std::nullopt, std::nullopt, Ter(tecFROZEN)); - } - }); + testAMM( + [&](AMM& ammAlice, Env& env) { + auto const fix330 = env.current()->rules().enabled(fixCleanup3_3_0); + ammAlice.deposit({.account = gw_, .asset1In = USD(1'000), .asset2In = XRP(1'000)}); + env(fset(gw_, asfGlobalFreeze)); + env.close(); + // Can withdraw non-frozen token + for (auto const& account : {alice_, gw_}) + { + ammAlice.withdraw(account, XRP(100)); + // Post-fixCleanup3_3_0 the issuer can withdraw their own + // frozen token from the pool. + auto const frozenErr = + (fix330 && account == gw_) ? Ter(tesSUCCESS) : Ter(tecFROZEN); + ammAlice.withdraw(account, USD(100), std::nullopt, std::nullopt, frozenErr); + ammAlice.withdraw(account, 1'000, std::nullopt, std::nullopt, frozenErr); + } + }, + std::nullopt, + 0, + std::nullopt, + amendmentCombinations({fixCleanup3_3_0})); // Individually frozen (AMM) account - testAMM([&](AMM& ammAlice, Env& env) { - env(trust(gw_, alice_["USD"](0), tfSetFreeze)); - env.close(); - // Can withdraw non-frozen token - ammAlice.withdraw(alice_, XRP(100)); - ammAlice.withdraw(alice_, 1'000, std::nullopt, std::nullopt, Ter(tecFROZEN)); - ammAlice.withdraw(alice_, USD(100), std::nullopt, std::nullopt, Ter(tecFROZEN)); - env(trust(gw_, alice_["USD"](0), tfClearFreeze)); - // Individually frozen AMM - env(trust( - gw_, STAmount{Issue{gw_["USD"].currency, ammAlice.ammAccount()}, 0}, tfSetFreeze)); - // Can withdraw non-frozen token - ammAlice.withdraw(alice_, XRP(100)); - ammAlice.withdraw(alice_, 1'000, std::nullopt, std::nullopt, Ter(tecFROZEN)); - ammAlice.withdraw(alice_, USD(100), std::nullopt, std::nullopt, Ter(tecFROZEN)); - }); + testAMM( + [&](AMM& ammAlice, Env& env) { + auto const fix330 = env.current()->rules().enabled(fixCleanup3_3_0); + env(trust(gw_, alice_["USD"](0), tfSetFreeze)); + env.close(); + // Can withdraw non-frozen token + ammAlice.withdraw(alice_, XRP(100)); + // Post-fixCleanup3_3_0 regular freeze no longer blocks + // self-withdrawal; only deep freeze does. + auto const indivFreezeErr = fix330 ? Ter(tesSUCCESS) : Ter(tecFROZEN); + ammAlice.withdraw(alice_, 1'000, std::nullopt, std::nullopt, indivFreezeErr); + ammAlice.withdraw(alice_, USD(100), std::nullopt, std::nullopt, indivFreezeErr); + env(trust(gw_, alice_["USD"](0), tfClearFreeze)); + // Individually frozen AMM — still blocked regardless of + // fixCleanup3_3_0 because the AMM account itself is frozen. + env(trust( + gw_, + STAmount{Issue{gw_["USD"].currency, ammAlice.ammAccount()}, 0}, + tfSetFreeze)); + ammAlice.withdraw(alice_, XRP(100)); + ammAlice.withdraw(alice_, 1'000, std::nullopt, std::nullopt, Ter(tecFROZEN)); + ammAlice.withdraw(alice_, USD(100), std::nullopt, std::nullopt, Ter(tecFROZEN)); + }, + std::nullopt, + 0, + std::nullopt, + amendmentCombinations({fixCleanup3_3_0})); // Carol withdraws more than she owns testAMM([&](AMM& ammAlice, Env&) { @@ -6649,11 +6698,11 @@ struct AMM_test : public jtx::AMMTest }); } - if (features[featureAMMClawback]) + if (features[featureAMMClawback] || features[fixCleanup3_3_0]) { // Deposit one asset which is not the frozen token, - // but the other asset is frozen. We should get tecFROZEN error - // when feature AMMClawback is enabled. + // but the other asset is frozen. tecFROZEN when either + // AMMClawback or fixCleanup3_3_0 is enabled. Env env(*this, features); testAMMDeposit(env, [&](AMM& amm) { amm.deposit( @@ -6663,8 +6712,8 @@ struct AMM_test : public jtx::AMMTest else { // Deposit one asset which is not the frozen token, - // but the other asset is frozen. We will get tecSUCCESS - // when feature AMMClawback is not enabled. + // but the other asset is frozen. tesSUCCESS only when + // neither AMMClawback nor fixCleanup3_3_0 is enabled. Env env(*this, features); testAMMDeposit(env, [&](AMM& amm) { amm.deposit( @@ -7187,8 +7236,8 @@ struct AMM_test : public jtx::AMMTest FeatureBitset const all{testableAmendments()}; testInvalidInstance(); testInstanceCreate(); - testInvalidDeposit(all); - testInvalidDeposit(all - featureAMMClawback); + for (auto const& f : amendmentCombinations({fixCleanup3_3_0, featureAMMClawback})) + testInvalidDeposit(f); testDeposit(); testInvalidWithdraw(); testWithdraw(); @@ -7238,8 +7287,8 @@ struct AMM_test : public jtx::AMMTest testAMMClawback(all - featureAMMClawback - featureSingleAssetVault); testAMMClawback(all - featureAMMClawback); testAMMClawback(all - fixAMMv1_1 - fixAMMv1_3 - featureAMMClawback); - testAMMDepositWithFrozenAssets(all); - testAMMDepositWithFrozenAssets(all - featureAMMClawback); + for (auto const& f : amendmentCombinations({fixCleanup3_3_0, featureAMMClawback})) + testAMMDepositWithFrozenAssets(f); testAMMDepositWithFrozenAssets(all - fixAMMv1_1 - featureAMMClawback); testAMMDepositWithFrozenAssets(all - fixAMMv1_1 - fixAMMv1_3 - featureAMMClawback); testFixReserveCheckOnWithdrawal(all); diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp index 0edb955b902..d880bf77f52 100644 --- a/src/test/app/LoanBroker_test.cpp +++ b/src/test/app/LoanBroker_test.cpp @@ -936,11 +936,7 @@ class LoanBroker_test : public beast::unit_test::Suite env.close(); env(coverDeposit(alice, brokerKeylet.key, vaultInfo.asset(10)), Ter(tecINSUFFICIENT_FUNDS)); - - // preclaim: tecFROZEN - env(fset(issuer, asfGlobalFreeze)); - env.close(); - env(coverDeposit(alice, brokerKeylet.key, vaultInfo.asset(10)), Ter(tecFROZEN)); + // Freeze/lock tests are in testCoverDepositFreezes/testCoverWithdrawFreezes } else { @@ -966,35 +962,20 @@ class LoanBroker_test : public beast::unit_test::Suite // preclaim: tecDST_TAG_NEEDED Account const dest{"dest"}; env.fund(XRP(1'000), dest); + env(fset(dest, asfRequireDest)); - env.close(); env(coverWithdraw(alice, brokerKeylet.key, asset(10)), kDestination(dest), Ter(tecDST_TAG_NEEDED)); + env(fclear(dest, asfRequireDest)); // preclaim: tecNO_PERMISSION - env(fclear(dest, asfRequireDest)); env(fset(dest, asfDepositAuth)); - env.close(); env(coverWithdraw(alice, brokerKeylet.key, asset(10)), kDestination(dest), Ter(tecNO_PERMISSION)); - - // preclaim: tecFROZEN - env(trust(dest, asset(1'000))); env(fclear(dest, asfDepositAuth)); - env(fset(issuer, asfGlobalFreeze)); - env.close(); - env(coverWithdraw(alice, brokerKeylet.key, asset(10)), - kDestination(dest), - Ter(tecFROZEN)); - - // preclaim:: tecFROZEN (deep frozen) - env(fclear(issuer, asfGlobalFreeze)); - env(trust(issuer, asset(1'000), dest, tfSetFreeze | tfSetDeepFreeze)); - env(coverWithdraw(alice, brokerKeylet.key, asset(10)), - kDestination(dest), - Ter(tecFROZEN)); + // Freeze/lock tests are in testCoverDepositFreezes/testCoverWithdrawFreezes // preclaim: tecPSEUDO_ACCOUNT env(coverWithdraw(alice, brokerKeylet.key, asset(10)), @@ -1784,6 +1765,444 @@ class LoanBroker_test : public beast::unit_test::Suite BEAST_EXPECT(aliceBalanceAfter == aliceBalanceBefore); } + void + testCoverDepositFreezes() + { + using namespace jtx; + using namespace loanBroker; + + Account const issuer{"issuer"}; + Account const alice{"alice"}; + + // === IOU === + { + testcase("LoanBrokerCoverDeposit IOU freeze checks"); + Env env(*this); + Vault const vault{env}; + + env.fund(XRP(100'000), issuer, alice); + env(trust(alice, issuer["IOU"](1'000'000))); + env.close(); + PrettyAsset const asset(issuer["IOU"]); + env(pay(issuer, alice, asset(100'000))); + env.close(); + + auto [tx, vaultKeylet] = vault.create({.owner = alice, .asset = asset}); + env(tx); + env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = asset(50)})); + env.close(); + + auto const brokerKeylet = keylet::loanbroker(alice.id(), env.seq(alice)); + env(set(alice, vaultKeylet.key)); + env.close(); + + auto const broker = env.le(brokerKeylet); + if (!BEAST_EXPECT(broker)) + return; + Account const brokerPseudo("pseudo", broker->at(sfAccount)); + + env(coverDeposit(alice, brokerKeylet.key, asset(10))); + env.close(); + + auto runTests = [&]() { + auto const fix330Enabled = env.current()->rules().enabled(fixCleanup3_3_0); + + // Global freeze + env(fset(issuer, asfGlobalFreeze)); + env(coverDeposit(alice, brokerKeylet.key, asset(1)), Ter(tecFROZEN)); + env(fclear(issuer, asfGlobalFreeze)); + + // Source regular freeze + env(trust(issuer, asset(0), alice, tfSetFreeze)); + env(coverDeposit(alice, brokerKeylet.key, asset(1)), Ter(tecFROZEN)); + env(trust(issuer, asset(0), alice, tfClearFreeze)); + + // Source deep freeze + env(trust(issuer, asset(0), alice, tfSetFreeze | tfSetDeepFreeze)); + env(coverDeposit(alice, brokerKeylet.key, asset(1)), Ter(tecFROZEN)); + env(trust(issuer, asset(0), alice, tfClearFreeze | tfClearDeepFreeze)); + + // Pseudo regular freeze — post-fix blocks, pre-fix allows (BUG) + TER const pseudoTer = fix330Enabled ? TER(tecFROZEN) : TER(tesSUCCESS); + env(trust(issuer, asset(0), brokerPseudo, tfSetFreeze)); + env(coverDeposit(alice, brokerKeylet.key, asset(1)), Ter(pseudoTer)); + env(trust(issuer, asset(0), brokerPseudo, tfClearFreeze)); + + // Pseudo deep freeze + env(trust(issuer, asset(0), brokerPseudo, tfSetFreeze | tfSetDeepFreeze)); + env(coverDeposit(alice, brokerKeylet.key, asset(1)), Ter(tecFROZEN)); + env(trust(issuer, asset(0), brokerPseudo, tfClearFreeze | tfClearDeepFreeze)); + }; + + runTests(); + env.disableFeature(fixCleanup3_3_0); + runTests(); + env.enableFeature(fixCleanup3_3_0); + } + + // === MPT === + { + testcase("LoanBrokerCoverDeposit MPT lock checks"); + Env env(*this); + Vault const vault{env}; + + env.fund(XRP(100'000), issuer, alice); + env.close(); + + MPTTester mptt{env, issuer, kMptInitNoFund}; + mptt.create({.flags = tfMPTCanClawback | tfMPTCanTransfer | tfMPTCanLock}); + PrettyAsset const mpt{mptt.issuanceID()}; + + mptt.authorize({.account = alice}); + env(pay(issuer, alice, mpt(100'000))); + env.close(); + + auto [tx, vaultKeylet] = vault.create({.owner = alice, .asset = mpt}); + env(tx); + env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = mpt(50)})); + env.close(); + + auto const brokerKeylet = keylet::loanbroker(alice.id(), env.seq(alice)); + env(set(alice, vaultKeylet.key)); + env.close(); + + auto const broker = env.le(brokerKeylet); + if (!BEAST_EXPECT(broker)) + return; + Account const brokerPseudo("pseudo", broker->at(sfAccount)); + + env(coverDeposit(alice, brokerKeylet.key, mpt(10))); + env.close(); + + // For MPT isDeepFrozen == isFrozen, so all locks block in + // both pre- and post-fix. No behavioral difference. + auto runTests = [&]() { + // Global lock + mptt.set({.flags = tfMPTLock}); + env.close(); + env(coverDeposit(alice, brokerKeylet.key, mpt(1)), Ter(tecLOCKED)); + mptt.set({.flags = tfMPTUnlock}); + env.close(); + + // Source (alice) individual lock + mptt.set({.holder = alice, .flags = tfMPTLock}); + env.close(); + env(coverDeposit(alice, brokerKeylet.key, mpt(1)), Ter(tecLOCKED)); + mptt.set({.holder = alice, .flags = tfMPTUnlock}); + env.close(); + + // Pseudo individual lock + mptt.set({.holder = brokerPseudo, .flags = tfMPTLock}); + env.close(); + env(coverDeposit(alice, brokerKeylet.key, mpt(1)), Ter(tecLOCKED)); + mptt.set({.holder = brokerPseudo, .flags = tfMPTUnlock}); + env.close(); + }; + + runTests(); + env.disableFeature(fixCleanup3_3_0); + runTests(); + env.enableFeature(fixCleanup3_3_0); + } + } + + // Focused demonstration: a cover-withdraw submitter under a regular + // individual IOU freeze can still withdraw to themselves (self-withdrawal). + // + // Pre-fixCleanup3_3_0: the old code only checked the pseudo-account source + // and the destination for deep-freeze; it did not check the submitter's + // individual freeze at all. Self-withdrawal therefore always succeeded. + // Post-fixCleanup3_3_0: checkWithdrawFreeze explicitly skips the submitter + // freeze check when submitter == destination, preserving the same result. + void + testCoverWithdrawSelfWhileFrozen() + { + testcase("LoanBrokerCoverWithdraw IOU self-withdrawal while individually frozen"); + + using namespace jtx; + using namespace loanBroker; + + Account const issuer{"issuer"}; + Account const alice{"alice"}; + Account const dest{"dest"}; + Env env{*this}; + Vault const vault{env}; + + env.fund(XRP(100'000), issuer, alice, dest); + env(trust(alice, issuer["IOU"](1'000'000))); + env(trust(dest, issuer["IOU"](1'000'000))); + env.close(); + + PrettyAsset const asset(issuer["IOU"]); + env(pay(issuer, alice, asset(100'000))); + env.close(); + + auto [vaultTx, vaultKeylet] = vault.create({.owner = alice, .asset = asset}); + env(vaultTx); + env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = asset(50)})); + env.close(); + + auto const brokerKeylet = keylet::loanbroker(alice.id(), env.seq(alice)); + env(set(alice, vaultKeylet.key)); + env.close(); + + env(coverDeposit(alice, brokerKeylet.key, asset(10))); + env.close(); + + auto runTests = [&]() { + auto const fix330Enabled = env.current()->rules().enabled(fixCleanup3_3_0); + + // Set a regular individual freeze on alice's IOU trustline. + env(trust(issuer, asset(0), alice, tfSetFreeze)); + env.close(); + + // Self-withdrawal: submitter == destination (no sfDestination in tx). + // Both pre- and post-fixCleanup3_3_0 this succeeds: + // pre-fix: old code never checked the submitter's freeze. + // post-fix: checkWithdrawFreeze skips submitter when submitter==dst. + env(coverWithdraw(alice, brokerKeylet.key, asset(1)), Ter(tesSUCCESS)); + + // Withdrawal to a third party is blocked by the submitter freeze + // under fixCleanup3_3_0; pre-fix it was not checked. + env(coverWithdraw(alice, brokerKeylet.key, asset(1)), + kDestination(dest), + Ter(fix330Enabled ? TER(tecFROZEN) : TER(tesSUCCESS))); + + env(trust(issuer, asset(0), alice, tfClearFreeze)); + env.close(); + }; + + runTests(); + env.disableFeature(fixCleanup3_3_0); + runTests(); + env.enableFeature(fixCleanup3_3_0); + } + + void + testCoverWithdrawFreezes() + { + using namespace jtx; + using namespace loanBroker; + + Account const issuer{"issuer"}; + Account const alice{"alice"}; + + // === IOU === + { + testcase("LoanBrokerCoverWithdraw IOU freeze checks"); + Env env(*this); + Vault const vault{env}; + + env.fund(XRP(100'000), issuer, alice); + env(trust(alice, issuer["IOU"](1'000'000))); + env.close(); + PrettyAsset const asset(issuer["IOU"]); + env(pay(issuer, alice, asset(100'000))); + env.close(); + + auto [tx, vaultKeylet] = vault.create({.owner = alice, .asset = asset}); + env(tx); + env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = asset(50)})); + env.close(); + + auto const brokerKeylet = keylet::loanbroker(alice.id(), env.seq(alice)); + env(set(alice, vaultKeylet.key)); + env.close(); + + auto const broker = env.le(brokerKeylet); + if (!BEAST_EXPECT(broker)) + return; + Account const brokerPseudo("pseudo", broker->at(sfAccount)); + + env(coverDeposit(alice, brokerKeylet.key, asset(10))); + env.close(); + + Account const dest{"dest"}; + env.fund(XRP(1'000), dest); + env(trust(dest, asset(1'000))); + + auto runTests = [&]() { + auto const fix330Enabled = env.current()->rules().enabled(fixCleanup3_3_0); + TER const expectedTec = fix330Enabled ? TER(tecFROZEN) : TER(tesSUCCESS); + + // Global freeze + env(fset(issuer, asfGlobalFreeze)); + env(coverWithdraw(alice, brokerKeylet.key, asset(1)), + kDestination(dest), + Ter(tecFROZEN)); + env(fclear(issuer, asfGlobalFreeze)); + + // Source (pseudo) regular freeze + env(trust(issuer, asset(0), brokerPseudo, tfSetFreeze)); + env(coverWithdraw(alice, brokerKeylet.key, asset(1)), + kDestination(dest), + Ter(tecFROZEN)); + env(trust(issuer, asset(0), brokerPseudo, tfClearFreeze)); + + // Source (pseudo) deep freeze + env(trust(issuer, asset(0), brokerPseudo, tfSetFreeze | tfSetDeepFreeze)); + env(coverWithdraw(alice, brokerKeylet.key, asset(1)), + kDestination(dest), + Ter(tecFROZEN)); + env(trust(issuer, asset(0), brokerPseudo, tfClearFreeze | tfClearDeepFreeze)); + + // Submitter regular freeze → dest + env(trust(issuer, asset(0), alice, tfSetFreeze)); + env(coverWithdraw(alice, brokerKeylet.key, asset(1)), + kDestination(dest), + Ter(expectedTec)); + // Submitter regular freeze → self: always allowed + env(coverWithdraw(alice, brokerKeylet.key, asset(1)), Ter(tesSUCCESS)); + env(trust(issuer, asset(0), alice, tfClearFreeze)); + env(coverDeposit( + alice, brokerKeylet.key, asset(isTesSuccess(expectedTec) ? 2 : 1))); + + // Submitter deep freeze → dest + env(trust(issuer, asset(0), alice, tfSetFreeze | tfSetDeepFreeze)); + env(coverWithdraw(alice, brokerKeylet.key, asset(1)), + kDestination(dest), + Ter(expectedTec)); + // Submitter deep freeze → self: blocked (checkDeepFrozen) + env(coverWithdraw(alice, brokerKeylet.key, asset(1)), Ter(tecFROZEN)); + env(trust(issuer, asset(0), alice, tfClearFreeze | tfClearDeepFreeze)); + if (isTesSuccess(expectedTec)) + env(coverDeposit(alice, brokerKeylet.key, asset(1))); + + // Destination regular freeze: only deep freeze blocks + env(trust(issuer, asset(0), dest, tfSetFreeze)); + env(coverWithdraw(alice, brokerKeylet.key, asset(1)), + kDestination(dest), + Ter(tesSUCCESS)); + env(trust(issuer, asset(0), dest, tfClearFreeze)); + env(coverDeposit(alice, brokerKeylet.key, asset(1))); + + // Destination deep freeze + env(trust(issuer, asset(0), dest, tfSetFreeze | tfSetDeepFreeze)); + env(coverWithdraw(alice, brokerKeylet.key, asset(1)), + kDestination(dest), + Ter(tecFROZEN)); + env(trust(issuer, asset(0), dest, tfClearFreeze | tfClearDeepFreeze)); + + // Submitter frozen → issuer: bypasses all freeze checks + env(trust(issuer, asset(0), alice, tfSetFreeze)); + env(coverWithdraw(alice, brokerKeylet.key, asset(1)), + kDestination(issuer), + Ter(tesSUCCESS)); + env(trust(issuer, asset(0), alice, tfClearFreeze)); + env(coverDeposit(alice, brokerKeylet.key, asset(1))); + }; + + runTests(); + env.disableFeature(fixCleanup3_3_0); + runTests(); + env.enableFeature(fixCleanup3_3_0); + } + + // === MPT === + { + testcase("LoanBrokerCoverWithdraw MPT lock checks"); + Env env(*this); + Vault const vault{env}; + + env.fund(XRP(100'000), issuer, alice); + env.close(); + + MPTTester mptt{env, issuer, kMptInitNoFund}; + mptt.create({.flags = tfMPTCanClawback | tfMPTCanTransfer | tfMPTCanLock}); + PrettyAsset const mpt{mptt.issuanceID()}; + + mptt.authorize({.account = alice}); + env(pay(issuer, alice, mpt(100'000))); + env.close(); + + auto [tx, vaultKeylet] = vault.create({.owner = alice, .asset = mpt}); + env(tx); + env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = mpt(50)})); + env.close(); + + auto const brokerKeylet = keylet::loanbroker(alice.id(), env.seq(alice)); + env(set(alice, vaultKeylet.key)); + env.close(); + + auto const broker = env.le(brokerKeylet); + if (!BEAST_EXPECT(broker)) + return; + Account const brokerPseudo("pseudo", broker->at(sfAccount)); + + env(coverDeposit(alice, brokerKeylet.key, mpt(10))); + env.close(); + + Account const dest{"dest"}; + env.fund(XRP(1'000), dest); + mptt.authorize({.account = dest}); + env.close(); + + auto runTests = [&]() { + auto const withFix = env.current()->rules().enabled(fixCleanup3_3_0); + // Only submitter-to-dest differs: post-fix blocks, pre-fix + // doesn't (BUG). All other locks block in both because for + // MPT isDeepFrozen == isFrozen. + TER const submitterToDest = withFix ? TER(tecLOCKED) : TER(tesSUCCESS); + + // Global lock + mptt.set({.flags = tfMPTLock}); + env.close(); + env(coverWithdraw(alice, brokerKeylet.key, mpt(1)), + kDestination(dest), + Ter(tecLOCKED)); + mptt.set({.flags = tfMPTUnlock}); + env.close(); + + // Source (pseudo) individual lock + mptt.set({.holder = brokerPseudo, .flags = tfMPTLock}); + env.close(); + env(coverWithdraw(alice, brokerKeylet.key, mpt(1)), + kDestination(dest), + Ter(tecLOCKED)); + mptt.set({.holder = brokerPseudo, .flags = tfMPTUnlock}); + env.close(); + + // Submitter individual lock → dest + mptt.set({.holder = alice, .flags = tfMPTLock}); + env.close(); + env(coverWithdraw(alice, brokerKeylet.key, mpt(1)), + kDestination(dest), + Ter(submitterToDest)); + // Submitter individual lock → self: blocked + env(coverWithdraw(alice, brokerKeylet.key, mpt(1)), Ter(tecLOCKED)); + mptt.set({.holder = alice, .flags = tfMPTUnlock}); + env.close(); + if (isTesSuccess(submitterToDest)) + env(coverDeposit(alice, brokerKeylet.key, mpt(1))); + env.close(); + + // Dest individual lock: blocked + mptt.set({.holder = dest, .flags = tfMPTLock}); + env.close(); + env(coverWithdraw(alice, brokerKeylet.key, mpt(1)), + kDestination(dest), + Ter(tecLOCKED)); + mptt.set({.holder = dest, .flags = tfMPTUnlock}); + env.close(); + + // Submitter locked → issuer: bypasses all freeze checks + mptt.set({.holder = alice, .flags = tfMPTLock}); + env.close(); + env(coverWithdraw(alice, brokerKeylet.key, mpt(1)), + kDestination(issuer), + Ter(tesSUCCESS)); + mptt.set({.holder = alice, .flags = tfMPTUnlock}); + env(coverDeposit(alice, brokerKeylet.key, mpt(1))); + env.close(); + }; + + runTests(); + env.disableFeature(fixCleanup3_3_0); + runTests(); + env.enableFeature(fixCleanup3_3_0); + } + } + void testRIPD4274IOU() { @@ -2244,6 +2663,13 @@ class LoanBroker_test : public beast::unit_test::Suite void run() override { + testInvalidLoanBrokerCoverClawback(); + testInvalidLoanBrokerCoverDeposit(); + testInvalidLoanBrokerCoverWithdraw(); + testCoverDepositFreezes(); + testCoverWithdrawFreezes(); + testCoverWithdrawSelfWhileFrozen(); + testCoverPrecisionGuard(); testLoanBrokerSetDebtMaximum(); @@ -2251,9 +2677,6 @@ class LoanBroker_test : public beast::unit_test::Suite testDisabled(); testLifecycle(); - testInvalidLoanBrokerCoverClawback(); - testInvalidLoanBrokerCoverDeposit(); - testInvalidLoanBrokerCoverWithdraw(); testInvalidLoanBrokerDelete(); testInvalidLoanBrokerSet(); testRequireAuth(); @@ -2268,7 +2691,6 @@ class LoanBroker_test : public beast::unit_test::Suite testLoanBrokerDeleteFrozenIOU(all_); testLoanBrokerDeleteFrozenIOU(all_ - fixCleanup3_2_0); - // TODO: Write clawback failure tests with an issuer / MPT that doesn't // have the right flags set. } diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index 2cab3e7c893..0b2092f50b0 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -7461,21 +7461,42 @@ class MPToken_test : public beast::unit_test::Suite // MPTLock is set usd.set({.flags = tfMPTLock}); - // carol and issuer can't withdraw - for (auto const& account : {carol, gw}) - { - amm.withdraw( - {.account = account, - .asset1Out = usd(1), - .asset2Out = eur(1), - .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)}); - } + auto const fix330 = env.current()->rules().enabled(fixCleanup3_3_0); + + // carol can't withdraw the locked token (any withdrawal type) + amm.withdraw( + {.account = carol, + .asset1Out = usd(1), + .asset2Out = eur(1), + .err = Ter(tecLOCKED)}); + amm.withdraw({.account = carol, .tokens = 1'000, .err = Ter(tecLOCKED)}); + // can single withdraw the non-locked asset + amm.withdraw( + {.account = carol, .asset1Out = eur(1), .assets = std::make_pair(eur, usd)}); + + // post-fixCleanup3_3_0 the issuer can redeem even when locked. + // Each successful withdrawal burns LP tokens, so replenish between + // each type to keep gw's LP balance stable. + auto const gwLockErr = fix330 ? Ter(tesSUCCESS) : Ter(tecLOCKED); + auto const replenish = [&](IOUAmount tokens) { + usd.set({.flags = tfMPTUnlock}); + amm.deposit({.account = gw, .tokens = tokens}); + usd.set({.flags = tfMPTLock}); + }; + + amm.withdraw( + {.account = gw, .asset1Out = usd(1), .asset2Out = eur(1), .err = gwLockErr}); + if (fix330) + replenish(IOUAmount{1}); + + amm.withdraw({.account = gw, .tokens = 1'000, .err = gwLockErr}); + if (fix330) + replenish(IOUAmount{1'000}); + + // can single withdraw the non-locked asset + amm.withdraw( + {.account = gw, .asset1Out = eur(1), .assets = std::make_pair(eur, usd)}); + usd.set({.flags = tfMPTUnlock}); // MPTRequireAuth is set diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index 065b6b00441..151684dce5e 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -1573,6 +1573,7 @@ class Vault_test : public beast::unit_test::Suite bool enableClawback = true; bool requireAuth = true; int initialXRP = 1000; + FeatureBitset features = testableAmendments(); }; auto testCase = [this]( @@ -1585,7 +1586,7 @@ class Vault_test : public beast::unit_test::Suite Vault& vault, MPTTester& mptt)> test, CaseArgs args = {}) { - Env env{*this, testableAmendments()}; + Env env{*this, args.features}; Account const issuer{"issuer"}; Account const owner{"owner"}; Account const depositor{"depositor"}; @@ -1632,6 +1633,8 @@ class Vault_test : public beast::unit_test::Suite env(tx, Ter(tecNO_ENTRY)); }); + // Freeze/lock tests are in testVaultDepositFreeze/testVaultWithdrawFreeze + testCase([this]( Env& env, Account const& issuer, @@ -1646,81 +1649,6 @@ class Vault_test : public beast::unit_test::Suite env(tx, Ter(tecLOCKED)); }); - testCase([this]( - Env& env, - Account const& issuer, - Account const& owner, - Account const& depositor, - Asset const& asset, - Vault& vault, - MPTTester& mptt) { - testcase("MPT global lock blocks deposit"); - auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); - env(tx); - env.close(); - - mptt.set({.account = issuer, .flags = tfMPTLock}); - env.close(); - - tx = vault.deposit({.depositor = depositor, .id = keylet.key, .amount = asset(100)}); - env(tx, Ter{tecLOCKED}); - env.close(); - - // Can delete empty vault, even if global lock - tx = vault.del({.owner = owner, .id = keylet.key}); - env(tx); - }); - - testCase([this]( - Env& env, - Account const& issuer, - Account const& owner, - Account const& depositor, - Asset const& asset, - Vault& vault, - MPTTester& mptt) { - testcase("MPT global lock blocks withdrawal"); - auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); - env(tx); - env.close(); - tx = vault.deposit({.depositor = depositor, .id = keylet.key, .amount = asset(100)}); - env(tx); - env.close(); - - // Check that the OutstandingAmount field of MPTIssuance - // accounts for the issued shares. - auto v = env.le(keylet); - BEAST_EXPECT(v); - MPTID const share = (*v)[sfShareMPTID]; - auto issuance = env.le(keylet::mptIssuance(share)); - BEAST_EXPECT(issuance); - Number const outstandingShares = issuance->at(sfOutstandingAmount); - BEAST_EXPECT(outstandingShares == 100); - - mptt.set({.account = issuer, .flags = tfMPTLock}); - env.close(); - - tx = vault.withdraw({.depositor = depositor, .id = keylet.key, .amount = asset(100)}); - env(tx, Ter(tecLOCKED)); - - tx[sfDestination] = issuer.human(); - env(tx, Ter(tecLOCKED)); - - // Clawback is still permitted, even with global lock - tx = vault.clawback( - {.issuer = issuer, .id = keylet.key, .holder = depositor, .amount = asset(0)}); - env(tx); - env.close(); - - // Clawback removed shares MPToken - auto const mptSle = env.le(keylet::mptoken(share, depositor.id())); - BEAST_EXPECT(mptSle == nullptr); - - // Can delete empty vault, even if global lock - tx = vault.del({.owner = owner, .id = keylet.key}); - env(tx); - }); - testCase([this]( Env& env, Account const& issuer, @@ -2155,57 +2083,6 @@ class Vault_test : public beast::unit_test::Suite env(vault.del({.owner = owner, .id = keylet.key})); }); - testCase([this]( - Env& env, - Account const& issuer, - Account const& owner, - Account const& depositor, - Asset const& asset, - Vault& vault, - MPTTester& mptt) { - testcase("MPT lock of vault pseudo-account"); - auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); - env(tx); - env.close(); - - auto const vaultAccount = [&env, keylet = keylet, this]() -> AccountID { - auto const vault = env.le(keylet); - BEAST_EXPECT(vault != nullptr); - return vault->at(sfAccount); - }(); - - tx = vault.deposit({.depositor = depositor, .id = keylet.key, .amount = asset(100)}); - env(tx); - env.close(); - - tx = [&]() { - json::Value jv; - jv[jss::Account] = issuer.human(); - jv[sfMPTokenIssuanceID] = to_string(asset.get().getMptID()); - jv[jss::Holder] = toBase58(vaultAccount); - jv[jss::TransactionType] = jss::MPTokenIssuanceSet; - jv[jss::Flags] = tfMPTLock; - return jv; - }(); - env(tx); - env.close(); - - tx = vault.deposit({.depositor = depositor, .id = keylet.key, .amount = asset(100)}); - env(tx, Ter(tecLOCKED)); - - tx = vault.withdraw({.depositor = depositor, .id = keylet.key, .amount = asset(100)}); - env(tx, Ter(tecLOCKED)); - - // Clawback works, even when locked - tx = vault.clawback( - {.issuer = issuer, .id = keylet.key, .holder = depositor, .amount = asset(100)}); - env(tx); - - // Can delete an empty vault even when asset is locked. - tx = vault.del({.owner = owner, .id = keylet.key}); - env(tx); - }); - { testcase("MPT shares to a vault"); @@ -2438,6 +2315,7 @@ class Vault_test : public beast::unit_test::Suite Number initialIOU = 200; double transferRate = 1.0; bool charlieRipple = true; + FeatureBitset features = testableAmendments(); }; auto testCase = [&, this]( @@ -2451,7 +2329,7 @@ class Vault_test : public beast::unit_test::Suite PrettyAsset const& asset, std::function issuanceId)> test, CaseArgs args = {}) { - Env env{*this, testableAmendments()}; + Env env{*this, args.features}; Account const owner{"owner"}; Account const issuer{"issuer"}; Account const charlie{"charlie"}; @@ -2543,83 +2421,6 @@ class Vault_test : public beast::unit_test::Suite env.close(); }); - testCase([&, this]( - Env& env, - Account const& owner, - Account const& issuer, - Account const& charlie, - auto vaultAccount, - Vault& vault, - PrettyAsset const& asset, - auto issuanceId) { - testcase("IOU frozen trust line to vault account"); - - auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); - env(tx); - env.close(); - - env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(100)})); - env.close(); - - Asset const share = Asset(issuanceId(keylet)); - - // Freeze the trustline to the vault - auto trustSet = [&, account = vaultAccount(keylet)]() { - json::Value jv; - jv[jss::Account] = issuer.human(); - { - auto& ja = jv[jss::LimitAmount] = - asset(0).value().getJson(JsonOptions::Values::None); - ja[jss::issuer] = toBase58(account); - } - jv[jss::TransactionType] = jss::TrustSet; - jv[jss::Flags] = tfSetFreeze; - return jv; - }(); - env(trustSet); - env.close(); - - { - // Note, the "frozen" state of the trust line to vault account - // is reported as "locked" state of the vault shares, because - // this state is attached to shares by means of the transitive - // isFrozen. - auto tx = - vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(80)}); - env(tx, Ter{tecLOCKED}); - } - - { - auto tx = - vault.withdraw({.depositor = owner, .id = keylet.key, .amount = asset(100)}); - env(tx, Ter{tecLOCKED}); - - // also when trying to withdraw to a 3rd party - tx[sfDestination] = charlie.human(); - env(tx, Ter{tecLOCKED}); - env.close(); - } - - { - // Clawback works, even when locked - auto tx = vault.clawback( - {.issuer = issuer, .id = keylet.key, .holder = owner, .amount = asset(50)}); - env(tx); - env.close(); - } - - // Clear the frozen state - trustSet[jss::Flags] = tfClearFreeze; - env(trustSet); - env.close(); - - env(vault.withdraw( - {.depositor = owner, .id = keylet.key, .amount = share(50'000'000)})); - - env(vault.del({.owner = owner, .id = keylet.key})); - env.close(); - }); - testCase( [&, this]( Env& env, @@ -2681,65 +2482,6 @@ class Vault_test : public beast::unit_test::Suite }, CaseArgs{.transferRate = 1.25}); - testCase([&, this]( - Env& env, - Account const& owner, - Account const& issuer, - Account const& charlie, - auto, - Vault& vault, - PrettyAsset const& asset, - auto&&...) { - testcase("IOU frozen trust line to depositor"); - - auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); - env(tx); - env.close(); - - env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(100)})); - env.close(); - - // Withdraw to 3rd party works - auto const withdrawToCharlie = [&](xrpl::Keylet keylet) { - auto tx = - vault.withdraw({.depositor = owner, .id = keylet.key, .amount = asset(10)}); - tx[sfDestination] = charlie.human(); - return tx; - }(keylet); - env(withdrawToCharlie); - - // Freeze the owner - env(trust(issuer, asset(0), owner, tfSetFreeze)); - env.close(); - - // Cannot withdraw - auto const withdraw = - vault.withdraw({.depositor = owner, .id = keylet.key, .amount = asset(10)}); - env(withdraw, Ter{tecFROZEN}); - - // Cannot withdraw to 3rd party - env(withdrawToCharlie, Ter{tecLOCKED}); - env.close(); - - { - // Cannot deposit some more - auto tx = - vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(10)}); - env(tx, Ter{tecFROZEN}); - } - - { - // Clawback still works - auto tx = vault.clawback( - {.issuer = issuer, .id = keylet.key, .holder = owner, .amount = asset(0)}); - env(tx); - env.close(); - } - - env(vault.del({.owner = owner, .id = keylet.key})); - env.close(); - }); - testCase([&, this]( Env& env, Account const& owner, @@ -3030,103 +2772,7 @@ class Vault_test : public beast::unit_test::Suite env(tx); env.close(); }, - CaseArgs{.initialXRP = acctReserve + (incReserve * 4) + 1}); - - testCase([&, this]( - Env& env, - Account const& owner, - Account const& issuer, - Account const& charlie, - auto, - Vault& vault, - PrettyAsset const& asset, - auto&&...) { - testcase("IOU frozen trust line to 3rd party"); - - auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); - env(tx); - env.close(); - - env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(100)})); - env.close(); - - // Withdraw to 3rd party works - auto const withdrawToCharlie = [&](xrpl::Keylet keylet) { - auto tx = - vault.withdraw({.depositor = owner, .id = keylet.key, .amount = asset(10)}); - tx[sfDestination] = charlie.human(); - return tx; - }(keylet); - env(withdrawToCharlie); - - // Freeze the 3rd party - env(trust(issuer, asset(0), charlie, tfSetFreeze)); - env.close(); - - // Can withdraw - auto const withdraw = - vault.withdraw({.depositor = owner, .id = keylet.key, .amount = asset(10)}); - env(withdraw); - env.close(); - - // Cannot withdraw to 3rd party - env(withdrawToCharlie, Ter{tecFROZEN}); - env.close(); - - env(vault.clawback( - {.issuer = issuer, .id = keylet.key, .holder = owner, .amount = asset(0)})); - env.close(); - - env(vault.del({.owner = owner, .id = keylet.key})); - env.close(); - }); - - testCase([&, this]( - Env& env, - Account const& owner, - Account const& issuer, - Account const& charlie, - auto, - Vault& vault, - PrettyAsset const& asset, - auto&&...) { - testcase("IOU global freeze"); - - auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); - env(tx); - env.close(); - - env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(100)})); - env.close(); - - env(fset(issuer, asfGlobalFreeze)); - env.close(); - - { - // Cannot withdraw - auto tx = - vault.withdraw({.depositor = owner, .id = keylet.key, .amount = asset(10)}); - env(tx, Ter{tecFROZEN}); - - // Cannot withdraw to 3rd party - tx[sfDestination] = charlie.human(); - env(tx, Ter{tecFROZEN}); - env.close(); - - // Cannot deposit some more - tx = vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(10)}); - - env(tx, Ter{tecFROZEN}); - } - - // Clawback is permitted - env(vault.clawback( - {.issuer = issuer, .id = keylet.key, .holder = owner, .amount = asset(0)})); - env.close(); - - env(vault.del({.owner = owner, .id = keylet.key})); - env.close(); - }); + CaseArgs{.initialXRP = acctReserve + (incReserve * 4) + 1}); } void @@ -7824,6 +7470,579 @@ class Vault_test : public beast::unit_test::Suite } } + void + testVaultDepositFreeze() + { + using namespace test::jtx; + + Account const issuer{"issuer"}; + Account const owner{"owner"}; + + // === IOU === + { + testcase("VaultDeposit IOU freeze checks"); + Env env{*this}; + Vault vault{env}; + + env.fund(XRP(100'000), issuer, owner); + env(fset(issuer, asfAllowTrustLineClawback)); + env.close(); + PrettyAsset const asset = issuer["IOU"]; + env.trust(asset(1'000'000), owner); + env(pay(issuer, owner, asset(100'000))); + env.close(); + + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + env(tx); + env.close(); + auto const vaultAcct = Account("vault", env.le(keylet)->at(sfAccount)); + + // Initial deposit so the vault pseudo-account has a trustline + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(100)})); + env.close(); + + auto runTests = [&]() { + auto const fix330Enabled = env.current()->rules().enabled(fixCleanup3_3_0); + + // Global freeze + env(fset(issuer, asfGlobalFreeze)); + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(1)}), + Ter(tecFROZEN)); + env(fclear(issuer, asfGlobalFreeze)); + + // Depositor regular freeze + env(trust(issuer, asset(0), owner, tfSetFreeze)); + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(1)}), + Ter(tecFROZEN)); + env(trust(issuer, asset(0), owner, tfClearFreeze)); + + // Depositor deep freeze + env(trust(issuer, asset(0), owner, tfSetFreeze | tfSetDeepFreeze)); + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(1)}), + Ter(tecFROZEN)); + env(trust(issuer, asset(0), owner, tfClearFreeze | tfClearDeepFreeze)); + + // Vault-account regular freeze + // Post-fix: checkDepositFreeze catches it → tecFROZEN + // Pre-fix: not checked directly, but the transitive share + // check triggers → tecLOCKED + { + auto trustSet = [&]() { + json::Value jv; + jv[jss::Account] = issuer.human(); + { + auto& ja = jv[jss::LimitAmount] = + asset(0).value().getJson(JsonOptions::Values::None); + ja[jss::issuer] = toBase58(vaultAcct.id()); + } + jv[jss::TransactionType] = jss::TrustSet; + return jv; + }(); + + trustSet[jss::Flags] = tfSetFreeze; + env(trustSet); + env.close(); + + TER const expected = fix330Enabled ? TER(tecFROZEN) : TER(tecLOCKED); + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(1)}), + Ter(expected)); + + trustSet[jss::Flags] = tfClearFreeze; + env(trustSet); + env.close(); + } + + // Vault-account deep freeze + { + auto trustSet = [&]() { + json::Value jv; + jv[jss::Account] = issuer.human(); + { + auto& ja = jv[jss::LimitAmount] = + asset(0).value().getJson(JsonOptions::Values::None); + ja[jss::issuer] = toBase58(vaultAcct.id()); + } + jv[jss::TransactionType] = jss::TrustSet; + return jv; + }(); + + trustSet[jss::Flags] = tfSetFreeze | tfSetDeepFreeze; + env(trustSet); + env.close(); + + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(1)}), + Ter(fix330Enabled ? TER(tecFROZEN) : TER(tecLOCKED))); + + trustSet[jss::Flags] = tfClearFreeze | tfClearDeepFreeze; + env(trustSet); + env.close(); + } + + // Clawback works while frozen + env(fset(issuer, asfGlobalFreeze)); + env(vault.clawback( + {.issuer = issuer, .id = keylet.key, .holder = owner, .amount = asset(1)})); + env(fclear(issuer, asfGlobalFreeze)); + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(1)})); + env.close(); + }; + + runTests(); + env.disableFeature(fixCleanup3_3_0); + runTests(); + env.enableFeature(fixCleanup3_3_0); + } + + // === MPT === + { + testcase("VaultDeposit MPT lock checks"); + Env env{*this}; + Vault vault{env}; + + env.fund(XRP(100'000), issuer, owner); + env.close(); + + MPTTester mptt{env, issuer, kMptInitNoFund}; + mptt.create( + {.flags = tfMPTCanClawback | tfMPTCanTransfer | tfMPTCanLock | tfMPTRequireAuth}); + PrettyAsset const mpt{mptt.issuanceID()}; + + mptt.authorize({.account = owner}); + mptt.authorize({.account = issuer, .holder = owner}); + env.close(); + env(pay(issuer, owner, mpt(100'000))); + env.close(); + + auto [tx, keylet] = vault.create({.owner = owner, .asset = mpt}); + env(tx); + env.close(); + auto const vaultAcctID = env.le(keylet)->at(sfAccount); + Account const vaultAcct("vault", vaultAcctID); + + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = mpt(100)})); + env.close(); + + // For MPT isDeepFrozen == isFrozen, so all locks block in + // both pre- and post-fix. + auto runTests = [&]() { + // Global lock + mptt.set({.flags = tfMPTLock}); + env.close(); + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = mpt(1)}), + Ter(tecLOCKED)); + mptt.set({.flags = tfMPTUnlock}); + env.close(); + + // Depositor individual lock + mptt.set({.holder = owner, .flags = tfMPTLock}); + env.close(); + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = mpt(1)}), + Ter(tecLOCKED)); + mptt.set({.holder = owner, .flags = tfMPTUnlock}); + env.close(); + + // Vault pseudo-account individual lock + mptt.set({.holder = vaultAcct, .flags = tfMPTLock}); + env.close(); + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = mpt(1)}), + Ter(tecLOCKED)); + mptt.set({.holder = vaultAcct, .flags = tfMPTUnlock}); + env.close(); + + // Clawback works while locked + mptt.set({.flags = tfMPTLock}); + env.close(); + env(vault.clawback( + {.issuer = issuer, .id = keylet.key, .holder = owner, .amount = mpt(1)})); + mptt.set({.flags = tfMPTUnlock}); + env.close(); + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = mpt(1)})); + env.close(); + }; + + runTests(); + env.disableFeature(fixCleanup3_3_0); + runTests(); + env.enableFeature(fixCleanup3_3_0); + } + } + + // Focused demonstration: a depositor under a regular individual IOU freeze + // can still withdraw to themselves (self-withdrawal), but is blocked from + // withdrawing to a third party. + // + // Pre-fixCleanup3_3_0: both the self-withdrawal AND the third-party + // withdrawal were blocked because the old code checked checkFrozen on the + // destination regardless of whether it was the submitter. + // Post-fixCleanup3_3_0: checkWithdrawFreeze skips the submitter freeze + // check when submitter == destination, so self-withdrawal succeeds. + void + testVaultSelfWithdrawWhileFrozen() + { + testcase("VaultWithdraw IOU self-withdrawal while individually frozen"); + + using namespace test::jtx; + + Account const issuer{"issuer"}; + Account const owner{"owner"}; + Account const charlie{"charlie"}; + Env env{*this}; + Vault vault{env}; + + env.fund(XRP(100'000), issuer, owner, charlie); + env(fset(issuer, asfAllowTrustLineClawback)); + env.close(); + + PrettyAsset const asset = issuer["IOU"]; + env.trust(asset(1'000'000), owner); + env.trust(asset(1'000'000), charlie); + env(pay(issuer, owner, asset(100'000))); + env.close(); + + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + env(tx); + env.close(); + + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(10)})); + env.close(); + + auto runTests = [&]() { + auto const fix330Enabled = env.current()->rules().enabled(fixCleanup3_3_0); + + // Set a regular individual freeze on the owner's IOU trustline. + env(trust(issuer, asset(0), owner, tfSetFreeze)); + env.close(); + + // Self-withdrawal: submitter == destination, so the submitter + // freeze check is skipped. + // Post-fix: tesSUCCESS. Pre-fix: tecFROZEN. + env(vault.withdraw({.depositor = owner, .id = keylet.key, .amount = asset(1)}), + Ter(fix330Enabled ? TER(tesSUCCESS) : TER(tecFROZEN))); + + // Withdrawal to a third party is blocked: submitter != destination + // so the submitter freeze check applies. + { + auto withdrawToCharlie = + vault.withdraw({.depositor = owner, .id = keylet.key, .amount = asset(1)}); + withdrawToCharlie[sfDestination] = charlie.human(); + // Post-fix: tecFROZEN (checkIndividualFrozen on submitter). + // Pre-fix: tecLOCKED (isFrozen on the vault share). + env(withdrawToCharlie, Ter(fix330Enabled ? TER(tecFROZEN) : TER(tecLOCKED))); + } + + env(trust(issuer, asset(0), owner, tfClearFreeze)); + env.close(); + }; + + runTests(); + env.disableFeature(fixCleanup3_3_0); + runTests(); + env.enableFeature(fixCleanup3_3_0); + } + + void + testVaultWithdrawFreeze() + { + using namespace test::jtx; + + Account const issuer{"issuer"}; + Account const owner{"owner"}; + + // === IOU === + { + testcase("VaultWithdraw IOU freeze checks"); + Env env{*this}; + Vault vault{env}; + + env.fund(XRP(100'000), issuer, owner); + env(fset(issuer, asfAllowTrustLineClawback)); + env.close(); + PrettyAsset const asset = issuer["IOU"]; + env.trust(asset(1'000'000), owner); + env(pay(issuer, owner, asset(100'000))); + env.close(); + + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + env(tx); + env.close(); + auto const vaultAcct = Account("vault", env.le(keylet)->at(sfAccount)); + + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(100)})); + env.close(); + + Account const charlie{"charlie"}; + env.fund(XRP(10'000), charlie); + env.trust(asset(1'000'000), charlie); + env.close(); + + auto runTests = [&]() { + auto const fix330Enabled = env.current()->rules().enabled(fixCleanup3_3_0); + // Post-fix: submitter freeze blocks withdraw to 3rd party + // Pre-fix: submitter's IOU freeze not checked, but + // checkFrozen(depositor, share) may trigger tecLOCKED + TER const submitterTo3rd = fix330Enabled ? TER(tecFROZEN) : TER(tecLOCKED); + + // Global freeze → self-withdraw + env(fset(issuer, asfGlobalFreeze)); + env(vault.withdraw({.depositor = owner, .id = keylet.key, .amount = asset(1)}), + Ter(tecFROZEN)); + // Global freeze → withdraw to 3rd party + { + auto withdrawToCharlie = + vault.withdraw({.depositor = owner, .id = keylet.key, .amount = asset(1)}); + withdrawToCharlie[sfDestination] = charlie.human(); + env(withdrawToCharlie, Ter(tecFROZEN)); + } + env(fclear(issuer, asfGlobalFreeze)); + + // Vault-account regular freeze + { + auto trustSet = [&]() { + json::Value jv; + jv[jss::Account] = issuer.human(); + { + auto& ja = jv[jss::LimitAmount] = + asset(0).value().getJson(JsonOptions::Values::None); + ja[jss::issuer] = toBase58(vaultAcct.id()); + } + jv[jss::TransactionType] = jss::TrustSet; + return jv; + }(); + + trustSet[jss::Flags] = tfSetFreeze; + env(trustSet); + env.close(); + + TER const vaultAcctFreeze = fix330Enabled ? TER(tecFROZEN) : TER(tecLOCKED); + + // Self-withdraw + env(vault.withdraw({.depositor = owner, .id = keylet.key, .amount = asset(1)}), + Ter(vaultAcctFreeze)); + // Withdraw to 3rd party + { + auto withdrawToCharlie = vault.withdraw( + {.depositor = owner, .id = keylet.key, .amount = asset(1)}); + withdrawToCharlie[sfDestination] = charlie.human(); + env(withdrawToCharlie, Ter(vaultAcctFreeze)); + } + + trustSet[jss::Flags] = tfClearFreeze; + env(trustSet); + env.close(); + } + + // Depositor regular freeze → self-withdraw + env(trust(issuer, asset(0), owner, tfSetFreeze)); + // Post-fix: self-withdraw allowed (submitter==dst skip) + // Pre-fix: isFrozen(depositor, iou) catches it + env(vault.withdraw({.depositor = owner, .id = keylet.key, .amount = asset(1)}), + Ter(fix330Enabled ? TER(tesSUCCESS) : TER(tecFROZEN))); + + // Depositor regular freeze → withdraw to 3rd party + { + auto withdrawTo3rd = + vault.withdraw({.depositor = owner, .id = keylet.key, .amount = asset(1)}); + withdrawTo3rd[sfDestination] = charlie.human(); + env(withdrawTo3rd, Ter(submitterTo3rd)); + } + env(trust(issuer, asset(0), owner, tfClearFreeze)); + // Replenish what was withdrawn + if (fix330Enabled) + { + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(1)})); + } + env.close(); + + // Depositor deep freeze → self-withdraw blocked + env(trust(issuer, asset(0), owner, tfSetFreeze | tfSetDeepFreeze)); + env(vault.withdraw({.depositor = owner, .id = keylet.key, .amount = asset(1)}), + Ter(fix330Enabled ? TER(tecFROZEN) : TER(tecFROZEN))); + env(trust(issuer, asset(0), owner, tfClearFreeze | tfClearDeepFreeze)); + + // Destination regular freeze → withdraw to 3rd party + env(trust(issuer, asset(0), charlie, tfSetFreeze)); + // Self-withdraw unaffected by charlie's freeze + env(vault.withdraw({.depositor = owner, .id = keylet.key, .amount = asset(1)})); + { + auto withdrawToCharlie = + vault.withdraw({.depositor = owner, .id = keylet.key, .amount = asset(1)}); + withdrawToCharlie[sfDestination] = charlie.human(); + // Post-fix: regular freeze on dst allowed + // Pre-fix: checkFrozen(dst, iou) catches it + env(withdrawToCharlie, Ter(fix330Enabled ? TER(tesSUCCESS) : TER(tecFROZEN))); + } + env(trust(issuer, asset(0), charlie, tfClearFreeze)); + // Replenish: 1 for self-withdraw + 1 if charlie withdraw succeeded + env(vault.deposit( + {.depositor = owner, + .id = keylet.key, + .amount = asset(fix330Enabled ? 2 : 1)})); + env.close(); + + // Destination deep freeze → withdraw to 3rd party blocked + env(trust(issuer, asset(0), charlie, tfSetFreeze | tfSetDeepFreeze)); + { + auto withdrawToCharlie = + vault.withdraw({.depositor = owner, .id = keylet.key, .amount = asset(1)}); + withdrawToCharlie[sfDestination] = charlie.human(); + env(withdrawToCharlie, Ter(tecFROZEN)); + } + // Destination deep freeze → self-withdraw unaffected + env(vault.withdraw({.depositor = owner, .id = keylet.key, .amount = asset(1)})); + env(trust(issuer, asset(0), charlie, tfClearFreeze | tfClearDeepFreeze)); + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(1)})); + env.close(); + + // Clawback works while frozen + env(fset(issuer, asfGlobalFreeze)); + env(vault.clawback( + {.issuer = issuer, .id = keylet.key, .holder = owner, .amount = asset(1)})); + env(fclear(issuer, asfGlobalFreeze)); + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(1)})); + env.close(); + }; + + runTests(); + env.disableFeature(fixCleanup3_3_0); + runTests(); + env.enableFeature(fixCleanup3_3_0); + } + + // === MPT === + { + testcase("VaultWithdraw MPT lock checks"); + Env env{*this}; + Vault vault{env}; + + env.fund(XRP(100'000), issuer, owner); + env.close(); + + MPTTester mptt{env, issuer, kMptInitNoFund}; + mptt.create( + {.flags = tfMPTCanClawback | tfMPTCanTransfer | tfMPTCanLock | tfMPTRequireAuth}); + PrettyAsset const mpt{mptt.issuanceID()}; + + mptt.authorize({.account = owner}); + mptt.authorize({.account = issuer, .holder = owner}); + env.close(); + env(pay(issuer, owner, mpt(100'000))); + env.close(); + + auto [tx, keylet] = vault.create({.owner = owner, .asset = mpt}); + env(tx); + env.close(); + Account const vaultAcct("vault", env.le(keylet)->at(sfAccount)); + + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = mpt(100)})); + env.close(); + + Account const charlie{"charlie"}; + env.fund(XRP(10'000), charlie); + env.close(); + mptt.authorize({.account = charlie}); + mptt.authorize({.account = issuer, .holder = charlie}); + env.close(); + + auto runTests = [&]() { + auto const fix330Enabled = env.current()->rules().enabled(fixCleanup3_3_0); + + // Global lock + mptt.set({.flags = tfMPTLock}); + env.close(); + env(vault.withdraw({.depositor = owner, .id = keylet.key, .amount = mpt(1)}), + Ter(tecLOCKED)); + + // Global lock → withdraw to issuer + // Post-fix: bypasses freeze checks, but accountHolds + // on the pseudo returns 0 under global lock + // Pre-fix: checkFrozen(dst=issuer) catches global lock + { + auto withdrawToIssuer = + vault.withdraw({.depositor = owner, .id = keylet.key, .amount = mpt(1)}); + withdrawToIssuer[sfDestination] = issuer.human(); + env(withdrawToIssuer, Ter(fix330Enabled ? TER(tesSUCCESS) : TER(tecLOCKED))); + } + mptt.set({.flags = tfMPTUnlock}); + env.close(); + if (fix330Enabled) + { + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = mpt(1)})); + } + env.close(); + + // Vault pseudo-account individual lock + mptt.set({.holder = vaultAcct, .flags = tfMPTLock}); + env.close(); + env(vault.withdraw({.depositor = owner, .id = keylet.key, .amount = mpt(1)}), + Ter(tecLOCKED)); + mptt.set({.holder = vaultAcct, .flags = tfMPTUnlock}); + env.close(); + + // Depositor individual lock → self-withdraw blocked + // (isDeepFrozen == isFrozen for MPT) + mptt.set({.holder = owner, .flags = tfMPTLock}); + env.close(); + env(vault.withdraw({.depositor = owner, .id = keylet.key, .amount = mpt(1)}), + Ter(tecLOCKED)); + // Depositor lock → withdraw to 3rd party also blocked + { + auto withdrawToCharlie = + vault.withdraw({.depositor = owner, .id = keylet.key, .amount = mpt(1)}); + withdrawToCharlie[sfDestination] = charlie.human(); + env(withdrawToCharlie, Ter(tecLOCKED)); + } + + // Depositor lock → withdraw to issuer + // Post-fix: issuer bypass in checkWithdrawFreezes + // Pre-fix: checkFrozen(depositor, share) blocks transitively + { + auto withdrawToIssuer = + vault.withdraw({.depositor = owner, .id = keylet.key, .amount = mpt(1)}); + withdrawToIssuer[sfDestination] = issuer.human(); + env(withdrawToIssuer, Ter(fix330Enabled ? TER(tesSUCCESS) : TER(tecLOCKED))); + } + mptt.set({.holder = owner, .flags = tfMPTUnlock}); + env.close(); + if (fix330Enabled) + { + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = mpt(1)})); + } + env.close(); + + // 3rd party destination lock → withdraw to 3rd party blocked + mptt.set({.holder = charlie, .flags = tfMPTLock}); + env.close(); + { + auto withdrawToCharlie = + vault.withdraw({.depositor = owner, .id = keylet.key, .amount = mpt(1)}); + withdrawToCharlie[sfDestination] = charlie.human(); + env(withdrawToCharlie, Ter{tecLOCKED}); + } + // 3rd party lock → self-withdraw unaffected + env(vault.withdraw({.depositor = owner, .id = keylet.key, .amount = mpt(1)})); + mptt.set({.holder = charlie, .flags = tfMPTUnlock}); + env.close(); + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = mpt(1)})); + env.close(); + + // Clawback works while locked + mptt.set({.flags = tfMPTLock}); + env.close(); + env(vault.clawback( + {.issuer = issuer, .id = keylet.key, .holder = owner, .amount = mpt(1)})); + mptt.set({.flags = tfMPTUnlock}); + env.close(); + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = mpt(1)})); + env.close(); + }; + + runTests(); + env.disableFeature(fixCleanup3_3_0); + runTests(); + env.enableFeature(fixCleanup3_3_0); + } + } + public: void run() override @@ -7864,6 +8083,10 @@ class Vault_test : public beast::unit_test::Suite testWithdrawSoleShareholderPartialFixedSharesUsesFullPrice(); testWithdrawSoleShareholderLoanRepaymentExit(); + testVaultDepositFreeze(); + testVaultWithdrawFreeze(); + testVaultSelfWithdrawWhileFrozen(); + testReferenceHolding(); testHoldingDeletionBlocked(); }