code-423n4 / 2024-03-zksync-findings

1 stars 1 forks source link

Gas Optimizations #115

Closed c4-bot-7 closed 2 months ago

c4-bot-7 commented 4 months ago

See the markdown file with the details of this report here.

c4-sponsor commented 3 months ago

saxenism (sponsor) confirmed

saxenism commented 3 months ago

Okayish Report. Nothing to write home about.

c4-judge commented 2 months ago

alex-ppg marked the issue as grade-c

albahaca0000 commented 2 months ago

Hi @alex-ppg thanks for judging

Thank you for your review. Regarding the gas findings, I followed a specific approach. I used a checklist to manually examine each finding.

I discovered expensive issues in my report that were not only identified by me but also by other wardens. These issues are particularly costly to address. Despite being marked as grade-A reports, not all of them were thoroughly examined In one report of other warden, these high-cost issues were notably overlooked, underscoring the importance of addressing such critical matters comprehensively.

I've identified additional issues in my report, but the ones mentioned above stand out due to their significant impact on gas usage

Additionally, it's worth noting that some wardens may not have conducted an exhaustive examination of the entire codebase, resulting in the identification of only a limited number of instances for each issue. In contrast, I meticulously reviewed the entirety of the codebase. Furthermore, while some issues may have been detected by automated bots, I ensured to cross-check with C4 bots. Moreover, where necessary, I included mitigation code for certain issues in my report.

this is summary of my report

issue instance
[G‑01] Use calldata instead of memory for function arguments that do not get mutated 17
[G‑02] Using storage instead of memory for structs/arrays saves gas 15
[G‑03] State variables that are used multiple times in a function should be cached in stack variables 4
[G‑04] Multiple address/ID mappings can be combined into a single mapping of an address/ID to a struct, where appropriate 8
[G‑05] Use inline assembly to check for address(0) 50
[G‑06] Structs can be packed into fewer storage slots 4
[G‑07] Constructors can be marked payable 10
[G‑08] State variable read in a loop 5
[G‑09] Consider using OZ EnumerateSet in place of nested mappings 8
[G‑10] Multiple accesses of a mapping/array should use a local variable cache 40
[G‑11] Internal functions only called once can be inlined to save gas 53
[G‑12] Use assembly to write address storage values 13
[G‑13] Use assembly to calculate hashes to save gas 59
[G‑14] REQUIRE()/REVERT() STRINGS LONGER THAN 32 BYTES COST EXTRA GAS 66
[G‑15] Sort Solidity operations using short-circuit mode 2
[G‑16] Avoid emitting storage values 12
[G‑17] Can Make The Variable Outside The Loop To Save Gas 20
[G‑18] A modifier used only once and not being inherited should be inlined to save gas 7
[G‑19] Refactor external/internal function to avoid unnecessary External Calls 3
[G‑20] Avoid updating storage when the value hasn't changed 31
[G‑21] Use assembly to emit an event 77
[G‑22] Do-While loops are cheaper than for loops 6
[G‑23] Initializers can be marked payable 8
[G‑24] require() or revert() statements that check input arguments should be at the top of the function 6
[G‑25] Use nested if statements instead of && 10
[G‑26] Amounts should be checked for 0 before calling a transfer 3
[G‑27] abi.encode() is less efficient than abi.encodePacked() 17
[G‑28] Do not calculate constants 2
[G‑29] Delete variables that you don't need 2
[G‑30] Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead 20
[G‑31] Storage Slot Reduction via Variable Resizing 2
[G‑32] Avoid fetching a low-level call's return data by using assembly 3
[G‑33] Use hardcode address instead address(this) 5
[G‑34] Duplicated require()/revert() checks should be refactored to a modifier or function 3
[G‑35] Unused named return variables without optimizer waste gas 4
[G‑36] Stack variable used as a cheaper cache for a state variable is only used once 13
[G‑37] Use of emit inside a loop 3
[G‑38] Structs should group like types together to save gas 11
[G‑39] Don't compare boolean expressions to boolean literals 1
[G‑40] Use selfbalance() instead of address(this).balance 4
[G‑41] Use assembly in place of abi.decode to extract calldata values more efficiently 10
[G‑42] "" has the same value as new bytes(0) but costs less gas 8
[G‑43] State variables only set in their definitions should be declared constant 3
[G‑44] Fewer storage slots can be used by storing timestamps in types smaller than uint256 3
[G‑45] Storage re-read via storage pointer 2
[G‑46] keccak256() should only need to be called on a specific string literal once 4
[G‑47] Private functions used once can be inlined 6
[G‑48] >=/<= costs less gas than >/< 3
[G‑49] Assigning state variables directly with named struct constructors wastes gas 1
[G‑50] Most important: avoid zero to one storage writes where possible 2
[G‑51] Instead of counting down in for statements, count up ~

