ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-11-13 07:35:57 +00:00
Code Issues Packages Projects Releases Wiki Activity

Start implementing LoanBrokerCoverWithdraw.Destination field

Browse Source 
- Refactor the IOU payment code out of the Payment transactor and use it
  for different destinations. This should enforce all of the trust line
  rules without having to reinvent a dozen wheels.
- TODO: Same for MPTs.
This commit is contained in:
Ed Hennis
2025-07-15 19:30:58 -04:00
parent 38cb371c72
commit b34f59eafc
7 changed files with 232 additions and 95 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
include/xrpl/protocol/detail/transactions.macro
View File

@@ -802,6 +802,7 @@ TRANSACTION(ttVAULT_WITHDRAW, 69, VaultWithdraw,
{sfVaultID, soeREQUIRED},
{sfAmount, soeREQUIRED, soeMPTSupported},
{sfDestination, soeOPTIONAL},
{sfDestinationTag, soeOPTIONAL},
}))
/** This transaction claws back tokens from a vault. */
@@ -875,6 +876,8 @@ TRANSACTION(ttLOAN_BROKER_COVER_WITHDRAW, 77, LoanBrokerCoverWithdraw,
noPriv, ({
{sfLoanBrokerID, soeREQUIRED},
{sfAmount, soeREQUIRED, soeMPTSupported},
{sfDestination, soeOPTIONAL},
{sfDestinationTag, soeOPTIONAL},
}))
/** This transaction creates a Loan */

19
src/test/app/LoanBroker_test.cpp
View File

