fix: address PR review comments

This commit is contained in:
Vito
2026-03-21 17:45:14 +01:00
parent d02f534987
commit 43547fcacc
6 changed files with 92 additions and 8 deletions

View File

@@ -1103,6 +1103,9 @@ sharesToAssetsWithdraw(
std::shared_ptr<SLE const> const& issuance,
STAmount const& shares);
[[nodiscard]] bool
isVaultDonate(Rules const& rules, STTx const& tx);
/** Has the specified time passed?
@param now the current time

View File

@@ -186,6 +186,10 @@ inline constexpr FlagValue tfUniversalMask = ~tfUniversal;
TF_FLAG(tfVaultShareNonTransferable, 0x00020000), \
MASK_ADJ(0)) \
\
TRANSACTION(VaultDeposit, \
TF_FLAG(tfVaultDonate, 0x00010000), \
MASK_ADJ(0)) \
\
TRANSACTION(Batch, \
TF_FLAG(tfAllOrNothing, 0x00010000) \
TF_FLAG(tfOnlyOne, 0x00020000) \
@@ -214,8 +218,6 @@ inline constexpr FlagValue tfUniversalMask = ~tfUniversal;
TF_FLAG(tfLoanUnimpair, 0x00040000), \
MASK_ADJ(0))
constexpr std::uint32_t const tfVaultDonate = 0x00010000;
constexpr std::uint32_t const tfVaultDepositMask = ~(tfUniversal | tfVaultDonate);
// clang-format on
// Create all the flag values.

View File

@@ -3723,6 +3723,12 @@ rippleUnlockEscrowMPT(
return tesSUCCESS;
}
[[nodiscard]] bool
isVaultDonate(Rules const& rules, STTx const& tx)
{
return rules.enabled(featureLendingProtocolV1_1) && tx.isFlag(tfVaultDonate);
}
bool
after(NetClock::time_point now, std::uint32_t mark)
{

View File

@@ -419,8 +419,7 @@ ValidVault::finalize(
return std::nullopt;
}();
bool const isDonate =
view.rules().enabled(featureLendingProtocolV1_1) && tx.isFlag(tfVaultDonate);
bool const isDonate = isVaultDonate(view.rules(), tx);
bool const shouldUpdateShares =
// Vault Asset donation is the only operation that can succeed without updating shares
((tx.getTxnType() == ttVAULT_DEPOSIT && !isDonate) || //
@@ -711,7 +710,7 @@ ValidVault::finalize(
}
// If assets are donated, check share invariants
if (view.rules().enabled(featureLendingProtocolV1_1) && tx.isFlag(tfVaultDonate))
if (isDonate)
{
auto const accountDeltaShares = deltaShares(tx[sfAccount]);
if (accountDeltaShares)

View File

@@ -77,7 +77,7 @@ VaultDeposit::preclaim(PreclaimContext const& ctx)
// LCOV_EXCL_STOP
}
if (ctx.view.rules().enabled(featureLendingProtocolV1_1) && ctx.tx.isFlag(tfVaultDonate))
if (isVaultDonate(ctx.view.rules(), ctx.tx))
{
if (account != vault->at(sfOwner))
{
@@ -168,8 +168,7 @@ VaultDeposit::doApply()
// LCOV_EXCL_STOP
}
auto const isDonate =
ctx_.view().rules().enabled(featureLendingProtocolV1_1) && ctx_.tx.isFlag(tfVaultDonate);
auto const isDonate = isVaultDonate(ctx_.view().rules(), ctx_.tx);
auto const& vaultAccount = vault->at(sfAccount);
// Note, vault owner is always authorized

View File

@@ -5434,6 +5434,81 @@ class Vault_test : public beast::unit_test::suite
BEAST_EXPECT(assetsAvailableAfterWithdraw == 0);
BEAST_EXPECT(assetsTotalAfterWithdraw == 0);
}
// Test donation with non-1:1 share ratio.
// A prior donation skews the ratio so that 1 share > 1 asset.
// The donated amount must land exactly, not rounded via shares.
{
testcase(prefix + " succeeds with non-1:1 share ratio");
// Create a fresh vault
auto const [createTx, vk] = vault.create({.owner = owner, .asset = xrpIssue()});
env(createTx, ter{tesSUCCESS});
env.close();
// Depositor puts in 10 XRP → gets 10 shares at 1:1
env(vault.deposit({
.depositor = depositor,
.id = vk.key,
.amount = XRP(10),
}),
ter{tesSUCCESS});
env.close();
// Owner donates 7 XRP → ratio becomes 17 assets / 10 shares
env(vault.deposit({
.depositor = owner,
.id = vk.key,
.amount = XRP(7),
.flags = tfVaultDonate,
}),
ter{tesSUCCESS});
env.close();
auto const sharesAfterFirstDonate = vaultShareBalance(vk);
auto const [availAfterFirstDonate, totalAfterFirstDonate] = vaultAssetBalance(vk);
// Shares unchanged (donation doesn't mint shares)
BEAST_EXPECT(sharesAfterFirstDonate == 10'000'000);
// Assets increased by exactly the donated amount
BEAST_EXPECT(availAfterFirstDonate == 17'000'000);
BEAST_EXPECT(totalAfterFirstDonate == 17'000'000);
// Donate again at the skewed 17:10 ratio — 3 XRP
env(vault.deposit({
.depositor = owner,
.id = vk.key,
.amount = XRP(3),
.flags = tfVaultDonate,
}),
ter{tesSUCCESS});
env.close();
auto const sharesAfterSecondDonate = vaultShareBalance(vk);
auto const [availAfterSecondDonate, totalAfterSecondDonate] = vaultAssetBalance(vk);
// Shares still unchanged
BEAST_EXPECT(sharesAfterSecondDonate == 10'000'000);
// Assets increased by exactly 3 XRP (20 total)
BEAST_EXPECT(availAfterSecondDonate == 20'000'000);
BEAST_EXPECT(totalAfterSecondDonate == 20'000'000);
// Depositor withdraws all shares — should get all 20 XRP
auto const sleVault = env.le(vk);
if (!BEAST_EXPECT(sleVault))
return;
Asset shareAsset(sleVault->at(sfShareMPTID));
env(vault.withdraw(
{.depositor = depositor,
.id = vk.key,
.amount = shareAsset(sharesAfterSecondDonate)}),
ter{tesSUCCESS});
env.close();
BEAST_EXPECT(vaultShareBalance(vk) == 0);
BEAST_EXPECT(vaultAssetBalance(vk).first == 0);
BEAST_EXPECT(vaultAssetBalance(vk).second == 0);
}
}
public: