balancer / balancer-v3-monorepo

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

Revert potentially dangerous optimization #778

Closed EndymionJkb closed 2 months ago

EndymionJkb commented 2 months ago

Description

As Juani pointed out, caching the bufferWrappedSurplus might be dangerous, given that there are external calls between when it's initialized and the last time it's used.

I tried a few variants that preserved some of it (e.g., just reloading as necessary), but as the point of this was to save bytecode and get the Vault to fit, simply reverting that change turned out to be best. (The variants were actually slightly more bytecode.)

Luckily, due to the other changes, it still fits with this change reverted.

Type of change

Checklist:

Issue Resolution

EndymionJkb commented 2 months ago

Looks good now!

One final ask: can we make the code symmetrical? Either option works (make the surplus variable as local as possible as wrap, or declare above and reuse as with unwrap).

Keeping the exact same structure helps readers follow through better. Same that happens with add / remove.

Sorry for being so annoying :D

NP; will do