code-423n4 / 2023-03-aragon-findings

0 stars 0 forks source link

Gas Optimizations #192

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

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

0xean commented 1 year ago

Looks like a lot of invalid suggestions auto generated from some tooling.

c4-judge commented 1 year ago

0xean marked the issue as grade-b

novaknole20 commented 1 year ago
  1. initialize's whole point is to ensure upgradeable contract can be initialized once. So constructor case which is mentioned in the issue is invalid.
  2. Use hardcode address instead address(this) - not a good suggestion. error-prone too much.
  3. State variables only set in the constructor should be declared immutable these contracts are deployed by Aragon, so we really didn't care much about gas here. can be disregarded.
  4. Remove unused struct - this mint config was used by some other contract, but got removed. This could be acceptable, but don't posses any gas optimization or security.
  5. Function Calls in a Loop Can Cause Denial of Service and out of gas due to not checking the Array length - this would not happen even for 5 plugins and if it happens, it's mostly a malicious user who would not gain anything.

Really hard to go through with everything. These seem to be some tooling results which we also ran. I'd love to disregard this. prove me otherwise why not.

Thank you.

c4-sponsor commented 1 year ago

novaknole20 marked the issue as sponsor disputed

c4-sponsor commented 1 year ago

novaknole20 requested judge review

0xean commented 1 year ago

There are both valid and invalid suggestions in this report. Yes, it definitely appears to be generated from automated tooling and lacks manual validation of the output of that tool.