XRP Ledger security record

`AMMDeposit(tfTwoAsset)` allows specifying `amount` and `amount2`. They are used as maximum values consumed by the deposit operation. However, that doesn't allow the user to set the protection from front-running. Other implementations of AMM, like Uniswap, allow the user to set the minimum amounts accepted: ```solidity function addLiquidity( address tokenA, address tokenB, uint amountADesired, uint amountBDesired, uint amountAMin, uint amountBMin, address to, uint deadline ``` If `amountAMin`/`amountBMin` conditions are not satisfied, the transaction is rejected. This effectively protects the user from front-running and sudden pool spot-price changes for other reasons. Other modes of `AMMDeposit` also don't provide protection from price fluctuations.

```cpp=242 if (computedPrice >= *bidMin && computedPrice <= *bidMax) return computedPrice; JLOG(ctx_.journal.debug()) << "AMM Bid: not in range " << computedPrice << *bidMin << " " << *bidMax; ``` `AMMBid` allows setting `sfBidMin` and `sfBidMax`. `computedPrice` is calculated based on the current slot state. The code doesn't allow raising the bid via setting `sfBinMin` if `sfBidMax` is also specified.

```cpp=81 cur.out = toAmount<TOut>( getIssue(balances.out), cur.out * fib[ammContext_.curIters() - 1], Number::rounding_mode::downward); cur.in = swapAssetOut(balances, cur.out, tradingFee_); ``` `generateFibSeqOffer()` is supposed to generate AMM offers with the offer size based on Fibonacci sequence. The starting `cur.in = InitialFibSeqPct * initialBalances_.in = 0.025% of initialBalances_.in`. `cur.out` is approximately the same. Then `cur.out` is multiplied by `fib` elements based on the iteration. Since `fib[17] = 4181`, `cur.out` will get bigger than `balances.out`. That is unexpected by `swapAssetOut()` - `cur.in` will become negative or overflow.

It is possible to perform a single asset withdrawal via `AMMWithdraw(tfOneAssetLPToken)`. However, the formula (8) is inaccurate. It is more advantageous for the user to perform `AMMWithdraw(tfLPToken)` and then swap the redundant token.

In `AMMWithdraw::preclaim()` if `sfAmount`/`sfAmount2` are set, it is checked that * account is authorized to hold the tokens and the trustline exists * AMM account or currency is not frozen * account is not frozen However, if `tfLPToken` is set, it is not checked that account is authorized to hold the tokens. If `tfWithdrawAll` is set, all three checks are skipped. As a result, the frozen account can get tokens from AMM or tokens can be extracted from frozen AMM.

