feat: skip empty withdrawable amounts

src/abstracts/SablierV2Lockup.sol

@@ -223,15 +223,18 @@ abstract contract SablierV2Lockup is
     }
 
     /// @inheritdoc ISablierV2Lockup
-    function withdrawMaxAndTransfer(uint256 streamId, address newRecipient) external override {
-        // Checks: the caller is the current recipient. This also checks for burned NFTs.
+    function withdrawMaxAndTransfer(uint256 streamId, address newRecipient) external override notNull(streamId) {
+        // Checks: the caller is the current recipient. This also checks that the NFT was not burned.
         address currentRecipient = _ownerOf(streamId);
         if (msg.sender != currentRecipient) {
             revert Errors.SablierV2Lockup_Unauthorized(streamId, msg.sender);
         }
 
-        // Checks, Effects and Interactions: make the max withdrawal.
-        withdraw({ streamId: streamId, to: currentRecipient, amount: _withdrawableAmountOf(streamId) });
+        // Skip the withdrawal if the withdrawable amount is zero.
+        uint128 withdrawableAmount = _withdrawableAmountOf(streamId);
+        if (withdrawableAmount > 0) {
+            _withdraw({ streamId: streamId, to: currentRecipient, amount: withdrawableAmount });
+        }
 
         // Checks and Effects: transfer the NFT.
         _transfer({ from: currentRecipient, to: newRecipient, tokenId: streamId });

src/interfaces/ISablierV2Lockup.sol

@@ -240,7 +240,7 @@ interface ISablierV2Lockup is
     ///
     /// Requirements:
     /// - Must not be delegate called.
-    /// - `streamId` must not reference a null, pending, or depleted stream.
+    /// - `streamId` must not reference a null or depleted stream.
     /// - `msg.sender` must be the stream's sender, the stream's recipient or an approved third party.
     /// - `to` must be the recipient if `msg.sender` is the stream's sender.
     /// - `to` must not be the zero address.
@@ -271,6 +271,7 @@ interface ISablierV2Lockup is
     /// @dev Emits a {WithdrawFromLockupStream} and a {Transfer} event.
     ///
     /// Notes:
+    /// - If the withdrawable amount is zero, the withdrawal is skipped.
     /// - Refer to the notes in {withdraw}.
     ///
     /// Requirements:

test/integration/basic/lockup/withdraw-max-and-transfer/withdrawMaxAndTransfer.t.sol

@@ -16,7 +16,13 @@ abstract contract WithdrawMaxAndTransfer_Integration_Basic_Test is
         WithdrawMaxAndTransfer_Integration_Shared_Test.setUp();
     }
 
-    function test_RevertWhen_CallerNotCurrentRecipient() external {
+    function test_RevertWhen_Null() external {
+        uint256 nullStreamId = 1729;
+        vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2Lockup_Null.selector, nullStreamId));
+        lockup.withdrawMaxAndTransfer({ streamId: nullStreamId, newRecipient: users.recipient });
+    }
+
+    function test_RevertWhen_CallerNotCurrentRecipient() external whenNotNull {
         // Make Eve the caller in this test.
         changePrank({ msgSender: users.eve });
 
@@ -27,7 +33,7 @@ abstract contract WithdrawMaxAndTransfer_Integration_Basic_Test is
         lockup.withdrawMaxAndTransfer({ streamId: defaultStreamId, newRecipient: users.eve });
     }
 
