refactor: change order in "claim"

chore: fix order and formatting in "claim" tests
sablier-labs · Nov 26, 2023 · 78fdd52 · 78fdd52
1 parent b2d5547
commit 78fdd52
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 55 deletions.
diff --git a/src/SablierV2MerkleStreamerFactory.sol b/src/SablierV2MerkleStreamerFactory.sol
@@ -59,15 +59,8 @@ contract SablierV2MerkleStreamerFactory is ISablierV2MerkleStreamerFactory {
         bytes32 salt = keccak256(abi.encodePacked(initialAdmin, lockupLinear, asset, merkleRoot, expiration));
 
         // Deploy the Merkle streamer with CREATE2.
-        merkleStreamerLL = new SablierV2MerkleStreamerLL{salt: salt} (
-            initialAdmin,
-            lockupLinear,
-            asset,
-            merkleRoot,
-            expiration,
-            streamDurations,
-            cancelable,
-            transferable
+        merkleStreamerLL = new SablierV2MerkleStreamerLL{ salt: salt }(
+            initialAdmin, lockupLinear, asset, merkleRoot, expiration, streamDurations, cancelable, transferable
         );
 
         // Effects: append the streamer in the admin's list.

diff --git a/src/abstracts/SablierV2MerkleStreamer.sol b/src/abstracts/SablierV2MerkleStreamer.sol
@@ -97,8 +97,8 @@ abstract contract SablierV2MerkleStreamer is
         // not call other unknown contracts.
         UD60x18 protocolFee = LOCKUP.comptroller().protocolFees(ASSET);
 
-        // Checks: if the protocol fee is not greater than zero, the campaign must be expired.
-        if (!protocolFee.gt(ud(0)) && !hasExpired()) {
+        // Checks: the campaign is not expired and the protocol fee is zero.
+        if (!hasExpired() && !protocolFee.gt(ud(0))) {
             revert Errors.SablierV2MerkleStreamer_CampaignNotExpired({
                 currentTime: block.timestamp,
                 expiration: EXPIRATION
@@ -118,15 +118,6 @@ abstract contract SablierV2MerkleStreamer is
 
     /// @dev Validates the parameters of the `claim` function, which is implemented by child contracts.
     function _checkClaim(uint256 index, bytes32 leaf, bytes32[] calldata merkleProof) internal view {
-        // Safe Interactions: query the protocol fee. This is safe because it's a known Sablier contract that does
-        // not call other unknown contracts.
-        UD60x18 protocolFee = LOCKUP.comptroller().protocolFees(ASSET);
-
-        // Checks: the protocol fee is zero.
-        if (protocolFee.gt(ud(0))) {
-            revert Errors.SablierV2MerkleStreamer_ProtocolFeeNotZero();
-        }
-
         // Checks: the campaign has not expired.
         if (hasExpired()) {
             revert Errors.SablierV2MerkleStreamer_CampaignExpired({
@@ -140,6 +131,15 @@ abstract contract SablierV2MerkleStreamer is
             revert Errors.SablierV2MerkleStreamer_StreamClaimed(index);
         }
 
+        // Safe Interactions: query the protocol fee. This is safe because it's a known Sablier contract that does
+        // not call other unknown contracts.
+        UD60x18 protocolFee = LOCKUP.comptroller().protocolFees(ASSET);
+
+        // Checks: the protocol fee is zero.
+        if (protocolFee.gt(ud(0))) {
+            revert Errors.SablierV2MerkleStreamer_ProtocolFeeNotZero();
+        }
+
         // Checks: the input claim is included in the Merkle tree.
         if (!MerkleProof.verify(merkleProof, MERKLE_ROOT, leaf)) {
             revert Errors.SablierV2MerkleStreamer_InvalidProof();

diff --git a/src/interfaces/ISablierV2MerkleStreamerLL.sol b/src/interfaces/ISablierV2MerkleStreamerLL.sol
@@ -27,9 +27,9 @@ interface ISablierV2MerkleStreamerLL is ISablierV2MerkleStreamer {
     /// @dev Emits a {Claim} event.
     ///
     /// Requirements:
-    /// - The protocol fee must be zero.
     /// - The campaign must not have expired.
     /// - The stream must not have been claimed already.
+    /// - The protocol fee must be zero.
     /// - The Merkle proof must be valid.
     ///
     /// @param index The index of the recipient in the Merkle tree.

diff --git a/src/libraries/Errors.sol b/src/libraries/Errors.sol
@@ -30,9 +30,6 @@ library Errors {
     /// @notice Thrown when trying to claim with an invalid Merkle proof.
     error SablierV2MerkleStreamer_InvalidProof();
 
-    /// @notice Thrown when trying to clawback when the protocol fee is zero.
-    error SablierV2MerkleStreamer_ProtocolFeeZero();
-
     /// @notice Thrown when trying to claim when the protocol fee is not zero.
     error SablierV2MerkleStreamer_ProtocolFeeNotZero();
 

diff --git a/test/integration/merkle-streamer/ll/claim/claim.t.sol b/test/integration/merkle-streamer/ll/claim/claim.t.sol
@@ -13,19 +13,7 @@ contract Claim_Integration_Test is MerkleStreamer_Integration_Test {
         MerkleStreamer_Integration_Test.setUp();
     }
 
-    function test_RevertGiven_ProtocolFeeNotZero() external {
-        bytes32[] memory merkleProof;
-        changePrank({ msgSender: users.admin.addr });
-        comptroller.setProtocolFee({ asset: asset, newProtocolFee: ud(0.03e18) });
-        vm.expectRevert(Errors.SablierV2MerkleStreamer_ProtocolFeeNotZero.selector);
-        merkleStreamerLL.claim({ index: 1, recipient: users.recipient1.addr, amount: 1, merkleProof: merkleProof });
-    }
-
-    modifier givenProtocolFeeZero() {
-        _;
-    }
-
-    function test_RevertGiven_CampaignExpired() external givenProtocolFeeZero {
+    function test_RevertGiven_CampaignExpired() external {
         uint40 expiration = defaults.EXPIRATION();
         uint256 warpTime = expiration + 1 seconds;
         bytes32[] memory merkleProof;
@@ -40,7 +28,7 @@ contract Claim_Integration_Test is MerkleStreamer_Integration_Test {
         _;
     }
 
-    function test_RevertGiven_AlreadyClaimed() external givenCampaignNotExpired givenProtocolFeeZero {
+    function test_RevertGiven_AlreadyClaimed() external givenCampaignNotExpired {
         claimLL();
         uint256 index1 = defaults.INDEX1();
         uint128 amount = defaults.CLAIM_AMOUNT();
@@ -53,15 +41,27 @@ contract Claim_Integration_Test is MerkleStreamer_Integration_Test {
         _;
     }
 
+    function test_RevertGiven_ProtocolFeeNotZero() external givenCampaignNotExpired givenNotClaimed {
+        bytes32[] memory merkleProof;
+        changePrank({ msgSender: users.admin.addr });
+        comptroller.setProtocolFee({ asset: asset, newProtocolFee: ud(0.03e18) });
+        vm.expectRevert(Errors.SablierV2MerkleStreamer_ProtocolFeeNotZero.selector);
+        merkleStreamerLL.claim({ index: 1, recipient: users.recipient1.addr, amount: 1, merkleProof: merkleProof });
+    }
+
+    modifier givenProtocolFeeZero() {
+        _;
+    }
+
     modifier givenNotIncludedInMerkleTree() {
         _;
     }
 
     function test_RevertWhen_InvalidIndex()
         external
         givenCampaignNotExpired
-        givenProtocolFeeZero
         givenNotClaimed
+        givenProtocolFeeZero
         givenNotIncludedInMerkleTree
     {
         uint256 invalidIndex = 1337;
@@ -74,8 +74,8 @@ contract Claim_Integration_Test is MerkleStreamer_Integration_Test {
     function test_RevertWhen_InvalidRecipient()
         external
         givenCampaignNotExpired
-        givenProtocolFeeZero
         givenNotClaimed
+        givenProtocolFeeZero
         givenNotIncludedInMerkleTree
     {
         uint256 index1 = defaults.INDEX1();
@@ -89,8 +89,8 @@ contract Claim_Integration_Test is MerkleStreamer_Integration_Test {
     function test_RevertWhen_InvalidAmount()
         external
         givenCampaignNotExpired
-        givenProtocolFeeZero
         givenNotClaimed
+        givenProtocolFeeZero
         givenNotIncludedInMerkleTree
     {
         uint256 index1 = defaults.INDEX1();
@@ -103,8 +103,8 @@ contract Claim_Integration_Test is MerkleStreamer_Integration_Test {
     function test_RevertWhen_InvalidMerkleProof()
         external
         givenCampaignNotExpired
-        givenProtocolFeeZero
         givenNotClaimed
+        givenProtocolFeeZero
         givenNotIncludedInMerkleTree
     {
         uint256 index1 = defaults.INDEX1();
@@ -121,8 +121,8 @@ contract Claim_Integration_Test is MerkleStreamer_Integration_Test {
     function test_Claim()
         external
         givenCampaignNotExpired
-        givenProtocolFeeZero
         givenNotClaimed
+        givenProtocolFeeZero
         givenIncludedInMerkleTree
     {
         uint256 expectedStreamId = lockupLinear.nextStreamId();

diff --git a/test/integration/merkle-streamer/ll/claim/claim.tree b/test/integration/merkle-streamer/ll/claim/claim.tree
@@ -1,13 +1,13 @@
 claim.t.sol
-├── given the protocol fee is greater than zero
+├── given the campaign has expired
 │  └── it should revert
-└── given the protocol fee is not greater than zero
-   ├── given the campaign has expired
+└── given the campaign has not expired
+   ├── given the recipient has claimed
    │  └── it should revert
-   └── given the campaign has not expired
-      ├── given the recipient has claimed
+   └── given the recipient has not claimed
+      ├── given the protocol fee is greater than zero
       │  └── it should revert
-      └── given the recipient has not claimed
+      └── given the protocol fee is not greater than zero
          ├── given the claim is not included in the Merkle tree
          │  ├── when the index is not valid
          │  │  └── it should revert

diff --git a/test/integration/merkle-streamer/ll/clawback/clawback.tree b/test/integration/merkle-streamer/ll/clawback/clawback.tree
@@ -9,9 +9,9 @@ clawback.t.sol
    │     ├── it should perform the ERC-20 transfer
    │     └── it should emit a {Clawback} event
    └── given the protocol fee is greater than zero
-         ├── given the campaign has not expired
-         │  ├── it should perform the ERC-20 transfer
-         │  └── it should emit a {Clawback} event
-         └── given the campaign has expired
-            ├── it should perform the ERC-20 transfer
-            └── it should emit a {Clawback} event
+      ├── given the campaign has not expired
+      │  ├── it should perform the ERC-20 transfer
+      │  └── it should emit a {Clawback} event
+      └── given the campaign has expired
+         ├── it should perform the ERC-20 transfer
+         └── it should emit a {Clawback} event