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 #109

Closed
howlbot-integration bot opened this issue Jun 21, 2024 · 7 comments
Closed

QA Report #109

howlbot-integration bot opened this issue Jun 21, 2024 · 7 comments
Labels
bug Something isn't working edited-by-warden grade-c QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@howlbot-integration
Copy link

howlbot-integration bot commented Jun 21, 2024

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

@howlbot-integration howlbot-integration bot added bug Something isn't working edited-by-warden QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sufficient quality report This report is of sufficient quality labels Jun 21, 2024
howlbot-integration bot added a commit that referenced this issue Jun 21, 2024
howlbot-integration bot added a commit that referenced this issue Jun 21, 2024
@c4-judge
Copy link
Contributor

c4-judge commented Jul 2, 2024

0xA5DF marked the issue as grade-b

@c4-judge c4-judge closed this as completed Jul 2, 2024
@c4-judge
Copy link
Contributor

c4-judge commented Jul 2, 2024

0xA5DF marked the issue as grade-c

@0xA5DF
Copy link

0xA5DF commented Jul 2, 2024

✅ Lack of checks for duplicate nominees in addNomineeEVM() and addNomineeNonEVM()

There's a check in the internal function

VoteWeighting.sol: Lack of input validation in getNextAllowedVotingTimes()

There is in the code that you quoted

StakingProxy.sol: Lack of access control for getImplementation() function

It's a view function

@c4-judge c4-judge added grade-c unsatisfactory does not satisfy C4 submission criteria; not eligible for awards and removed grade-b labels Jul 2, 2024
@CrystallineButterfly
Copy link

Hey @0xA5DF Thank you for judging, I notice I made mistakes with a couple of these, but I provided over 90, please re-review as I know most of these are valid. As given you said 3 are invalid there as still 88 that I suggest are valid.

All the best- Kell

@0xA5DF
Copy link

0xA5DF commented Jul 2, 2024

Those aren't just 3, those are 3 that I found by just skimming over the top of the report.
Have you checked the rest 88 that they're valid before you're asking me to review them?

For example, ✅ Lack of checks for duplicate service IDs in stake() is also false since staking transfers an ERC721

@CrystallineButterfly
Copy link

CrystallineButterfly commented Jul 3, 2024

@0xA5DF I made these lows. I understand I made mistakes but you are saying all 91 are invalid? As you skimmed over them. Please re-review the report as I made sure there is a lot more value in it than you are currently expressing. And the 3 you mentioned being invalid. The code snippets I did, I added the changes in each one. The original code isn't optimised in the ways I said. I respectfully ask you to re-evaluate the report and the 91 suggested lows.

All the best- K42

@0xA5DF
Copy link

0xA5DF commented Jul 3, 2024

I'm sorry, but this doesn't meet the standard expected for QA reports.
Too many false findings, and I don't see many valuable findings here.
Leaving as judged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working edited-by-warden grade-c QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

3 participants