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

Open
code423n4 opened this issue Apr 20, 2022 · 0 comments
Open

QA Report #207

code423n4 opened this issue Apr 20, 2022 · 0 comments
Labels
bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax

Comments

@code423n4
Copy link
Contributor

Issue 1 (Low) - Starting time check should be inclusive

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/SupplySchedule.sol#L184

The require statement should include the globalStartTimestamp. > should be changed to >=

Issue 2 (Low) - Front-runnable initializers

There are several initialize() functions that can be frontrun, requiring redeployment. Consider adding simple access control to these functions.

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/CitadelToken.sol#L22-L29

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/SupplySchedule.sol#L43-L46

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/SupplySchedule.sol#L43-L46

Issue 3 (Low)- Floating Pragma

A single contract contains a floating pragma. It is recommended to deploy all contracts with a single, specific compiler version to reduce the risk of compiler-specific bugs and contracts deployed with different versions. In the case of the forked contacts, I recommend deploying with the exact version that the current live versions were deployed with.

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/CitadelToken.sol#L2

Issue 4 (Low) - Code Consistency: Use of Days vs Seconds

In one contract, Days is used. In another 86400 seconds is used. For consistency, consider switching to a single method.

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/StakedCitadelVester.sol#L34

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/SupplySchedule.sol#L25

Issue 5 (Low)- Unnecessary parameter

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/KnightingRound.sol#L209

The tokenOutAmount_ parameter is not used. Instead it is always set in the function.

Issue 6 (Low) - CEI: Move state changes to after token transfer

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/KnightingRound.sol#L193-L199

Though no risk of reentrancy, it's a good practice to move all state changes to after the token is transferred.

Issue 7 (Non-critical) - Use all caps for constant

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/SupplySchedule.sol#L25

Issue 8 (Non-critical) - _pool parameter should be renamed _poolSize

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/StakedCitadel.sol#L880

_pool sounds like it's referring to the pool id.

Issue 9 (Non-critical) - Typo in comment

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/Funding.sol#L333

@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 Apr 20, 2022
code423n4 added a commit that referenced this issue Apr 20, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
Projects
None yet
Development

No branches or pull requests

1 participant