From 5199c5e073288f64885cd123dbdb267d35904598 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 15 Jul 2025 15:16:02 -0400 Subject: [PATCH] Improve / add freeze checking helper functions - Add an override of isDeepFrozen that can take an Asset, and thus an MPTIssue. The MPT version just calls isFrozen, since they're equivalent for MPTs. - Add wrappers checkFrozen and checkDeepFrozen that return the appropriate TER code, so the Asset type doesn't have to be checked at every #*%@ing caller. - Convert the Loan* transactors to use these functions. --- .../app/tx/detail/LoanBrokerCoverDeposit.cpp | 14 +-- .../app/tx/detail/LoanBrokerCoverWithdraw.cpp | 17 +--- src/xrpld/app/tx/detail/LoanDraw.cpp | 15 ++- src/xrpld/app/tx/detail/LoanPay.cpp | 17 ++-- src/xrpld/app/tx/detail/LoanSet.cpp | 26 +++-- src/xrpld/ledger/View.h | 97 +++++++++++++++++++ 6 files changed, 136 insertions(+), 50 deletions(-) diff --git a/src/xrpld/app/tx/detail/LoanBrokerCoverDeposit.cpp b/src/xrpld/app/tx/detail/LoanBrokerCoverDeposit.cpp index 923703a6a0..5b9e8bba0e 100644 --- a/src/xrpld/app/tx/detail/LoanBrokerCoverDeposit.cpp +++ b/src/xrpld/app/tx/detail/LoanBrokerCoverDeposit.cpp @@ -89,15 +89,11 @@ LoanBrokerCoverDeposit::preclaim(PreclaimContext const& ctx) auto const pseudoAccountID = sleBroker->at(sfAccount); // Cannot transfer a frozen Asset - if (isFrozen(ctx.view, account, vaultAsset)) - return vaultAsset.holds() ? tecFROZEN : tecLOCKED; - if (vaultAsset.holds()) - { - auto const issue = vaultAsset.get(); - if (isDeepFrozen( - ctx.view, pseudoAccountID, issue.currency, issue.account)) - return tecFROZEN; - } + 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 (accountHolds( ctx.view, diff --git a/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp b/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp index a1e1233b89..d1705082d6 100644 --- a/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp +++ b/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp @@ -90,18 +90,11 @@ LoanBrokerCoverWithdraw::preclaim(PreclaimContext const& ctx) auto const pseudoAccountID = sleBroker->at(sfAccount); // Cannot transfer a frozen Asset - /* - if (isFrozen(ctx.view, account, vaultAsset)) - return vaultAsset.holds() ? tecFROZEN : tecLOCKED; - */ - if (isFrozen(ctx.view, pseudoAccountID, vaultAsset)) - return vaultAsset.holds() ? tecFROZEN : tecLOCKED; - if (vaultAsset.holds()) - { - auto const issue = vaultAsset.get(); - if (isDeepFrozen(ctx.view, account, issue.currency, issue.account)) - return tecFROZEN; - } + if (auto const ret = checkFrozen(ctx.view, pseudoAccountID, vaultAsset)) + return ret; + // Account cannot receive if asset is deep frozen + if (auto const ret = checkDeepFrozen(ctx.view, account, vaultAsset)) + return ret; auto const coverAvail = sleBroker->at(sfCoverAvailable); // Cover Rate is in 1/10 bips units diff --git a/src/xrpld/app/tx/detail/LoanDraw.cpp b/src/xrpld/app/tx/detail/LoanDraw.cpp index 2e957c6f79..3b99ccf900 100644 --- a/src/xrpld/app/tx/detail/LoanDraw.cpp +++ b/src/xrpld/app/tx/detail/LoanDraw.cpp @@ -131,19 +131,16 @@ LoanDraw::preclaim(PreclaimContext const& ctx) return tecINSUFFICIENT_FUNDS; } - if (isFrozen(ctx.view, brokerPseudoAccount, asset)) + if (auto const ret = checkFrozen(ctx.view, brokerPseudoAccount, asset)) { JLOG(ctx.j.warn()) << "Loan Broker pseudo-account is frozen."; - return asset.holds() ? tecFROZEN : tecLOCKED; + return ret; } - if (asset.holds()) + if (auto const ret = checkDeepFrozen(ctx.view, account, asset)) { - auto const issue = asset.get(); - if (isDeepFrozen(ctx.view, account, issue.currency, issue.account)) - { - JLOG(ctx.j.warn()) << "Borrower account is frozen."; - return tecFROZEN; - } + JLOG(ctx.j.warn()) + << "Borrower account cannot receive funds (deep frozen)."; + return ret; } if (hasExpired(ctx.view, loanSle->at(sfNextPaymentDueDate))) diff --git a/src/xrpld/app/tx/detail/LoanPay.cpp b/src/xrpld/app/tx/detail/LoanPay.cpp index e83e4ae27e..866a5d9914 100644 --- a/src/xrpld/app/tx/detail/LoanPay.cpp +++ b/src/xrpld/app/tx/detail/LoanPay.cpp @@ -131,19 +131,16 @@ LoanPay::preclaim(PreclaimContext const& ctx) return tecWRONG_ASSET; } - if (isFrozen(ctx.view, brokerPseudoAccount, asset)) + if (auto const ret = checkFrozen(ctx.view, account, asset)) { - JLOG(ctx.j.warn()) << "Loan Broker pseudo-account is frozen."; - return asset.holds() ? tecFROZEN : tecLOCKED; + JLOG(ctx.j.warn()) << "Borrower account is frozen."; + return ret; } - if (asset.holds()) + if (auto const ret = checkDeepFrozen(ctx.view, brokerPseudoAccount, asset)) { - auto const issue = asset.get(); - if (isDeepFrozen(ctx.view, account, issue.currency, issue.account)) - { - JLOG(ctx.j.warn()) << "Borrower account is frozen."; - return tecFROZEN; - } + JLOG(ctx.j.warn()) << "Loan Broker pseudo-account can not receive " + "funds (deep frozen)."; + return ret; } return tesSUCCESS; diff --git a/src/xrpld/app/tx/detail/LoanSet.cpp b/src/xrpld/app/tx/detail/LoanSet.cpp index c6cfed0335..b42cdb1cbc 100644 --- a/src/xrpld/app/tx/detail/LoanSet.cpp +++ b/src/xrpld/app/tx/detail/LoanSet.cpp @@ -204,19 +204,25 @@ LoanSet::preclaim(PreclaimContext const& ctx) if (auto const ter = canAddHolding(ctx.view, asset)) return ter; - if (isFrozen(ctx.view, brokerOwner, asset) || - isFrozen(ctx.view, brokerPseudo, asset)) + if (auto const ret = checkFrozen(ctx.view, brokerOwner, asset)) { - JLOG(ctx.j.warn()) << "One of the affected accounts is frozen."; - return asset.holds() ? tecFROZEN : tecLOCKED; + JLOG(ctx.j.warn()) << "Broker owner account is frozen."; + return ret; } - if (asset.holds()) + if (auto const ret = checkFrozen(ctx.view, brokerPseudo, asset)) { - auto const issue = asset.get(); - if (isDeepFrozen(ctx.view, borrower, issue.currency, issue.account)) - return tecFROZEN; - if (isDeepFrozen(ctx.view, brokerPseudo, issue.currency, issue.account)) - return tecFROZEN; + JLOG(ctx.j.warn()) << "Broker pseudo-account account is frozen."; + return ret; + } + if (auto const ret = checkDeepFrozen(ctx.view, borrower, asset)) + { + JLOG(ctx.j.warn()) << "Borrower account is deep frozen."; + return ret; + } + if (auto const ret = checkDeepFrozen(ctx.view, brokerPseudo, asset)) + { + JLOG(ctx.j.warn()) << "Broker pseudo-account account is deep frozen."; + return ret; } auto const principalRequested = tx[sfPrincipalRequested]; diff --git a/src/xrpld/ledger/View.h b/src/xrpld/ledger/View.h index 4f19e34b97..3a01638591 100644 --- a/src/xrpld/ledger/View.h +++ b/src/xrpld/ledger/View.h @@ -175,6 +175,29 @@ isFrozen( asset.value()); } +[[nodiscard]] inline TER +checkFrozen(ReadView const& view, AccountID const& account, Issue const& issue) +{ + return isFrozen(view, account, issue) ? (TER)tecFROZEN : (TER)tesSUCCESS; +} + +[[nodiscard]] inline TER +checkFrozen( + ReadView const& view, + AccountID const& account, + MPTIssue const& mptIssue) +{ + return isFrozen(view, account, mptIssue) ? (TER)tecLOCKED : (TER)tesSUCCESS; +} + +[[nodiscard]] inline TER +checkFrozen(ReadView const& view, AccountID const& account, Asset const& asset) +{ + return std::visit( + [&](auto const& issue) { return checkFrozen(view, account, issue); }, + asset.value()); +} + [[nodiscard]] bool isAnyFrozen( ReadView const& view, @@ -220,6 +243,80 @@ isDeepFrozen( Currency const& currency, AccountID const& issuer); +[[nodiscard]] inline bool +isDeepFrozen( + ReadView const& view, + AccountID const& account, + Issue const& issue, + int = 0 /*ignored*/) +{ + return isDeepFrozen(view, account, issue.currency, issue.account); +} + +[[nodiscard]] inline bool +isDeepFrozen( + ReadView const& view, + AccountID const& account, + MPTIssue const& mptIssue, + int depth = 0) +{ + // Unlike IOUs, frozen / locked MPTs are not allowed to send or receive + // funds, so checking "deep frozen" is the same as checking "frozen". + return isFrozen(view, account, mptIssue, depth); +} + +/** + * isFrozen check is recursive for MPT shares in a vault, descending to + * assets in the vault, up to maxAssetCheckDepth recursion depth. This is + * purely defensive, as we currently do not allow such vaults to be created. + */ +[[nodiscard]] inline bool +isDeepFrozen( + ReadView const& view, + AccountID const& account, + Asset const& asset, + int depth = 0) +{ + return std::visit( + [&](auto const& issue) { + return isDeepFrozen(view, account, issue, depth); + }, + asset.value()); +} + +[[nodiscard]] inline TER +checkDeepFrozen( + ReadView const& view, + AccountID const& account, + Issue const& issue) +{ + return isDeepFrozen(view, account, issue) ? (TER)tecFROZEN + : (TER)tesSUCCESS; +} + +[[nodiscard]] inline TER +checkDeepFrozen( + ReadView const& view, + AccountID const& account, + MPTIssue const& mptIssue) +{ + return isDeepFrozen(view, account, mptIssue) ? (TER)tecLOCKED + : (TER)tesSUCCESS; +} + +[[nodiscard]] inline TER +checkDeepFrozen( + ReadView const& view, + AccountID const& account, + Asset const& asset) +{ + return std::visit( + [&](auto const& issue) { + return checkDeepFrozen(view, account, issue); + }, + asset.value()); +} + [[nodiscard]] bool isLPTokenFrozen( ReadView const& view,