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

4 stars 0 forks source link

QA Report #455

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

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

c4-judge commented 1 year ago

hansfriese marked the issue as grade-a

c4-sponsor commented 1 year ago

TimTinkers marked the issue as sponsor acknowledged

TimTinkers commented 1 year ago

This report is acknowledged and the effort that went into it is greatly appreciated. However, compared to several other QA reports it includes recommendations that run counter to our assumptions of an honest and competent administrator. It also includes some suggestions that I feel are matters of opinion.

  1. Disagree. We can fundamentally make this safe while maintaining the gas benefits of unchecked math.
  2. Why? This seems entirely stylistic and not something I am aware of mattering. The function IMO is still readable.
  3. Totally valid.
  4. Totally valid.
  5. Was clarified on Discord that this is an administrative misconfiguration error.
  6. Likewise this is an administrative misconfiguration error.
  7. Stylistically valid but pointless in the presence of the reentrancy guard.
  8. Why? This seems stylistic.
  9. Okay.
  10. Okay.
  11. No thanks, I'll use capital letters for these admin-configured "constants" which in practice will probably actually remain constant once their deployed addresses are known (there's even a locking function).
  12. We assume a competent admin.
  13. We assume a competent admin.
  14. The storage variables referenced from the mapping are directly manipulated.
hansfriese commented 1 year ago

1, 5, 6, and 7 seem to be invalid or out of scope and most of the others are non-critical.

c4-judge commented 1 year ago

hansfriese marked the issue as grade-b