donoso-eth / spool-foundry

3 stars 1 forks source link

Would it be beneficial to make some public storage fields public immutable instead? #1

Closed hellwolf closed 1 year ago

hellwolf commented 1 year ago

I am not a proponent of over-optimization for storage cost, but neither a fan of wasteful functionalities:

Checking https://github.com/donoso-eth/spool-foundry/blob/main/src/PoolState-V1.sol, would those public fields better be public immutable instead? Since exposing an effectively-immutable data as a storage field is wasteful.

donoso-eth commented 1 year ago

the contracts are upgradeale and I use this state contract to be inherit by the proxy, the implementation and also the internal contract receiving the delegateCalls(), like this I ensure they share the same storage layout.

As I am using the initialize() method, I though I can't declare immutable variables.

On the other hand, most of these variables are public for testing purposes, I'll have to check but I think, only the erc20 associated and one or two will remain public

hellwolf commented 1 year ago

Maybe I am missing the necessary details, if so ignore me. What I think is: if the updating of these parameters only happen when logic contract is updated, then the logic contract could contain those as immutable/constant variables (which is part of the logic contract code after instantiation).

donoso-eth commented 1 year ago

I'll have to learn a bit more, but if the immutable variables can be updated when updating the logic contract, then I will update accordingly. Even more, I have methods that update some of these variables/parameters, I could also think of removing these methods and only changing them by updating the contract. Thanks!

donoso-eth commented 1 year ago

just changed and tested and it works! thks