use delegatecall in withdrawMultiple

sablier-labs · Nov 29, 2024 · bc8488a · bc8488a
1 parent afcbd68
commit bc8488a
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 15 deletions.
diff --git a/.solhint.json b/.solhint.json
@@ -10,7 +10,6 @@
     "func-visibility": ["error", { "ignoreConstructors": true }],
     "gas-custom-errors": "off",
     "max-states-count": ["warn", 20],
-    "imports-order": "warn",
     "max-line-length": ["error", 124],
     "named-parameters-mapping": "warn",
     "no-empty-blocks": "off",

diff --git a/src/abstracts/SablierLockupBase.sol b/src/abstracts/SablierLockupBase.sol
@@ -506,14 +506,15 @@ abstract contract SablierLockupBase is
             revert Errors.SablierLockupBase_WithdrawArrayCountsNotEqual(streamIdsCount, amountsCount);
         }
 
-        // Iterate over the provided array of stream IDs and try to withdraw from each stream to the recipient.
+        // Iterate over the provided array of stream IDs and withdraw from each stream to the recipient.
         for (uint256 i = 0; i < streamIdsCount; ++i) {
-            // Checks, Effects and Interactions: try the withdrawal. The `msg.sender` in the `withdraw` call will be
-            // `this` contract.
-            try this.withdraw({ streamId: streamIds[i], to: _ownerOf(streamIds[i]), amount: amounts[i] }) { }
+            // Checks, Effects and Interactions: withdraw using delegatecall.
+            (bool success, bytes memory result) = address(this).delegatecall(
+                abi.encodeCall(ISablierLockupBase.withdraw, (streamIds[i], _ownerOf(streamIds[i]), amounts[i]))
+            );
             // If the withdrawal reverts, log it using an event, and continue with the next stream.
-            catch (bytes memory errorData) {
-                emit InvalidWithdrawalInWithdrawMultiple(streamIds[i], errorData);
+            if (!success) {
+                emit InvalidWithdrawalInWithdrawMultiple(streamIds[i], result);
             }
         }
     }

diff --git a/src/interfaces/ISablierLockupBase.sol b/src/interfaces/ISablierLockupBase.sol
@@ -384,9 +384,7 @@ interface ISablierLockupBase is
     /// reverted the withdrawal, it emits an {InvalidWithdrawalInWithdrawMultiple} event.
     ///
     /// Notes:
-    /// - This function attempts to call a hook on the recipient of each stream, even when `msg.sender` is the
-    /// recipient. This is because the the `try` statement makes an external call to `withdraw` which resets the
-    /// `msg.sender` to the contract itself.
+    /// - This function attempts to call a hook on the recipient of each stream, unless `msg.sender` is the recipient.
     ///
     /// Requirements:
     /// - Must not be delegate called.

diff --git a/tests/integration/concrete/lockup-base/withdraw-multiple/withdrawMultiple.t.sol b/tests/integration/concrete/lockup-base/withdraw-multiple/withdrawMultiple.t.sol
@@ -80,7 +80,7 @@ contract WithdrawMultiple_Integration_Concrete_Test is Integration_Test {
         vm.expectEmit({ emitter: address(lockup) });
         emit ISablierLockupBase.InvalidWithdrawalInWithdrawMultiple({
             streamId: nullStreamId,
-            errorData: abi.encodeWithSelector(Errors.SablierLockupBase_Null.selector, nullStreamId)
+            revertData: abi.encodeWithSelector(Errors.SablierLockupBase_Null.selector, nullStreamId)
         });
 
         // It should emit 2 {WithdrawFromLockupStream} events.
@@ -120,7 +120,7 @@ contract WithdrawMultiple_Integration_Concrete_Test is Integration_Test {
         vm.expectEmit({ emitter: address(lockup) });
         emit ISablierLockupBase.InvalidWithdrawalInWithdrawMultiple({
             streamId: withdrawMultipleStreamIds[0],
-            errorData: abi.encodeWithSelector(
+            revertData: abi.encodeWithSelector(
                 Errors.SablierLockupBase_StreamDepleted.selector, withdrawMultipleStreamIds[0]
             )
         });
@@ -169,14 +169,14 @@ contract WithdrawMultiple_Integration_Concrete_Test is Integration_Test {
         vm.expectEmit({ emitter: address(lockup) });
         emit ISablierLockupBase.InvalidWithdrawalInWithdrawMultiple({
             streamId: withdrawMultipleStreamIds[1],
-            errorData: abi.encodeWithSelector(
+            revertData: abi.encodeWithSelector(
                 Errors.SablierLockupBase_WithdrawAmountZero.selector, withdrawMultipleStreamIds[1]
             )
         });
         vm.expectEmit({ emitter: address(lockup) });
         emit ISablierLockupBase.InvalidWithdrawalInWithdrawMultiple({
             streamId: withdrawMultipleStreamIds[2],
-            errorData: abi.encodeWithSelector(
+            revertData: abi.encodeWithSelector(
                 Errors.SablierLockupBase_WithdrawAmountZero.selector, withdrawMultipleStreamIds[2]
             )
         });
@@ -218,7 +218,7 @@ contract WithdrawMultiple_Integration_Concrete_Test is Integration_Test {
         vm.expectEmit({ emitter: address(lockup) });
         emit ISablierLockupBase.InvalidWithdrawalInWithdrawMultiple({
             streamId: withdrawMultipleStreamIds[2],
-            errorData: abi.encodeWithSelector(
+            revertData: abi.encodeWithSelector(
                 Errors.SablierLockupBase_Overdraw.selector,
                 withdrawMultipleStreamIds[2],
                 MAX_UINT128,