fix: Check trustline limits for share-denominated vault withdrawals (#6645)

This commit is contained in:
Vito Tumas
2026-04-01 21:31:45 +02:00
committed by GitHub
parent 6e2452207d
commit 02fa55df8d
2 changed files with 146 additions and 4 deletions

View File

@@ -5231,6 +5231,102 @@ class Vault_test : public beast::unit_test::suite
}
}
// Reproduction: canWithdraw IOU limit check bypassed when
// withdrawal amount is specified in shares (MPT) rather than in assets.
void
testBug6_LimitBypassWithShares()
{
using namespace test::jtx;
testcase("Bug6 - limit bypass with share-denominated withdrawal");
auto const allAmendments = testable_amendments() | featureSingleAssetVault;
for (auto const& features : {allAmendments, allAmendments - fixSecurity3_1_3})
{
bool const withFix = features[fixSecurity3_1_3];
Env env{*this, features};
Account const owner{"owner"};
Account const issuer{"issuer"};
Account const depositor{"depositor"};
Account const charlie{"charlie"};
Vault const vault{env};
env.fund(XRP(1000), issuer, owner, depositor, charlie);
env(fset(issuer, asfAllowTrustLineClawback));
env.close();
PrettyAsset const asset = issuer["IOU"];
env.trust(asset(1000), owner);
env.trust(asset(1000), depositor);
env(pay(issuer, owner, asset(200)));
env(pay(issuer, depositor, asset(200)));
env.close();
// Charlie gets a LOW trustline limit of 5
env.trust(asset(5), charlie);
env.close();
auto const [tx, keylet] = vault.create({.owner = owner, .asset = asset});
env(tx);
env.close();
auto const depositTx =
vault.deposit({.depositor = depositor, .id = keylet.key, .amount = asset(100)});
env(depositTx);
env.close();
// Get the share MPT info
auto const vaultSle = env.le(keylet);
if (!BEAST_EXPECT(vaultSle))
return;
auto const mptIssuanceID = vaultSle->at(sfShareMPTID);
MPTIssue const shares(mptIssuanceID);
PrettyAsset const share(shares);
// CONTROL: Withdraw 10 IOU (asset-denominated) to charlie.
// Charlie's limit is 5, so this should be rejected with tecNO_LINE
// regardless of the amendment.
{
auto withdrawTx =
vault.withdraw({.depositor = depositor, .id = keylet.key, .amount = asset(10)});
withdrawTx[sfDestination] = charlie.human();
env(withdrawTx, ter{tecNO_LINE});
env.close();
}
auto const charlieBalanceBefore = env.balance(charlie, asset.raw().get<Issue>());
// Withdraw the equivalent amount in shares to charlie.
// Post-fix: rejected (tecNO_LINE) because the share amount is
// converted to assets and the trustline limit is checked.
// Pre-fix: succeeds (tesSUCCESS) because the limit check was
// skipped for share-denominated withdrawals.
{
auto withdrawTx = vault.withdraw(
{.depositor = depositor,
.id = keylet.key,
.amount = STAmount(share, 10'000'000)});
withdrawTx[sfDestination] = charlie.human();
env(withdrawTx, ter{withFix ? TER{tecNO_LINE} : TER{tesSUCCESS}});
env.close();
auto const charlieBalanceAfter = env.balance(charlie, asset.raw().get<Issue>());
if (withFix)
{
// Post-fix: charlie's balance is unchanged — the withdrawal
// was correctly rejected despite being share-denominated.
BEAST_EXPECT(charlieBalanceAfter == charlieBalanceBefore);
}
else
{
// Pre-fix: charlie received the assets, bypassing the
// trustline limit.
BEAST_EXPECT(charlieBalanceAfter > charlieBalanceBefore);
}
}
}
}
public:
void
run() override
@@ -5251,6 +5347,7 @@ public:
testVaultClawbackBurnShares();
testVaultClawbackAssets();
testAssetsMaximum();
testBug6_LimitBypassWithShares();
}
};