address feedback

sablier-labs · Dec 14, 2024 · 18aec2c · 18aec2c
1 parent b5c81cd
commit 18aec2c
Show file tree

Hide file tree

Showing 6 changed files with 52 additions and 115 deletions.
diff --git a/tests/fork/Flow.t.sol b/tests/fork/Flow.t.sol
@@ -95,6 +95,9 @@ contract Flow_Fork_Test is Fork_Test {
             // Make sure that fuzzed users don't overlap with Flow address.
             checkUsers(params.recipient, params.sender);
 
+            // Make sure that the sender has enough funds.
+            vm.deal({ account: params.sender, newBalance: 1_000_000 ether });
+
             // Warp to a different time.
             params.timeJump = _passTime(params.timeJump);
 
@@ -224,15 +227,11 @@ contract Flow_Fork_Test is Fork_Test {
         );
 
         // Make sure the requirements are respected.
-        address sender = flow.getSender(streamId);
-        resetPrank({ msgSender: sender });
+        resetPrank({ msgSender: flow.getSender(streamId) });
         if (flow.isPaused(streamId)) {
             flow.restart(streamId, RATE_PER_SECOND);
         }
 
-        // Fund the sender to pay the fee.
-        vm.deal({ account: sender, newBalance: sender.balance + FEE });
-
         UD21x18 oldRatePerSecond = flow.getRatePerSecond(streamId);
         if (newRatePerSecond.unwrap() == oldRatePerSecond.unwrap()) {
             newRatePerSecond = ud21x18(newRatePerSecond.unwrap() + 1);
@@ -299,7 +298,6 @@ contract Flow_Fork_Test is Fork_Test {
         });
 
         resetPrank({ msgSender: sender });
-        vm.deal({ account: sender, newBalance: sender.balance + FEE });
 
         vars.actualStreamId = flow.create{ value: FEE }({
             recipient: recipient,
@@ -355,7 +353,6 @@ contract Flow_Fork_Test is Fork_Test {
         address sender = flow.getSender(streamId);
         resetPrank({ msgSender: sender });
         deal({ token: address(token), to: sender, give: depositAmount });
-        vm.deal({ account: sender, newBalance: sender.balance + FEE });
         safeApprove(depositAmount);
 
         // Expect the relevant events to be emitted.
@@ -402,14 +399,11 @@ contract Flow_Fork_Test is Fork_Test {
             flow.restart(streamId, RATE_PER_SECOND);
         }
 
-        // Fund the sender to pay the fee.
-        vm.deal({ account: sender, newBalance: sender.balance + FEE });
-
         // Expect the relevant events to be emitted.
         vm.expectEmit({ emitter: address(flow) });
         emit ISablierFlow.PauseFlowStream({
             streamId: streamId,
-            sender: flow.getSender(streamId),
+            sender: sender,
             recipient: flow.getRecipient(streamId),
             totalDebt: flow.totalDebtOf(streamId)
         });
@@ -443,9 +437,6 @@ contract Flow_Fork_Test is Fork_Test {
             depositOnStream(streamId, depositAmount);
         }
 
-        // Fund the sender to pay the fee.
-        vm.deal({ account: sender, newBalance: sender.balance + FEE });
-
         // Bound the refund amount to avoid error.
         refundAmount = boundUint128(refundAmount, 1, flow.refundableAmountOf(streamId));
 
@@ -491,9 +482,6 @@ contract Flow_Fork_Test is Fork_Test {
         address sender = flow.getSender(streamId);
         resetPrank({ msgSender: sender });
 
-        // Fund the sender to pay the fee.
-        vm.deal({ account: sender, newBalance: sender.balance + FEE });
-
         if (!flow.isPaused(streamId)) {
             flow.pause(streamId);
         }
@@ -535,9 +523,6 @@ contract Flow_Fork_Test is Fork_Test {
 
         resetPrank({ msgSender: sender });
 
-        // Fund the sender to pay the fee.
-        vm.deal({ account: sender, newBalance: sender.balance + FEE });
-
         if (uncoveredDebt > 0) {
             expectedTotalDebt = flow.getBalance(streamId);
         } else {
@@ -598,7 +583,6 @@ contract Flow_Fork_Test is Fork_Test {
 
         (, address caller,) = vm.readCallers();
         address recipient = flow.getRecipient(streamId);
-        vm.deal({ account: caller, newBalance: caller.balance + FEE });
 
         vars.expectedAggregateAmount = flow.aggregateBalance(token) - withdrawAmount;
 

diff --git a/tests/integration/concrete/collect-fees/collectFees.t.sol b/tests/integration/concrete/collect-fees/collectFees.t.sol
@@ -10,15 +10,19 @@ contract CollectFees_Integration_Concrete_Test is Shared_Integration_Concrete_Te
     function setUp() public override {
         Shared_Integration_Concrete_Test.setUp();
         depositToDefaultStream();
+
+        // Make a withdrawal and pay the fee.
+        flow.withdrawMax{ value: FEE }({ streamId: defaultStreamId, to: users.recipient });
+
+        resetPrank({ msgSender: users.admin });
     }
 
-    function test_WhenAdminIsNotContract() external {
+    function test_GivenAdminIsNotContract() external {
         _test_CollectFees(users.admin);
     }
 
-    function test_RevertWhen_AdminDoesNotImplementReceiveFunction() external whenAdminIsContract {
+    function test_RevertGiven_AdminDoesNotImplementReceiveFunction() external givenAdminIsContract {
         // Transfer the admin to a contract that does not implement the receive function.
-        resetPrank({ msgSender: users.admin });
         flow.transferAdmin(address(contractWithoutReceive));
 
         // Make the contract the caller.
@@ -35,9 +39,8 @@ contract CollectFees_Integration_Concrete_Test is Shared_Integration_Concrete_Te
         flow.collectFees();
     }
 
-    function test_WhenAdminImplementsReceiveFunction() external whenAdminIsContract {
+    function test_GivenAdminImplementsReceiveFunction() external givenAdminIsContract {
         // Transfer the admin to a contract that implements the receive function.
-        resetPrank({ msgSender: users.admin });
         flow.transferAdmin(address(contractWithReceive));
 
         // Make the contract the caller.
@@ -53,12 +56,6 @@ contract CollectFees_Integration_Concrete_Test is Shared_Integration_Concrete_Te
         // Load the initial ETH balance of the admin.
         uint256 initialAdminBalance = admin.balance;
 
-        // Make recipient the caller.
-        resetPrank({ msgSender: users.recipient });
-
-        // Make a withdrawal and pay the fee.
-        flow.withdrawMax{ value: FEE }({ streamId: defaultStreamId, to: users.recipient });
-
         // It should emit a {CollectFees} event.
         vm.expectEmit({ emitter: address(flow) });
         emit ISablierFlowBase.CollectFees({ admin: admin, feeAmount: FEE });

diff --git a/tests/integration/concrete/collect-fees/collectFees.tree b/tests/integration/concrete/collect-fees/collectFees.tree
@@ -1,12 +1,12 @@
 CollectFees_Integration_Concrete_Test
-├── when admin is not contract
+├── given admin is not contract
 │  ├── it should transfer fee
 │  ├── it should decrease contract balance to zero
 │  └── it should emit a {CollectFees} event
-└── when admin is contract
-   ├── when admin does not implement receive function
+└── given admin is contract
+   ├── given admin does not implement receive function
    │  └── it should revert
-   └── when admin implements receive function
+   └── given admin implements receive function
       ├── it should transfer fee
       ├── it should decrease contract balance to zero
       └── it should emit a {CollectFees} event
diff --git a/tests/integration/concrete/payable/payable.t.sol b/tests/integration/concrete/payable/payable.t.sol
@@ -16,97 +16,69 @@ contract Payable_Integration_Concrete_Test is Shared_Integration_Concrete_Test {
         resetPrank({ msgSender: users.sender });
     }
 
-    function _test_ETHBalance(bytes memory callData) private {
-        // Load the initial ETH balance of the sender.
-        uint256 initialSenderBalance = users.sender.balance;
-        uint256 initialFlowBalance = address(flow).balance;
-
-        (bool success,) = address(flow).call{ value: FEE }(callData);
-        assertTrue(success, "payable call failed");
-
-        // Assert that both the sender and the contract have the expected balances.
-        assertEq(users.sender.balance, initialSenderBalance - FEE);
-        assertEq(address(flow).balance, initialFlowBalance + FEE);
-    }
-
     function test_AdjustRatePerSecondWhenETHValueNotZero() external {
-        _test_ETHBalance({
-            callData: abi.encodeCall(flow.adjustRatePerSecond, (defaultStreamId, ud21x18(RATE_PER_SECOND_U128 + 1)))
-        });
+        flow.adjustRatePerSecond{ value: FEE }(defaultStreamId, ud21x18(RATE_PER_SECOND_U128 + 1));
     }
 
     function test_CreateWhenETHValueNotZero() external {
-        _test_ETHBalance({
-            callData: abi.encodeCall(flow.create, (users.sender, users.recipient, RATE_PER_SECOND, usdc, TRANSFERABLE))
-        });
+        flow.create{ value: FEE }(users.sender, users.recipient, RATE_PER_SECOND, usdc, TRANSFERABLE);
     }
 
     function test_CreateAndDepositWhenETHValueNotZero() external {
-        _test_ETHBalance({
-            callData: abi.encodeCall(
-                flow.createAndDeposit,
-                (users.sender, users.recipient, RATE_PER_SECOND, usdc, TRANSFERABLE, DEPOSIT_AMOUNT_6D)
-            )
-        });
+        flow.createAndDeposit{ value: FEE }(
+            users.sender, users.recipient, RATE_PER_SECOND, usdc, TRANSFERABLE, DEPOSIT_AMOUNT_6D
+        );
     }
 
     function test_DepositWhenETHValueNotZero() external {
-        _test_ETHBalance({
-            callData: abi.encodeCall(flow.deposit, (defaultStreamId, DEPOSIT_AMOUNT_6D, users.sender, users.recipient))
-        });
+        flow.deposit{ value: FEE }(defaultStreamId, DEPOSIT_AMOUNT_6D, users.sender, users.recipient);
     }
 
     function test_DepositAndPauseWhenETHValueNotZero() external {
-        _test_ETHBalance({ callData: abi.encodeCall(flow.depositAndPause, (defaultStreamId, DEPOSIT_AMOUNT_6D)) });
+        flow.depositAndPause{ value: FEE }(defaultStreamId, DEPOSIT_AMOUNT_6D);
     }
 
     function test_DepositViaBrokerWhenETHValueNotZero() external {
-        _test_ETHBalance({
-            callData: abi.encodeCall(
-                flow.depositViaBroker, (defaultStreamId, DEPOSIT_AMOUNT_6D, users.sender, users.recipient, defaultBroker)
-            )
-        });
+        flow.depositViaBroker{ value: FEE }(
+            defaultStreamId, DEPOSIT_AMOUNT_6D, users.sender, users.recipient, defaultBroker
+        );
     }
 
     function test_PauseWhenETHValueNotZero() external {
-        _test_ETHBalance({ callData: abi.encodeCall(flow.pause, (defaultStreamId)) });
+        flow.pause{ value: FEE }(defaultStreamId);
     }
 
     function test_RefundWhenETHValueNotZero() external {
-        _test_ETHBalance({ callData: abi.encodeCall(flow.refund, (defaultStreamId, REFUND_AMOUNT_6D)) });
+        flow.refund{ value: FEE }(defaultStreamId, REFUND_AMOUNT_6D);
     }
 
     function test_RefundAndPauseWhenETHValueNotZero() external {
-        _test_ETHBalance({ callData: abi.encodeCall(flow.refundAndPause, (defaultStreamId, REFUND_AMOUNT_6D)) });
+        flow.refundAndPause{ value: FEE }(defaultStreamId, REFUND_AMOUNT_6D);
     }
 
     function test_RefundMaxWhenETHValueNotZero() external {
-        _test_ETHBalance({ callData: abi.encodeCall(flow.refundMax, (defaultStreamId)) });
+        flow.refundMax{ value: FEE }(defaultStreamId);
     }
 
     function test_RestartWhenETHValueNotZero() external {
         flow.pause(defaultStreamId);
-        _test_ETHBalance({ callData: abi.encodeCall(flow.restart, (defaultStreamId, RATE_PER_SECOND)) });
+        flow.restart{ value: FEE }(defaultStreamId, RATE_PER_SECOND);
     }
 
     function test_RestartAndDepositWhenETHValueNotZero() external {
         flow.pause(defaultStreamId);
-        _test_ETHBalance({
-            callData: abi.encodeCall(flow.restartAndDeposit, (defaultStreamId, RATE_PER_SECOND, DEPOSIT_AMOUNT_6D))
-        });
+        flow.restartAndDeposit{ value: FEE }(defaultStreamId, RATE_PER_SECOND, DEPOSIT_AMOUNT_6D);
     }
 
     function test_VoidWhenETHValueNotZero() external {
-        _test_ETHBalance({ callData: abi.encodeCall(flow.void, (defaultStreamId)) });
+        flow.void{ value: FEE }(defaultStreamId);
     }
 
     function test_WithdrawWhenETHValueNotZero() external {
-        _test_ETHBalance({
-            callData: abi.encodeCall(flow.withdraw, (defaultStreamId, users.recipient, WITHDRAW_AMOUNT_6D))
-        });
+        flow.withdraw{ value: FEE }(defaultStreamId, users.recipient, WITHDRAW_AMOUNT_6D);
     }
 
     function test_WithdrawMaxWhenETHValueNotZero() external {
-        _test_ETHBalance({ callData: abi.encodeCall(flow.withdrawMax, (defaultStreamId, users.recipient)) });
+        flow.withdrawMax{ value: FEE }(defaultStreamId, users.recipient);
     }
 }
diff --git a/tests/integration/concrete/payable/payable.tree b/tests/integration/concrete/payable/payable.tree
@@ -1,74 +1,58 @@
 Payable_Integration_Concrete_Test::adjustRatePerSecond
 └── when ETH value not zero
-   ├── it should update the caller ETH balance
-   └── it should update the flow contract ETH balance
+   └── it should make the call
 
 Payable_Integration_Concrete_Test::create
 └── when ETH value not zero
-   ├── it should update the caller ETH balance
-   └── it should update the flow contract ETH balance
+   └── it should make the call
 
 Payable_Integration_Concrete_Test::createAndDeposit
 └── when ETH value not zero
-   ├── it should update the caller ETH balance
-   └── it should update the flow contract ETH balance
+   └── it should make the call
 
 Payable_Integration_Concrete_Test::deposit
 └── when ETH value not zero
-   ├── it should update the caller ETH balance
-   └── it should update the flow contract ETH balance
+   └── it should make the call
 
 Payable_Integration_Concrete_Test::depositAndPause
 └── when ETH value not zero
-   ├── it should update the caller ETH balance
-   └── it should update the flow contract ETH balance
+   └── it should make the call
 
 Payable_Integration_Concrete_Test::depositViaBroker
 └── when ETH value not zero
-   ├── it should update the caller ETH balance
-   └── it should update the flow contract ETH balance
+   └── it should make the call
 
 Payable_Integration_Concrete_Test::pause
 └── when ETH value not zero
-   ├── it should update the caller ETH balance
-   └── it should update the flow contract ETH balance
+   └── it should make the call
 
 Payable_Integration_Concrete_Test::refund
 └── when ETH value not zero
-   ├── it should update the caller ETH balance
-   └── it should update the flow contract ETH balance
+   └── it should make the call
 
 Payable_Integration_Concrete_Test::refundAndPause
 └── when ETH value not zero
-   ├── it should update the caller ETH balance
-   └── it should update the flow contract ETH balance
+   └── it should make the call
 
 Payable_Integration_Concrete_Test::refundMax
 └── when ETH value not zero
-   ├── it should update the caller ETH balance
-   └── it should update the flow contract ETH balance
+   └── it should make the call
 
 Payable_Integration_Concrete_Test::restart
 └── when ETH value not zero
-   ├── it should update the caller ETH balance
-   └── it should update the flow contract ETH balance
+   └── it should make the call
 
 Payable_Integration_Concrete_Test::restartAndDeposit
 └── when ETH value not zero
-   ├── it should update the caller ETH balance
-   └── it should update the flow contract ETH balance
+   └── it should make the call
 
 Payable_Integration_Concrete_Test::void
 └── when ETH value not zero
-   ├── it should update the caller ETH balance
-   └── it should update the flow contract ETH balance
+   └── it should make the call
 
 Payable_Integration_Concrete_Test::withdraw
 └── when ETH value not zero
-   ├── it should update the caller ETH balance
-   └── it should update the flow contract ETH balance
+   └── it should make the call
 
 Payable_Integration_Concrete_Test::withdrawMax
-└── when ETH value not zero
-   ├── it should update the caller ETH balance
-   └── it should update the flow contract ETH balance
+   └── it should make the call
diff --git a/tests/utils/Modifiers.sol b/tests/utils/Modifiers.sol
@@ -68,7 +68,7 @@ abstract contract Modifiers is Utils {
                                     COLLECT-FEES
     //////////////////////////////////////////////////////////////////////////*/
 
-    modifier whenAdminIsContract() {
+    modifier givenAdminIsContract() {
         _;
     }