-    function test_RevertWhen_NFTBurned() external whenCallerCurrentRecipient {
+    function test_RevertWhen_NFTBurned() external whenNotNull whenCallerCurrentRecipient {
         // Deplete the stream.
         vm.warp({ timestamp: defaults.END_TIME() });
         lockup.withdrawMax({ streamId: defaultStreamId, to: users.recipient });
@@ -39,10 +45,27 @@ abstract contract WithdrawMaxAndTransfer_Integration_Basic_Test is
         vm.expectRevert(
             abi.encodeWithSelector(Errors.SablierV2Lockup_Unauthorized.selector, defaultStreamId, users.recipient)
         );
-        lockup.withdrawMaxAndTransfer({ streamId: defaultStreamId, newRecipient: users.recipient });
+        lockup.withdrawMaxAndTransfer({ streamId: defaultStreamId, newRecipient: users.alice });
+    }
+
+    function test_WithdrawMaxAndTransfer_WithdrawableAmountZero()
+        external
+        whenNotNull
+        whenCallerCurrentRecipient
+        whenNFTNotBurned
+    {
+        vm.warp({ timestamp: defaults.END_TIME() });
+        lockup.withdrawMax({ streamId: defaultStreamId, to: users.recipient });
+        lockup.withdrawMaxAndTransfer({ streamId: defaultStreamId, newRecipient: users.alice });
     }
 
-    function test_WithdrawMaxAndTransfer() external whenCallerCurrentRecipient whenNFTNotBurned {
+    function test_WithdrawMaxAndTransfer()
+        external
+        whenNotNull
+        whenCallerCurrentRecipient
+        whenNFTNotBurned
+        whenWithdrawableAmountNotZero
+    {
         // Simulate the passage of time.
         vm.warp({ timestamp: defaults.WARP_26_PERCENT() });
 

test/integration/basic/lockup/withdraw-max-and-transfer/withdrawMaxAndTransfer.tree

@@ -1,12 +1,18 @@
 withdrawMaxAndTransfer.t.sol
-├── when the caller is not the current recipient
+├── when the id references a null stream
 │  └── it should revert
-└── when the caller is the current recipient
-   ├── when the NFT has been burned
+└── when the id does not reference a null stream
+   ├── when the caller is not the current recipient
    │  └── it should revert
-   └── when the NFT has not been burned
-      ├── it should make the max withdrawal
-      ├── it should update the withdrawn amount
-      ├── it should transfer the NFT
-      ├── it should emit a {WithdrawFromLockupStream} event
-      └── it should emit a {Transfer} event
+   └── when the caller is the current recipient
+      ├── when the NFT has been burned
+      │  └── it should revert
+      └── when the NFT has not been burned
+         ├── when the withdrawable amount is zero
+         │  └── it should skip the withdrawal
+         └── when the withdrawable amount is not zero
+            ├── it should make the max withdrawal
+            ├── it should update the withdrawn amount
+            ├── it should transfer the NFT
+            ├── it should emit a {WithdrawFromLockupStream} event
+            └── it should emit a {Transfer} event

test/integration/fuzz/lockup/withdrawMaxAndTransfer.t.sol

@@ -14,29 +14,36 @@ abstract contract WithdrawMaxAndTransfer_Integration_Fuzz_Test is
         WithdrawMaxAndTransfer_Integration_Shared_Test.setUp();
     }
 
+    /// @dev Given enough test runs, all of the following scenarios will be fuzzed:
+    ///
+    /// - New recipient same and different from the current one
+    /// - Withdrawable amount zero and not zero
     function testFuzz_WithdrawMaxAndTransfer(
         uint256 timeJump,
         address newRecipient
     )
         external
+        whenNotNull
         whenCallerCurrentRecipient
         whenNFTNotBurned
     {
         vm.assume(newRecipient != address(0));
-        timeJump = _bound(timeJump, defaults.TOTAL_DURATION(), defaults.TOTAL_DURATION() * 2);
+        timeJump = _bound(timeJump, 0, defaults.TOTAL_DURATION() * 2);
 
         // Simulate the passage of time.
         vm.warp({ timestamp: defaults.START_TIME() + timeJump });
 
         // Get the withdraw amount.
         uint128 withdrawAmount = lockup.withdrawableAmountOf(defaultStreamId);
 
-        // Expect the assets to be transferred to the fuzzed recipient.
-        expectCallToTransfer({ to: users.recipient, amount: withdrawAmount });
+        if (withdrawAmount > 0) {
+            // Expect the assets to be transferred to the fuzzed recipient.
+            expectCallToTransfer({ to: users.recipient, amount: withdrawAmount });
 
-        // Expect a {WithdrawFromLockupStream} event to be emitted.
-        vm.expectEmit({ emitter: address(lockup) });
-        emit WithdrawFromLockupStream({ streamId: defaultStreamId, to: users.recipient, amount: withdrawAmount });
+            // Expect a {WithdrawFromLockupStream} event to be emitted.
+            vm.expectEmit({ emitter: address(lockup) });
+            emit WithdrawFromLockupStream({ streamId: defaultStreamId, to: users.recipient, amount: withdrawAmount });
+        }
 
         // Expect a {Transfer} event to be emitted.
         vm.expectEmit({ emitter: address(lockup) });

test/integration/shared/lockup/withdrawMaxAndTransfer.t.sol

@@ -11,11 +11,19 @@ abstract contract WithdrawMaxAndTransfer_Integration_Shared_Test is Lockup_Integ
         changePrank({ msgSender: users.recipient });
     }
 
+    modifier whenNotNull() {
+        _;
+    }
+
     modifier whenCallerCurrentRecipient() {
         _;
     }
 
     modifier whenNFTNotBurned() {
         _;
     }
+
+    modifier whenWithdrawableAmountNotZero() {
+        _;
+    }
 }