-
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
61 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,61 @@ | ||
## 1. USE `safeTransferOwnership` INSTEAD OF `transferOwnership` | ||
Affected Code: File: QuestFactory.sol line [ 100](https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L100),[131](https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L131) | ||
Description: | ||
`transferOwnership` function is used to change Ownership from Owned.sol. | ||
|
||
Use a 2 structure transferOwnership which is safer. | ||
`safeTransferOwnership`, use it is more secure due to 2-stage ownership transfer. | ||
|
||
Recommendation: | ||
Use [Ownable2Step.sol](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/access/Ownable2Step.sol) | ||
|
||
## 2.LOCK PRAGMAS TO SPECIFIC COMPILER VERSION | ||
Affected Code: | ||
QuestFactory.sol line [2](https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L2) | ||
RabbitHoleReceipt.sol line [2](https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L2) | ||
RabbitHoleTickets.sol line [2](https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleTickets.sol#L2) | ||
RecieptRenderer.sol line [2](https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/ReceiptRenderer.sol#L2) | ||
Quest.sol line [2](https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Quest.sol#L2) | ||
IQuest.sol line [2](https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/interfaces/IQuest.sol#L2) | ||
IQuestFactory.sol line [2](https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/interfaces/IQuestFactory.sol#L2) | ||
\ | ||
Description: | ||
Pragma statements can be allowed to float when a contract is intended for consumption by other developers, as in the case with contracts in a library or EthPM package. Otherwise, the developer would need to manually update the pragma in order to compile locally. | ||
https://swcregistry.io/docs/SWC-103 | ||
|
||
Recommendation: | ||
Ethereum Smart Contract Best Practices - Lock pragmas to specific compiler version. | ||
solidity-specific/locking-pragmas | ||
|
||
## 3.Long lines are not suitable for the Solidity Style Guide | ||
Affected Code: | ||
RabbitHoleReceipt.sol [ #L158-160](https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L158),[ #L192](https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L190-L192) | ||
QuestFactory.sol [#L210](https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L210),[ #L219](https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L219) | ||
RabbitHoleTickets.sol [#L83](https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleTickets.sol#L83),[#L102](https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleTickets.sol#L102) | ||
Erc20Quest.sol [#L74](https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc20Quest.sol#L74) | ||
Description:It is generally recommended that lines in the source code should not exceed 80-120 characters. Today’s screens are much larger, so in some cases it makes sense to expand that. The lines above should be split when they reach that length, as the files will most likely be on GitHub and GitHub always uses a scrollbar when the length is more than 164 characters. | ||
|
||
See [why-is-80-characters-the-standard-limit-for-code-width](https://softwareengineering.stackexchange.com/questions/148677/why-is-80-characters-the-standard-limit-for-code-width) | ||
|
||
Recommendation: | ||
|
||
Multiline output parameters and return statements should follow the same style recommended for wrapping long lines found in the Maximum Line Length section. | ||
|
||
https://docs.soliditylang.org/en/v0.8.17/style-guide.html#introduction | ||
|
||
``` | ||
thisFunctionCallIsReallyLong( | ||
longArgument1, | ||
longArgument2, | ||
longArgument3 | ||
); | ||
``` | ||
## 4. Missing Event for critical parameters init and change | ||
Affected Code: | ||
Quest.sol[#L38-L43](https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Quest.sol#L38-L43) | ||
Erc20Quest.sol[#L38-L39](https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc20Quest.sol#L38-L39) | ||
Description: | ||
Events help non-contract tools to track changes, and events prevent users from being surprised by changes | ||
|
||
Recommendation: | ||
Add Event-Emit. |