balancer / balancer-v3-monorepo

GNU General Public License v3.0

30 stars 7 forks source link

Gas optimization: tIncrement and tDecrement #679

Closed elshan-eth closed 1 month ago

elshan-eth commented 1 month ago

Description

I tried different ways and there we can save only 100 gas.

Type of change

[ ] Bug fix
[ ] New feature
[ ] Breaking change
[ ] Dependency changes
[ ] Code refactor / cleanup
[ ] Documentation or wording changes
[ ] Other

Checklist:

[ ] The diff is legible and has no extraneous changes
[ ] Complex code has been commented, including external interfaces
[ ] Tests have 100% code coverage
[ ] The base branch is either main, or there's a description of how to merge

Issue Resolution

Closes #461

elshan-eth commented 1 month ago

The alternative I had in mind was to ditch tIncrement and tDecrement and just inline the calls to tstore / tload. We only use this in vaultCommon so perhaps it's not that bad. Do you think that'd impact readability?

I tried this but it had the same gas saving but I will compare bytecode, maybe with inline is a little bit better

elshan-eth commented 1 month ago

built-in method with the same value in terms of gas savings and bytecode

jubeira commented 1 month ago

built-in method with the same value in terms of gas savings and bytecode

Then I was misguided by forge snaps when I wrote #461 :(

I'd leave it with checked math. It should be OK, but it takes a few minutes to reason about it; it's not immediately obvious when you look at the code. And given that the gas savings are not huge, I'd rather preserve readability here.

Thanks anyway for investigating!