refactor: move protocol fee check at the bottom

sablier-labs · Nov 28, 2023 · dce577a · dce577a
1 parent 78fdd52
commit dce577a
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 36 deletions.
diff --git a/src/abstracts/SablierV2MerkleStreamer.sol b/src/abstracts/SablierV2MerkleStreamer.sol
@@ -131,6 +131,11 @@ abstract contract SablierV2MerkleStreamer is
             revert Errors.SablierV2MerkleStreamer_StreamClaimed(index);
         }
 
+        // Checks: the input claim is included in the Merkle tree.
+        if (!MerkleProof.verify(merkleProof, MERKLE_ROOT, leaf)) {
+            revert Errors.SablierV2MerkleStreamer_InvalidProof();
+        }
+
         // 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);
@@ -139,10 +144,5 @@ abstract contract SablierV2MerkleStreamer is
         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/ISablierV2MerkleStreamer.sol b/src/interfaces/ISablierV2MerkleStreamer.sol
@@ -66,9 +66,12 @@ interface ISablierV2MerkleStreamer is IAdminable {
     ///
     /// @dev Emits a {Clawback} event.
     ///
+    /// Notes:
+    /// - If the protocol is not zero, the expiration check is not made.
+    ///
     /// Requirements:
     /// - The caller must be the admin.
-    /// - The Merkle streamer must have expired.
+    /// - The campaign must either be expired or not have an expiration.
     ///
     /// @param to The address to receive the tokens.
     /// @param amount The amount of tokens to claw back.

diff --git a/test/integration/merkle-streamer/ll/claim/claim.t.sol b/test/integration/merkle-streamer/ll/claim/claim.t.sol
@@ -41,18 +41,6 @@ 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() {
         _;
     }
@@ -61,7 +49,6 @@ contract Claim_Integration_Test is MerkleStreamer_Integration_Test {
         external
         givenCampaignNotExpired
         givenNotClaimed
-        givenProtocolFeeZero
         givenNotIncludedInMerkleTree
     {
         uint256 invalidIndex = 1337;
@@ -75,7 +62,6 @@ contract Claim_Integration_Test is MerkleStreamer_Integration_Test {
         external
         givenCampaignNotExpired
         givenNotClaimed
-        givenProtocolFeeZero
         givenNotIncludedInMerkleTree
     {
         uint256 index1 = defaults.INDEX1();
@@ -90,7 +76,6 @@ contract Claim_Integration_Test is MerkleStreamer_Integration_Test {
         external
         givenCampaignNotExpired
         givenNotClaimed
-        givenProtocolFeeZero
         givenNotIncludedInMerkleTree
     {
         uint256 index1 = defaults.INDEX1();
@@ -104,7 +89,6 @@ contract Claim_Integration_Test is MerkleStreamer_Integration_Test {
         external
         givenCampaignNotExpired
         givenNotClaimed
-        givenProtocolFeeZero
         givenNotIncludedInMerkleTree
     {
         uint256 index1 = defaults.INDEX1();
@@ -118,12 +102,35 @@ contract Claim_Integration_Test is MerkleStreamer_Integration_Test {
         _;
     }
 
+    function test_RevertGiven_ProtocolFeeNotZero()
+        external
+        givenCampaignNotExpired
+        givenNotClaimed
+        givenIncludedInMerkleTree
+    {
+        uint128 claimAmount = defaults.CLAIM_AMOUNT();
+        bytes32[] memory merkleProof = defaults.index1Proof();
+        changePrank({ msgSender: users.admin.addr });
+        comptroller.setProtocolFee({ asset: asset, newProtocolFee: ud(0.1e18) });
+        vm.expectRevert(Errors.SablierV2MerkleStreamer_ProtocolFeeNotZero.selector);
+        merkleStreamerLL.claim({
+            index: 1,
+            recipient: users.recipient1.addr,
+            amount: claimAmount,
+            merkleProof: merkleProof
+        });
+    }
+
+    modifier givenProtocolFeeZero() {
+        _;
+    }
+
     function test_Claim()
         external
         givenCampaignNotExpired
         givenNotClaimed
-        givenProtocolFeeZero
         givenIncludedInMerkleTree
+        givenProtocolFeeZero
     {
         uint256 expectedStreamId = lockupLinear.nextStreamId();
 

diff --git a/test/integration/merkle-streamer/ll/claim/claim.tree b/test/integration/merkle-streamer/ll/claim/claim.tree
@@ -5,19 +5,19 @@ claim.t.sol
    ├── given the recipient has claimed
    │  └── it should revert
    └── given the recipient has not claimed
-      ├── given the protocol fee is greater than zero
-      │  └── it should revert
-      └── 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
-         │  ├── when the recipient address is not valid
-         │  │  └── it should revert
-         │  ├── when the amount is not valid
-         │  │  └── it should revert
-         │  └── when the Merkle proof is not valid
-         │     └── it should revert
-         └── given the claim is included in the Merkle tree
+      ├── given the claim is not included in the Merkle tree
+      │  ├── when the index is not valid
+      │  │  └── it should revert
+      │  ├── when the recipient address is not valid
+      │  │  └── it should revert
+      │  ├── when the amount is not valid
+      │  │  └── it should revert
+      │  └── when the Merkle proof is not valid
+      │     └── it should revert
+      └── given the claim is included in the Merkle tree
+         ├── given the protocol fee is greater than zero
+         │  └── it should revert
+         └── given the protocol fee is not greater than zero
             ├── it should mark the index as claimed
             ├── it should create a LockupLinear stream
             └── it should emit a {Claim} event