I believe my report merits a grade-A

thanks agin for judging .

alex-ppg commented 2 months ago

Hey @albahaca0000, thanks for contributing to the PJQA process! The gas optimization report has been graded as C due to containing invalid findings, incorrect contents, and being imprecise. As you have raised 25 concerns, I will pick the first 13 to demonstrate this:

We can observe above that 9 out of 13 are invalid recommendations and some lead to a gas increase. The report has been graded as C, and this grade will remain whilst no further input is expected.

albahaca0000 commented 2 months ago

Hi @alex-ppg

Thank you once again for your meticulous assessment of my gas optimization report. Your insights have been incredibly valuable, and I'm grateful for the opportunity to learn and improve from your feedback.

I understand your rationale in selecting 13 issues from my report for evaluation, highlighting both valid and invalid recommendations. However, I'd like to emphasize that similar instances have been found in other grade-A reports, indicating a level of subjectivity in the evaluation process.

Upon reviewing other grade-A reports, it's evident that they too contain a mix of valid and invalid recommendations. Each report is a product of individual assessment and interpretation. While some issues may align with best practices, others may require further scrutiny.

Considering the nature of the optimization recommendations and their applicability, it's important to recognize that some instances may appear in various reports, both as valid and invalid findings. In my submission of 51 issues, if we extrapolate from the 13 instances you've assessed and considering that approximately 4 out of every 13 issues are deemed valid, it's reasonable to expect around 16 valid recommendations in total.

Based on this assessment, I respectfully request reconsideration of my report's grade to either A or B. I firmly believe that a significant portion of the issues identified in my report hold merit and contribute to the overall optimization effort.

I'm committed to ensuring the quality and accuracy of my submissions, and I will certainly take your feedback into account in refining future reports. However, I believe it's crucial to maintain a balanced perspective when evaluating the merit of each recommendation.

Therefore, I kindly urge a thorough review of all aspects of my report, taking into consideration the potential validity of a broader range of issues. Your comprehensive assessment will undoubtedly contribute to a fair and accurate grading process.

Thank you once again for your attention to detail and constructive feedback.

alex-ppg commented 2 months ago

Hey @albahaca0000, thanks for contributing further to the PJQA process. I did not select the 13 issues I evaluated as those were highlighted in your earlier response as high-cost issues, and I selected the upper half of the issues. They represent the highest-value findings in your gas optimization report based on your selection criteria.

While extrapolating the 4 out of every 13 metric is a correct approach and would yield 16 valid recommendations as a "median", the actual value provided by the optimizations needs to be taken into account. Significant gas savings outweigh minuscule gas savings, such as G-07, and influence the ultimate score of a report to a far greater degree.

I reviewed your submission and identified that most issues that would be considered valid represent minuscule optimizations that would also significantly hinder readability, and thus do not amount to a cumulative savings value that would result in the report being assigned a B grade.

I understand that this is not the outcome you were hoping for, and I am more than happy to provide you feedback as to how to improve your bot, but I would like to highlight that the zkSync Era contest was active for 3 weeks during which meticulous quality assessment could have been carried out. I concur with your opinion that other reports also contain a mixture of invalid and valid findings, however, they also contain unique items that led to significant gas savings as the Sponsors themselves have confirmed.

I advise identifying those unique findings that were missing from your bot's report and incorporating them for future contests to increase the rewards you capture. If you have any concerns with the present judgment, you are more than happy to open a C4 organization issue to discuss them but PJQA has concluded and no further input is requested for this submission.