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

QA Report #51

Open
code423n4 opened this issue Sep 3, 2023 · 4 comments
Open

QA Report #51

code423n4 opened this issue Sep 3, 2023 · 4 comments
Labels
bug Something isn't working grade-a Q-34 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue sufficient quality report This report is of sufficient quality

Comments

@code423n4
Copy link
Contributor

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

@code423n4 code423n4 added bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax labels Sep 3, 2023
code423n4 added a commit that referenced this issue Sep 3, 2023
code423n4 added a commit that referenced this issue Sep 3, 2023
@c4-pre-sort c4-pre-sort added the sufficient quality report This report is of sufficient quality label Sep 8, 2023
@c4-pre-sort
Copy link

raymondfam marked the issue as sufficient quality report

@c4-judge
Copy link
Contributor

kirk-baird marked the issue as grade-a

@ali2251
Copy link

ali2251 commented Sep 25, 2023

NC-1 | ADD A TIMELOCK TO CRITICAL FUNCTIONS.
This will increase complexity and not needed for setting rate limits

NC-2 | ADD TO BLACKLIST FUNCTION
no NFTS are used anywhere in the code so this is completely irrelevant

NC-3 | GENERATE PERFECT CODE HEADERS EVERY TIME
this makes sense but doesnt add much value

NC-4 | SAME CONSTANT REDEFINED ELSEWHERE
there is one constant defined in 2 places which is absolutely fine, hence no value add

NC-5 | IMPLEMENTATION CONTRACT MAY NOT BE INITIALIZED
This is not true because initializers are disabled in the contructor

NC-6 | MARK VISIBILITY OF INITIALIZE(…) FUNCTIONS AS EXTERNAL
doesnt matter because there are no child contracts

NC-7 | CONSTANT VALUES SUCH AS A CALL TO KECCAK256(), SHOULD USE IMMUTABLE RATHER THAN CONSTANT
doesnt make sense at all, there is no reason for a deterministic function to be immutable unless it can be changed in the constructor

NC-8 | LACK OF EVENT EMISSION AFTER CRITICAL INITIALIZE() FUNCTIONS
its arguable whether it needs an event since no one uses it

NC-9 | FOR EXTENDED “USING-FOR” USAGE, USE THE LATEST PRAGMA VERSION
its not required

NC-10 | NO SAME VALUE INPUT CONTROL
doesnt make sense

NC-11 | Add parameter to event-emit
NC-12 | SOLIDITY COMPILER OPTIMIZATIONS CAN BE PROBLEMATIC
NC-13 | LINES ARE TOO LONG

as shown above, most of these issues does either not exist or are improvements that dont help security or readability

@c4-sponsor
Copy link

ali2251 (sponsor) disputed

@c4-sponsor c4-sponsor added the sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue label Sep 25, 2023
@C4-Staff C4-Staff added the Q-34 label Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working grade-a Q-34 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

6 participants