Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
289 changes: 288 additions & 1 deletion include/xrpl/ledger/helpers/EscrowHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

#include <xrpl/basics/Log.h>
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/beast/utility/Zero.h>
#include <xrpl/ledger/ApplyView.h>
#include <xrpl/ledger/ReadView.h>
#include <xrpl/ledger/helpers/AccountRootHelpers.h>
#include <xrpl/ledger/helpers/MPTokenHelpers.h>
#include <xrpl/ledger/helpers/RippleStateHelpers.h>
Expand All @@ -25,6 +27,290 @@

namespace xrpl {

/** Validate that @p account may lock @p amount of a token for later delivery
* to @p dest.
*
* The lock-side counterpart of escrowUnlockPreclaimHelper: every issuer
* control (locking opt-in, authorization, freeze/lock, transferability,
* spendable balance) that gates locking token value lives here, so any
* transactor that locks funds applies the same rules. The signature is
* view-based rather than PreclaimContext-based so it can also run from
* doApply.
*/
template <ValidIssueType T>
TER
escrowLockPreclaimHelper(
ReadView const& view,
AccountID const& account,
AccountID const& dest,
STAmount const& amount,
beast::Journal j);

template <>
inline TER
escrowLockPreclaimHelper<Issue>(
ReadView const& view,
AccountID const& account,
AccountID const& dest,
STAmount const& amount,
beast::Journal j)
{
auto const& issue = amount.get<Issue>();
AccountID const& issuer = amount.getIssuer();
// If the issuer is the same as the account, return tecNO_PERMISSION
if (issuer == account)
return tecNO_PERMISSION;

// If the lsfAllowTrustLineLocking is not enabled, return tecNO_PERMISSION
auto const sleIssuer = view.read(keylet::account(issuer));
if (!sleIssuer)
return tecNO_ISSUER;
if (!sleIssuer->isFlag(lsfAllowTrustLineLocking))
return tecNO_PERMISSION;

// If the account does not have a trustline to the issuer, return tecNO_LINE
auto const sleRippleState = view.read(keylet::trustLine(account, issuer, issue.currency));
if (!sleRippleState)
return tecNO_LINE;

STAmount const balance = (*sleRippleState)[sfBalance];

// If balance is positive, issuer must have higher address than account
if (balance > beast::kZero && issuer < account)
return tecNO_PERMISSION; // LCOV_EXCL_LINE

// If balance is negative, issuer must have lower address than account
if (balance < beast::kZero && issuer > account)
return tecNO_PERMISSION; // LCOV_EXCL_LINE

// If the issuer has requireAuth set, check if the account is authorized
if (auto const ter = requireAuth(view, issue, account); !isTesSuccess(ter))
return ter;

// If the issuer has requireAuth set, check if the destination is authorized
if (auto const ter = requireAuth(view, issue, dest); !isTesSuccess(ter))
return ter;

// If the issuer has frozen the account, return tecFROZEN
if (isFrozen(view, account, issue))
return tecFROZEN;

// If the issuer has frozen the destination, return tecFROZEN
if (isFrozen(view, dest, issue))
return tecFROZEN;

STAmount const spendableAmount =
accountHolds(view, account, issue.currency, issuer, FreezeHandling::IgnoreFreeze, j);

// If the balance is less than or equal to 0, return tecINSUFFICIENT_FUNDS
if (spendableAmount <= beast::kZero)
return tecINSUFFICIENT_FUNDS;

// If the spendable amount is less than the amount, return
// tecINSUFFICIENT_FUNDS
if (spendableAmount < amount)
return tecINSUFFICIENT_FUNDS;

// If the amount is not addable to the balance, return tecPRECISION_LOSS
if (!canAdd(spendableAmount, amount))
return tecPRECISION_LOSS;

return tesSUCCESS;
}

template <>
inline TER
escrowLockPreclaimHelper<MPTIssue>(
ReadView const& view,
AccountID const& account,
AccountID const& dest,
STAmount const& amount,
beast::Journal j)
{
AccountID const issuer = amount.getIssuer();
// If the issuer is the same as the account, return tecNO_PERMISSION
if (issuer == account)
return tecNO_PERMISSION;

// If the mpt does not exist, return tecOBJECT_NOT_FOUND
auto const issuanceKey = keylet::mptokenIssuance(amount.get<MPTIssue>().getMptID());
auto const sleIssuance = view.read(issuanceKey);
if (!sleIssuance)
return tecOBJECT_NOT_FOUND;

// If the lsfMPTCanEscrow is not enabled, return tecNO_PERMISSION
if (!sleIssuance->isFlag(lsfMPTCanEscrow))
return tecNO_PERMISSION;

// If the issuer is not the same as the issuer of the mpt, return
// tecNO_PERMISSION
if (sleIssuance->getAccountID(sfIssuer) != issuer)
return tecNO_PERMISSION; // LCOV_EXCL_LINE

// If the account does not have the mpt, return tecOBJECT_NOT_FOUND
if (!view.exists(keylet::mptoken(issuanceKey.key, account)))
return tecOBJECT_NOT_FOUND;

// If the issuer has requireAuth set, check if the account is
// authorized
auto const& mptIssue = amount.get<MPTIssue>();
if (auto const ter = requireAuth(view, mptIssue, account, AuthType::WeakAuth);
!isTesSuccess(ter))
return ter;

// If the issuer has requireAuth set, check if the destination is
// authorized
if (auto const ter = requireAuth(view, mptIssue, dest, AuthType::WeakAuth); !isTesSuccess(ter))
return ter;

// If the issuer has frozen the account, return tecLOCKED
if (isFrozen(view, account, mptIssue))
return tecLOCKED;

// If the issuer has frozen the destination, return tecLOCKED
if (isFrozen(view, dest, mptIssue))
return tecLOCKED;

// If the mpt cannot be transferred, return tecNO_AUTH
if (auto const ter = canTransfer(view, mptIssue, account, dest); !isTesSuccess(ter))
return ter;

STAmount const spendableAmount = accountHolds(
view,
account,
amount.get<MPTIssue>(),
FreezeHandling::IgnoreFreeze,
AuthHandling::IgnoreAuth,
j);

// If the balance is less than or equal to 0, return tecINSUFFICIENT_FUNDS
if (spendableAmount <= beast::kZero)
return tecINSUFFICIENT_FUNDS;

// If the spendable amount is less than the amount, return
// tecINSUFFICIENT_FUNDS
if (spendableAmount < amount)
return tecINSUFFICIENT_FUNDS;

return tesSUCCESS;
}

template <ValidIssueType T>
TER
escrowLockApplyHelper(
ApplyView& view,
AccountID const& issuer,
AccountID const& sender,
STAmount const& amount,
beast::Journal journal);

template <>
inline TER
escrowLockApplyHelper<Issue>(
ApplyView& view,
AccountID const& issuer,
AccountID const& sender,
STAmount const& amount,
beast::Journal journal)
{
// Defensive: Issuer cannot create an escrow
if (issuer == sender)
return tecINTERNAL; // LCOV_EXCL_LINE

auto const ter =
directSendNoFee(view, sender, issuer, amount, !amount.holds<MPTIssue>(), journal);
if (!isTesSuccess(ter))
return ter; // LCOV_EXCL_LINE
return tesSUCCESS;
}

template <>
inline TER
escrowLockApplyHelper<MPTIssue>(
ApplyView& view,
AccountID const& issuer,
AccountID const& sender,
STAmount const& amount,
beast::Journal journal)
{
// Defensive: Issuer cannot create an escrow
if (issuer == sender)
return tecINTERNAL; // LCOV_EXCL_LINE

auto const ter = lockEscrowMPT(view, sender, amount, journal);
if (!isTesSuccess(ter))
return ter; // LCOV_EXCL_LINE
return tesSUCCESS;
}

template <ValidIssueType T>
TER
escrowUnlockPreclaimHelper(
ReadView const& view,
AccountID const& account,
STAmount const& amount,
bool checkFreeze = true);

template <>
inline TER
escrowUnlockPreclaimHelper<Issue>(
ReadView const& view,
AccountID const& account,
STAmount const& amount,
bool checkFreeze)
{
AccountID const& issuer = amount.getIssuer();
// If the issuer is the same as the account, return tesSUCCESS
if (issuer == account)
return tesSUCCESS;

// If the issuer has requireAuth set, check if the destination is authorized
if (auto const ter = requireAuth(view, amount.get<Issue>(), account); !isTesSuccess(ter))
return ter;

// If the issuer has deep frozen the destination, return tecFROZEN
if (checkFreeze &&
isDeepFrozen(view, account, amount.get<Issue>().currency, amount.getIssuer()))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IOU unlock path gates delivery on isDeepFrozen(), which only checks lsfHighDeepFreeze/lsfLowDeepFreeze on the trust line — it ignores both the issuer's global freeze (lsfGlobalFreeze on their AccountRoot) and regular per-line freezes (lsfHighFreeze/lsfLowFreeze). This is directly inconsistent with the corresponding lock path in escrowLockPreclaimHelper<Issue>, which correctly uses isFrozen() covering all three freeze forms, and with the MPT unlock counterpart (escrowUnlockPreclaimHelper<MPTIssue>) which also uses isFrozen(). Since neither EscrowFinish::doApply nor PaymentChannelClaim::doApply perform any additional freeze check, this preclaim helper is the sole enforcement point. An issuer who applies a compliance freeze after an IOU escrow or payment channel is created cannot prevent the frozen destination from receiving the tokens. Replace isDeepFrozen(view, account, amount.get<Issue>().currency, amount.getIssuer()) with isFrozen(view, account, amount.get<Issue>()) to match the lock-side semantics and the MPT specialization.

Suggested fix

Replace isDeepFrozen(view, account, amount.get<Issue>().currency, amount.getIssuer()) with isFrozen(view, account, amount.get<Issue>()). This matches the lock path (escrowLockPreclaimHelper<Issue>) and the MPT unlock path (escrowUnlockPreclaimHelper<MPTIssue>), both of which use isFrozen(). The isFrozen() overload for Issue internally checks global freeze on the issuer's AccountRoot and both per-line freeze flags on the trust line — all three mechanisms a compliant issuer would use.

return tecFROZEN;

return tesSUCCESS;
}

template <>
inline TER
escrowUnlockPreclaimHelper<MPTIssue>(
ReadView const& view,
AccountID const& account,
STAmount const& amount,
bool checkFreeze)
{
AccountID const& issuer = amount.getIssuer();
// If the issuer is the same as the account, return tesSUCCESS
if (issuer == account)
return tesSUCCESS;

// If the mpt does not exist, return tecOBJECT_NOT_FOUND
auto const issuanceKey = keylet::mptokenIssuance(amount.get<MPTIssue>().getMptID());
auto const sleIssuance = view.read(issuanceKey);
if (!sleIssuance)
return tecOBJECT_NOT_FOUND;

// If the issuer has requireAuth set, check if the account is
// authorized
auto const& mptIssue = amount.get<MPTIssue>();
if (auto const ter = requireAuth(view, mptIssue, account, AuthType::WeakAuth);
!isTesSuccess(ter))
return ter;

// If the issuer has frozen the account, return tecLOCKED
if (checkFreeze && isFrozen(view, account, mptIssue))
return tecLOCKED;

return tesSUCCESS;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MPT unlock preclaim has no canTransfer check, while the symmetric lock preclaim at line 175 explicitly gates on canTransfer(view, mptIssue, account, dest). If an issuer clears lsfMPTCanTransfer after the escrow is created, the unlock path silently succeeds for non-issuer destinations — the invariant check (ValidMPTTransfer) doesn't catch this either because unlockEscrowMPT decrements sfLockedAmount rather than sfMPTAmount, so the invariant sees zero senders. Other MPT recovery paths that intentionally bypass transferability (vault/lending) do so with an explicit WaiveMPTCanTransfer::Yes argument and are whitelisted in the invariant; escrow/paychan does neither. If the design intent is a deliberate "don't trap funds" recovery policy, it needs a comment, a WaiveMPTCanTransfer parameter, and a corresponding entry in the ValidMPTTransfer invariant whitelist; if it's an oversight, add the canTransfer check (or pass the original escrow owner's AccountID into the helper so the check can be formed correctly).

Suggested fix

Either: (1) document the intentional omission, add an explicit WaiveMPTCanTransfer concept analogous to vault/lending, and whitelist ttESCROW_FINISH/ttPAYCHAN_CLAIM in ValidMPTTransfer; or (2) add the original escrow sender as a parameter to escrowUnlockPreclaimHelper and call canTransfer(view, mptIssue, sender, account) before the final return.

}

//------------------------------------------------------------------------------

template <ValidIssueType T>
TER
escrowUnlockApplyHelper(
Expand Down Expand Up @@ -58,14 +344,15 @@ escrowUnlockApplyHelper<Issue>(
bool const recvLow = issuer > receiver;
bool const senderIssuer = issuer == sender;
bool const receiverIssuer = issuer == receiver;
bool const lineExisted = ctx.view.exists(trustLineKey);

if (senderIssuer)
return tecINTERNAL; // LCOV_EXCL_LINE

if (receiverIssuer)
return tesSUCCESS;

if (!ctx.view.exists(trustLineKey) && createAsset)
if (!lineExisted && createAsset)
{
// Can the account cover the trust line's reserve?
auto const sponsorSle = getEffectiveTxReserveSponsor(ctx, sleDest);
Expand Down
10 changes: 9 additions & 1 deletion include/xrpl/ledger/helpers/PaymentChannelHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <xrpl/basics/base_uint.h>
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/ledger/ApplyView.h>
#include <xrpl/protocol/AccountID.h>
#include <xrpl/protocol/Rules.h>
#include <xrpl/protocol/STLedgerEntry.h>
#include <xrpl/protocol/TER.h>
Expand All @@ -17,12 +18,19 @@ namespace xrpl {
* @param slep The SLE for the PayChannel object to close.
* @param view The apply view in which ledger state modifications are made.
* @param key The ledger key identifying the PayChannel entry.
* @param txAccount The account submitting the transaction that closes the
* channel.
* @param j Journal used for fatal-level diagnostic messages.
* @return tesSUCCESS on success; tefBAD_LEDGER if a directory removal
* fails; tefINTERNAL if the source account SLE cannot be found.
*/
TER
closeChannel(SLE::ref slep, ApplyView& view, uint256 const& key, beast::Journal j);
closeChannel(
SLE::ref slep,
ApplyView& view,
uint256 const& key,
AccountID const& txAccount,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the non-XRP (IOU/MPT) branch of closeChannel, createAsset is computed as src == txAccount, meaning it is false whenever the transaction submitter is not the channel source. Two call sites reach closeChannel on expired channels with no prior authorization gate — PaymentChannelFund::doApply and the expired-channel path in PaymentChannelClaim::doApply (which fires before the src/dst authorization check). A third call site reaches it when the channel destination closes a non-dry channel via tfClose. In all three cases txAccount != src, so createAsset = false. With createAsset = false, escrowUnlockApplyHelper<Issue> enforces the source's trust-line limit when crediting the returned residual balance. If the source has lowered their trust-line limit below the residual amount after the channel was created, any third-party or destination-initiated close is rejected and the funds are inaccessible until the source self-closes — because only the source's own close produces createAsset = true, which bypasses the limit check. The correct semantic for returning locked funds to the channel owner in a close is that createAsset should be true unconditionally in closeChannel (the source is always the recipient of their own returned funds), matching the invariant that a valid close must always be able to drain the residual balance.

Suggested fix

Remove the dependency on txAccount for computing createAsset inside closeChannel. Since the channel source is always the recipient of the residual balance in a close, hardcode createAsset = true in the std::visit call, and drop the txAccount parameter entirely. If there is a deliberate design intent to distinguish self-close from third-party close for some other purpose, that intent must be documented and the trust-line limit asymmetry addressed separately (e.g., do the limit check only when the source chose to reduce their limit below the locked amount post-creation).

beast::Journal j);

/** Add two uint32_t values with saturation at UINT32_MAX.
*
Expand Down
Loading