From bc8488a97d297f12ab43033abd8327bf16508a55 Mon Sep 17 00:00:00 2001 From: smol-ninja Date: Fri, 29 Nov 2024 23:35:59 +0000 Subject: [PATCH] use delegatecall in withdrawMultiple --- .solhint.json | 1 - src/abstracts/SablierLockupBase.sol | 13 +++++++------ src/interfaces/ISablierLockupBase.sol | 4 +--- .../withdraw-multiple/withdrawMultiple.t.sol | 10 +++++----- 4 files changed, 13 insertions(+), 15 deletions(-) diff --git a/.solhint.json b/.solhint.json index 0d4c50cbc..b7274a4e4 100644 --- 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 index 0f72c52e1..a25ffda32 100644 --- 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 index 339ec64a8..ac11b35b5 100644 --- 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 index dab7de20f..9ec58da19 100644 --- 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,