save gas by not doing double checks

rabbitholegg · Nov 15, 2023 · f2a7072 · f2a7072
1 parent e6ef614
commit f2a7072
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 22 deletions.
diff --git a/contracts/Quest.sol b/contracts/Quest.sol
@@ -98,25 +98,26 @@ contract Quest is ReentrancyGuardUpgradeable, PausableUpgradeable, Ownable, IQue
     /*//////////////////////////////////////////////////////////////
                                MODIFIERS
     //////////////////////////////////////////////////////////////*/
-    /// @notice Prevents reward withdrawal until the Quest has ended
-    modifier onlyWithdrawAfterEnd() {
-        if (block.timestamp < endTime) revert NoWithdrawDuringClaim();
+    /// @notice Checks if the quest end time is passed
+    modifier onlyEnded() {
+        if (block.timestamp < endTime) revert NotEnded();
         _;
     }
 
-    /// @notice Checks if quest has started both at the function level and at the start time
-    modifier onlyQuestActive() {
-        if (block.timestamp < startTime) revert ClaimWindowNotStarted();
+    modifier onlyQuestFactory() {
+        if (msg.sender != address(questFactoryContract)) revert NotQuestFactory();
         _;
     }
 
-    modifier onlyProtocolFeeRecipientOrOwner() {
-        if (msg.sender != protocolFeeRecipient && msg.sender != owner()) revert AuthOwnerRecipient();
+    /// @notice Checks if the quest start time is passed
+    modifier onlyStarted() {
+        if (block.timestamp < startTime) revert NotStarted();
         _;
     }
 
-    modifier onlyQuestFactory() {
-        if (msg.sender != address(questFactoryContract)) revert NotQuestFactory();
+    /// @notice Checks if the quest end time has not passed
+    modifier whenNotEnded() {
+        if (block.timestamp > endTime) revert QuestEnded();
         _;
     }
 
@@ -141,7 +142,8 @@ contract Quest is ReentrancyGuardUpgradeable, PausableUpgradeable, Ownable, IQue
         external
         virtual
         nonReentrant
-        onlyQuestActive
+        onlyStarted
+        whenNotEnded
         whenNotPaused
         onlyQuestFactory
     {
@@ -152,7 +154,7 @@ contract Quest is ReentrancyGuardUpgradeable, PausableUpgradeable, Ownable, IQue
 
     /// @notice Function to withdraw the remaining tokens in the contract, distributes the protocol fee and returns remaining tokens to owner
     /// @dev Can only be called after the quest has ended
-    function withdrawRemainingTokens() external onlyWithdrawAfterEnd {
+    function withdrawRemainingTokens() external onlyEnded {
         if (hasWithdrawn) revert AlreadyWithdrawn();
         hasWithdrawn = true;
 

diff --git a/contracts/Quest1155.sol b/contracts/Quest1155.sol
@@ -126,7 +126,6 @@ contract Quest1155 is ERC1155Holder, ReentrancyGuardUpgradeable, PausableUpgrade
     function singleClaim(address account_)
         external
         virtual
-        nonReentrant
         whenNotPaused
         whenNotEnded
         onlyStarted

diff --git a/contracts/QuestFactory.sol b/contracts/QuestFactory.sol
@@ -557,9 +557,6 @@ contract QuestFactory is Initializable, LegacyStorage, OwnableRoles, IQuestFacto
     {
         Quest storage currentQuest = quests[claimData_.questId];
         IQuest1155Ownable questContract_ = IQuest1155Ownable(currentQuest.questAddress);
-        if (!questContract_.queued()) revert QuestNotQueued();
-        if (block.timestamp < questContract_.startTime()) revert QuestNotStarted();
-        if (block.timestamp > questContract_.endTime()) revert QuestEnded();
 
         currentQuest.addressMinted[claimData_.claimer] = true;
         ++currentQuest.numberMinted;
@@ -605,9 +602,6 @@ contract QuestFactory is Initializable, LegacyStorage, OwnableRoles, IQuestFacto
     {
         Quest storage currentQuest = quests[claimData_.questId];
         IQuestOwnable questContract_ = IQuestOwnable(currentQuest.questAddress);
-        if (!questContract_.queued()) revert QuestNotQueued();
-        if (block.timestamp < questContract_.startTime()) revert QuestNotStarted();
-        if (block.timestamp > questContract_.endTime()) revert QuestEnded();
 
         currentQuest.addressMinted[claimData_.claimer] = true;
         ++currentQuest.numberMinted;

diff --git a/contracts/interfaces/IQuest.sol b/contracts/interfaces/IQuest.sol
@@ -9,16 +9,16 @@ interface IQuest {
     error AlreadyClaimed();
     error AlreadyWithdrawn();
     error AmountExceedsBalance();
-    error ClaimWindowNotStarted();
     error EndTimeInPast();
     error EndTimeLessThanOrEqualToStartTime();
     error InvalidRefundToken();
     error MustImplementInChild();
     error NotQuestFactory();
-    error NoWithdrawDuringClaim();
     error NotStarted();
     error TotalAmountExceedsBalance();
     error AuthOwnerRecipient();
+    error QuestEnded();
+    error NotEnded();
 
     function initialize(
         address rewardTokenAddress_,

diff --git a/test/QuestFactory.t.sol b/test/QuestFactory.t.sol
@@ -528,7 +528,7 @@ contract TestQuestFactory is Test, Errors, Events, TestUtils {
         bytes32 msgHash = keccak256(abi.encodePacked(participant, "questId"));
         bytes memory signature = signHash(msgHash, claimSignerPrivateKey);
 
-        vm.expectRevert(abi.encodeWithSelector(QuestNotStarted.selector));
+        vm.expectRevert(abi.encodeWithSelector(NotStarted.selector));
         vm.startPrank(participant);
         questFactory.claim{value: MINT_FEE}("questId", msgHash, signature, address(0));
     }