ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-09-26 23:19:07 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: Relax MPT authorize cap for LoanSet and VaultWithdraw

Browse Source
This commit is contained in:
Vito Tumas
2026-09-11 16:59:08 +02:00
committed by Bart
parent a0c12420b5
commit f6c80fef68
5 changed files with 294 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

59
src/libxrpl/tx/invariants/MPTInvariant.cpp
View File

@@ -299,27 +299,46 @@ ValidMPTIssuance::finalize(
return false;
}
}
else if (lendingProtocolEnabled && (mptokensCreated_ + mptokensDeleted_) > 1)
else
{
JLOG(j.fatal()) << "Invariant failed: MPT authorize succeeded "
"but created/deleted bad number mptokens";
return false;
}
else if (submittedByIssuer && (mptokensCreated_ > 0 || mptokensDeleted_ > 0))
{
JLOG(j.fatal()) << "Invariant failed: MPT authorize submitted by issuer "
"succeeded but created/deleted mptokens";
return false;
}
else if (
!submittedByIssuer && hasPrivilege(tx, Privilege::MustAuthorizeMpt) &&
(mptokensCreated_ + mptokensDeleted_ != 1))
{
// if the holder submitted this tx, then a mptoken must be
// either created or deleted.
JLOG(j.fatal()) << "Invariant failed: MPT authorize submitted by holder "
"succeeded but created/deleted bad number of mptokens";
return false;
// Cap on MPToken creates and deletes while featureLendingProtocol is enabled.
// - LoanSet: at most two creates and no deletes.
// - VaultWithdraw: at most one create and one delete.
// - Other MayAuthorizeMpt types: created + deleted <= 1.
// - MustAuthorizeMpt still requires exactly one create or delete below.
auto const mptokensExceedAuthorizeCap = [&] {
if (!lendingProtocolEnabled)
return false;
if (rules.enabled(fixCleanup3_4_0))
{
if (txnType == ttLOAN_SET)
return mptokensDeleted_ != 0 || mptokensCreated_ > 2;
if (txnType == ttVAULT_WITHDRAW)
return mptokensCreated_ > 1 || mptokensDeleted_ > 1;
}
return (mptokensCreated_ + mptokensDeleted_) > 1;
};
if (mptokensExceedAuthorizeCap())
{
JLOG(j.fatal()) << "Invariant failed: MPT authorize succeeded "
"but created/deleted bad number mptokens";
return false;
}
if (submittedByIssuer && (mptokensCreated_ > 0 || mptokensDeleted_ > 0))
{
JLOG(j.fatal()) << "Invariant failed: MPT authorize submitted by issuer "
"succeeded but created/deleted mptokens";
return false;
}
if (!submittedByIssuer && hasPrivilege(tx, Privilege::MustAuthorizeMpt) &&
(mptokensCreated_ + mptokensDeleted_ != 1))
{
// if the holder submitted this tx, then a mptoken must be
// either created or deleted.
JLOG(j.fatal()) << "Invariant failed: MPT authorize submitted by holder "
"succeeded but created/deleted bad number of mptokens";
return false;
}
}
return true;

69
src/test/app/invariants/InvariantsMPT_test.cpp
View File

