refactor newSupplyFrom to reuse getAccruedValue

[guidozanon]

Reuse getAccruedValue to avoid mistakes on calculating supply on human account

[santisiri]

This makes sense to me codewise, but have you checked how this could impact in terms of gas? Can't remember if we refactored the contract this way in order to optimize gas usage /cc @clesaege

[guidozanon]

Good catch. I haven't run a gas test (tbh I don't even know how to do it). Codewise, as you said, seems to be executing the same lines (but in your code, you have an IF that will avoid running 2 lines if the conditions are not meet). I'll investigate the gas consumption and back to you.

[santisiri]

if you do npx hardhat test in your console, you will be able to run the tests and at the end you'll see a chart that calculates the cost in gas for each function.

Without your improvements, it currently looks like this:

·----------------------------------------------|---------------------------|-------------|----------------------------·
|             Solc version: 0.7.3              ·  Optimizer enabled: true  ·  Runs: 200  ·  Block limit: 9500000 gas  │
···············································|···························|·············|·····························
|  Methods                                                                                                            │
·····················|·························|·············|·············|·············|··············|··············
|  Contract          ·  Method                 ·  Min        ·  Max        ·  Avg        ·  # calls     ·  eur (avg)  │
·····················|·························|·············|·············|·············|··············|··············
|  ERC20Upgradeable  ·  transfer               ·      54650  ·      89952  ·      77374  ·           4  ·          -  │
·····················|·························|·············|·············|·············|··············|··············
|  UBI               ·  burn                   ·      46568  ·      53528  ·      50048  ·           4  ·          -  │
·····················|·························|·············|·············|·············|··············|··············
|  UBI               ·  changeProofOfHumanity  ·          -  ·          -  ·      31118  ·           1  ·          -  │
·····················|·························|·············|·············|·············|··············|··············
|  UBI               ·  reportRemoval          ·          -  ·          -  ·      36704  ·           1  ·          -  │
·····················|·························|·············|·············|·············|··············|··············
|  UBI               ·  startAccruing          ·          -  ·          -  ·      52957  ·           4  ·          -  │
·····················|·························|·············|·············|·············|··············|··············
|  Vote              ·  changeProofOfHumanity  ·          -  ·          -  ·      28445  ·           1  ·          -  │
·····················|·························|·············|·············|·············|··············|··············
|  Vote              ·  register               ·          -  ·          -  ·     243568  ·           2  ·          -  │
·····················|·························|·············|·············|·············|··············|··············
|  Vote              ·  snapshot               ·      29925  ·      44925  ·      39925  ·           3  ·          -  │
·····················|·························|·············|·············|·············|··············|··············
|  Deployments                                 ·                                         ·  % of limit  ·             │
···············································|·············|·············|·············|··············|··············
|  UBI                                         ·          -  ·          -  ·    1481868  ·      15.6 %  ·          -  │
···············································|·············|·············|·············|··············|··············
|  Vote                                        ·          -  ·          -  ·     803827  ·       8.5 %  ·          -  │
·----------------------------------------------|-------------|-------------|-------------|--------------|-------------·

If you can compare the values you see in your functions to the ones I pasted here, then we'll know if we are okay regarding gas.

[guidozanon]

Oh got it. I was running that but just to make sure about the unit tests. So basically this change does not make sense from the gas point of view, as it's quite worst.

[santisiri]

Thanks for checking it out. Great contribution nonetheless.. but yes, optimization more often than not means inconvenient code. Thanks @guidozanon!