refactor: add "recipient" in cancel event

docs: refine explanatory comments refactor: reorder parameters in events
sablier-labs · Oct 18, 2023 · b69d326 · b69d326
1 parent 927cedd
commit b69d326
Show file tree

Hide file tree

Showing 16 changed files with 57 additions and 46 deletions.
diff --git a/src/SablierV2LockupDynamic.sol b/src/SablierV2LockupDynamic.sol
@@ -506,14 +506,14 @@ contract SablierV2LockupDynamic is
         address sender = _streams[streamId].sender;
         address recipient = _ownerOf(streamId);
 
+        // Retrieve the ERC-20 asset from storage.
         IERC20 asset = _streams[streamId].asset;
 
         // Interactions: refund the sender.
         asset.safeTransfer({ to: sender, value: senderAmount });
 
-        // Interactions: if the recipient is a contract, try to invoke the cancel
-        // hook on the recipient without reverting if the hook is not implemented, and without bubbling up any
-        // potential revert.
+        // Interactions: if the recipient is a contract, try to invoke the cancel hook on the recipient without
+        // reverting if the hook is not implemented, and without bubbling up any potential revert.
         if (recipient.code.length > 0) {
             try ISablierV2LockupRecipient(recipient).onStreamCanceled({
                 streamId: streamId,
@@ -524,7 +524,7 @@ contract SablierV2LockupDynamic is
         }
 
         // Log the cancellation.
-        emit ISablierV2Lockup.CancelLockupStream(streamId, sender, asset, senderAmount, recipientAmount);
+        emit ISablierV2Lockup.CancelLockupStream(streamId, sender, recipient, asset, senderAmount, recipientAmount);
     }
 
     /// @dev See the documentation for the user-facing functions that call this internal function.
@@ -642,6 +642,6 @@ contract SablierV2LockupDynamic is
         asset.safeTransfer({ to: to, value: amount });
 
         // Log the withdrawal.
-        emit ISablierV2Lockup.WithdrawFromLockupStream(streamId, to, amount, asset);
+        emit ISablierV2Lockup.WithdrawFromLockupStream(streamId, to, asset, amount);
     }
 }
diff --git a/src/SablierV2LockupLinear.sol b/src/SablierV2LockupLinear.sol
@@ -421,14 +421,14 @@ contract SablierV2LockupLinear is
         address sender = _streams[streamId].sender;
         address recipient = _ownerOf(streamId);
 
+        // Retrieve the ERC-20 asset from storage.
         IERC20 asset = _streams[streamId].asset;
 
         // Interactions: refund the sender.
         asset.safeTransfer({ to: sender, value: senderAmount });
 
-        // Interactions: if the recipient is a contract, try to invoke the cancel
-        // hook on the recipient without reverting if the hook is not implemented, and without bubbling up any
-        // potential revert.
+        // Interactions: if the recipient is a contract, try to invoke the cancel hook on the recipient without
+        // reverting if the hook is not implemented, and without bubbling up any potential revert.
         if (recipient.code.length > 0) {
             try ISablierV2LockupRecipient(recipient).onStreamCanceled({
                 streamId: streamId,
@@ -439,7 +439,7 @@ contract SablierV2LockupLinear is
         }
 
         // Log the cancellation.
-        emit ISablierV2Lockup.CancelLockupStream(streamId, sender, asset, senderAmount, recipientAmount);
+        emit ISablierV2Lockup.CancelLockupStream(streamId, sender, recipient, asset, senderAmount, recipientAmount);
     }
 
     /// @dev See the documentation for the user-facing functions that call this internal function.
@@ -548,6 +548,6 @@ contract SablierV2LockupLinear is
         asset.safeTransfer({ to: to, value: amount });
 
         // Log the withdrawal.
-        emit ISablierV2Lockup.WithdrawFromLockupStream(streamId, to, amount, asset);
+        emit ISablierV2Lockup.WithdrawFromLockupStream(streamId, to, asset, amount);
     }
 }
