From 53e5c8c6ae3f4a574a87879a7f22f20e529e83d6 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Mon, 22 Jun 2026 13:52:12 +0200 Subject: [PATCH] fix: Align pseudo-account withdraw freeze handling for issuer redemption Restore unconditional IgnoreFreeze in VaultWithdraw::doApply when fixCleanup3_3_0 is enabled. That accountHolds reads the submitter's share balance, which recurses into the underlying asset, so freeze gating must stay in preclaim; a dstAcct==issuer guard there wrongly blocked self-withdrawal under a regular freeze. Apply issuer-aware freeze handling to LoanBrokerCoverWithdraw::preclaim so the issuer can redeem their own token from the broker pseudo-account even under a global freeze, matching AMM and Vault. Here accountHolds reads the pseudo-account's holdings, so the issuer-only IgnoreFreeze is both necessary and sufficient. Rename the checkWithdrawFreeze source parameter to srcAcct to match its definition, drop a misleading comment, and hoist the fixCleanup feature flags into named locals. --- include/xrpl/ledger/helpers/TokenHelpers.h | 2 +- .../lending/LoanBrokerCoverDeposit.cpp | 7 +++---- .../lending/LoanBrokerCoverWithdraw.cpp | 15 ++++++++++----- .../tx/transactors/vault/VaultDeposit.cpp | 6 ++++-- .../tx/transactors/vault/VaultWithdraw.cpp | 18 ++++++++++-------- 5 files changed, 28 insertions(+), 20 deletions(-) diff --git a/include/xrpl/ledger/helpers/TokenHelpers.h b/include/xrpl/ledger/helpers/TokenHelpers.h index ce661bae9c..9061f14a0f 100644 --- a/include/xrpl/ledger/helpers/TokenHelpers.h +++ b/include/xrpl/ledger/helpers/TokenHelpers.h @@ -159,7 +159,7 @@ checkDeepFrozen(ReadView const& view, AccountID const& account, Asset const& ass [[nodiscard]] TER checkWithdrawFreeze( ReadView const& view, - AccountID const& sourceAcct, + AccountID const& srcAcct, AccountID const& submitterAcct, AccountID const& dstAcct, Asset const& asset); diff --git a/src/libxrpl/tx/transactors/lending/LoanBrokerCoverDeposit.cpp b/src/libxrpl/tx/transactors/lending/LoanBrokerCoverDeposit.cpp index b0b241c804..9f9d524929 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]; @@ -78,7 +80,7 @@ LoanBrokerCoverDeposit::preclaim(PreclaimContext const& ctx) if (auto const ret = canTransfer(ctx.view, vaultAsset, account, pseudoAccountID)) return ret; - if (ctx.view.rules().enabled(fixCleanup3_3_0)) + if (fix330Enabled) { if (auto const ret = checkDepositFreeze(ctx.view, account, pseudoAccountID, vaultAsset)) return ret; @@ -88,8 +90,6 @@ LoanBrokerCoverDeposit::preclaim(PreclaimContext const& ctx) if (auto const ret = checkFrozen(ctx.view, account, vaultAsset)) return ret; - // Unlike regular accounts, pseudo-accounts cannot receive assets - // Under a regular freeze because those funds cannot be later withdrawn if (auto const ret = checkDeepFrozen(ctx.view, pseudoAccountID, vaultAsset)) return ret; } @@ -103,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 c4d8fbd58c..ef40abd1db 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,7 +126,7 @@ LoanBrokerCoverWithdraw::preclaim(PreclaimContext const& ctx) if (auto const ter = requireAuth(ctx.view, vaultAsset, dstAcct, authType)) return ter; - if (ctx.view.rules().enabled(fixCleanup3_3_0)) + if (fix330Enabled) { if (auto const ret = checkWithdrawFreeze(ctx.view, pseudoAccountID, account, dstAcct, vaultAsset)) @@ -148,7 +149,7 @@ LoanBrokerCoverWithdraw::preclaim(PreclaimContext const& ctx) // 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); @@ -167,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 f73832e089..6179a0d55d 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,7 +110,7 @@ VaultDeposit::preclaim(PreclaimContext const& ctx) // LCOV_EXCL_STOP } - if (ctx.view.rules().enabled(fixCleanup3_3_0)) + if (fix330Enabled) { if (auto const ret = checkDepositFreeze(ctx.view, account, vaultAccount, vaultAsset)) return ret; @@ -149,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 32e42c0958..438706ffea 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,15 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx) if (auto const ter = requireAuth(ctx.view, vaultAsset, dstAcct, authType); !isTesSuccess(ter)) return ter; - if (ctx.view.rules().enabled(fixCleanup3_3_0)) + if (fix330Enabled) { - // checkWithdrawFreezes checks the underlying asset on the source + // 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 checkWithdrawFreezes already covers. + // underlying asset, which checkWithdrawFreeze covers. if (auto const ret = checkWithdrawFreeze(ctx.view, vaultAccount, account, dstAcct, vaultAsset)) return ret; @@ -269,8 +272,7 @@ VaultWithdraw::doApply() return tecPATH_DRY; } - // Post-fixCleanup3_3_0: preclaim already handles all freeze checks, so they are safe to skip - // here + // Post-fixCleanup3_3_0: preclaim already handles freeze checks auto const freezeHandling = view().rules().enabled(fixCleanup3_3_0) ? FreezeHandling::IgnoreFreeze : FreezeHandling::ZeroIfFrozen;