From discussion with @gregtatcam: Revert "Payment"

- Revert Payment transactor (Payment.*) back to what is in "develop".
- Change LoanBrokerCoverWithdraw to do a direct transfer, whether it's
  too the account or the destination. Similar to VaultWithdraw, and as
  specified
- https://github.com/XRPLF/rippled/pull/5270#discussion_r2554560222
This commit is contained in:
Ed Hennis
2025-11-23 23:06:44 -05:00
parent 4c28e750ff
commit dd5464d921
3 changed files with 165 additions and 306 deletions

View File

@@ -159,60 +159,25 @@ LoanBrokerCoverWithdraw::doApply()
return ter;
}
// Move the funds from the broker's pseudo-account to the dstAcct
if (dstAcct == account_ || amount.native())
// Sanity check
if (accountHolds(
view(),
brokerPseudoID,
amount.asset(),
FreezeHandling::fhIGNORE_FREEZE,
AuthHandling::ahIGNORE_AUTH,
j_) < amount)
{
// Transfer assets directly from pseudo-account to depositor.
// Because this is either a self-transfer or an XRP payment, there is no
// need to use the payment engine.
return accountSend(
view(), brokerPseudoID, dstAcct, amount, j_, WaiveTransferFee::Yes);
// LCOV_EXCL_START
JLOG(j_.error()) << "LoanBrokerCoverWithdraw: negative balance of "
"broker cover assets.";
return tefINTERNAL;
// LCOV_EXCL_STOP
}
{
// If sending the Cover to a different account, then this is
// effectively a payment. Use the Payment transaction code to call
// the payment engine, though only a subset of the functionality is
// supported in this transaction. e.g. No paths, no partial
// payments.
bool const mptDirect = amount.holds<MPTIssue>();
STAmount const maxSourceAmount =
Payment::getMaxSourceAmount(brokerPseudoID, amount);
SLE::pointer sleDst = view().peek(keylet::account(dstAcct));
if (!sleDst)
return tecINTERNAL; // LCOV_EXCL_LINE
Payment::RipplePaymentParams paymentParams{
.ctx = ctx_,
.maxSourceAmount = maxSourceAmount,
.srcAccountID = brokerPseudoID,
.dstAccountID = dstAcct,
.sleDst = sleDst,
.dstAmount = amount,
.paths = STPathSet{},
.deliverMin = std::nullopt,
.j = j_};
TER ret;
if (mptDirect)
{
ret = Payment::makeMPTDirectPayment(paymentParams);
}
else
{
ret = Payment::makeRipplePayment(paymentParams);
}
// Always claim a fee
if (!isTesSuccess(ret) && !isTecClaim(ret))
{
JLOG(j_.info())
<< "LoanBrokerCoverWithdraw: changing result from "
<< transToken(ret)
<< " to tecPATH_DRY for IOU payment with Destination";
return tecPATH_DRY;
}
return ret;
}
// Move the funds directly from the broker's pseudo-account to the dstAcct
return accountSend(
view(), brokerPseudoID, dstAcct, amount, j_, WaiveTransferFee::Yes);
}
//------------------------------------------------------------------------------

View File

