sablier-labs / v2-core

⏳ Core smart contracts of the Sablier V2 token distribution protocol
https://sablier.com
Other
292 stars 40 forks source link

Inconsistency in the order of parameters in structs between LockupLinear and LockupDynamic #728

Closed andreivladbrg closed 6 months ago

andreivladbrg commented 8 months ago

We should have a consistent order of parameter structs:

LD:

https://github.com/sablier-labs/v2-core/blob/e1a53b4c839912573996c313d447aeeed6eb434c/src/types/DataTypes.sol#L111-L121

LL:

https://github.com/sablier-labs/v2-core/blob/e1a53b4c839912573996c313d447aeeed6eb434c/src/types/DataTypes.sol#L223-L232

PaulRBerg commented 8 months ago

Isn't the difference due to gas efficiency reasons?

andreivladbrg commented 8 months ago

These structs are the ones passed in the create functions, the order of loading parameters in the stack does not affect the gas prices. Let's not confuse with the storage Stream structs

PaulRBerg commented 8 months ago

That's fair - an explanation in the OP would have been helpful

PaulRBerg commented 6 months ago

Pasting @smol-ninja's comment from here:

If I may answer your answer @PaulRBerg, since struct CreateWithDurations is only used in memory or on the stack (and never storage), the order of parameters does not affect gas usage. Each variable has its own slot.

So ordering does not matter here.