@@ -967,6 +967,75 @@ class InvariantsMPT_test : public InvariantsBase
});
}
// LoanSet / VaultWithdraw MayAuthorizeMpt caps (fixCleanup3_4_0):
// LoanSet allows at most two creates and no deletes; VaultWithdraw
// allows at most one of each. Fabricate one extra mutation so a
// too-loose cap would miss these.
{
auto const insertHolderTokens =
[](Account const& issuer, Account const& holder, ApplyContext& ac, int n) {
auto const sle = ac.view().peek(keylet::account(issuer.id()));
if (!sle)
return false;
auto seq = sle->getFieldU32(sfSequence);
for (int i = 0; i < n; ++i)
{
MPTIssue const mpt{makeMptID(seq + i, issuer)};
auto sleNew =
std::make_shared<SLE>(keylet::mptoken(mpt.getMptID(), holder));
(*sleNew)[sfAccount] = holder.id();
(*sleNew)[sfMPTokenIssuanceID] = mpt.getMptID();
ac.view().insert(sleNew);
}
return true;
};
std::array<std::pair<xrpl::TxType, std::uint8_t>, 2> const createOverCap{
{{ttLOAN_SET, 3}, {ttVAULT_WITHDRAW, 2}}};
for (auto const& [txnType, nTokens] : createOverCap)
{
doInvariantCheck(
{{"MPT authorize succeeded but created/deleted bad number mptokens"}},
[&](Account const& a1, Account const& a2, ApplyContext& ac) {
return insertHolderTokens(a1, a2, ac, nTokens);
},
XRPAmount{},
STTx{txnType, [](STObject&) {}},
{tecINVARIANT_FAILED, tefINVARIANT_FAILED});
}
MPTID id;
auto const precloseTwoHolders = [&id](Account const& a1, Account const& a2, Env& env) {
Account const gw("gw");
env.fund(XRP(1'000), gw);
MPTTester const mpt({.env = env, .issuer = gw, .holders = {a1, a2}});
id = mpt.issuanceID();
return true;
};
std::array<std::pair<xrpl::TxType, std::uint8_t>, 2> const deleteOverCap{
{{ttLOAN_SET, 1}, {ttVAULT_WITHDRAW, 2}}};
for (auto const& [txnType, nTokens] : deleteOverCap)
{
doInvariantCheck(
{{"MPT authorize succeeded but created/deleted bad number mptokens"}},
[&](Account const& a1, Account const& a2, ApplyContext& ac) {
std::array const holders{a1, a2};
for (int i = 0; i < nTokens; ++i)
{
auto sle = ac.view().peek(keylet::mptoken(id, holders[i]));
if (!sle)
return false;
ac.view().erase(sle);
}
return true;
},
XRPAmount{},
STTx{txnType, [](STObject&) {}},
{tecINVARIANT_FAILED, tefINVARIANT_FAILED},
precloseTwoHolders);
}
}
// sfReferenceHolding can only be set on creation by VaultCreate. A
// non-VaultCreate transaction that creates an MPTokenIssuance with
// sfReferenceHolding present must trip the invariant.

102
src/test/app/lending/LoanSet_test.cpp
View File

@@ -22,6 +22,7 @@
#include <xrpl/protocol/LedgerFormats.h>
#include <xrpl/protocol/Protocol.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/SeqProxy.h>
#include <xrpl/protocol/TER.h>
#include <xrpl/protocol/TxFlags.h>
#include <xrpl/protocol/Units.h>
@@ -597,6 +598,105 @@ private:
nullptr);
}
void
testLoanSetOriginationFeeTwoMptCreates(FeatureBitset features)
{
using namespace jtx;
using namespace loan;
bool const fix340Enabled = features[fixCleanup3_4_0];
testcase << "LoanSet: borrower and broker owner missing MPToken"
<< (fix340Enabled ? "" : " pre-fixCleanup3_4_0");
Account const issuer{"issuer"};
Account const lender{"lender"};
Account const borrower{"borrower"};
Env env(*this, features);
env.fund(XRP(1'000'000), issuer, lender, borrower);
env.close();
MPTTester mptt{env, issuer, kMptInitNoFund};
mptt.create({.flags = tfMPTCanTransfer | tfMPTCanLock});
env.close();
PrettyAsset const asset = mptt.issuanceID();
mptt.authorize({.account = lender});
mptt.authorize({.account = borrower});
env.close();
env(pay(issuer, lender, asset(10'000'000)));
env.close();
auto const broker = createVaultAndBroker(env, asset, lender);
// Delete borrower's asset MPToken.
mptt.authorize({.account = borrower, .flags = tfMPTUnauthorize});
env.close();
// Pay out and delete the broker owner's asset MPToken.
auto const lenderMPToken = keylet::mptoken(mptt.issuanceID(), lender);
auto const sleLenderMPT = env.le(lenderMPToken);
if (!BEAST_EXPECT(sleLenderMPT))
return;
env(pay(lender, issuer, asset(sleLenderMPT->at(sfMPTAmount))));
env.close();
mptt.authorize({.account = lender, .flags = tfMPTUnauthorize});
env.close();
auto const borrowerMPToken = keylet::mptoken(mptt.issuanceID(), borrower);
auto const brokerKeylet = keylet::loanBroker(broker.brokerID);
auto const sleBrokerBefore = env.le(brokerKeylet);
if (!BEAST_EXPECT(sleBrokerBefore))
return;
auto const loanSequence = sleBrokerBefore->at(sfLoanSequence);
auto const debtTotalBefore = sleBrokerBefore->at(sfDebtTotal);
auto const loanKeylet = keylet::loan(broker.brokerID, SeqProxy::rawSequence(loanSequence));
auto const sleVaultBefore = env.le(keylet::vault(broker.vaultID));
if (!BEAST_EXPECT(sleVaultBefore))
return;
auto const assetsAvailableBefore = sleVaultBefore->at(sfAssetsAvailable);
env(set(borrower, broker.brokerID, asset(1'000).value()),
kLoanOriginationFee(asset(1).value()),
kCounterparty(lender),
Sig(sfCounterpartySignature, lender),
Fee(env.current()->fees().base * 5),
Ter{fix340Enabled ? TER{tesSUCCESS} : TER{tecINVARIANT_FAILED}});
env.close();
auto const sleBorrowerAfter = env.le(borrowerMPToken);
auto const sleLenderAfter = env.le(lenderMPToken);
auto const sleLoanAfter = env.le(loanKeylet);
auto const sleBrokerAfter = env.le(brokerKeylet);
auto const sleVaultAfter = env.le(keylet::vault(broker.vaultID));
if (!BEAST_EXPECT(sleVaultAfter))
return;
if (fix340Enabled)
{
if (!BEAST_EXPECT(sleBorrowerAfter && sleLenderAfter && sleLoanAfter && sleBrokerAfter))
return;
BEAST_EXPECT(sleBorrowerAfter->at(sfMPTAmount) == 999);
BEAST_EXPECT(sleLenderAfter->at(sfMPTAmount) == 1);
BEAST_EXPECT(sleLoanAfter->at(sfPrincipalOutstanding) == Number{1'000});
BEAST_EXPECT(sleBrokerAfter->at(sfLoanSequence) == loanSequence + 1);
BEAST_EXPECT(
sleVaultAfter->at(sfAssetsAvailable) == assetsAvailableBefore - Number{1'000});
}
else
{
// The whole transaction must roll back.
BEAST_EXPECT(!sleBorrowerAfter);
BEAST_EXPECT(!sleLenderAfter);
BEAST_EXPECT(!sleLoanAfter);
if (!BEAST_EXPECT(sleBrokerAfter))
return;
BEAST_EXPECT(sleBrokerAfter->at(sfLoanSequence) == loanSequence);
BEAST_EXPECT(sleBrokerAfter->at(sfDebtTotal) == debtTotalBefore);
BEAST_EXPECT(sleVaultAfter->at(sfAssetsAvailable) == assetsAvailableBefore);
}
}
// LoanSet in a closed-ended vault — phase gating and maturity bound.
void
testLoanSetClosedEnded()
@@ -838,6 +938,8 @@ public:
testLoanSetClosedEnded();
testLoanSetExistingLineAfterIssuerClearsDefaultRipple();
testLoanSetOriginationFeeTwoMptCreates(all_);
testLoanSetOriginationFeeTwoMptCreates(all_ - fixCleanup3_4_0);
}
};

12
src/test/app/vault/VaultBugs_test.cpp
View File

@@ -1586,14 +1586,10 @@ private:
// which for an integral MPT asset the destination check would reject if
// it were reached.
//
// ValidMPTIssuance is a separate checker and still runs. It only trips on
// the one arm that both creates and deletes an MPToken: Alice's last
// share with the asset MPToken missing, where addEmptyHolding creates the
// asset token while her share token is deleted (created + deleted > 1).
// Leftover shares with the token missing is create-only, and a last share
// with the token present is delete-only; neither exceeds one. Bob still
// owns shares throughout, so this is never the vault's final outstanding
// share.
// ValidMPTIssuance: pre-fixCleanup3_4_0, a VaultWithdraw that both
// creates and deletes an MPToken fails. Post-fixCleanup3_4_0 that is
// allowed.
//
// Post-fixCleanup3_4_0, doWithdraw skips addEmptyHolding on a zero
// payout and zeroDeltaIsLegitimate lets the vault-delta and

80
src/test/app/vault/VaultLifecycle_test.cpp
View File

@@ -795,6 +795,86 @@ private:
},
{.requireAuth = false});
auto const redeemAllNoAssetMpt = [this](TER expected) {
return [this, expected](
Env& env,
Account const&,
Account const& owner,
Account const& depositor,
Asset const& asset,
Vault& vault,
MPTTester& mptt) {
testcase << "MPT non-owner redeems all shares with no asset MPToken"
<< (isTesSuccess(expected) ? "" : " pre-fixCleanup3_4_0");
auto [tx, keylet] = vault.create({.owner = owner, .asset = asset});
env(tx);
env.close();
tx = vault.deposit(
{.depositor = depositor,
.id = keylet.key,
.amount = asset(1000)}); // all assets held by depositor
env(tx);
env.close();
auto const vaultSle = env.le(keylet);
if (!BEAST_EXPECT(vaultSle))
return;
auto const shareMPTID = vaultSle->at(sfShareMPTID);
// Depositor's asset MPToken balance is now zero; delete it.
mptt.authorize({.account = depositor, .flags = tfMPTUnauthorize});
env.close();
auto const mptoken = keylet::mptoken(mptt.issuanceID(), depositor);
auto const shareKeylet = keylet::mptoken(shareMPTID, depositor.id());
auto const sleShareBefore = env.le(shareKeylet);
if (!BEAST_EXPECT(sleShareBefore))
return;
auto const shareAmountBefore = sleShareBefore->at(sfMPTAmount);
auto const assetsTotalBefore = vaultSle->at(sfAssetsTotal);
auto const assetsAvailableBefore = vaultSle->at(sfAssetsAvailable);
// Redeeming ALL shares in one transaction both erases the
// now-empty share MPToken and re-creates the asset MPToken.
tx = vault.withdraw(
{.depositor = depositor, .id = keylet.key, .amount = asset(1000)});
env(tx, Ter{expected});
env.close();
auto const sleAsset = env.le(mptoken);
auto const sleShare = env.le(shareKeylet);
auto const vaultAfter = env.le(keylet);
if (!BEAST_EXPECT(vaultAfter))
return;
if (isTesSuccess(expected))
{
if (!BEAST_EXPECT(sleAsset))
return;
BEAST_EXPECT(sleAsset->at(sfMPTAmount) == 1000);
BEAST_EXPECT(!sleShare);
BEAST_EXPECT(vaultAfter->at(sfAssetsTotal) == beast::kZero);
BEAST_EXPECT(vaultAfter->at(sfAssetsAvailable) == beast::kZero);
}
else
{
BEAST_EXPECT(!sleAsset);
if (!BEAST_EXPECT(sleShare))
return;
BEAST_EXPECT(sleShare->at(sfMPTAmount) == shareAmountBefore);
BEAST_EXPECT(vaultAfter->at(sfAssetsTotal) == assetsTotalBefore);
BEAST_EXPECT(vaultAfter->at(sfAssetsAvailable) == assetsAvailableBefore);
}
};
};
testCase(redeemAllNoAssetMpt(tesSUCCESS), {.requireAuth = false});
testCase(
redeemAllNoAssetMpt(tecINVARIANT_FAILED),
{.requireAuth = false, .features = testableAmendments() - fixCleanup3_4_0});
auto const [acctReserve, incReserve] = [this]() -> std::pair<int, int> {
Env const env{*this, testableAmendments()};
return {