Skip to content

Latest commit

 

History

History
171 lines (124 loc) · 11.6 KB

low.md

File metadata and controls

171 lines (124 loc) · 11.6 KB
Raw

Low issues

Validate totalParticipants in Quest constructor

The Quest contract should validate that there is at least one participant in the quest, i.e. totalParticipants > 0.

Unbounded gas usage in claim function of Quest contract

The claim function has an unbounded gas usage that traverses different arrays many times.

  1. The call to RabbitHoleReceipt.getOwnedTokenIdsOfQuest iterates all receipts for the account and then copies the ones for the given quest into a new array
  2. Then it iterates this array to calculate how many of them were already claimed.
  3. Finally it iterates the array again to mark the token ids as claimed in the _setClaimed function.

Prefer _disableInitializers() over initializer modifier in constructor of upgradeable contract

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L35

Prefer using _disableInitializers as this better expresses the intention and also prevents future re-initializers in implementation contracts.

Validate quest id in mintReceipt function of QuestFactory contract

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L219

Validate that the given questId_ parameter corresponds to an existing valid quest.

if (quests[questId_].questAddress == address(0)) revert InvalidQuestId();

Missing call to base initializer

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L43

RabbitHoleReceipt inherits from ERC721EnumerableUpgradeable but doesn't call its initializer (__ERC721Enumerable_init()).

Validate royalty fee is within bounds

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L90 https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleTickets.sol#L66

The function setRoyaltyFee present in the RabbitHoleReceipt and RabbitHoleTickets contracts should validate that the updated fee is lower or equal to the max basis points.

function setRoyaltyFee(uint256 royaltyFee_) public onlyOwner {
    require(royaltyFee_ <= 10_000);
    royaltyFee = royaltyFee_;
    emit RoyaltyFeeSet(royaltyFee_);
}

Non-critical issues

Missing event for contract parameter change

Important parameter or configuration changes should trigger an event to allow being tracked off-chain.

Occurrences:

Quest contract can be defined as an abstract contract

Consider defining the Quest contract as an abstract contract. The following functions can be marked as abstract and delegate the implementation to child contracts: _calculateRewards, _transferRewards and withdrawRemainingTokens.

Comparing boolean variables or expressions to literal values

Instead of comparing boolean values or expressions to literal values, just use that value or the negation of it. For example, instead of using someBooleanValue == true just use someBooleanValue, or instead of using someBooleanValue == false just use !someBooleanValue.

Occurrences:

Use uint256 type instead of uint

Prefer using the uint256 type over its uint alias.

Occurrences:

Quest contract type can be defined as en enum

Instead of using string for quest types ("erc20" and "erc115"), define these as an enum.

enum QuestType {
    ERC20,
    ERC1155
}

RabbitHoleReceipt token counter skips first value

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L99

The mint first increments the counter and uses its value.

Remove unused variables

Unused variables in callbacks or implemented functions from interfaces can be commented out to remove the warning. For example:

function foo(uint256 someUnusedValue) ... {

Can be commented out as:

function foo(uint256 /* someUnusedValue */) ... {

Occurrences: