refactor: document rounding behaviour, consolidate deposit NAV check

This commit is contained in:
Vito
2026-03-19 17:48:08 +01:00
parent 59148bb7a5
commit a8914da275
2 changed files with 41 additions and 11 deletions

View File

@@ -50,6 +50,26 @@ computeDeposit(
STAmount const& assets,
beast::Journal j);
/** Compute a withdrawal given a fixed asset amount.
*
* Converts assets → shares → assets (round-trip) to determine the
* exact shares to redeem and assets to return.
*
* The intermediate shares value is stored as an MPT (integer), which
* uses banker's rounding (round-to-nearest, even on tie). When shares
* round up, the back-calculated assets may exceed the requested amount.
*
* Example (scale=1, vault=87.5 IOU, 875 shares):
* assetsToSharesWithdraw(3.75) → round(37.5) = 38 shares
* sharesToAssetsWithdraw(38) → 3.8 IOU (> 3.75 requested)
*
* This matches v1 behaviour. See XLS-0065 §3.1.7.1.
*
* @return ExchangeResult{assets, shares} on success.
* @return tecPRECISION_LOSS if computed shares are zero.
* @return tecPATH_DRY on arithmetic overflow.
* @return tecINTERNAL on invalid input or vault state.
*/
[[nodiscard]] Expected<ExchangeResult, TER>
computeWithdrawByAssets(
Rules const& rules,

View File

@@ -10,6 +10,10 @@ namespace xrpl::vault {
namespace detail {
// Deposit uses NAV = assetsTotal - interestUnrealized (excludes loss).
// Withdraw uses NAV = assetsTotal - interestUnrealized - lossUnrealized.
// See XLS-0065 §3.1.7.1 "Share Valuation" for rationale.
STAmount
assetsToSharesDeposit(SLE::const_ref vault, SLE::const_ref issuance, STAmount const& assets)
{
@@ -100,9 +104,6 @@ sharesToAssetsWithdraw(SLE::const_ref vault, SLE::const_ref issuance, STAmount c
} // namespace detail
// v1 math is intentionally not factored out like v2. Since Single Asset Vault
// is already released, refactoring v1 risks introducing behavioral changes in
// production code that we cannot gate behind an amendment.
namespace {
namespace v1 {
@@ -181,10 +182,22 @@ validateVaultState(SLE::const_ref vault, SLE::const_ref issuance, beast::Journal
Number const interestUnrealized = vault->at(sfInterestUnrealized);
Number const lossUnrealized = vault->at(sfLossUnrealized);
Number const nav = assetTotal - interestUnrealized - lossUnrealized;
if (nav < 0)
// Deposit NAV excludes loss; withdrawal NAV excludes both.
// Deposit NAV <= 0 means all vault value is unrealized interest, which should be impossible.
Number const depositNAV = assetTotal - interestUnrealized;
if (depositNAV <= 0)
{
JLOG(j.error()) << "vault state: NAV < 0"
JLOG(j.error()) << "vault state: deposit NAV <= 0"
<< " (assetsTotal=" << assetTotal
<< ", interestUnrealized=" << interestUnrealized << ")";
return tecINTERNAL;
}
Number const withdrawNAV = depositNAV - lossUnrealized;
if (withdrawNAV < 0)
{
JLOG(j.error()) << "vault state: withdrawal NAV < 0"
<< " (assetsTotal=" << assetTotal
<< ", interestUnrealized=" << interestUnrealized
<< ", lossUnrealized=" << lossUnrealized << ")";
@@ -192,19 +205,16 @@ validateVaultState(SLE::const_ref vault, SLE::const_ref issuance, beast::Journal
}
Number const shareTotal = issuance->at(sfOutstandingAmount);
if (nav > 0 && shareTotal <= 0)
if (withdrawNAV > 0 && shareTotal <= 0)
{
JLOG(j.error()) << "vault state: no outstanding shares with positive NAV"
<< " (NAV=" << nav << ", shareTotal=" << shareTotal << ")";
<< " (NAV=" << withdrawNAV << ", shareTotal=" << shareTotal << ")";
return tecINTERNAL;
}
return tesSUCCESS;
}
// Dispatch to v1 or v2 math based on amendment.
// v1 is frozen — no refactoring to preserve existing behaviour.
STAmount
assetsToSharesDeposit(
Rules const& rules,