diff --git a/test/foundry/differential/README.md b/test/foundry/differential/README.md new file mode 100644 index 0000000..eb6b2c3 --- /dev/null +++ b/test/foundry/differential/README.md @@ -0,0 +1,139 @@ +# New `SmartVaultYieldManager::_swapToRatio` implementation considerations + +1) Safety catch-all: in `SmartVaultYieldManager::_deposit`, the call to `UniProxy::deposit` can fail if the balances in the contract are not at the same ratio as the Hypervisor requires. Here, a fail safe could be implemented to check that the ratio is correct by calling `UniProxy::.getDepositAmount`. Any excess balance could then be returned to the Smart Vault (`msg.sender`) before the call to `UniProxy::deposit`, guaranteeing that the deposit will always succeed. + +2) The second branch (swap `B→A`) in the new `_swapToRatio()` is unnecessary. As the `SmartVaultYieldManager` shouldn't hold any tokens, and only `tokenA` is moved to the contract, there shouldn't be any `tokenB` present. In the unlikely scenario that there is more `tokenB` than required for the ratio, the surplus can be caught using the same catch-all as described above. This would mean some changes to the differential fuzz test as well. + +The implementation is somewhat unclear due to stack management, so derivation of the maths is provided below: + +## Derivation +### Without pool fees + +Let's start without pool fees to describe the general maths. + +Given: +1) amount $a$ of `tokenA`, and amount $b$ of `tokenB` in the contract. +2) price $p = \frac{\Delta a}{\Delta b}$ +3) target ratio $r$ is $r = a/b_{mid}$, $b_{mid}$ is `_midRatio` in the code, hence is known. +4) target ratio is also target amount of a over target amount of b: $r = a_t / b_t$ + +#### swap A -> B + +Gives: $a_t = a - \Delta a$ and $b_t = b + \Delta b$ + +thus the ratio: + +$$ +r = \frac{a - \Delta a}{b + \Delta b} => a - \Delta a = r ( b + \Delta b ) +$$ + +we know $\Delta b = \frac{\Delta a}{p}$ thus: + +$$ +a - \Delta a = r (b + \frac{\Delta a}{p}) => a - \Delta a = rb + \frac{r\Delta a}{p} +$$ + +$$ +-\Delta a - \frac{r\Delta a}{p} = rb - a => \Delta a + \frac{r\Delta a}{p} = a - rb +$$ + +$$ +\Delta a (1 + \frac{r}{p}) = a - rb => \Delta a = \frac{a - rb}{1 + \frac{r}{p}} +$$ + +Sanity checking this, $a = 100$, $b= 100$, $p = 1/2$, $r = 1/2$: + +$$ +\Delta a = \frac{100 - 1/2 * 100}{1 + \frac{1/2}{1/2}} => \Delta a = \frac{100 - 50}{1 + 1} = 25 +$$ + +Swapping `25 A` would give you `50 B` which would give the end result `75 A` and `150 B` which indeed was the ratio. + +#### swap B -> A + +$a_t = a + \Delta a$ and $b_t = b - \Delta b$ + +thus the ratio: + +$$ +r = \frac{a + \Delta a}{b - \Delta b} => a + \Delta a = r ( b - \Delta b ) +$$ + +we know $\Delta b = \frac{\Delta a}{p}$ thus: + +$$ +a + \Delta a = r (b - \frac{\Delta a}{p}) => a + \Delta a = rb - \frac{r\Delta a}{p} +$$ + +$$ +\Delta a + \frac{r\Delta a}{p} = rb - a +$$ + +$$ +\Delta a( 1 + \frac{r}{p}) = rb - a => \Delta a = \frac{rb - a}{1 + \frac{r}{p}} +$$ + +Sanity checking $a = 100$, $b = 300$, $r$ and $p$ both $1/2$ again: + + +$$ +\Delta a = \frac{1/2 * 300 - 100}{1 + 1} = 25 +$$ + +To get `25 A` out, you need to provide `50 B` which would end up at `125 A`, `250 B` after the swap, which is the target ratio. + +### With fees + +Introducing pool fees changes the definition of the price, $p$. + +For a swap A->B, the price for $\Delta a$ amount in is described as: + +$$ +p = \frac{\Delta a - \Delta a f}{\Delta b} +$$ + +where $f$ is the pool fee. + +For a swap B->A, the price for $\Delta a$ amount out is similarly: + +$$ +p = \frac{\Delta a}{\Delta b - \Delta b f} +$$ + +#### Swap A -> B + +Using the equation from above: + +$$ +r = \frac{a - \Delta a}{b + \Delta b} => a - \Delta a = r ( b + \Delta b ) +$$ + +the new $\Delta b$ using fee will now be: $\Delta b = \frac{\Delta a(1-f)}{p}$ thus: + +$$ +a - \Delta a = r (b + \frac{\Delta a (1 - f)}{p}) +$$ + +Using the same rearranging as above gives us a final equation for $\delta a$: + +$$ + \Delta a = \frac{a - rb}{1 + \frac{r(1-f)}{p}} +$$ + +A quick sanity check tells us that increasing the fee $f$ here will decrease the denominator resulting in a higher $\Delta a$ which is what we expect (as more will be lost to the fee). + +#### Swap B -> A + +Using $p = \frac{\Delta a}{\Delta b (1-f)}$ gives us: + +$$ +a - \Delta a = r (b + \frac{\Delta a}{(1 - f)p}) +$$ + +Which at the end gives us $\Delta a$ as: + +$$ +\Delta a = \frac{rb - a}{1 + \frac{r}{(1-f)p}} +$$ + +A quick sanity check tells us that increasing the fee $f$ here will increase the denominator resulting in a lower $\Delta a$ which is what we expect (as more will be lost to the fee, thus less out). diff --git a/test/foundry/invariant/README.md b/test/foundry/invariant/README.md new file mode 100644 index 0000000..12d6441 --- /dev/null +++ b/test/foundry/invariant/README.md @@ -0,0 +1,5 @@ +# Invariant test suite considerations + +1) Heavy mocking has made the test fixtures quite complicated by comparison to the fork fixture. Swap router rates could be replaced with properly mocked pools – currently we have mock pools deployed purely for the new `_swapToRatio()` implementation while all swaps still use the mock rates. Ideally, rewrite all unit tests as fork tests using a snapshot of real on-chain state. + +2) Invariant tests are very much a work in progress. DoS and other false positives may arise either due to mock setup (e.g. `ERC20: transfer amount exceeds balance in the mock swap router`) or missing expected errors, but validate they are actually false positives and not unexpected reverts (could also just remove the `checkExpectedErrors` modifier from specific target functions to silence DoS invariants if they aren't reliable). Coverage is good and state space should be relatively small by isolating a single smart vault and adding collateral once during setup. Again, ideally we use the fork fixture for invariant tests and add handlers for the Hypervisors, but this is quite a bit of extra work. Have been using `SizeCredit/size-solidity` repo as a loose reference. All todo items and notes like this can be easily searched using `TODO:` and `NOTE:`. At least one of the critical findings gets flagged by medusa with `REMOVE_ASSET_05` reverting.