@@ -29,8 +29,8 @@ Payment::makeTxConsequences(PreflightContext const& ctx)
}
STAmount
Payment::getMaxSourceAmount(
AccountID const& senderAccount,
getMaxSourceAmount(
AccountID const& account,
STAmount const& dstAmount,
std::optional<STAmount> const& sendMax)
{
@@ -40,7 +40,7 @@ Payment::getMaxSourceAmount(
return dstAmount;
else
return STAmount(
Issue{dstAmount.get<Issue>().currency, senderAccount},
Issue{dstAmount.get<Issue>().currency, account},
dstAmount.mantissa(),
dstAmount.exponent(),
dstAmount < beast::zero);
@@ -422,35 +422,155 @@ Payment::doApply()
if (ripple)
{
return makeRipplePayment(RipplePaymentParams{
.ctx = ctx_,
.maxSourceAmount = maxSourceAmount,
.srcAccountID = account_,
.dstAccountID = dstAccountID,
.sleDst = sleDst,
.dstAmount = dstAmount,
.paths = ctx_.tx.getFieldPathSet(sfPaths),
.deliverMin = deliverMin,
.partialPaymentAllowed = partialPaymentAllowed,
.defaultPathsAllowed = defaultPathsAllowed,
.limitQuality = limitQuality,
.j = j_});
// Ripple payment with at least one intermediate step and uses
// transitive balances.
// An account that requires authorization has two ways to get an
// IOU Payment in:
// 1. If Account == Destination, or
// 2. If Account is deposit preauthorized by destination.
if (auto err = verifyDepositPreauth(
ctx_.tx,
ctx_.view(),
account_,
dstAccountID,
sleDst,
ctx_.journal);
!isTesSuccess(err))
return err;
path::RippleCalc::Input rcInput;
rcInput.partialPaymentAllowed = partialPaymentAllowed;
rcInput.defaultPathsAllowed = defaultPathsAllowed;
rcInput.limitQuality = limitQuality;
rcInput.isLedgerOpen = view().open();
path::RippleCalc::Output rc;
{
PaymentSandbox pv(&view());
JLOG(j_.debug()) << "Entering RippleCalc in payment: "
<< ctx_.tx.getTransactionID();
rc = path::RippleCalc::rippleCalculate(
pv,
maxSourceAmount,
dstAmount,
dstAccountID,
account_,
ctx_.tx.getFieldPathSet(sfPaths),
ctx_.tx[~sfDomainID],
ctx_.app.logs(),
&rcInput);
// VFALCO NOTE We might not need to apply, depending
// on the TER. But always applying *should*
// be safe.
pv.apply(ctx_.rawView());
}
// TODO: is this right? If the amount is the correct amount, was
// the delivered amount previously set?
if (rc.result() == tesSUCCESS && rc.actualAmountOut != dstAmount)
{
if (deliverMin && rc.actualAmountOut < *deliverMin)
rc.setResult(tecPATH_PARTIAL);
else
ctx_.deliver(rc.actualAmountOut);
}
auto terResult = rc.result();
// Because of its overhead, if RippleCalc
// fails with a retry code, claim a fee
// instead. Maybe the user will be more
// careful with their path spec next time.
if (isTerRetry(terResult))
terResult = tecPATH_DRY;
return terResult;
}
else if (mptDirect)
{
return makeMPTDirectPayment(RipplePaymentParams{
.ctx = ctx_,
.maxSourceAmount = maxSourceAmount,
.srcAccountID = account_,
.dstAccountID = dstAccountID,
.sleDst = sleDst,
.dstAmount = dstAmount,
.paths = ctx_.tx.getFieldPathSet(sfPaths),
.deliverMin = deliverMin,
.partialPaymentAllowed = partialPaymentAllowed,
.defaultPathsAllowed = defaultPathsAllowed,
.limitQuality = limitQuality,
.j = j_});
JLOG(j_.trace()) << " dstAmount=" << dstAmount.getFullText();
auto const& mptIssue = dstAmount.get<MPTIssue>();
if (auto const ter = requireAuth(view(), mptIssue, account_);
ter != tesSUCCESS)
return ter;
if (auto const ter = requireAuth(view(), mptIssue, dstAccountID);
ter != tesSUCCESS)
return ter;
if (auto const ter =
canTransfer(view(), mptIssue, account_, dstAccountID);
ter != tesSUCCESS)
return ter;
if (auto err = verifyDepositPreauth(
ctx_.tx,
ctx_.view(),
account_,
dstAccountID,
sleDst,
ctx_.journal);
!isTesSuccess(err))
return err;
auto const& issuer = mptIssue.getIssuer();
// Transfer rate
Rate rate{QUALITY_ONE};
// Payment between the holders
if (account_ != issuer && dstAccountID != issuer)
{
// If globally/individually locked then
// - can't send between holders
// - holder can send back to issuer
// - issuer can send to holder
if (isAnyFrozen(view(), {account_, dstAccountID}, mptIssue))
return tecLOCKED;
// Get the rate for a payment between the holders.
rate = transferRate(view(), mptIssue.getMptID());
}
// Amount to deliver.
STAmount amountDeliver = dstAmount;
// Factor in the transfer rate.
// No rounding. It'll change once MPT integrated into DEX.
STAmount requiredMaxSourceAmount = multiply(dstAmount, rate);
// Send more than the account wants to pay or less than
// the account wants to deliver (if no SendMax).
// Adjust the amount to deliver.
if (partialPaymentAllowed && requiredMaxSourceAmount > maxSourceAmount)
{
requiredMaxSourceAmount = maxSourceAmount;
// No rounding. It'll change once MPT integrated into DEX.
amountDeliver = divide(maxSourceAmount, rate);
}
if (requiredMaxSourceAmount > maxSourceAmount ||
(deliverMin && amountDeliver < *deliverMin))
return tecPATH_PARTIAL;
PaymentSandbox pv(&view());
auto res = accountSend(
pv, account_, dstAccountID, amountDeliver, ctx_.journal);
if (res == tesSUCCESS)
{
pv.apply(ctx_.rawView());
// If the actual amount delivered is different from the original
// amount due to partial payment or transfer fee, we need to update
// DelieveredAmount using the actual delivered amount
if (view().rules().enabled(fixMPTDeliveredAmount) &&
amountDeliver != dstAmount)
ctx_.deliver(amountDeliver);
}
else if (res == tecINSUFFICIENT_FUNDS || res == tecPATH_DRY)
res = tecPATH_PARTIAL;
return res;
}
XRPL_ASSERT(dstAmount.native(), "ripple::Payment::doApply : amount is XRP");
@@ -543,199 +663,4 @@ Payment::doApply()
return tesSUCCESS;
}
// Reusable helpers
TER
Payment::makeRipplePayment(Payment::RipplePaymentParams const& p)
{
// Set up some copies/references so the code can be moved from
// Payment::doApply otherwise unmodified
//
// Note that some of these variable names use trailing '_', which is
// usually reserved for member variables. After, or just before these
// changes are merged, a follow-up can clean up the names for consistency.
ApplyContext& ctx_ = p.ctx;
STAmount const& maxSourceAmount = p.maxSourceAmount;
AccountID const& account_ = p.srcAccountID;
AccountID const& dstAccountID = p.dstAccountID;
SLE::pointer sleDst = p.sleDst;
STAmount const& dstAmount = p.dstAmount;
STPathSet const& paths = p.paths;
std::optional<STAmount> const& deliverMin = p.deliverMin;
bool partialPaymentAllowed = p.partialPaymentAllowed;
bool defaultPathsAllowed = p.defaultPathsAllowed;
bool limitQuality = p.limitQuality;
beast::Journal j_ = p.j;
auto view = [&p]() -> ApplyView& { return p.ctx.view(); };
// Below this line, copied straight from Payment::doApply
// except `ctx_.tx.getFieldPathSet(sfPaths)` replaced with `paths`,
// because not all transactions have that field available, and using
// it will throw
//-------------------------------------------------------
// Ripple payment with at least one intermediate step and uses
// transitive balances.
// An account that requires authorization has two ways to get an
// IOU Payment in:
// 1. If Account == Destination, or
// 2. If Account is deposit preauthorized by destination.
if (auto err = verifyDepositPreauth(
ctx_.tx, ctx_.view(), account_, dstAccountID, sleDst, ctx_.journal);
!isTesSuccess(err))
return err;
path::RippleCalc::Input rcInput;
rcInput.partialPaymentAllowed = partialPaymentAllowed;
rcInput.defaultPathsAllowed = defaultPathsAllowed;
rcInput.limitQuality = limitQuality;
rcInput.isLedgerOpen = view().open();
path::RippleCalc::Output rc;
{
PaymentSandbox pv(&view());
JLOG(j_.debug()) << "Entering RippleCalc in payment: "
<< ctx_.tx.getTransactionID();
rc = path::RippleCalc::rippleCalculate(
pv,
maxSourceAmount,
dstAmount,
dstAccountID,
account_,
paths,
ctx_.tx[~sfDomainID],
ctx_.app.logs(),
&rcInput);
// VFALCO NOTE We might not need to apply, depending
// on the TER. But always applying *should*
// be safe.
pv.apply(ctx_.rawView());
}
// TODO: is this right? If the amount is the correct amount, was
// the delivered amount previously set?
if (rc.result() == tesSUCCESS && rc.actualAmountOut != dstAmount)
{
if (deliverMin && rc.actualAmountOut < *deliverMin)
rc.setResult(tecPATH_PARTIAL);
else
ctx_.deliver(rc.actualAmountOut);
}
auto terResult = rc.result();
// Because of its overhead, if RippleCalc
// fails with a retry code, claim a fee
// instead. Maybe the user will be more
// careful with their path spec next time.
if (isTerRetry(terResult))
terResult = tecPATH_DRY;
return terResult;
}
TER
Payment::makeMPTDirectPayment(Payment::RipplePaymentParams const& p)
{
// Set up some copies/references so the code can be moved from
// Payment::doApply otherwise unmodified
//
// Note that some of these variable names use trailing '_', which is
// usually reserved for member variables. After, or just before these
// changes are merged, a follow-up can clean up the names for consistency.
ApplyContext& ctx_ = p.ctx;
STAmount const& maxSourceAmount = p.maxSourceAmount;
AccountID const& account_ = p.srcAccountID;
AccountID const& dstAccountID = p.dstAccountID;
SLE::pointer sleDst = p.sleDst;
STAmount const& dstAmount = p.dstAmount;
// STPathSet const& paths = p.paths;
std::optional<STAmount> const& deliverMin = p.deliverMin;
bool partialPaymentAllowed = p.partialPaymentAllowed;
// bool defaultPathsAllowed = p.defaultPathsAllowed;
// bool limitQuality = p.limitQuality;
beast::Journal j_ = p.j;
auto view = [&p]() -> ApplyView& { return p.ctx.view(); };
// Below this line, copied straight from Payment::doApply
//-------------------------------------------------------
JLOG(j_.trace()) << " dstAmount=" << dstAmount.getFullText();
auto const& mptIssue = dstAmount.get<MPTIssue>();
if (auto const ter = requireAuth(view(), mptIssue, account_);
ter != tesSUCCESS)
return ter;
if (auto const ter = requireAuth(view(), mptIssue, dstAccountID);
ter != tesSUCCESS)
return ter;
if (auto const ter = canTransfer(view(), mptIssue, account_, dstAccountID);
ter != tesSUCCESS)
return ter;
if (auto err = verifyDepositPreauth(
ctx_.tx, ctx_.view(), account_, dstAccountID, sleDst, ctx_.journal);
!isTesSuccess(err))
return err;
auto const& issuer = mptIssue.getIssuer();
// Transfer rate
Rate rate{QUALITY_ONE};
// Payment between the holders
if (account_ != issuer && dstAccountID != issuer)
{
// If globally/individually locked then
// - can't send between holders
// - holder can send back to issuer
// - issuer can send to holder
if (isAnyFrozen(view(), {account_, dstAccountID}, mptIssue))
return tecLOCKED;
// Get the rate for a payment between the holders.
rate = transferRate(view(), mptIssue.getMptID());
}
// Amount to deliver.
STAmount amountDeliver = dstAmount;
// Factor in the transfer rate.
// No rounding. It'll change once MPT integrated into DEX.
STAmount requiredMaxSourceAmount = multiply(dstAmount, rate);
// Send more than the account wants to pay or less than
// the account wants to deliver (if no SendMax).
// Adjust the amount to deliver.
if (partialPaymentAllowed && requiredMaxSourceAmount > maxSourceAmount)
{
requiredMaxSourceAmount = maxSourceAmount;
// No rounding. It'll change once MPT integrated into DEX.
amountDeliver = divide(maxSourceAmount, rate);
}
if (requiredMaxSourceAmount > maxSourceAmount ||
(deliverMin && amountDeliver < *deliverMin))
return tecPATH_PARTIAL;
PaymentSandbox pv(&view());
auto res =
accountSend(pv, account_, dstAccountID, amountDeliver, ctx_.journal);
if (res == tesSUCCESS)
{
pv.apply(ctx_.rawView());
// If the actual amount delivered is different from the original
// amount due to partial payment or transfer fee, we need to update
// DelieveredAmount using the actual delivered amount
if (view().rules().enabled(fixMPTDeliveredAmount) &&
amountDeliver != dstAmount)
ctx_.deliver(amountDeliver);
}
else if (res == tecINSUFFICIENT_FUNDS || res == tecPATH_DRY)
res = tecPATH_PARTIAL;
return res;
}
} // namespace ripple

View File

@@ -40,37 +40,6 @@ public:
TER
doApply() override;
// Helpers for this class and other transactors that make "Payments"
struct RipplePaymentParams
{
ApplyContext& ctx;
STAmount const& maxSourceAmount;
AccountID const& srcAccountID;
AccountID const& dstAccountID;
SLE::pointer sleDst;
STAmount const& dstAmount;
// Paths need to be explicitly included because other transactions don't
// have them defined
STPathSet const& paths;
std::optional<STAmount> const& deliverMin;
bool partialPaymentAllowed = false;
bool defaultPathsAllowed = true;
bool limitQuality = false;
beast::Journal j;
};
static STAmount
getMaxSourceAmount(
AccountID const& senderAccount,
STAmount const& dstAmount,
std::optional<STAmount> const& sendMax = {});
static TER
makeRipplePayment(RipplePaymentParams const& p);
static TER
makeMPTDirectPayment(RipplePaymentParams const& p);
};
} // namespace ripple