-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Description
Issue Description
In ammAssetOut (src/libxrpl/tx/transactors/dex/AMMHelpers.cpp, lines 120–131), the denominator (t1 * f - 1) is used without an explicit assertion:
auto const frac = (t1 * t1 - t1 * (2 - f)) / (t1 * f - 1);Where t1 = lpTokens / lptAMMBalance and f = getFee(tfee).
Under analysis, the denominator cannot actually be zero for valid inputs:
f = getFee(tfee)returnstfee / 100000, sofranges [0, 0.01] (maxtfeeis 1000)t1ranges (0, 1] (can't withdraw more tokens than total supply)- Therefore
t1 * franges [0, 0.01], making the denominator range [-1, -0.99]
Additionally, Number::operator/= throws std::overflow_error on division by zero rather than causing UB.
However, a defensive XRPL_ASSERT would make this invariant self-documenting and catch any future changes that might widen the input ranges.
Steps to Reproduce
Not currently reproducible — the denominator cannot be zero given the valid input constraints.
Expected Result
Add a defensive XRPL_ASSERT before the division to document and enforce the invariant:
auto const denom = t1 * f - 1;
XRPL_ASSERT(denom != Number{0}, "xrpl::ammAssetOut : denominator is non-zero");Actual Result
The division relies on an implicit invariant with no assertion.
Environment
All versions with AMM support (XLS-30).
Supporting Files
N/A