balancer / balancer-v3-monorepo

GNU General Public License v3.0
37 stars 10 forks source link

Simplify base pool math #797

Closed jubeira closed 1 month ago

jubeira commented 1 month ago

Description

This PR saves some bytecode and gas by simplifying math optimizations to BasePoolMath.

There are a few instances where we're doing a.mulDown(b).divDown(c) (or up). In those instances, as long as we multiply first and divide later we don't need to use FP math, and it should also be more precise not to use FP math. Since FP operations require a multiplication and a division, this approach is more efficient even when inlining the math inside a for-loop, without saving intermediate results.

Type of change

Checklist:

Issue Resolution

N/A

jubeira commented 1 month ago

Thanks for the feedback.

I've adjusted the code to preserve the rounding direction in the places I think it's worth it, adding comments about it. Savings are not as good as they were, but it's still something.

jubeira commented 1 month ago

It lowers the improvement, but it seems a bit dangerous to just remove the rounding (though you did change removeLiquiditySingleToken, where it was rounding up and up again, and now it rounds down and then up). Maybe a question for the auditors.

This should be fine; the result of the multiplication is a fixed point with 36 decimals which doesn't lose precision; the division is the one that can't be represented perfectly up to the very last wei and needs to be rounded either up or down.