QA Report

[c4-submissions]

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

[c4-pre-sort]

bytes032 marked the issue as sufficient quality report

[c4-pre-sort]

bytes032 marked the issue as insufficient quality report

[bytes032]

Code does not follow the best practice of check-effects-interaction TODO Consider implementing two-step procedure for updating protocol addresses -3 Division by zero not prevented TODO Empty receive()/fallback() function -3 Loss of precision -3 Missing checks for address(0x0) in the constructor -3 Missing checks for address(0x0) when updating address state variables -3 Missing contract-existence checks before yul call() TODO Numbers downcast to addresses may result in collisions -3 receive()/payable fallback() function does not authorize requests -r require() should be used instead of assert() -3 safeApprove() is deprecated -3 Some tokens may revert when large transfers are made -3 State variables not capped at reasonable values TODO Subtraction may underflow if multiplication is too large TODO Large or complicated code bases should implement invariant tests TODO Adding a return statement when the function defines a named return variable, is redundant TODO Missing checks for address(0) when assigning values to address state variables -3 Array indices should be referenced via enums rather than via numeric literals TODO Array is push()ed but not pop()ed -3 Assembly blocks should have extensive comments TODO Avoid the use of sensitive terms TODO Cast to bytes or bytes32 for clearer semantic meaning TODO Complex casting TODO Common functions should be refactored to a common base contract TODO Consider adding formal verification proofs TODO Consider bounding input array length TODO Consider moving msg.sender checks to a common authorization modifier TODO Consider using delete rather than assigning zero/false to clear values TODO Consider using descriptive constants when passing zero as a function argument TODO Constant redefined elsewhere TODO Constants in comparisons should appear on the left side TODO constants should be defined rather than using magic numbers TODO Contracts should expose an interface TODO Contract timekeeping will break earlier than the Ethereum network itself will stop working TODO Contracts should have full test coverage TODO Control structures do not follow the Solidity Style Guide TODO Custom errors should be used rather than revert()/require() TODO else-block not required TODO Empty bytes check is missing TODO Empty function body TODO Error messages should be descriptive, rather than cryptic TODO Event is not properly indexed TODO Events are missing sender information TODO Events may be emitted out of order due to reentrancy TODO Events should use parameters to convey information TODO Events that mark critical parameter changes should contain both the old and the new value TODO Expressions for constant values should use immutable rather than constant TODO High cyclomatic complexity TODO if-statement can be converted to a ternary TODO Import declarations should import specific identifiers, rather than the whole file TODO Interfaces should be defined in separate files from their usage TODO internal functions not called by the contract should be removed TODO Large numeric literals should use underscores for readability TODO Long functions should be refactored into multiple, smaller, functions TODO Missing checks constructor/initializer assignments TODO Missing checks for state variable assignments TODO Multiple address/ID mappings can be combined into a single mapping of an address/ID to a struct, for readability TODO Named imports of parent contracts are missing TODO NatSpec: Contract declarations should have @author tags -3 NatSpec: Contract declarations should have descriptions -3 NatSpec: Contract declarations should have @dev tags -3 NatSpec: Contract declarations should have @notice tags -3 NatSpec: Contract declarations should have @title tags -3 NatSpec: Event declarations should have descriptions -3 NatSpec: file is missing NatSpec -3 NatSpec: Function @param tag is missing -3 NatSpec: Function @return tag is missing -3 NatSpec: Function declarations should have @notice tags -3 NatSpec: Function declarations should have descriptions -3 NatSpec: Modifier declarations should have descriptions -3 NatSpec: state variable declarations should have descriptions -3 Not using the named return variables anywhere in the function is confusing TODO Overflows in unchecked blocks TODO Polymorphic functions make security audits more time-consuming and error-prone TODO Functions not used internally could be marked external TODO pure function accesses storage TODO Duplicated require()/revert() checks should be refactored to a modifier or function. TODO require()/revert() statements should have descriptive reason strings TODO Return values of approve() not checked -3 Setters should prevent re-setting of the same value TODO Style guide: Contract does not follow the Solidity style guide’s suggested layout ordering TODO Style guide: Function names should use lowerCamelCase TODO Style guide: Function ordering does not follow the Solidity style guide TODO Style guide: Lines are too long TODO Style guide: Non-external/public function names should begin with an underscore TODO Style guide: Non-external/public variable names should begin with an underscore TODO Style guide: State and local variables should be named using lowerCamelCase TODO Style guide: Strings should use double quotes rather than single quotes TODO Style guide: Top level pragma declarations should be separated by two blank lines TODO Style guide: Variable names for constants are improperly named TODO Style guide: Variable names for immutables should use CONSTANT_CASE TODO TODO Left in the code TODO Unused event definition TODO Unused public contract variable TODO Unused file TODO Unused function parameter TODO Unused import TODO Use bit shifts in an immutable variable rather than long bit masks of a single bit, for readability TODO Use bytes.concat() on bytes instead of abi.encodePacked() for clearer semantic meaning TODO Use of override is unnecessary. TODO Utility contracts can be made into libraries TODO Variables need not be initialized to zero TODO Visibility should be set explicitly rather than defaulting to internal TODO 123: _data = bytes32(1 << uint256(roleIds)); TODO

[c4-judge]

jhsagd76 marked the issue as grade-c