| Issue | Instances | |
|---|---|---|
| GAS-1 | Use assembly to check for address(0) |
2 |
| GAS-2 | Using bools for storage incurs overhead | 5 |
| GAS-3 | Cache array length outside of loop | 2 |
| GAS-4 | State variables should be cached in stack variables rather than re-reading them from storage | 2 |
| GAS-5 | Use calldata instead of memory for function arguments that do not get mutated | 26 |
| GAS-6 | For Operations that will not overflow, you could use unchecked | 214 |
| GAS-7 | Don't initialize variables with default value | 7 |
| GAS-8 | Long revert strings | 1 |
| GAS-9 | Functions guaranteed to revert when called by normal users can be marked payable |
25 |
| GAS-10 | ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too) |
8 |
| GAS-11 | Using private rather than public for constants, saves gas |
1 |
| GAS-12 | Use != 0 instead of > 0 for unsigned integer comparison | 1 |
Saves 6 gas per instance
Instances (2):
File: QuestFactory.sol
70: if (quests[questId_].questAddress != address(0)) revert QuestIdUsed();
166: if (protocolFeeRecipient_ == address(0)) revert AddressZeroNotAllowed();
Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from ‘false’ to ‘true’, after having been ‘true’ in the past. See source.
Instances (5):
File: Quest.sol
19: bool public hasStarted;
20: bool public isPaused;
24: mapping(uint256 => bool) private claimedList;
File: QuestFactory.sol
20: mapping(address => bool) addressMinted;
30: mapping(address => bool) public rewardAllowlist;
If not cached, the solidity compiler will always read the length of the array during each iteration. That is, if it is a storage array, this is an extra sload operation (100 additional extra gas for each iteration except for the first) and if it is a memory array, this is an extra mload operation (3 additional gas for each iteration except for the first).
Instances (2):
File: Quest.sol
70: for (uint i = 0; i < tokenIds_.length; i++) {
104: for (uint i = 0; i < tokens.length; i++) {
[GAS-4] State variables should be cached in stack variables rather than re-reading them from storage
The instances below point to the second+ access of a state variable within a function. Caching of a state variable replaces each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.
Saves 100 gas per instance
Instances (2):
File: QuestFactory.sol
115: address(rabbitholeReceiptContract)
146: _revokeRole(CREATE_QUEST_ROLE, account_);
Mark data types as calldata instead of memory where possible. This makes it so that the data is not automatically loaded into memory. If the data passed into the function does not need to be changed (like updating values in an array), it can be passed in as calldata. The one exception to this is if the argument must later be passed into another function that takes an argument that specifies memory storage.
Instances (26):
File: Erc1155Quest.sol
19: string memory questId_,
File: Erc20Quest.sol
23: string memory questId_,
File: Quest.sol
32: string memory questId_,
File: QuestFactory.sol
67: string memory contractType_,
68: string memory questId_
193: function getNumberMinted(string memory questId_) external view returns (uint) {
199: function questInfo(string memory questId_) external view returns (address, uint, uint) {
210: function recoverSigner(bytes32 hash_, bytes memory signature_) public pure returns (address) {
219: function mintReceipt(string memory questId_, bytes32 hash_, bytes memory signature_) public {
219: function mintReceipt(string memory questId_, bytes32 hash_, bytes memory signature_) public {
File: RabbitHoleReceipt.sol
98: function mint(address to_, string memory questId_) public onlyMinter {
110: string memory questId_,
File: RabbitHoleTickets.sol
83: function mint(address to_, uint256 id_, uint256 amount_, bytes memory data_) public onlyMinter {
94: uint256[] memory ids_,
95: uint256[] memory amounts_,
96: bytes memory data_
File: ReceiptRenderer.sol
23: string memory questId_,
42: string memory questId_,
82: function generateAttribute(string memory key, string memory value) public pure returns (string memory) {
82: function generateAttribute(string memory key, string memory value) public pure returns (string memory) {
100: function generateSVG(uint tokenId_, string memory questId_) public pure returns (string memory) {
File: interfaces/IQuestFactory.sol
19: function questInfo(string memory questId_) external view returns (address, uint, uint);
File: test/SampleERC20.sol
27: string memory name,
28: string memory symbol,
File: test/TestERC20.sol
7: constructor(string memory name_, string memory symbol_, uint amountToMint) ERC20(name_, symbol_) {
7: constructor(string memory name_, string memory symbol_, uint amountToMint) ERC20(name_, symbol_) {
113: uint256 royaltyPayment = (salePrice_ * royaltyFee) / 10_000;
113: uint256 royaltyPayment = (salePrice_ * royaltyFee) / 10_000;
16: _mint(to, amount - old);
18: _burn(to, old - amount);
Instances (7):
File: Quest.sol
70: for (uint i = 0; i < tokenIds_.length; i++) {
103: uint256 redeemableTokenCount = 0;
104: for (uint i = 0; i < tokens.length; i++) {
File: RabbitHoleReceipt.sol
115: uint foundTokens = 0;
117: for (uint i = 0; i < msgSenderBalance; i++) {
126: uint filterTokensIndexTracker = 0;
128: for (uint i = 0; i < msgSenderBalance; i++) {
Instances (1):
File: RabbitHoleReceipt.sol
161: require(_exists(tokenId_), 'ERC721URIStorage: URI query for nonexistent token');
If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.
Instances (25):
File: Erc1155Quest.sol
54: function withdrawRemainingTokens(address to_) public override onlyOwner {
File: Erc20Quest.sol
81: function withdrawRemainingTokens(address to_) public override onlyOwner {
102: function withdrawFee() public onlyAdminWithdrawAfterEnd {
File: Quest.sol
50: function start() public virtual onlyOwner {
57: function pause() public onlyOwner onlyStarted {
63: function unPause() public onlyOwner onlyStarted {
96: function claim() public virtual onlyQuestActive {
150: function withdrawRemainingTokens(address to_) public virtual onlyOwner onlyAdminWithdrawAfterEnd {}
File: QuestFactory.sol
142: function changeCreateQuestRole(address account_, bool canCreateQuest_) public onlyOwner {
159: function setClaimSignerAddress(address claimSignerAddress_) public onlyOwner {
165: function setProtocolFeeRecipient(address protocolFeeRecipient_) public onlyOwner {
172: function setRabbitHoleReceiptContract(address rabbitholeReceiptContract_) public onlyOwner {
179: function setRewardAllowlistAddress(address rewardAddress_, bool allowed_) public onlyOwner {
186: function setQuestFee(uint256 questFee_) public onlyOwner {
File: RabbitHoleReceipt.sol
65: function setReceiptRenderer(address receiptRenderer_) public onlyOwner {
71: function setRoyaltyRecipient(address royaltyRecipient_) public onlyOwner {
77: function setQuestFactory(address questFactory_) public onlyOwner {
83: function setMinterAddress(address minterAddress_) public onlyOwner {
90: function setRoyaltyFee(uint256 royaltyFee_) public onlyOwner {
98: function mint(address to_, string memory questId_) public onlyMinter {
File: RabbitHoleTickets.sol
54: function setTicketRenderer(address ticketRenderer_) public onlyOwner {
60: function setRoyaltyRecipient(address royaltyRecipient_) public onlyOwner {
66: function setRoyaltyFee(uint256 royaltyFee_) public onlyOwner {
73: function setMinterAddress(address minterAddress_) public onlyOwner {
83: function mint(address to_, uint256 id_, uint256 amount_, bytes memory data_) public onlyMinter {
Saves 5 gas per loop
Instances (8):
File: Quest.sol
70: for (uint i = 0; i < tokenIds_.length; i++) {
104: for (uint i = 0; i < tokens.length; i++) {
106: redeemableTokenCount++;
File: QuestFactory.sol
226: quests[questId_].numberMinted++;
File: RabbitHoleReceipt.sol
117: for (uint i = 0; i < msgSenderBalance; i++) {
121: foundTokens++;
128: for (uint i = 0; i < msgSenderBalance; i++) {
131: filterTokensIndexTracker++;
If needed, the values can be read from the verified contract source code, or if there are multiple values there can be a single getter function that returns a tuple of the values of all currently-public constants. Saves 3406-3606 gas in deployment gas due to the compiler not having to create non-payable getter functions for deployment calldata, not having to store the bytes of the value outside of where it's used, and not adding another entry to the method ID table
Instances (1):
File: QuestFactory.sol
17: bytes32 public constant CREATE_QUEST_ROLE = keccak256('CREATE_QUEST_ROLE');
Instances (1):
File: RabbitHoleReceipt.sol
129: if (tokenIdsForQuest[i] > 0) {
| Issue | Instances | |
|---|---|---|
| NC-1 | Missing checks for address(0) when assigning values to address state variables |
8 |
| NC-2 | Event is missing indexed fields |
1 |
| NC-3 | Functions not used internally could be marked external | 32 |
Instances (8):
File: Erc20Quest.sol
39: protocolFeeRecipient = protocolFeeRecipient_;
File: Quest.sol
40: rewardToken = rewardTokenAddress_;
File: QuestFactory.sol
45: claimSignerAddress = claimSignerAddress_;
160: claimSignerAddress = claimSignerAddress_;
File: RabbitHoleReceipt.sol
52: royaltyRecipient = royaltyRecipient_;
72: royaltyRecipient = royaltyRecipient_;
File: RabbitHoleTickets.sol
41: royaltyRecipient = royaltyRecipient_;
61: royaltyRecipient = royaltyRecipient_;
Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (three fields). Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.
Instances (1):
File: interfaces/IQuest.sol
8: event Claimed(address account, uint256 amount);
Instances (32):
File: Erc1155Quest.sol
33: function start() public override {
54: function withdrawRemainingTokens(address to_) public override onlyOwner {
File: Erc20Quest.sol
58: function start() public override {
81: function withdrawRemainingTokens(address to_) public override onlyOwner {
102: function withdrawFee() public onlyAdminWithdrawAfterEnd {
File: Quest.sol
57: function pause() public onlyOwner onlyStarted {
63: function unPause() public onlyOwner onlyStarted {
140: function getRewardAmount() public view returns (uint256) {
145: function getRewardToken() public view returns (address) {
File: QuestFactory.sol
37: function initialize(
61: function createQuest(
142: function changeCreateQuestRole(address account_, bool canCreateQuest_) public onlyOwner {
159: function setClaimSignerAddress(address claimSignerAddress_) public onlyOwner {
172: function setRabbitHoleReceiptContract(address rabbitholeReceiptContract_) public onlyOwner {
179: function setRewardAllowlistAddress(address rewardAddress_, bool allowed_) public onlyOwner {
219: function mintReceipt(string memory questId_, bytes32 hash_, bytes memory signature_) public {
File: RabbitHoleReceipt.sol
43: function initialize(
65: function setReceiptRenderer(address receiptRenderer_) public onlyOwner {
71: function setRoyaltyRecipient(address royaltyRecipient_) public onlyOwner {
77: function setQuestFactory(address questFactory_) public onlyOwner {
83: function setMinterAddress(address minterAddress_) public onlyOwner {
90: function setRoyaltyFee(uint256 royaltyFee_) public onlyOwner {
98: function mint(address to_, string memory questId_) public onlyMinter {
109: function getOwnedTokenIdsOfQuest(
File: RabbitHoleTickets.sol
32: function initialize(
54: function setTicketRenderer(address ticketRenderer_) public onlyOwner {
60: function setRoyaltyRecipient(address royaltyRecipient_) public onlyOwner {
66: function setRoyaltyFee(uint256 royaltyFee_) public onlyOwner {
73: function setMinterAddress(address minterAddress_) public onlyOwner {
83: function mint(address to_, uint256 id_, uint256 amount_, bytes memory data_) public onlyMinter {
92: function mintBatch(
File: TicketRenderer.sol
16: function generateTokenURI(
| Issue | Instances | |
|---|---|---|
| L-1 | abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256() |
6 |
| L-2 | Empty Function Body - Consider commenting why | 3 |
| L-3 | Initializers could be front-run | 15 |
[L-1] abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()
Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456). "Unless there is a compelling reason, abi.encode should be preferred". If there is only one argument to abi.encodePacked() it can often be cast to bytes() or bytes32() instead.
If all arguments are strings and or bytes, bytes.concat() should be used instead
Instances (6):
File: QuestFactory.sol
72: if (keccak256(abi.encodePacked(contractType_)) == keccak256(abi.encodePacked('erc20'))) {
72: if (keccak256(abi.encodePacked(contractType_)) == keccak256(abi.encodePacked('erc20'))) {
105: if (keccak256(abi.encodePacked(contractType_)) == keccak256(abi.encodePacked('erc1155'))) {
105: if (keccak256(abi.encodePacked(contractType_)) == keccak256(abi.encodePacked('erc1155'))) {
211: bytes32 messageDigest = keccak256(abi.encodePacked('\x19Ethereum Signed Message:\n32', hash_));
222: if (keccak256(abi.encodePacked(msg.sender, questId_)) != hash_) revert InvalidHash();
Instances (3):
File: Erc1155Quest.sol
30: ){}
File: Quest.sol
150: function withdrawRemainingTokens(address to_) public virtual onlyOwner onlyAdminWithdrawAfterEnd {}
File: QuestFactory.sol
35: constructor() initializer {}
Initializers could be front-run, allowing an attacker to either set their own values, take ownership of the contract, and in the best case forcing a re-deployment
Instances (15):
File: QuestFactory.sol
35: constructor() initializer {}
37: function initialize(
41: ) public initializer {
42: __Ownable_init();
43: __AccessControl_init();
File: RabbitHoleReceipt.sol
43: function initialize(
48: ) public initializer {
49: __ERC721_init('RabbitHoleReceipt', 'RHR');
50: __ERC721URIStorage_init();
51: __Ownable_init();
File: RabbitHoleTickets.sol
32: function initialize(
37: ) public initializer {
38: __ERC1155_init('');
39: __Ownable_init();
40: __ERC1155Burnable_init();
| Issue | Instances | |
|---|---|---|
| M-1 | Centralization Risk for trusted owners | 23 |
Contracts have owners with privileged rights to perform admin tasks and need to be trusted to not perform malicious updates or drain funds.
Instances (23):
File: Erc1155Quest.sol
54: function withdrawRemainingTokens(address to_) public override onlyOwner {
File: Erc20Quest.sol
81: function withdrawRemainingTokens(address to_) public override onlyOwner {
File: Quest.sol
12: contract Quest is Ownable, IQuest {
50: function start() public virtual onlyOwner {
57: function pause() public onlyOwner onlyStarted {
63: function unPause() public onlyOwner onlyStarted {
150: function withdrawRemainingTokens(address to_) public virtual onlyOwner onlyAdminWithdrawAfterEnd {}
File: QuestFactory.sol
69: ) public onlyRole(CREATE_QUEST_ROLE) returns (address) {
142: function changeCreateQuestRole(address account_, bool canCreateQuest_) public onlyOwner {
159: function setClaimSignerAddress(address claimSignerAddress_) public onlyOwner {
165: function setProtocolFeeRecipient(address protocolFeeRecipient_) public onlyOwner {
172: function setRabbitHoleReceiptContract(address rabbitholeReceiptContract_) public onlyOwner {
179: function setRewardAllowlistAddress(address rewardAddress_, bool allowed_) public onlyOwner {
186: function setQuestFee(uint256 questFee_) public onlyOwner {
File: RabbitHoleReceipt.sol
65: function setReceiptRenderer(address receiptRenderer_) public onlyOwner {
71: function setRoyaltyRecipient(address royaltyRecipient_) public onlyOwner {
77: function setQuestFactory(address questFactory_) public onlyOwner {
83: function setMinterAddress(address minterAddress_) public onlyOwner {
90: function setRoyaltyFee(uint256 royaltyFee_) public onlyOwner {
File: RabbitHoleTickets.sol
54: function setTicketRenderer(address ticketRenderer_) public onlyOwner {
60: function setRoyaltyRecipient(address royaltyRecipient_) public onlyOwner {
66: function setRoyaltyFee(uint256 royaltyFee_) public onlyOwner {
73: function setMinterAddress(address minterAddress_) public onlyOwner {