diff --git a/src/interfaces/ISablierV2Lockup.sol b/src/interfaces/ISablierV2Lockup.sol
@@ -21,14 +21,16 @@ interface ISablierV2Lockup is
     /// @notice Emitted when a stream is canceled.
     /// @param streamId The id of the stream.
     /// @param sender The address of the stream's sender.
+    /// @param recipient The address of the stream's recipient.
     /// @param asset The contract address of the ERC-20 asset used for streaming.
     /// @param senderAmount The amount of assets refunded to the stream's sender, denoted in units of the asset's
     /// decimals.
     /// @param recipientAmount The amount of assets left for the stream's recipient to withdraw, denoted in units of the
     /// asset's decimals.
     event CancelLockupStream(
-        uint256 indexed streamId,
+        uint256 streamId,
         address indexed sender,
+        address indexed recipient,
         IERC20 indexed asset,
         uint128 senderAmount,
         uint128 recipientAmount
@@ -49,9 +51,9 @@ interface ISablierV2Lockup is
     /// @notice Emitted when assets are withdrawn from a stream.
     /// @param streamId The id of the stream.
     /// @param to The address that has received the withdrawn assets.
-    /// @param amount The amount of assets withdrawn, denoted in units of the asset's decimals.
     /// @param asset The contract address of the ERC-20 asset used for streaming.
-    event WithdrawFromLockupStream(uint256 indexed streamId, address indexed to, uint128 amount, IERC20 indexed asset);
+    /// @param amount The amount of assets withdrawn, denoted in units of the asset's decimals.
+    event WithdrawFromLockupStream(uint256 indexed streamId, address indexed to, IERC20 indexed asset, uint128 amount);
 
     /*//////////////////////////////////////////////////////////////////////////
                                  CONSTANT FUNCTIONS

diff --git a/test/fork/LockupDynamic.t.sol b/test/fork/LockupDynamic.t.sol
@@ -288,8 +288,8 @@ abstract contract LockupDynamic_Fork_Test is Fork_Test {
             emit WithdrawFromLockupStream({
                 streamId: vars.streamId,
                 to: params.recipient,
-                amount: params.withdrawAmount,
-                asset: ASSET
+                asset: ASSET,
+                amount: params.withdrawAmount
             });
             vm.expectEmit({ emitter: address(lockupDynamic) });
             emit MetadataUpdate({ _tokenId: vars.streamId });
@@ -351,7 +351,9 @@ abstract contract LockupDynamic_Fork_Test is Fork_Test {
             vm.expectEmit({ emitter: address(lockupDynamic) });
             vars.senderAmount = lockupDynamic.refundableAmountOf(vars.streamId);
             vars.recipientAmount = lockupDynamic.withdrawableAmountOf(vars.streamId);
-            emit CancelLockupStream(vars.streamId, params.sender, ASSET, vars.senderAmount, vars.recipientAmount);
+            emit CancelLockupStream(
+                vars.streamId, params.sender, params.recipient, ASSET, vars.senderAmount, vars.recipientAmount
+            );
             vm.expectEmit({ emitter: address(lockupDynamic) });
             emit MetadataUpdate({ _tokenId: vars.streamId });
 

diff --git a/test/fork/LockupLinear.t.sol b/test/fork/LockupLinear.t.sol
@@ -274,8 +274,8 @@ abstract contract LockupLinear_Fork_Test is Fork_Test {
             emit WithdrawFromLockupStream({
                 streamId: vars.streamId,
                 to: params.recipient,
-                amount: params.withdrawAmount,
-                asset: ASSET
+                asset: ASSET,
+                amount: params.withdrawAmount
             });
             vm.expectEmit({ emitter: address(lockupLinear) });
             emit MetadataUpdate({ _tokenId: vars.streamId });
@@ -335,7 +335,9 @@ abstract contract LockupLinear_Fork_Test is Fork_Test {
             vm.expectEmit({ emitter: address(lockupLinear) });
             vars.senderAmount = lockupLinear.refundableAmountOf(vars.streamId);
             vars.recipientAmount = lockupLinear.withdrawableAmountOf(vars.streamId);
-            emit CancelLockupStream(vars.streamId, params.sender, ASSET, vars.senderAmount, vars.recipientAmount);
+            emit CancelLockupStream(
+                vars.streamId, params.sender, params.recipient, ASSET, vars.senderAmount, vars.recipientAmount
+            );
             vm.expectEmit({ emitter: address(lockupLinear) });
             emit MetadataUpdate({ _tokenId: vars.streamId });
 

diff --git a/test/integration/concrete/lockup/cancel-multiple/cancelMultiple.t.sol b/test/integration/concrete/lockup/cancel-multiple/cancelMultiple.t.sol
@@ -80,7 +80,7 @@ abstract contract CancelMultiple_Integration_Concrete_Test is
         givenAllStreamsWarm
         whenCallerUnauthorized
     {
-        // Make the recipient the caller in this test.
+        // Make the Recipient the caller in this test.
         changePrank({ msgSender: users.recipient });
 
         // Run the test.
@@ -119,7 +119,7 @@ abstract contract CancelMultiple_Integration_Concrete_Test is
         givenAllStreamsWarm
         whenCallerUnauthorized
     {
-        // Make the recipient the caller in this test.
+        // Make the Recipient the caller in this test.
         changePrank({ msgSender: users.recipient });
 
         // Run the test.
@@ -181,6 +181,7 @@ abstract contract CancelMultiple_Integration_Concrete_Test is
         emit CancelLockupStream({
             streamId: testStreamIds[0],
             sender: users.sender,
+            recipient: users.recipient,
             asset: dai,
             senderAmount: senderAmount0,
             recipientAmount: defaults.DEPOSIT_AMOUNT() - senderAmount0
@@ -189,6 +190,7 @@ abstract contract CancelMultiple_Integration_Concrete_Test is
         emit CancelLockupStream({
             streamId: testStreamIds[1],
             sender: users.sender,
+            recipient: users.recipient,
             asset: dai,
             senderAmount: senderAmount1,
             recipientAmount: defaults.DEPOSIT_AMOUNT() - senderAmount1

diff --git a/test/integration/concrete/lockup/cancel/cancel.t.sol b/test/integration/concrete/lockup/cancel/cancel.t.sol
@@ -71,7 +71,7 @@ abstract contract Cancel_Integration_Concrete_Test is Integration_Test, Cancel_I
         givenStreamWarm
         whenCallerUnauthorized
     {
-        // Make recipient the caller in this test.
+        // Make the Recipient the caller in this test.
         changePrank({ msgSender: users.recipient });
 
         // Run the test.
@@ -255,7 +255,7 @@ abstract contract Cancel_Integration_Concrete_Test is Integration_Test, Cancel_I
 
         // Expect the relevant events to be emitted.
         vm.expectEmit({ emitter: address(lockup) });
-        emit CancelLockupStream(streamId, users.sender, dai, senderAmount, recipientAmount);
+        emit CancelLockupStream(streamId, users.sender, address(goodRecipient), dai, senderAmount, recipientAmount);
         vm.expectEmit({ emitter: address(lockup) });
         emit MetadataUpdate({ _tokenId: streamId });
 

diff --git a/test/integration/concrete/lockup/withdraw-multiple/withdrawMultiple.t.sol b/test/integration/concrete/lockup/withdraw-multiple/withdrawMultiple.t.sol
@@ -310,22 +310,22 @@ abstract contract WithdrawMultiple_Integration_Concrete_Test is
         emit WithdrawFromLockupStream({
             streamId: testStreamIds[0],
             to: users.recipient,
-            amount: testAmounts[0],
-            asset: dai
+            asset: dai,
+            amount: testAmounts[0]
         });
         vm.expectEmit({ emitter: address(lockup) });
         emit WithdrawFromLockupStream({
             streamId: testStreamIds[1],
             to: users.recipient,
-            amount: testAmounts[1],
-            asset: dai
+            asset: dai,
+            amount: testAmounts[1]
         });
         vm.expectEmit({ emitter: address(lockup) });
         emit WithdrawFromLockupStream({
             streamId: testStreamIds[2],
             to: users.recipient,
-            amount: testAmounts[2],
-            asset: dai
+            asset: dai,
+            amount: testAmounts[2]
         });
 
         // Make the withdrawals.

diff --git a/test/integration/concrete/lockup/withdraw/withdraw.t.sol b/test/integration/concrete/lockup/withdraw/withdraw.t.sol
@@ -299,8 +299,8 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr
         emit WithdrawFromLockupStream({
             streamId: defaultStreamId,
             to: users.recipient,
-            amount: withdrawAmount,
-            asset: dai
+            asset: dai,
+            amount: withdrawAmount
         });
 
         // Make the withdrawal.
@@ -499,8 +499,8 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr
         emit WithdrawFromLockupStream({
             streamId: streamId,
             to: address(goodRecipient),
-            amount: withdrawAmount,
-            asset: dai
+            asset: dai,
+            amount: withdrawAmount
         });
         vm.expectEmit({ emitter: address(lockup) });
         emit MetadataUpdate({ _tokenId: streamId });

diff --git a/test/integration/fuzz/lockup/cancel.t.sol b/test/integration/fuzz/lockup/cancel.t.sol
@@ -81,7 +81,7 @@ abstract contract Cancel_Integration_Fuzz_Test is Integration_Test, Cancel_Integ
         // Expect the relevant events to be emitted.
         uint128 recipientAmount = lockup.withdrawableAmountOf(streamId);
         vm.expectEmit({ emitter: address(lockup) });
-        emit CancelLockupStream(streamId, users.sender, dai, senderAmount, recipientAmount);
+        emit CancelLockupStream(streamId, users.sender, address(goodRecipient), dai, senderAmount, recipientAmount);
         vm.expectEmit({ emitter: address(lockup) });
         emit MetadataUpdate({ _tokenId: streamId });
 

diff --git a/test/integration/fuzz/lockup/cancelMultiple.t.sol b/test/integration/fuzz/lockup/cancelMultiple.t.sol
@@ -47,6 +47,7 @@ abstract contract CancelMultiple_Integration_Fuzz_Test is Integration_Test, Canc
         emit CancelLockupStream({
             streamId: streamIds[0],
             sender: users.sender,
+            recipient: users.recipient,
             asset: dai,
             senderAmount: senderAmount0,
             recipientAmount: defaults.DEPOSIT_AMOUNT() - senderAmount0
@@ -55,6 +56,7 @@ abstract contract CancelMultiple_Integration_Fuzz_Test is Integration_Test, Canc
         emit CancelLockupStream({
             streamId: streamIds[1],
             sender: users.sender,
+            recipient: users.recipient,
             asset: dai,
             senderAmount: senderAmount1,
             recipientAmount: defaults.DEPOSIT_AMOUNT() - senderAmount1

diff --git a/test/integration/fuzz/lockup/withdraw.t.sol b/test/integration/fuzz/lockup/withdraw.t.sol
@@ -88,7 +88,7 @@ abstract contract Withdraw_Integration_Fuzz_Test is Integration_Test, Withdraw_I
 
         // Expect the relevant events to be emitted.
         vm.expectEmit({ emitter: address(lockup) });
-        emit WithdrawFromLockupStream(defaultStreamId, to, withdrawAmount, dai);
+        emit WithdrawFromLockupStream(defaultStreamId, to, dai, withdrawAmount);
         vm.expectEmit({ emitter: address(lockup) });
         emit MetadataUpdate({ _tokenId: defaultStreamId });
 
@@ -152,7 +152,7 @@ abstract contract Withdraw_Integration_Fuzz_Test is Integration_Test, Withdraw_I
 
         // Expect the relevant events to be emitted.
         vm.expectEmit({ emitter: address(lockup) });
-        emit WithdrawFromLockupStream(defaultStreamId, to, withdrawAmount, dai);
+        emit WithdrawFromLockupStream(defaultStreamId, to, dai, withdrawAmount);
         vm.expectEmit({ emitter: address(lockup) });
         emit MetadataUpdate({ _tokenId: defaultStreamId });
 

diff --git a/test/integration/fuzz/lockup/withdrawMax.t.sol b/test/integration/fuzz/lockup/withdrawMax.t.sol
@@ -25,8 +25,8 @@ abstract contract WithdrawMax_Integration_Fuzz_Test is Integration_Test, Withdra
         emit WithdrawFromLockupStream({
             streamId: defaultStreamId,
             to: users.recipient,
-            amount: defaults.DEPOSIT_AMOUNT(),
-            asset: dai
+            asset: dai,
+            amount: defaults.DEPOSIT_AMOUNT()
         });
 
         // Make the max withdrawal.
@@ -69,8 +69,8 @@ abstract contract WithdrawMax_Integration_Fuzz_Test is Integration_Test, Withdra
         emit WithdrawFromLockupStream({
             streamId: defaultStreamId,
             to: users.recipient,
-            amount: withdrawAmount,
-            asset: dai
+            asset: dai,
+            amount: withdrawAmount
         });
 
         // Make the max withdrawal.

diff --git a/test/integration/fuzz/lockup/withdrawMaxAndTransfer.t.sol b/test/integration/fuzz/lockup/withdrawMaxAndTransfer.t.sol
@@ -45,8 +45,8 @@ abstract contract WithdrawMaxAndTransfer_Integration_Fuzz_Test is
             emit WithdrawFromLockupStream({
                 streamId: defaultStreamId,
                 to: users.recipient,
-                amount: withdrawAmount,
-                asset: dai
+                asset: dai,
+                amount: withdrawAmount
             });
         }
 

diff --git a/test/integration/fuzz/lockup/withdrawMultiple.t.sol b/test/integration/fuzz/lockup/withdrawMultiple.t.sol
@@ -64,9 +64,9 @@ abstract contract WithdrawMultiple_Integration_Fuzz_Test is
 
         // Expect the relevant events to be emitted.
         vm.expectEmit({ emitter: address(lockup) });
-        emit WithdrawFromLockupStream({ streamId: ongoingStreamId, to: to, amount: ongoingWithdrawAmount, asset: dai });
+        emit WithdrawFromLockupStream({ streamId: ongoingStreamId, to: to, asset: dai, amount: ongoingWithdrawAmount });
         vm.expectEmit({ emitter: address(lockup) });
-        emit WithdrawFromLockupStream({ streamId: settledStreamId, to: to, amount: settledWithdrawAmount, asset: dai });
+        emit WithdrawFromLockupStream({ streamId: settledStreamId, to: to, asset: dai, amount: settledWithdrawAmount });
 
         // Make the withdrawals.
         uint256[] memory streamIds = Solarray.uint256s(ongoingStreamId, settledStreamId);

diff --git a/test/utils/Events.sol b/test/utils/Events.sol
@@ -69,8 +69,9 @@ abstract contract Events {
     //////////////////////////////////////////////////////////////////////////*/
 
     event CancelLockupStream(
-        uint256 indexed streamId,
+        uint256 streamId,
         address indexed sender,
+        address indexed recipient,
         IERC20 indexed asset,
         uint128 senderAmount,
         uint128 recipientAmount
@@ -82,7 +83,7 @@ abstract contract Events {
         address indexed admin, ISablierV2NFTDescriptor oldNFTDescriptor, ISablierV2NFTDescriptor newNFTDescriptor
     );
 
-    event WithdrawFromLockupStream(uint256 indexed streamId, address indexed to, uint128 amount, IERC20 indexed asset);
+    event WithdrawFromLockupStream(uint256 indexed streamId, address indexed to, IERC20 indexed asset, uint128 amount);
 
     /*//////////////////////////////////////////////////////////////////////////
                              SABLIER-V2-LOCKUP-DYNAMIC