fix: Clamp VaultClawback to assetsAvailable for both zero and non-zero amounts

Move the zero-amount clawback path inside the try block so it shares
the same clamping logic as non-zero clawback. Previously, zero-amount
clawback bypassed the assetsAvailable cap, allowing more assets to be
recovered than available when there was an outstanding loan.

Add tests for both zero and non-zero clawback clamping with loans.
This commit is contained in:
Vito
2026-03-23 12:21:03 +01:00
parent be1cc48d84
commit 4a4e83de2f
2 changed files with 148 additions and 25 deletions

View File

@@ -11,6 +11,7 @@
#include <xrpl/tx/transactors/vault/VaultClawback.h>
#include <optional>
#include <utility>
namespace xrpl {
NotTEC
@@ -223,40 +224,41 @@ VaultClawback::assetsToClawback(
auto const assetsAvailable = vault->at(sfAssetsAvailable);
auto const mptIssuanceID = *vault->at(sfShareMPTID);
MPTIssue const share{mptIssuanceID};
if (clawbackAmount == beast::zero)
{
auto const sharesDestroyed = accountHolds(
view(),
holder,
share,
FreezeHandling::fhIGNORE_FREEZE,
AuthHandling::ahIGNORE_AUTH,
j_);
auto const maybeAssets = sharesToAssetsWithdraw(vault, sleShareIssuance, sharesDestroyed);
if (!maybeAssets)
return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE
return std::make_pair(*maybeAssets, sharesDestroyed);
}
STAmount sharesDestroyed;
STAmount assetsRecovered = clawbackAmount;
STAmount assetsRecovered;
try
{
if (clawbackAmount == beast::zero)
{
sharesDestroyed = accountHolds(
view(),
holder,
share,
FreezeHandling::fhIGNORE_FREEZE,
AuthHandling::ahIGNORE_AUTH,
j_);
auto const maybeAssets =
sharesToAssetsWithdraw(vault, sleShareIssuance, sharesDestroyed);
if (!maybeAssets)
return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE
assetsRecovered = *maybeAssets;
}
else
{
auto const maybeShares =
assetsToSharesWithdraw(vault, sleShareIssuance, assetsRecovered);
assetsToSharesWithdraw(vault, sleShareIssuance, clawbackAmount);
if (!maybeShares)
return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE
sharesDestroyed = *maybeShares;
auto const maybeAssets =
sharesToAssetsWithdraw(vault, sleShareIssuance, sharesDestroyed);
if (!maybeAssets)
return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE
assetsRecovered = *maybeAssets;
}
auto const maybeAssets = sharesToAssetsWithdraw(vault, sleShareIssuance, sharesDestroyed);
if (!maybeAssets)
return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE
assetsRecovered = *maybeAssets;
// Clamp to maximum.
if (assetsRecovered > *assetsAvailable)
{

View File

@@ -4797,6 +4797,7 @@ class Vault_test : public beast::unit_test::suite
auto const& vaultSle = env.le(vaultKeylet);
BEAST_EXPECT(vaultSle != nullptr);
env.memoize(Account("vault", vaultSle->at(sfAccount)));
env(vault.deposit(
{.depositor = depositor, .id = vaultKeylet.key, .amount = asset(100)}),
ter(tesSUCCESS));
@@ -4953,6 +4954,126 @@ class Vault_test : public beast::unit_test::suite
}),
ter(tesSUCCESS));
}
{
testcase(
"VaultClawback (asset) - " + prefix +
" zero-amount clawback clamped with outstanding loan");
auto [vault, vaultKeylet] = setupVault(asset, owner, depositor, issuer);
auto const vaultSle = env.le(vaultKeylet);
BEAST_EXPECT(vaultSle != nullptr);
if (!vaultSle)
return;
PrettyAsset shares = MPTIssue(vaultSle->at(sfShareMPTID));
// Create a loan broker backed by this vault
auto const brokerKeylet = keylet::loanbroker(owner.id(), env.seq(owner));
env(set(owner, vaultKeylet.key));
env.close();
// Depositor borrows 40 units, reducing assetsAvailable to 60
// while assetsTotal stays at 100
env(set(depositor, brokerKeylet.key, asset(40).value()),
loan::interestRate(TenthBips32(0)),
gracePeriod(60),
paymentInterval(120),
paymentTotal(10),
sig(sfCounterpartySignature, owner),
fee(env.current()->fees().base * 2),
ter(tesSUCCESS));
env.close();
{
auto const sle = env.le(vaultKeylet);
BEAST_EXPECT(sle->at(sfAssetsAvailable) == asset(60).value());
BEAST_EXPECT(sle->at(sfAssetsTotal) == asset(100).value());
}
auto const sharesBefore = env.balance(depositor, shares.raw().get<MPTIssue>());
// Zero-amount clawback (= "clawback all") should succeed,
// clamped to assetsAvailable (60) rather than the full
// share value (100).
env(vault.clawback({
.issuer = issuer,
.id = vaultKeylet.key,
.holder = depositor,
}),
ter(tesSUCCESS));
env.close();
// Only 60 assets clawed back; loan's 40 still outstanding
{
auto const sle = env.le(vaultKeylet);
BEAST_EXPECT(sle != nullptr);
BEAST_EXPECT(sle->at(sfAssetsAvailable) == asset(0).value());
BEAST_EXPECT(sle->at(sfAssetsTotal) == asset(40).value());
// 60 of 100 shares destroyed (1:1 ratio), 40 remain
auto const sharesAfter = env.balance(depositor, shares.raw().get<MPTIssue>());
BEAST_EXPECT(sharesAfter == shares(Number{4, sle->at(sfScale) + 1}));
}
}
{
testcase(
"VaultClawback (asset) - " + prefix +
" non-zero clawback clamped with outstanding loan");
auto [vault, vaultKeylet] = setupVault(asset, owner, depositor, issuer);
auto const vaultSle = env.le(vaultKeylet);
BEAST_EXPECT(vaultSle != nullptr);
if (!vaultSle)
return;
PrettyAsset shares = MPTIssue(vaultSle->at(sfShareMPTID));
// Create a loan broker backed by this vault
auto const brokerKeylet = keylet::loanbroker(owner.id(), env.seq(owner));
env(set(owner, vaultKeylet.key));
env.close();
// Depositor borrows 40 units
env(set(depositor, brokerKeylet.key, asset(40).value()),
loan::interestRate(TenthBips32(0)),
gracePeriod(60),
paymentInterval(120),
paymentTotal(10),
sig(sfCounterpartySignature, owner),
fee(env.current()->fees().base * 2),
ter(tesSUCCESS));
env.close();
{
auto const sle = env.le(vaultKeylet);
BEAST_EXPECT(sle->at(sfAssetsAvailable) == asset(60).value());
BEAST_EXPECT(sle->at(sfAssetsTotal) == asset(100).value());
}
auto const sharesBefore = env.balance(depositor, shares);
// Request 100 but only 60 available — clamped to 60
env(vault.clawback({
.issuer = issuer,
.id = vaultKeylet.key,
.holder = depositor,
.amount = asset(100).value(),
}),
ter(tesSUCCESS));
env.close();
{
auto const sle = env.le(vaultKeylet);
BEAST_EXPECT(sle != nullptr);
BEAST_EXPECT(sle->at(sfAssetsAvailable) == asset(0).value());
BEAST_EXPECT(sle->at(sfAssetsTotal) == asset(40).value());
}
auto const sharesAfter = env.balance(depositor, shares);
BEAST_EXPECT(sharesAfter < sharesBefore);
BEAST_EXPECT(sharesAfter > shares(0));
}
};
Account owner{"alice"};