It is possible to perform a single asset deposit via `AMMDeposit(tfSingleAsset)`. However, the formulas [(3) and (4)](https://github.com/XRPLF/XRPL-Standards/discussions/78) are inaccurate. Fees are always taken for half of the deposited asset and pool value change due to fees is not accounted for. The variable amount should be virtually swapped with the corresponding fee taken. So the users will prefer `AMMDeposit(tfSingleAsset)` then `AMMWithdraw(tfLPToken)` instead of normal swap to save some fees. Current logic behind single asset deposit formula **(3)**: $$\Delta_{LPTokens} = \Gamma_{LPTokens} * \left[ \left(1+ \frac{\Delta_{B}-TFee*(1-W_B)*\Delta_B}{\Gamma_B} \right) ^ {W_B} - 1 \right] $$ 1. The deposited amount $$ \Delta_B $$ is split into $$ W_B\Delta_B + (1-W_B)\Delta_B = W_B\Delta_B + W_A\Delta_B$$ 2. Then $$W_A\Delta_B$$ of AssetB is virtually swapped for AssetA 3. Pool spot-price $$ SP = \frac{\frac{\Gamma_A}{W_A}}{\frac{\Gamma_B}{W_B}} $$ (see **(II)**) is used to get $$ \Delta_A = W_B\Delta_B \frac{\Gamma_A}{\Gamma_B}$$ 4. Two assets $$\left( \Delta_A, W_B\Delta_B \right) = \left( W_B\Delta_B \frac{\Gamma_A}{\Gamma_B} , W_B\Delta_B \right)$$ are deposited to pool with $$\left( \Gamma_A, \Gamma_B \right)$$ reserves 5. The pool ratio $$ \frac{\Gamma_A}{\Gamma_B} $$ (and therefore pool spot-price) remains the same since $$ \frac{\Gamma'_A}{\Gamma'_B} = \frac{\Gamma_A + W_B\Delta_B \frac{\Gamma_A}{\Gamma_B}}{\Gamma_B + W_B\Delta_B} = \frac{\Gamma_A}{\Gamma_B} $$ 6. Since we virtually swapped $$W_A\Delta_B$$, only $$ TFee W_A $$ was taken, that's why $$ \Delta_B $$ in **(3)** is reduced by $$ \left( 1-TFee(1-W_B)\right) $$ However, the pool state changes during the swap operation, so we can't perform the swap using the spot-price and then perform a deposit using the same spot-price. In case $$ \Delta_B $$ is comparable to $$ \Gamma_B $$ and $$ Tfee $$ is significant, the resulting $$ \Delta_{LPTokens} $$ will be higher than obtainable via real swap and `AMMDeposit(tfTwoAsset)`. See the example in the Scenario section. The proposed logic of single asset deposit is: 1. Let $$ W_A = W_B = 0.5 $$ 2. We want to split $$ \Delta_B $$ into two parts $$ x + (\Delta_B - x) $$ and swap $$ x $$ for $$ \Delta_A = \Gamma_A \frac{x(1-TFee)}{\Gamma_B + x(1-TFee)} $$ (see **(9)**) 3. The $$x$$ should be so that the final pool reserves ratio $$\frac{\Gamma_A}{\Gamma_B+\Delta_B} $$ is equal to the ratio of assets deposited $$ \frac{\Delta_A}{\Delta_B - x} $$ and is equal to the pool reserves ratio after the swap $$ \frac{\Gamma_A - \Delta_A}{\Gamma_B + x} $$ 4. Using $$ \frac{\Gamma_A}{\Gamma_B + \Delta_B} = \frac{\Delta_A}{\Delta_B-x} $$ and substituting $$ \Delta_A $$ we get $$ (\Delta_B - x)(\Gamma_B + x(1-TFee)) = (\Gamma_B + \Delta_B)x(1-TFee) $$ 5. Solving it gives $$ x = \Gamma_B (\sqrt{ \left( \frac{1-TFee/2}{1-TFee} \right) ^2 + \frac{\Delta_B}{\Gamma_B(1-TFee)}} - \frac{1-TFee/2}{1-TFee} )$$ 6. After the swap of $$ x $$ of AssetB for $$ \Delta_A $$ of AssetA, we perform all assets deposit with $$ \Delta_{LPTokens} = \Gamma_{LPTokens} \frac{\Delta_A}{\Gamma_A - \Delta_A} = \Gamma_{LPTokens} \frac{\Delta_B-x}{\Gamma_B + x} $$ The final formula instead of **(3)** would be: $$ \Delta_{LPTokens} = \Gamma_{LPTokens} \frac{\Delta_B-x}{\Gamma_B + x} = \Gamma_{LPTokens} \frac{ \frac{\Delta_B}{\Gamma_B} - (\sqrt{ \left( \frac{1-TFee/2}{1-TFee} \right) ^2 + \frac{\Delta_B}{\Gamma_B(1-TFee)}} - \frac{1-TFee/2}{1-TFee} ) }{1 + {\sqrt{ \left( \frac{1-TFee/2}{1-TFee} \right) ^2 + \frac{\Delta_B}{\Gamma_B(1-TFee)}} - \frac{1-TFee/2}{1-TFee} } } $$

It is possible to perform a single asset withdrawal via `AMMWithdraw(tfSingleAsset)`. However, the formulas [(7) and (8)](https://github.com/XRPLF/XRPL-Standards/discussions/78) are inaccurate. It is more advantageous for the user to perform `AMMWithdraw(tfLPToken)` and then swap the redundant token. We propose to follow this logic: 1. Our goal is to get $$ \Delta_B = b_{withdrawn} + b_{swapped} $$ 2. Assuming we first withdraw $$ x $$ fraction of $$ \Gamma_{LPTokens} $$ 3. That gives $$ (x \Gamma_A, x\Gamma_B) $$ and pool in state $$ ((1-x)\Gamma_A, (1-x)\Gamma_B) $$ 4. Using **(9)** we can swap $$ x\Gamma_A $$ of AssetA for $$ b_{swapped} = \Gamma'_B \frac{x\Gamma_A(1-TFee)}{\Gamma'_A + x\Gamma_A(1-TFee)} $$ of AssetB 5. Using $$ \Delta_B = b_{withdrawn}+ b_{swapped} = x\Gamma_B + \Gamma'_B \frac{x\Gamma_A(1-TFee)}{\Gamma'_A + x\Gamma_A(1-TFee)} $$ we get $$ \Delta_B - x\Gamma_B = (1-x)\Gamma_B \frac{x\Gamma_A(1-TFee)}{(1-x)\Gamma_A + x\Gamma_A(1-TFee)} $$ 6. Simplifying that we get $$ (\frac{\Delta_B}{\Gamma_B} - x)(1-x + x(1-TFee)) = (1-x)x(1-TFee)$$ 7. Which gives $$ x^2 + x(TFee - 2 - \frac{\Delta_B}{\Gamma_B}Tfee) + \frac{\Delta_B}{\Gamma_B} = 0 $$ 8. Updated **(7')** is $$ \frac{\Delta_{LPTokens}}{\Gamma_{LPTokens}}= x = \frac{ \frac{\Delta_B}{\Gamma_B}Tfee + 2 - TFee - \sqrt{(\frac{\Delta_B}{\Gamma_B}Tfee + 2 - TFee)^2 - 4\frac{\Delta_B}{\Gamma_B}}}{2} $$ With current formulas, users should never `AMMWithdraw(tfSingleAsset)` to prevent extra spending.

`MinSlotPrice` is set to zero. So, if `bidMin` is not specified, `sfPrice` is also set to zero according to the formula. There is no motivation for arbitrageurs to raise the price. They can always buy a slot for zero, perform the arbitrage and give up the slot to the next participant for free.

```cpp=1567 if (maxNodesToDelete && ++deleted > *maxNodesToDelete) ``` `maxNodesToDelete` is `std::optional<uint16_t>`, `deleted` is `uint16_t`. If `maxNodesToDelete == 65535` and there are enough entries in the account directory, the `deleted` will overflow and the code will not work as expected. `maxDeletableAMMTrustLines` passed to `cleanupOnAccountDelete()` is `size_t` set `512`. The ledger allows more than 65535 entries in the account directory according to `View.cpp::confineOwnerCount()`.

There are typos in the code: - "truslines" is supposed to be "trustlines" - "view" is supposed to be "View"