-
Notifications
You must be signed in to change notification settings - Fork 2
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
1 changed file
with
80 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
|
|
||
| ## QUEST SHOULD NOT BE PAUSED IF IT HAS NOT STARTED YET | ||
|
|
||
| ### Description: | ||
|
|
||
| In the function pause here https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Quest.sol#L57 there should be a check something like `if(!hasStarted) revert` | ||
| because we should not be able to pause a quest that has not been started yet. | ||
|
|
||
|
|
||
| ## DECLARE A VARIABLE FOR MAXIMUM FEE | ||
|
|
||
| ### Description: | ||
|
|
||
| Instead of using `10_000` a global constant variable , something like MAX_QUEST_FEE should be declared and used to increase code readabiluty. | ||
| https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L187 | ||
|
|
||
| ## questFee SHOULD HAVE A MINIMUM CHECK TOO | ||
|
|
||
| ### Description: | ||
|
|
||
| The setter for questFee here https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L186 should have a minimum check too , something like | ||
| require(questFee > MIN_FEE) or should not be 0 . | ||
|
|
||
| ## USE OF BYTES.CONCAT() INSTEAD OF ABI.ENCODEPACKED(,) | ||
|
|
||
| ### Description: | ||
|
|
||
| Rather than using abi.encodePacked for appending bytes, since version 0.8.4, bytes.concat() is enabled | ||
|
|
||
| Since version 0.8.4 for appending bytes, bytes.concat() can be used instead of abi.encodePacked(,). | ||
|
|
||
| Affected Instances: | ||
| https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L72 | ||
| https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L105 | ||
| https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L211 | ||
| https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L222 | ||
|
|
||
| ## LACK OF ZERO ADDRESS/AMOUNT CHECKS | ||
|
|
||
| ### Description: | ||
|
|
||
| There lacks zero address check at some places throughout the codebase , it can be problematic if these checks are missed in functions like | ||
| transfer(transferring to a zero address i.e. loss of funds) , mint or setting up privileged roles. | ||
|
|
||
| Affected instances: | ||
| https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L179 | ||
| https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L46 | ||
| https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L84 | ||
| https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L91 (fee should not be 0) | ||
| https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L98 | ||
| https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L149 | ||
| https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L150 | ||
| https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleTickets.sol#L35 | ||
| https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleTickets.sol#L66 | ||
| https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleTickets.sol#L73 | ||
| https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleTickets.sol#L83 | ||
| https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleTickets.sol#L93 | ||
| https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc20Quest.sol#L81 | ||
| https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc20Quest.sol#L38 | ||
| https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc1155Quest.sol#L54 | ||
|
|
||
| ## MISSING COMMENTS/INFORMATION ABOUT STORAGE VARIABLES | ||
|
|
||
| ### Description: | ||
|
|
||
| Even though all the functions have been well documented the storage variables lacks information , there should be proper comments | ||
| describing the purpose of each storage variable , for example here https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L30-L36 there is no information about these variables. | ||
|
|
||
| ## ALWAYS CHECK IF OPENZEPPELIN CONTRACTS ARE UP TO DATE | ||
|
|
||
| ### Description: | ||
|
|
||
| The openzeppelin version used is 4.8.0 whereas the latest version is 4.8.1 , it should always be ensured that latest version is used to avoid errors/bugs | ||
| in the previous versions. | ||
|
|
||
| ## TYPO | ||
|
|
||
| ### Description: | ||
|
|
||
| Change `remave` to `remove` here https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L176 |