From f2a7072bbcd2d7c169ee4db4d4e89a11bdfeb487 Mon Sep 17 00:00:00 2001
From: Wayne <W@WayneHoover.com>
Date: Tue, 14 Nov 2023 15:20:36 -1000
Subject: [PATCH] save gas by not doing double checks

---
 contracts/Quest.sol             | 26 ++++++++++++++------------
 contracts/Quest1155.sol         |  1 -
 contracts/QuestFactory.sol      |  6 ------
 contracts/interfaces/IQuest.sol |  4 ++--
 test/QuestFactory.t.sol         |  2 +-
 5 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/contracts/Quest.sol b/contracts/Quest.sol
index 4423346d..5864b33c 100644
--- 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
index dc1c7df2..de3d13c4 100644
--- 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
index 446f5f5a..73793695 100644
--- 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
index ff7e04e3..17b20eba 100644
--- 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
index f3da8e4a..4fe464cd 100644
--- 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));
     }