88mphapp / 88mph-contracts

GNU General Public License v3.0
83 stars 31 forks source link

use uint32 for timestamps (saves gas) #14

Closed uwe closed 3 years ago

uwe commented 3 years ago

We can save gas if we use a smaller datatype for the two timestamps (maturationTimestamp and depositTimestamp). I can also change this to uint64, if you consider uint32 too small (year 2038).

Gas changes:

12c12
< |  CERC20Mock                        ·  mint                                  ·     330471  ·     472428  ·     364902  ·           5  ·       0.04  │
---
> |  CERC20Mock                        ·  mint                                  ·     329190  ·     471147  ·     363621  ·           5  ·       0.04  │
14c14
< |  DInterest                         ·  deposit                               ·     552499  ·     738399  ·     668213  ·         110  ·       0.07  │
---
> |  DInterest                         ·  deposit                               ·     533379  ·     719279  ·     649093  ·         110  ·       0.06  │
20c20
< |  DInterest                         ·  rolloverDeposit                       ·     529215  ·     672300  ·     566370  ·           5  ·       0.06  │
---
> |  DInterest                         ·  rolloverDeposit                       ·     509327  ·     652412  ·     546479  ·           5  ·       0.05  │
22c22
< |  DInterest                         ·  topupDeposit                          ·     261787  ·     403744  ·     296218  ·          15  ·       0.03  │
---
> |  DInterest                         ·  topupDeposit                          ·     260506  ·     402463  ·     294937  ·          15  ·       0.03  │
26c26
< |  DInterest                         ·  withdraw                              ·      78906  ·     382749  ·     188683  ·          90  ·       0.02  │
---
> |  DInterest                         ·  withdraw                              ·      78522  ·     381213  ·     187915  ·          90  ·       0.02  │
50c50
< |  Factory                           ·  createZeroCouponBond                  ·    1060529  ·    1166574  ·    1100116  ·           5  ·       0.11  │
---
> |  Factory                           ·  createZeroCouponBond                  ·    1041409  ·    1147454  ·    1080999  ·           5  ·       0.11  │
100c100
< |  DInterest                                                                  ·          -  ·          -  ·    4904146  ·      51.6 %  ·       0.49  │
---
> |  DInterest                                                                  ·          -  ·          -  ·    4947266  ·      52.1 %  ·       0.49  │
112c112
< |  HarvestStakingMock                                                         ·    1125189  ·    1125201  ·    1125199  ·      11.8 %  ·       0.11  │
---
> |  HarvestStakingMock                                                         ·    1125177  ·    1125201  ·    1125199  ·      11.8 %  ·       0.11  │
122c122
< |  MPHMinter                                                                  ·          -  ·          -  ·    2403768  ·      25.3 %  ·       0.24  │
---
> |  MPHMinter                                                                  ·          -  ·          -  ·    2403756  ·      25.3 %  ·       0.24  │
134c134
< |  Vesting02                                                                  ·          -  ·          -  ·    2387390  ·      25.1 %  ·       0.24  │
---
> |  Vesting02                                                                  ·          -  ·          -  ·    2396025  ·      25.2 %  ·       0.24  │
138c138
< |  ZeroCouponBond                                                             ·          -  ·          -  ·    2010356  ·      21.2 %  ·       0.20  │
---
> |  ZeroCouponBond                                                             ·          -  ·          -  ·    2019627  ·      21.3 %  ·       0.20  │

It does help a bit for deposit, rolloverDeposit and withdraw (between 2 % and 3.5 %), but it makes the deployments slightly more expensive.

If you would consider this PR, shall we use uint32 or uint64 for the timestamps? Can maybe fundingID also shortened? If we use uint64 for all 3 variables, they would fit together in one slot.

Edit: I also talked about changing averageRecordedIncomeIndex to uint64, but I was mislead by Index - it should rather stay uint256.

uwe commented 3 years ago

https://en.wikipedia.org/wiki/Year_2038_problem - this is about signed int32, as we are using uint32 we are good to go until 2106.