Swap limit bug fix #221
Swap limit bug fix #221
Conversation
Swap limit bug fix
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
aalavandhan
force-pushed
the
swap-limit-fix
branch
from
e8377bb to
23de243
Compare
aalavandhan
force-pushed
the
swap-limit-fix
branch
from
23de243 to
547840f
Compare
| _enforceUnderlyingBalAfterSwap(underlying_, s.vaultTVL); | ||
| // Revert if swap reduces vault's available liquidity. | ||
| uint256 underlyingBalPost = underlying_.balanceOf(address(this)); | ||
| if (underlyingBalPost <= underlyingBalPre) { |
There was a problem hiding this comment.
| if (underlyingBalPost <= underlyingBalPre) { | |
| if (underlyingBalPost < underlyingBalPre) { |
Should be ok if it remains unchanged?
| _enforceUnderlyingBalAfterSwap(underlying_, s.vaultTVL); | ||
| // - Absolute balance is strictly greater than `minUnderlyingBal`. | ||
| // - Ratio of the balance to the vault's TVL is strictly greater than `minUnderlyingPerc`. | ||
| uint256 underlyingBal = underlying_.balanceOf(address(this)); |
There was a problem hiding this comment.
I guess there's a weird corner case where underlying balance could go up (or maybe just remain constant?) after an underlying->perp swap.
e.g. If the vault was sitting on a bunch of As, and the new Zs free those up.
In that case I think we'd want to allow that to go through.
Perhaps the most generalized logic would be:
If underlying bal increases or stays the same, continue
else, execute the check logic you have here.
That situation would be exceptionally rare, though. Worth it? It would be one extra var and a conditional, I think.
There was a problem hiding this comment.
cool. So basically if the underlying balances goes down, we then ensure that it doesn't go down too much. If it stays the same or increases its fine..
Skipping liquidity check when swapping from perps to underlying.
Swapping from perps to underlying should almost always increase the vault's liquidity, so we only check if underlying balance doesn't decrease.