Implicitly authorize Vault and LoanBroker pseudo-accounts (#5976)

- Vault and LoanBroker pseudo-accounts can hold MPTs, regardless of MPTRequireAuth setting.
- Add requireAuth check in LoanBrokerCoverDeposit and LoanPay.
- Fail attempts to unauthorize pseudo-accounts by MPT issuers.
This commit is contained in:
Gregory Tsipenyuk
2025-11-07 00:39:34 -05:00
committed by GitHub
parent ecc429e521
commit ebfca636fc
7 changed files with 234 additions and 11 deletions

View File

@@ -654,14 +654,17 @@ createPseudoAccount(
uint256 const& pseudoOwnerKey,
SField const& ownerField);
// Returns true iff sleAcct is a pseudo-account.
// Returns true iff sleAcct is a pseudo-account or specific
// pseudo-accounts in pseudoFieldFilter.
//
// Returns false if sleAcct is
// * NOT a pseudo-account OR
// * NOT a ltACCOUNT_ROOT OR
// * null pointer
[[nodiscard]] bool
isPseudoAccount(std::shared_ptr<SLE const> sleAcct);
isPseudoAccount(
std::shared_ptr<SLE const> sleAcct,
std::set<SField const*> const& pseudoFieldFilter = {});
// Returns the list of fields that define an ACCOUNT_ROOT as a pseudo-account if
// set
@@ -675,9 +678,13 @@ isPseudoAccount(std::shared_ptr<SLE const> sleAcct);
getPseudoAccountFields();
[[nodiscard]] inline bool
isPseudoAccount(ReadView const& view, AccountID const& accountId)
isPseudoAccount(
ReadView const& view,
AccountID const& accountId,
std::set<SField const*> const& pseudoFieldFilter = {})
{
return isPseudoAccount(view.read(keylet::account(accountId)));
return isPseudoAccount(
view.read(keylet::account(accountId)), pseudoFieldFilter);
}
[[nodiscard]] TER
@@ -1001,7 +1008,8 @@ requireAuth(
* purely defensive, as we currently do not allow such vaults to be created.
*
* If StrongAuth then return tecNO_AUTH if MPToken doesn't exist or
* lsfMPTRequireAuth is set and MPToken is not authorized.
* lsfMPTRequireAuth is set and MPToken is not authorized. Vault and LoanBroker
* pseudo-accounts are implicitly authorized.
*
* If WeakAuth then return tecNO_AUTH if lsfMPTRequireAuth is set and MPToken
* doesn't exist or is not authorized (explicitly or via credentials, if

View File

@@ -1204,7 +1204,8 @@ getPseudoAccountFields()
{
// LCOV_EXCL_START
LogicError(
"ripple::isPseudoAccount : unable to find account root ledger "
"ripple::getPseudoAccountFields : unable to find account root "
"ledger "
"format");
// LCOV_EXCL_STOP
}
@@ -1222,7 +1223,9 @@ getPseudoAccountFields()
}
[[nodiscard]] bool
isPseudoAccount(std::shared_ptr<SLE const> sleAcct)
isPseudoAccount(
std::shared_ptr<SLE const> sleAcct,
std::set<SField const*> const& pseudoFieldFilter)
{
auto const& fields = getPseudoAccountFields();
@@ -1230,8 +1233,12 @@ isPseudoAccount(std::shared_ptr<SLE const> sleAcct)
// semantics of true return value clean.
return sleAcct && sleAcct->getType() == ltACCOUNT_ROOT &&
std::count_if(
fields.begin(), fields.end(), [&sleAcct](SField const* sf) -> bool {
return sleAcct->isFieldPresent(*sf);
fields.begin(),
fields.end(),
[&sleAcct, &pseudoFieldFilter](SField const* sf) -> bool {
return sleAcct->isFieldPresent(*sf) &&
(pseudoFieldFilter.empty() ||
pseudoFieldFilter.contains(sf));
}) > 0;
}
@@ -3099,7 +3106,10 @@ requireAuth(
if (mptIssuer == account) // Issuer won't have MPToken
return tesSUCCESS;
if (view.rules().enabled(featureSingleAssetVault))
bool const featureSAVEnabled =
view.rules().enabled(featureSingleAssetVault);
if (featureSAVEnabled)
{
if (depth >= maxAssetCheckDepth)
return tecINTERNAL; // LCOV_EXCL_LINE
@@ -3158,6 +3168,13 @@ requireAuth(
// belong to someone who is explicitly authorized e.g. a vault owner.
}
if (featureSAVEnabled)
{
// Implicitly authorize Vault and LoanBroker pseudo-accounts
if (isPseudoAccount(view, account, {&sfVaultID, &sfLoanBrokerID}))
return tesSUCCESS;
}
// mptoken must be authorized if issuance enabled requireAuth
if (sleIssuance->isFlag(lsfMPTRequireAuth) &&
(!sleToken || !sleToken->isFlag(lsfMPTAuthorized)))

View File

@@ -1265,6 +1265,126 @@ class LoanBroker_test : public beast::unit_test::suite
(void)LoanBrokerCoverDeposit::preclaim(pctx);
}
void
testRequireAuth()
{
testcase("Require Auth - Implicit Pseudo-account authorization");
using namespace jtx;
using namespace loanBroker;
Account const issuer{"issuer"};
Account const alice{"alice"};
Env env(*this);
Vault vault{env};
env.fund(XRP(100'000), issuer, alice);
env.close();
auto asset = MPTTester({
.env = env,
.issuer = issuer,
.holders = {alice},
.flags = MPTDEXFlags | tfMPTRequireAuth | tfMPTCanClawback |
tfMPTCanLock,
.authHolder = true,
});
env(pay(issuer, alice, asset(100'000)));
env.close();
// Alice is not authorized, can still create the vault
asset.authorize(
{.account = issuer, .holder = alice, .flags = tfMPTUnauthorize});
auto [tx, vaultKeylet] = vault.create({.owner = alice, .asset = asset});
env(tx);
env.close();
auto const le = env.le(vaultKeylet);
VaultInfo vaultInfo = [&]() {
if (BEAST_EXPECT(le))
return VaultInfo{asset, vaultKeylet.key, le->at(sfAccount)};
return VaultInfo{asset, {}, {}};
}();
if (vaultInfo.vaultID == uint256{})
return;
// Can't unauthorize Vault pseudo-account
asset.authorize(
{.account = issuer,
.holder = vaultInfo.pseudoAccount,
.flags = tfMPTUnauthorize,
.err = tecNO_PERMISSION});
auto forUnauthAuth = [&](auto&& doTx) {
for (auto const flag : {tfMPTUnauthorize, 0u})
{
asset.authorize(
{.account = issuer, .holder = alice, .flags = flag});
env.close();
doTx(flag == 0);
env.close();
}
};
// Can't deposit into Vault if the vault owner is not authorized
forUnauthAuth([&](bool authorized) {
auto const err = !authorized ? ter(tecNO_AUTH) : ter(tesSUCCESS);
env(vault.deposit(
{.depositor = alice,
.id = vaultKeylet.key,
.amount = asset(51)}),
err);
});
// Can't withdraw from Vault if the vault owner is not authorized
forUnauthAuth([&](bool authorized) {
auto const err = !authorized ? ter(tecNO_AUTH) : ter(tesSUCCESS);
env(vault.withdraw(
{.depositor = alice,
.id = vaultKeylet.key,
.amount = asset(1)}),
err);
});
auto const brokerKeylet =
keylet::loanbroker(alice.id(), env.seq(alice));
// Can create LoanBroker if the vault owner is not authorized
forUnauthAuth([&](auto) { env(set(alice, vaultInfo.vaultID)); });
auto const broker = env.le(brokerKeylet);
if (!BEAST_EXPECT(broker))
return;
Account brokerPseudo("pseudo", broker->at(sfAccount));
// Can't unauthorize LoanBroker pseudo-account
asset.authorize(
{.account = issuer,
.holder = brokerPseudo,
.flags = tfMPTUnauthorize,
.err = tecNO_PERMISSION});
// Can't cover deposit into Vault if the vault owner is not authorized
forUnauthAuth([&](bool authorized) {
auto const err = !authorized ? ter(tecNO_AUTH) : ter(tesSUCCESS);
env(coverDeposit(alice, brokerKeylet.key, vaultInfo.asset(10)),
err);
});
// Can't cover withdraw from Vault if the vault owner is not authorized
forUnauthAuth([&](bool authorized) {
auto const err = !authorized ? ter(tecNO_AUTH) : ter(tesSUCCESS);
env(coverWithdraw(alice, brokerKeylet.key, vaultInfo.asset(5)),
err);
});
// Issuer can always cover clawback. The holder authorization is n/a.
forUnauthAuth([&](bool) {
env(coverClawback(issuer),
loanBrokerID(brokerKeylet.key),
amount(vaultInfo.asset(1)));
});
}
public:
void
run() override
@@ -1278,6 +1398,7 @@ public:
testInvalidLoanBrokerCoverWithdraw();
testInvalidLoanBrokerDelete();
testInvalidLoanBrokerSet();
testRequireAuth();
// TODO: Write clawback failure tests with an issuer / MPT that doesn't
// have the right flags set.

View File

@@ -5333,6 +5333,65 @@ protected:
}
}
void
testRequireAuth()
{
testcase("Require Auth - Implicit Pseudo-account authorization");
using namespace jtx;
using namespace loan;
Account const lender{"lender"};
Account const issuer{"issuer"};
Account const borrower{"borrower"};
Env env(*this);
env.fund(XRP(100'000), issuer, lender, borrower);
env.close();
auto asset = MPTTester({
.env = env,
.issuer = issuer,
.holders = {lender, borrower},
.flags = MPTDEXFlags | tfMPTRequireAuth | tfMPTCanClawback |
tfMPTCanLock,
.authHolder = true,
});
env(pay(issuer, lender, asset(5'000'000)));
BrokerInfo brokerInfo{createVaultAndBroker(env, asset, lender)};
auto const loanSetFee = fee(env.current()->fees().base * 2);
STAmount const debtMaximumRequest = brokerInfo.asset(1'000).value();
auto forUnauthAuth = [&](auto&& doTx) {
for (auto const flag : {tfMPTUnauthorize, 0u})
{
asset.authorize(
{.account = issuer, .holder = borrower, .flags = flag});
env.close();
doTx(flag == 0);
env.close();
}
};
// Can't create a loan if the borrower is not authorized
forUnauthAuth([&](bool authorized) {
auto const err = !authorized ? ter(tecNO_AUTH) : ter(tesSUCCESS);
env(set(borrower, brokerInfo.brokerID, debtMaximumRequest),
sig(sfCounterpartySignature, lender),
loanSetFee,
err);
});
std::uint32_t constexpr loanSequence = 1;
auto const loanKeylet = keylet::loan(brokerInfo.brokerID, loanSequence);
// Can't loan pay if the borrower is not authorized
forUnauthAuth([&](bool authorized) {
auto const err = !authorized ? ter(tecNO_AUTH) : ter(tesSUCCESS);
env(pay(borrower, loanKeylet.key, debtMaximumRequest), err);
});
}
#if LOANTODO
void
testCoverDepositAllowsNonTransferableMPT()
@@ -6193,6 +6252,8 @@ public:
testLoanPayComputePeriodicPaymentValidTotalPrincipalPaidInvariant();
testLoanPayComputePeriodicPaymentValidTotalInterestPaidInvariant();
testLoanNextPaymentDueDateOverflow();
testRequireAuth();
}
};

View File

@@ -65,6 +65,10 @@ LoanBrokerCoverDeposit::preclaim(PreclaimContext const& ctx)
// Pseudo-account cannot receive if asset is deep frozen
if (auto const ret = checkDeepFrozen(ctx.view, pseudoAccountID, vaultAsset))
return ret;
// Cannot transfer unauthorized asset
if (auto const ret =
requireAuth(ctx.view, vaultAsset, account, AuthType::StrongAuth))
return ret;
if (accountHolds(
ctx.view,

View File

@@ -187,6 +187,11 @@ LoanPay::preclaim(PreclaimContext const& ctx)
<< "Vault pseudo-account can not receive funds (deep frozen).";
return ret;
}
if (auto const ret = requireAuth(ctx.view, asset, account))
{
JLOG(ctx.j.warn()) << "Borrower account is not authorized.";
return ret;
}
// Make sure the borrower has enough funds to make the payment!
// Do not support "partial payments" - if the transaction says to pay X,
// then the account must have X available, even if the loan payment takes

View File

@@ -94,7 +94,8 @@ MPTokenAuthorize::preclaim(PreclaimContext const& ctx)
return tesSUCCESS;
}
if (!ctx.view.exists(keylet::account(*holderID)))
auto const sleHolder = ctx.view.read(keylet::account(*holderID));
if (!sleHolder)
return tecNO_DST;
auto const sleMptIssuance =
@@ -124,6 +125,12 @@ MPTokenAuthorize::preclaim(PreclaimContext const& ctx)
keylet::mptoken(ctx.tx[sfMPTokenIssuanceID], *holderID)))
return tecOBJECT_NOT_FOUND;
// Can't unauthorize the pseudo-accounts because they are implicitly
// always authorized. No need to amendment gate since Vault and LoanBroker
// can only be created if the Vault amendment is enabled.
if (isPseudoAccount(ctx.view, *holderID, {&sfVaultID, &sfLoanBrokerID}))
return tecNO_PERMISSION;
return tesSUCCESS;
}