balancer / balancer-core

Balancer on the EVM
GNU General Public License v3.0
333 stars 168 forks source link

Code-quality issues in BPool #211

Closed ggrieco-tob closed 4 years ago

ggrieco-tob commented 4 years ago

It is unclear how to determinate the minimal amount of tokens to swap

A user looking to swap a small amount of tokens in a pool has no documentation on how to determinate valid amounts to do it.

Functions like swapExactAmountIn and swapExactAmountOut have specific code to avoid rounding issues:

https://github.com/balancer-labs/balancer-core/blob/942a51e202cc5bf9158bad77162bc72aa0a8afaf/contracts/BPool.sol#L473

https://github.com/balancer-labs/balancer-core/blob/942a51e202cc5bf9158bad77162bc72aa0a8afaf/contracts/BPool.sol#L535

While such code is necessary to avoid the computation of incorrect values, it also make significantly harder to determinate valid amounts to swap, in particular when they are small.

Users should be carefully when exiting pool to avoid trapping an small amount of tokens

If exit fee cannot be payed because there are not enough share tokens, a user will not be able to call exitPool:

https://github.com/balancer-labs/balancer-core/blob/942a51e202cc5bf9158bad77162bc72aa0a8afaf/contracts/BPool.sol#L392-L407

This will trap a small amount of funds. Make sure users are aware that they need to exit with amounts divisible by EXIT_FEE.

mikemcdonald commented 4 years ago

EXIT_FEE will be 0 and therefore won't effect a users trade.

Any trade small enough to trigger require(spotPriceAfter >= spotPriceBefore, "ERR_MATH_APPROX"); is not an expected use case and should revert