@@ -107,6 +107,7 @@ class LoanBroker_test : public beast::unit_test::suite
jtx::Env& env,
jtx::Account const& alice,
jtx::Account const& evan,
jtx::Account const& bystander,
VaultInfo const& vault,
std::function<jtx::JTx(jtx::JTx const&)> modifyJTx,
std::function<void(SLE::const_ref)> checkBroker,
@@ -240,6 +241,11 @@ class LoanBroker_test : public beast::unit_test::suite
env(coverWithdraw(alice, keylet.key, vault.asset(900)),
ter(tecINSUFFICIENT_FUNDS));
env(coverWithdraw(alice, keylet.key, vault.asset(1)),
destination(bystander),
ter(tecNO_LINE));
verifyCoverAmount(10);
// Withdraw some of the cover amount
env(coverWithdraw(alice, keylet.key, vault.asset(7)));
if (BEAST_EXPECT(broker = env.le(keylet)))
@@ -254,8 +260,10 @@ class LoanBroker_test : public beast::unit_test::suite
verifyCoverAmount(8);
}
// Withdraw some more
env(coverWithdraw(alice, keylet.key, vault.asset(2)));
// Withdraw some more. Send it to Evan. Very generous, considering
// how much trouble he's been.
env(coverWithdraw(alice, keylet.key, vault.asset(2)),
destination(evan));
if (BEAST_EXPECT(broker = env.le(keylet)))
{
verifyCoverAmount(6);
@@ -348,11 +356,14 @@ class LoanBroker_test : public beast::unit_test::suite
Account alice{"alice"};
// Evan will attempt to be naughty
Account evan{"evan"};
// Bystander doesn't have anything to do with the SAV or Broker, or any
// of the relevant tokens
Account bystander{"bystander"};
Vault vault{env};
// Fund the accounts and trust lines with the same amount so that tests
// can use the same values regardless of the asset.
env.fund(XRP(100'000), issuer, noripple(alice, evan));
env.fund(XRP(100'000), issuer, noripple(alice, evan, bystander));
env.close();
// Create assets
@@ -459,6 +470,7 @@ class LoanBroker_test : public beast::unit_test::suite
env,
alice,
evan,
bystander,
vault,
// No modifications
{},
@@ -543,6 +555,7 @@ class LoanBroker_test : public beast::unit_test::suite
env,
alice,
evan,
bystander,
vault,
[&](jtx::JTx const& jv) {
testData = "spam spam spam spam";

2
src/test/jtx/TestHelpers.h
View File

@@ -752,6 +752,8 @@ auto const coverRateMinimum =
auto const coverRateLiquidation =
valueUnitWrapper<SF_UINT32, unit::TenthBipsTag>(sfCoverRateLiquidation);
auto const destination = JTxFieldWrapper<accountIDField>(sfDestination);
} // namespace loanBroker
/* Loan */

109
src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp
View File

@@ -20,6 +20,7 @@
#include <xrpld/app/tx/detail/LoanBrokerCoverWithdraw.h>
//
#include <xrpld/app/misc/LendingHelpers.h>
#include <xrpld/app/tx/detail/Payment.h>
#include <xrpld/ledger/ApplyView.h>
#include <xrpld/ledger/View.h>
@@ -57,6 +58,14 @@ LoanBrokerCoverWithdraw::preflight(PreflightContext const& ctx)
if (ctx.tx[sfAmount] <= beast::zero)
return temBAD_AMOUNT;
if (auto const destination = ctx.tx[~sfDestination];
destination && *destination == beast::zero)
{
JLOG(ctx.j.debug())
<< "LoanBrokerCoverWithdraw: zero/empty destination account.";
return temMALFORMED;
}
return tesSUCCESS;
}
@@ -69,6 +78,12 @@ LoanBrokerCoverWithdraw::preclaim(PreclaimContext const& ctx)
auto const brokerID = tx[sfLoanBrokerID];
auto const amount = tx[sfAmount];
auto const dstAcct = [&]() -> AccountID {
if (auto const dst = tx[~sfDestination])
return *dst;
return account;
}();
auto const sleBroker = ctx.view.read(keylet::loanbroker(brokerID));
if (!sleBroker)
{
@@ -86,15 +101,45 @@ LoanBrokerCoverWithdraw::preclaim(PreclaimContext const& ctx)
if (amount.asset() != vaultAsset)
return tecWRONG_ASSET;
// Cannot transfer a frozen Asset
// Withdrawal to a 3rd party destination account is essentially a transfer.
// Enforce all the usual asset transfer checks.
AuthType authType = AuthType::Legacy;
if (account != dstAcct)
{
auto const sleDst = ctx.view.read(keylet::account(dstAcct));
if (sleDst == nullptr)
return tecNO_DST;
if (sleDst->isFlag(lsfRequireDestTag) &&
!tx.isFieldPresent(sfDestinationTag))
return tecDST_TAG_NEEDED; // Cannot send without a tag
if (sleDst->isFlag(lsfDepositAuth))
{
if (!ctx.view.exists(keylet::depositPreauth(dstAcct, account)))
return tecNO_PERMISSION;
}
// The destination account must have consented to receive the asset by
// creating a RippleState or MPToken
authType = AuthType::StrongAuth;
}
// Destination MPToken must exist (if asset is an MPT)
if (auto const ter = requireAuth(ctx.view, vaultAsset, dstAcct))
return ter;
// The broker's pseudo-account is the source of funds.
auto const pseudoAccountID = sleBroker->at(sfAccount);
// Cannot transfer a frozen Asset
// Cannot send a frozen Asset
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;
if (dstAcct != vaultAsset.getIssuer())
{
// 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
@@ -128,28 +173,56 @@ LoanBrokerCoverWithdraw::doApply()
auto const brokerID = tx[sfLoanBrokerID];
auto const amount = tx[sfAmount];
auto const dstAcct = [&]() -> AccountID {
if (auto const dst = tx[~sfDestination])
return *dst;
return account_;
}();
auto broker = view().peek(keylet::loanbroker(brokerID));
if (!broker)
return tefBAD_LEDGER; // LCOV_EXCL_LINE
return tecINTERNAL; // LCOV_EXCL_LINE
auto const brokerPseudoID = broker->at(sfAccount);
// Transfer assets from pseudo-account to depositor.
if (auto ter = accountSend(
view(),
brokerPseudoID,
account_,
amount,
j_,
WaiveTransferFee::Yes))
return ter;
// Increase the LoanBroker's CoverAvailable by Amount
// Decrease the LoanBroker's CoverAvailable by Amount
broker->at(sfCoverAvailable) -= amount;
view().update(broker);
return tesSUCCESS;
if (dstAcct != account_ && !amount.native() && !amount.holds<MPTIssue>())
{
STAmount const maxSourceAmount(
Issue{amount.get<Issue>().currency, brokerPseudoID},
amount.mantissa(),
amount.exponent(),
amount < beast::zero);
SLE::pointer sleDst = view().peek(keylet::account(dstAcct));
auto ret = Payment::makeRipplePayment(Payment::RipplePaymentParams{
.ctx = ctx_,
.maxSourceAmount = maxSourceAmount,
.srcAccountID = account_,
.dstAccountID = dstAcct,
.sleDst = sleDst,
.dstAmount = amount,
.deliverMin = std::nullopt,
.j = j_});
// 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";
ret = tecPATH_DRY;
}
return ret;
}
// Transfer assets from pseudo-account to depositor.
return accountSend(
view(), brokerPseudoID, dstAcct, amount, j_, WaiveTransferFee::Yes);
}
//------------------------------------------------------------------------------

156
src/xrpld/app/tx/detail/Payment.cpp
View File

@@ -447,73 +447,18 @@ Payment::doApply()
if (ripple)
{
// Ripple payment with at least one intermediate step and uses
// transitive balances.
if (depositPreauth && depositAuth)
{
// If depositPreauth is enabled, then 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;
return makeRipplePayment(RipplePaymentParams{
.ctx = ctx_,
.maxSourceAmount = maxSourceAmount,
.srcAccountID = account_,
.dstAccountID = dstAccountID,
.sleDst = sleDst,
.dstAmount = dstAmount,
.deliverMin = deliverMin,
.partialPaymentAllowed = partialPaymentAllowed,
.defaultPathsAllowed = defaultPathsAllowed,
.limitQuality = limitQuality,
.j = j_});
}
else if (mptDirect)
{
@@ -685,4 +630,81 @@ Payment::doApply()
return tesSUCCESS;
}
// Reusable helpers
TER
Payment::makeRipplePayment(Payment::RipplePaymentParams const& p)
{
// Ripple payment with at least one intermediate step and uses
// transitive balances.
bool const depositPreauth =
p.ctx.view().rules().enabled(featureDepositPreauth);
bool const depositAuth = p.ctx.view().rules().enabled(featureDepositAuth);
if (depositPreauth && depositAuth)
{
// If depositPreauth is enabled, then 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(
p.ctx.tx,
p.ctx.view(),
p.srcAccountID,
p.dstAccountID,
p.sleDst,
p.ctx.journal);
!isTesSuccess(err))
return err;
}
path::RippleCalc::Input rcInput;
rcInput.partialPaymentAllowed = p.partialPaymentAllowed;
rcInput.defaultPathsAllowed = p.defaultPathsAllowed;
rcInput.limitQuality = p.limitQuality;
rcInput.isLedgerOpen = p.ctx.view().open();
path::RippleCalc::Output rc;
{
PaymentSandbox pv(&p.ctx.view());
JLOG(p.j.debug()) << "Entering RippleCalc in payment: "
<< p.ctx.tx.getTransactionID();
rc = path::RippleCalc::rippleCalculate(
pv,
p.maxSourceAmount,
p.dstAmount,
p.dstAccountID,
p.srcAccountID,
p.ctx.tx.getFieldPathSet(sfPaths),
p.ctx.tx[~sfDomainID],
p.ctx.app.logs(),
&rcInput);
// VFALCO NOTE We might not need to apply, depending
// on the TER. But always applying *should*
// be safe.
pv.apply(p.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 != p.dstAmount)
{
if (p.deliverMin && rc.actualAmountOut < *p.deliverMin)
rc.setResult(tecPATH_PARTIAL);
else
p.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;
}
} // namespace ripple

19
src/xrpld/app/tx/detail/Payment.h
View File

@@ -59,6 +59,25 @@ 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;
std::optional<STAmount> const& deliverMin;
bool partialPaymentAllowed = false;
bool defaultPathsAllowed = true;
bool limitQuality = false;
beast::Journal j;
};
static TER
makeRipplePayment(RipplePaymentParams const& p);
};
} // namespace ripple

19
src/xrpld/app/tx/detail/VaultWithdraw.cpp
View File

@@ -120,33 +120,38 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx)
// Withdrawal to a 3rd party destination account is essentially a transfer,
// via shares in the vault. Enforce all the usual asset transfer checks.
AuthType authType = AuthType::Legacy;
if (account != dstAcct)
{
auto const sleDst = ctx.view.read(keylet::account(dstAcct));
if (sleDst == nullptr)
return tecNO_DST;
if (sleDst->getFlags() & lsfRequireDestTag)
if (sleDst->isFlag(lsfRequireDestTag) &&
!ctx.tx.isFieldPresent(sfDestinationTag))
return tecDST_TAG_NEEDED; // Cannot send without a tag
if (sleDst->getFlags() & lsfDepositAuth)
if (sleDst->isFlag(lsfDepositAuth))
{
if (!ctx.view.exists(keylet::depositPreauth(dstAcct, account)))
return tecNO_PERMISSION;
}
// The destination account must have consented to receive the asset by
// creating a RippleState or MPToken
authType = AuthType::StrongAuth;
}
// Destination MPToken must exist (if asset is an MPT)
if (auto const ter = requireAuth(ctx.view, vaultAsset, dstAcct);
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 (isFrozen(ctx.view, dstAcct, vaultAsset))
return vaultAsset.holds<Issue>() ? tecFROZEN : tecLOCKED;
if (auto const ret = checkFrozen(ctx.view, dstAcct, vaultAsset))
return ret;
if (isFrozen(ctx.view, account, vaultShare))
return tecLOCKED;
if (auto const ret = checkFrozen(ctx.view, account, vaultShare))
return ret;
return tesSUCCESS;
}