Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Foundry's update on gas report output breaks compatibility #186

Closed
penandlim opened this issue Jan 2, 2025 · 4 comments · Fixed by #187
Closed

Foundry's update on gas report output breaks compatibility #186

penandlim opened this issue Jan 2, 2025 · 4 comments · Fixed by #187

Comments

@penandlim
Copy link
Contributor

New format outputs a table like below

Suite result: ok. 96 passed; 0 failed; 0 skipped; finished in 352.58s (3137.75s CPU time)

╭----------------------------------------------------------------------------+-----------------+-------+--------+-------+---------╮
| dependencies/euler-price-oracle-1/src/EulerRouter.sol:EulerRouter Contract |                 |       |        |       |         |
+=================================================================================================================================+
| Deployment Cost                                                            | Deployment Size |       |        |       |         |
|----------------------------------------------------------------------------+-----------------+-------+--------+-------+---------|
| 752721                                                                     | 3467            |       |        |       |         |
|----------------------------------------------------------------------------+-----------------+-------+--------+-------+---------|
|                                                                            |                 |       |        |       |         |
|----------------------------------------------------------------------------+-----------------+-------+--------+-------+---------|
| Function Name                                                              | Min             | Avg   | Median | Max   | # Calls |
|----------------------------------------------------------------------------+-----------------+-------+--------+-------+---------|
| getQuote                                                                   | 2375            | 4471  | 2377   | 8877  | 32821   |
|----------------------------------------------------------------------------+-----------------+-------+--------+-------+---------|
| govSetConfig                                                               | 29030           | 45203 | 48930  | 49146 | 1914    |
╰----------------------------------------------------------------------------+-----------------+-------+--------+-------+---------╯

Where as old format is like below

Test result: ok. 17 passed; 0 failed; finished in 118.90s
| contracts/compound/IncentivesVault.sol:IncentivesVault contract |                 |       |        |       |         |
|-----------------------------------------------------------------|-----------------|-------|--------|-------|---------|
| Deployment Cost                                                 | Deployment Size |       |        |       |         |
| 640594                                                          | 3430            |       |        |       |         |
| Function Name                                                   | min             | avg   | median | max   | # calls |
| MAX_BASIS_POINTS                                                | 305             | 305   | 305    | 305   | 1       |
| bonus                                                           | 318             | 318   | 318    | 318   | 1       |
| incentivesTreasuryVault                                         | 415             | 415   | 415    | 415   | 1       |
| isPaused                                                        | 377             | 377   | 377    | 377   | 2       |
| oracle                                                          | 393             | 393   | 393    | 393   | 1       |
| setBonus                                                        | 2452            | 15520 | 21480  | 25580 | 5       |
| setIncentivesTreasuryVault                                      | 2605            | 7111  | 8614   | 8614  | 4       |
| setOracle                                                       | 2606            | 5610  | 5610   | 8615  | 2       |
| setPauseStatus                                                  | 1436            | 13858 | 14151  | 25694 | 4       |
| tradeCompForMorphoTokens                                        | 443             | 33672 | 36386  | 65651 | 6       |
| transferTokensToDao                                             | 2604            | 20542 | 20542  | 38481 | 2       |

Which causes the foundry-gas-diff action to fail to parse the lines.

@Rubilmax
Copy link
Owner

Rubilmax commented Jan 2, 2025

Thanks for the issue! Will fix that asap

@Rubilmax
Copy link
Owner

Rubilmax commented Jan 7, 2025

Fixed by faff2f7 available in v3.20 (v3 points to it now)

@penandlim
Copy link
Contributor Author

Hey @Rubilmax looks like a slightly different format was used for the mocks. Updated it with the above format in #187

@ilovelili
Copy link

Hi @Rubilmax it looks the old format is good but the new format is broken? In our case the gas report output is like the bellowing and the unit test fails with No deployment cost or deployment size found. Is this a forge gas report?

╭--------------------------------------------------------------------------+-----------------+-------+--------+-------+---------╮
| test/TransferPoolFeeCalculator.t.sol:MockStableTokenPriceOracle Contract |                 |       |        |       |         |
+===============================================================================================================================+
| Deployment Cost                                                          | Deployment Size |       |        |       |         |
|--------------------------------------------------------------------------+-----------------+-------+--------+-------+---------|
| 291141                                                                   | 1128            |       |        |       |         |
|--------------------------------------------------------------------------+-----------------+-------+--------+-------+---------|
|                                                                          |                 |       |        |       |         |
|--------------------------------------------------------------------------+-----------------+-------+--------+-------+---------|
| Function Name                                                            | Min             | Avg   | Median | Max   | # Calls |
|--------------------------------------------------------------------------+-----------------+-------+--------+-------+---------|
| getCurrentPriceAndDecimals                                               | 672             | 1172  | 672    | 4672  | 16      |
|--------------------------------------------------------------------------+-----------------+-------+--------+-------+---------|
| getCurrentPriceDeviationStatus                                           | 469             | 1135  | 469    | 2469  | 3       |
|--------------------------------------------------------------------------+-----------------+-------+--------+-------+---------|
| setCurrentPrice                                                          | 66245           | 66245 | 66245  | 66245 | 10      |
|--------------------------------------------------------------------------+-----------------+-------+--------+-------+---------|
| setPriceDeviationStatus                                                  | 21797           | 38482 | 43709  | 43709 | 8       |
╰--------------------------------------------------------------------------+-----------------+-------+--------+-------+---------╯

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants