search

code-423n4 / 2022-08-foundation-findings

0 stars 0 forks source link

QA Report #285

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

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

HardlyDifficult commented 2 years ago

Unresolved TODO comments

Agree, will fix.

abi.encodePacked() should not be used

It's a gas savings to use encodePacked for the use cases we support here. There does not appear to be a compelling reason to change.

Missing natspec comments

Fair feedback -- for natspec we aim for complete coverage of the public interfaces but for internal/private/libraries we have some gaps in order to reduce redundancy, for those we aim to include comments where things are unclear or not obvious from the function and variable names.

@inheritdoc is a natspec supported standard that effectively inlines comments from another file -- for those examples we should have complete coverage already.

BuyReferralPaid event should index buyReferrer

Agree but won't fix at this time. We have already started emitting this event on mainnet, to simplify integrations such as subgraph we are going to continue with this event as is. However it's a good catch and I agree that buyReferrer ideally would have been indexed here.

N-03 Remove commented out code

Disagree - this comment is detailing how that awkwardly placed gap was previously used so future me can understand why it's important for upgrade compatibility that we don't leverage that space again (since those slots are dirty with data).