test: move all allowedToHook calls in Integration contract (#1141)

tests/integration/Integration.t.sol

@@ -24,6 +24,8 @@ abstract contract Integration_Test is Base_Test {
     // Common stream IDs to be used across the tests.
     // Default stream ID.
     uint256 internal defaultStreamId;
+    // A stream with a recipient contract that is not allowed to hook.
+    uint256 internal notAllowedtoHookStreamId;
     // A non-cancelable stream ID.
     uint256 internal notCancelableStreamId;
     // A non-transferable stream ID.
@@ -57,8 +59,11 @@ abstract contract Integration_Test is Base_Test {
                                    TEST CONTRACTS
     //////////////////////////////////////////////////////////////////////////*/
 
+    // The following recipients are not allowed to hook.
     RecipientInterfaceIDIncorrect internal recipientInterfaceIDIncorrect;
     RecipientInterfaceIDMissing internal recipientInterfaceIDMissing;
+
+    // The following recipients are allowed to hook.
     RecipientInvalidSelector internal recipientInvalidSelector;
     RecipientReentrant internal recipientReentrant;
     RecipientReverting internal recipientReverting;
@@ -106,6 +111,7 @@ abstract contract Integration_Test is Base_Test {
 
     function initializeDefaultStreams() internal {
         defaultStreamId = createDefaultStream();
+        notAllowedtoHookStreamId = createDefaultStreamWithRecipient(address(recipientInterfaceIDIncorrect));
         notCancelableStreamId = createDefaultStreamNonCancelable();
         notTransferableStreamId = createDefaultStreamNonTransferable();
         recipientGoodStreamId = createDefaultStreamWithRecipient(address(recipientGood));
@@ -126,8 +132,11 @@ abstract contract Integration_Test is Base_Test {
         vm.label({ account: address(recipientReentrant), newLabel: "Recipient Reentrant" });
         vm.label({ account: address(recipientReverting), newLabel: "Recipient Reverting" });
 
-        // Allow the recipients to Hook.
+        // Allow the selected recipients to hook.
         resetPrank({ msgSender: users.admin });
+        lockup.allowToHook(address(recipientGood));
+        lockup.allowToHook(address(recipientInvalidSelector));
+        lockup.allowToHook(address(recipientReentrant));
         lockup.allowToHook(address(recipientReverting));
         resetPrank({ msgSender: users.sender });
     }

tests/integration/concrete/batch/batch.t.sol

@@ -108,7 +108,7 @@ contract Batch_Integration_Concrete_Test is Integration_Test {
 
         uint256[] memory streamIds = new uint256[](2);
         streamIds[0] = recipientGoodStreamId;
-        streamIds[1] = recipientInvalidSelectorStreamId;
+        streamIds[1] = notTransferableStreamId;
         calls[1] = abi.encodeCall(lockup.cancelMultiple, (streamIds));
 
         calls[2] = abi.encodeCall(lockup.renounce, (recipientReentrantStreamId));

tests/integration/concrete/lockup-base/allow-to-hook/allowToHook.t.sol

@@ -4,6 +4,7 @@ pragma solidity >=0.8.22 <0.9.0;
 import { ISablierLockupBase } from "src/interfaces/ISablierLockupBase.sol";
 import { Errors } from "src/libraries/Errors.sol";
 
+import { RecipientGood } from "../../../../mocks/Hooks.sol";
 import { Integration_Test } from "../../../Integration.t.sol";
 
 contract AllowToHook_Integration_Concrete_Test is Integration_Test {
@@ -41,15 +42,18 @@ contract AllowToHook_Integration_Concrete_Test is Integration_Test {
     }
 
     function test_WhenProvidedAddressReturnsInterfaceId() external whenCallerAdmin whenProvidedAddressContract {
+        // Define a recipient that implementes the interface correctly.
+        RecipientGood recipientWithInterfaceId = new RecipientGood();
+
         // It should emit a {AllowToHook} event.
         vm.expectEmit({ emitter: address(lockup) });
-        emit ISablierLockupBase.AllowToHook(users.admin, address(recipientGood));
+        emit ISablierLockupBase.AllowToHook(users.admin, address(recipientWithInterfaceId));
 
         // Allow the provided address to hook.
-        lockup.allowToHook(address(recipientGood));
+        lockup.allowToHook(address(recipientWithInterfaceId));
 
         // It should put the address on the allowlist.
-        bool isAllowedToHook = lockup.isAllowedToHook(address(recipientGood));
+        bool isAllowedToHook = lockup.isAllowedToHook(address(recipientWithInterfaceId));
         assertTrue(isAllowedToHook, "address not put on the allowlist");
     }
 }

tests/integration/concrete/lockup-base/cancel/cancel.t.sol

@@ -105,22 +105,22 @@ abstract contract Cancel_Integration_Concrete_Test is Integration_Test {
         givenSTREAMINGStatus
     {
         // It should not make Sablier run the recipient hook.
-        uint128 senderAmount = lockup.refundableAmountOf(recipientGoodStreamId);
-        uint128 recipientAmount = lockup.withdrawableAmountOf(recipientGoodStreamId);
+        uint128 senderAmount = lockup.refundableAmountOf(notAllowedtoHookStreamId);
+        uint128 recipientAmount = lockup.withdrawableAmountOf(notAllowedtoHookStreamId);
         vm.expectCall({
             callee: address(recipientGood),
             data: abi.encodeCall(
                 ISablierLockupRecipient.onSablierLockupCancel,
-                (recipientGoodStreamId, users.sender, senderAmount, recipientAmount)
+                (notAllowedtoHookStreamId, users.sender, senderAmount, recipientAmount)
             ),
             count: 0
         });
 
         // Cancel the stream.
-        lockup.cancel(recipientGoodStreamId);
+        lockup.cancel(notAllowedtoHookStreamId);
 
         // It should mark the stream as canceled.
-        Lockup.Status actualStatus = lockup.statusOf(recipientGoodStreamId);
+        Lockup.Status actualStatus = lockup.statusOf(notAllowedtoHookStreamId);
         Lockup.Status expectedStatus = Lockup.Status.CANCELED;
         assertEq(actualStatus, expectedStatus);
     }
@@ -153,11 +153,6 @@ abstract contract Cancel_Integration_Concrete_Test is Integration_Test {
         givenRecipientAllowedToHook
         whenNonRevertingRecipient
     {
-        // Allow the recipient to hook.
-        resetPrank({ msgSender: users.admin });
-        lockup.allowToHook(address(recipientInvalidSelector));
-        resetPrank({ msgSender: users.sender });
-
         // It should revert.
         vm.expectRevert(
             abi.encodeWithSelector(
@@ -181,11 +176,6 @@ abstract contract Cancel_Integration_Concrete_Test is Integration_Test {
         whenNonRevertingRecipient
         whenRecipientReturnsValidSelector
     {
-        // Allow the recipient to hook.
-        resetPrank({ msgSender: users.admin });
-        lockup.allowToHook(address(recipientReentrant));
-        resetPrank({ msgSender: users.sender });
-
         // It should make Sablier run the recipient hook.
         uint128 senderAmount = lockup.refundableAmountOf(recipientReentrantStreamId);
         uint128 recipientAmount = lockup.withdrawableAmountOf(recipientReentrantStreamId);
@@ -230,11 +220,6 @@ abstract contract Cancel_Integration_Concrete_Test is Integration_Test {
         whenNonRevertingRecipient
         whenRecipientReturnsValidSelector
     {
-        // Allow the recipient to hook.
-        resetPrank({ msgSender: users.admin });
-        lockup.allowToHook(address(recipientGood));
-        resetPrank({ msgSender: users.sender });
-
         // It should refund the sender.
         uint128 senderAmount = lockup.refundableAmountOf(recipientGoodStreamId);
         expectCallToTransfer({ to: users.sender, value: senderAmount });

tests/integration/concrete/lockup-base/getters/getters.t.sol

@@ -211,14 +211,11 @@ contract Getters_Integration_Concrete_Test is Integration_Test {
     //////////////////////////////////////////////////////////////////////////*/
 
     function test_IsAllowedToHookGivenProvidedAddressNotAllowedToHook() external view {
-        bool result = lockup.isAllowedToHook(address(recipientGood));
+        bool result = lockup.isAllowedToHook(address(recipientInterfaceIDIncorrect));
         assertFalse(result, "isAllowedToHook");
     }
 
-    function test_IsAllowedToHookGivenProvidedAddressAllowedToHook() external {
-        resetPrank({ msgSender: users.admin });
-        lockup.allowToHook(address(recipientGood));
-
+    function test_IsAllowedToHookGivenProvidedAddressAllowedToHook() external view {
         bool result = lockup.isAllowedToHook(address(recipientGood));
         assertTrue(result, "isAllowedToHook");
     }

tests/integration/concrete/lockup-base/withdraw-hooks/withdrawHooks.t.sol

@@ -16,11 +16,6 @@ contract WithdrawHooks_Integration_Concrete_Test is Integration_Test {
 
         differentSenderRecipientStreamId = createDefaultStreamWithRecipient(address(recipientGood));
         withdrawAmount = defaults.WITHDRAW_AMOUNT();
-
-        // Allow the good recipient to hook.
-        resetPrank({ msgSender: users.admin });
-        lockup.allowToHook(address(recipientGood));
-        resetPrank({ msgSender: users.sender });
     }
 
     function test_GivenRecipientSameAsSender() external {

tests/integration/concrete/lockup-base/withdraw/withdraw.t.sol

@@ -303,21 +303,25 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test {
         givenNotCanceledStream
     {
         // It should not make Sablier run the recipient hook.
-        uint128 withdrawAmount = lockup.withdrawableAmountOf(recipientGoodStreamId);
+        uint128 withdrawAmount = lockup.withdrawableAmountOf(notAllowedtoHookStreamId);
         vm.expectCall({
             callee: address(recipientGood),
             data: abi.encodeCall(
                 ISablierLockupRecipient.onSablierLockupWithdraw,
-                (recipientGoodStreamId, users.sender, address(recipientGood), withdrawAmount)
+                (notAllowedtoHookStreamId, users.sender, address(recipientInterfaceIDIncorrect), withdrawAmount)
             ),
             count: 0
         });
 
         // Make the withdrawal.
-        lockup.withdraw({ streamId: recipientGoodStreamId, to: address(recipientGood), amount: withdrawAmount });
+        lockup.withdraw({
+            streamId: notAllowedtoHookStreamId,
+            to: address(recipientInterfaceIDIncorrect),
+            amount: withdrawAmount
+        });
 
         // It should update the withdrawn amount.
-        uint128 actualWithdrawnAmount = lockup.getWithdrawnAmount(recipientGoodStreamId);
+        uint128 actualWithdrawnAmount = lockup.getWithdrawnAmount(notAllowedtoHookStreamId);
         uint128 expectedWithdrawnAmount = withdrawAmount;
         assertEq(actualWithdrawnAmount, expectedWithdrawnAmount, "withdrawnAmount");
     }
@@ -359,11 +363,6 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test {
         givenRecipientAllowedToHook
         whenNonRevertingRecipient
     {
-        // Allow the recipient to hook.
-        resetPrank({ msgSender: users.admin });
-        lockup.allowToHook(address(recipientInvalidSelector));
-        resetPrank({ msgSender: users.sender });
-
         // Expect a revert.
         uint128 withdrawAmount = defaults.WITHDRAW_AMOUNT();
         vm.expectRevert(
@@ -396,11 +395,6 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test {
         whenNonRevertingRecipient
         whenHookReturnsValidSelector
     {
-        // Allow the recipient to hook.
-        resetPrank({ msgSender: users.admin });
-        lockup.allowToHook(address(recipientReentrant));
-        resetPrank({ msgSender: users.sender });
-
         // Halve the withdraw amount so that the recipient can re-entry and make another withdrawal.
         uint128 withdrawAmount = defaults.WITHDRAW_AMOUNT() / 2;
 
@@ -443,11 +437,6 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test {
         whenNonRevertingRecipient
         whenHookReturnsValidSelector
     {
-        // Allow the recipient to hook.
-        resetPrank({ msgSender: users.admin });
-        lockup.allowToHook(address(recipientGood));
-        resetPrank({ msgSender: users.sender });
-
         // Set the withdraw amount to the default amount.
         uint128 withdrawAmount = defaults.WITHDRAW_AMOUNT();
 

tests/integration/fuzz/lockup-base/cancel.t.sol

@@ -58,11 +58,6 @@ abstract contract Cancel_Integration_Fuzz_Test is Integration_Test {
     {
         timeJump = _bound(timeJump, defaults.WARP_26_PERCENT_DURATION(), defaults.TOTAL_DURATION() - 1 seconds);
 
-        // Allow the recipient to hook.
-        resetPrank({ msgSender: users.admin });
-        lockup.allowToHook(address(recipientGood));
-        resetPrank({ msgSender: users.sender });
-
         // Simulate the passage of time.
         vm.warp({ newTimestamp: defaults.START_TIME() + timeJump });